Re: [PATCH 1/1] cpufreq: Set policy->min and max as real QoS constraints

Pierre Gondois
2026-04-30T13:42:26Z

Hello Jie,

On 4/27/26 05:08, Jie Zhan wrote:
> Hi Pierre,
>
> Thanks for updating this.
> A few questions inline.
>
> Regards,
> Jie
>
> On 4/23/2026 4:47 PM, Pierre Gondois wrote:
>> cpufreq_set_policy() will ultimately override the policy min/max
>> values written in the .init() callback through:
>> cpufreq_policy_online()
>> \-cpufreq_init_policy()
>> \-cpufreq_set_policy()
>> \-/* Set policy->min/max */
>> Thus the policy min/max values provided are only temporary.
>>
>> There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
> Just a check, you mean this is the only place that policy->min/max set by
> driver->init() may ever be actually used, right?
I think I meant that this is the only place which might use

policy->min or max between:

- cpufreq_driver->init ();  ## which set policy->min/max

- cpufreq_set_policy(); ## which reset policy->min/max

so the min/max values set by the driver .init() callback
should only be used in the above call stack.

------

This patch sets policy->min/max values with:

/*
If the driver didn't set QoS constraints, policy->min/max still
need to be set as they are used to clamp frequency requests.
*/

So even if there are other users in the meantime, they
should have correct values.

>> cpufreq_policy_online()
>> \-cpufreq_init_policy()
>> \-__cpufreq_driver_target()
>> \-cpufreq_driver->target()
> cpufreq_init_policy() doesn't seem to be involved here?
> It's supposed to be:
> cpufreq_policy_online()
> \-__cpufreq_driver_target() /* CPUFREQ_NEED_INITIAL_FREQ_CHECK branch */
> \-cpufreq_driver->target()
Yes right indeed, cpufreq_init_policy() should be removed.
>> is called. To avoid any regression, set policy->min/max in cpufreq.c
>> if the values were not initialized.
>>
>> In this patch:
>> - Setting policy->min or max value in driver .init() cb is
>> interpreted as setting a QoS constraint.
>> - Remove policy->min/max initialization in drivers if the values
>> are similar to policy->cpuinfo.min_freq/max_freq.
> Why is this necessary?
> Doing this will touch many drivers.
> Is this mainly for cleaning up? or is there any bugs if we directly take
> the existing policy->min/max initialized by drivers (mostly equal to
> cpufreq_min/max_freq) as QoS constraints?
I think I should also mention the following commit in the
message:

521223d8b3ec ("cpufreq: Fix initialization of min and max frequency QoS
requests")

Prior to that commit, drivers were setting policy->min/max and
the value was used as a QoS constraint. After that, the value
was only temporarily used. Thus this commit helps coming
back to the first behaviour.

Pengjie Zhang recently wanted to be able to set a default min frequency
for the cppc driver at [1]. So this also helps for his case.

[1]
https://lore.kernel.org/lkml/20260213100633.15413-1-zhangpengjie2@huawei.com/

>> The only drivers where these values are different are:
>> - gx-suspmod.c
>> - cppc-cpufreq.c
>> - longrun.c
>> - For the cppc-cpufreq driver, the lowest non-linear freq. is
>> used as a min QoS constraint as suggested at:
>> https://lore.kernel.org/lkml/20260213100633.15413-1-zhangpengjie2@huawei.com/
>>
>> Signed-off-by: Pierre Gondois<pierre.gondois@arm.com>
>> ---
>> drivers/cpufreq/amd-pstate.c | 16 ++++++++--------
>> drivers/cpufreq/cppc_cpufreq.c | 11 +++++++----
>> drivers/cpufreq/cpufreq-nforce2.c | 4 ++--
>> drivers/cpufreq/cpufreq.c | 19 +++++++++++++++++--
>> drivers/cpufreq/freq_table.c | 7 +++----
>> drivers/cpufreq/gx-suspmod.c | 9 +++++----
>> drivers/cpufreq/intel_pstate.c | 3 ---
>> drivers/cpufreq/pcc-cpufreq.c | 8 ++++----
>> drivers/cpufreq/pxa3xx-cpufreq.c | 4 ++--
>> drivers/cpufreq/sh-cpufreq.c | 4 ++--
>> drivers/cpufreq/virtual-cpufreq.c | 5 +----
>> 11 files changed, 51 insertions(+), 39 deletions(-)
>>
> [...]
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 44eb1b7e7fc1b..b30bfa3e27daa 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -1453,6 +1453,14 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>> cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>>
>> if (new_policy) {
>> + unsigned int min, max;
>> +
>> + /* Use policy->min/max set by the driver as QoS requests. */
>> + min = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
>> + if (policy->max)
>> + max = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
>> + else
>> + max = FREQ_QOS_MAX_DEFAULT_VALUE;
> Is it practical (free of bugs) to set the default policy->min/max to
> FREQ_QOS_MIN/MAX_DEFAULT_VALUE in cpufreq_policy_alloc(), and keep drivers
> initializing policy->min/max?
>
> Such that we may only need to change the following two lines of
> FREQ_QOS_MIN/MAX_DEFAULT_VALUE to policy->min/max in this function, without
> adding the other two trunks.

One other goal of the patch is to distinguish drivers which:
1. actually requested a QoS min/max limit
2. have set min/max values by default, but don't actually meant it.

Cf. 521223d8b3ec ("cpufreq: Fix initialization of min and max frequency
QoS requests")
Setting policy->max as a QoS constraint might result in performance
limitations, so we should try to avoid that.

FWIU, removing the default initialization of policy->min/max
must be done to handle case 2., but maybe I missed something.

>> for_each_cpu(j, policy->related_cpus) {
>> per_cpu(cpufreq_cpu_data, j) = policy;
>> add_cpu_dev_symlink(policy, j, get_cpu_device(j));
>> @@ -1469,18 +1477,25 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
>>
>> ret = freq_qos_add_request(&policy->constraints,
>> &policy->min_freq_req, FREQ_QOS_MIN,
>> - FREQ_QOS_MIN_DEFAULT_VALUE);
>> + min);
>> if (ret < 0)
>> goto out_destroy_policy;
>>
>> ret = freq_qos_add_request(&policy->constraints,
>> &policy->max_freq_req, FREQ_QOS_MAX,
>> - FREQ_QOS_MAX_DEFAULT_VALUE);
>> + max);
>> if (ret < 0)
>> goto out_destroy_policy;
>>
>> blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>> CPUFREQ_CREATE_POLICY, policy);
>> +
>> + /*
>> + * If the driver didn't set QoS constraints, policy->min/max still
>> + * need to be set as they are used to clamp frequency requests.
>> + */
>> + policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
>> + policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;
>> }
>>
>> if (cpufreq_driver->get && has_target()) {
> …