Re: [PATCH] cpufreq: CPPC: use 10ms delay instead of 2us to avoid high error

[Pierre Gondois <pierre.gondois@arm.com>]

> 
> Just 2 other comments:
> a-
> It might be interesting to change the order in which cpc registers are read
> just to see if it has an impact, but even if it has, I m not sure how this
> could be exploitable.
> Just in case, I mean doing that, but I think that b. might be better to try.
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index c51d3ccb4cca..479b55006020 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1350,8 +1350,8 @@ 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);
> +       cpc_read(cpunum, delivered_reg, &delivered);
>           cpc_read(cpunum, ref_perf_reg, &ref_perf);
>    
>           /*
> 
> b-
> In the trace that you shared, the cpc_read() calls in the fist
> cppc_get_perf_ctrs() calls seem to always take a bit more time than in the
> second cppc_get_perf_ctrs() call.
> Would it be possible to collect traces similar as above with 3 or 4 calls to
> cppc_get_perf_ctrs() instead of 2 ? It would allow to check whether in the first
> call, accessing the cpc registers takes more time than in the following calls,
> due to cache misses or other reasons.
> Ideally statistics on the result would be the best, or if you have a trace.dat
> to share containing a trace with multiple cppc_cpufreq_get_rate() calls.
> 
> Example of code where we do 4 calls to cppc_get_perf_ctrs():
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 022e3555407c..6370f2f0bdad 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -853,6 +853,20 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>    
>           udelay(2); /* 2usec delay between sampling */
>    
> +       ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
> +       if (ret)
> +               return ret;
> +
> +       udelay(2); /* 2usec delay between sampling */
> +
> +       /* Do a third call. */
> +       ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
> +       if (ret)
> +               return ret;
> +
> +       udelay(2); /* 2usec delay between sampling */
> +
> +       /* Do a fourth call. */
>           ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
>           if (ret)
>                   return ret;

And also, if the cpc_read() calls in the third/fourth call are actually faster,
would it be possible to check whether the computed frequency is more accurate
(i.e. no over/undershoot) ?