On Mon, Dec 12, 2022 at 05:13:38PM +0100, Andi Shyti wrote: > From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > After applying an engine reset, on some platforms like Jasperlake, we > occasionally detect that the engine state is not cleared until shortly > after the resume. As we try to resume the engine with volatile internal > state, the first request fails with a spurious CS event (it looks like > it reports a lite-restore to the hung context, instead of the expected > idle->active context switch). > > Signed-off-by: Chris Wilson <hris@xxxxxxxxxxxxxxxxxx> There's a typo in the signature email I'm afraid... Other than that, have we checked the possibility of using the driver-initiated-flr bit instead of this second loop? That should be the right way to guarantee everything is cleared on gen11+... > Cc: stable@xxxxxxxxxxxxxxx > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> > Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/gt/intel_reset.c | 34 ++++++++++++++++++++++----- > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > index ffde89c5835a4..88dfc0c5316ff 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > @@ -268,6 +268,7 @@ static int ilk_do_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask, > static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) > { > struct intel_uncore *uncore = gt->uncore; > + int loops = 2; > int err; > > /* > @@ -275,18 +276,39 @@ static int gen6_hw_domain_reset(struct intel_gt *gt, u32 hw_domain_mask) > * for fifo space for the write or forcewake the chip for > * the read > */ > - intel_uncore_write_fw(uncore, GEN6_GDRST, hw_domain_mask); > + do { > + intel_uncore_write_fw(uncore, GEN6_GDRST, hw_domain_mask); > > - /* Wait for the device to ack the reset requests */ > - err = __intel_wait_for_register_fw(uncore, > - GEN6_GDRST, hw_domain_mask, 0, > - 500, 0, > - NULL); > + /* > + * Wait for the device to ack the reset requests. > + * > + * On some platforms, e.g. Jasperlake, we see see that the > + * engine register state is not cleared until shortly after > + * GDRST reports completion, causing a failure as we try > + * to immediately resume while the internal state is still > + * in flux. If we immediately repeat the reset, the second > + * reset appears to serialise with the first, and since > + * it is a no-op, the registers should retain their reset > + * value. However, there is still a concern that upon > + * leaving the second reset, the internal engine state > + * is still in flux and not ready for resuming. > + */ > + err = __intel_wait_for_register_fw(uncore, GEN6_GDRST, > + hw_domain_mask, 0, > + 2000, 0, > + NULL); > + } while (err == 0 && --loops); > if (err) > GT_TRACE(gt, > "Wait for 0x%08x engines reset failed\n", > hw_domain_mask); > > + /* > + * As we have observed that the engine state is still volatile > + * after GDRST is acked, impose a small delay to let everything settle. > + */ > + udelay(50); > + > return err; > } > > -- > 2.38.1 >