From: Ulf Hansson <ulf.hansson@linaro.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Sudeep Holla <sudeep.holla@arm.com>, Mark Rutland <mark.rutland@arm.com> Cc: Lina Iyer <ilina@codeaurora.org>, "Rafael J. Wysocki" <rafael@kernel.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Linux PM <linux-pm@vger.kernel.org>, Kevin Hilman <khilman@kernel.org>, Lina Iyer <lina.iyer@linaro.org>, Rob Herring <robh+dt@kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, Vincent Guittot <vincent.guittot@linaro.org>, Stephen Boyd <sboyd@kernel.org>, Juri Lelli <juri.lelli@arm.com>, Geert Uytterhoeven <geert+renesas@glider.be>, Linux ARM <linux-arm-kernel@lists.infradead.org>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v8 09/26] kernel/cpu_pm: Manage runtime PM in the idle path for CPUs Date: Fri, 24 Aug 2018 14:24:20 +0200 [thread overview] Message-ID: <CAPDyKFr6vx+rHj0kP5e_7_NzK413df7J72QPoH7eDbK5386D-w@mail.gmail.com> (raw) In-Reply-To: <20180815104449.GA29108@red-moon> Lorenzo, Sudeep, Mark On 15 August 2018 at 12:44, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Fri, Aug 10, 2018 at 02:18:15PM -0600, Lina Iyer wrote: > > [...] > >> >>But, the OSI feature is critical for QCOM mobile platforms. The >> >>last man activities during cpuidle save quite a lot of power. >> > >> >What I expressed above was that, in PSCI based systems (OSI or PC >> >alike), it is up to firmware/hardware to detect "the last man" not >> >the kernel. >> > >> >I need to understand what you mean by "last man activities" to >> >provide feedback here. >> > >> When the last CPU goes down during deep sleep, the following would be >> done >> - Lower resource requirements for shared resources such as clocks, >> busses and regulators that were used by drivers in AP. These shared >> resources when not used by other processors in the SoC may be turned >> off and put in low power state by a remote processor. [1][2] >> - Enable and setup wakeup capable interrupts on an always-on interrupt >> controller, so the GIC and the GPIO controllers may be put in low >> power state. [3][4] >> - Write next known wakeup value to the timer, so the blocks that were >> powered off, may be brought back into operational before the wakeup. >> [4][5] >> >> These are commonly done during suspend, but to achieve a good power >> efficiency, we have to do this when all the CPUs are just executing CPU >> idle. Also, they cannot be done from the firmware (because the data >> required for all this is part of Linux). OSI plays a crucial role in >> determining when to do all this. > > No it does not. It is the power domain cpumasks that allow this code to > make an educated guess on the last cpu running (the kernel), PSCI OSI is > not crucial at all (it is crucial in QC platforms because that's the > only mode supported but that's not a reason I accept as valid since it > does not comply with the PSCI specifications). We can keep argue on this back and forward, but it seems to lead nowhere. As a matter of fact I am also surprised that this kind of discussion pops up, again. I thought we had sorted this out, especially since we have also met face to face, discussing this in detail, several times by now. Well, well, let's try again. :-) First, in regards to complying with the PSCI spec, to me, that sounds like nonsense, sorry! Is the spec stating that the PSCI FW needs to support all the idle states in PC mode, when the optional OSI mode also is supported? To me, it looks like the QCOM PSCI FW supports PC mode, but in that mode only a subset of the idle states can be reached, so that should be fine, no? Moving forward, I am wondering if a more detailed technical description, comparing the benefits from OSI mode vs the benefits from PC mode could help? Or is just a waste of everybody time, as you all already know this? Anyway I am willing to try, just tell me and I provide you with the best details I can give, about why OSI is better suited for these kind of QCOM SoCs. I trust Lina to help to fill in, if/when needed. Why? Simply because I doubt we ever see the QCOM FW for the battery driven embedded devices to support all idles states in the PC mode, so doing a comparison on for example the 410c platform, just doesn't seems to be possible, sorry! I also have another, quite important, concern. That is, ARM decided to put the OSI mode into the PSCI spec, I assume there were reasons for it. Then, when the ARM community wants to implement support for OSI mode, you are now requiring us to proof the justification of it in the spec. To me, that is, nicely stated, weird. :-) But it also worries me, ARM vendors observes the behavior. That said, in the end we are discussing a quite limited amount of lines of code to support PSCI OSI (some which may not even be considered as OSI specific). It's ~200 lines of code, where most of the code lives in a separate new c-file (psci_pm_domain.c). Additionally, existing PC mode only platforms should still work as before, without drawbacks. Really, why are we arguing about this at all? > > As I mentioned in another thread[1] the generic part of this > series may be applicable in a platform agnostic way to the > CPUidle framework, whether that's beneficial it has to be proven > and it is benchmark specific anyway. I don't think this can be made fully platform agnostic. Or maybe you are suggesting another helper layer above the new genpd infrastructure? Anyway, my point is, the genpd backend driver requires knowledge about the FW and the last man standing algorithm, hence a platform agnostic backend doesn't sound feasible to me. > > Lorenzo > > [1]: https://marc.info/?l=linux-pm&m=153382916513032&w=2 Kind regards Uffe
WARNING: multiple messages have this Message-ID (diff)
From: ulf.hansson@linaro.org (Ulf Hansson) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v8 09/26] kernel/cpu_pm: Manage runtime PM in the idle path for CPUs Date: Fri, 24 Aug 2018 14:24:20 +0200 [thread overview] Message-ID: <CAPDyKFr6vx+rHj0kP5e_7_NzK413df7J72QPoH7eDbK5386D-w@mail.gmail.com> (raw) In-Reply-To: <20180815104449.GA29108@red-moon> Lorenzo, Sudeep, Mark On 15 August 2018 at 12:44, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Fri, Aug 10, 2018 at 02:18:15PM -0600, Lina Iyer wrote: > > [...] > >> >>But, the OSI feature is critical for QCOM mobile platforms. The >> >>last man activities during cpuidle save quite a lot of power. >> > >> >What I expressed above was that, in PSCI based systems (OSI or PC >> >alike), it is up to firmware/hardware to detect "the last man" not >> >the kernel. >> > >> >I need to understand what you mean by "last man activities" to >> >provide feedback here. >> > >> When the last CPU goes down during deep sleep, the following would be >> done >> - Lower resource requirements for shared resources such as clocks, >> busses and regulators that were used by drivers in AP. These shared >> resources when not used by other processors in the SoC may be turned >> off and put in low power state by a remote processor. [1][2] >> - Enable and setup wakeup capable interrupts on an always-on interrupt >> controller, so the GIC and the GPIO controllers may be put in low >> power state. [3][4] >> - Write next known wakeup value to the timer, so the blocks that were >> powered off, may be brought back into operational before the wakeup. >> [4][5] >> >> These are commonly done during suspend, but to achieve a good power >> efficiency, we have to do this when all the CPUs are just executing CPU >> idle. Also, they cannot be done from the firmware (because the data >> required for all this is part of Linux). OSI plays a crucial role in >> determining when to do all this. > > No it does not. It is the power domain cpumasks that allow this code to > make an educated guess on the last cpu running (the kernel), PSCI OSI is > not crucial at all (it is crucial in QC platforms because that's the > only mode supported but that's not a reason I accept as valid since it > does not comply with the PSCI specifications). We can keep argue on this back and forward, but it seems to lead nowhere. As a matter of fact I am also surprised that this kind of discussion pops up, again. I thought we had sorted this out, especially since we have also met face to face, discussing this in detail, several times by now. Well, well, let's try again. :-) First, in regards to complying with the PSCI spec, to me, that sounds like nonsense, sorry! Is the spec stating that the PSCI FW needs to support all the idle states in PC mode, when the optional OSI mode also is supported? To me, it looks like the QCOM PSCI FW supports PC mode, but in that mode only a subset of the idle states can be reached, so that should be fine, no? Moving forward, I am wondering if a more detailed technical description, comparing the benefits from OSI mode vs the benefits from PC mode could help? Or is just a waste of everybody time, as you all already know this? Anyway I am willing to try, just tell me and I provide you with the best details I can give, about why OSI is better suited for these kind of QCOM SoCs. I trust Lina to help to fill in, if/when needed. Why? Simply because I doubt we ever see the QCOM FW for the battery driven embedded devices to support all idles states in the PC mode, so doing a comparison on for example the 410c platform, just doesn't seems to be possible, sorry! I also have another, quite important, concern. That is, ARM decided to put the OSI mode into the PSCI spec, I assume there were reasons for it. Then, when the ARM community wants to implement support for OSI mode, you are now requiring us to proof the justification of it in the spec. To me, that is, nicely stated, weird. :-) But it also worries me, ARM vendors observes the behavior. That said, in the end we are discussing a quite limited amount of lines of code to support PSCI OSI (some which may not even be considered as OSI specific). It's ~200 lines of code, where most of the code lives in a separate new c-file (psci_pm_domain.c). Additionally, existing PC mode only platforms should still work as before, without drawbacks. Really, why are we arguing about this at all? > > As I mentioned in another thread[1] the generic part of this > series may be applicable in a platform agnostic way to the > CPUidle framework, whether that's beneficial it has to be proven > and it is benchmark specific anyway. I don't think this can be made fully platform agnostic. Or maybe you are suggesting another helper layer above the new genpd infrastructure? Anyway, my point is, the genpd backend driver requires knowledge about the FW and the last man standing algorithm, hence a platform agnostic backend doesn't sound feasible to me. > > Lorenzo > > [1]: https://marc.info/?l=linux-pm&m=153382916513032&w=2 Kind regards Uffe
next prev parent reply other threads:[~2018-08-24 12:24 UTC|newest] Thread overview: 165+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-20 17:22 [PATCH v8 00/26] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM) Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 01/26] PM / Domains: Don't treat zero found compatible idle states as an error Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 02/26] PM / Domains: Deal with multiple states but no governor in genpd Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 03/26] PM / Domains: Add generic data pointer to genpd_power_state struct Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-24 21:09 ` Rafael J. Wysocki 2018-06-24 21:09 ` Rafael J. Wysocki 2018-06-25 8:34 ` Ulf Hansson 2018-06-25 8:34 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 04/26] PM / Domains: Add support for CPU devices to genpd Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-19 10:25 ` Rafael J. Wysocki 2018-07-19 10:25 ` Rafael J. Wysocki 2018-08-03 11:43 ` Ulf Hansson 2018-08-03 11:43 ` Ulf Hansson 2018-08-06 9:36 ` Rafael J. Wysocki 2018-08-06 9:36 ` Rafael J. Wysocki 2018-08-24 6:47 ` Ulf Hansson 2018-08-24 6:47 ` Ulf Hansson 2018-09-14 9:26 ` Rafael J. Wysocki 2018-09-14 9:26 ` Rafael J. Wysocki 2018-06-20 17:22 ` [PATCH v8 05/26] PM / Domains: Add helper functions to attach/detach CPUs to/from genpd Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-19 10:22 ` Rafael J. Wysocki 2018-07-19 10:22 ` Rafael J. Wysocki 2018-08-03 11:44 ` Ulf Hansson 2018-08-03 11:44 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 06/26] timer: Export next wakeup time of a CPU Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-19 10:15 ` Rafael J. Wysocki 2018-07-19 10:15 ` Rafael J. Wysocki 2018-06-20 17:22 ` [PATCH v8 07/26] PM / Domains: Add genpd governor for CPUs Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-19 10:32 ` Rafael J. Wysocki 2018-07-19 10:32 ` Rafael J. Wysocki 2018-07-26 9:14 ` Rafael J. Wysocki 2018-07-26 9:14 ` Rafael J. Wysocki 2018-08-03 14:28 ` Ulf Hansson 2018-08-03 14:28 ` Ulf Hansson 2018-08-03 14:28 ` Ulf Hansson 2018-08-06 9:20 ` Rafael J. Wysocki 2018-08-06 9:20 ` Rafael J. Wysocki 2018-08-06 9:20 ` Rafael J. Wysocki 2018-08-09 15:39 ` Lorenzo Pieralisi 2018-08-09 15:39 ` Lorenzo Pieralisi 2018-08-09 15:39 ` Lorenzo Pieralisi 2018-08-24 9:26 ` Ulf Hansson 2018-08-24 9:26 ` Ulf Hansson 2018-08-24 9:26 ` Ulf Hansson 2018-08-24 10:38 ` Lorenzo Pieralisi 2018-08-24 10:38 ` Lorenzo Pieralisi 2018-08-24 10:38 ` Lorenzo Pieralisi 2018-08-30 13:36 ` Ulf Hansson 2018-08-30 13:36 ` Ulf Hansson 2018-08-30 13:36 ` Ulf Hansson 2018-09-13 15:37 ` Lorenzo Pieralisi 2018-09-13 15:37 ` Lorenzo Pieralisi 2018-09-13 15:37 ` Lorenzo Pieralisi 2018-09-14 9:50 ` Rafael J. Wysocki 2018-09-14 9:50 ` Rafael J. Wysocki 2018-09-14 9:50 ` Rafael J. Wysocki 2018-09-14 10:44 ` Lorenzo Pieralisi 2018-09-14 10:44 ` Lorenzo Pieralisi 2018-09-14 10:44 ` Lorenzo Pieralisi 2018-09-14 11:34 ` Rafael J. Wysocki 2018-09-14 11:34 ` Rafael J. Wysocki 2018-09-14 11:34 ` Rafael J. Wysocki 2018-09-14 12:30 ` Lorenzo Pieralisi 2018-09-14 12:30 ` Lorenzo Pieralisi 2018-09-14 12:30 ` Lorenzo Pieralisi 2018-08-24 8:29 ` Ulf Hansson 2018-08-24 8:29 ` Ulf Hansson 2018-08-24 8:29 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 08/26] PM / Domains: Extend genpd CPU governor to cope with QoS constraints Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-19 10:35 ` Rafael J. Wysocki 2018-07-19 10:35 ` Rafael J. Wysocki 2018-08-03 11:42 ` Ulf Hansson 2018-08-03 11:42 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 09/26] kernel/cpu_pm: Manage runtime PM in the idle path for CPUs Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-18 10:11 ` Rafael J. Wysocki 2018-07-18 10:11 ` Rafael J. Wysocki 2018-07-19 10:12 ` Rafael J. Wysocki 2018-07-19 10:12 ` Rafael J. Wysocki 2018-07-19 10:39 ` Rafael J. Wysocki 2018-07-19 10:39 ` Rafael J. Wysocki 2018-08-03 11:42 ` Ulf Hansson 2018-08-03 11:42 ` Ulf Hansson 2018-08-06 9:37 ` Rafael J. Wysocki 2018-08-06 9:37 ` Rafael J. Wysocki 2018-08-08 10:56 ` Lorenzo Pieralisi 2018-08-08 10:56 ` Lorenzo Pieralisi 2018-08-08 18:02 ` Lina Iyer 2018-08-08 18:02 ` Lina Iyer 2018-08-09 8:16 ` Rafael J. Wysocki 2018-08-09 8:16 ` Rafael J. Wysocki 2018-08-09 8:16 ` Rafael J. Wysocki 2018-08-10 20:36 ` Lina Iyer 2018-08-10 20:36 ` Lina Iyer 2018-08-12 9:53 ` Rafael J. Wysocki 2018-08-12 9:53 ` Rafael J. Wysocki 2018-08-12 9:53 ` Rafael J. Wysocki 2018-08-09 9:58 ` Sudeep Holla 2018-08-09 9:58 ` Sudeep Holla 2018-08-09 9:58 ` Sudeep Holla 2018-08-09 10:25 ` Lorenzo Pieralisi 2018-08-09 10:25 ` Lorenzo Pieralisi 2018-08-10 20:18 ` Lina Iyer 2018-08-10 20:18 ` Lina Iyer 2018-08-15 10:44 ` Lorenzo Pieralisi 2018-08-15 10:44 ` Lorenzo Pieralisi 2018-08-24 12:24 ` Ulf Hansson [this message] 2018-08-24 12:24 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 10/26] dt: psci: Update DT bindings to support hierarchical PSCI states Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 11/26] of: base: Add of_get_cpu_state_node() to get idle states for a CPU node Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 12/26] cpuidle: dt: Support hierarchical CPU idle states Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 13/26] drivers: firmware: psci: Move psci to separate directory Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 14/26] MAINTAINERS: Update files for PSCI Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 15/26] drivers: firmware: psci: Split psci_dt_cpu_init_idle() Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 16/26] drivers: firmware: psci: Support hierarchical CPU idle states Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 17/26] drivers: firmware: psci: Simplify error path of psci_dt_init() Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 18/26] drivers: firmware: psci: Announce support for OS initiated suspend mode Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 19/26] drivers: firmware: psci: Prepare to use " Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 20/26] drivers: firmware: psci: Share a few internal PSCI functions Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 21/26] drivers: firmware: psci: Add support for PM domains using genpd Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 22/26] drivers: firmware: psci: Introduce psci_dt_topology_init() Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 23/26] drivers: firmware: psci: Try to attach CPU devices to their PM domains Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 24/26] drivers: firmware: psci: Deal with CPU hotplug when using OSI mode Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-11-19 19:50 ` Raju P L S S S N 2018-11-19 19:50 ` Raju P L S S S N 2018-11-20 9:50 ` Ulf Hansson 2018-11-20 9:50 ` Ulf Hansson 2018-11-20 10:47 ` Raju P L S S S N 2018-11-20 10:47 ` Raju P L S S S N 2018-06-20 17:22 ` [PATCH v8 25/26] arm64: kernel: Respect the hierarchical CPU topology in DT for PSCI Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-06-20 17:22 ` [PATCH v8 26/26] arm64: dts: Convert to the hierarchical CPU topology layout for MSM8916 Ulf Hansson 2018-06-20 17:22 ` Ulf Hansson 2018-07-03 5:44 ` [PATCH v8 00/26] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM) Ulf Hansson 2018-07-03 5:44 ` Ulf Hansson 2018-07-03 7:54 ` Rafael J. Wysocki 2018-07-03 7:54 ` Rafael J. Wysocki 2018-07-09 11:42 ` Ulf Hansson 2018-07-09 11:42 ` Ulf Hansson
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=CAPDyKFr6vx+rHj0kP5e_7_NzK413df7J72QPoH7eDbK5386D-w@mail.gmail.com \ --to=ulf.hansson@linaro.org \ --cc=daniel.lezcano@linaro.org \ --cc=geert+renesas@glider.be \ --cc=ilina@codeaurora.org \ --cc=juri.lelli@arm.com \ --cc=khilman@kernel.org \ --cc=lina.iyer@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mark.rutland@arm.com \ --cc=rafael@kernel.org \ --cc=rjw@rjwysocki.net \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=sudeep.holla@arm.com \ --cc=tglx@linutronix.de \ --cc=vincent.guittot@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.