From: Lucas Stach <l.stach@pengutronix.de>
To: Eric Anholt <eric@anholt.net>, dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/v3d: Remove the bad signaled() implementation.
Date: Fri, 08 Jun 2018 12:21:05 +0200 [thread overview]
Message-ID: <1528453265.26356.10.camel@pengutronix.de> (raw)
In-Reply-To: <20180605190302.18279-2-eric@anholt.net>
Am Dienstag, den 05.06.2018, 12:03 -0700 schrieb Eric Anholt:
> Since our seqno value comes from a counter associated with the GPU
> ring, not the entity (aka client), they'll be completed out of order.
> There's actually no need for this code at all, since we don't have
> enable_signaling() and thus DMA_FENCE_SIGNALED_BIT will be set before
> we could be called.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/gpu/drm/v3d/v3d_drv.h | 1 -
> drivers/gpu/drm/v3d/v3d_fence.c | 13 ++++---------
> drivers/gpu/drm/v3d/v3d_gem.c | 7 ++-----
> drivers/gpu/drm/v3d/v3d_irq.c | 3 ---
> 4 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_drv.h
> b/drivers/gpu/drm/v3d/v3d_drv.h
> index 26005abd9c5d..f32ac8c98f37 100644
> --- a/drivers/gpu/drm/v3d/v3d_drv.h
> +++ b/drivers/gpu/drm/v3d/v3d_drv.h
> @@ -25,7 +25,6 @@ struct v3d_queue_state {
>
> u64 fence_context;
> u64 emit_seqno;
> - u64 finished_seqno;
> };
>
> struct v3d_dev {
> diff --git a/drivers/gpu/drm/v3d/v3d_fence.c
> b/drivers/gpu/drm/v3d/v3d_fence.c
> index 087d49c8cb12..bfe31a89668b 100644
> --- a/drivers/gpu/drm/v3d/v3d_fence.c
> +++ b/drivers/gpu/drm/v3d/v3d_fence.c
> @@ -40,19 +40,14 @@ static bool v3d_fence_enable_signaling(struct
> dma_fence *fence)
> return true;
> }
>
> -static bool v3d_fence_signaled(struct dma_fence *fence)
> -{
> - struct v3d_fence *f = to_v3d_fence(fence);
> - struct v3d_dev *v3d = to_v3d_dev(f->dev);
> -
> - return v3d->queue[f->queue].finished_seqno >= f->seqno;
> -}
> -
> const struct dma_fence_ops v3d_fence_ops = {
> .get_driver_name = v3d_fence_get_driver_name,
> .get_timeline_name = v3d_fence_get_timeline_name,
> .enable_signaling = v3d_fence_enable_signaling,
> - .signaled = v3d_fence_signaled,
> + /* Each of our fences gets signaled as complete by the IRQ
> + * handler, so we rely on the core's tracking of signaling.
> + */
> + .signaled = NULL,
> .wait = dma_fence_default_wait,
> .release = dma_fence_free,
> };
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c
> b/drivers/gpu/drm/v3d/v3d_gem.c
> index 9ea83bdb9a30..d06d6697e089 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -657,17 +657,14 @@ void
> v3d_gem_destroy(struct drm_device *dev)
> {
> struct v3d_dev *v3d = to_v3d_dev(dev);
> - enum v3d_queue q;
>
> v3d_sched_fini(v3d);
>
> /* Waiting for exec to finish would need to be done before
> * unregistering V3D.
> */
> - for (q = 0; q < V3D_MAX_QUEUES; q++) {
> - WARN_ON(v3d->queue[q].emit_seqno !=
> - v3d->queue[q].finished_seqno);
> - }
> + WARN_ON(v3d->bin_job);
> + WARN_ON(v3d->render_job);
>
> drm_mm_takedown(&v3d->mm);
>
> diff --git a/drivers/gpu/drm/v3d/v3d_irq.c
> b/drivers/gpu/drm/v3d/v3d_irq.c
> index 77e1fa046c10..e07514eb11b5 100644
> --- a/drivers/gpu/drm/v3d/v3d_irq.c
> +++ b/drivers/gpu/drm/v3d/v3d_irq.c
> @@ -87,15 +87,12 @@ v3d_irq(int irq, void *arg)
> }
>
> if (intsts & V3D_INT_FLDONE) {
> - v3d->queue[V3D_BIN].finished_seqno++;
> dma_fence_signal(v3d->bin_job->bin.done_fence);
> status = IRQ_HANDLED;
> }
>
> if (intsts & V3D_INT_FRDONE) {
> - v3d->queue[V3D_RENDER].finished_seqno++;
> dma_fence_signal(v3d->render_job-
> >render.done_fence);
> -
> status = IRQ_HANDLED;
> }
>
next prev parent reply other threads:[~2018-06-08 10:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-05 19:03 [PATCH 1/3] drm/v3d: Take a lock across GPU scheduler job creation and queuing Eric Anholt
2018-06-05 19:03 ` [PATCH 2/3] drm/v3d: Remove the bad signaled() implementation Eric Anholt
2018-06-08 10:21 ` Lucas Stach [this message]
2018-06-05 19:03 ` [PATCH 3/3] drm/v3d: Add a note about locking of v3d_fence_create() Eric Anholt
2018-06-08 10:24 ` Lucas Stach
2018-06-08 17:08 ` Eric Anholt
2018-06-06 8:46 ` [PATCH 1/3] drm/v3d: Take a lock across GPU scheduler job creation and queuing Lucas Stach
2018-06-06 8:52 ` Christian König
2018-06-06 17:48 ` [PATCH 1/3 v2] " Eric Anholt
2018-06-07 8:37 ` Lucas Stach
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=1528453265.26356.10.camel@pengutronix.de \
--to=l.stach@pengutronix.de \
--cc=amd-gfx@lists.freedesktop.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=eric@anholt.net \
--cc=linux-kernel@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).