On Fri, May 26, 2023 at 02:28:52AM +0200, Konrad Dybcio wrote: > On 26.05.2023 01:39, Konrad Dybcio wrote: > > On 17.05.2023 20:48, Stephan Gerhold wrote: > >> Some of the regulators must be always-on to ensure correct operation of > >> the system, e.g. PM8916 L2 for the LPDDR RAM, L5 for most digital I/O > >> and L7 for the CPU PLL (strictly speaking the CPU PLL might only need > >> an active-only vote but this is not supported for regulators in > >> mainline currently). > > Would you be interested in implementing this? At least on MSM8916 there is currently no advantage implementing this. The "active-only" votes only have the CPU as limited use case. S1 (aka MSM8916_VDDCX) and L3 (MSM8916_VDDMX) are both used via rpmpd/power domains which already provides separate active-only variants. L7 (for the CPU PLL) is the only other regulator used in "active-only" mode. However, at least on MSM8916 L7 seems to stay always-on no matter what I do, so having an active-only vote on L7 doesn't provide any advantage. > Actually, I think currently all votes are active-only votes and what > we're missing is sleep-only (and active-sleep if we vote on both) If you only send the "active" votes but no "sleep" votes for a resource then the RPM firmware treats it as active+sleep, see [1]. The active/sleep separation only starts once a separate sleep vote has been sent for a resource for the first time. Therefore, all requests from the SMD regulator driver apply for both active+sleep at the moment. [1]: https://git.codelinaro.org/clo/la/kernel/msm-3.10/-/blob/LA.BR.1.2.9.1-02310-8x16.0/drivers/regulator/rpm-smd-regulator.c#L202-204 > > > > Ancient downstream defines a second device (vregname_ao) and basically > > seems to select QCOM_SMD_(ACTIVE/SLEEP)_STATE based on that.. > > > > Looks like `struct regulator` stores voltage in an array that wouldn't > > you know it, depends on the PM state. Perhaps that could be something > > to explore! > > Don't get confused by the similar naming here. RPM sleep votes are unrelated to the "system suspend" voltages the regulator framework supports. :) RPM sleep votes become active if the cpuidle reaches the deepest state for the (cpu/)cluster(/CCI). This can happen anytime at runtime when the system is idle long enough. On the other hand, the regulator suspend voltages are meant to become active during system suspend (where all the devices get suspended as well). Since we do have "active-only" support in rpmpd I think the question is if it is worth bringing the feature also to regulators. Perhaps one could simply treat all regulators that are needed by the CPU as power domain. For example, L7 on MSM8916 is fixed at 1.8V so while it doesn't have corners the simple enable/disable votes could also be sent via rpmpd. In some places in downstream L7 is also called VDDPX, similar to VDDCX and VDDMX which are already in rpmpd. Thanks, Stephan
