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

From: Zeng Heng
Date: Fri May 26 2023 - 04:59:03 EST



在 2023/5/17 16:17, Pierre Gondois 写道:
+Ionela, Sumit, Yang,

Hello Zeng,

I think solutions around related issues were suggested at:

[1] https://lore.kernel.org/all/20230418113459.12860-7-sumitg@xxxxxxxxxx/
[2] https://lore.kernel.org/all/20230328193846.8757-1-yang@xxxxxxxxxxxxxxxxxxxxxx/
[3] https://lore.kernel.org/all/ZEl1Fms%2FJmdEZsVn@xxxxxxx/

About this patch, it seems to mean that CPPC counters of CPUx are always
accessed from CPUx, even when they are not AMUs. For instance CPPC
counters could be memory mapped and accessible from any CPU.
cpu_has_amu_feat() should allow to probe if a CPU uses AMUs or not,
and [2] had an implementation using it.

Another comment about PATCH 2/2 is that if the counters are accessed
through FFH, arm64 version of cpc_read_ffh() is calling
counters_read_on_cpu(), and a comment in counters_read_on_cpu() seems
to specify the function must be called with interrupt enabled.

I think the best solution so far was the one at [3], suggested by Ionela,
but it doesn't seem to solve your issue. Indeed, it is not checked whether
the counters are AMU counters and that they must be remotely read (to
have the CPU awake),

Regards,
Pierre


Here is segment I picked from patch[3], and there is a modification is deserved to be discussed:

---------------------------------------------------------------------------

@@ -1336,8 +1361,25 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
         }
     }

-    cpc_read(cpunum, delivered_reg, &delivered);
-    cpc_read(cpunum, reference_reg, &reference);
+    ctrs.cpunum = cpunum;
+    ctrs.delivered_reg = delivered_reg;
+    ctrs.reference_reg = reference_reg;
+    ctrs.delivered = &delivered;
+    ctrs.reference = &reference;
+

+    if (CPC_IN_FFH(delivered_reg) && CPC_IN_FFH(reference_reg)) {

Here we call cpu_has_amu_feat() as precondition (like Sumit's mail mentions), which could be compatible with

any SoCs with AMU that could be accessible via sys-register and system memory both.


+        ret = smp_call_on_cpu(cpunum, cppc_get_cycle_ctrs, &ctrs, false);
+    } else {
+        if (CPC_IN_SYSTEM_MEMORY(delivered_reg) &&
+            CPC_IN_SYSTEM_MEMORY(reference_reg)) {
+            local_irq_save(flags);
+            cppc_get_cycle_ctrs(&ctrs);
+            local_irq_restore(flags);
+        } else {
+            cppc_get_cycle_ctrs(&ctrs);
+        }
+    }
+
     cpc_read(cpunum, ref_perf_reg, &ref_perf);

-----------------------------------------------------------------------------------

If there were a new version patch released, please loop me at that time.

Thanks a lot in advance.


B.R.,

Zeng Heng