diff mbox series

[3/3] cpufreq: scmi: Set transition_delay_us

Message ID 20240222135702.2005635-4-pierre.gondois@arm.com
State New
Headers
Series [1/3] firmware: arm_scmi: Populate perf commands rate_limit |

Commit Message

Pierre Gondois Feb. 22, 2024, 1:57 p.m. UTC 
  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(+)

Comments

Cristian Marussi March 4, 2024, 8:15 a.m. UTC | #1 
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 mbox series

Patch

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: