Re: [PATCH 2/2] ACPI: CPPC: Add ospm_nominal_perf support

[Mario Limonciello <mario.limonciello@amd.com>]

On 4/27/26 00:18, Sumit Gupta wrote:
> Add acpi_cppc/ospm_nominal_perf sysfs attribute (write-only) and
> cppc_set_ospm_nominal_perf() API for the OSPM Nominal Performance
> register (ACPI 6.6, Section 8.4.6.1.2.6).
> 
> The register conveys the desired nominal performance level at which
> the platform may run. OSPM can request a lower level than platform
> nominal. Valid range is [Lowest Performance, Nominal Performance].
> The value tells the platform what OSPM considers nominal. The
> platform classifies performance above this as boosted and below as
> throttled. It uses that for its power/thermal decisions.
> 
> Initialize to platform nominal at policy init. Override via sysfs
> if needed.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>

Even though it's a write only register; does it make sense to be a write 
only sysfs file?

I say this because if we're writing it at init time and we're the OSPM 
we should be able to track the state of what we wrote and display that 
to userspace if it wants to know.

Furthermore; tracking the state could mean store_ospm_nominal_perf() can 
avoid the extra register write if the state we tracked is the same 
userspace tried to write.

> ---
>   drivers/acpi/cppc_acpi.c       | 42 ++++++++++++++++++++++++++++++++++
>   drivers/cpufreq/cppc_cpufreq.c | 10 ++++++++
>   include/acpi/cppc_acpi.h       |  5 ++++
>   3 files changed, 57 insertions(+)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index a1c91ce20cc8..83b4b14652fb 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -155,6 +155,10 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
>   static struct kobj_attribute _name =		\
>   __ATTR(_name, 0444, show_##_name, NULL)
>   
> +#define define_one_cppc_wo(_name)		\
> +static struct kobj_attribute _name =		\
> +__ATTR(_name, 0200, NULL, store_##_name)
> +
>   #define to_cpc_desc(a) container_of(a, struct cpc_desc, kobj)
>   
>   #define show_cppc_data(access_fn, struct_name, member_name)		\
> @@ -211,6 +215,26 @@ static ssize_t show_feedback_ctrs(struct kobject *kobj,
>   }
>   define_one_cppc_ro(feedback_ctrs);
>   
> +static ssize_t store_ospm_nominal_perf(struct kobject *kobj,
> +				       struct kobj_attribute *attr,
> +				       const char *buf, size_t count)
> +{
> +	struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
> +	u64 val;
> +	int ret;
> +
> +	ret = kstrtou64(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = cppc_set_ospm_nominal_perf(cpc_ptr->cpu_id, val);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +define_one_cppc_wo(ospm_nominal_perf);
> +
>   static struct attribute *cppc_attrs[] = {
>   	&feedback_ctrs.attr,
>   	&reference_perf.attr,
> @@ -222,6 +246,7 @@ static struct attribute *cppc_attrs[] = {
>   	&nominal_perf.attr,
>   	&nominal_freq.attr,
>   	&lowest_freq.attr,
> +	&ospm_nominal_perf.attr,
>   	NULL
>   };
>   ATTRIBUTE_GROUPS(cppc);
> @@ -1683,6 +1708,23 @@ int cppc_set_epp(int cpu, u64 epp_val)
>   }
>   EXPORT_SYMBOL_GPL(cppc_set_epp);
>   
> +/**
> + * cppc_set_ospm_nominal_perf() - Write OSPM Nominal Performance register.
> + * @cpu: CPU on which to write register.
> + * @ospm_nominal_perf: Value to write to the OSPM Nominal Performance register.
> + *
> + * OSPM Nominal Performance allows OSPM to inform the platform of the nominal
> + * performance level it intends to maintain. This is a write-only register per
> + * ACPI specification.
> + *
> + * Return: 0 for success, -EOPNOTSUPP if not supported, -EIO otherwise.
> + */
> +int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
> +{
> +	return cppc_set_reg_val(cpu, OSPM_NOMINAL_PERF, ospm_nominal_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_ospm_nominal_perf);
> +
>   /**
>    * cppc_get_auto_act_window() - Read autonomous activity window register.
>    * @cpu: CPU from which to read register.
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 7e7f9dfb7a24..d06cba963550 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -715,6 +715,16 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>   		goto out;
>   	}
>   
> +	/*
> +	 * Initialize OSPM Nominal Performance to inform firmware of
> +	 * OSPM's nominal level. Performance above this value = boost;
> +	 * below = throttle. Uses platform nominal by default.
> +	 */
> +	ret = cppc_set_ospm_nominal_perf(cpu, caps->nominal_perf);
> +	if (ret && ret != -EOPNOTSUPP)
> +		pr_debug("Failed to set ospm_nominal_perf for CPU%d: %d\n",
> +			 cpu, ret);
> +
>   	cppc_cpufreq_cpu_fie_init(policy);
>   	return 0;
>   
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 8693890a7275..ad1035b0e1de 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -180,6 +180,7 @@ extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
>   extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf);
>   extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable);
>   extern int cppc_set_epp(int cpu, u64 epp_val);
> +extern int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf);
>   extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window);
>   extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window);
>   extern int cppc_get_auto_sel(int cpu, bool *enable);
> @@ -266,6 +267,10 @@ static inline int cppc_set_epp(int cpu, u64 epp_val)
>   {
>   	return -EOPNOTSUPP;
>   }
> +static inline int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
>   static inline int cppc_get_auto_act_window(int cpu, u64 *auto_act_window)
>   {
>   	return -EOPNOTSUPP;