Re: [PATCH] drm/i915: Prevent use of global_seqno=0

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> We are not allowed to assign rq->global_seqno=0 as it has a special
> meaning of "inactive" (not executing on HW).
>
> Fixes: 6faf5916e6be ("drm/i915: Remove HW semaphores for gen7 inter-engine synchronisation")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_request.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 5403d4e2cee0..5e178f5ac18b 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -343,6 +343,13 @@ static void move_to_timeline(struct i915_request *request,
>  	spin_unlock(&request->timeline->lock);
>  }
>  
> +static u32 next_global_seqno(struct i915_timeline *tl)
> +{
> +	if (!++tl->seqno)
> +		++tl->seqno;
> +	return tl->seqno;
> +}
> +
>  void __i915_request_submit(struct i915_request *request)
>  {
>  	struct intel_engine_cs *engine = request->engine;
> @@ -359,7 +366,7 @@ void __i915_request_submit(struct i915_request *request)
>  
>  	GEM_BUG_ON(request->global_seqno);
>  
> -	seqno = timeline_get_seqno(&engine->timeline);
> +	seqno = next_global_seqno(&engine->timeline);

Does it matter that we will allow dma fence to be
with seqno zero?

In other words, if we keep the global seqnos and timeline
seqnos 'type similar' for readability reasons, should
we enforce the 'not zero' in here too.

Dma fence code seems to handle 32bits seqnos
even tho the type is 64bit wide.

-Mika

>  	GEM_BUG_ON(!seqno);
>  	GEM_BUG_ON(intel_engine_signaled(engine, seqno));
>  
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx