2023-05-15 09:35:37

by Zeng Heng

Subject: [PATCH 1/2] cpufreq: CPPC: keep target core awake when reading its cpufreq rate

As ARM AMU's document says, all counters are subject to any changes
in clock frequency, including clock stopping caused by the WFI and WFE
instructions.

Therefore, using smp_call_function_single() to trigger target CPU to
read self's AMU counters, which ensures the counters are working
properly during calculation.

Signed-off-by: Zeng Heng <[email protected]>
---
drivers/cpufreq/cppc_cpufreq.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 022e3555407c..169af7ff9a2a 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -837,29 +837,31 @@ static int cppc_perf_from_fbctrs(struct cppc_cpudata *cpu_data,
return (reference_perf * delta_delivered) / delta_reference;
}

+static void cppc_get_perf_ctrs_smp(void *val)
+{
+ int cpu = smp_processor_id();
+ struct cppc_perf_fb_ctrs *fb_ctrs = val;
+
+ cppc_get_perf_ctrs(cpu, fb_ctrs);
+
+ udelay(2); /* 2usec delay between sampling */
+
+ cppc_get_perf_ctrs(cpu, fb_ctrs + 1);
+}
+
static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
{
- struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
+ struct cppc_perf_fb_ctrs fb_ctrs[2] = {0};
struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
struct cppc_cpudata *cpu_data = policy->driver_data;
u64 delivered_perf;
- int ret;

cpufreq_cpu_put(policy);

- ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t0);
- if (ret)
- return ret;
-
- udelay(2); /* 2usec delay between sampling */
-
- ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
- if (ret)
- return ret;
-
- delivered_perf = cppc_perf_from_fbctrs(cpu_data, &fb_ctrs_t0,
- &fb_ctrs_t1);
+ smp_call_function_single(cpu, cppc_get_perf_ctrs_smp, fb_ctrs, 1);

+ delivered_perf = cppc_perf_from_fbctrs(cpu_data, fb_ctrs,
+ fb_ctrs + 1);
return cppc_cpufreq_perf_to_khz(cpu_data, delivered_perf);
}

--
2.25.1

2023-05-15 10:47:31

by liwei (GF)

Subject: Re: [PATCH 1/2] cpufreq: CPPC: keep target core awake when reading its cpufreq rate

On 2023/5/15 18:00, Zeng Heng wrote:
> As ARM AMU's document says, all counters are subject to any changes
> in clock frequency, including clock stopping caused by the WFI and WFE
> instructions.
>
> Therefore, using smp_call_function_single() to trigger target CPU to
> read self's AMU counters, which ensures the counters are working
> properly during calculation.
>
> Signed-off-by: Zeng Heng <[email protected]>
> ---
> drivers/cpufreq/cppc_cpufreq.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 022e3555407c..169af7ff9a2a 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -837,29 +837,31 @@ static int cppc_perf_from_fbctrs(struct cppc_cpudata *cpu_data,
> return (reference_perf * delta_delivered) / delta_reference;
> }
>
> +static void cppc_get_perf_ctrs_smp(void *val)
> +{
> + int cpu = smp_processor_id();
> + struct cppc_perf_fb_ctrs *fb_ctrs = val;
> +
> + cppc_get_perf_ctrs(cpu, fb_ctrs);
> +
> + udelay(2); /* 2usec delay between sampling */
> +
> + cppc_get_perf_ctrs(cpu, fb_ctrs + 1);
> +}
> +
> static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
> {
> - struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
> + struct cppc_perf_fb_ctrs fb_ctrs[2] = {0};
> struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> struct cppc_cpudata *cpu_data = policy->driver_data;
> u64 delivered_perf;
> - int ret;
>
> cpufreq_cpu_put(policy);
>
> - ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t0);
> - if (ret)
> - return ret;
> -
> - udelay(2); /* 2usec delay between sampling */
> -
> - ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
> - if (ret)
> - return ret;
> -
> - delivered_perf = cppc_perf_from_fbctrs(cpu_data, &fb_ctrs_t0,
> - &fb_ctrs_t1);
> + smp_call_function_single(cpu, cppc_get_perf_ctrs_smp, fb_ctrs, 1);

cppc_get_perf_ctrs() may call down_write(), while the callback for smp_call_function_single()
should be non-blocking, you can use smp_call_on_cpu() instead.

>
> + delivered_perf = cppc_perf_from_fbctrs(cpu_data, fb_ctrs,
> + fb_ctrs + 1);
> return cppc_cpufreq_perf_to_khz(cpu_data, delivered_perf);
> }
>