Re: [PATCH 2/2] arm64: topology: read CPPC FFH feedback counters in one operation

[Sumit Gupta <sumitg@nvidia.com>]

On 10/04/26 15:11, Pengjie Zhang wrote:
> External email: Use caution opening links or attachments
>
>
> arm64 implements CPPC FFH feedback-counter reads using AMU counters.
> Because those counters must be sampled on the target CPU, reading the
> delivered and reference counters separately widens the observation window
> between them.
>
> Implement the paired FFH feedback-counter read hook on arm64 and sample
> both AMU counters together before decoding the requested CPC register
> values.
>
> Also factor the FFH bitfield extraction logic into a helper and reuse
> it from the existing single-counter FFH read path.
>
> Signed-off-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> ---
>   arch/arm64/kernel/topology.c | 75 ++++++++++++++++++++++++++++++++----
>   1 file changed, 67 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index b32f13358fbb..b90a767b2a1f 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -50,6 +50,16 @@ struct amu_cntr_sample {
>          unsigned long   last_scale_update;
>   };
>
> +struct amu_ffh_ctrs {
> +       u64 corecnt;
> +       u64 constcnt;
> +};
> +
> +enum cpc_ffh_ctr_id {
> +       CPC_FFH_CTR_CORE  = 0x0,
> +       CPC_FFH_CTR_CONST = 0x1,
> +};
> +
>   static DEFINE_PER_CPU_SHARED_ALIGNED(struct amu_cntr_sample, cpu_amu_samples);
>
>   void update_freq_counters_refs(void)
> @@ -397,7 +407,7 @@ static void cpu_read_constcnt(void *val)
>   }
>
>   static inline
> -int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
> +int counters_read_on_cpu(int cpu, smp_call_func_t func, void *val)
>   {
>          /*
>           * Abort call on counterless CPU.
> @@ -447,24 +457,73 @@ bool cpc_ffh_supported(void)
>          return true;
>   }
>
> +static void amu_read_core_const_ctrs(void *val)
> +{
> +       struct amu_ffh_ctrs *ctrs = val;
> +
> +       cpu_read_constcnt(&ctrs->constcnt);
> +       cpu_read_corecnt(&ctrs->corecnt);
> +}

Any reason to flip the order?
Harmless as they are read back to back, but better to add a comment
if it's intentional.

Thanks,
Sumit
....