[RFC v3 3/4] platform/x86/amd: dptc: Add platform profile support

Antheas Kapenekakis posted 4 patches 4 weeks ago
There is a newer version of this series
[RFC v3 3/4] platform/x86/amd: dptc: Add platform profile support
Posted by Antheas Kapenekakis 4 weeks ago
Register a platform_profile handler so the driver exposes standard
power profiles (low-power, balanced, performance, max-power) alongside
the manual tunable interface. max-power is only supplied when the
performance profile's preset values are below the tunable limits.

When a non-custom profile is active, parameter writes are blocked
(-EPERM) and current_value reflects the profile's preset values.
Selecting the "custom" profile returns control to the user for manual
staging and committing. On resume, the active profile is automatically
re-applied.

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/amd/Kconfig |   1 +
 drivers/platform/x86/amd/dptc.c  | 116 +++++++++++++++++++++++++++++--
 2 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
index d610092467fc..41ffbd722524 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -48,6 +48,7 @@ config AMD_ISP_PLATFORM
 config AMD_DPTC
 	tristate "AMD Dynamic Power and Thermal Configuration Interface (DPTCi)"
 	depends on X86_64 && ACPI && DMI
+	select ACPI_PLATFORM_PROFILE
 	select FIRMWARE_ATTRIBUTES_CLASS
 	help
 	  Driver for AMD AGESA ALIB Function 0x0C, the Dynamic Power and
diff --git a/drivers/platform/x86/amd/dptc.c b/drivers/platform/x86/amd/dptc.c
index acfe9cc01bab..1f07e2e6f278 100644
--- a/drivers/platform/x86/amd/dptc.c
+++ b/drivers/platform/x86/amd/dptc.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/platform_profile.h>
 #include <linux/processor.h>
 #include <linux/sysfs.h>
 #include <linux/unaligned.h>
@@ -56,8 +57,13 @@ struct dptc_param_limits {
 	u32 expanded_max;
 };
 
