On Tue, Apr 25, 2017 at 11:55:07AM +0300, Imre Deak wrote: > On Tue, Apr 25, 2017 at 08:21:49AM +0200, Lukas Wunner wrote: > > On Mon, Apr 24, 2017 at 10:56:40PM +0200, Rafael J. Wysocki wrote: > > > On Monday, April 24, 2017 10:42:42 PM Lukas Wunner wrote: > > > > On Mon, Apr 24, 2017 at 10:02:30PM +0200, Lukas Wunner wrote: > > > > > On Mon, Apr 24, 2017 at 05:27:42PM +0300, Imre Deak wrote: > > > > > > Some drivers - like i915 - may not support the system suspend direct > > > > > > complete optimization due to differences in their runtime and system > > > > > > suspend sequence. Add a flag that when set resumes the device before > > > > > > calling the driver's system suspend handlers which effectively disables > > > > > > the optimization. > > > > > > > > > > FWIW, there are at least two alternative solutions to this problem which > > > > > do not require changes to the PCI core: > > > > > > > > > > (1) Add a ->prepare hook to i915_pm_ops which calls pm_runtime_get_sync() > > > > > and a ->complete hook which calls pm_runtime_put(). > > > > > > > > Thinking a bit more about this, it's even simpler: The PM core acquires > > > > a runtime PM ref in device_prepare() and releases it in device_complete(), > > > > so it's sufficient to just call pm_runtime_resume() in a ->prepare hook > > > > that's newly added to i915. No ->complete hook necessary. Tentative > > > > patch below, based on drm-intel-fixes, would replace both of your patches. > > > > > > Calling it in ->prepare() means that everybody is now waiting for you to > > > resume. > > > > > > Not quite optimal IMO. > > > > Okay, understood. > > > > However in the absence of a device_link from the HDA device (consumer) > > to the GPU (supplier), there's no guarantee that the GPU is resumed > > when the HDA device needs it due to the asynchronous invocation of > > the ->suspend hooks. This is assuming that the HDA device already > > needs the GPU during ->suspend phase. > > i915 has ->resume_early and ->suspend_late hooks that provides the above > guarantee. Employing device links is preferable IMO: * Less code. Just find the HDA device in your ->probe hook and call device_link_add(). Alternatively do it from the HDA driver. Even adding the link from both drivers should be fine. I can provide you with an evaluation patch if you'd like. * Avoidance of doing things behind the PM core's back. * Takes care of more stuff than just system sleep (e.g. shutdown ordering). Thanks, Lukas
