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

[Lucas Stach]

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