On Tue, Apr 11, 2017 at 08:09:17PM +0300, Imre Deak wrote: > 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()). >From our pov, this is ok. Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> And we can apply this whilst we investigate whether this is the best approach to avoiding the suspend optimization. -Chris -- Chris Wilson, Intel Open Source Technology Centre