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

[Jie Zhan]

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@xxxxxxxxxx/
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>
> ---
>  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()) {
[...]