Re: [PATCH v2] drm/i915: Break i915_spin_request() if we see an interrupt

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

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

> If an interrupt has been posted, and we were spinning on the active
> seqno waiting for it to advance but it did not, then we can expect that
> it will not see its advance in the immediate future and should call into
> the irq-seqno barrier. We can stop spinning at this point, and leave the
> difficulty of handling the coherency to the caller.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 7760d7481f85..de52ac18e215 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -972,7 +972,8 @@ static bool busywait_stop(unsigned long timeout, unsigned int cpu)
>  bool __i915_spin_request(const struct drm_i915_gem_request *req,
>  			 u32 seqno, int state, unsigned long timeout_us)
>  {
> -	unsigned int cpu;
> +	struct intel_engine_cs *engine = req->engine;
> +	unsigned int irq, cpu;
>  
>  	/* When waiting for high frequency requests, e.g. during synchronous
>  	 * rendering split between the CPU and GPU, the finite amount of time
> @@ -984,15 +985,23 @@ bool __i915_spin_request(const struct drm_i915_gem_request *req,
>  	 * takes to sleep on a request, on the order of a microsecond.
>  	 */
>  
> +	irq = atomic_read(&engine->irq_count);
>  	timeout_us += local_clock_us(&cpu);
>  	do {
>  		if (seqno != i915_gem_request_global_seqno(req))
>  			break;
>  
> -		if (i915_seqno_passed(intel_engine_get_seqno(req->engine),
> -				      seqno))
> +		if (i915_seqno_passed(intel_engine_get_seqno(engine), seqno))
>  			return true;
>  
> +		/* Seqno are meant to be ordered *before* the interrupt. If
> +		 * we see an interrupt without a corresponding seqno advance,
> +		 * assume we won't see one in the near future but require
> +		 * the engine->seqno_barrier() to fixup coherency.
> +		 */
> +		if (atomic_read(&engine->irq_count) != irq)
> +			break;
> +

Looks good but now need to wait for patch to introduce
irq_counts to materialize.

-Mika

>  		if (signal_pending_state(state, current))
>  			break;
>  
> -- 
> 2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx