* Re: [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
2026-09-02 8:34 [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs() Yufan Dou
@ 2026-09-03 5:11 ` Sunil V L
2026-09-07 11:29 ` Jie Zhan
2026-09-09 7:15 ` [PATCH v2] " Yufan Dou
2 siblings, 0 replies; 7+ messages in thread
From: Sunil V L @ 2026-09-03 5:11 UTC (permalink / raw)
To: Yufan Dou
Cc: Sunil V L, Rafael J . Wysocki, Len Brown, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-acpi,
linux-riscv, linux-kernel, yuchao.46, zhanjie9, yang.yicong,
geshijian
On Wed, Sep 2, 2026 at 2:18 PM Yufan Dou <douyufan@picoheart.com> wrote:
>
> On RISC-V, each cpc_read_ffh() for a CSR-type FFH register sends a
> separate IPI to the target hart via smp_call_function_single().
> cppc_get_perf_ctrs() therefore samples the delivered and reference
> counters in two separate IPIs, at two different instants. The skew
> between the two sampling points distorts the delivered/reference
> ratio and thus the frequency reported by cpufreq.
>
> Implement the cpc_read_ffh_fb_ctrs() hook to read both CSR counters
> back-to-back in a single IPI, which narrows the sampling window to a
> few instructions and halves the number of cross-CPU calls. This
> improves the accuracy of the reported frequency in the same way as
> the arm64 implementation of the hook, which reads both AMU counters
> in a single counters_read_on_cpu() call.
>
> SBI-type FFH registers still fall back to individual reads, since
> pairing them would not reduce the number of SBI calls on the target
> hart.
>
> Testing was performed while CPU1 was kept busy with:
>
> # stress-ng --cpu 1 --taskset 1
>
> On a CPU with cpuinfo_max_freq of 3000000 kHz:
>
> Before:
>
> Maximum observed cpuinfo_cur_freq: 3201369 kHz
> Maximum observed error: +201369 kHz (+6.71%)
>
> After:
>
> Maximum observed cpuinfo_cur_freq: 3009646 kHz
> Maximum observed error: +9646 kHz (+0.32%)
>
> The maximum observed error is reduced by 95.2%.
>
> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
> ---
> drivers/acpi/riscv/cppc.c | 47 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 42c1a9052470..2ca98fa69e9a 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -85,6 +85,20 @@ static void cppc_ffh_csr_write(void *write_data)
> data->ret.error = -EINVAL;
> }
>
> +struct sbi_cppc_fb_ctrs_data {
> + struct sbi_cppc_data first;
> + struct sbi_cppc_data second;
> +};
> +
> +static void cppc_ffh_csr_read_fb_ctrs(void *read_data)
> +{
> + struct sbi_cppc_fb_ctrs_data *data = read_data;
> +
> + cppc_ffh_csr_read(&data->first);
> + if (!data->first.ret.error)
> + cppc_ffh_csr_read(&data->second);
> +}
> +
> /*
> * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
> * below.
> @@ -125,6 +139,39 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
> return -EINVAL;
> }
>
> +int cpc_read_ffh_fb_ctrs(int cpu, struct cpc_reg *reg1, u64 *val1,
> + struct cpc_reg *reg2, u64 *val2)
> +{
> + struct sbi_cppc_fb_ctrs_data data;
> + int ret;
> +
> + if (WARN_ON_ONCE(irqs_disabled()))
> + return -EPERM;
> +
> + /* Only CSR counters can be paired within a single IPI. */
> + if (FFH_CPPC_TYPE(reg1->address) != FFH_CPPC_CSR ||
> + FFH_CPPC_TYPE(reg2->address) != FFH_CPPC_CSR)
> + return -EOPNOTSUPP;
> +
>
While I agree we can not reduce the SBI calls, can we reduce the IPI at least?
> + data.first.reg = FFH_CPPC_CSR_NUM(reg1->address);
> + data.second.reg = FFH_CPPC_CSR_NUM(reg2->address);
> +
> + ret = smp_call_function_single(cpu, cppc_ffh_csr_read_fb_ctrs,
> + &data, 1);
> + if (ret)
> + return ret;
> +
> + if (data.first.ret.error)
> + return data.first.ret.error;
> + if (data.second.ret.error)
> + return data.second.ret.error;
> +
> + *val1 = data.first.ret.value;
> + *val2 = data.second.ret.value;
> +
> + return 0;
> +}
> +
> int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
> {
> struct sbi_cppc_data data;
>
> base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
> --
> 2.34.1
>
Otherwise, LGTM. Thanks!
Reviewed-by: Sunil V L <sunilvl@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
2026-09-02 8:34 [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs() Yufan Dou
2026-09-03 5:11 ` Sunil V L
@ 2026-09-07 11:29 ` Jie Zhan
2026-09-07 13:17 ` Yicong Yang
2026-09-09 7:15 ` [PATCH v2] " Yufan Dou
2 siblings, 1 reply; 7+ messages in thread
From: Jie Zhan @ 2026-09-07 11:29 UTC (permalink / raw)
To: Yufan Dou, Sunil V L, Rafael J . Wysocki, Len Brown,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-acpi, linux-riscv, linux-kernel
Cc: yuchao.46, yang.yicong, geshijian
On 9/2/2026 4:34 PM, Yufan Dou wrote:
> On RISC-V, each cpc_read_ffh() for a CSR-type FFH register sends a
> separate IPI to the target hart via smp_call_function_single().
> cppc_get_perf_ctrs() therefore samples the delivered and reference
> counters in two separate IPIs, at two different instants. The skew
> between the two sampling points distorts the delivered/reference
> ratio and thus the frequency reported by cpufreq.
>
> Implement the cpc_read_ffh_fb_ctrs() hook to read both CSR counters
> back-to-back in a single IPI, which narrows the sampling window to a
> few instructions and halves the number of cross-CPU calls. This
> improves the accuracy of the reported frequency in the same way as
> the arm64 implementation of the hook, which reads both AMU counters
> in a single counters_read_on_cpu() call.
>
> SBI-type FFH registers still fall back to individual reads, since
> pairing them would not reduce the number of SBI calls on the target
> hart.
>
> Testing was performed while CPU1 was kept busy with:
>
> # stress-ng --cpu 1 --taskset 1
>
> On a CPU with cpuinfo_max_freq of 3000000 kHz:
>
> Before:
>
> Maximum observed cpuinfo_cur_freq: 3201369 kHz
> Maximum observed error: +201369 kHz (+6.71%)
>
> After:
>
> Maximum observed cpuinfo_cur_freq: 3009646 kHz
> Maximum observed error: +9646 kHz (+0.32%)
>
> The maximum observed error is reduced by 95.2%.
Glad to see that works for RISC-V as well.
>
> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
> ---
> drivers/acpi/riscv/cppc.c | 47 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 42c1a9052470..2ca98fa69e9a 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -85,6 +85,20 @@ static void cppc_ffh_csr_write(void *write_data)
> data->ret.error = -EINVAL;
> }
>
> +struct sbi_cppc_fb_ctrs_data {
> + struct sbi_cppc_data first;
> + struct sbi_cppc_data second;
> +};
> +
> +static void cppc_ffh_csr_read_fb_ctrs(void *read_data)
> +{
> + struct sbi_cppc_fb_ctrs_data *data = read_data;
> +
> + cppc_ffh_csr_read(&data->first);
> + if (!data->first.ret.error)
> + cppc_ffh_csr_read(&data->second);
> +}
> +
> /*
> * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
> * below.
> @@ -125,6 +139,39 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
> return -EINVAL;
> }
>
> +int cpc_read_ffh_fb_ctrs(int cpu, struct cpc_reg *reg1, u64 *val1,
> + struct cpc_reg *reg2, u64 *val2)
> +{
> + struct sbi_cppc_fb_ctrs_data data;
> + int ret;
> +
> + if (WARN_ON_ONCE(irqs_disabled()))
> + return -EPERM;
AFAICS, this may cause CPPC FIE to constantly fail to read counters because
FIE runs in hardirq context.
Any test on CPPC FIE?
> +
> + /* Only CSR counters can be paired within a single IPI. */
> + if (FFH_CPPC_TYPE(reg1->address) != FFH_CPPC_CSR ||
> + FFH_CPPC_TYPE(reg2->address) != FFH_CPPC_CSR)
> + return -EOPNOTSUPP;
> +
> + data.first.reg = FFH_CPPC_CSR_NUM(reg1->address);
> + data.second.reg = FFH_CPPC_CSR_NUM(reg2->address);
> +
> + ret = smp_call_function_single(cpu, cppc_ffh_csr_read_fb_ctrs,
> + &data, 1);
> + if (ret)
> + return ret;
> +
> + if (data.first.ret.error)
> + return data.first.ret.error;
> + if (data.second.ret.error)
> + return data.second.ret.error;
> +
> + *val1 = data.first.ret.value;
> + *val2 = data.second.ret.value;
> +
> + return 0;
> +}
> +
> int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
> {
> struct sbi_cppc_data data;
>
> base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
2026-09-07 11:29 ` Jie Zhan
@ 2026-09-07 13:17 ` Yicong Yang
2026-09-07 13:38 ` Jie Zhan
0 siblings, 1 reply; 7+ messages in thread
From: Yicong Yang @ 2026-09-07 13:17 UTC (permalink / raw)
To: Jie Zhan
Cc: Yufan Dou, Sunil V L, Rafael J . Wysocki, Len Brown,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-acpi, linux-riscv, linux-kernel, yang.yicong, yuchao.46,
geshijian
On 9/7/26 7:29 PM, Jie Zhan wrote:
>
>
> On 9/2/2026 4:34 PM, Yufan Dou wrote:
>> On RISC-V, each cpc_read_ffh() for a CSR-type FFH register sends a
>> separate IPI to the target hart via smp_call_function_single().
>> cppc_get_perf_ctrs() therefore samples the delivered and reference
>> counters in two separate IPIs, at two different instants. The skew
>> between the two sampling points distorts the delivered/reference
>> ratio and thus the frequency reported by cpufreq.
>>
>> Implement the cpc_read_ffh_fb_ctrs() hook to read both CSR counters
>> back-to-back in a single IPI, which narrows the sampling window to a
>> few instructions and halves the number of cross-CPU calls. This
>> improves the accuracy of the reported frequency in the same way as
>> the arm64 implementation of the hook, which reads both AMU counters
>> in a single counters_read_on_cpu() call.
>>
>> SBI-type FFH registers still fall back to individual reads, since
>> pairing them would not reduce the number of SBI calls on the target
>> hart.
>>
>> Testing was performed while CPU1 was kept busy with:
>>
>> # stress-ng --cpu 1 --taskset 1
>>
>> On a CPU with cpuinfo_max_freq of 3000000 kHz:
>>
>> Before:
>>
>> Maximum observed cpuinfo_cur_freq: 3201369 kHz
>> Maximum observed error: +201369 kHz (+6.71%)
>>
>> After:
>>
>> Maximum observed cpuinfo_cur_freq: 3009646 kHz
>> Maximum observed error: +9646 kHz (+0.32%)
>>
>> The maximum observed error is reduced by 95.2%.
> Glad to see that works for RISC-V as well.
>>
>> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
>> ---
>> drivers/acpi/riscv/cppc.c | 47 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>>
>> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
>> index 42c1a9052470..2ca98fa69e9a 100644
>> --- a/drivers/acpi/riscv/cppc.c
>> +++ b/drivers/acpi/riscv/cppc.c
>> @@ -85,6 +85,20 @@ static void cppc_ffh_csr_write(void *write_data)
>> data->ret.error = -EINVAL;
>> }
>>
>> +struct sbi_cppc_fb_ctrs_data {
>> + struct sbi_cppc_data first;
>> + struct sbi_cppc_data second;
>> +};
>> +
>> +static void cppc_ffh_csr_read_fb_ctrs(void *read_data)
>> +{
>> + struct sbi_cppc_fb_ctrs_data *data = read_data;
>> +
>> + cppc_ffh_csr_read(&data->first);
>> + if (!data->first.ret.error)
>> + cppc_ffh_csr_read(&data->second);
>> +}
>> +
>> /*
>> * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
>> * below.
>> @@ -125,6 +139,39 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
>> return -EINVAL;
>> }
>>
>> +int cpc_read_ffh_fb_ctrs(int cpu, struct cpc_reg *reg1, u64 *val1,
>> + struct cpc_reg *reg2, u64 *val2)
>> +{
>> + struct sbi_cppc_fb_ctrs_data data;
>> + int ret;
>> +
>> + if (WARN_ON_ONCE(irqs_disabled()))
>> + return -EPERM;
> AFAICS, this may cause CPPC FIE to constantly fail to read counters because
> FIE runs in hardirq context.
I suppose it's mainly due to the smp_call_function_single() cannot work
with irqs_disabled().
seems we need to do the similar handling like arm64's
counters_read_on_cpu() - read the counters locally when irqs_disabled()
if local cpu is the target cpu.
so is cpc_read_ffh() of riscv. but I suppose this should be a fix of below
commit, as it were in the kthread for FIE...
997c021abc6e ("cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs")
>
> Any test on CPPC FIE?
>> +
>> + /* Only CSR counters can be paired within a single IPI. */
>> + if (FFH_CPPC_TYPE(reg1->address) != FFH_CPPC_CSR ||
>> + FFH_CPPC_TYPE(reg2->address) != FFH_CPPC_CSR)
>> + return -EOPNOTSUPP;
>> +
>> + data.first.reg = FFH_CPPC_CSR_NUM(reg1->address);
>> + data.second.reg = FFH_CPPC_CSR_NUM(reg2->address);
>> +
>> + ret = smp_call_function_single(cpu, cppc_ffh_csr_read_fb_ctrs,
>> + &data, 1);
>> + if (ret)
>> + return ret;
>> +
>> + if (data.first.ret.error)
>> + return data.first.ret.error;
>> + if (data.second.ret.error)
>> + return data.second.ret.error;
>> +
>> + *val1 = data.first.ret.value;
>> + *val2 = data.second.ret.value;
>> +
>> + return 0;
>> +}
>> +
>> int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
>> {
>> struct sbi_cppc_data data;
>>
>> base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
2026-09-07 13:17 ` Yicong Yang
@ 2026-09-07 13:38 ` Jie Zhan
0 siblings, 0 replies; 7+ messages in thread
From: Jie Zhan @ 2026-09-07 13:38 UTC (permalink / raw)
To: Yicong Yang
Cc: Yufan Dou, Sunil V L, Rafael J . Wysocki, Len Brown,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-acpi, linux-riscv, linux-kernel, yuchao.46, geshijian
On 9/7/2026 9:17 PM, Yicong Yang wrote:
> On 9/7/26 7:29 PM, Jie Zhan wrote:
>>
>>
>> On 9/2/2026 4:34 PM, Yufan Dou wrote:
>>> On RISC-V, each cpc_read_ffh() for a CSR-type FFH register sends a
>>> separate IPI to the target hart via smp_call_function_single().
>>> cppc_get_perf_ctrs() therefore samples the delivered and reference
>>> counters in two separate IPIs, at two different instants. The skew
>>> between the two sampling points distorts the delivered/reference
>>> ratio and thus the frequency reported by cpufreq.
>>>
>>> Implement the cpc_read_ffh_fb_ctrs() hook to read both CSR counters
>>> back-to-back in a single IPI, which narrows the sampling window to a
>>> few instructions and halves the number of cross-CPU calls. This
>>> improves the accuracy of the reported frequency in the same way as
>>> the arm64 implementation of the hook, which reads both AMU counters
>>> in a single counters_read_on_cpu() call.
>>>
>>> SBI-type FFH registers still fall back to individual reads, since
>>> pairing them would not reduce the number of SBI calls on the target
>>> hart.
>>>
>>> Testing was performed while CPU1 was kept busy with:
>>>
>>> # stress-ng --cpu 1 --taskset 1
>>>
>>> On a CPU with cpuinfo_max_freq of 3000000 kHz:
>>>
>>> Before:
>>>
>>> Maximum observed cpuinfo_cur_freq: 3201369 kHz
>>> Maximum observed error: +201369 kHz (+6.71%)
>>>
>>> After:
>>>
>>> Maximum observed cpuinfo_cur_freq: 3009646 kHz
>>> Maximum observed error: +9646 kHz (+0.32%)
>>>
>>> The maximum observed error is reduced by 95.2%.
>> Glad to see that works for RISC-V as well.
>>>
>>> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
>>> ---
>>> drivers/acpi/riscv/cppc.c | 47 +++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
>>> index 42c1a9052470..2ca98fa69e9a 100644
>>> --- a/drivers/acpi/riscv/cppc.c
>>> +++ b/drivers/acpi/riscv/cppc.c
>>> @@ -85,6 +85,20 @@ static void cppc_ffh_csr_write(void *write_data)
>>> data->ret.error = -EINVAL;
>>> }
>>>
>>> +struct sbi_cppc_fb_ctrs_data {
>>> + struct sbi_cppc_data first;
>>> + struct sbi_cppc_data second;
>>> +};
>>> +
>>> +static void cppc_ffh_csr_read_fb_ctrs(void *read_data)
>>> +{
>>> + struct sbi_cppc_fb_ctrs_data *data = read_data;
>>> +
>>> + cppc_ffh_csr_read(&data->first);
>>> + if (!data->first.ret.error)
>>> + cppc_ffh_csr_read(&data->second);
>>> +}
>>> +
>>> /*
>>> * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
>>> * below.
>>> @@ -125,6 +139,39 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
>>> return -EINVAL;
>>> }
>>>
>>> +int cpc_read_ffh_fb_ctrs(int cpu, struct cpc_reg *reg1, u64 *val1,
>>> + struct cpc_reg *reg2, u64 *val2)
>>> +{
>>> + struct sbi_cppc_fb_ctrs_data data;
>>> + int ret;
>>> +
>>> + if (WARN_ON_ONCE(irqs_disabled()))
>>> + return -EPERM;
>> AFAICS, this may cause CPPC FIE to constantly fail to read counters because
>> FIE runs in hardirq context.
>
> I suppose it's mainly due to the smp_call_function_single() cannot work
> with irqs_disabled().
>
> seems we need to do the similar handling like arm64's
> counters_read_on_cpu() - read the counters locally when irqs_disabled()
> if local cpu is the target cpu.
Exactly.
>
> so is cpc_read_ffh() of riscv. but I suppose this should be a fix of below
> commit, as it were in the kthread for FIE...
> 997c021abc6e ("cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs")
>
The fix on riscv has been posted.
https://lore.kernel.org/all/20260609192856874Y8AaPpKL5TnmbgiWI8DI3@zte.com.cn/
cpc_read_ffh_fb_ctrs() and cpc_read_ffh() both call counters_read_on_cpu()
on arm64, so fixing counters_read_on_cpu() is fine.
But the two ABIs are implemented separately here on riscv, so copy the same
fix to cpc_read_ffh_fb_ctrs() or factor out a common process.
>> Any test on CPPC FIE?
>>> +
>>> + /* Only CSR counters can be paired within a single IPI. */
>>> + if (FFH_CPPC_TYPE(reg1->address) != FFH_CPPC_CSR ||
>>> + FFH_CPPC_TYPE(reg2->address) != FFH_CPPC_CSR)
>>> + return -EOPNOTSUPP;
>>> +
>>> + data.first.reg = FFH_CPPC_CSR_NUM(reg1->address);
>>> + data.second.reg = FFH_CPPC_CSR_NUM(reg2->address);
>>> +
>>> + ret = smp_call_function_single(cpu, cppc_ffh_csr_read_fb_ctrs,
>>> + &data, 1);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (data.first.ret.error)
>>> + return data.first.ret.error;
>>> + if (data.second.ret.error)
>>> + return data.second.ret.error;
>>> +
>>> + *val1 = data.first.ret.value;
>>> + *val2 = data.second.ret.value;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
>>> {
>>> struct sbi_cppc_data data;
>>>
>>> base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
2026-09-02 8:34 [PATCH] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs() Yufan Dou
2026-09-03 5:11 ` Sunil V L
2026-09-07 11:29 ` Jie Zhan
@ 2026-09-09 7:15 ` Yufan Dou
2026-09-17 9:11 ` Jie Zhan
2 siblings, 1 reply; 7+ messages in thread
From: Yufan Dou @ 2026-09-09 7:15 UTC (permalink / raw)
To: Sunil V L, Rafael J . Wysocki, Len Brown, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-acpi,
linux-riscv, linux-kernel
Cc: yuchao.46, zhanjie9, yang.yicong, lihuisong, geshijian, wang.yechao255
On RISC-V, reading FFH feedback counters on a remote hart through
cpc_read_ffh() requires a separate IPI for each register.
cppc_get_perf_ctrs() therefore samples the delivered and reference
counters in two separate IPIs. Variation in the delay between these
reads distorts the delivered/reference ratio and thus the frequency
reported by cpufreq.
Implement cpc_read_ffh_fb_ctrs() to read both counters back-to-back
in a single callback on the target hart. For remote reads, this
reduces two IPIs to one and narrows the separation between the
counter samples. Support CSR-type, SBI-type and mixed-type pairs.
Pairing SBI reads does not reduce the number of SBI calls, but
still avoids a separate IPI for each counter.
Factor the execution-context handling into cppc_ffh_read_on_cpu(),
shared by cpc_read_ffh() and cpc_read_ffh_fb_ctrs(). CPPC frequency
invariance (FIE) reads non-PCC counters directly from the scheduler
tick, with local interrupts disabled. Unconditionally rejecting
such reads prevents the frequency scale from being updated.
Following arm64's counters_read_on_cpu(), invoke the callback
directly when interrupts are disabled and the target is the current
CPU. Reject remote reads in that context. Otherwise, use
smp_call_function_single() and propagate its return value before
accessing the callback results.
Frequency accuracy testing used CSR-type delivered and reference
counters, with CPU1 kept busy by:
# stress-ng --cpu 1 --taskset 1
On a CPU with cpuinfo_max_freq of 3000000 kHz:
Before:
Maximum observed cpuinfo_cur_freq: 3201369 kHz
Maximum observed deviation: +201369 kHz (+6.71%)
After:
Maximum observed cpuinfo_cur_freq: 3009646 kHz
Maximum observed deviation: +9646 kHz (+0.32%)
The maximum observed deviation from cpuinfo_max_freq decreased
by 95.2%.
Additional testing covered the CPPC FIE tick path and SBI-type
FFH register reads.
Co-developed-by: Yicong Yang <yang.yicong@picoheart.com>
Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
Signed-off-by: Yufan Dou <douyufan@picoheart.com>
Reviewed-by: Sunil V L <sunilvl@oss.qualcomm.com>
---
Changes in v2:
- Support SBI-type and mixed-type counter pairs in a single callback
on the target hart, reducing remote reads to one IPI.
- Factor out cppc_ffh_read_on_cpu() for both FFH read interfaces.
Allow local reads with interrupts disabled and reject remote reads
in that context, following arm64's counters_read_on_cpu().
- Propagate synchronous cross-CPU call errors in cpc_read_ffh().
- Test the CPPC FIE tick path and SBI-type FFH register reads.
v1:
https://lore.kernel.org/all/20260902083450.2348-1-douyufan@picoheart.com/
drivers/acpi/riscv/cppc.c | 102 ++++++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
index 42c1a9052470..0f580cd6c21e 100644
--- a/drivers/acpi/riscv/cppc.c
+++ b/drivers/acpi/riscv/cppc.c
@@ -85,6 +85,55 @@ static void cppc_ffh_csr_write(void *write_data)
data->ret.error = -EINVAL;
}
+struct cppc_ffh_ctr {
+ struct sbi_cppc_data data;
+ u64 type;
+};
+
+struct cppc_ffh_fb_ctrs_data {
+ struct cppc_ffh_ctr first;
+ struct cppc_ffh_ctr second;
+};
+
+static void cppc_ffh_read_fb_ctrs(void *read_data)
+{
+ struct cppc_ffh_fb_ctrs_data *data = read_data;
+
+ if (data->first.type == FFH_CPPC_SBI)
+ sbi_cppc_read(&data->first.data);
+ else
+ cppc_ffh_csr_read(&data->first.data);
+
+ if (data->second.type == FFH_CPPC_SBI)
+ sbi_cppc_read(&data->second.data);
+ else
+ cppc_ffh_csr_read(&data->second.data);
+}
+
+static int cppc_ffh_ctr_errno(const struct cppc_ffh_ctr *ctr)
+{
+ if (!ctr->data.ret.error)
+ return 0;
+
+ return ctr->type == FFH_CPPC_SBI ?
+ sbi_err_map_linux_errno(ctr->data.ret.error) :
+ ctr->data.ret.error;
+}
+
+static int cppc_ffh_read_on_cpu(int cpu, smp_call_func_t func, void *data)
+{
+ if (irqs_disabled()) {
+ /* Remote reads require IPIs, which are unsafe with IRQs disabled. */
+ if (WARN_ON_ONCE(cpu != smp_processor_id()))
+ return -EPERM;
+
+ func(data);
+ return 0;
+ }
+
+ return smp_call_function_single(cpu, func, data, 1);
+}
+
/*
* Refer to drivers/acpi/cppc_acpi.c for the description of the functions
* below.
@@ -97,9 +146,7 @@ bool cpc_ffh_supported(void)
int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
{
struct sbi_cppc_data data;
-
- if (WARN_ON_ONCE(irqs_disabled()))
- return -EPERM;
+ int ret;
if (FFH_CPPC_TYPE(reg->address) == FFH_CPPC_SBI) {
if (!cppc_ext_present)
@@ -107,7 +154,9 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
data.reg = FFH_CPPC_SBI_REG(reg->address);
- smp_call_function_single(cpu, sbi_cppc_read, &data, 1);
+ ret = cppc_ffh_read_on_cpu(cpu, sbi_cppc_read, &data);
+ if (ret)
+ return ret;
*val = data.ret.value;
@@ -115,7 +164,9 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
} else if (FFH_CPPC_TYPE(reg->address) == FFH_CPPC_CSR) {
data.reg = FFH_CPPC_CSR_NUM(reg->address);
- smp_call_function_single(cpu, cppc_ffh_csr_read, &data, 1);
+ ret = cppc_ffh_read_on_cpu(cpu, cppc_ffh_csr_read, &data);
+ if (ret)
+ return ret;
*val = data.ret.value;
@@ -125,6 +176,47 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
return -EINVAL;
}
+int cpc_read_ffh_fb_ctrs(int cpu, struct cpc_reg *reg1, u64 *val1,
+ struct cpc_reg *reg2, u64 *val2)
+{
+ struct cppc_ffh_fb_ctrs_data data;
+ int ret;
+
+ data.first.type = FFH_CPPC_TYPE(reg1->address);
+ data.second.type = FFH_CPPC_TYPE(reg2->address);
+
+ if ((data.first.type != FFH_CPPC_SBI && data.first.type != FFH_CPPC_CSR) ||
+ (data.second.type != FFH_CPPC_SBI && data.second.type != FFH_CPPC_CSR))
+ return -EINVAL;
+
+ if ((data.first.type == FFH_CPPC_SBI || data.second.type == FFH_CPPC_SBI) &&
+ !cppc_ext_present)
+ return -EINVAL;
+
+ data.first.data.reg = data.first.type == FFH_CPPC_SBI ?
+ FFH_CPPC_SBI_REG(reg1->address) :
+ FFH_CPPC_CSR_NUM(reg1->address);
+ data.second.data.reg = data.second.type == FFH_CPPC_SBI ?
+ FFH_CPPC_SBI_REG(reg2->address) :
+ FFH_CPPC_CSR_NUM(reg2->address);
+
+ ret = cppc_ffh_read_on_cpu(cpu, cppc_ffh_read_fb_ctrs, &data);
+ if (ret)
+ return ret;
+
+ ret = cppc_ffh_ctr_errno(&data.first);
+ if (ret)
+ return ret;
+ ret = cppc_ffh_ctr_errno(&data.second);
+ if (ret)
+ return ret;
+
+ *val1 = data.first.data.ret.value;
+ *val2 = data.second.data.ret.value;
+
+ return 0;
+}
+
int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
{
struct sbi_cppc_data data;
base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
2026-09-09 7:15 ` [PATCH v2] " Yufan Dou
@ 2026-09-17 9:11 ` Jie Zhan
0 siblings, 0 replies; 7+ messages in thread
From: Jie Zhan @ 2026-09-17 9:11 UTC (permalink / raw)
To: Yufan Dou, Sunil V L, Rafael J . Wysocki, Len Brown,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-acpi, linux-riscv, linux-kernel
Cc: yuchao.46, yang.yicong, lihuisong, geshijian, wang.yechao255
On 9/9/2026 3:15 PM, Yufan Dou wrote:
> On RISC-V, reading FFH feedback counters on a remote hart through
> cpc_read_ffh() requires a separate IPI for each register.
> cppc_get_perf_ctrs() therefore samples the delivered and reference
> counters in two separate IPIs. Variation in the delay between these
> reads distorts the delivered/reference ratio and thus the frequency
> reported by cpufreq.
>
> Implement cpc_read_ffh_fb_ctrs() to read both counters back-to-back
> in a single callback on the target hart. For remote reads, this
> reduces two IPIs to one and narrows the separation between the
> counter samples. Support CSR-type, SBI-type and mixed-type pairs.
> Pairing SBI reads does not reduce the number of SBI calls, but
> still avoids a separate IPI for each counter.
>
> Factor the execution-context handling into cppc_ffh_read_on_cpu(),
> shared by cpc_read_ffh() and cpc_read_ffh_fb_ctrs(). CPPC frequency
> invariance (FIE) reads non-PCC counters directly from the scheduler
> tick, with local interrupts disabled. Unconditionally rejecting
> such reads prevents the frequency scale from being updated.
>
> Following arm64's counters_read_on_cpu(), invoke the callback
> directly when interrupts are disabled and the target is the current
> CPU. Reject remote reads in that context. Otherwise, use
> smp_call_function_single() and propagate its return value before
> accessing the callback results.
>
> Frequency accuracy testing used CSR-type delivered and reference
> counters, with CPU1 kept busy by:
>
> # stress-ng --cpu 1 --taskset 1
>
> On a CPU with cpuinfo_max_freq of 3000000 kHz:
>
> Before:
>
> Maximum observed cpuinfo_cur_freq: 3201369 kHz
> Maximum observed deviation: +201369 kHz (+6.71%)
>
> After:
>
> Maximum observed cpuinfo_cur_freq: 3009646 kHz
> Maximum observed deviation: +9646 kHz (+0.32%)
>
> The maximum observed deviation from cpuinfo_max_freq decreased
> by 95.2%.
>
> Additional testing covered the CPPC FIE tick path and SBI-type
> FFH register reads.
>
> Co-developed-by: Yicong Yang <yang.yicong@picoheart.com>
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
> Reviewed-by: Sunil V L <sunilvl@oss.qualcomm.com>
LGTM, thanks!
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
> Changes in v2:
> - Support SBI-type and mixed-type counter pairs in a single callback
> on the target hart, reducing remote reads to one IPI.
> - Factor out cppc_ffh_read_on_cpu() for both FFH read interfaces.
> Allow local reads with interrupts disabled and reject remote reads
> in that context, following arm64's counters_read_on_cpu().
> - Propagate synchronous cross-CPU call errors in cpc_read_ffh().
> - Test the CPPC FIE tick path and SBI-type FFH register reads.
>
> v1:
> https://lore.kernel.org/all/20260902083450.2348-1-douyufan@picoheart.com/
>
> drivers/acpi/riscv/cppc.c | 102 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 97 insertions(+), 5 deletions(-)
>
[ ... ]
^ permalink raw reply [flat|nested] 7+ messages in thread