amd-pstate optimizations

PATCH In review Mario Limonciello (AMD) 2 participants 13 messages Oct 9, 2025 – Oct 15, 2025

Subsystems (2)

CPU frequency scaling framework AMD pstate driver

lore

Share & tools

lore

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #0 lore raw

v2 cover amd-pstate optimizations

Some optimizations for EPP sysfs files and then various other small things that I noticed while looking at the code again.

Mario Limonciello (AMD) (6):
cpufreq/amd-pstate: Use sysfs_match_string() for epp cpufreq/amd-pstate: Drop NULL value from amd_pstate_mode_string cpufreq/amd-pstate: Make amd_pstate_get_mode_string() never return
NULL
cpufreq/amd-pstate: Adjust return values in amd_pstate_update_status() cpufreq/amd-pstate: Fix some whitespace issues cpufreq/amd-pstate: Add static asserts for EPP indices

 drivers/cpufreq/amd-pstate.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

Signature · 1 line

2.43.0

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #1 lore raw

PATCH v2 1/6 cpufreq/amd-pstate: Use sysfs_match_string() for epp

Rather than scanning the buffer and manually matching the string use the sysfs macros.

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
v2:
* Drop NULL from energy_perf_strings too (Gautham)

---
 drivers/cpufreq/amd-pstate.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

drivers/cpufreq/amd-pstate.c +4 ~~-11~~

