Re: [PATCH 2/3] drm/v3d: Remove the bad signaled() implementation.

From: Lucas Stach
Date: Fri Jun 08 2018 - 06:21:19 EST


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@xxxxxxxxxx>

Reviewed-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>

> ---
> Â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;
> Â }
> Â