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

On 27/04/26 21:06, Mario Limonciello wrote:
> External email: Use caution opening links or attachments
>
>
> 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 <[email protected]>
>
> 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.

Agreed. Will update in v2 to:
- Make sysfs attribute rw.
- Cache the value in cpc_desc and skip register write when unchanged.
- Have show return the cached value.

Thank you,
Sumit Gupta


>
>> ---
>>   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;
>