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

[Ananthu C V <ananthu.cv@oss.qualcomm.com>]

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.

> > 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