[Intel-gfx] [PATCH v2] drm/i915/gt: Allow clobbering gpu resets if the display is off

[Chris Wilson]

Quoting Ville Syrjälä (2019-09-11 11:52:29)
> On Wed, Sep 11, 2019 at 11:19:59AM +0100, Chris Wilson wrote:
> > If the display is inactive, we need not worry about the gpu reset
> > clobbering the display! To prevent the display changing state between us
> > checking the active state and doing the hard reset, we introduce the
> > lightweight reset lock to the atomic commit for the affected (legacy)
> > platforms.
> > 
> > Testcase: igt/gem_eio/kms
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++++++
> >  drivers/gpu/drm/i915/gt/intel_reset.c        | 18 +++++++++++++++++-
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 4ee750fa3ef0..a92487d8f4cf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13986,6 +13986,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >       struct intel_crtc *crtc;
> >       u64 put_domains[I915_MAX_PIPES] = {};
> >       intel_wakeref_t wakeref = 0;
> > +     int srcu;
> >       int i;
> >  
> >       intel_atomic_commit_fence_wait(state);
> > @@ -14005,6 +14006,12 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >               }
> >       }
> >  
> > +     /* Prevent starting a GPU reset while we change global display state */
> > +     srcu = -ENODEV;
> > +     if (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display)
> > +             /* only fails if interrupted */
> > +             srcu = intel_gt_reset_trylock(&dev_priv->gt);
> 
> Hmm. What happens if we're holding the modeset locks and there's an
> ongoing reset blocked on said locks?

I was about to say it's the inner lock in both cases, but it's not.
Anyway doesn't that interaction (intel_prepare_reset) prevent
crtc->active changing as we use it from inside intel_gt_reset (which
calls intel_gt_unset_wedged)?
-Chris