[Intel-gfx] [PATCH] drm/i915: Disable outputs during unregister

[Chris Wilson]

Quoting Ville Syrjälä (2020-12-04 16:01:11)
> On Tue, Dec 01, 2020 at 10:38:57PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2020-12-01 16:05:17)
> > > On Fri, Nov 27, 2020 at 10:05:48PM +0000, Chris Wilson wrote:
> > > > Switch off the scanout during driver unregister, so we can shutdown the
> > > > HW immediately for unbind.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 320856b665a1..62d188e5cb8d 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -738,6 +738,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
> > > >        * events.
> > > >        */
> > > >       drm_kms_helper_poll_fini(&dev_priv->drm);
> > > > +     drm_atomic_helper_shutdown(&dev_priv->drm);
> > > 
> > > Looks like we already have this in remove(). Is that too late?
> > 
> > For the operations we do during unbind, yes.
> > 
> > For the core_hotplug/rebind dance, we have to reset the GPU while we
> > still have runtime-pm operational and have pushed the reset to
> > unregister (from experimentation that's as late as we can put it where
> > the GPU works after rebinding and we don't corrupt the system on unbind,
> > with the current hooks). You can guess how well gen3 likes that.
> > 
> > But I don't think the right answer is to skip the reset for gen3.
> > Suppose we enable context support for gen3, then the reset would be
> > required as well, and so we would still need the whole display
> > shenanigans to turn it off. Moving the modeset to turn the display off
> > to the end of userspace seems reasonable.
> 
> Yeah, just a bit odd to have the same call twice in the
> sequence. Can we remove the second call at least?

I think we can, but I am sufficiently paranoid to leave it.
I presume if it is a no-op, it will return without touching HW?
 
> Also a bit annoying the unload sequence no longer matches the
> suspend sequence. Well, I guess it was never 100% anyway but
> I think it was a bit closer before this patch. But the whole
> thing is rather messy anyway so I guess t's not significantly
> worse after this.

Yes, I feel things have been thrown into a bit of disarray by
haphazardly fixing unbind.

The last* remaining fly in the ointment is rebinding iommu. Once we have
that solid (and the system stops randomly eating itself 1-10 minutes
after the test passes), we should be in a much better spot to safely
remove duplication and refine the flow.

* that I am aware of.
-Chris