Re: [PATCH v2 2/2] cpufreq: fix schedutil not returning to non-boost freq when boost is disabled

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

Hi Ananthu,

On 9/21/2026 6:34 PM, Ananthu C V wrote:
> Hi Zhongqiu,
> 
> On Fri, Sep 18, 2026 at 05:15:07PM +0800, Zhongqiu Han wrote:
>> Hi Ananthu,
>>
>> On 9/8/2026 4:30 PM, 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,
>>> causing cpuinfo max 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. When a freq table is available, use max_table_freq/
>>> max_base_freq instead of cpuinfo->max_freq to control boost values, so
>>> the value can decrease again when boost is disabled.
>>
>> This issue does not appear to be limited to schedutil, so the subject
>> seems too restrictive.
> 
> That makes sense, I'll update it on the next run.
> 
>>>
>>> Fixes: 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known")
>>
>> I already commented on the Fixes: tag in v1.
>> https://lore.kernel.org/all/3a20b69e-b072-4723-925f-514d8162e259@oss.qualcomm.com/#t
>>
>> IMO it should be: 6e39ba4e5a82 ("cpufreq: Add boost_freq_req QoS
>> request")
>>
>> Could you please comment on this?
> 
> The specific issue we are trying to fix is that once boost is disabled the
> frequency is not able to come down to a non boost value, which was introduced
> by the upward guard added in 538b0188da46. Consequently, if that guard is removed,
> the issue does not exist. So it makes sense to add a fixes for that specific
> commit.

I just did some testing with the SCMI driver on the SM8850 platform, and
the results matched my previous v1 comments[1]:

02/15/2021  538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly
if max boost is known") ---> Not reproduced

03/26/2026  6e39ba4e5a82 ("cpufreq: Add boost_freq_req QoS request")
---> Not reproduced

03/28/2026  db80ad776cd2 ("cpufreq: Remove driver default policy->min
/max init")  ---> Reproduced the issue

[1]: 
https://lore.kernel.org/all/3a20b69e-b072-4723-925f-514d8162e259@oss.qualcomm.com/#t

538b0188da46 made cpuinfo.max_freq increase-only, so it stays pinned at
the boost frequency once boost has been enabled. This stayed harmless
because cpufreq_frequency_table_cpuinfo() also assigned policy->max =
max_freq; directly, recomputed per boost state on every call.

6e39ba4e5a82 moved the boost QoS update to the dirty cpuinfo.max_freq,
but since that value is unchanged on disable, freq_qos_update_request()
bails out early, cpufreq_set_policy() is never invoked and the directly
written policy->max survives, so the bug remained invisible.

db80ad776cd2 then deleted policy->max = max_freq;, leaving nothing to
bring policy->max back down from the boost value that
cpufreq_set_policy() had installed when boost was enabled.

Correcting myself: from the point of view of where the issue first
becomes reproducible, the Fixes tag can be:

Fixes: db80ad776cd2 ("cpufreq: Remove driver default policy->min/max
init")


Comments are welcome.

> 
>>> Signed-off-by: Ananthu C V <ananthu.cv@oss.qualcomm.com>
>>> ---
>>>    drivers/cpufreq/cpufreq.c    | 14 +++++++++++++-
>>>    drivers/cpufreq/freq_table.c |  5 +++++
>>>    include/linux/cpufreq.h      |  1 +
>>>    3 files changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>> index 0d0df986fa3d..a13e72711597 100644
>>> --- a/drivers/cpufreq/cpufreq.c
>>> +++ b/drivers/cpufreq/cpufreq.c
>>> @@ -574,6 +574,7 @@ 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;
>>>    	int ret;
>>>    	if (policy->boost_enabled == enable)
>>> @@ -587,7 +588,18 @@ 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);
>>> +	if (policy->freq_table) {
>>
>> acpi-cpufreq has a freq table but never sets CPUFREQ_BOOST_FREQ, so
>> max_table_freq == max_base_freq == _PSS P0 here, and the real boost
>> ceiling kept in cpuinfo.max_freq is lost. It seems that the condition
>> needs to be "does the freq table list boost frequencies" rather than "is
>> there a freq table" — e.g. recorded during the table scan, the same way
>> boost_supported is derived from the flags in
>> cpufreq_table_validate_and_sort(). And then:
>>
>> if (policy->cpuinfo.boost_in_table) {
>>      xxx;
>> }
> 
> That makes sense. I think the best thing to do here will be to export/move
> policy_has_boost_freq from freq_table.c and reuse it. Comments on this are
> welcome.
> 
>>> +		max_freq = enable ? policy->cpuinfo.max_table_freq :
>>> +				    policy->cpuinfo.max_base_freq;
>>> +
>>> +		if (!max_freq)
>>> +			/* when the freq table contains only boost frequencies */
>>> +			max_freq = policy->cpuinfo.max_table_freq;
>>> +	} else {
>>> +		max_freq = policy->cpuinfo.max_freq;
>>> +	}
>>> +
>>> +	ret = freq_qos_update_request(&policy->boost_freq_req, max_freq);
>>>    	if (ret < 0) {
>>>    		policy->boost_enabled = !policy->boost_enabled;
>>>    		cpufreq_driver->set_boost(policy, policy->boost_enabled);
>>> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
>>> index 4984142dc08a..7e183e16162d 100644
>>> --- a/drivers/cpufreq/freq_table.c
>>> +++ b/drivers/cpufreq/freq_table.c
>>> @@ -34,6 +34,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
>>>    	unsigned int min_freq = ~0;
>>>    	unsigned int max_freq = 0;
>>>    	unsigned int max_table_freq = 0;
>>> +	unsigned int max_base_freq = 0;
>>>    	unsigned int freq, i;
>>>    	cpufreq_for_each_valid_entry_idx(pos, table, i) {
>>> @@ -42,6 +43,9 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
>>>    		if (freq > max_table_freq)
>>>    			max_table_freq = freq;
>>> +		if (!(pos->flags & CPUFREQ_BOOST_FREQ) && freq > max_base_freq)
>>> +			max_base_freq = freq;
>>> +
>>>    		if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
>>>    		    && (pos->flags & CPUFREQ_BOOST_FREQ))
>>>    			continue;
>>> @@ -62,6 +66,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)
>>>    		policy->cpuinfo.max_freq = max_freq;
>>>    	policy->cpuinfo.max_table_freq = max_table_freq;
>>> +	policy->cpuinfo.max_base_freq = max_base_freq;
>>>    	if (min_freq == ~0)
>>>    		return -EINVAL;
>>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>>> index 3f3b1380251a..419c71ccff7c 100644
>>> --- a/include/linux/cpufreq.h
>>> +++ b/include/linux/cpufreq.h
>>> @@ -46,6 +46,7 @@ struct cpufreq_cpuinfo {
>>>    	unsigned int		max_freq;
>>>    	unsigned int		min_freq;
>>>    	unsigned int		max_table_freq; /* Highest valid frequency in the table */
>>> +	unsigned int		max_base_freq; /* Highest non-boost frequency in the table */
>>>    	/* in 10^(-9) s = nanoseconds */
>>>    	unsigned int		transition_latency;
>>>
>>
>>
>> -- 
>> Thx and BRs,
>> Zhongqiu Han
> 
> Best,
> Ananthu


-- 
Thx and BRs,
Zhongqiu Han