On Wed, Apr 20, 2016 at 04:13:25PM +0300, Imre Deak wrote: > 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@xxxxxxxxxxxxxxxxxx> > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > > > --- > > > > > > 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. Ok, if this is needed anyway for driver unload I think it's ok. But otherwise keeping stuff working without DMC is imo a fringe use case we shouldn't bother about. > > 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. I more meant that there's not yet another little bit we need to do at exactly the right time in an already massive sequence of events. It certainly doesn't make suspend/resume simpler ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
