Re: [PATCH v3] cpufreq: cppc: Add update_limits support for Highest Performance changes

[Pierre Gondois <pierre.gondois@arm.com>]

Hello Xueqin,

On 5/11/26 09:56, Xueqin Luo wrote:
> ACPI CPPC specification requires OSPM to re-evaluate the Highest
> Performance register when Notify(0x85) is received for a processor
> device.
>
> Implement cppc_cpufreq_update_limits() to refresh the cached
> highest_perf capability through cppc_get_highest_perf() and update
> policy->cpuinfo.max_freq accordingly.
>
> When autonomous selection mode is enabled, reprogram the runtime
> MIN_PERF/MAX_PERF envelope against the updated Highest Performance
> capability through cppc_cpufreq_set_autonomous_perf().
>
> --
> change v2->v3:
> - Fix build issues reported by kernel test robot
> - Fold cppc_cpufreq_set_autonomous_perf() refactoring into
>    this patch to keep it self-contained
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202605101404.fqz0MXIe-lkp@intel.com/
> Signed-off-by: Xueqin Luo <luoxueqin@kylinos.cn>
> ---
>   drivers/cpufreq/cppc_cpufreq.c | 89 +++++++++++++++++++++++++++++-----
>   1 file changed, 76 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 7e7f9dfb7a24..9184d5570de8 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -843,6 +843,80 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
>   	return 0;
>   }
>   
> +/**
> + * cppc_cpufreq_set_autonomous_perf - Configure performance bounds for
> + *                                     autonomous mode
> + * @policy: cpufreq policy structure
> + *
> + * When enabling autonomous selection, program MIN_PERF and MAX_PERF from
> + * current policy limits so that the platform uses the correct performance
> + * bounds immediately.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +static int cppc_cpufreq_set_autonomous_perf(struct cpufreq_policy *policy)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	u32 old_min_perf = cpu_data->perf_ctrls.min_perf;
> +	u32 old_max_perf = cpu_data->perf_ctrls.max_perf;
> +	int ret;
> +
> +	cppc_cpufreq_update_perf_limits(cpu_data, policy);
> +
> +	ret = cppc_set_perf(policy->cpu, &cpu_data->perf_ctrls);
> +	if (ret) {
> +		cpu_data->perf_ctrls.min_perf = old_min_perf;
> +		cpu_data->perf_ctrls.max_perf = old_max_perf;
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void cppc_cpufreq_update_limits(struct cpufreq_policy *policy)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	u64 prev_highest_perf;
> +	u64 highest_perf;
> +	u64 nominal_perf;
> +	int ret;
> +
> +	guard(cpufreq_policy_write)(policy);
> +
> +	prev_highest_perf = cpu_data->perf_caps.highest_perf;
> +
> +	ret = cppc_get_highest_perf(policy->cpu, &highest_perf);
> +	if (ret)
> +		return;
> +
> +	if (highest_perf == prev_highest_perf)
> +		return;
> +
> +	cpu_data->perf_caps.highest_perf = highest_perf;
> +	nominal_perf = min_t(u64, highest_perf,
> +			     cpu_data->perf_caps.nominal_perf);
> +

I sent a patch to try to remove setting policy->max in drivers
in cpufreq drivers in general [1]. IMO policy->max should be
updated through a callback handler, maybe:

refresh_frequency_limits()
\-cpufreq_set_policy()

[1] https://lore.kernel.org/all/20260511135538.522653-4-pierre.gondois@arm.com/


> +	policy->max = cppc_perf_to_khz(&cpu_data->perf_caps,
> +			policy->boost_enabled ?
> +			highest_perf : nominal_perf);
> +
There might be other things to check:

- Is the new highest_perf higher/equal than nominal_perf ?
Depending on the answer the boost capabilities/status will have to
be updated.

- IIUC it will be possible to update the nominal_perf in the next spec,
so this makes things a bit more complicated aswell.
https://lore.kernel.org/all/20260430142430.755437-3-sumitg@nvidia.com/

- Do we need to rebuild the whole topology ? Cf. highest_perf
is used as the CPU capacity in topology_init_cpu_capacity_cppc().

(maybe off topic)
- What is the relation between highest_perf and the (not yet introduced [1])
highest_freq value ? Since the highest_perf can evolve over time there
is no clear correlation between them IIUC.
https://mantis.uefi.org/mantis/view.php?id=2570

Some of the questions are more spec. related,
but this patch might be the right place to discuss them.


> +	policy->cpuinfo.max_freq = policy->max;
> +
> +	/*
> +	 * Autonomous selection mode uses MIN/MAX performance as runtime
> +	 * hardware control bounds.
> +	 *
> +	 * Re-program them when highest_perf changes.
> +	 */
> +	if (cpu_data->perf_ctrls.auto_sel)
> +		cppc_cpufreq_set_autonomous_perf(policy);
> +
> +	pr_debug("CPU%d: highest_perf updated %llu -> %llu\n",
> +		 policy->cpu,
> +		 prev_highest_perf,
> +		 highest_perf);
> +}
> +
>   static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>   {
>   	struct cppc_cpudata *cpu_data = policy->driver_data;
> @@ -885,20 +959,8 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
>   	cpu_data->perf_ctrls.auto_sel = val;
>   
>   	if (val) {
> -		u32 old_min_perf = cpu_data->perf_ctrls.min_perf;
> -		u32 old_max_perf = cpu_data->perf_ctrls.max_perf;
> -
> -		/*
> -		 * When enabling autonomous selection, program MIN_PERF and
> -		 * MAX_PERF from current policy limits so that the platform
> -		 * uses the correct performance bounds immediately.
> -		 */
> -		cppc_cpufreq_update_perf_limits(cpu_data, policy);
> -
> -		ret = cppc_set_perf(policy->cpu, &cpu_data->perf_ctrls);
> +		ret = cppc_cpufreq_set_autonomous_perf(policy);
>   		if (ret) {
> -			cpu_data->perf_ctrls.min_perf = old_min_perf;
> -			cpu_data->perf_ctrls.max_perf = old_max_perf;
>   			cppc_set_auto_sel(policy->cpu, false);
>   			cpu_data->perf_ctrls.auto_sel = false;
>   			return ret;
> @@ -1009,6 +1071,7 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
>   	.init = cppc_cpufreq_cpu_init,
>   	.exit = cppc_cpufreq_cpu_exit,
>   	.set_boost = cppc_cpufreq_set_boost,
> +	.update_limits = cppc_cpufreq_update_limits,
>   	.attr = cppc_cpufreq_attr,
>   	.name = "cppc_cpufreq",
>   };