Drivers taking different actions depending on sleep state

[Florian Fainelli]

On 06/21/2017 02:59 PM, Rafael J. Wysocki wrote:
> On Wed, Jun 21, 2017 at 11:16 PM, Florian Fainelli <f.fainelli at gmail.com> wrote:
>> On 06/09/2017 03:53 PM, Rafael J. Wysocki wrote:
>>> Hi,
>>>
>>> On Fri, Jun 9, 2017 at 5:20 PM, Mason <slash.tmp at free.fr> wrote:
>>>> Hello,
>>>>
>>>> I read the "Sleep States" documentation:
>>>> https://www.kernel.org/doc/Documentation/power/states.txt
>>>>
>>>> It mentions /sys/power/mem_sleep but I don't have that in 4.9
>>>> # ll /sys/power/
>>>> -rw-r--r--    1 root     root          4096 Jan  1 00:31 pm_async
>>>> -rw-r--r--    1 root     root          4096 Jan  1 00:31 pm_freeze_timeout
>>>> -rw-r--r--    1 root     root          4096 Jan  1 00:31 state
>>>> -rw-r--r--    1 root     root          4096 Jan  1 00:31 wakeup_count
>>>>
>>>> # cat /sys/power/state
>>>> freeze mem
>>>>
>>>> Currently my platform's "mem" is a true suspend-to-RAM trigger,
>>>> where drivers are supposed to save their state (register values
>>>> will be lost), then Linux hands control over to firmware which
>>>> enables RAM self-refresh and powers the chip down. When the system
>>>> resumes, drivers restore their state from their copy in memory.
>>>>
>>>> One driver is responsible for loading/unloading microcode running
>>>> on the DSPs. This operation is required only when powering down
>>>> the chip, but it should be avoided for "low-latency" sleeps.
>>>>
>>>> The problem is that, if I understand correctly, drivers have no way
>>>> of knowing which sleep state is being entered/exited?
>>>>
>>>> How can I have the microcode driver take different decisions
>>>> based on the sleep state?
>>>
>>> The cleanest way would be to run that code from one of the platform
>>> suspend hooks that receive information on what sleep state is to be
>>> entered.
>>
>> I am not sure this would be cleaner, because that would create a tighter
>> dependency between different drivers, each of them having their
>> suspend/resume routings and the driver that implements the
>> platform_suspend_ops, that could also create some nice layering
>> violations and some difficult to solve dependencies.
>>
>>>
>>> Alternatively, those hooks can set/clear flags that can be accessed by
>>> drivers, but that of course may your drivers depend on the platform
>>> (still, in the microcode case the driver seems to be
>>> platform-dependent anyway).
>>
>> It may be platform dependent, but the actual system-wide suspend/resume
>> implementations can vary a lot. For example you may have started with
>> some particular CPU architecture on your platforms, with one driver
>> implementing an instance of platform_suspend_ops, and then as you moved
>> to another CPU architecture, some of that could be managed by a generic
>> driver (e.g: ARM SCPI, ACPI etc. etc.).
>>
>> The same HW blocks are likely to be present on these different SoCs, and
>> have the same requirements where they need to see a slightly different
>> path taken on suspend/resume. If we have to patch both the "legacy"
>> platform_suspend_ops, and the "new" platform_suspend_ops that does not
>> really scale.
>>
>> Would it be that much of a stretch if we reflected e.g:
>> PM_SUSPEND_STANDBY, PM_SUSPEND_MEM into the pm_message_t that is
>> communicated to platform_driver::suspend and platform_driver::resume?
> 
> I'm not sure what you mean, really.
> 
> The ->suspend callback in struct platform_driver has been long deprecated.

What I mean is that we could take advantage of the pm_message_t argument
passed to platform_driver::resume and platform_driver::resume to
communicate the system sleep state we are about to enter (and conversely
exit).

This would allow drivers to take a different path whether e.g:
pm_message_t == PM_SUSPEND_STANDBY or PM_SUSPEND_MEM.

If these are deprecated, then should we introduce a global getter
function that reflects which suspend state we are about to enter?  This
would allow drivers do to something like (pseudo code):

static int drv_suspend(struct device d)
{
	suspend_state_t state = suspend_get_state();

	switch (state) {
	case PM_SUSPEND_STANDBY:
		return 0;
	case PM_SUSPEND_MEM:
		/* save HW context */
}
-- 
Florian