Re: [PATCH v2] cpufreq: check only freq_table in __resolve_freq()

From: Kajetan Puchalski
Date: Mon Aug 22 2022 - 12:01:14 EST


On Tue, Aug 16, 2022 at 01:01:57PM +0100, Lukasz Luba wrote:
> There is no need to check if the cpufreq driver implements callback
> cpufreq_driver::target_index. The logic in the __resolve_freq uses
> the frequency table available in the policy. It doesn't matter if the
> driver provides 'target_index' or 'target' callback. It just has to
> populate the 'policy->freq_table'.
>
> Thus, check only frequency table during the frequency resolving call.
>
> Acked-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> Signed-off-by: Lukasz Luba <lukasz.luba@xxxxxxx>
> ---
> Changes:
> v2:
> - collected ACK from Viresh
> - corrected patch description (Viresh)

Just to stress how important this fix is, without this patch while
running tests on a Pixel 6 cpufreq was reporting frequencies that
were completely not aligned with the OPPs on the device and harming
performance in the process.

For instance, after applying the patch the average score in Geekbench 5
increased by over 130 (about 5%).

The average score in Speedometer 2 increased by about 6 (over 6%).

> drivers/cpufreq/cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 7820c4e74289..69b3d61852ac 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -532,7 +532,7 @@ static unsigned int __resolve_freq(struct cpufreq_policy *policy,
>
> target_freq = clamp_val(target_freq, policy->min, policy->max);
>
> - if (!cpufreq_driver->target_index)
> + if (!policy->freq_table)
> return target_freq;
>
> idx = cpufreq_frequency_table_target(policy, target_freq, relation);
> --
> 2.17.1
>