[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZeWDI3_-s8fCmVJx@pluto>
Date: Mon, 4 Mar 2024 08:15:31 +0000
From: Cristian Marussi <cristian.marussi@....com>
To: Pierre Gondois <pierre.gondois@....com>
Cc: linux-kernel@...r.kernel.org,
Christian Loehle <christian.loehle@....com>,
Ionela Voinescu <ionela.voinescu@....com>,
Sudeep Holla <sudeep.holla@....com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
linux-arm-kernel@...ts.infradead.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 3/3] cpufreq: scmi: Set transition_delay_us
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@....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
Powered by blists - more mailing lists