+struct dptc_profile {
+	u32 vals[DPTC_NUM_PARAMS];	/* 0 = don't set / unstage this param */
+};
+
 struct dptc_device_limits {
 	struct dptc_param_limits params[DPTC_NUM_PARAMS];
+	struct dptc_profile profiles[PLATFORM_PROFILE_LAST];
 };
 
 struct dptc_param_desc {
@@ -85,6 +91,10 @@ static const struct dptc_device_limits limits_maxhh = { .params = {
 	[DPTC_PPT_PL2_SPPT] = {  0,  4, 27,  82, 100 },
 	[DPTC_PPT_PL3_FPPT] = {  0,  4, 40,  85, 100 },
 	[DPTC_CPU_TEMP]     = { 60, 70, 95,  95, 100 },
+}, .profiles = {
+	[PLATFORM_PROFILE_LOW_POWER]   = { .vals = { 15, 15, 25, 0 } },
+	[PLATFORM_PROFILE_BALANCED]    = { .vals = { 25, 27, 40, 0 } },
+	[PLATFORM_PROFILE_PERFORMANCE] = { .vals = { 60, 63, 85, 0 } },
 }};
 
 /* Substring matches against boot_cpu_data.x86_model_id; order matters. */
@@ -134,6 +144,9 @@ struct dptc_priv {
 
 	bool expanded;
 
+	enum platform_profile_option profile;
+	struct device *ppdev;
+
 	enum dptc_save_mode { SAVE_SINGLE, SAVE_BULK } save_mode;
 
 	u32 staged[DPTC_NUM_PARAMS];
@@ -269,6 +282,14 @@ static ssize_t dptc_current_value_show(struct kobject *kobj,
 
 	guard(mutex)(&dptc->lock);
 
+	if (dptc->profile != PLATFORM_PROFILE_CUSTOM) {
+		u32 val = dptc->dev_limits->profiles[dptc->profile].vals[ps->idx];
+
+		if (!val)
+			return sysfs_emit(buf, "\n");
+		return sysfs_emit(buf, "%u\n", val);
+	}
+
 	if (!dptc->has_staged[ps->idx])
 		return sysfs_emit(buf, "\n");
 	return sysfs_emit(buf, "%u\n", dptc->staged[ps->idx]);
@@ -287,6 +308,8 @@ static ssize_t dptc_current_value_store(struct kobject *kobj,
 	if (count == 1 && buf[0] == '\n') {
 		guard(mutex)(&dptc->lock);
 
+		if (dptc->profile != PLATFORM_PROFILE_CUSTOM)
+			return -EPERM;
 		dptc->has_staged[ps->idx] = false;
 		return count;
 	}
@@ -297,6 +320,8 @@ static ssize_t dptc_current_value_store(struct kobject *kobj,
 
 	guard(mutex)(&dptc->lock);
 
+	if (dptc->profile != PLATFORM_PROFILE_CUSTOM)
+		return -EPERM;
 	min = dptc_get_min(dptc, ps->idx);
 	max = dptc_get_max(dptc, ps->idx);
 	if (val < min || (max && val > max))
@@ -426,6 +451,8 @@ static ssize_t dptc_expanded_current_value_store(struct kobject *kobj,
 
 	guard(mutex)(&dptc->lock);
 
+	if (dptc->profile != PLATFORM_PROFILE_CUSTOM)
+		return -EPERM;
 	dptc->expanded = val;
 	/* Clear staged values: limits changed, old values may be out of range */
 	memset(dptc->has_staged, 0, sizeof(dptc->has_staged));
@@ -594,6 +621,78 @@ static void dptc_kset_unregister(void *data)
 	kset_unregister(data);
 }
 
+/* Platform profile */
+
+static void dptc_apply_profile(struct dptc_priv *dptc,
+			       enum platform_profile_option profile)
+{
+	const struct dptc_profile *pp;
+	int i;
+
+	memset(dptc->has_staged, 0, sizeof(dptc->has_staged));
+
+	if (profile == PLATFORM_PROFILE_CUSTOM)
+		return;
+
+	pp = &dptc->dev_limits->profiles[profile];
+	for (i = 0; i < DPTC_NUM_PARAMS; i++) {
+		if (!pp->vals[i])
+			continue;
+		dptc->staged[i] = pp->vals[i];
+		dptc->has_staged[i] = true;
+	}
+}
+
+static int dptc_pp_probe(void *drvdata, unsigned long *choices)
+{
+	struct dptc_priv *dptc = drvdata;
+	int i, j;
+
+	set_bit(PLATFORM_PROFILE_CUSTOM, choices);
+	for (i = 0; i < PLATFORM_PROFILE_LAST; i++) {
+		for (j = 0; j < DPTC_NUM_PARAMS; j++) {
+			if (dptc->dev_limits->profiles[i].vals[j]) {
+				set_bit(i, choices);
+				break;
+			}
+		}
+	}
+	return 0;
+}
+
+static int dptc_pp_get(struct device *dev,
+		       enum platform_profile_option *profile)
+{
+	struct dptc_priv *dptc = dev_get_drvdata(dev);
+
+	guard(mutex)(&dptc->lock);
+
+	*profile = dptc->profile;
+	return 0;
+}
+
+static int dptc_pp_set(struct device *dev,
+		       enum platform_profile_option profile)
+{
+	struct dptc_priv *dptc = dev_get_drvdata(dev);
+	int ret = 0;
+
+	guard(mutex)(&dptc->lock);
+
+	dptc->profile = profile;
+	dptc_apply_profile(dptc, profile);
+	if (profile != PLATFORM_PROFILE_CUSTOM)
+		ret = dptc_alib_save(dptc);
+
+	return ret;
+}
+
+static const struct platform_profile_ops dptc_pp_ops = {
+	.probe       = dptc_pp_probe,
+	.profile_get = dptc_pp_get,
+	.profile_set = dptc_pp_set,
+};
+
 static int dptc_resume(struct device *dev)
 {
 	struct dptc_priv *dptc = dev_get_drvdata(dev);
@@ -601,10 +700,14 @@ static int dptc_resume(struct device *dev)
 
 	guard(mutex)(&dptc->lock);
 
-	if (dptc->save_mode == SAVE_SINGLE)
+	if (dptc->profile != PLATFORM_PROFILE_CUSTOM) {
+		dptc_apply_profile(dptc, dptc->profile);
 		ret = dptc_alib_save(dptc);
-	else
+	} else if (dptc->save_mode == SAVE_SINGLE) {
+		ret = dptc_alib_save(dptc);
+	} else {
 		ret = 0;
+	}
 
 	if (ret)
 		dev_warn(dev, "failed to restore settings on resume: %d\n", ret);
@@ -636,8 +739,7 @@ static int dptc_probe(struct platform_device *pdev)
 		 boot_cpu_data.x86_model_id);
 
 	dptc->fw_attr_dev = device_create(&firmware_attributes_class,
-					  NULL, MKDEV(0, 0), NULL,
-					  DRIVER_NAME);
+					  NULL, MKDEV(0, 0), NULL, DRIVER_NAME);
 	if (IS_ERR(dptc->fw_attr_dev))
 		return PTR_ERR(dptc->fw_attr_dev);
 
@@ -680,6 +782,12 @@ static int dptc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	dptc->profile = PLATFORM_PROFILE_CUSTOM;
+	dptc->ppdev = devm_platform_profile_register(dev, "amd-dptc", dptc,
+						     &dptc_pp_ops);
+	if (IS_ERR(dptc->ppdev))
+		return PTR_ERR(dptc->ppdev);
+
 	return 0;
 }
 
-- 
2.52.0
Re: [RFC v3 3/4] platform/x86/amd: dptc: Add platform profile support
Posted by Rong Zhang 4 weeks ago
Hi Antheas,

On Sat, 2026-03-07 at 12:55 +0100, Antheas Kapenekakis wrote:
> Register a platform_profile handler so the driver exposes standard
> power profiles (low-power, balanced, performance, max-power) alongside
> the manual tunable interface. max-power is only supplied when the
> performance profile's preset values are below the tunable limits.
> 
> When a non-custom profile is active, parameter writes are blocked
> (-EPERM) and current_value reflects the profile's preset values.
> Selecting the "custom" profile returns control to the user for manual
> staging and committing. On resume, the active profile is automatically
> re-applied.
> 
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/platform/x86/amd/Kconfig |   1 +
>  drivers/platform/x86/amd/dptc.c  | 116 +++++++++++++++++++++++++++++--
>  2 files changed, 113 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
> index d610092467fc..41ffbd722524 100644
> --- a/drivers/platform/x86/amd/Kconfig
> +++ b/drivers/platform/x86/amd/Kconfig
> @@ -48,6 +48,7 @@ config AMD_ISP_PLATFORM
>  config AMD_DPTC
>  	tristate "AMD Dynamic Power and Thermal Configuration Interface (DPTCi)"
>  	depends on X86_64 && ACPI && DMI
> +	select ACPI_PLATFORM_PROFILE
>  	select FIRMWARE_ATTRIBUTES_CLASS
>  	help
>  	  Driver for AMD AGESA ALIB Function 0x0C, the Dynamic Power and
> diff --git a/drivers/platform/x86/amd/dptc.c b/drivers/platform/x86/amd/dptc.c
> index acfe9cc01bab..1f07e2e6f278 100644
> --- a/drivers/platform/x86/amd/dptc.c
> +++ b/drivers/platform/x86/amd/dptc.c
> @@ -23,6 +23,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/platform_device.h>
> +#include <linux/platform_profile.h>
>  #include <linux/processor.h>
>  #include <linux/sysfs.h>
>  #include <linux/unaligned.h>
> @@ -56,8 +57,13 @@ struct dptc_param_limits {
>  	u32 expanded_max;
>  };
>  
> +struct dptc_profile {
> +	u32 vals[DPTC_NUM_PARAMS];	/* 0 = don't set / unstage this param */
> +};
> +
>  struct dptc_device_limits {
>  	struct dptc_param_limits params[DPTC_NUM_PARAMS];
> +	struct dptc_profile profiles[PLATFORM_PROFILE_LAST];
>  };
>  
>  struct dptc_param_desc {
> @@ -85,6 +91,10 @@ static const struct dptc_device_limits limits_maxhh = { .params = {
>  	[DPTC_PPT_PL2_SPPT] = {  0,  4, 27,  82, 100 },
>  	[DPTC_PPT_PL3_FPPT] = {  0,  4, 40,  85, 100 },
>  	[DPTC_CPU_TEMP]     = { 60, 70, 95,  95, 100 },
> +}, .profiles = {
> +	[PLATFORM_PROFILE_LOW_POWER]   = { .vals = { 15, 15, 25, 0 } },
> +	[PLATFORM_PROFILE_BALANCED]    = { .vals = { 25, 27, 40, 0 } },
> +	[PLATFORM_PROFILE_PERFORMANCE] = { .vals = { 60, 63, 85, 0 } },
>  }};
>  
>  /* Substring matches against boot_cpu_data.x86_model_id; order matters. */
> @@ -134,6 +144,9 @@ struct dptc_priv {
>  
>  	bool expanded;
>  
> +	enum platform_profile_option profile;
> +	struct device *ppdev;
> +
>  	enum dptc_save_mode { SAVE_SINGLE, SAVE_BULK } save_mode;
>  
>  	u32 staged[DPTC_NUM_PARAMS];
> @@ -269,6 +282,14 @@ static ssize_t dptc_current_value_show(struct kobject *kobj,
>  
>  	guard(mutex)(&dptc->lock);
>  
> +	if (dptc->profile != PLATFORM_PROFILE_CUSTOM) {
> +		u32 val = dptc->dev_limits->profiles[dptc->profile].vals[ps->idx];
> +
> +		if (!val)
> +			return sysfs_emit(buf, "\n");
> +		return sysfs_emit(buf, "%u\n", val);
> +	}
> +
>  	if (!dptc->has_staged[ps->idx])
>  		return sysfs_emit(buf, "\n");
>  	return sysfs_emit(buf, "%u\n", dptc->staged[ps->idx]);
> @@ -287,6 +308,8 @@ static ssize_t dptc_current_value_store(struct kobject *kobj,
>  	if (count == 1 && buf[0] == '\n') {
>  		guard(mutex)(&dptc->lock);
>  
> +		if (dptc->profile != PLATFORM_PROFILE_CUSTOM)
> +			return -EPERM;

It has nothing to do with permissions. I'd prefer -EBUSY.

To maintain readability, usually a blank line should be added after
returning early.

>  		dptc->has_staged[ps->idx] = false;
>  		return count;
>  	}
> @@ -297,6 +320,8 @@ static ssize_t dptc_current_value_store(struct kobject *kobj,
>  
>  	guard(mutex)(&dptc->lock);
>  
> +	if (dptc->profile != PLATFORM_PROFILE_CUSTOM)
> +		return -EPERM;
>  	min = dptc_get_min(dptc, ps->idx);
>  	max = dptc_get_max(dptc, ps->idx);
>  	if (val < min || (max && val > max))
> @@ -426,6 +451,8 @@ static ssize_t dptc_expanded_current_value_store(struct kobject *kobj,
>  
>  	guard(mutex)(&dptc->lock);
>  
> +	if (dptc->profile != PLATFORM_PROFILE_CUSTOM)
> +		return -EPERM;

IMO kstrtou32() isn't really a blocker of acquiring the mutex earlier
to prevent duplicating the same check. After all, firmware-attributes
is not a performance critical interface, and dptc_save_settings_store()
also calls sysfs_eq() with the mutex held.

>  	dptc->expanded = val;
>  	/* Clear staged values: limits changed, old values may be out of range */
>  	memset(dptc->has_staged, 0, sizeof(dptc->has_staged));
> @@ -594,6 +621,78 @@ static void dptc_kset_unregister(void *data)
>  	kset_unregister(data);
>  }
>  
> +/* Platform profile */
> +
> +static void dptc_apply_profile(struct dptc_priv *dptc,
> +			       enum platform_profile_option profile)
> +{
> +	const struct dptc_profile *pp;
> +	int i;
> +
> +	memset(dptc->has_staged, 0, sizeof(dptc->has_staged));
> +
> +	if (profile == PLATFORM_PROFILE_CUSTOM)
> +		return;
> +
> +	pp = &dptc->dev_limits->profiles[profile];
> +	for (i = 0; i < DPTC_NUM_PARAMS; i++) {
> +		if (!pp->vals[i])
> +			continue;
> +		dptc->staged[i] = pp->vals[i];
> +		dptc->has_staged[i] = true;
> +	}
> +}
> +
> +static int dptc_pp_probe(void *drvdata, unsigned long *choices)
> +{
> +	struct dptc_priv *dptc = drvdata;
> +	int i, j;
> +
> +	set_bit(PLATFORM_PROFILE_CUSTOM, choices);
> +	for (i = 0; i < PLATFORM_PROFILE_LAST; i++) {
> +		for (j = 0; j < DPTC_NUM_PARAMS; j++) {
> +			if (dptc->dev_limits->profiles[i].vals[j]) {
> +				set_bit(i, choices);
> +				break;
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int dptc_pp_get(struct device *dev,
> +		       enum platform_profile_option *profile)
> +{
> +	struct dptc_priv *dptc = dev_get_drvdata(dev);
> +
> +	guard(mutex)(&dptc->lock);
> +
> +	*profile = dptc->profile;
> +	return 0;
> +}
> +
> +static int dptc_pp_set(struct device *dev,
> +		       enum platform_profile_option profile)
> +{
> +	struct dptc_priv *dptc = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	guard(mutex)(&dptc->lock);
> +
> +	dptc->profile = profile;
> +	dptc_apply_profile(dptc, profile);
> +	if (profile != PLATFORM_PROFILE_CUSTOM)
> +		ret = dptc_alib_save(dptc);

I'd prefer simply calling dptc_alib_save() in dptc_apply_profile()
since the latter already checks the profile.

> +
> +	return ret;
> +}
> +
> +static const struct platform_profile_ops dptc_pp_ops = {
> +	.probe       = dptc_pp_probe,
> +	.profile_get = dptc_pp_get,
> +	.profile_set = dptc_pp_set,
> +};
> +
>  static int dptc_resume(struct device *dev)
>  {
>  	struct dptc_priv *dptc = dev_get_drvdata(dev);
> @@ -601,10 +700,14 @@ static int dptc_resume(struct device *dev)
>  
>  	guard(mutex)(&dptc->lock);
>  
> -	if (dptc->save_mode == SAVE_SINGLE)
> +	if (dptc->profile != PLATFORM_PROFILE_CUSTOM) {
> +		dptc_apply_profile(dptc, dptc->profile);
>  		ret = dptc_alib_save(dptc);
> -	else
> +	} else if (dptc->save_mode == SAVE_SINGLE) {
> +		ret = dptc_alib_save(dptc);
> +	} else {
>  		ret = 0;
> +	}
>  
>  	if (ret)
>  		dev_warn(dev, "failed to restore settings on resume: %d\n", ret);
> @@ -636,8 +739,7 @@ static int dptc_probe(struct platform_device *pdev)
>  		 boot_cpu_data.x86_model_id);
>  
>  	dptc->fw_attr_dev = device_create(&firmware_attributes_class,
> -					  NULL, MKDEV(0, 0), NULL,
> -					  DRIVER_NAME);
> +					  NULL, MKDEV(0, 0), NULL, DRIVER_NAME);

Squash the change into the previous patch.

Thanks,
Rong

>  	if (IS_ERR(dptc->fw_attr_dev))
>  		return PTR_ERR(dptc->fw_attr_dev);
>  
> @@ -680,6 +782,12 @@ static int dptc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	dptc->profile = PLATFORM_PROFILE_CUSTOM;
> +	dptc->ppdev = devm_platform_profile_register(dev, "amd-dptc", dptc,
> +						     &dptc_pp_ops);
> +	if (IS_ERR(dptc->ppdev))
> +		return PTR_ERR(dptc->ppdev);
> +
>  	return 0;
>  }
>