On Tue, Apr 11, 2017 at 05:54:07PM +0100, Chris Wilson wrote: > On Tue, Apr 11, 2017 at 07:12:35PM +0300, Imre Deak wrote: > > +static int i915_pm_prepare(struct device *kdev) > > +{ > > + /* > > + * Get a reference to disable the direct complete optimization. This > > + * is needed, since we have a suspend sequence specific to system > > + * suspend (that is different from runtime suspend) and because we > > + * need to provide power to the sound driver while its system suspend > > + * handler is running. This is not possible with the optimization in > > + * effect, when the i915 runtime PM is disabled for the whole duration > > + * of the suspend sequence if the device was already runtime > > + * suspended at the beginning of the sequence. In this case the i915 > > + * suspend/resume hooks would be also skipped (besides its prepare and > > + * complete hooks). > > + */ > > + intel_runtime_pm_get(kdev_to_i915(kdev)); > > + > > + return 0; > > +} > > + > > +static void i915_pm_complete(struct device *kdev) > > +{ > > + /* Put the ref taken in the prepare step. */ > > + intel_runtime_pm_put(kdev_to_i915(kdev)); > > Do we always call i915_pm_complete() if any of the post-prepare suspend > steps fail? Otherwise, it looks very sensible from our pov. Yes, it's called even in the failure case (for S3 for example suspend_devices_and_enter()->Recover_platform:->Resume_devices:-> dpm_resume_end()->dpm_complete()). --Imre