Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

[Kevin Hilman <khilman@linaro.org>]

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> On Friday, May 09, 2014 03:48:21 PM Kevin Hilman wrote:
>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>> 
>> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> >
>> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
>> > resume all runtime-suspended devices during system suspend, mostly
>> > because those devices may need to be reprogrammed due to different
>> > wakeup settings for system sleep and for runtime PM.
>> >
>> > For some devices, though, it's OK to remain in runtime suspend 
>> > throughout a complete system suspend/resume cycle (if the device was in
>> > runtime suspend at the start of the cycle).  We would like to do this
>> > whenever possible, to avoid the overhead of extra power-up and power-down
>> > events.
>> >
>> > However, problems may arise because the device's descendants may require
>> > it to be at full power at various points during the cycle.  Therefore the
>> > most straightforward way to do this safely is if the device and all its
>> > descendants can remain runtime suspended until the resume stage of system
>> > resume.
>> >
>> > To this end, introduce dev->power.leave_runtime_suspended.
>> > If a subsystem or driver sets this flag during the ->prepare() callback,
>> > and if the flag is set in all of the device's descendants, and if the
>> > device is still in runtime suspend at the beginning of the ->suspend()
>> > callback, that callback is allowed to return 0 without clearing
>> > power.leave_runtime_suspended and without changing the state of the
>> > device, unless the current state of the device is not appropriate for
>> > the upcoming system sleep state (for example, the device is supposed to
>> > wake up the system from that state and its current wakeup settings are
>> > not suitable for that).  Then, the PM core will not invoke the device's
>> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
>> > ->resume() callbacks.  
>> 
>> Up to here, this sounds great.
>> 
>> > Instead, it will invoke ->runtime_resume() during the device resume
>> > stage of system resume.
>> 
>> But this part I'm not fully following...
>
> You're not looking at the most recent one. :-)

Sorry about that, I haven't been able to keep up with the versions.

> Please look here: https://patchwork.kernel.org/patch/4139181/

OK.  

>> > By leaving this flag set after ->suspend(), a driver or subsystem tells
>> > the PM core that the device is runtime suspended, it is in a suitable
>> > state for system suspend (for example, the wakeup setting does not
>> > need to be changed), and it does not need to return to full
>> > power until the resume stage.
>> 
>> But taking this "leave runtime suspended" idea the next logical step,
>> why would/should a device need to return to full power at the ->resume()
>> stage?  especially when it wasn't at full power when ->suspend()
>> happened?
>
> Good question and I've been thinking about that for a while.
>
> Generally, the main reason for resuming is that on some platforms devices are
> automatically powered up by firmware and in those cases it's better to
> resume them (to make the runtime PM status reflect the physical state) and
> suspend again later.
>
> Generally speaking, subsystems that need to do that know what they are and
> that's what I was talking about in the most recent reply to Alan:
>
> http://marc.info/?l=linux-pm&m=139967477806094&w=4
>
> Currently, I think, there are two options on the table really.
>
>  1. Do more or less what https://patchwork.kernel.org/patch/4139181/ does
>     with a modification to check that ->suspend() doesn't "cheat" (by setting
>     the flag that had been unset before it was called).  The subsystem's
>     ->resume() would then decide what to do with the device (resume it or
>     leave it suspended).
>
>  2. Do what Alan was suggesting, that is set the flag in ->prepare() and
>     make the PM core skip *all* of the system suspend/resume callbacks
>     for devices with that flag set and let the ->complete() callback
>     decide what to do with the device.
>
> I'm leaning a bit towards 2, but still considering 1 too.

If it matters, I have a slight preference for 2 also, though as long as
the subsytem/device gets to decide whether to resume, I think I'm OK
with either approach.

Kevin