Re: [PATCH v2 2/2] ACPI: CPPC: Add ospm_nominal_perf support
From: Sumit Gupta
Date: Thu May 07 2026 - 17:07:48 EST
On 30/04/26 21:55, Pierre Gondois wrote:
External email: Use caution opening links or attachments
Hello Sumit,
On 4/30/26 16:24, Sumit Gupta wrote:
Add acpi_cppc/ospm_nominal_perf sysfs attribute (read-write) andI think the spec also requests to have a value in the range
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.
Although the register is write-only per spec, cache the OSPM-written
value in cpc_desc so userspace can observe it via sysfs, and to
skip redundant writes.
Initialize to platform nominal at policy init. Override via sysfs
if needed.
Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>
---
drivers/acpi/cppc_acpi.c | 69 ++++++++++++++++++++++++++++++++++
drivers/cpufreq/cppc_cpufreq.c | 10 +++++
include/acpi/cppc_acpi.h | 6 +++
3 files changed, 85 insertions(+)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index a1c91ce20cc8..fbc620adafad 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_rw(_name) \
+static struct kobj_attribute _name = \
+__ATTR(_name, 0644, show_##_name, 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,38 @@ static ssize_t show_feedback_ctrs(struct kobject *kobj,
}
define_one_cppc_ro(feedback_ctrs);
+static ssize_t show_ospm_nominal_perf(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+ u64 val = READ_ONCE(cpc_ptr->ospm_nominal_perf);
+
+ if (!val)
+ return -ENODATA;
+
+ return sysfs_emit(buf, "%llu\n", val);
+}
+
+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_rw(ospm_nominal_perf);
+
static struct attribute *cppc_attrs[] = {
&feedback_ctrs.attr,
&reference_perf.attr,
@@ -222,6 +258,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 +1720,38 @@ 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.
+ *
+ * Return: 0 for success, -EINVAL on invalid input, -EOPNOTSUPP if not
+ * supported, -EIO otherwise.
+ */
+int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
+{
+ struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+ int ret;
+
+ if (!ospm_nominal_perf || ospm_nominal_perf > U32_MAX)
+ return -EINVAL;
[lowest:nominal]. As these registers are read-only it should
be ok to read the values here ?
Will add the [lowest_perf, nominal_perf] range check in v3,
fetching the bounds via cppc_get_perf_caps().
+
+ if (cpc_desc &&
+ READ_ONCE(cpc_desc->ospm_nominal_perf) == ospm_nominal_perf)
+ return 0;
+
+ ret = cppc_set_reg_val(cpu, OSPM_NOMINAL_PERF, ospm_nominal_perf);
+ if (ret)
+ return ret;
+
Shouldn't we have some protection against concurrent accesses ?
+ WRITE_ONCE(cpc_desc->ospm_nominal_perf, ospm_nominal_perf);
+ return 0;
+}
+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);
+
IIUC, if (ospm_nominal_perf == nominal_perf), the firmware should
not behave differently. Is this really useful ?
Right, it's a no-op from the firmware's side. The init was only so that
sysfs would show a value (platform nominal) before any userspace write.
Will drop it in v3 and return 0 from sysfs until userspace writes a value.
------------
Also this seems like there will need some synchronization
mechanism to keep-up with the boost state.
If the ospm_nominal_perf is lowered and boost is disabled,
a freq. update should happen. IMO it looks like this could
be handled with (another) freq_qos_request.
This new freq_qos_request, if we name it ospm_nominal_freq_req,
should only be taken into account if boost is disabled.
Otherwise, if boost is enabled, ospm_nominal_freq_req
should be ignored.
Agreed, will add the new freq_qos_request in a follow-up patch.
------------
Also, the function seems to set the ospm_nominal_freq for
a single CPU when the policy might be common for multiple
CPUs right ?
In v3, after dropping the change from cppc_cpufreq_cpu_init,
the problem won't come in this specific instance.
The issues this field raises seems similar to the auto_sel
ones. I.e. :
- concurrency accesses + need for a scratch value
- what should happen when unloading the driver
- the value can be set for single CPUs but we might
want to have the same value for the whole policy
Maybe a common solution should be found.
(I m not suggesting anything right now unfortunately).
One way to address this is to move the sysfs from per-CPU acpi_cppc to
a per-policy node under cpufreq (ospm_nominal_perf_freq, kHz).
In the sysfs callback, we can convert kHz to perf and write the register
on every CPU in policy->cpus.
Concurrency is already covered by policy->rwsem at the cpufreq layer.
This is similar to how we were handling min/max_perf in earlier version.
Does this approach make sense?
Thank you,
Sumit Gupta
....