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 <[email protected]>
Signed-off-by: Lukasz Luba <[email protected]>
---
Changes:
v2:
- collected ACK from Viresh
- corrected patch description (Viresh)
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
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 <[email protected]>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> 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
>
On Tue, Aug 16, 2022 at 2:02 PM Lukasz Luba <[email protected]> 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 <[email protected]>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> Changes:
> v2:
> - collected ACK from Viresh
> - corrected patch description (Viresh)
>
>
> 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);
> --
Applied as 6.0-rc material, thanks!
On 8/23/22 19:02, Rafael J. Wysocki wrote:
> On Tue, Aug 16, 2022 at 2:02 PM Lukasz Luba <[email protected]> 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 <[email protected]>
>> Signed-off-by: Lukasz Luba <[email protected]>
>> ---
>> Changes:
>> v2:
>> - collected ACK from Viresh
>> - corrected patch description (Viresh)
>>
>>
>> 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);
>> --
>
> Applied as 6.0-rc material, thanks!
Thanks Rafael!