[PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf

Sumit Gupta posted 9 patches 2 weeks, 4 days ago
There is a newer version of this series
[PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by Sumit Gupta 2 weeks, 4 days ago
Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read and
write the MIN_PERF and MAX_PERF registers.

Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
to expose these controls to userspace. The sysfs values are in frequency
(kHz) for consistency with other cpufreq sysfs files.

A mutex is used to serialize sysfs store operations to ensure hardware
register writes and perf_ctrls updates are atomic.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/acpi/cppc_acpi.c       |  44 +++++++++
 drivers/cpufreq/cppc_cpufreq.c | 157 +++++++++++++++++++++++++++++++++
 include/acpi/cppc_acpi.h       |  20 +++++
 3 files changed, 221 insertions(+)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 45c6bd6ec24b..46bf45f8b0f3 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
 }
 EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
 
+/**
+ * cppc_get_min_perf - Read minimum performance register.
+ * @cpu: CPU from which to read register.
+ * @min_perf: Return address.
+ */
+int cppc_get_min_perf(int cpu, u64 *min_perf)
+{
+	return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
+}
+EXPORT_SYMBOL_GPL(cppc_get_min_perf);
+
+/**
+ * cppc_set_min_perf - Write minimum performance register.
+ * @cpu: CPU to which to write register.
+ * @min_perf: the desired minimum performance value to be updated.
+ */
+int cppc_set_min_perf(int cpu, u32 min_perf)
+{
+	return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
+}
+EXPORT_SYMBOL_GPL(cppc_set_min_perf);
+
+/**
+ * cppc_get_max_perf - Read maximum performance register.
+ * @cpu: CPU from which to read register.
+ * @max_perf: Return address.
+ */
+int cppc_get_max_perf(int cpu, u64 *max_perf)
+{
+	return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
+}
+EXPORT_SYMBOL_GPL(cppc_get_max_perf);
+
+/**
+ * cppc_set_max_perf - Write maximum performance register.
+ * @cpu: CPU to which to write register.
+ * @max_perf: the desired maximum performance value to be updated.
+ */
+int cppc_set_max_perf(int cpu, u32 max_perf)
+{
+	return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
+}
+EXPORT_SYMBOL_GPL(cppc_set_max_perf);
+
 /**
  * cppc_set_enable - Set to enable CPPC on the processor by writing the
  * Continuous Performance Control package EnableRegister field.
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 229880c4eedb..66e183b45fb0 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -28,6 +28,8 @@
 
 static struct cpufreq_driver cppc_cpufreq_driver;
 
+static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
+
 #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
 static enum {
 	FIE_UNSET = -1,
@@ -570,6 +572,35 @@ static void populate_efficiency_class(void)
 }
 #endif
 
+/* Set min/max performance HW register and cache the value */
+static int cppc_cpufreq_set_mperf_reg(struct cpufreq_policy *policy,
+				      u64 val, bool is_min)
+{
+	struct cppc_cpudata *cpu_data = policy->driver_data;
+	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
+	unsigned int cpu = policy->cpu;
+	u32 perf;
+	int ret;
+
+	perf = clamp(val, caps->lowest_perf, caps->highest_perf);
+
+	ret = is_min ? cppc_set_min_perf(cpu, perf) :
+		       cppc_set_max_perf(cpu, perf);
+	if (ret) {
+		if (ret != -EOPNOTSUPP)
+			pr_warn("CPU%d: set %s_perf=%u failed (%d)\n",
+				cpu, is_min ? "min" : "max", perf, ret);
+		return ret;
+	}
+
+	if (is_min)
+		cpu_data->perf_ctrls.min_perf = perf;
+	else
+		cpu_data->perf_ctrls.max_perf = perf;
+
+	return 0;
+}
+
 static struct cppc_cpudata *cppc_cpufreq_get_cpu_data(unsigned int cpu)
 {
 	struct cppc_cpudata *cpu_data;
@@ -918,16 +949,142 @@ CPPC_CPUFREQ_ATTR_RW_U64(auto_act_window, cppc_get_auto_act_window,
 CPPC_CPUFREQ_ATTR_RW_U64(energy_performance_preference_val,
 			 cppc_get_epp_perf, cppc_set_epp)
 
+/**
+ * show_min_perf - Show minimum performance as frequency (kHz)
+ * @policy: cpufreq policy
+ * @buf: buffer to write the frequency value to
+ *
+ * Reads the MIN_PERF register and converts the performance value to
+ * frequency (kHz).
+ */
+static ssize_t show_min_perf(struct cpufreq_policy *policy, char *buf)
+{
+	struct cppc_cpudata *cpu_data = policy->driver_data;
+	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
+	u64 perf;
+	int ret;
+
+	ret = cppc_get_min_perf(policy->cpu, &perf);
+	if (ret == -EOPNOTSUPP)
+		return sysfs_emit(buf, "<unsupported>\n");
+	if (ret)
+		return ret;
+
+	/* Use lowest_perf if register is uninitialized (0) */
+	if (perf == 0)
+		perf = caps->lowest_perf;
+
+	/* Convert performance to frequency (kHz) for user */
+	return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
+}
+
+/**
+ * store_min_perf - Set minimum performance from frequency (kHz)
+ * @policy: cpufreq policy
+ * @buf: buffer containing the frequency value
+ * @count: size of @buf
+ *
+ * Converts the user-provided frequency (kHz) to a performance value
+ * and writes it to the MIN_PERF register.
+ */
+static ssize_t store_min_perf(struct cpufreq_policy *policy, const char *buf,
+			      size_t count)
+{
+	struct cppc_cpudata *cpu_data = policy->driver_data;
+	unsigned int freq_khz;
+	u64 perf;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &freq_khz);
+	if (ret)
+		return ret;
+
+	/* Convert frequency (kHz) to performance value */
+	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
+
+	guard(mutex)(&cppc_cpufreq_autonomous_lock);
+	ret = cppc_cpufreq_set_mperf_reg(policy, perf, true);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+/**
+ * show_max_perf - Show maximum performance as frequency (kHz)
+ * @policy: cpufreq policy
+ * @buf: buffer to write the frequency value to
+ *
+ * Reads the MAX_PERF register and converts the performance value to
+ * frequency (kHz).
+ */
+static ssize_t show_max_perf(struct cpufreq_policy *policy, char *buf)
+{
+	struct cppc_cpudata *cpu_data = policy->driver_data;
+	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
+	u64 perf;
+	int ret;
+
+	ret = cppc_get_max_perf(policy->cpu, &perf);
+	if (ret == -EOPNOTSUPP)
+		return sysfs_emit(buf, "<unsupported>\n");
+	if (ret)
+		return ret;
+
+	/* Use highest_perf if register is uninitialized or out of range */
+	if (perf == 0 || perf > caps->highest_perf)
+		perf = caps->highest_perf;
+
+	/* Convert performance to frequency (kHz) for user */
+	return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
+}
+
+/**
+ * store_max_perf - Set maximum performance from frequency (kHz)
+ * @policy: cpufreq policy
+ * @buf: buffer containing the frequency value
+ * @count: size of @buf
+ *
+ * Converts the user-provided frequency (kHz) to a performance value
+ * and writes it to the MAX_PERF register.
+ */
+static ssize_t store_max_perf(struct cpufreq_policy *policy, const char *buf,
+			      size_t count)
+{
+	struct cppc_cpudata *cpu_data = policy->driver_data;
+	unsigned int freq_khz;
+	u64 perf;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &freq_khz);
+	if (ret)
+		return ret;
+
+	/* Convert frequency (kHz) to performance value */
+	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
+
+	guard(mutex)(&cppc_cpufreq_autonomous_lock);
+	ret = cppc_cpufreq_set_mperf_reg(policy, perf, false);
+	if (ret)
+		return ret;
+
+	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(min_perf);
+cpufreq_freq_attr_rw(max_perf);
 
 static struct freq_attr *cppc_cpufreq_attr[] = {
 	&freqdomain_cpus,
 	&auto_select,
 	&auto_act_window,
 	&energy_performance_preference_val,
+	&min_perf,
+	&max_perf,
 	NULL,
 };
 
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 3fc796c0d902..b358440cd0e2 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -174,6 +174,10 @@ 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);
 extern int cppc_set_auto_sel(int cpu, bool enable);
+extern int cppc_get_min_perf(int cpu, u64 *min_perf);
+extern int cppc_set_min_perf(int cpu, u32 min_perf);
+extern int cppc_get_max_perf(int cpu, u64 *max_perf);
+extern int cppc_set_max_perf(int cpu, u32 max_perf);
 extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
 extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator);
 extern int amd_detect_prefcore(bool *detected);
@@ -270,6 +274,22 @@ static inline int cppc_set_auto_sel(int cpu, bool enable)
 {
 	return -EOPNOTSUPP;
 }
+static inline int cppc_get_min_perf(int cpu, u64 *min_perf)
+{
+	return -EOPNOTSUPP;
+}
+static inline int cppc_set_min_perf(int cpu, u32 min_perf)
+{
+	return -EOPNOTSUPP;
+}
+static inline int cppc_get_max_perf(int cpu, u64 *max_perf)
+{
+	return -EOPNOTSUPP;
+}
+static inline int cppc_set_max_perf(int cpu, u32 max_perf)
+{
+	return -EOPNOTSUPP;
+}
 static inline int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf)
 {
 	return -ENODEV;
-- 
2.34.1
Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by zhenglifeng (A) 2 weeks, 2 days ago
On 2026/1/20 22:56, Sumit Gupta wrote:
> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read and
> write the MIN_PERF and MAX_PERF registers.
> 
> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
> to expose these controls to userspace. The sysfs values are in frequency
> (kHz) for consistency with other cpufreq sysfs files.
> 
> A mutex is used to serialize sysfs store operations to ensure hardware
> register writes and perf_ctrls updates are atomic.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>  drivers/acpi/cppc_acpi.c       |  44 +++++++++
>  drivers/cpufreq/cppc_cpufreq.c | 157 +++++++++++++++++++++++++++++++++
>  include/acpi/cppc_acpi.h       |  20 +++++
>  3 files changed, 221 insertions(+)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 45c6bd6ec24b..46bf45f8b0f3 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>  }
>  EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>  
> +/**
> + * cppc_get_min_perf - Read minimum performance register.
> + * @cpu: CPU from which to read register.
> + * @min_perf: Return address.
> + */
> +int cppc_get_min_perf(int cpu, u64 *min_perf)
> +{
> +	return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
> +
> +/**
> + * cppc_set_min_perf - Write minimum performance register.
> + * @cpu: CPU to which to write register.
> + * @min_perf: the desired minimum performance value to be updated.
> + */
> +int cppc_set_min_perf(int cpu, u32 min_perf)
> +{
> +	return cppc_set_reg_val(cpu, MIN_PERF, min_perf);

ACPI spec says it 'must be set to a value that is less than or equal to
that specified by the Maximum Performance Register'. So it may be better
to check it before setting value.

> +}
> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
> +
> +/**
> + * cppc_get_max_perf - Read maximum performance register.
> + * @cpu: CPU from which to read register.
> + * @max_perf: Return address.
> + */
> +int cppc_get_max_perf(int cpu, u64 *max_perf)
> +{
> +	return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
> +
> +/**
> + * cppc_set_max_perf - Write maximum performance register.
> + * @cpu: CPU to which to write register.
> + * @max_perf: the desired maximum performance value to be updated.
> + */
> +int cppc_set_max_perf(int cpu, u32 max_perf)
> +{
> +	return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
> +
>  /**
>   * cppc_set_enable - Set to enable CPPC on the processor by writing the
>   * Continuous Performance Control package EnableRegister field.
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 229880c4eedb..66e183b45fb0 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -28,6 +28,8 @@
>  
>  static struct cpufreq_driver cppc_cpufreq_driver;
>  
> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
> +
>  #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>  static enum {
>  	FIE_UNSET = -1,
> @@ -570,6 +572,35 @@ static void populate_efficiency_class(void)
>  }
>  #endif
>  
> +/* Set min/max performance HW register and cache the value */
> +static int cppc_cpufreq_set_mperf_reg(struct cpufreq_policy *policy,
> +				      u64 val, bool is_min)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> +	unsigned int cpu = policy->cpu;
> +	u32 perf;
> +	int ret;
> +
> +	perf = clamp(val, caps->lowest_perf, caps->highest_perf);
> +
> +	ret = is_min ? cppc_set_min_perf(cpu, perf) :
> +		       cppc_set_max_perf(cpu, perf);
> +	if (ret) {
> +		if (ret != -EOPNOTSUPP)
> +			pr_warn("CPU%d: set %s_perf=%u failed (%d)\n",
> +				cpu, is_min ? "min" : "max", perf, ret);
> +		return ret;
> +	}
> +
> +	if (is_min)
> +		cpu_data->perf_ctrls.min_perf = perf;
> +	else
> +		cpu_data->perf_ctrls.max_perf = perf;
> +
> +	return 0;

I think cppc_set_XXX and updating cpudata->perf_ctrls.XXX can be extract
out for not only min_perf and max_perf but also auto_sel and energy_perf
and anything else in perf_ctrls.

> +}
> +
>  static struct cppc_cpudata *cppc_cpufreq_get_cpu_data(unsigned int cpu)
>  {
>  	struct cppc_cpudata *cpu_data;
> @@ -918,16 +949,142 @@ CPPC_CPUFREQ_ATTR_RW_U64(auto_act_window, cppc_get_auto_act_window,
>  CPPC_CPUFREQ_ATTR_RW_U64(energy_performance_preference_val,
>  			 cppc_get_epp_perf, cppc_set_epp)
>  
> +/**
> + * show_min_perf - Show minimum performance as frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer to write the frequency value to
> + *
> + * Reads the MIN_PERF register and converts the performance value to
> + * frequency (kHz).
> + */
> +static ssize_t show_min_perf(struct cpufreq_policy *policy, char *buf)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> +	u64 perf;
> +	int ret;
> +
> +	ret = cppc_get_min_perf(policy->cpu, &perf);
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "<unsupported>\n");
> +	if (ret)
> +		return ret;
> +
> +	/* Use lowest_perf if register is uninitialized (0) */
> +	if (perf == 0)
> +		perf = caps->lowest_perf;
> +
> +	/* Convert performance to frequency (kHz) for user */
> +	return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
> +}
> +
> +/**
> + * store_min_perf - Set minimum performance from frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer containing the frequency value
> + * @count: size of @buf
> + *
> + * Converts the user-provided frequency (kHz) to a performance value
> + * and writes it to the MIN_PERF register.
> + */
> +static ssize_t store_min_perf(struct cpufreq_policy *policy, const char *buf,
> +			      size_t count)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	unsigned int freq_khz;
> +	u64 perf;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &freq_khz);
> +	if (ret)
> +		return ret;
> +
> +	/* Convert frequency (kHz) to performance value */
> +	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
> +
> +	guard(mutex)(&cppc_cpufreq_autonomous_lock);
> +	ret = cppc_cpufreq_set_mperf_reg(policy, perf, true);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +/**
> + * show_max_perf - Show maximum performance as frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer to write the frequency value to
> + *
> + * Reads the MAX_PERF register and converts the performance value to
> + * frequency (kHz).
> + */
> +static ssize_t show_max_perf(struct cpufreq_policy *policy, char *buf)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> +	u64 perf;
> +	int ret;
> +
> +	ret = cppc_get_max_perf(policy->cpu, &perf);
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "<unsupported>\n");
> +	if (ret)
> +		return ret;
> +
> +	/* Use highest_perf if register is uninitialized or out of range */
> +	if (perf == 0 || perf > caps->highest_perf)
> +		perf = caps->highest_perf;
> +
> +	/* Convert performance to frequency (kHz) for user */
> +	return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
> +}
> +
> +/**
> + * store_max_perf - Set maximum performance from frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer containing the frequency value
> + * @count: size of @buf
> + *
> + * Converts the user-provided frequency (kHz) to a performance value
> + * and writes it to the MAX_PERF register.
> + */
> +static ssize_t store_max_perf(struct cpufreq_policy *policy, const char *buf,
> +			      size_t count)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	unsigned int freq_khz;
> +	u64 perf;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &freq_khz);
> +	if (ret)
> +		return ret;
> +
> +	/* Convert frequency (kHz) to performance value */
> +	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
> +
> +	guard(mutex)(&cppc_cpufreq_autonomous_lock);
> +	ret = cppc_cpufreq_set_mperf_reg(policy, perf, false);
> +	if (ret)
> +		return ret;
> +
> +	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(min_perf);
> +cpufreq_freq_attr_rw(max_perf);
>  
>  static struct freq_attr *cppc_cpufreq_attr[] = {
>  	&freqdomain_cpus,
>  	&auto_select,
>  	&auto_act_window,
>  	&energy_performance_preference_val,
> +	&min_perf,
> +	&max_perf,
>  	NULL,
>  };
>  
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 3fc796c0d902..b358440cd0e2 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -174,6 +174,10 @@ 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);
>  extern int cppc_set_auto_sel(int cpu, bool enable);
> +extern int cppc_get_min_perf(int cpu, u64 *min_perf);
> +extern int cppc_set_min_perf(int cpu, u32 min_perf);
> +extern int cppc_get_max_perf(int cpu, u64 *max_perf);
> +extern int cppc_set_max_perf(int cpu, u32 max_perf);
>  extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
>  extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator);
>  extern int amd_detect_prefcore(bool *detected);
> @@ -270,6 +274,22 @@ static inline int cppc_set_auto_sel(int cpu, bool enable)
>  {
>  	return -EOPNOTSUPP;
>  }
> +static inline int cppc_get_min_perf(int cpu, u64 *min_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_set_min_perf(int cpu, u32 min_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_get_max_perf(int cpu, u64 *max_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_set_max_perf(int cpu, u32 max_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
>  static inline int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf)
>  {
>  	return -ENODEV;
Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by Sumit Gupta 2 weeks ago
On 22/01/26 18:05, zhenglifeng (A) wrote:
> External email: Use caution opening links or attachments
>
>
> On 2026/1/20 22:56, Sumit Gupta wrote:
>> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read and
>> write the MIN_PERF and MAX_PERF registers.
>>
>> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
>> to expose these controls to userspace. The sysfs values are in frequency
>> (kHz) for consistency with other cpufreq sysfs files.
>>
>> A mutex is used to serialize sysfs store operations to ensure hardware
>> register writes and perf_ctrls updates are atomic.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>   drivers/acpi/cppc_acpi.c       |  44 +++++++++
>>   drivers/cpufreq/cppc_cpufreq.c | 157 +++++++++++++++++++++++++++++++++
>>   include/acpi/cppc_acpi.h       |  20 +++++
>>   3 files changed, 221 insertions(+)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 45c6bd6ec24b..46bf45f8b0f3 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>>   }
>>   EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>>
>> +/**
>> + * cppc_get_min_perf - Read minimum performance register.
>> + * @cpu: CPU from which to read register.
>> + * @min_perf: Return address.
>> + */
>> +int cppc_get_min_perf(int cpu, u64 *min_perf)
>> +{
>> +     return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
>> +
>> +/**
>> + * cppc_set_min_perf - Write minimum performance register.
>> + * @cpu: CPU to which to write register.
>> + * @min_perf: the desired minimum performance value to be updated.
>> + */
>> +int cppc_set_min_perf(int cpu, u32 min_perf)
>> +{
>> +     return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
> ACPI spec says it 'must be set to a value that is less than or equal to
> that specified by the Maximum Performance Register'. So it may be better
> to check it before setting value.

Yes, I added that check in v1[1]. But missed adding it in later versions.
Will add below check in v7. Thank you for pointing.
--------
   static ssize_t store_min_perf(struct cpufreq_policy *policy, const 
char *buf,
                               size_t count)
   {
         .....
         /* Convert frequency (kHz) to performance value */
         perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);

+       if (perf > cpu_data->perf_ctrls.max_perf)
+               return -EINVAL;
--------
[1] https://lore.kernel.org/lkml/20250211103737.447704-4-sumitg@nvidia.com/

>> +}
>> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
>> +
>> +/**
>> + * cppc_get_max_perf - Read maximum performance register.
>> + * @cpu: CPU from which to read register.
>> + * @max_perf: Return address.
>> + */
>> +int cppc_get_max_perf(int cpu, u64 *max_perf)
>> +{
>> +     return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
>> +
>> +/**
>> + * cppc_set_max_perf - Write maximum performance register.
>> + * @cpu: CPU to which to write register.
>> + * @max_perf: the desired maximum performance value to be updated.
>> + */
>> +int cppc_set_max_perf(int cpu, u32 max_perf)
>> +{
>> +     return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
>> +
>>   /**
>>    * cppc_set_enable - Set to enable CPPC on the processor by writing the
>>    * Continuous Performance Control package EnableRegister field.
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 229880c4eedb..66e183b45fb0 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -28,6 +28,8 @@
>>
>>   static struct cpufreq_driver cppc_cpufreq_driver;
>>
>> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
>> +
>>   #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>>   static enum {
>>        FIE_UNSET = -1,
>> @@ -570,6 +572,35 @@ static void populate_efficiency_class(void)
>>   }
>>   #endif
>>
>> +/* Set min/max performance HW register and cache the value */
>> +static int cppc_cpufreq_set_mperf_reg(struct cpufreq_policy *policy,
>> +                                   u64 val, bool is_min)
>> +{
>> +     struct cppc_cpudata *cpu_data = policy->driver_data;
>> +     struct cppc_perf_caps *caps = &cpu_data->perf_caps;
>> +     unsigned int cpu = policy->cpu;
>> +     u32 perf;
>> +     int ret;
>> +
>> +     perf = clamp(val, caps->lowest_perf, caps->highest_perf);
>> +
>> +     ret = is_min ? cppc_set_min_perf(cpu, perf) :
>> +                    cppc_set_max_perf(cpu, perf);
>> +     if (ret) {
>> +             if (ret != -EOPNOTSUPP)
>> +                     pr_warn("CPU%d: set %s_perf=%u failed (%d)\n",
>> +                             cpu, is_min ? "min" : "max", perf, ret);
>> +             return ret;
>> +     }
>> +
>> +     if (is_min)
>> +             cpu_data->perf_ctrls.min_perf = perf;
>> +     else
>> +             cpu_data->perf_ctrls.max_perf = perf;
>> +
>> +     return 0;
> I think cppc_set_XXX and updating cpudata->perf_ctrls.XXX can be extract
> out for not only min_perf and max_perf but also auto_sel and energy_perf
> and anything else in perf_ctrls.

Updating cached value of auto_sel and energy_perf in patch 9.
I think the current code is simple. Adding an abstraction seems to be
overdoing it for this case.

Thank you,
Sumit Gupta

....


Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by Pierre Gondois 2 weeks, 2 days ago
On 1/20/26 15:56, Sumit Gupta wrote:
> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read and
> write the MIN_PERF and MAX_PERF registers.
>
> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
> to expose these controls to userspace. The sysfs values are in frequency
> (kHz) for consistency with other cpufreq sysfs files.
>
> A mutex is used to serialize sysfs store operations to ensure hardware
> register writes and perf_ctrls updates are atomic.
>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>   drivers/acpi/cppc_acpi.c       |  44 +++++++++
>   drivers/cpufreq/cppc_cpufreq.c | 157 +++++++++++++++++++++++++++++++++
>   include/acpi/cppc_acpi.h       |  20 +++++
>   3 files changed, 221 insertions(+)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 45c6bd6ec24b..46bf45f8b0f3 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>   }
>   EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>   
> +/**
> + * cppc_get_min_perf - Read minimum performance register.
> + * @cpu: CPU from which to read register.
> + * @min_perf: Return address.
> + */
> +int cppc_get_min_perf(int cpu, u64 *min_perf)
> +{
> +	return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
> +
> +/**
> + * cppc_set_min_perf - Write minimum performance register.
> + * @cpu: CPU to which to write register.
> + * @min_perf: the desired minimum performance value to be updated.
> + */
> +int cppc_set_min_perf(int cpu, u32 min_perf)
> +{
> +	return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
> +
> +/**
> + * cppc_get_max_perf - Read maximum performance register.
> + * @cpu: CPU from which to read register.
> + * @max_perf: Return address.
> + */
> +int cppc_get_max_perf(int cpu, u64 *max_perf)
> +{
> +	return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
> +
> +/**
> + * cppc_set_max_perf - Write maximum performance register.
> + * @cpu: CPU to which to write register.
> + * @max_perf: the desired maximum performance value to be updated.
> + */
> +int cppc_set_max_perf(int cpu, u32 max_perf)
> +{
> +	return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
> +
>   /**
>    * cppc_set_enable - Set to enable CPPC on the processor by writing the
>    * Continuous Performance Control package EnableRegister field.
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 229880c4eedb..66e183b45fb0 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -28,6 +28,8 @@
>   
>   static struct cpufreq_driver cppc_cpufreq_driver;
>   
> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
> +

Shouldn't concurrent access be handled by the policy->rwsem ?

I think this can be checked using either:
- lockdep_assert_held_write(&policy->rwsem)
- lockdep_assert_held_read(&policy->rwsem)

in store/show_max_perf() for instance.


>   #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>   static enum {
>   	FIE_UNSET = -1,
> @@ -570,6 +572,35 @@ static void populate_efficiency_class(void)
>   }
>   #endif
>   
> +/* Set min/max performance HW register and cache the value */
> +static int cppc_cpufreq_set_mperf_reg(struct cpufreq_policy *policy,
> +				      u64 val, bool is_min)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> +	unsigned int cpu = policy->cpu;
> +	u32 perf;
> +	int ret;
> +
> +	perf = clamp(val, caps->lowest_perf, caps->highest_perf);
> +
> +	ret = is_min ? cppc_set_min_perf(cpu, perf) :
> +		       cppc_set_max_perf(cpu, perf);
> +	if (ret) {
> +		if (ret != -EOPNOTSUPP)
> +			pr_warn("CPU%d: set %s_perf=%u failed (%d)\n",
> +				cpu, is_min ? "min" : "max", perf, ret);
> +		return ret;
> +	}
> +
> +	if (is_min)
> +		cpu_data->perf_ctrls.min_perf = perf;
> +	else
> +		cpu_data->perf_ctrls.max_perf = perf;
> +
> +	return 0;
> +}
> +
>   static struct cppc_cpudata *cppc_cpufreq_get_cpu_data(unsigned int cpu)
>   {
>   	struct cppc_cpudata *cpu_data;
> @@ -918,16 +949,142 @@ CPPC_CPUFREQ_ATTR_RW_U64(auto_act_window, cppc_get_auto_act_window,
>   CPPC_CPUFREQ_ATTR_RW_U64(energy_performance_preference_val,
>   			 cppc_get_epp_perf, cppc_set_epp)
>   
> +/**
> + * show_min_perf - Show minimum performance as frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer to write the frequency value to
> + *
> + * Reads the MIN_PERF register and converts the performance value to
> + * frequency (kHz).
> + */
> +static ssize_t show_min_perf(struct cpufreq_policy *policy, char *buf)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> +	u64 perf;
> +	int ret;
> +
> +	ret = cppc_get_min_perf(policy->cpu, &perf);
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "<unsupported>\n");
> +	if (ret)
> +		return ret;
> +
> +	/* Use lowest_perf if register is uninitialized (0) */
> +	if (perf == 0)
> +		perf = caps->lowest_perf;
> +
> +	/* Convert performance to frequency (kHz) for user */
> +	return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
> +}
> +
> +/**
> + * store_min_perf - Set minimum performance from frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer containing the frequency value
> + * @count: size of @buf
> + *
> + * Converts the user-provided frequency (kHz) to a performance value
> + * and writes it to the MIN_PERF register.
> + */
> +static ssize_t store_min_perf(struct cpufreq_policy *policy, const char *buf,
> +			      size_t count)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	unsigned int freq_khz;
> +	u64 perf;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &freq_khz);
> +	if (ret)
> +		return ret;
> +
> +	/* Convert frequency (kHz) to performance value */
> +	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
> +
> +	guard(mutex)(&cppc_cpufreq_autonomous_lock);
> +	ret = cppc_cpufreq_set_mperf_reg(policy, perf, true);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +/**
> + * show_max_perf - Show maximum performance as frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer to write the frequency value to
> + *
> + * Reads the MAX_PERF register and converts the performance value to
> + * frequency (kHz).
> + */
> +static ssize_t show_max_perf(struct cpufreq_policy *policy, char *buf)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> +	u64 perf;
> +	int ret;
> +
> +	ret = cppc_get_max_perf(policy->cpu, &perf);
> +	if (ret == -EOPNOTSUPP)
> +		return sysfs_emit(buf, "<unsupported>\n");
> +	if (ret)
> +		return ret;
> +
> +	/* Use highest_perf if register is uninitialized or out of range */
> +	if (perf == 0 || perf > caps->highest_perf)
> +		perf = caps->highest_perf;
> +
> +	/* Convert performance to frequency (kHz) for user */
> +	return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
> +}
> +
> +/**
> + * store_max_perf - Set maximum performance from frequency (kHz)
> + * @policy: cpufreq policy
> + * @buf: buffer containing the frequency value
> + * @count: size of @buf
> + *
> + * Converts the user-provided frequency (kHz) to a performance value
> + * and writes it to the MAX_PERF register.
> + */
> +static ssize_t store_max_perf(struct cpufreq_policy *policy, const char *buf,
> +			      size_t count)
> +{
> +	struct cppc_cpudata *cpu_data = policy->driver_data;
> +	unsigned int freq_khz;
> +	u64 perf;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &freq_khz);
> +	if (ret)
> +		return ret;
> +
> +	/* Convert frequency (kHz) to performance value */
> +	perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
> +
> +	guard(mutex)(&cppc_cpufreq_autonomous_lock);
> +	ret = cppc_cpufreq_set_mperf_reg(policy, perf, false);
> +	if (ret)
> +		return ret;
> +
> +	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(min_perf);
> +cpufreq_freq_attr_rw(max_perf);
>   
>   static struct freq_attr *cppc_cpufreq_attr[] = {
>   	&freqdomain_cpus,
>   	&auto_select,
>   	&auto_act_window,
>   	&energy_performance_preference_val,
> +	&min_perf,
> +	&max_perf,
>   	NULL,
>   };
>   
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 3fc796c0d902..b358440cd0e2 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -174,6 +174,10 @@ 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);
>   extern int cppc_set_auto_sel(int cpu, bool enable);
> +extern int cppc_get_min_perf(int cpu, u64 *min_perf);
> +extern int cppc_set_min_perf(int cpu, u32 min_perf);
> +extern int cppc_get_max_perf(int cpu, u64 *max_perf);
> +extern int cppc_set_max_perf(int cpu, u32 max_perf);
>   extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
>   extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator);
>   extern int amd_detect_prefcore(bool *detected);
> @@ -270,6 +274,22 @@ static inline int cppc_set_auto_sel(int cpu, bool enable)
>   {
>   	return -EOPNOTSUPP;
>   }
> +static inline int cppc_get_min_perf(int cpu, u64 *min_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_set_min_perf(int cpu, u32 min_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_get_max_perf(int cpu, u64 *max_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_set_max_perf(int cpu, u32 max_perf)
> +{
> +	return -EOPNOTSUPP;
> +}
>   static inline int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf)
>   {
>   	return -ENODEV;
Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by Sumit Gupta 2 weeks ago
On 22/01/26 17:06, Pierre Gondois wrote:
> External email: Use caution opening links or attachments
>
>
> On 1/20/26 15:56, Sumit Gupta wrote:
>> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read and
>> write the MIN_PERF and MAX_PERF registers.
>>
>> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
>> to expose these controls to userspace. The sysfs values are in frequency
>> (kHz) for consistency with other cpufreq sysfs files.
>>
>> A mutex is used to serialize sysfs store operations to ensure hardware
>> register writes and perf_ctrls updates are atomic.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>   drivers/acpi/cppc_acpi.c       |  44 +++++++++
>>   drivers/cpufreq/cppc_cpufreq.c | 157 +++++++++++++++++++++++++++++++++
>>   include/acpi/cppc_acpi.h       |  20 +++++
>>   3 files changed, 221 insertions(+)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 45c6bd6ec24b..46bf45f8b0f3 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>>   }
>>   EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>>
>> +/**
>> + * cppc_get_min_perf - Read minimum performance register.
>> + * @cpu: CPU from which to read register.
>> + * @min_perf: Return address.
>> + */
>> +int cppc_get_min_perf(int cpu, u64 *min_perf)
>> +{
>> +     return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
>> +
>> +/**
>> + * cppc_set_min_perf - Write minimum performance register.
>> + * @cpu: CPU to which to write register.
>> + * @min_perf: the desired minimum performance value to be updated.
>> + */
>> +int cppc_set_min_perf(int cpu, u32 min_perf)
>> +{
>> +     return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
>> +
>> +/**
>> + * cppc_get_max_perf - Read maximum performance register.
>> + * @cpu: CPU from which to read register.
>> + * @max_perf: Return address.
>> + */
>> +int cppc_get_max_perf(int cpu, u64 *max_perf)
>> +{
>> +     return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
>> +
>> +/**
>> + * cppc_set_max_perf - Write maximum performance register.
>> + * @cpu: CPU to which to write register.
>> + * @max_perf: the desired maximum performance value to be updated.
>> + */
>> +int cppc_set_max_perf(int cpu, u32 max_perf)
>> +{
>> +     return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
>> +
>>   /**
>>    * cppc_set_enable - Set to enable CPPC on the processor by writing 
>> the
>>    * Continuous Performance Control package EnableRegister field.
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c 
>> b/drivers/cpufreq/cppc_cpufreq.c
>> index 229880c4eedb..66e183b45fb0 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -28,6 +28,8 @@
>>
>>   static struct cpufreq_driver cppc_cpufreq_driver;
>>
>> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
>> +
>
> Shouldn't concurrent access be handled by the policy->rwsem ?
>
> I think this can be checked using either:
> - lockdep_assert_held_write(&policy->rwsem)
> - lockdep_assert_held_read(&policy->rwsem)
>
> in store/show_max_perf() for instance.
>

You're right. The cpufreq sysfs already holds policy->rwsem for
show/store callbacks. I'll remove the mutex and add lockdep
assertions for the expected locking.
--------
File: drivers/cpufreq/cpufreq.c
   static ssize_t store(struct kobject *kobj, struct attribute *attr,
                      const char *buf, size_t count)
   {
       struct cpufreq_policy *policy = to_policy(kobj);
       ....
       guard(cpufreq_policy_write)(policy);
--------

Thank you,
Sumit Gupta


>
>>   #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>>   static enum {
>>       FIE_UNSET = -1,
>> @@ -570,6 +572,35 @@ static void populate_efficiency_class(void)
>>   }
>>   #endif
>>
>> +/* Set min/max performance HW register and cache the value */
>> +static int cppc_cpufreq_set_mperf_reg(struct cpufreq_policy *policy,
>> +                                   u64 val, bool is_min)
>> +{
>> +     struct cppc_cpudata *cpu_data = policy->driver_data;
>> +     struct cppc_perf_caps *caps = &cpu_data->perf_caps;
>> +     unsigned int cpu = policy->cpu;
>> +     u32 perf;
>> +     int ret;
>> +
>> +     perf = clamp(val, caps->lowest_perf, caps->highest_perf);
>> +
>> +     ret = is_min ? cppc_set_min_perf(cpu, perf) :
>> +                    cppc_set_max_perf(cpu, perf);
>> +     if (ret) {
>> +             if (ret != -EOPNOTSUPP)
>> +                     pr_warn("CPU%d: set %s_perf=%u failed (%d)\n",
>> +                             cpu, is_min ? "min" : "max", perf, ret);
>> +             return ret;
>> +     }
>> +
>> +     if (is_min)
>> +             cpu_data->perf_ctrls.min_perf = perf;
>> +     else
>> +             cpu_data->perf_ctrls.max_perf = perf;
>> +
>> +     return 0;
>> +}
>> +
>>   static struct cppc_cpudata *cppc_cpufreq_get_cpu_data(unsigned int 
>> cpu)
>>   {
>>       struct cppc_cpudata *cpu_data;
>> @@ -918,16 +949,142 @@ CPPC_CPUFREQ_ATTR_RW_U64(auto_act_window, 
>> cppc_get_auto_act_window,
>>   CPPC_CPUFREQ_ATTR_RW_U64(energy_performance_preference_val,
>>                        cppc_get_epp_perf, cppc_set_epp)
>>
>> +/**
>> + * show_min_perf - Show minimum performance as frequency (kHz)
>> + * @policy: cpufreq policy
>> + * @buf: buffer to write the frequency value to
>> + *
>> + * Reads the MIN_PERF register and converts the performance value to
>> + * frequency (kHz).
>> + */
>> +static ssize_t show_min_perf(struct cpufreq_policy *policy, char *buf)
>> +{
>> +     struct cppc_cpudata *cpu_data = policy->driver_data;
>> +     struct cppc_perf_caps *caps = &cpu_data->perf_caps;
>> +     u64 perf;
>> +     int ret;
>> +
>> +     ret = cppc_get_min_perf(policy->cpu, &perf);
>> +     if (ret == -EOPNOTSUPP)
>> +             return sysfs_emit(buf, "<unsupported>\n");
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* Use lowest_perf if register is uninitialized (0) */
>> +     if (perf == 0)
>> +             perf = caps->lowest_perf;
>> +
>> +     /* Convert performance to frequency (kHz) for user */
>> +     return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
>> +}
>> +
>> +/**
>> + * store_min_perf - Set minimum performance from frequency (kHz)
>> + * @policy: cpufreq policy
>> + * @buf: buffer containing the frequency value
>> + * @count: size of @buf
>> + *
>> + * Converts the user-provided frequency (kHz) to a performance value
>> + * and writes it to the MIN_PERF register.
>> + */
>> +static ssize_t store_min_perf(struct cpufreq_policy *policy, const 
>> char *buf,
>> +                           size_t count)
>> +{
>> +     struct cppc_cpudata *cpu_data = policy->driver_data;
>> +     unsigned int freq_khz;
>> +     u64 perf;
>> +     int ret;
>> +
>> +     ret = kstrtouint(buf, 0, &freq_khz);
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* Convert frequency (kHz) to performance value */
>> +     perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
>> +
>> +     guard(mutex)(&cppc_cpufreq_autonomous_lock);
>> +     ret = cppc_cpufreq_set_mperf_reg(policy, perf, true);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return count;
>> +}
>> +
>> +/**
>> + * show_max_perf - Show maximum performance as frequency (kHz)
>> + * @policy: cpufreq policy
>> + * @buf: buffer to write the frequency value to
>> + *
>> + * Reads the MAX_PERF register and converts the performance value to
>> + * frequency (kHz).
>> + */
>> +static ssize_t show_max_perf(struct cpufreq_policy *policy, char *buf)
>> +{
>> +     struct cppc_cpudata *cpu_data = policy->driver_data;
>> +     struct cppc_perf_caps *caps = &cpu_data->perf_caps;
>> +     u64 perf;
>> +     int ret;
>> +
>> +     ret = cppc_get_max_perf(policy->cpu, &perf);
>> +     if (ret == -EOPNOTSUPP)
>> +             return sysfs_emit(buf, "<unsupported>\n");
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* Use highest_perf if register is uninitialized or out of 
>> range */
>> +     if (perf == 0 || perf > caps->highest_perf)
>> +             perf = caps->highest_perf;
>> +
>> +     /* Convert performance to frequency (kHz) for user */
>> +     return sysfs_emit(buf, "%u\n", cppc_perf_to_khz(caps, perf));
>> +}
>> +
>> +/**
>> + * store_max_perf - Set maximum performance from frequency (kHz)
>> + * @policy: cpufreq policy
>> + * @buf: buffer containing the frequency value
>> + * @count: size of @buf
>> + *
>> + * Converts the user-provided frequency (kHz) to a performance value
>> + * and writes it to the MAX_PERF register.
>> + */
>> +static ssize_t store_max_perf(struct cpufreq_policy *policy, const 
>> char *buf,
>> +                           size_t count)
>> +{
>> +     struct cppc_cpudata *cpu_data = policy->driver_data;
>> +     unsigned int freq_khz;
>> +     u64 perf;
>> +     int ret;
>> +
>> +     ret = kstrtouint(buf, 0, &freq_khz);
>> +     if (ret)
>> +             return ret;
>> +
>> +     /* Convert frequency (kHz) to performance value */
>> +     perf = cppc_khz_to_perf(&cpu_data->perf_caps, freq_khz);
>> +
>> +     guard(mutex)(&cppc_cpufreq_autonomous_lock);
>> +     ret = cppc_cpufreq_set_mperf_reg(policy, perf, false);
>> +     if (ret)
>> +             return ret;
>> +
>> +     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(min_perf);
>> +cpufreq_freq_attr_rw(max_perf);
>>
>>   static struct freq_attr *cppc_cpufreq_attr[] = {
>>       &freqdomain_cpus,
>>       &auto_select,
>>       &auto_act_window,
>>       &energy_performance_preference_val,
>> +     &min_perf,
>> +     &max_perf,
>>       NULL,
>>   };
>>
>> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
>> index 3fc796c0d902..b358440cd0e2 100644
>> --- a/include/acpi/cppc_acpi.h
>> +++ b/include/acpi/cppc_acpi.h
>> @@ -174,6 +174,10 @@ 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);
>>   extern int cppc_set_auto_sel(int cpu, bool enable);
>> +extern int cppc_get_min_perf(int cpu, u64 *min_perf);
>> +extern int cppc_set_min_perf(int cpu, u32 min_perf);
>> +extern int cppc_get_max_perf(int cpu, u64 *max_perf);
>> +extern int cppc_set_max_perf(int cpu, u32 max_perf);
>>   extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
>>   extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 
>> *numerator);
>>   extern int amd_detect_prefcore(bool *detected);
>> @@ -270,6 +274,22 @@ static inline int cppc_set_auto_sel(int cpu, 
>> bool enable)
>>   {
>>       return -EOPNOTSUPP;
>>   }
>> +static inline int cppc_get_min_perf(int cpu, u64 *min_perf)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +static inline int cppc_set_min_perf(int cpu, u32 min_perf)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +static inline int cppc_get_max_perf(int cpu, u64 *max_perf)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +static inline int cppc_set_max_perf(int cpu, u32 max_perf)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>>   static inline int amd_get_highest_perf(unsigned int cpu, u32 
>> *highest_perf)
>>   {
>>       return -ENODEV;
Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by Pierre Gondois 1 week, 5 days ago
On 1/24/26 21:32, Sumit Gupta wrote:
>
> On 22/01/26 17:06, Pierre Gondois wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 1/20/26 15:56, Sumit Gupta wrote:
>>> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read 
>>> and
>>> write the MIN_PERF and MAX_PERF registers.
>>>
>>> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
>>> to expose these controls to userspace. The sysfs values are in 
>>> frequency
>>> (kHz) for consistency with other cpufreq sysfs files.
>>>
>>> A mutex is used to serialize sysfs store operations to ensure hardware
>>> register writes and perf_ctrls updates are atomic.
>>>
>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>>> ---
>>>   drivers/acpi/cppc_acpi.c       |  44 +++++++++
>>>   drivers/cpufreq/cppc_cpufreq.c | 157 
>>> +++++++++++++++++++++++++++++++++
>>>   include/acpi/cppc_acpi.h       |  20 +++++
>>>   3 files changed, 221 insertions(+)
>>>
>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>> index 45c6bd6ec24b..46bf45f8b0f3 100644
>>> --- a/drivers/acpi/cppc_acpi.c
>>> +++ b/drivers/acpi/cppc_acpi.c
>>> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>>>   }
>>>   EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>>>
>>> +/**
>>> + * cppc_get_min_perf - Read minimum performance register.
>>> + * @cpu: CPU from which to read register.
>>> + * @min_perf: Return address.
>>> + */
>>> +int cppc_get_min_perf(int cpu, u64 *min_perf)
>>> +{
>>> +     return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
>>> +
>>> +/**
>>> + * cppc_set_min_perf - Write minimum performance register.
>>> + * @cpu: CPU to which to write register.
>>> + * @min_perf: the desired minimum performance value to be updated.
>>> + */
>>> +int cppc_set_min_perf(int cpu, u32 min_perf)
>>> +{
>>> +     return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
>>> +
>>> +/**
>>> + * cppc_get_max_perf - Read maximum performance register.
>>> + * @cpu: CPU from which to read register.
>>> + * @max_perf: Return address.
>>> + */
>>> +int cppc_get_max_perf(int cpu, u64 *max_perf)
>>> +{
>>> +     return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
>>> +
>>> +/**
>>> + * cppc_set_max_perf - Write maximum performance register.
>>> + * @cpu: CPU to which to write register.
>>> + * @max_perf: the desired maximum performance value to be updated.
>>> + */
>>> +int cppc_set_max_perf(int cpu, u32 max_perf)
>>> +{
>>> +     return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
>>> +
>>>   /**
>>>    * cppc_set_enable - Set to enable CPPC on the processor by 
>>> writing the
>>>    * Continuous Performance Control package EnableRegister field.
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c 
>>> b/drivers/cpufreq/cppc_cpufreq.c
>>> index 229880c4eedb..66e183b45fb0 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -28,6 +28,8 @@
>>>
>>>   static struct cpufreq_driver cppc_cpufreq_driver;
>>>
>>> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
>>> +
>>
>> Shouldn't concurrent access be handled by the policy->rwsem ?
>>
>> I think this can be checked using either:
>> - lockdep_assert_held_write(&policy->rwsem)
>> - lockdep_assert_held_read(&policy->rwsem)
>>
>> in store/show_max_perf() for instance.
>>
>
> You're right. The cpufreq sysfs already holds policy->rwsem for
> show/store callbacks. I'll remove the mutex and add lockdep
> assertions for the expected locking.

I think it's ok not to have lockde assertions.

It seems that it is a common assumption sysfs files cannot be modified

concurrently. None of the cpufreq driver seems to use lockdep assertion.


> --------
> File: drivers/cpufreq/cpufreq.c
>   static ssize_t store(struct kobject *kobj, struct attribute *attr,
>                      const char *buf, size_t count)
>   {
>       struct cpufreq_policy *policy = to_policy(kobj);
>       ....
>       guard(cpufreq_policy_write)(policy);
> --------
>
> Thank you,
> Sumit Gupta
>
Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max_perf
Posted by Sumit Gupta 1 week, 4 days ago
>>> On 1/20/26 15:56, Sumit Gupta wrote:
>>>> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read
>>>> and
>>>> write the MIN_PERF and MAX_PERF registers.
>>>>
>>>> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
>>>> to expose these controls to userspace. The sysfs values are in
>>>> frequency
>>>> (kHz) for consistency with other cpufreq sysfs files.
>>>>
>>>> A mutex is used to serialize sysfs store operations to ensure hardware
>>>> register writes and perf_ctrls updates are atomic.
>>>>
>>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>>>> ---
>>>>   drivers/acpi/cppc_acpi.c       |  44 +++++++++
>>>>   drivers/cpufreq/cppc_cpufreq.c | 157
>>>> +++++++++++++++++++++++++++++++++
>>>>   include/acpi/cppc_acpi.h       |  20 +++++
>>>>   3 files changed, 221 insertions(+)
>>>>
>>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>>> index 45c6bd6ec24b..46bf45f8b0f3 100644
>>>> --- a/drivers/acpi/cppc_acpi.c
>>>> +++ b/drivers/acpi/cppc_acpi.c
>>>> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>>>>
>>>> +/**
>>>> + * cppc_get_min_perf - Read minimum performance register.
>>>> + * @cpu: CPU from which to read register.
>>>> + * @min_perf: Return address.
>>>> + */
>>>> +int cppc_get_min_perf(int cpu, u64 *min_perf)
>>>> +{
>>>> +     return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
>>>> +
>>>> +/**
>>>> + * cppc_set_min_perf - Write minimum performance register.
>>>> + * @cpu: CPU to which to write register.
>>>> + * @min_perf: the desired minimum performance value to be updated.
>>>> + */
>>>> +int cppc_set_min_perf(int cpu, u32 min_perf)
>>>> +{
>>>> +     return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
>>>> +
>>>> +/**
>>>> + * cppc_get_max_perf - Read maximum performance register.
>>>> + * @cpu: CPU from which to read register.
>>>> + * @max_perf: Return address.
>>>> + */
>>>> +int cppc_get_max_perf(int cpu, u64 *max_perf)
>>>> +{
>>>> +     return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
>>>> +
>>>> +/**
>>>> + * cppc_set_max_perf - Write maximum performance register.
>>>> + * @cpu: CPU to which to write register.
>>>> + * @max_perf: the desired maximum performance value to be updated.
>>>> + */
>>>> +int cppc_set_max_perf(int cpu, u32 max_perf)
>>>> +{
>>>> +     return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
>>>> +
>>>>   /**
>>>>    * cppc_set_enable - Set to enable CPPC on the processor by
>>>> writing the
>>>>    * Continuous Performance Control package EnableRegister field.
>>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c
>>>> b/drivers/cpufreq/cppc_cpufreq.c
>>>> index 229880c4eedb..66e183b45fb0 100644
>>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>>> @@ -28,6 +28,8 @@
>>>>
>>>>   static struct cpufreq_driver cppc_cpufreq_driver;
>>>>
>>>> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
>>>> +
>>>
>>> Shouldn't concurrent access be handled by the policy->rwsem ?
>>>
>>> I think this can be checked using either:
>>> - lockdep_assert_held_write(&policy->rwsem)
>>> - lockdep_assert_held_read(&policy->rwsem)
>>>
>>> in store/show_max_perf() for instance.
>>>
>>
>> You're right. The cpufreq sysfs already holds policy->rwsem for
>> show/store callbacks. I'll remove the mutex and add lockdep
>> assertions for the expected locking.
>
> I think it's ok not to have lockde assertions.
>
> It seems that it is a common assumption sysfs files cannot be modified
>
> concurrently. None of the cpufreq driver seems to use lockdep assertion.
>

Sure. I will just remove the mutex, keeping it consistent with other 
cpufreq drivers.

Thank you,
Sumit Gupta


>
>> --------
>> File: drivers/cpufreq/cpufreq.c
>>   static ssize_t store(struct kobject *kobj, struct attribute *attr,
>>                      const char *buf, size_t count)
>>   {
>>       struct cpufreq_policy *policy = to_policy(kobj);
>>       ....
>>       guard(cpufreq_policy_write)(policy);
>> --------
>>
>> Thank you,
>> Sumit Gupta
>>