[Intel-gfx] [PATCH 7/7] drm/i915/uc: Don't forget to prepare GuC for the reset

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-17 17:54:53)
> On Fri, 17 May 2019 18:27:44 +0200, Chris Wilson  
> <chris at chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2019-05-17 17:22:27)
> >> When we reset engines using ALL_ENGINES mask, we will do
> >> full GPU reset and GuC will be also affected. Let GuC be
> >> prepared for upcoming reset.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/gt/intel_reset.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c  
> >> b/drivers/gpu/drm/i915/gt/intel_reset.c
> >> index 464369bc55ad..ca6e40b6b4e2 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> >> @@ -564,6 +564,10 @@ static int gen8_reset_engines(struct  
> >> drm_i915_private *i915,
> >>                  */
> >>         }
> >>
> >> +       /* We are about to do full GPU reset, don't forget about GuC */
> >> +       if (engine_mask == ALL_ENGINES)
> >> +               intel_uc_reset_prepare(i915);
> >
> > Eh, this is done in reset_prepare already. The only other path to call
> > intel_gpu_reset() directly is along sanitization, which should also have
> > already sanitized the guc as well. No?
> 
> There is igt_atomic_reset selftest which does not call reset_prepare.
> And since we lost GuC in gen6_reset_engines due to GEN6_GRDOM_FULL,
> our later graceful goodbye with GuC was not working.

(Apologies if resent.)

Ah, so the intent there was to prevent sleeps slipping into the
device-level reset (so that we could pull it under a heavy hammer like
stop_machine() to serialise the reset with direct user access). At one
point, I hoped we could make i915_reset() completely fast and lockless,
but that was a fleeting fancy.

It looks reasonable to put that under a reset_prepare/reset_finish and
keep the test semantics. That would also mean we have to move it out of
the increasingly misnamed selftest_hangcheck.c into a selftest_reset.c
-Chris