On Thu, Feb 17, 2022 at 08:52:35AM +0000, Marc Zyngier wrote: > On Thu, 17 Feb 2022 07:31:32 +0000, > Shawn Guo <shawn.guo@xxxxxxxxxx> wrote: > > > > On Wed, Feb 16, 2022 at 03:58:41PM +0000, Marc Zyngier wrote: > > > On 2022-02-16 14:49, Sudeep Holla wrote: > > > > +Ulf (as you he is the author of cpuidle-psci-domains.c and can help you > > > > with that if you require) > > > > Thanks, Sudeep! > > > > > > > > > > On Wed, Feb 16, 2022 at 09:28:28PM +0800, Shawn Guo wrote: > > > > > Make a call to cpu_cluster_pm_enter() on the last CPU going to low > > > > > power > > > > > state (and cpu_cluster_pm_exit() on the firt CPU coming back), so that > > > > > platforms can be notified to set up hardware for getting into the > > > > > cluster > > > > > low power state. > > > > > > > > > > > > > NACK. We are not getting the notion of CPU cluster back to cpuidle > > > > again. > > > > That must die. Remember the cluster doesn't map to idle states > > > > especially > > > > in the DSU systems where HMP CPUs are in the same cluster but can be in > > > > different power domains. > > > > The 'cluster' in cpu_cluster_pm_enter() doesn't necessarily means > > a physical CPU cluster. I think the documentation of the function has a > > better description. > > > > * Notifies listeners that all cpus in a power domain are entering a low power > > * state that may cause some blocks in the same power domain to reset. > > > > So cpu_domain_pm_enter() might be a better name? Anyways ... > > > > > > > > > > You need to decide which PSCI CPU_SUSPEND mode you want to use first. If > > > > it is > > > > Platform Co-ordinated(PC), then you need not notify anything to the > > > > platform. > > > > Just request the desired idle state on each CPU and platform will take > > > > care > > > > from there. > > > > > > > > If for whatever reason you have chosen OS initiated mode(OSI), then > > > > specify > > > > the PSCI power domains correctly in the DT which will make use of the > > > > cpuidle-psci-domains and handle the so called "cluster" state correctly. > > > > Yes, I'm running a Qualcomm platform that has OSI supported in PSCI. > > > > > > > > My understanding is that what Shawn is after is a way to detect the "last > > > man standing" on the system to kick off some funky wake-up controller that > > > really should be handled by the power controller (because only that guy > > > knows for sure who is the last CPU on the block). > > > > > > There was previously some really funky stuff (copy pasted from the existing > > > rpmh_rsc_cpu_pm_callback()), which I totally objected to having hidden in > > > an irqchip driver. > > > > > > My ask was that if we needed such information, and assuming that it is > > > possible to obtain it in a reliable way, this should come from the core > > > code, and not be invented by random drivers. > > > > Thanks Marc for explain my problem! > > > > Right, all I need is a notification in MPM irqchip driver when the CPU > > domain/cluster is about to enter low power state. As cpu_pm - > > kernel/cpu_pm.c, already has helper cpu_cluster_pm_enter() sending > > CPU_CLUSTER_PM_ENTER event, I just need to find a caller to this cpu_pm > > helper. > > > > Is .power_off hook of generic_pm_domain a better place for calling the > > helper? > > I really don't understand why you want a cluster PM event generated by > the idle driver. Specially considering that you are not after a > *cluster* PM event, but after some sort of system-wide event (last man > standing). That's primarily because MPM driver is used in the idle context, either s2idle or cpuidle, and idle driver already has some infrastructure that could help trigger the event. > It looks to me that having a predicate that can be called from a PM > notifier event to find out whether you're the last in line would be > better suited, and could further be used to remove the crap from the > rpmh-rsc driver. I will see if I can come up with a patch for discussion. Shawn
