On Thu, Oct 26, 2023 at 10:24:54AM +0800, Zeng Heng wrote:
在 2023/10/25 19:13, Sudeep Holla 写道:Well that's true but this change doesn't belong to cppc_cpufreq.c, it must
On Wed, Oct 25, 2023 at 05:38:46PM +0800, Zeng Heng wrote:According to drivers/cpufreq/Makefile, cppc_cpufreq.c is only compiled with
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_on_cpu() to trigger target CPU to
read self's AMU counters, which ensures the counters are working
properly while cstate feature is enabled.
Reported-by: Sumit Gupta <sumitg@xxxxxxxxxx>
Link: https://lore.kernel.org/all/20230418113459.12860-7-sumitg@xxxxxxxxxx/
Signed-off-by: Zeng Heng <zengheng4@xxxxxxxxxx>
---
drivers/cpufreq/cppc_cpufreq.c | 39 ++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index fe08ca419b3d..321a9dc9484d 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -850,18 +871,18 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)Have you compiled this on x86 ? Even if you have somehow managed to,
cpufreq_cpu_put(policy);
- ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t0);
- if (ret)
- return 0;
-
- udelay(2); /* 2usec delay between sampling */
+ if (cpu_has_amu_feat(cpu))
this is not the right place to check the presence of AMU feature on
the CPU.
If AMU registers are used in CPPC, they must be using FFH GAS, in which
case the interpretation of FFH is architecture dependent code.
ARM architecture.
be part of drivers/acpi/cppc_acpi.c IMO and sorry I assumed that without
explicitly mentioning that here.
But here, I would change cpu_has_amu_feat() with cpc_ffh_supported(), whichIt is not like that. cppc_acpi.c will know the GAS is FFH based so no need to
belongs to FFH APIs.
check anything there. I see counters_read_on_cpu() called from cpc_ffh_read()
already takes care of reading the AMUs on the right CPU. What exactly is
the issue you are seeing ? I don't if this change is needed at all.
--
Regards,
Sudeep