LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	"Rob Clark" <robdclark@chromium.org>,
	"open list:DRM DRIVER FOR MSM ADRENO GPU"
	<freedreno@lists.freedesktop.org>,
	"David Airlie" <airlied@linux.ie>,
	"open list:DRM DRIVER FOR MSM ADRENO GPU"
	<linux-arm-msm@vger.kernel.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"Christian König" <christian.koenig@amd.com>,
	"moderated list:DMA BUFFER SHARING FRAMEWORK"
	<linaro-mm-sig@lists.linaro.org>,
	"Daniel Vetter" <daniel@ffwll.ch>, "Sean Paul" <sean@poorly.run>,
	"open list:DMA BUFFER SHARING FRAMEWORK"
	<linux-media@vger.kernel.org>
Subject: Re: [Linaro-mm-sig] [PATCH] drm/msm: Add fence->wait() op
Date: Tue, 20 Jul 2021 11:30:53 -0700	[thread overview]
Message-ID: <CAF6AEGtOW3EjZWo36ij8U1om=gAqvg8CSkJJq2GkyHFGWUH4kQ@mail.gmail.com> (raw)
In-Reply-To: <60ffb6f3-e932-d9af-3b90-81adf0c15250@gmail.com>

On Tue, Jul 20, 2021 at 11:03 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Hi Rob,
>
> Am 20.07.21 um 17:07 schrieb Rob Clark:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Somehow we had neither ->wait() nor dma_fence_signal() calls, and no
> > one noticed.  Oops.
>
>
> I'm not sure if that is a good idea.
>
> The dma_fence->wait() callback is pretty much deprecated and should not
> be used any more.
>
> What exactly do you need that for?

Well, the alternative is to track the set of fences which have
signalling enabled, and then figure out which ones to signal, which
seems like a lot more work, vs just re-purposing the wait
implementation we already have for non-dma_fence cases ;-)

Why is the ->wait() callback (pretty much) deprecated?

BR,
-R

> Regards,
> Christian.
>
> >
> > Note that this removes the !timeout case, which has not been used in
> > a long time.
>
>
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >   drivers/gpu/drm/msm/msm_fence.c | 59 +++++++++++++++++++--------------
> >   1 file changed, 34 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
> > index cd59a5918038..8ee96b90ded6 100644
> > --- a/drivers/gpu/drm/msm/msm_fence.c
> > +++ b/drivers/gpu/drm/msm/msm_fence.c
> > @@ -38,11 +38,10 @@ static inline bool fence_completed(struct msm_fence_context *fctx, uint32_t fenc
> >       return (int32_t)(fctx->completed_fence - fence) >= 0;
> >   }
> >
> > -/* legacy path for WAIT_FENCE ioctl: */
> > -int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
> > -             ktime_t *timeout, bool interruptible)
> > +static signed long wait_fence(struct msm_fence_context *fctx, uint32_t fence,
> > +             signed long remaining_jiffies, bool interruptible)
> >   {
> > -     int ret;
> > +     signed long ret;
> >
> >       if (fence > fctx->last_fence) {
> >               DRM_ERROR_RATELIMITED("%s: waiting on invalid fence: %u (of %u)\n",
> > @@ -50,33 +49,34 @@ int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
> >               return -EINVAL;
> >       }
> >
> > -     if (!timeout) {
> > -             /* no-wait: */
> > -             ret = fence_completed(fctx, fence) ? 0 : -EBUSY;
> > +     if (interruptible) {
> > +             ret = wait_event_interruptible_timeout(fctx->event,
> > +                     fence_completed(fctx, fence),
> > +                     remaining_jiffies);
> >       } else {
> > -             unsigned long remaining_jiffies = timeout_to_jiffies(timeout);
> > -
> > -             if (interruptible)
> > -                     ret = wait_event_interruptible_timeout(fctx->event,
> > -                             fence_completed(fctx, fence),
> > -                             remaining_jiffies);
> > -             else
> > -                     ret = wait_event_timeout(fctx->event,
> > -                             fence_completed(fctx, fence),
> > -                             remaining_jiffies);
> > -
> > -             if (ret == 0) {
> > -                     DBG("timeout waiting for fence: %u (completed: %u)",
> > -                                     fence, fctx->completed_fence);
> > -                     ret = -ETIMEDOUT;
> > -             } else if (ret != -ERESTARTSYS) {
> > -                     ret = 0;
> > -             }
> > +             ret = wait_event_timeout(fctx->event,
> > +                     fence_completed(fctx, fence),
> > +                     remaining_jiffies);
> > +     }
> > +
> > +     if (ret == 0) {
> > +             DBG("timeout waiting for fence: %u (completed: %u)",
> > +                             fence, fctx->completed_fence);
> > +             ret = -ETIMEDOUT;
> > +     } else if (ret != -ERESTARTSYS) {
> > +             ret = 0;
> >       }
> >
> >       return ret;
> >   }
> >
> > +/* legacy path for WAIT_FENCE ioctl: */
> > +int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
> > +             ktime_t *timeout, bool interruptible)
> > +{
> > +     return wait_fence(fctx, fence, timeout_to_jiffies(timeout), interruptible);
> > +}
> > +
> >   /* called from workqueue */
> >   void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence)
> >   {
> > @@ -114,10 +114,19 @@ static bool msm_fence_signaled(struct dma_fence *fence)
> >       return fence_completed(f->fctx, f->base.seqno);
> >   }
> >
> > +static signed long msm_fence_wait(struct dma_fence *fence, bool intr,
> > +             signed long timeout)
> > +{
> > +     struct msm_fence *f = to_msm_fence(fence);
> > +
> > +     return wait_fence(f->fctx, fence->seqno, timeout, intr);
> > +}
> > +
> >   static const struct dma_fence_ops msm_fence_ops = {
> >       .get_driver_name = msm_fence_get_driver_name,
> >       .get_timeline_name = msm_fence_get_timeline_name,
> >       .signaled = msm_fence_signaled,
> > +     .wait = msm_fence_wait,
> >   };
> >
> >   struct dma_fence *
>

  reply	other threads:[~2021-07-20 18:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-20 15:07 Rob Clark
2021-07-20 18:03 ` [Linaro-mm-sig] " Christian König
2021-07-20 18:30   ` Rob Clark [this message]
2021-07-20 20:55     ` Daniel Vetter
2021-07-20 22:36       ` Rob Clark
2021-07-21  7:59         ` Daniel Vetter
2021-07-21 16:34           ` Rob Clark
2021-07-21 19:03             ` Daniel Vetter
2021-07-22  8:42               ` Christian König
2021-07-22  9:08                 ` Daniel Vetter
2021-07-22  9:28                   ` Christian König
2021-07-22 10:47                     ` Daniel Vetter
2021-07-22 11:33                       ` Christian König
2021-07-22 15:46                     ` Rob Clark
2021-07-22 15:40                 ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAF6AEGtOW3EjZWo36ij8U1om=gAqvg8CSkJJq2GkyHFGWUH4kQ@mail.gmail.com' \
    --to=robdclark@gmail.com \
    --cc=airlied@linux.ie \
    --cc=christian.koenig@amd.com \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=robdclark@chromium.org \
    --cc=sean@poorly.run \
    --subject='Re: [Linaro-mm-sig] [PATCH] drm/msm: Add fence->wait() op' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).