| Message ID | 20240222135702.2005635-4-pierre.gondois@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scmi-cpufreq: Set transition_delay_us | expand |
On Thu, Feb 22, 2024 at 02:57:01PM +0100, Pierre Gondois wrote: > Make use of the newly added callbacks: > - rate_limit_get() > - fast_switch_rate_limit() > to populate policies's `transition_delay_us`, defined as the > 'Preferred average time interval between consecutive > invocations of the driver to set the frequency for this policy.' > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/cpufreq/scmi-cpufreq.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > Hi, > diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c > index 4ee23f4ebf4a..0b483bd0d3ca 100644 > --- a/drivers/cpufreq/scmi-cpufreq.c > +++ b/drivers/cpufreq/scmi-cpufreq.c > @@ -144,6 +144,29 @@ scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, > return 0; > } > > +static int > +scmi_get_rate_limit(u32 domain, bool has_fast_switch) > +{ > + int ret, rate_limit; > + > + if (has_fast_switch) { > + /* > + * Fast channels are used whenever available, > + * so use their rate_limit value if populated. > + */ > + ret = perf_ops->fast_switch_rate_limit(ph, domain, > + &rate_limit); > + if (!ret && rate_limit) > + return rate_limit; > + } > + > + ret = perf_ops->rate_limit_get(ph, domain, &rate_limit); > + if (ret) > + return 0; > + > + return rate_limit; > +} > + > static int scmi_cpufreq_init(struct cpufreq_policy *policy) > { > int ret, nr_opp, domain; > @@ -250,6 +273,9 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > policy->fast_switch_possible = > perf_ops->fast_switch_possible(ph, domain); > > + policy->transition_delay_us = > + scmi_get_rate_limit(domain, policy->fast_switch_possible); > + > return 0; > As a second thought, I have just realized that now we have 2 ops to get the rate_limit for a domain, one used in case of FCs and another in case of std messaging w/out FCs, BUT given that we always use FCs when available, AND we do not indeed have any way from perf_ops to explicitly request a set/get ops NOT to use FCs when available, does it even make sense to expose such 2 functions ? Do we need such flexibility ? Shouldn't we just expose one single rate_limit perf_ops and let the SCMI core decide what to return depending on the presence or not of the FCs for that domain ? Maybe @Sudeep thinks differently. Thanks, Cristian
diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index 4ee23f4ebf4a..0b483bd0d3ca 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -144,6 +144,29 @@ scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, return 0; } +static int +scmi_get_rate_limit(u32 domain, bool has_fast_switch) +{ + int ret, rate_limit; + + if (has_fast_switch) { + /* + * Fast channels are used whenever available, + * so use their rate_limit value if populated. + */ + ret = perf_ops->fast_switch_rate_limit(ph, domain, + &rate_limit); + if (!ret && rate_limit) + return rate_limit; + } + + ret = perf_ops->rate_limit_get(ph, domain, &rate_limit); + if (ret) + return 0; + + return rate_limit; +} + static int scmi_cpufreq_init(struct cpufreq_policy *policy) { int ret, nr_opp, domain; @@ -250,6 +273,9 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) policy->fast_switch_possible = perf_ops->fast_switch_possible(ph, domain); + policy->transition_delay_us = + scmi_get_rate_limit(domain, policy->fast_switch_possible); + return 0; out_free_opp:
Make use of the newly added callbacks: - rate_limit_get() - fast_switch_rate_limit() to populate policies's `transition_delay_us`, defined as the 'Preferred average time interval between consecutive invocations of the driver to set the frequency for this policy.' Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/cpufreq/scmi-cpufreq.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)