| Message ID | 20230713141738.23970-10-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | arm_scmi/cpufreq: Add generic performance scaling support | expand |
On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > The performance domain-id can be described in DT using the power-domains > property or the clock property. The latter is already supported, so let's > add support for the power-domains too. > How is this supposed to work for the CPUs ? The CPU power domains are generally PSCI on most of the platforms and the one using OSI explicitly need to specify the details while ones using PC will not need to. Also they can never be performance domains too. So I am not sure if I am following this correctly. -- Regards, Sudeep
On Wed, 19 Jul 2023 at 17:24, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > > The performance domain-id can be described in DT using the power-domains > > property or the clock property. The latter is already supported, so let's > > add support for the power-domains too. > > > > How is this supposed to work for the CPUs ? The CPU power domains are > generally PSCI on most of the platforms and the one using OSI explicitly > need to specify the details while ones using PC will not need to. Also they > can never be performance domains too. So I am not sure if I am following this > correctly. Your concerns are certainly correct, I completely forgot about this. We need to specify what power-domain index belongs to what, by using power-domain-names in DT. So a CPU node, that has both psci for power and scmi for performance would then typically look like this: power-domains = <&CPU_PD0>, <&scmi_dvfs 4>; power-domain-names = "psci", "scmi"; I will take care of this in the next version - and thanks a lot for pointing this out! Kind regards Uffe
On Fri, Jul 21, 2023 at 01:52:17PM +0200, Ulf Hansson wrote: > On Wed, 19 Jul 2023 at 17:24, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > > > The performance domain-id can be described in DT using the power-domains > > > property or the clock property. The latter is already supported, so let's > > > add support for the power-domains too. > > > > > > > How is this supposed to work for the CPUs ? The CPU power domains are > > generally PSCI on most of the platforms and the one using OSI explicitly > > need to specify the details while ones using PC will not need to. Also they > > can never be performance domains too. So I am not sure if I am following this > > correctly. > > Your concerns are certainly correct, I completely forgot about this. > We need to specify what power-domain index belongs to what, by using > power-domain-names in DT. So a CPU node, that has both psci for power > and scmi for performance would then typically look like this: > > power-domains = <&CPU_PD0>, <&scmi_dvfs 4>; > power-domain-names = "psci", "scmi"; > > I will take care of this in the next version - and thanks a lot for > pointing this out! Yes something like this will work. Just curious will this impact the idle paths ? By that I mean will the presence of additional domains add more work or will they be skipped as early as possible with just one additional check ?
On Fri, Jul 21, 2023 at 01:52:17PM +0200, Ulf Hansson wrote: > On Wed, 19 Jul 2023 at 17:24, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > > > The performance domain-id can be described in DT using the power-domains > > > property or the clock property. The latter is already supported, so let's > > > add support for the power-domains too. > > > > > > > How is this supposed to work for the CPUs ? The CPU power domains are > > generally PSCI on most of the platforms and the one using OSI explicitly > > need to specify the details while ones using PC will not need to. Also they > > can never be performance domains too. So I am not sure if I am following this > > correctly. > > Your concerns are certainly correct, I completely forgot about this. > We need to specify what power-domain index belongs to what, by using > power-domain-names in DT. So a CPU node, that has both psci for power > and scmi for performance would then typically look like this: > > power-domains = <&CPU_PD0>, <&scmi_dvfs 4>; > power-domain-names = "psci", "scmi"; That is completely backwards. Entries are named based on the consumer side. The function of each clock or interrupt for example. Here your entries are based on the provider which should be opaque to the consumer. Rob
On Fri, 21 Jul 2023 at 16:37, Rob Herring <robh@kernel.org> wrote: > > On Fri, Jul 21, 2023 at 01:52:17PM +0200, Ulf Hansson wrote: > > On Wed, 19 Jul 2023 at 17:24, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > > > > The performance domain-id can be described in DT using the power-domains > > > > property or the clock property. The latter is already supported, so let's > > > > add support for the power-domains too. > > > > > > > > > > How is this supposed to work for the CPUs ? The CPU power domains are > > > generally PSCI on most of the platforms and the one using OSI explicitly > > > need to specify the details while ones using PC will not need to. Also they > > > can never be performance domains too. So I am not sure if I am following this > > > correctly. > > > > Your concerns are certainly correct, I completely forgot about this. > > We need to specify what power-domain index belongs to what, by using > > power-domain-names in DT. So a CPU node, that has both psci for power > > and scmi for performance would then typically look like this: > > > > power-domains = <&CPU_PD0>, <&scmi_dvfs 4>; > > power-domain-names = "psci", "scmi"; > > That is completely backwards. Entries are named based on the consumer > side. The function of each clock or interrupt for example. Here your > entries are based on the provider which should be opaque to the > consumer. Okay, so you would rather prefer something along the lines of the below? power-domain-names = "power", "perf"; The "psci" name is already part of the current cpus DT binding (Documentation/devicetree/bindings/arm/cpus.yaml), so then it looks like that deserves an update too. Right? Kind regards Uffe
On Fri, 21 Jul 2023 at 13:59, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Fri, Jul 21, 2023 at 01:52:17PM +0200, Ulf Hansson wrote: > > On Wed, 19 Jul 2023 at 17:24, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > > > > The performance domain-id can be described in DT using the power-domains > > > > property or the clock property. The latter is already supported, so let's > > > > add support for the power-domains too. > > > > > > > > > > How is this supposed to work for the CPUs ? The CPU power domains are > > > generally PSCI on most of the platforms and the one using OSI explicitly > > > need to specify the details while ones using PC will not need to. Also they > > > can never be performance domains too. So I am not sure if I am following this > > > correctly. > > > > Your concerns are certainly correct, I completely forgot about this. > > We need to specify what power-domain index belongs to what, by using > > power-domain-names in DT. So a CPU node, that has both psci for power > > and scmi for performance would then typically look like this: > > > > power-domains = <&CPU_PD0>, <&scmi_dvfs 4>; > > power-domain-names = "psci", "scmi"; > > > > I will take care of this in the next version - and thanks a lot for > > pointing this out! > > > Yes something like this will work. Just curious will this impact the idle > paths ? By that I mean will the presence of additional domains add more > work or will they be skipped as early as possible with just one additional > check ? Unless I misunderstand your concern, I don't think there is any impact on the idle path whatsoever. This should be entirely orthogonal. The scmi-cpufreq driver should only have to care about the scmi-performance domain, while the cpuidle-psci driver cares only about psci. Did that make sense? Kind regards Uffe
On Wed, 26 Jul 2023 at 13:20, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Fri, 21 Jul 2023 at 16:37, Rob Herring <robh@kernel.org> wrote: > > > > On Fri, Jul 21, 2023 at 01:52:17PM +0200, Ulf Hansson wrote: > > > On Wed, 19 Jul 2023 at 17:24, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > > > On Thu, Jul 13, 2023 at 04:17:36PM +0200, Ulf Hansson wrote: > > > > > The performance domain-id can be described in DT using the power-domains > > > > > property or the clock property. The latter is already supported, so let's > > > > > add support for the power-domains too. > > > > > > > > > > > > > How is this supposed to work for the CPUs ? The CPU power domains are > > > > generally PSCI on most of the platforms and the one using OSI explicitly > > > > need to specify the details while ones using PC will not need to. Also they > > > > can never be performance domains too. So I am not sure if I am following this > > > > correctly. > > > > > > Your concerns are certainly correct, I completely forgot about this. > > > We need to specify what power-domain index belongs to what, by using > > > power-domain-names in DT. So a CPU node, that has both psci for power > > > and scmi for performance would then typically look like this: > > > > > > power-domains = <&CPU_PD0>, <&scmi_dvfs 4>; > > > power-domain-names = "psci", "scmi"; > > > > That is completely backwards. Entries are named based on the consumer > > side. The function of each clock or interrupt for example. Here your > > entries are based on the provider which should be opaque to the > > consumer. > > Okay, so you would rather prefer something along the lines of the below? > > power-domain-names = "power", "perf"; > > The "psci" name is already part of the current cpus DT binding > (Documentation/devicetree/bindings/arm/cpus.yaml), so then it looks > like that deserves an update too. Right? Rob, may I have your thoughts around this? Is this an acceptable way forward? Please advise me on what power-domain-names I should use then. Or, if you are insisting on using the performance-domain bindings? Kind regards Uffe
diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index 78f53e388094..b42f43d9bd89 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -72,13 +72,19 @@ static unsigned int scmi_cpufreq_fast_switch(struct cpufreq_policy *policy, static int scmi_cpu_domain_id(struct device *cpu_dev) { - struct of_phandle_args clkspec; + struct of_phandle_args domain_id; if (of_parse_phandle_with_args(cpu_dev->of_node, "clocks", - "#clock-cells", 0, &clkspec)) - return -EINVAL; + "#clock-cells", 0, &domain_id)) { - return clkspec.args[0]; + if (of_parse_phandle_with_args(cpu_dev->of_node, + "power-domains", + "#power-domain-cells", 0, + &domain_id)) + return -EINVAL; + } + + return domain_id.args[0]; } static int
The performance domain-id can be described in DT using the power-domains property or the clock property. The latter is already supported, so let's add support for the power-domains too. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- Changes in v2: - New patch. --- drivers/cpufreq/scmi-cpufreq.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)