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

Jie Zhan
2026-04-27T03:08:55Z

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?
> 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()
> 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?
> 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.
> 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()) { …