Re: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable

[Dietmar Eggemann]

On 29.09.26 13:38, Ananthu C V wrote:
> Commit 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly if
> max boost is known") introduced a guard for cpuinfo max updates to only
> increase, to preserve driver-set values above the freq table maximum.
> Later commit db80ad776cd2 ("cpufreq: Remove driver default policy->min/max
> init") dropped the 'policy->max = max_freq' assignment from
> cpufreq_frequency_table_cpuinfo, leaving that one-way-guard to be the only
> path for cpuinfo max and consequently policy max to be updated, causing
> both values to be stuck at boost frequency even when boost is disabled.
> 
> Unconditionally track the highest non-boost frequency (max_base_freq)
> in the freq table. If boost frequencies are available, use the highest
> frequency from the table/max_base_freq instead of cpuinfo->max_freq to
> control boost values, so the value can decrease again when boost is
> disabled.
> 
> Fixes: db80ad776cd2 ("cpufreq: Remove driver default policy->min/max init")
> Signed-off-by: Ananthu C V <ananthu.cv@xxxxxxxxxxxxxxxx>
> ---
>  drivers/cpufreq/cpufreq.c    | 16 +++++++++++++++-
>  drivers/cpufreq/freq_table.c |  9 ++++++++-
>  include/linux/cpufreq.h      |  3 +++
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 96515880b4ac..5e6afb6f8826 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -574,6 +574,8 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)
>  
>  static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
>  {
> +	unsigned int max_freq;
> +	unsigned int max_table_freq;
>  	int ret;
>  
>  	if (policy->boost_enabled == enable)
> @@ -587,7 +589,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)
>  		return ret;
>  	}
>  
> -	ret = freq_qos_update_request(&policy->boost_freq_req, policy->cpuinfo.max_freq);
> +	max_table_freq = cpufreq_frequency_table_max(policy);
> +
> +	if (policy_has_boost_freq(policy)) {
> +		max_freq = enable ? max_table_freq : policy->cpuinfo.max_base_freq;
> +
> +		if (!max_freq)
> +			/* when the freq table contains only boost frequencies */
> +			max_freq = max_table_freq;
> +	} else {
> +		max_freq = policy->cpuinfo.max_freq;
> +	}
> +
> +	ret = freq_qos_update_request(&policy->boost_freq_req, max_freq);

This looks pretty convoluted. You have to find the correct freq argument
for freq_qos_update_request() for freq_table drivers but work around
this for e.g. the CPPC driver.

cpufreq_driver->set_boost() of e.g. the freq_table scmi-cpufreq driver
does not adjust policy->cpuinfo.max_freq for 'boost state = 0':

cpufreq_boost_set_sw() {

 cpufreq_frequency_table_cpuinfo()

  policy->cpuinfo.min_freq = min_freq

  if (policy->cpuinfo.max_freq < max_freq) <-- (*)
   policy->cpuinfo.max_freq = max_freq
}

whereas the one of the cppc_cpufreq driver does:

cppc_cpufreq_set_boost() {

 if (state)
   policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf)
 else
   policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf)
}

(*) This was added by 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq
directly if max boost is known"). With this the

policy_set_boost(policy, enable) {

 cpufreq_driver->set_boost(policy, enable)

 freq_qos_update_request(..., policy->cpuinfo.max_freq)
}

[...]