Re: [PATCH v2 4/4] drm/i915/gen9: Fix runtime PM refcounting in case DMC firmware isn't loaded

[Imre Deak <imre.deak@intel.com>]

On ke, 2016-04-20 at 15:02 +0200, Daniel Vetter wrote:
> On Mon, Apr 18, 2016 at 02:48:21PM +0300, Imre Deak wrote:
> > While we disable runtime PM and with that display power well support if
> > the DMC firmware isn't loaded, we still want to disable power wells
> > during system suspend and driver unload. So drop/reacquire the
> > corresponding power refcount during suspend/resume and driver unloading.
> > This also means we have to check if DMC is not loaded and skip enabling
> > DC states in the power well code.
> > 
> > v2:
> > - Reuse intel_csr_ucode_suspend() in intel_csr_ucode_fini() instead of
> >   opencoding the former. (Chris)
> > - Add docbook comment to the public resume and suspend functions.
> > 
> > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > 
> > ---
> > 
> > As this fixes the special case when firmware loading failed with the
> > result of possibly leaving power wells enabled during suspend/resume and
> > driver unloading it's not for -fixes or stable.
> 
> Do we even care about this to bother fixing it? We pretty much agreed that
> we need dmc to make power management work on skl, so if it doesn't work
> too well over suspend/resume (soix), meh.

It's possible that someone runs w/o the firmware, either because of not
having the latest version for a given kernel or on purpose disabling
DMC functionality (for example for debugging). We want to keep things
working in this case too. And it's also the right thing to do during
driver unload, we'd leak the RPM reference otherwise.

> And your patch here does make the suspend/resume code even more fragile
> and filled with special cases, so not too sure we should apply this one.

It's the same case for all DMC platforms.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx