From: "Rafael J. Wysocki" <rafael@kernel.org> To: Florian Fainelli <f.fainelli@gmail.com> Cc: "Rafael J. Wysocki" <rafael@kernel.org>, Mason <slash.tmp@free.fr>, Ulf Hansson <ulf.hansson@linaro.org>, JB <jb_lescher@sigmadesigns.com>, linux-pm <linux-pm@vger.kernel.org>, Thibaud Cornic <thibaud_cornic@sigmadesigns.com>, Daniel Lezcano <daniel.lezcano@linaro.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Kevin Hilman <khilman@kernel.org>, Pavel Machek <pavel@ucw.cz>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: Drivers taking different actions depending on sleep state Date: Thu, 22 Jun 2017 00:57:38 +0200 [thread overview] Message-ID: <CAJZ5v0ihqn5Xcj1yoZcrb7caYiH+x8YivLJit6BqvDGradZJ=Q@mail.gmail.com> (raw) In-Reply-To: <7001768d-9134-4975-127e-e8444e00f677@gmail.com> On Thu, Jun 22, 2017 at 12:48 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: > On 06/21/2017 02:59 PM, Rafael J. Wysocki wrote: >> On Wed, Jun 21, 2017 at 11:16 PM, Florian Fainelli <f.fainelli@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@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). 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
WARNING: multiple messages have this Message-ID (diff)
From: rafael@kernel.org (Rafael J. Wysocki) To: linux-arm-kernel@lists.infradead.org Subject: Drivers taking different actions depending on sleep state Date: Thu, 22 Jun 2017 00:57:38 +0200 [thread overview] Message-ID: <CAJZ5v0ihqn5Xcj1yoZcrb7caYiH+x8YivLJit6BqvDGradZJ=Q@mail.gmail.com> (raw) In-Reply-To: <7001768d-9134-4975-127e-e8444e00f677@gmail.com> On Thu, Jun 22, 2017 at 12:48 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: > On 06/21/2017 02:59 PM, Rafael J. Wysocki wrote: >> On Wed, Jun 21, 2017 at 11:16 PM, Florian Fainelli <f.fainelli@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@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). 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
next prev parent reply other threads:[~2017-06-21 22:57 UTC|newest] Thread overview: 118+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-09 15:20 Drivers taking different actions depending on sleep state Mason 2017-06-09 15:20 ` Mason 2017-06-09 16:27 ` Mason 2017-06-09 16:27 ` Mason 2017-06-09 21:30 ` Pavel Machek 2017-06-09 21:30 ` Pavel Machek 2017-06-10 9:16 ` Mason 2017-06-10 9:16 ` Mason 2017-06-09 22:05 ` Florian Fainelli 2017-06-09 22:05 ` Florian Fainelli 2017-06-09 22:53 ` Rafael J. Wysocki 2017-06-09 22:53 ` Rafael J. Wysocki 2017-06-21 21:16 ` Florian Fainelli 2017-06-21 21:16 ` Florian Fainelli 2017-06-21 21:59 ` Rafael J. Wysocki 2017-06-21 21:59 ` Rafael J. Wysocki 2017-06-21 22:48 ` Florian Fainelli 2017-06-21 22:48 ` Florian Fainelli 2017-06-21 22:57 ` Rafael J. Wysocki [this message] 2017-06-21 22:57 ` Rafael J. Wysocki 2017-06-21 23:55 ` Florian Fainelli 2017-06-21 23:55 ` Florian Fainelli 2017-06-22 0:03 ` Rafael J. Wysocki 2017-06-22 0:03 ` Rafael J. Wysocki 2017-06-22 15:18 ` Florian Fainelli 2017-06-22 15:18 ` Florian Fainelli 2017-06-22 16:09 ` Rafael J. Wysocki 2017-06-22 16:09 ` Rafael J. Wysocki 2017-06-22 8:51 ` Alexandre Belloni 2017-06-22 8:51 ` Alexandre Belloni 2017-06-22 16:00 ` Rafael J. Wysocki 2017-06-22 16:00 ` Rafael J. Wysocki 2017-06-23 1:08 ` [RFC 0/2] PM / suspend: Add platform_suspend_target_state() Florian Fainelli 2017-06-23 1:08 ` Florian Fainelli 2017-06-23 1:08 ` [RFC 1/2] " Florian Fainelli 2017-06-23 1:08 ` Florian Fainelli 2017-06-29 23:00 ` Rafael J. Wysocki 2017-06-29 23:00 ` Rafael J. Wysocki 2017-07-12 18:08 ` Florian Fainelli 2017-07-12 18:08 ` Florian Fainelli 2017-07-14 22:16 ` Rafael J. Wysocki 2017-07-14 22:16 ` Rafael J. Wysocki 2017-07-15 6:28 ` Pavel Machek 2017-07-15 6:28 ` Pavel Machek 2017-07-15 12:17 ` Rafael J. Wysocki 2017-07-15 12:17 ` Rafael J. Wysocki 2017-07-15 16:46 ` Pavel Machek 2017-07-15 16:46 ` Pavel Machek 2017-07-15 17:20 ` Florian Fainelli 2017-07-15 17:20 ` Florian Fainelli 2017-07-15 18:33 ` Alexandre Belloni 2017-07-15 18:33 ` Alexandre Belloni 2017-07-06 3:18 ` Pavel Machek 2017-07-06 3:18 ` Pavel Machek 2017-07-16 13:41 ` Alexandre Belloni 2017-07-16 13:41 ` Alexandre Belloni 2017-07-16 15:35 ` Florian Fainelli 2017-07-16 15:35 ` Florian Fainelli 2017-07-15 23:24 ` Rafael J. Wysocki 2017-07-15 23:24 ` Rafael J. Wysocki 2017-07-15 23:34 ` Mason 2017-07-15 23:34 ` Mason 2017-07-15 23:38 ` Rafael J. Wysocki 2017-07-15 23:38 ` Rafael J. Wysocki 2017-07-16 2:36 ` Florian Fainelli 2017-07-16 2:36 ` Florian Fainelli 2017-07-16 10:22 ` Rafael J. Wysocki 2017-07-16 10:22 ` Rafael J. Wysocki 2017-07-16 13:38 ` Alexandre Belloni 2017-07-16 13:38 ` Alexandre Belloni 2017-07-16 18:24 ` Pavel Machek 2017-07-16 18:24 ` Pavel Machek 2017-07-16 15:41 ` Florian Fainelli 2017-07-16 15:41 ` Florian Fainelli 2017-07-15 23:29 ` Rafael J. Wysocki 2017-07-15 23:29 ` Rafael J. Wysocki 2017-07-06 3:17 ` Pavel Machek 2017-07-06 3:17 ` Pavel Machek 2017-07-16 10:28 ` Rafael J. Wysocki 2017-07-16 10:28 ` Rafael J. Wysocki 2017-07-16 18:22 ` Pavel Machek 2017-07-16 18:22 ` Pavel Machek 2017-06-23 1:08 ` [RFC 2/2] soc: bcm: brcmstb: PM: Implement target_state callback Florian Fainelli 2017-06-23 1:08 ` Florian Fainelli 2017-06-29 23:04 ` Rafael J. Wysocki 2017-06-29 23:04 ` Rafael J. Wysocki 2017-07-16 2:36 ` [PATCH 0/2] PM / suspend: Add platform_suspend_target_state() Florian Fainelli 2017-07-16 2:36 ` Florian Fainelli 2017-07-16 2:36 ` [PATCH 1/2] " Florian Fainelli 2017-07-16 2:36 ` Florian Fainelli 2017-07-06 3:18 ` Pavel Machek 2017-07-06 3:18 ` Pavel Machek 2017-07-16 15:41 ` Florian Fainelli 2017-07-16 15:41 ` Florian Fainelli 2017-07-16 10:30 ` Rafael J. Wysocki 2017-07-16 10:30 ` Rafael J. Wysocki 2017-07-16 2:36 ` [PATCH 2/2] soc: bcm: brcmstb: PM: Implement target_state callback Florian Fainelli 2017-07-16 2:36 ` Florian Fainelli 2017-07-17 20:06 ` [PATCH v2] PM / suspend: Add suspend_target_state() Florian Fainelli 2017-07-17 20:06 ` Florian Fainelli 2017-07-17 20:16 ` Pavel Machek 2017-07-17 20:16 ` Pavel Machek 2017-07-17 21:03 ` Rafael J. Wysocki 2017-07-17 21:03 ` Rafael J. Wysocki 2017-07-17 21:21 ` Florian Fainelli 2017-07-17 21:21 ` Florian Fainelli 2017-07-20 8:03 ` Pavel Machek 2017-07-20 8:03 ` Pavel Machek 2017-07-17 22:10 ` [PATCH v3] PM / suspend: Export pm_suspend_target_state Florian Fainelli 2017-07-17 22:10 ` Florian Fainelli 2017-07-17 23:24 ` Rafael J. Wysocki 2017-07-17 23:24 ` Rafael J. Wysocki 2017-07-18 0:19 ` [PATCH v4] " Florian Fainelli 2017-07-18 0:19 ` Florian Fainelli 2017-07-24 20:55 ` Rafael J. Wysocki 2017-07-24 20:55 ` Rafael J. Wysocki 2017-07-13 12:03 ` Drivers taking different actions depending on sleep state Pavel Machek 2017-07-13 12:03 ` Pavel Machek
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAJZ5v0ihqn5Xcj1yoZcrb7caYiH+x8YivLJit6BqvDGradZJ=Q@mail.gmail.com' \ --to=rafael@kernel.org \ --cc=daniel.lezcano@linaro.org \ --cc=f.fainelli@gmail.com \ --cc=jb_lescher@sigmadesigns.com \ --cc=khilman@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-pm@vger.kernel.org \ --cc=pavel@ucw.cz \ --cc=rjw@rjwysocki.net \ --cc=slash.tmp@free.fr \ --cc=thibaud_cornic@sigmadesigns.com \ --cc=ulf.hansson@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.