[PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Jeremy Linton]

CPPC uses a pair of registers cycling at different frequencies to
determine an accumulated performance level. For userspace reporting we
want to convert this to an instantaneous CPU frequency, but over short
time periods small errors caused by CPPC counter reads can cause
fairly significant reported frequency variations even when the core
CPU clock isn't changing.

Reduce this by keeping a start sample fixed and retrying the end
sample until the counter deltas are large enough to reduce short
window error, or until adjacent delivered performance estimates are
within the CPU's observed CPPC read noise floor.

To begin, resample the initial pair a small fixed number of times
looking for matching delivered performance deltas.  This reduces the
chance that a disturbed start sample anchors the rest of the
calculation.

Then look for an end sample while updating the noise floor from the
best error seen between samples.  The floor remains zero on systems
with stable feedback reads, but lets noisy systems stop early once
another retry is unlikely to improve the result.  The retry loop is
capped at 200 iterations, giving an ~20 usec explicit delay budget
derived from ndelay(100).

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 7e7f9dfb7a24..362c08def420 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -50,7 +50,7 @@ struct cppc_freq_invariance {
 static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
 static struct kthread_worker *kworker_fie;
 
-static int cppc_perf_from_fbctrs(u64 reference_perf,
+static u64 cppc_perf_from_fbctrs(u64 reference_perf,
 				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
 				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
 
@@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
 	return (u32)t1 - (u32)t0;
 }
 
-static int cppc_perf_from_fbctrs(u64 reference_perf,
+static u64 cppc_perf_from_fbctrs(u64 reference_perf,
 				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
 				 struct cppc_perf_fb_ctrs *fb_ctrs_t1)
 {
@@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
 	return (reference_perf * delta_delivered) / delta_reference;
 }
 
-static int cppc_get_perf_ctrs_sample(int cpu,
+/* CPPC read noise floor for early retry exit. */
+static DEFINE_PER_CPU(u64, err_floor);
+
+#define CPPC_SAMPLE_MAX_RETRIES	200
+
+static int cppc_get_perf_ctrs_sample(int cpu, u64 ref,
 				     struct cppc_perf_fb_ctrs *fb_ctrs_t0,
 				     struct cppc_perf_fb_ctrs *fb_ctrs_t1)
 {
 	int ret;
+	s64 last_delivered = 0;
+	u64 smallest_error = 0;
+	int tries = 0;
+	u64 min_counts = ref * 2000;
+
+	/* Two subsequent reads with the same offset avoids one off large jitter values */
+	for (int x = 0; x < 10; x++) {
+		ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
+		if (ret)
+			return ret;
+
+		ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
+		if (ret)
+			return ret;
+
+		if (last_delivered == cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1))
+			break;
+
+		last_delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
+	}
+	last_delivered = 0;
+again:
+	ndelay(100);
 
-	ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
+	ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
 	if (ret)
 		return ret;
 
-	udelay(2); /* 2usec delay between sampling */
+	/*
+	 * We want at least two significant figures, if the counts are low, then there
+	 * can be rounding errors that show up as frequency that is swinging around a few hundred
+	 * Mhz. OTOH, if the delay gets too long the clock rate can be affected.
+	 * So we want it exactly long enough to have sufficient counter turn over, and
+	 * a repeatable low error value.
+	 */
+	if ((get_delta(fb_ctrs_t1->reference, fb_ctrs_t0->reference) < min_counts) ||
+	    (get_delta(fb_ctrs_t1->delivered, fb_ctrs_t0->delivered) < min_counts)) {
+		s64 delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
+		u64 error = abs(last_delivered - delivered);
+
+		if (smallest_error == 0 || smallest_error > error)
+			smallest_error = error;
+
+		if (error > per_cpu(err_floor, cpu)) {
+			last_delivered = delivered;
+			tries++;
+			if (tries < CPPC_SAMPLE_MAX_RETRIES)
+				goto again;
+		}
+	}
 
-	return cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
+	/* compute a running error */
+	per_cpu(err_floor, cpu) = (per_cpu(err_floor, cpu) + smallest_error) / 2;
+
+	return ret;
 }
 
 static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
@@ -799,7 +851,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
 
 	cpu_data = policy->driver_data;
 
-	ret = cppc_get_perf_ctrs_sample(cpu, &fb_ctrs_t0, &fb_ctrs_t1);
+	ret = cppc_get_perf_ctrs_sample(cpu, cpu_data->perf_caps.reference_perf,
+					&fb_ctrs_t0, &fb_ctrs_t1);
+
 	if (ret) {
 		if (ret == -EFAULT)
 			/* Any of the associated CPPC regs is 0. */
-- 
2.54.0

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Breno Leitao]

On Tue, Jun 02, 2026 at 04:20:52PM -0500, Jeremy Linton wrote:
> CPPC uses a pair of registers cycling at different frequencies to
> determine an accumulated performance level. For userspace reporting we
> want to convert this to an instantaneous CPU frequency, but over short
> time periods small errors caused by CPPC counter reads can cause
> fairly significant reported frequency variations even when the core
> CPU clock isn't changing.
> 
> Reduce this by keeping a start sample fixed and retrying the end
> sample until the counter deltas are large enough to reduce short
> window error, or until adjacent delivered performance estimates are
> within the CPU's observed CPPC read noise floor.
> 
> To begin, resample the initial pair a small fixed number of times
> looking for matching delivered performance deltas.  This reduces the
> chance that a disturbed start sample anchors the rest of the
> calculation.
> 
> Then look for an end sample while updating the noise floor from the
> best error seen between samples.  The floor remains zero on systems
> with stable feedback reads, but lets noisy systems stop early once
> another retry is unlikely to improve the result.  The retry loop is
> capped at 200 iterations, giving an ~20 usec explicit delay budget
> derived from ndelay(100).
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
>  1 file changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 7e7f9dfb7a24..362c08def420 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
>  static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
>  static struct kthread_worker *kworker_fie;
>  
> -static int cppc_perf_from_fbctrs(u64 reference_perf,
> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
>  
> @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
>  	return (u32)t1 - (u32)t0;
>  }
>  
> -static int cppc_perf_from_fbctrs(u64 reference_perf,
> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>  {
> @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
>  	return (reference_perf * delta_delivered) / delta_reference;
>  }
>  
> -static int cppc_get_perf_ctrs_sample(int cpu,
> +/* CPPC read noise floor for early retry exit. */
> +static DEFINE_PER_CPU(u64, err_floor);
> +
> +#define CPPC_SAMPLE_MAX_RETRIES	200

Could the remaining tuning literals get the same treatment?
Specifically:

- the 10 initial-resample iteration count
- the 2000 multiplier in ref * 2000
- the 100 ns in ndelay(100)

Thanks
--breno

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Jeremy Linton]

Hi,

Thanks for looking at this.

On 6/3/26 5:54 AM, Breno Leitao wrote:
> On Tue, Jun 02, 2026 at 04:20:52PM -0500, Jeremy Linton wrote:
>> CPPC uses a pair of registers cycling at different frequencies to
>> determine an accumulated performance level. For userspace reporting we
>> want to convert this to an instantaneous CPU frequency, but over short
>> time periods small errors caused by CPPC counter reads can cause
>> fairly significant reported frequency variations even when the core
>> CPU clock isn't changing.
>>
>> Reduce this by keeping a start sample fixed and retrying the end
>> sample until the counter deltas are large enough to reduce short
>> window error, or until adjacent delivered performance estimates are
>> within the CPU's observed CPPC read noise floor.
>>
>> To begin, resample the initial pair a small fixed number of times
>> looking for matching delivered performance deltas.  This reduces the
>> chance that a disturbed start sample anchors the rest of the
>> calculation.
>>
>> Then look for an end sample while updating the noise floor from the
>> best error seen between samples.  The floor remains zero on systems
>> with stable feedback reads, but lets noisy systems stop early once
>> another retry is unlikely to improve the result.  The retry loop is
>> capped at 200 iterations, giving an ~20 usec explicit delay budget
>> derived from ndelay(100).
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
>>   1 file changed, 61 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 7e7f9dfb7a24..362c08def420 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
>>   static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
>>   static struct kthread_worker *kworker_fie;
>>   
>> -static int cppc_perf_from_fbctrs(u64 reference_perf,
>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
>>   
>> @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
>>   	return (u32)t1 - (u32)t0;
>>   }
>>   
>> -static int cppc_perf_from_fbctrs(u64 reference_perf,
>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>   {
>> @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
>>   	return (reference_perf * delta_delivered) / delta_reference;
>>   }
>>   
>> -static int cppc_get_perf_ctrs_sample(int cpu,
>> +/* CPPC read noise floor for early retry exit. */
>> +static DEFINE_PER_CPU(u64, err_floor);
>> +
>> +#define CPPC_SAMPLE_MAX_RETRIES	200
> 
> Could the remaining tuning literals get the same treatment?
> Specifically:
> 
> - the 10 initial-resample iteration count
> - the 2000 multiplier in ref * 2000
> - the 100 ns in ndelay(100)

Sure. A few of these were personal judgment from the platforms I tried 
it on. I had some instrumentation at the bottom which was printing loop 
counts and error values and largely I picked those values based on how 
they were behaving, or back of the evelope estimates. For example, that 
200 is afaik overkill, its usually settles down around 20 or less, which 
makes this faster than the old method on at least one platform I tried 
it on. And they are all intended to be "upper bound" exit the loop 
because something isn't working right values.


I'm interested in whether this patch stabilizes the frequency reporting 
in some of the cases I've heard people talking about.

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Breno Leitao]

On Wed, Jun 03, 2026 at 11:46:51AM -0500, Jeremy Linton wrote:
> Hi,
> 
> Thanks for looking at this.
> 
> On 6/3/26 5:54 AM, Breno Leitao wrote:
> > On Tue, Jun 02, 2026 at 04:20:52PM -0500, Jeremy Linton wrote:
> > > CPPC uses a pair of registers cycling at different frequencies to
> > > determine an accumulated performance level. For userspace reporting we
> > > want to convert this to an instantaneous CPU frequency, but over short
> > > time periods small errors caused by CPPC counter reads can cause
> > > fairly significant reported frequency variations even when the core
> > > CPU clock isn't changing.
> > > 
> > > Reduce this by keeping a start sample fixed and retrying the end
> > > sample until the counter deltas are large enough to reduce short
> > > window error, or until adjacent delivered performance estimates are
> > > within the CPU's observed CPPC read noise floor.
> > > 
> > > To begin, resample the initial pair a small fixed number of times
> > > looking for matching delivered performance deltas.  This reduces the
> > > chance that a disturbed start sample anchors the rest of the
> > > calculation.
> > > 
> > > Then look for an end sample while updating the noise floor from the
> > > best error seen between samples.  The floor remains zero on systems
> > > with stable feedback reads, but lets noisy systems stop early once
> > > another retry is unlikely to improve the result.  The retry loop is
> > > capped at 200 iterations, giving an ~20 usec explicit delay budget
> > > derived from ndelay(100).
> > > 
> > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> > > ---
> > >   drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
> > >   1 file changed, 61 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > > index 7e7f9dfb7a24..362c08def420 100644
> > > --- a/drivers/cpufreq/cppc_cpufreq.c
> > > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > > @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
> > >   static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
> > >   static struct kthread_worker *kworker_fie;
> > > -static int cppc_perf_from_fbctrs(u64 reference_perf,
> > > +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
> > >   				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
> > >   				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
> > > @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
> > >   	return (u32)t1 - (u32)t0;
> > >   }
> > > -static int cppc_perf_from_fbctrs(u64 reference_perf,
> > > +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
> > >   				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
> > >   				 struct cppc_perf_fb_ctrs *fb_ctrs_t1)
> > >   {
> > > @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
> > >   	return (reference_perf * delta_delivered) / delta_reference;
> > >   }
> > > -static int cppc_get_perf_ctrs_sample(int cpu,
> > > +/* CPPC read noise floor for early retry exit. */
> > > +static DEFINE_PER_CPU(u64, err_floor);
> > > +
> > > +#define CPPC_SAMPLE_MAX_RETRIES	200
> > 
> > Could the remaining tuning literals get the same treatment?
> > Specifically:
> > 
> > - the 10 initial-resample iteration count
> > - the 2000 multiplier in ref * 2000
> > - the 100 ns in ndelay(100)
> 
> Sure. A few of these were personal judgment from the platforms I tried it
> on. I had some instrumentation at the bottom which was printing loop counts
> and error values and largely I picked those values based on how they were
> behaving, or back of the evelope estimates. For example, that 200 is afaik
> overkill, its usually settles down around 20 or less, which makes this
> faster than the old method on at least one platform I tried it on. And they
> are all intended to be "upper bound" exit the loop because something isn't
> working right values.
> 
> 
> I'm interested in whether this patch stabilizes the frequency reporting in
> some of the cases I've heard people talking about.

Ack. I will try to get this tested tomorrow, and report it back in here.

Thanks
--breno

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Breno Leitao]

On Tue, Jun 02, 2026 at 04:20:52PM -0500, Jeremy Linton wrote:
> CPPC uses a pair of registers cycling at different frequencies to
> determine an accumulated performance level. For userspace reporting we
> want to convert this to an instantaneous CPU frequency, but over short
> time periods small errors caused by CPPC counter reads can cause
> fairly significant reported frequency variations even when the core
> CPU clock isn't changing.
> 
> Reduce this by keeping a start sample fixed and retrying the end
> sample until the counter deltas are large enough to reduce short
> window error, or until adjacent delivered performance estimates are
> within the CPU's observed CPPC read noise floor.
> 
> To begin, resample the initial pair a small fixed number of times
> looking for matching delivered performance deltas.  This reduces the
> chance that a disturbed start sample anchors the rest of the
> calculation.
> 
> Then look for an end sample while updating the noise floor from the
> best error seen between samples.  The floor remains zero on systems
> with stable feedback reads, but lets noisy systems stop early once
> another retry is unlikely to improve the result.  The retry loop is
> capped at 200 iterations, giving an ~20 usec explicit delay budget
> derived from ndelay(100).
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

Tested-by: Breno Leitao <leitao@debian.org>

I've tested this on my grace host, checking "cat
/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq" thousand times,
and the result is quite good, reducing the Std Dev by 5x, and getting
a better value.

# cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
 3339000

Baseline:
	Average: 3505050.00
	P50 (Median): 3388500.00
	Std Dev: 462607.66
	Min: 2709000.00
	Max: 4464000.00


Wth this patch:
	Average: 2393550.00
	P50 (Median): 2394000.00
	Std Dev: 68629.06
	Min: 2241000.00
	Max: 2529000.00

Thanks,
--breno

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Jie Zhan]

Hi Jeremy,

On 6/3/2026 5:20 AM, Jeremy Linton wrote:
> CPPC uses a pair of registers cycling at different frequencies to
> determine an accumulated performance level. For userspace reporting we
> want to convert this to an instantaneous CPU frequency, but over short
> time periods small errors caused by CPPC counter reads can cause
> fairly significant reported frequency variations even when the core
> CPU clock isn't changing.
> 
> Reduce this by keeping a start sample fixed and retrying the end
> sample until the counter deltas are large enough to reduce short
> window error, or until adjacent delivered performance estimates are
> within the CPU's observed CPPC read noise floor.
> 
> To begin, resample the initial pair a small fixed number of times
> looking for matching delivered performance deltas.  This reduces the
> chance that a disturbed start sample anchors the rest of the
> calculation.
> 
> Then look for an end sample while updating the noise floor from the
> best error seen between samples.  The floor remains zero on systems
> with stable feedback reads, but lets noisy systems stop early once
> another retry is unlikely to improve the result.  The retry loop is
> capped at 200 iterations, giving an ~20 usec explicit delay budget
> derived from ndelay(100).
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
>  1 file changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 7e7f9dfb7a24..362c08def420 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
>  static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
>  static struct kthread_worker *kworker_fie;
>  
> -static int cppc_perf_from_fbctrs(u64 reference_perf,
> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
>  
> @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
>  	return (u32)t1 - (u32)t0;
>  }
>  
> -static int cppc_perf_from_fbctrs(u64 reference_perf,
> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>  				 struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>  {
> @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
>  	return (reference_perf * delta_delivered) / delta_reference;
>  }
>  
> -static int cppc_get_perf_ctrs_sample(int cpu,
> +/* CPPC read noise floor for early retry exit. */
> +static DEFINE_PER_CPU(u64, err_floor);
> +
> +#define CPPC_SAMPLE_MAX_RETRIES	200
> +
> +static int cppc_get_perf_ctrs_sample(int cpu, u64 ref,
>  				     struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>  				     struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>  {
>  	int ret;
> +	s64 last_delivered = 0;
> +	u64 smallest_error = 0;
> +	int tries = 0;
> +	u64 min_counts = ref * 2000;
> +
> +	/* Two subsequent reads with the same offset avoids one off large jitter values */
> +	for (int x = 0; x < 10; x++) {
> +		ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
> +		if (ret)
> +			return ret;
> +
> +		ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
> +		if (ret)
> +			return ret;
> +
> +		if (last_delivered == cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1))
> +			break;
> +
> +		last_delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
> +	}
Actually I don't quite understand this hunk.
Is it trying to get two exactly same samples of delivered performance?
Isn't that very hard or impossible to meet?

> +	last_delivered = 0;
> +again:
> +	ndelay(100);
>  
> -	ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
> +	ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>  	if (ret)
>  		return ret;
>  
> -	udelay(2); /* 2usec delay between sampling */
> +	/*
> +	 * We want at least two significant figures, if the counts are low, then there
> +	 * can be rounding errors that show up as frequency that is swinging around a few hundred
> +	 * Mhz. OTOH, if the delay gets too long the clock rate can be affected.
> +	 * So we want it exactly long enough to have sufficient counter turn over, and
> +	 * a repeatable low error value.
> +	 */
> +	if ((get_delta(fb_ctrs_t1->reference, fb_ctrs_t0->reference) < min_counts) ||
> +	    (get_delta(fb_ctrs_t1->delivered, fb_ctrs_t0->delivered) < min_counts)) {
> +		s64 delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
> +		u64 error = abs(last_delivered - delivered);
> +
> +		if (smallest_error == 0 || smallest_error > error)
> +			smallest_error = error;
> +
> +		if (error > per_cpu(err_floor, cpu)) {
> +			last_delivered = delivered;
> +			tries++;
> +			if (tries < CPPC_SAMPLE_MAX_RETRIES)
> +				goto again;
> +		}
> +	}
>  
> -	return cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
> +	/* compute a running error */
> +	per_cpu(err_floor, cpu) = (per_cpu(err_floor, cpu) + smallest_error) / 2;
> +
> +	return ret;
Correct me if I'm wrong.  The key point is something like a self-adaptation
process to get an appropriate sampling delay for different platforms or
even each CPU?
>  }
>  
>  static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
> @@ -799,7 +851,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>  
>  	cpu_data = policy->driver_data;
>  
> -	ret = cppc_get_perf_ctrs_sample(cpu, &fb_ctrs_t0, &fb_ctrs_t1);
> +	ret = cppc_get_perf_ctrs_sample(cpu, cpu_data->perf_caps.reference_perf,
> +					&fb_ctrs_t0, &fb_ctrs_t1);
> +
>  	if (ret) {
>  		if (ret == -EFAULT)
>  			/* Any of the associated CPPC regs is 0. */

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Jeremy Linton]

Hi,

On 6/25/26 4:55 AM, Jie Zhan wrote:
> Hi Jeremy,
> 
> On 6/3/2026 5:20 AM, Jeremy Linton wrote:
>> CPPC uses a pair of registers cycling at different frequencies to
>> determine an accumulated performance level. For userspace reporting we
>> want to convert this to an instantaneous CPU frequency, but over short
>> time periods small errors caused by CPPC counter reads can cause
>> fairly significant reported frequency variations even when the core
>> CPU clock isn't changing.
>>
>> Reduce this by keeping a start sample fixed and retrying the end
>> sample until the counter deltas are large enough to reduce short
>> window error, or until adjacent delivered performance estimates are
>> within the CPU's observed CPPC read noise floor.
>>
>> To begin, resample the initial pair a small fixed number of times
>> looking for matching delivered performance deltas.  This reduces the
>> chance that a disturbed start sample anchors the rest of the
>> calculation.
>>
>> Then look for an end sample while updating the noise floor from the
>> best error seen between samples.  The floor remains zero on systems
>> with stable feedback reads, but lets noisy systems stop early once
>> another retry is unlikely to improve the result.  The retry loop is
>> capped at 200 iterations, giving an ~20 usec explicit delay budget
>> derived from ndelay(100).
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
>>   1 file changed, 61 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 7e7f9dfb7a24..362c08def420 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
>>   static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
>>   static struct kthread_worker *kworker_fie;
>>   
>> -static int cppc_perf_from_fbctrs(u64 reference_perf,
>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t1);
>>   
>> @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
>>   	return (u32)t1 - (u32)t0;
>>   }
>>   
>> -static int cppc_perf_from_fbctrs(u64 reference_perf,
>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>   				 struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>   {
>> @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
>>   	return (reference_perf * delta_delivered) / delta_reference;
>>   }
>>   
>> -static int cppc_get_perf_ctrs_sample(int cpu,
>> +/* CPPC read noise floor for early retry exit. */
>> +static DEFINE_PER_CPU(u64, err_floor);
>> +
>> +#define CPPC_SAMPLE_MAX_RETRIES	200
>> +
>> +static int cppc_get_perf_ctrs_sample(int cpu, u64 ref,
>>   				     struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>   				     struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>   {
>>   	int ret;
>> +	s64 last_delivered = 0;
>> +	u64 smallest_error = 0;
>> +	int tries = 0;
>> +	u64 min_counts = ref * 2000;
>> +
>> +	/* Two subsequent reads with the same offset avoids one off large jitter values */
>> +	for (int x = 0; x < 10; x++) {
>> +		ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (last_delivered == cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1))
>> +			break;
>> +
>> +		last_delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
>> +	}
> Actually I don't quite understand this hunk.
> Is it trying to get two exactly same samples of delivered performance?
> Isn't that very hard or impossible to meet?

Short version, yes, but it seems like its not difficult on machines with 
lowish frequency delivered/ref registers.

So we are looking for two a-b samples that are equal under the 
assumption that interrupts/whatever won't have two identical offsets and 
the CPU can sample them faster than they roll over. If that isn't the 
case then the platform probably doesn't have a lot of issues with this 
code path because the counters are running at a fairly high frequency.

If your curious, just add some debugging to print abs(last_delivered 
-cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1)) on your platform 
before last_delivered is re-assigned.

So, this exits almost right away, sometimes, the rest of the time it 
just exits after the 10 iterations and we potentially we have a little 
jitter in the start sample. Although interestingly one platform I tried 
it on the jitter settles and the abs() value there goes down towards 0 
later in the loop even when it doesn't reach it.

> 
>> +	last_delivered = 0;
>> +again:
>> +	ndelay(100);
>>   
>> -	ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
>> +	ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>   	if (ret)
>>   		return ret;
>>   
>> -	udelay(2); /* 2usec delay between sampling */
>> +	/*
>> +	 * We want at least two significant figures, if the counts are low, then there
>> +	 * can be rounding errors that show up as frequency that is swinging around a few hundred
>> +	 * Mhz. OTOH, if the delay gets too long the clock rate can be affected.
>> +	 * So we want it exactly long enough to have sufficient counter turn over, and
>> +	 * a repeatable low error value.
>> +	 */
>> +	if ((get_delta(fb_ctrs_t1->reference, fb_ctrs_t0->reference) < min_counts) ||
>> +	    (get_delta(fb_ctrs_t1->delivered, fb_ctrs_t0->delivered) < min_counts)) {
>> +		s64 delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
>> +		u64 error = abs(last_delivered - delivered);
>> +
>> +		if (smallest_error == 0 || smallest_error > error)
>> +			smallest_error = error;
>> +
>> +		if (error > per_cpu(err_floor, cpu)) {
>> +			last_delivered = delivered;
>> +			tries++;
>> +			if (tries < CPPC_SAMPLE_MAX_RETRIES)
>> +				goto again;
>> +		}
>> +	}
>>   
>> -	return cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>> +	/* compute a running error */
>> +	per_cpu(err_floor, cpu) = (per_cpu(err_floor, cpu) + smallest_error) / 2;
>> +
>> +	return ret;
> Correct me if I'm wrong.  The key point is something like a self-adaptation
> process to get an appropriate sampling delay for different platforms or
> even each CPU?

Yes, and one of the changes I've considered here was tracking what we 
think is the lowest error sample in these loops and picking that sample 
rather than the last one.

This is a bit ugly and that makes it even worse.

I was under the impression when I started this that the "CPPC: reduce 
FFH feedback-counter sampling skew on arm64" patch wasn't helping on one 
of the platforms, so I wrote this.

But I'm going to merge that one into my tree and rerun the across a 
couple platforms before reposting this one. I think they are vaguely 
complementary, but lets see.

I'm also sitting on a cpupower patch that re-exposes this on arm64 via 
that tool if the results don't look wildly wrong most of the time.


>>   }
>>   
>>   static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>> @@ -799,7 +851,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>>   
>>   	cpu_data = policy->driver_data;
>>   
>> -	ret = cppc_get_perf_ctrs_sample(cpu, &fb_ctrs_t0, &fb_ctrs_t1);
>> +	ret = cppc_get_perf_ctrs_sample(cpu, cpu_data->perf_caps.reference_perf,
>> +					&fb_ctrs_t0, &fb_ctrs_t1);
>> +
>>   	if (ret) {
>>   		if (ret == -EFAULT)
>>   			/* Any of the associated CPPC regs is 0. */

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Jie Zhan]

On 6/26/2026 4:27 AM, Jeremy Linton wrote:
> Hi,
> 
> On 6/25/26 4:55 AM, Jie Zhan wrote:
>> Hi Jeremy,
>>
>> On 6/3/2026 5:20 AM, Jeremy Linton wrote:
>>> CPPC uses a pair of registers cycling at different frequencies to
>>> determine an accumulated performance level. For userspace reporting we
>>> want to convert this to an instantaneous CPU frequency, but over short
>>> time periods small errors caused by CPPC counter reads can cause
>>> fairly significant reported frequency variations even when the core
>>> CPU clock isn't changing.
>>>
>>> Reduce this by keeping a start sample fixed and retrying the end
>>> sample until the counter deltas are large enough to reduce short
>>> window error, or until adjacent delivered performance estimates are
>>> within the CPU's observed CPPC read noise floor.
>>>
>>> To begin, resample the initial pair a small fixed number of times
>>> looking for matching delivered performance deltas.  This reduces the
>>> chance that a disturbed start sample anchors the rest of the
>>> calculation.
>>>
>>> Then look for an end sample while updating the noise floor from the
>>> best error seen between samples.  The floor remains zero on systems
>>> with stable feedback reads, but lets noisy systems stop early once
>>> another retry is unlikely to improve the result.  The retry loop is
>>> capped at 200 iterations, giving an ~20 usec explicit delay budget
>>> derived from ndelay(100).
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>>   drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
>>>   1 file changed, 61 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>> index 7e7f9dfb7a24..362c08def420 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
>>>   static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
>>>   static struct kthread_worker *kworker_fie;
>>>   -static int cppc_perf_from_fbctrs(u64 reference_perf,
>>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>>                    struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>>                    struct cppc_perf_fb_ctrs *fb_ctrs_t1);
>>>   @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
>>>       return (u32)t1 - (u32)t0;
>>>   }
>>>   -static int cppc_perf_from_fbctrs(u64 reference_perf,
>>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>>                    struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>>                    struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>>   {
>>> @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
>>>       return (reference_perf * delta_delivered) / delta_reference;
>>>   }
>>>   -static int cppc_get_perf_ctrs_sample(int cpu,
>>> +/* CPPC read noise floor for early retry exit. */
>>> +static DEFINE_PER_CPU(u64, err_floor);
>>> +
>>> +#define CPPC_SAMPLE_MAX_RETRIES    200
>>> +
>>> +static int cppc_get_perf_ctrs_sample(int cpu, u64 ref,
>>>                        struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>>                        struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>>   {
>>>       int ret;
>>> +    s64 last_delivered = 0;
>>> +    u64 smallest_error = 0;
>>> +    int tries = 0;
>>> +    u64 min_counts = ref * 2000;
>>> +
>>> +    /* Two subsequent reads with the same offset avoids one off large jitter values */
>>> +    for (int x = 0; x < 10; x++) {
>>> +        ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        if (last_delivered == cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1))
>>> +            break;
>>> +
>>> +        last_delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
>>> +    }
>> Actually I don't quite understand this hunk.
>> Is it trying to get two exactly same samples of delivered performance?
>> Isn't that very hard or impossible to meet?
> 
> Short version, yes, but it seems like its not difficult on machines with lowish frequency delivered/ref registers.
Ok, so the low resolution of a platform's CPPC performance range enlarges
error tolerance and hence the above condition can be met.

That's an unreasonable assumption.  For a platform with a high resolution
of CPPC performance, i.e. a large highest performance value (ACPI allows 32
bits), the above approach would be purely overheads I think.
> 
> So we are looking for two a-b samples that are equal under the assumption that interrupts/whatever won't have two identical offsets and the CPU can sample them faster than they roll over. If that isn't the case then the platform probably doesn't have a lot of issues with this code path because the counters are running at a fairly high frequency.
> 
> If your curious, just add some debugging to print abs(last_delivered -cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1)) on your platform before last_delivered is re-assigned.
> 
> So, this exits almost right away, sometimes, the rest of the time it just exits after the 10 iterations and we potentially we have a little jitter in the start sample. Although interestingly one platform I tried it on the jitter settles and the abs() value there goes down towards 0 later in the loop even when it doesn't reach it.
> 
Interesting, no idea what hardware jitters can make such a pattern.
>>
>>> +    last_delivered = 0;
>>> +again:
>>> +    ndelay(100);
>>>   -    ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
>>> +    ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>>       if (ret)
>>>           return ret;
>>>   -    udelay(2); /* 2usec delay between sampling */
>>> +    /*
>>> +     * We want at least two significant figures, if the counts are low, then there
>>> +     * can be rounding errors that show up as frequency that is swinging around a few hundred
>>> +     * Mhz. OTOH, if the delay gets too long the clock rate can be affected.
>>> +     * So we want it exactly long enough to have sufficient counter turn over, and
>>> +     * a repeatable low error value.
>>> +     */
>>> +    if ((get_delta(fb_ctrs_t1->reference, fb_ctrs_t0->reference) < min_counts) ||
>>> +        (get_delta(fb_ctrs_t1->delivered, fb_ctrs_t0->delivered) < min_counts)) {
>>> +        s64 delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
>>> +        u64 error = abs(last_delivered - delivered);
>>> +
>>> +        if (smallest_error == 0 || smallest_error > error)
>>> +            smallest_error = error;
>>> +
>>> +        if (error > per_cpu(err_floor, cpu)) {
>>> +            last_delivered = delivered;
>>> +            tries++;
>>> +            if (tries < CPPC_SAMPLE_MAX_RETRIES)
>>> +                goto again;
>>> +        }
>>> +    }
>>>   -    return cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>> +    /* compute a running error */
>>> +    per_cpu(err_floor, cpu) = (per_cpu(err_floor, cpu) + smallest_error) / 2;
>>> +
>>> +    return ret;
>> Correct me if I'm wrong.  The key point is something like a self-adaptation
>> process to get an appropriate sampling delay for different platforms or
>> even each CPU?
> 
> Yes, and one of the changes I've considered here was tracking what we think is the lowest error sample in these loops and picking that sample rather than the last one.
> 
> This is a bit ugly and that makes it even worse.
> 
> I was under the impression when I started this that the "CPPC: reduce FFH feedback-counter sampling skew on arm64" patch wasn't helping on one of the platforms, so I wrote this.
For quick reference: https://lore.kernel.org/all/20260410094145.4132082-1-zhangpengjie2@huawei.com/

Yeah that patch only solves the issue of platforms with FFH counters, but
that does quite well for FFH counters in my view --- less overhead (merge
CPC reads) and less error (should be <1%).

Just curious, what's the register type of feedback counters on your platform
that doesn't work?
> 
> But I'm going to merge that one into my tree and rerun the across a couple platforms before reposting this one. I think they are vaguely complementary, but lets see.
Yeah eventually we should have solutions for all register types, i.e. FFH,
system memory, PCC, etc.

Sure look forward to the new version then.
> 
> I'm also sitting on a cpupower patch that re-exposes this on arm64 via that tool if the results don't look wildly wrong most of the time.
> 
> 
>>>   }
>>>     static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>>> @@ -799,7 +851,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>>>         cpu_data = policy->driver_data;
>>>   -    ret = cppc_get_perf_ctrs_sample(cpu, &fb_ctrs_t0, &fb_ctrs_t1);
>>> +    ret = cppc_get_perf_ctrs_sample(cpu, cpu_data->perf_caps.reference_perf,
>>> +                    &fb_ctrs_t0, &fb_ctrs_t1);
>>> +
>>>       if (ret) {
>>>           if (ret == -EFAULT)
>>>               /* Any of the associated CPPC regs is 0. */
> 
> 

Re: [PATCH] cpufreq: cppc: Reduce cppc delivered perf sampling jitter

[Jeremy Linton]

Hi,

On 6/26/26 10:13 PM, Jie Zhan wrote:
> 
> 
> On 6/26/2026 4:27 AM, Jeremy Linton wrote:
>> Hi,
>>
>> On 6/25/26 4:55 AM, Jie Zhan wrote:
>>> Hi Jeremy,
>>>
>>> On 6/3/2026 5:20 AM, Jeremy Linton wrote:
>>>> CPPC uses a pair of registers cycling at different frequencies to
>>>> determine an accumulated performance level. For userspace reporting we
>>>> want to convert this to an instantaneous CPU frequency, but over short
>>>> time periods small errors caused by CPPC counter reads can cause
>>>> fairly significant reported frequency variations even when the core
>>>> CPU clock isn't changing.
>>>>
>>>> Reduce this by keeping a start sample fixed and retrying the end
>>>> sample until the counter deltas are large enough to reduce short
>>>> window error, or until adjacent delivered performance estimates are
>>>> within the CPU's observed CPPC read noise floor.
>>>>
>>>> To begin, resample the initial pair a small fixed number of times
>>>> looking for matching delivered performance deltas.  This reduces the
>>>> chance that a disturbed start sample anchors the rest of the
>>>> calculation.
>>>>
>>>> Then look for an end sample while updating the noise floor from the
>>>> best error seen between samples.  The floor remains zero on systems
>>>> with stable feedback reads, but lets noisy systems stop early once
>>>> another retry is unlikely to improve the result.  The retry loop is
>>>> capped at 200 iterations, giving an ~20 usec explicit delay budget
>>>> derived from ndelay(100).
>>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>>    drivers/cpufreq/cppc_cpufreq.c | 68 ++++++++++++++++++++++++++++++----
>>>>    1 file changed, 61 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>>> index 7e7f9dfb7a24..362c08def420 100644
>>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>>> @@ -50,7 +50,7 @@ struct cppc_freq_invariance {
>>>>    static DEFINE_PER_CPU(struct cppc_freq_invariance, cppc_freq_inv);
>>>>    static struct kthread_worker *kworker_fie;
>>>>    -static int cppc_perf_from_fbctrs(u64 reference_perf,
>>>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>>>                     struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>>>                     struct cppc_perf_fb_ctrs *fb_ctrs_t1);
>>>>    @@ -750,7 +750,7 @@ static inline u64 get_delta(u64 t1, u64 t0)
>>>>        return (u32)t1 - (u32)t0;
>>>>    }
>>>>    -static int cppc_perf_from_fbctrs(u64 reference_perf,
>>>> +static u64 cppc_perf_from_fbctrs(u64 reference_perf,
>>>>                     struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>>>                     struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>>>    {
>>>> @@ -771,19 +771,71 @@ static int cppc_perf_from_fbctrs(u64 reference_perf,
>>>>        return (reference_perf * delta_delivered) / delta_reference;
>>>>    }
>>>>    -static int cppc_get_perf_ctrs_sample(int cpu,
>>>> +/* CPPC read noise floor for early retry exit. */
>>>> +static DEFINE_PER_CPU(u64, err_floor);
>>>> +
>>>> +#define CPPC_SAMPLE_MAX_RETRIES    200
>>>> +
>>>> +static int cppc_get_perf_ctrs_sample(int cpu, u64 ref,
>>>>                         struct cppc_perf_fb_ctrs *fb_ctrs_t0,
>>>>                         struct cppc_perf_fb_ctrs *fb_ctrs_t1)
>>>>    {
>>>>        int ret;
>>>> +    s64 last_delivered = 0;
>>>> +    u64 smallest_error = 0;
>>>> +    int tries = 0;
>>>> +    u64 min_counts = ref * 2000;
>>>> +
>>>> +    /* Two subsequent reads with the same offset avoids one off large jitter values */
>>>> +    for (int x = 0; x < 10; x++) {
>>>> +        ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +
>>>> +        ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +
>>>> +        if (last_delivered == cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1))
>>>> +            break;
>>>> +
>>>> +        last_delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
>>>> +    }
>>> Actually I don't quite understand this hunk.
>>> Is it trying to get two exactly same samples of delivered performance?
>>> Isn't that very hard or impossible to meet?
>>
>> Short version, yes, but it seems like its not difficult on machines with lowish frequency delivered/ref registers.
> Ok, so the low resolution of a platform's CPPC performance range enlarges
> error tolerance and hence the above condition can be met.
> 
> That's an unreasonable assumption.  For a platform with a high resolution
> of CPPC performance, i.e. a large highest performance value (ACPI allows 32
> bits), the above approach would be purely overheads I think.

Hmmm, I'm going to reiterate and expand what I said. There seems to be 
three cases here, there is a case where the counter error seems to decay 
with repeat sampling and arrives at 0, or close before the 10th sample, 
ones where the values are hovering around single digits, so the samples 
look random and it will hit zero, usually before the 10th sample, and 
ones which are all over the place usually 10's to 100's of cycles 
different. The first two cases definitly benifit, from this loop. Adding 
https://lkml.org/lkml/2026/7/8/719 seems to make the first two cases 
more likely.

But that said, what helps everywhere is simply adding some "best sample" 
logic to the loop where the sample that is used is the one which 
reflects a minimum between last_delivered and the current sample pair.

With just that change and the above set the worst case error percent 
seems to consistently be less than a 0.2% on the sweep I just did. I've 
seen worse case sample errors but they are consistently off by a small 
but fixed amount that seems to be the result of fixed variation between 
the requested clocks and the AMU registers, and if the requested clock 
is adjusted to match (ex 2600 vs 2630Mhz) the errors fall to less than a 
percent again. I still think noisy systems could cause some largish 
errors but this set's resampling should avoid one off errors that can 
still happen with just the other one.


>>
>> So we are looking for two a-b samples that are equal under the assumption that interrupts/whatever won't have two identical offsets and the CPU can sample them faster than they roll over. If that isn't the case then the platform probably doesn't have a lot of issues with this code path because the counters are running at a fairly high frequency.
>>
>> If your curious, just add some debugging to print abs(last_delivered -cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1)) on your platform before last_delivered is re-assigned.
>>
>> So, this exits almost right away, sometimes, the rest of the time it just exits after the 10 iterations and we potentially we have a little jitter in the start sample. Although interestingly one platform I tried it on the jitter settles and the abs() value there goes down towards 0 later in the loop even when it doesn't reach it.
>>
> Interesting, no idea what hardware jitters can make such a pattern.
>>>
>>>> +    last_delivered = 0;
>>>> +again:
>>>> +    ndelay(100);
>>>>    -    ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t0);
>>>> +    ret = cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>>>        if (ret)
>>>>            return ret;
>>>>    -    udelay(2); /* 2usec delay between sampling */
>>>> +    /*
>>>> +     * We want at least two significant figures, if the counts are low, then there
>>>> +     * can be rounding errors that show up as frequency that is swinging around a few hundred
>>>> +     * Mhz. OTOH, if the delay gets too long the clock rate can be affected.
>>>> +     * So we want it exactly long enough to have sufficient counter turn over, and
>>>> +     * a repeatable low error value.
>>>> +     */
>>>> +    if ((get_delta(fb_ctrs_t1->reference, fb_ctrs_t0->reference) < min_counts) ||
>>>> +        (get_delta(fb_ctrs_t1->delivered, fb_ctrs_t0->delivered) < min_counts)) {
>>>> +        s64 delivered = cppc_perf_from_fbctrs(ref, fb_ctrs_t0, fb_ctrs_t1);
>>>> +        u64 error = abs(last_delivered - delivered);
>>>> +
>>>> +        if (smallest_error == 0 || smallest_error > error)
>>>> +            smallest_error = error;
>>>> +
>>>> +        if (error > per_cpu(err_floor, cpu)) {
>>>> +            last_delivered = delivered;
>>>> +            tries++;
>>>> +            if (tries < CPPC_SAMPLE_MAX_RETRIES)
>>>> +                goto again;
>>>> +        }
>>>> +    }
>>>>    -    return cppc_get_perf_ctrs(cpu, fb_ctrs_t1);
>>>> +    /* compute a running error */
>>>> +    per_cpu(err_floor, cpu) = (per_cpu(err_floor, cpu) + smallest_error) / 2;
>>>> +
>>>> +    return ret;
>>> Correct me if I'm wrong.  The key point is something like a self-adaptation
>>> process to get an appropriate sampling delay for different platforms or
>>> even each CPU?
>>
>> Yes, and one of the changes I've considered here was tracking what we think is the lowest error sample in these loops and picking that sample rather than the last one.
>>
>> This is a bit ugly and that makes it even worse.
>>
>> I was under the impression when I started this that the "CPPC: reduce FFH feedback-counter sampling skew on arm64" patch wasn't helping on one of the platforms, so I wrote this.
> For quick reference: https://lore.kernel.org/all/20260410094145.4132082-1-zhangpengjie2@huawei.com/
> 
> Yeah that patch only solves the issue of platforms with FFH counters, but
> that does quite well for FFH counters in my view --- less overhead (merge
> CPC reads) and less error (should be <1%).
> 
> Just curious, what's the register type of feedback counters on your platform
> that doesn't work?
>>
>> But I'm going to merge that one into my tree and rerun the across a couple platforms before reposting this one. I think they are vaguely complementary, but lets see.
> Yeah eventually we should have solutions for all register types, i.e. FFH,
> system memory, PCC, etc.
> 
> Sure look forward to the new version then.
>>
>> I'm also sitting on a cpupower patch that re-exposes this on arm64 via that tool if the results don't look wildly wrong most of the time.
>>
>>
>>>>    }
>>>>      static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>>>> @@ -799,7 +851,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>>>>          cpu_data = policy->driver_data;
>>>>    -    ret = cppc_get_perf_ctrs_sample(cpu, &fb_ctrs_t0, &fb_ctrs_t1);
>>>> +    ret = cppc_get_perf_ctrs_sample(cpu, cpu_data->perf_caps.reference_perf,
>>>> +                    &fb_ctrs_t0, &fb_ctrs_t1);
>>>> +
>>>>        if (ret) {
>>>>            if (ret == -EFAULT)
>>>>                /* Any of the associated CPPC regs is 0. */
>>
>>