[Intel-gfx] [PATCH] drm/i915: Prevent the system suspend complete optimization

Rafael J. Wysocki rafael.j.wysocki at intel.com
Thu Apr 13 02:29:41 UTC 2017


Hi,

First off, sorry for introducing the problem and thanks for taking care 
of it!

On 4/11/2017 7:09 PM, 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()).

->prepare and ->complete are not the most friendly places to do these 
things, though, because then the whole kernel needs to wait for them to 
return and if they take time, it goes ugly.

Have you considered adding a need_resume flag to struct pci_dev, setting 
it for i915 and checking it along with platform_pci_need_resume() in 
pci_dev_keep_suspended()?

Thanks,
Rafael



More information about the Intel-gfx mailing list