index b44f0f7a5ba1c..0bc5013448873 100644
@@ -118,7 +118,6 @@ static const char * const energy_perf_strings[] = {
 	[EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
 	[EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
 	[EPP_INDEX_POWERSAVE] = "power",
-	NULL
 };
 
 static unsigned int epp_values[] = {

@@ -1137,16 +1136,15 @@ static ssize_t show_amd_pstate_hw_prefcore(struct cpufreq_policy *policy,
 static ssize_t show_energy_performance_available_preferences(
 				struct cpufreq_policy *policy, char *buf)
 {
-	int i = 0;
-	int offset = 0;
+	int offset = 0, i;
 	struct amd_cpudata *cpudata = policy->driver_data;
 
 	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
 		return sysfs_emit_at(buf, offset, "%s\n",
 				energy_perf_strings[EPP_INDEX_PERFORMANCE]);
 
-	while (energy_perf_strings[i] != NULL)
-		offset += sysfs_emit_at(buf, offset, "%s ", energy_perf_strings[i++]);
+	for (i = 0; i < ARRAY_SIZE(energy_perf_strings); i++)
+		offset += sysfs_emit_at(buf, offset, "%s ", energy_perf_strings[i]);
 
 	offset += sysfs_emit_at(buf, offset, "\n");

@@ -1157,15 +1155,10 @@ static ssize_t store_energy_performance_preference(
 		struct cpufreq_policy *policy, const char *buf, size_t count)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
-	char str_preference[21];
 	ssize_t ret;
 	u8 epp;
 
-	ret = sscanf(buf, "%20s", str_preference);
-	if (ret != 1)
-		return -EINVAL;
-
-	ret = match_string(energy_perf_strings, -1, str_preference);
+	ret = sysfs_match_string(energy_perf_strings, buf);
 	if (ret < 0)
 		return -EINVAL;

Signature · 1 line

2.43.0

Gautham R. Shenoy

Reviewed-by on v2 1/6

2025-10-15 08:46 UTC #2 lore raw

Re: [PATCH v2 1/6] cpufreq/amd-pstate: Use sysfs_match_string() for epp

Quoted 4 lines · Mario Limonciello (AMD)

Rather than scanning the buffer and manually matching the string use the sysfs macros.
Signed-off-by Mario Limonciello (AMD) <[email protected]>

The patch looks good to me.

Reviewed-by Gautham R. Shenoy <[email protected]>

Quoted 59 lines

---
v2:
* Drop NULL from energy_perf_strings too (Gautham)

---
 drivers/cpufreq/amd-pstate.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

drivers/cpufreq/amd-pstate.c +4 ~~-11~~

index b44f0f7a5ba1c..0bc5013448873 100644
@@ -118,7 +118,6 @@ static const char * const energy_perf_strings[] = {
 	[EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
 	[EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
 	[EPP_INDEX_POWERSAVE] = "power",
-	NULL
 };
 
 static unsigned int epp_values[] = {

@@ -1137,16 +1136,15 @@ static ssize_t show_amd_pstate_hw_prefcore(struct cpufreq_policy *policy,
 static ssize_t show_energy_performance_available_preferences(
 				struct cpufreq_policy *policy, char *buf)
 {
-	int i = 0;
-	int offset = 0;
+	int offset = 0, i;
 	struct amd_cpudata *cpudata = policy->driver_data;
 
 	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
 		return sysfs_emit_at(buf, offset, "%s\n",
 				energy_perf_strings[EPP_INDEX_PERFORMANCE]);
 
-	while (energy_perf_strings[i] != NULL)
-		offset += sysfs_emit_at(buf, offset, "%s ", energy_perf_strings[i++]);
+	for (i = 0; i < ARRAY_SIZE(energy_perf_strings); i++)
+		offset += sysfs_emit_at(buf, offset, "%s ", energy_perf_strings[i]);
 
 	offset += sysfs_emit_at(buf, offset, "\n");

@@ -1157,15 +1155,10 @@ static ssize_t store_energy_performance_preference(
 		struct cpufreq_policy *policy, const char *buf, size_t count)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
-	char str_preference[21];
 	ssize_t ret;
 	u8 epp;
 
-	ret = sscanf(buf, "%20s", str_preference);
-	if (ret != 1)
-		return -EINVAL;
-
-	ret = match_string(energy_perf_strings, -1, str_preference);
+	ret = sysfs_match_string(energy_perf_strings, buf);
 	if (ret < 0)
 		return -EINVAL;

Signature · 1 line

2.43.0

Signature · 2 lines

Thanks and Regards
gautham.

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #3 lore raw

PATCH v2 2/6 cpufreq/amd-pstate: Drop NULL value from amd_pstate_mode_string

In reply to Mario Limonciello (AMD)

None of the users actually look for the NULL value. To avoid risk of regression introducing a new value but forgetting to add a string add a static assert to test AMD_PSTATE_MAX matches the array size.

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/cpufreq/amd-pstate.c +1 -1

index 0bc5013448873..a5b9e5baf4234 100644
@@ -65,8 +65,8 @@ static const char * const amd_pstate_mode_string[] = {
 	[AMD_PSTATE_PASSIVE]     = "passive",
 	[AMD_PSTATE_ACTIVE]      = "active",
 	[AMD_PSTATE_GUIDED]      = "guided",
-	NULL,
 };
+static_assert(ARRAY_SIZE(amd_pstate_mode_string) == AMD_PSTATE_MAX);
 
 const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode)
 {

Signature · 1 line

2.43.0

Gautham R. Shenoy

Reviewed-by on v2 2/6

2025-10-15 08:47 UTC #4 lore raw

Re: [PATCH v2 2/6] cpufreq/amd-pstate: Drop NULL value from amd_pstate_mode_string

Quoted 5 lines · Mario Limonciello (AMD)

None of the users actually look for the NULL value. To avoid risk of regression introducing a new value but forgetting to add a string add a static assert to test AMD_PSTATE_MAX matches the array size.
Signed-off-by Mario Limonciello (AMD) <[email protected]>

Thanks for getting rid of the NULL entry.

Reviewed-by Gautham R. Shenoy <[email protected]>

Quoted 21 lines

---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/cpufreq/amd-pstate.c +1 -1

index 0bc5013448873..a5b9e5baf4234 100644
@@ -65,8 +65,8 @@ static const char * const amd_pstate_mode_string[] = {
 	[AMD_PSTATE_PASSIVE]     = "passive",
 	[AMD_PSTATE_ACTIVE]      = "active",
 	[AMD_PSTATE_GUIDED]      = "guided",
-	NULL,
 };
+static_assert(ARRAY_SIZE(amd_pstate_mode_string) == AMD_PSTATE_MAX);
 
 const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode)
 {

Signature · 1 line

2.43.0

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #5 lore raw

PATCH v2 3/6 cpufreq/amd-pstate: Make amd_pstate_get_mode_string() never return NULL

In reply to Mario Limonciello (AMD)

amd_pstate_get_mode_string() is only used by amd-pstate-ut. Set the failure path to use AMD_PSTATE_UNDEFINED ("undefined") to avoid showing "(null)" as a string when running test suite.

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/cpufreq/amd-pstate.c +2 -2

index a5b9e5baf4234..5feb9f5e3a491 100644
@@ -70,8 +70,8 @@ static_assert(ARRAY_SIZE(amd_pstate_mode_string) == AMD_PSTATE_MAX);
 
 const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode)
 {
-	if (mode < 0 || mode >= AMD_PSTATE_MAX)
-		return NULL;
+	if (mode < AMD_PSTATE_UNDEFINED || mode >= AMD_PSTATE_MAX)
+		mode = AMD_PSTATE_UNDEFINED;
 	return amd_pstate_mode_string[mode];
 }
 EXPORT_SYMBOL_GPL(amd_pstate_get_mode_string);

Signature · 1 line

2.43.0

Gautham R. Shenoy

Reviewed-by on v2 3/6

2025-10-15 08:51 UTC #6 lore raw

Re: [PATCH v2 3/6] cpufreq/amd-pstate: Make amd_pstate_get_mode_string() never return NULL

Quoted 3 lines · Mario Limonciello (AMD)

amd_pstate_get_mode_string() is only used by amd-pstate-ut. Set the failure path to use AMD_PSTATE_UNDEFINED ("undefined") to avoid showing "(null)" as a string when running test suite.

Reviewed-by Gautham R. Shenoy <[email protected]>

Quoted 24 lines

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/cpufreq/amd-pstate.c +2 -2

index a5b9e5baf4234..5feb9f5e3a491 100644
@@ -70,8 +70,8 @@ static_assert(ARRAY_SIZE(amd_pstate_mode_string) == AMD_PSTATE_MAX);
 
 const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode)
 {
-	if (mode < 0 || mode >= AMD_PSTATE_MAX)
-		return NULL;
+	if (mode < AMD_PSTATE_UNDEFINED || mode >= AMD_PSTATE_MAX)
+		mode = AMD_PSTATE_UNDEFINED;
 	return amd_pstate_mode_string[mode];
 }
 EXPORT_SYMBOL_GPL(amd_pstate_get_mode_string);

Signature · 1 line

2.43.0

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #7 lore raw

PATCH v2 4/6 cpufreq/amd-pstate: Adjust return values in amd_pstate_update_status()

In reply to Mario Limonciello (AMD)

get_mode_idx_from_str() already checks the upper boundary for a string sent. Drop the extra check in amd_pstate_update_status() and pass the return code if there is a failure.

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/cpufreq/amd-pstate.c +2 -3

index 5feb9f5e3a491..2d2ef53d12447 100644
@@ -1346,9 +1346,8 @@ int amd_pstate_update_status(const char *buf, size_t size)
 		return -EINVAL;
 
 	mode_idx = get_mode_idx_from_str(buf, size);
-
-	if (mode_idx < 0 || mode_idx >= AMD_PSTATE_MAX)
-		return -EINVAL;
+	if (mode_idx < 0)
+		return mode_idx;
 
 	if (mode_state_machine[cppc_state][mode_idx]) {
 		guard(mutex)(&amd_pstate_driver_lock);

Signature · 1 line

2.43.0

Gautham R. Shenoy

Reviewed-by on v2 4/6

2025-10-15 09:02 UTC #8 lore raw

Re: [PATCH v2 4/6] cpufreq/amd-pstate: Adjust return values in amd_pstate_update_status()

Hello Mario,

Quoted 3 lines · Mario Limonciello (AMD)

get_mode_idx_from_str() already checks the upper boundary for a string sent. Drop the extra check in amd_pstate_update_status() and pass the return code if there is a failure.

Looks good to me. This patch doesn't introduce any behavioural change since in the absence of a match, get_mode_idx_from_str() returns -EINVAL, which is what was being returned by amd_pstate_update_status() previously when @buf contains an invalid string.

Reviewed-by Gautham R. Shenoy <[email protected]>

Quoted 25 lines

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/cpufreq/amd-pstate.c +2 -3

index 5feb9f5e3a491..2d2ef53d12447 100644
@@ -1346,9 +1346,8 @@ int amd_pstate_update_status(const char *buf, size_t size)
 		return -EINVAL;
 
 	mode_idx = get_mode_idx_from_str(buf, size);
-
-	if (mode_idx < 0 || mode_idx >= AMD_PSTATE_MAX)
-		return -EINVAL;
+	if (mode_idx < 0)
+		return mode_idx;
 
 	if (mode_state_machine[cppc_state][mode_idx]) {
 		guard(mutex)(&amd_pstate_driver_lock);

Signature · 1 line

2.43.0

Signature · 2 lines

Thanks and Regards
gautham.

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #9 lore raw

PATCH v2 5/6 cpufreq/amd-pstate: Fix some whitespace issues

In reply to Mario Limonciello (AMD)

Add whitespace around the equals and remove leading space.

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/cpufreq/amd-pstate.c +2 -2

index 2d2ef53d12447..a0f21ac1205af 100644
@@ -126,7 +126,7 @@ static unsigned int epp_values[] = {
 	[EPP_INDEX_BALANCE_PERFORMANCE] = AMD_CPPC_EPP_BALANCE_PERFORMANCE,
 	[EPP_INDEX_BALANCE_POWERSAVE] = AMD_CPPC_EPP_BALANCE_POWERSAVE,
 	[EPP_INDEX_POWERSAVE] = AMD_CPPC_EPP_POWERSAVE,
- };
+};
 
 typedef int (*cppc_mode_transition_fn)(int);

@@ -182,7 +182,7 @@ static inline int get_mode_idx_from_str(const char *str, size_t size)
 {
 	int i;
 
-	for (i=0; i < AMD_PSTATE_MAX; i++) {
+	for (i = 0; i < AMD_PSTATE_MAX; i++) {
 		if (!strncmp(str, amd_pstate_mode_string[i], size))
 			return i;
 	}

Signature · 1 line

2.43.0

Gautham R. Shenoy

Reviewed-by on v2 5/6

2025-10-15 09:04 UTC #10 lore raw

Re: [PATCH v2 5/6] cpufreq/amd-pstate: Fix some whitespace issues

Quoted 2 lines · Mario Limonciello (AMD)

Add whitespace around the equals and remove leading space.

Thanks for cleaning this up.

Reviewed-by Gautham R. Shenoy <[email protected]>

Quoted 30 lines

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/cpufreq/amd-pstate.c +2 -2

index 2d2ef53d12447..a0f21ac1205af 100644
@@ -126,7 +126,7 @@ static unsigned int epp_values[] = {
 	[EPP_INDEX_BALANCE_PERFORMANCE] = AMD_CPPC_EPP_BALANCE_PERFORMANCE,
 	[EPP_INDEX_BALANCE_POWERSAVE] = AMD_CPPC_EPP_BALANCE_POWERSAVE,
 	[EPP_INDEX_POWERSAVE] = AMD_CPPC_EPP_POWERSAVE,
- };
+};
 
 typedef int (*cppc_mode_transition_fn)(int);

@@ -182,7 +182,7 @@ static inline int get_mode_idx_from_str(const char *str, size_t size)
 {
 	int i;
 
-	for (i=0; i < AMD_PSTATE_MAX; i++) {
+	for (i = 0; i < AMD_PSTATE_MAX; i++) {
 		if (!strncmp(str, amd_pstate_mode_string[i], size))
 			return i;
 	}

Signature · 1 line

2.43.0

Signature · 2 lines

Thanks and Regards
gautham.

Mario Limonciello (AMD)

author

2025-10-09 16:18 UTC #11 lore raw

PATCH v2 6/6 cpufreq/amd-pstate: Add static asserts for EPP indices

In reply to Mario Limonciello (AMD)

In case a new index is introduced add a static assert to make sure that strings and values are updated.

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/cpufreq/amd-pstate.c +3 -0

index a0f21ac1205af..b3dad7cde46f0 100644
@@ -110,6 +110,7 @@ enum energy_perf_value_index {
 	EPP_INDEX_BALANCE_PERFORMANCE,
 	EPP_INDEX_BALANCE_POWERSAVE,
 	EPP_INDEX_POWERSAVE,
+	EPP_INDEX_MAX,
 };
 
 static const char * const energy_perf_strings[] = {

@@ -119,6 +120,7 @@ static const char * const energy_perf_strings[] = {
 	[EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
 	[EPP_INDEX_POWERSAVE] = "power",
 };
+static_assert(ARRAY_SIZE(energy_perf_strings) == EPP_INDEX_MAX);
 
 static unsigned int epp_values[] = {
 	[EPP_INDEX_DEFAULT] = 0,

@@ -127,6 +129,7 @@ static unsigned int epp_values[] = {
 	[EPP_INDEX_BALANCE_POWERSAVE] = AMD_CPPC_EPP_BALANCE_POWERSAVE,
 	[EPP_INDEX_POWERSAVE] = AMD_CPPC_EPP_POWERSAVE,
 };
+static_assert(ARRAY_SIZE(epp_values) == EPP_INDEX_MAX);
 
 typedef int (*cppc_mode_transition_fn)(int);

Signature · 1 line

2.43.0

Gautham R. Shenoy

Reviewed-by on v2 6/6

2025-10-15 09:11 UTC #12 lore raw

Re: [PATCH v2 6/6] cpufreq/amd-pstate: Add static asserts for EPP indices

Hello Mario,

Quoted 3 lines · Mario Limonciello (AMD)

In case a new index is introduced add a static assert to make sure that strings and values are updated.

Thanks for hardening this part of the code.

Reviewed-by Gautham R. Shenoy <[email protected]>

Quoted 36 lines

Signed-off-by Mario Limonciello (AMD) <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/cpufreq/amd-pstate.c +3 -0

index a0f21ac1205af..b3dad7cde46f0 100644
@@ -110,6 +110,7 @@ enum energy_perf_value_index {
 	EPP_INDEX_BALANCE_PERFORMANCE,
 	EPP_INDEX_BALANCE_POWERSAVE,
 	EPP_INDEX_POWERSAVE,
+	EPP_INDEX_MAX,
 };
 
 static const char * const energy_perf_strings[] = {

@@ -119,6 +120,7 @@ static const char * const energy_perf_strings[] = {
 	[EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
 	[EPP_INDEX_POWERSAVE] = "power",
 };
+static_assert(ARRAY_SIZE(energy_perf_strings) == EPP_INDEX_MAX);
 
 static unsigned int epp_values[] = {
 	[EPP_INDEX_DEFAULT] = 0,

@@ -127,6 +129,7 @@ static unsigned int epp_values[] = {
 	[EPP_INDEX_BALANCE_POWERSAVE] = AMD_CPPC_EPP_BALANCE_POWERSAVE,
 	[EPP_INDEX_POWERSAVE] = AMD_CPPC_EPP_POWERSAVE,
 };
+static_assert(ARRAY_SIZE(epp_values) == EPP_INDEX_MAX);
 
 typedef int (*cppc_mode_transition_fn)(int);

Signature · 1 line

2.43.0

Signature · 2 lines

Thanks and Regards
gautham.