Re: [PATCH 2/3] cpufreq: Avoid redundant target() calls for unchanged limits

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

On 5/21/2026 7:39 PM, Viresh Kumar wrote:
> Drivers setting CPUFREQ_NEED_UPDATE_LIMITS expect target() to be
> invoked even if the target frequency remains unchanged, so they can
> update their internal policy limits state.
> 
> Currently the core invokes target() unconditionally whenever the
> requested frequency matches policy->cur for such drivers, even if
> policy->min and policy->max haven't changed since the previous update.
> 
> Track pending policy limit updates explicitly and skip redundant
> target() invocations when neither the target frequency nor the
> effective limits changed.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/cpufreq/cpufreq.c      | 31 ++++++++++++++++++++++---------
>   drivers/cpufreq/intel_pstate.c |  1 +
>   include/linux/cpufreq.h        |  3 +++
>   3 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 44eb1b7e7fc1..225228e9f4ed 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2366,9 +2366,13 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
>   	 * exactly same freq is called again and so we can save on few function
>   	 * calls.
>   	 */
> -	if (target_freq == policy->cur &&
> -	    !(cpufreq_driver->flags & CPUFREQ_NEED_UPDATE_LIMITS))
> -		return 0;
> +	if (target_freq == policy->cur) {
> +		if (!(cpufreq_driver->flags & CPUFREQ_NEED_UPDATE_LIMITS) ||
> +		    !policy->update_limits)
> +			return 0;
> +
> +		policy->update_limits = false;
> +	}
>   
>   	if (cpufreq_driver->target) {
>   		/*
> @@ -2620,6 +2624,7 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>   {
>   	struct cpufreq_policy_data new_data;
>   	struct cpufreq_governor *old_gov;
> +	unsigned int freq;
>   	int ret;
>   
>   	memcpy(&new_data.cpuinfo, &policy->cpuinfo, sizeof(policy->cpuinfo));
> @@ -2652,12 +2657,20 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>   	 * compiler optimizations around them because they may be accessed
>   	 * concurrently by cpufreq_driver_resolve_freq() during the update.
>   	 */
> -	WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
> -					       new_data.min, new_data.max,
> -					       CPUFREQ_RELATION_H));
> -	new_data.min = __resolve_freq(policy, new_data.min, new_data.min,
> -				      new_data.max, CPUFREQ_RELATION_L);
> -	WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
> +	freq = __resolve_freq(policy, new_data.max, new_data.min, new_data.max,
> +			      CPUFREQ_RELATION_H);
> +	if (freq != policy->max) {
> +		WRITE_ONCE(policy->max, freq);
> +		policy->update_limits = true;
> +	}
> +
> +	freq = __resolve_freq(policy, new_data.min, new_data.min, new_data.max,
> +			      CPUFREQ_RELATION_L);
> +	freq = min(freq, policy->max);
> +	if (freq != policy->min) {
> +		WRITE_ONCE(policy->min, freq);
> +		policy->update_limits = true;
> +	}
>   
>   	trace_cpu_frequency_limits(policy);
>   
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 1292da53e5fc..7f19ec6b70ba 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2942,6 +2942,7 @@ static void intel_pstate_adjust_policy_max(struct cpudata *cpu,
>   	    policy->max > cpu->pstate.max_freq) {
>   		pr_debug("policy->max > max non turbo frequency\n");
>   		policy->max = policy->cpuinfo.max_freq;
> +		policy->update_limits = true;


This patch looks good to me overall.

Just one concern:

Could this be redundant?

It seems that intel_pstate_adjust_policy_max() is only triggered when
!hwp_active, whereas CPUFREQ_NEED_UPDATE_LIMITS is set when
hwp_active is enabled. If so, the update_limits flag set here may not
actually be consumed, as __cpufreq_driver_target() would skip the
update_limits check for drivers without CPUFREQ_NEED_UPDATE_LIMITS.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/intel_pstate.c#n3808

Also, it looks like intel_pstate_adjust_policy_max() uses struct
cpufreq_policy_data rather than cpufreq_policy.


>   	}
>   }
>   
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 4d4b4ed24b30..ae9d1ce4f49c 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -146,6 +146,9 @@ struct cpufreq_policy {
>   	/* Per policy boost supported flag. */
>   	bool			boost_supported;
>   
> +	/* Pending policy->min/max update for the driver */
> +	bool			update_limits;
> +
>   	 /* Cached frequency lookup from cpufreq_driver_resolve_freq. */
>   	unsigned int cached_target_freq;
>   	unsigned int cached_resolved_idx;


-- 
Thx and BRs,
Zhongqiu Han