Re: [PATCH v7 2/3] cpufreq: CPPC: Add ospm_nominal_freq attribute

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

On 8/7/26 23:48, Sumit Gupta wrote:
> OSPM Nominal Performance (ACPI 6.6, Section 8.4.6.1.2.6) lets the OS
> request a nominal performance level below the platform-reported one. The
> platform treats performance above that level as boosted and below it as
> throttled for its power and thermal decisions. A lower value moves that
> boundary down, so sustained work runs at a lower point while the range
> above it remains available as boost.
>
> Expose it as a per-policy cpufreq attribute in kHz, matching the unit
> convention of the other frequency attributes:
>
>    /sys/devices/system/cpu/cpuX/cpufreq/ospm_nominal_freq
>
> The attribute is write-only as the register cannot be read back. Writes
> are converted with cppc_khz_to_perf() and rejected unless they fall in
> [Lowest Performance, Nominal Performance].
>
> Also track the register in the OSPM-set save/restore table, so a
> requested value survives CPU hotplug and suspend/resume. The store
> handler records the request rather than init() capturing a firmware
> value, and driver unload reverts the register to the platform-reported
> Nominal Performance.
>
> Signed-off-by: Sumit Gupta<sumitg@nvidia.com>
> ---
>   .../ABI/testing/sysfs-devices-system-cpu      | 24 +++++++
>   drivers/cpufreq/cppc_cpufreq.c                | 70 +++++++++++++++++--
>   2 files changed, 90 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> index 82d10d556cc8..59aafcb2af97 100644
> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> @@ -346,6 +346,30 @@ Description:	Performance Limited
>   
>   		This file is only present if the cppc-cpufreq driver is in use.
>   
> +What:		/sys/devices/system/cpu/cpuX/cpufreq/ospm_nominal_freq
> +Date:		August 2026
> +Contact:	linux-pm@vger.kernel.org
> +Description:	OSPM Nominal Performance (kHz), write-only
> +
> +		OSPM uses this attribute to request a nominal performance level
> +		lower than the platform-reported nominal. The platform treats
> +		performance above this level as boost and below as throttle for
> +		power and thermal decisions.
> +
> +		Write a value in kHz, between the frequencies corresponding to
> +		Lowest Performance and Nominal Performance. The register cannot
> +		be read back, so this attribute has no read side.
> +
> +		Note that tasks may be migrated from one CPU to another by the
> +		scheduler's load-balancing algorithm, and if different OSPM
> +		Nominal Performance values are set for those CPUs (through
> +		different cpufreq policies), that may lead to undesirable
> +		outcomes. To avoid such issues it is better to set the same
> +		value across all policies, or to pin every task potentially
> +		sensitive to it to a specific CPU.
> +
> +		This file is only present if the cppc-cpufreq driver is in use.
> +
>   What:		/sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
>   Date:		August 2008
>   KernelVersion:	2.6.27
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 32f38b0c492b..fe714e71826a 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -33,11 +33,13 @@ static struct cpufreq_driver cppc_cpufreq_driver;
>    * reapplied from online() across CPU hotplug, and the firmware value is
>    * restored from offline().
>    *
> - * Autonomous Selection (auto_sel) is kept first, as writes to the registers
> - * listed after it only have meaning while autonomous selection is enabled.
> + * Autonomous Selection (auto_sel) splits the list: the registers before it are
> + * independent of it, and those after it have meaning only while autonomous
> + * selection is enabled. Place a new register on the matching side.
>    */
>   enum cppc_saved_reg_id {
> -	CPPC_SAVED_AUTO_SEL,
> +	CPPC_SAVED_OSPM_NOMINAL_PERF,
> +	CPPC_SAVED_AUTO_SEL,	/* Entries below need auto_sel enabled. */
>   	CPPC_SAVED_EPP,
>   	CPPC_SAVED_AUTO_ACT_WINDOW,
>   	CPPC_NR_SAVED_REGS,
> @@ -50,6 +52,11 @@ struct cppc_saved_reg {
>   };
>   
>   static const struct cppc_saved_reg cppc_saved_regs[CPPC_NR_SAVED_REGS] = {
> +	/* Write-only: the requested value is tracked in software. */
> +	[CPPC_SAVED_OSPM_NOMINAL_PERF] = {
> +		.name = "ospm_nominal_perf",
> +		.set = cppc_set_ospm_nominal_perf,
> +	},
>   	[CPPC_SAVED_AUTO_SEL] = {
>   		.name = "auto_sel",
>   		.get = cppc_get_auto_sel,
> @@ -79,6 +86,9 @@ enum cppc_saved_type {
>    *                   could not be read
>    *   requested_val - value in effect when the policy last went offline,
>    *                   reapplied at online(). U64_MAX if none
> + *
> + * A write-only register cannot be read back, so its store handler sets
> + * firmware_val and requested_val instead of init() and offline().
>    */
>   struct cppc_saved_vals {
>   	u64 firmware_val;
> @@ -135,7 +145,17 @@ static void cppc_cpufreq_save_regs(struct cpufreq_policy *policy,
>   		st->suspend_regs_handled = false;
>   
>   	for (i = 0; i < CPPC_NR_SAVED_REGS; i++) {
> -		if (cppc_saved_regs[i].get(cpu, &val))
> +		const struct cppc_saved_reg *reg = &cppc_saved_regs[i];
> +
> +		/*
> +		 * A write-only register cannot be read back. It has no
> +		 * firmware value to capture, and its requested value comes
> +		 * from the store handler, so do not overwrite it here.
> +		 */
> +		if (!reg->get && saved_type == CPPC_SAVED_REQUESTED)
> +			continue;
> +
> +		if (!reg->get || reg->get(cpu, &val))
>   			val = U64_MAX;
>   
>   		if (saved_type == CPPC_SAVED_FIRMWARE) {
> @@ -197,6 +217,12 @@ static void cppc_cpufreq_apply_saved_regs(struct cpufreq_policy *policy,
>   	u64 auto_sel, val;
>   	int i;
>   
> +	/* Registers before auto_sel do not depend on it. */
> +	for (i = 0; i < CPPC_SAVED_AUTO_SEL; i++) {
> +		val = cppc_cpufreq_saved_reg_value(st, i, saved_type);
> +		cppc_cpufreq_write_saved_reg(cpu, i, val, saved_type);
> +	}
> +
>   	auto_sel = cppc_cpufreq_saved_reg_value(st, CPPC_SAVED_AUTO_SEL,
>   						saved_type);
>   
> @@ -1385,11 +1411,46 @@ static int cppc_get_perf_limited_filtered(int cpu, u64 *perf_limited)
>   CPPC_CPUFREQ_ATTR_RW_U64(perf_limited, cppc_get_perf_limited_filtered,
>   			 cppc_set_perf_limited)
>   
> +static ssize_t store_ospm_nominal_freq(struct cpufreq_policy *policy,
> +				       const char *buf, size_t count)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_saved_vals *st;
> +	unsigned int freq_khz;
> +	u32 perf;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &freq_khz);
> +	if (ret)
> +		return ret;
> +
> +	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
> +	if (perf < cpu_data->perf_caps.lowest_perf ||
> +	    perf > cpu_data->perf_caps.nominal_perf)
> +		return -EINVAL;
> +
> +	ret = cppc_set_ospm_nominal_perf(policy->cpu, perf);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Track the request in software: requested_val is reapplied across
> +	 * hotplug, and firmware_val makes the register revert to the platform
> +	 * Nominal on driver unload, since the value cannot be read back.
> +	 */
> +	st = &cppc_cpufreq_policy_state(policy)->regs[CPPC_SAVED_OSPM_NOMINAL_PERF];
> +	st->requested_val = perf;
Shouldn't we set this value at init and never update it again ?
> +	st->firmware_val = cpu_data->perf_caps.nominal_perf;
> +
> +	return count;
> +}
> +
>   cpufreq_freq_attr_ro(freqdomain_cpus);
>   cpufreq_freq_attr_rw(auto_select);
>   cpufreq_freq_attr_rw(auto_act_window);
>   cpufreq_freq_attr_rw(energy_performance_preference_val);
>   cpufreq_freq_attr_rw(perf_limited);
> +cpufreq_freq_attr_wo(ospm_nominal_freq);
>   
>   static struct freq_attr *cppc_cpufreq_attr[] = {
>   	&freqdomain_cpus,
> @@ -1397,6 +1458,7 @@ static struct freq_attr *cppc_cpufreq_attr[] = {
>   	&auto_act_window,
>   	&energy_performance_preference_val,
>   	&perf_limited,
> +	&ospm_nominal_freq,
>   	NULL,
>   };
>