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

[Ville Syrjälä]

On Fri, Dec 04, 2020 at 04:14:05PM +0000, Chris Wilson wrote:
> 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?

One can hope at least.

>  
> > 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

-- 
Ville Syrjälä
Intel