Re: [Intel-gfx] [PATCH] drm/i915/gt: Add arbitration check before semaphore wait

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 17/01/2021 11:04, Chris Wilson wrote:
> Similar to commit 49b20dbf7497 ("drm/i915/gt: Perform an arbitration
> check before busywaiting"), also add a check prior to the busywait
> on gen8+, as we have now seen (because we added a selftest to add fault
> injection into the engine resets) the same engine reset failure leading
> to an indefinite wait on the ring-stop semaphore. So not a Tigerlake
> specific bug after all, though it still seems odd behaviour for the
> busywait as we do get the arbitration point elsewhere on a miss.
> 
> Testcase: igt_reset_fail_engine
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 5b932d2dbfd3..07ba524da90b 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -488,6 +488,7 @@ static u32 *gen8_emit_wa_tail(struct i915_request *rq, u32 *cs)
>   
>   static u32 *emit_preempt_busywait(struct i915_request *rq, u32 *cs)
>   {
> +	*cs++ = MI_ARB_CHECK; /* trigger IDLE->ACTIVE first */
>   	*cs++ = MI_SEMAPHORE_WAIT |
>   		MI_SEMAPHORE_GLOBAL_GTT |
>   		MI_SEMAPHORE_POLL |
> @@ -495,6 +496,7 @@ static u32 *emit_preempt_busywait(struct i915_request *rq, u32 *cs)
>   	*cs++ = 0;
>   	*cs++ = preempt_address(rq->engine);
>   	*cs++ = 0;
> +	*cs++ = MI_NOOP;
>   
>   	return cs;
>   }
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx