On Thu, Jun 22, 2017 at 12:48 AM, Florian Fainelli [off-list ref] wrote:

On 06/21/2017 02:59 PM, Rafael J. Wysocki wrote:

quoted

On Wed, Jun 21, 2017 at 11:16 PM, Florian Fainelli [off-list ref] wrote:

quoted

On 06/09/2017 03:53 PM, Rafael J. Wysocki wrote:

quoted

Hi,

On Fri, Jun 9, 2017 at 5:20 PM, Mason [off-list ref] wrote:

quoted

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.

quoted

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).

So the ->suspend and ->resume callbacks in struct platform_driver
(which I think is what you are referring to) are legacy.

The majority of drivers I know about use struct dev_pm_ops and the
callbacks in there do not take the pm_message_t argument.

Moreover, even if they did take it, the exact meaning of
PM_SUSPEND_STANDBY and PM_SUSPEND_MEM is platform-specific and drivers
would need to find out what they actually mean for the given platform
somehow anyway.

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 */
}

That is conceivable, but again, the meaning of STANDBY and MEM is
platform-specific.  Actions to be taken for, say, STANDBY, may differ
from one platform to another.

Thanks,
Rafael