From: Sudeep Holla <sudeep.holla@arm.com> To: Florian Fainelli <f.fainelli@gmail.com> Cc: linux-arm-kernel@lists.infradead.org, "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" <bcm-kernel-feedback-list@broadcom.com>, Mark Rutland <mark.rutland@arm.com>, Sudeep Holla <sudeep.holla@arm.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 0/4] Broadcom STB PM PSCI extensions Date: Thu, 3 Feb 2022 18:52:21 +0000 [thread overview] Message-ID: <20220203185221.aw7kayj6qklmh5is@bogus> (raw) In-Reply-To: <34938793-cecc-2ad8-a4eb-81bb278ce9b5@gmail.com> Correction: it is known as "freeze" rather than "idle" in terms of values as per /sys/power/state. Sorry for referring it as "idle" and creating any confusion. On Thu, Feb 03, 2022 at 09:36:28AM -0800, Florian Fainelli wrote: > > > On 2/3/2022 3:14 AM, Sudeep Holla wrote: > > On Fri, Jan 21, 2022 at 07:54:17PM -0800, Florian Fainelli wrote: > > > Hi all, > > > > > > This patch series contains the Broadcom STB PSCI extensions which adds > > > some additional functions on top of the existing standard PSCI interface > > > which is the reason for having the driver implement a custom > > > suspend_ops. > > > > > > These platforms have traditionally supported a mode that is akin to > > > ACPI's S2 with the CPU in WFI and all of the chip being clock gated > > > which is entered with "echo standby > /sys/power/state". Additional a > > > true suspend to DRAM as defined in ACPI by S3 is implemented with "echo > > > mem > /sys/power/state". > > > > How different is the above "standby" state compare to the standard "idle" > > (a.k.a suspend-to-idle which is different from system-to-ram/S3) ? > > There are a few differences: > > - s2idle does not power gate the secondary CPUs > Not sure what you mean by that ? S2I takes CPUs to deepest idle state. If you want shallower states, one possible option is the disable deeper states from the userspace. > - s2idle requires the use of in-band interrupts for wake-up > I am not sure if that is true. S2I behaves very similar to S2R except it has low wake latency as all secondaries CPUs are not hotplugged out. > The reasons for implementing "standby" are largely two fold: > > - we need to achieve decent power savings (typically below 0.5W for the > whole system while allowing Wake-on-WLAN, GPIO, RTC, infrared, etc.) > I fail to understand how that is a problem from S2I. It is probably worth checking if there are any unnecessary IRQF_NO_SUSPEND users. Check section IRQF_NO_SUSPEND and enable_irq_wake() in [1]. I don't see any issues other wise in terms of unnecessary/spurious wakeup by in-band(to be precise no-wake up) interrupts. > - we have a security subsystem that requires the CPUs to be either power > gated or idle in order the hardware state machine that lets the system enter > such a state and allows the out of band interrupts from being wake-up > sources > It should work unless I have completely misunderstood how S2I works. > > Suspend to idle takes all the CPUs to lowest possible power state instead > > of cpu-hotplug in S2R. Also I assume some userspace has to identify when > > to enter "standby" vs "mem" right ? I am trying to see how addition of > > "idle" changes that(if it does). Sorry for too many questions. > > > > Right that user-space in our case is either custom (like RDK, or completely > custom), or is Android. For Android it looks like we are carrying a patch > that makes "mem" de-generate into "standby" but this is largely because we > had historically problems with "mem" that are being addressed (completely > orthogonal). > Thanks for the info. > I did not consider it as a viable option at the time, but if we were to > implement "standby" in drivers/firmware/psci/psci.c would that be somewhat > acceptable? > We have been pointing anyone needing standby so far to S2I and so far no one has shouted that it doesn't suffice. Let me know what is missing. -- Regards, Sudeep [1] Documentation/power/suspend-and-interrupts.rst
WARNING: multiple messages have this Message-ID (diff)
From: Sudeep Holla <sudeep.holla@arm.com> To: Florian Fainelli <f.fainelli@gmail.com> Cc: linux-arm-kernel@lists.infradead.org, "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" <bcm-kernel-feedback-list@broadcom.com>, Mark Rutland <mark.rutland@arm.com>, Sudeep Holla <sudeep.holla@arm.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 0/4] Broadcom STB PM PSCI extensions Date: Thu, 3 Feb 2022 18:52:21 +0000 [thread overview] Message-ID: <20220203185221.aw7kayj6qklmh5is@bogus> (raw) In-Reply-To: <34938793-cecc-2ad8-a4eb-81bb278ce9b5@gmail.com> Correction: it is known as "freeze" rather than "idle" in terms of values as per /sys/power/state. Sorry for referring it as "idle" and creating any confusion. On Thu, Feb 03, 2022 at 09:36:28AM -0800, Florian Fainelli wrote: > > > On 2/3/2022 3:14 AM, Sudeep Holla wrote: > > On Fri, Jan 21, 2022 at 07:54:17PM -0800, Florian Fainelli wrote: > > > Hi all, > > > > > > This patch series contains the Broadcom STB PSCI extensions which adds > > > some additional functions on top of the existing standard PSCI interface > > > which is the reason for having the driver implement a custom > > > suspend_ops. > > > > > > These platforms have traditionally supported a mode that is akin to > > > ACPI's S2 with the CPU in WFI and all of the chip being clock gated > > > which is entered with "echo standby > /sys/power/state". Additional a > > > true suspend to DRAM as defined in ACPI by S3 is implemented with "echo > > > mem > /sys/power/state". > > > > How different is the above "standby" state compare to the standard "idle" > > (a.k.a suspend-to-idle which is different from system-to-ram/S3) ? > > There are a few differences: > > - s2idle does not power gate the secondary CPUs > Not sure what you mean by that ? S2I takes CPUs to deepest idle state. If you want shallower states, one possible option is the disable deeper states from the userspace. > - s2idle requires the use of in-band interrupts for wake-up > I am not sure if that is true. S2I behaves very similar to S2R except it has low wake latency as all secondaries CPUs are not hotplugged out. > The reasons for implementing "standby" are largely two fold: > > - we need to achieve decent power savings (typically below 0.5W for the > whole system while allowing Wake-on-WLAN, GPIO, RTC, infrared, etc.) > I fail to understand how that is a problem from S2I. It is probably worth checking if there are any unnecessary IRQF_NO_SUSPEND users. Check section IRQF_NO_SUSPEND and enable_irq_wake() in [1]. I don't see any issues other wise in terms of unnecessary/spurious wakeup by in-band(to be precise no-wake up) interrupts. > - we have a security subsystem that requires the CPUs to be either power > gated or idle in order the hardware state machine that lets the system enter > such a state and allows the out of band interrupts from being wake-up > sources > It should work unless I have completely misunderstood how S2I works. > > Suspend to idle takes all the CPUs to lowest possible power state instead > > of cpu-hotplug in S2R. Also I assume some userspace has to identify when > > to enter "standby" vs "mem" right ? I am trying to see how addition of > > "idle" changes that(if it does). Sorry for too many questions. > > > > Right that user-space in our case is either custom (like RDK, or completely > custom), or is Android. For Android it looks like we are carrying a patch > that makes "mem" de-generate into "standby" but this is largely because we > had historically problems with "mem" that are being addressed (completely > orthogonal). > Thanks for the info. > I did not consider it as a viable option at the time, but if we were to > implement "standby" in drivers/firmware/psci/psci.c would that be somewhat > acceptable? > We have been pointing anyone needing standby so far to S2I and so far no one has shouted that it doesn't suffice. Let me know what is missing. -- Regards, Sudeep [1] Documentation/power/suspend-and-interrupts.rst _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-02-03 18:53 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-22 3:54 [PATCH 0/4] Broadcom STB PM PSCI extensions Florian Fainelli 2022-01-22 3:54 ` Florian Fainelli 2022-01-22 3:54 ` [PATCH 1/4] firmware: psci: Export a couple of suspend symbols Florian Fainelli 2022-01-22 3:54 ` Florian Fainelli 2022-01-22 12:22 ` kernel test robot 2022-01-22 12:22 ` kernel test robot 2022-01-22 3:54 ` [PATCH 2/4] soc: bcm: brcmstb: Make legacy PM code depend on !ARM_PSCI_FW Florian Fainelli 2022-01-22 3:54 ` Florian Fainelli 2022-01-22 3:54 ` [PATCH 3/4] soc: bcm: brcmstb: Added support for PSCI system suspend operations Florian Fainelli 2022-01-22 3:54 ` Florian Fainelli 2022-01-22 6:09 ` kernel test robot 2022-01-22 6:09 ` kernel test robot 2022-01-22 7:10 ` kernel test robot 2022-01-22 7:10 ` kernel test robot 2022-02-03 12:09 ` Mark Rutland 2022-02-03 12:09 ` Mark Rutland 2022-02-03 18:45 ` Florian Fainelli 2022-02-03 18:45 ` Florian Fainelli 2022-01-22 3:54 ` [PATCH 4/4] Documentation: ABI: Document Broadcom STB PSCI firmware files Florian Fainelli 2022-01-22 3:54 ` Florian Fainelli 2022-01-27 3:55 ` [PATCH 0/4] Broadcom STB PM PSCI extensions Florian Fainelli 2022-01-27 3:55 ` Florian Fainelli 2022-02-03 10:47 ` Mark Rutland 2022-02-03 10:47 ` Mark Rutland 2022-02-03 18:32 ` Florian Fainelli 2022-02-03 18:32 ` Florian Fainelli 2022-02-03 11:14 ` Sudeep Holla 2022-02-03 11:14 ` Sudeep Holla 2022-02-03 17:36 ` Florian Fainelli 2022-02-03 17:36 ` Florian Fainelli 2022-02-03 18:52 ` Sudeep Holla [this message] 2022-02-03 18:52 ` Sudeep Holla 2022-02-03 19:33 ` Florian Fainelli 2022-02-03 19:33 ` Florian Fainelli 2022-02-07 16:27 ` Sudeep Holla 2022-02-07 16:27 ` Sudeep Holla 2022-02-14 18:12 ` Florian Fainelli 2022-02-14 18:12 ` Florian Fainelli
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=20220203185221.aw7kayj6qklmh5is@bogus \ --to=sudeep.holla@arm.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=f.fainelli@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mark.rutland@arm.com \ /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.