| Message ID | 20210117110418.3361-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Add arbitration check before semaphore wait | expand |
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
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; }
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(+)