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
>
