[PATCH] power/amd_pstate: fix frequency matching for continuous scaling

[PATCH] power/amd_pstate: fix frequency matching for continuous scaling

[Stephen Hemminger]

The power_init_for_setting_freq() function fails on systems using the
amd-pstate-epp driver because the current CPU frequency read from
scaling_setspeed does not exactly match any of the synthesized
frequency buckets. Unlike acpi_cpufreq which provides a discrete list
of frequencies, amd-pstate operates with continuously variable
frequencies, so an exact match will rarely succeed.

For example, on a Ryzen 9 7945HX the sysfs file reports 2797172
which rounds to 2797000, but this value does not appear in the
generated frequency table.

Replace the exact match lookup with a nearest-frequency search.

Also fix several issues in the same function:
 - strtoul() was called with NULL endptr, making parse failures
   undetectable
 - errno was not checked, so stale errors could cause false positives
 - freq was declared as uint32_t, truncating the unsigned long
   return value of strtoul() on LP64 platforms
 - no error was logged when frequency matching failed

Bugzilla ID: 1915
Fixes: 1ed04d33cf19 ("power: support amd-pstate cpufreq driver")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/power/amd_pstate/amd_pstate_cpufreq.c | 56 ++++++++++++++-----
 1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/power/amd_pstate/amd_pstate_cpufreq.c b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
index 95495bff7d..14625ffd73 100644
--- a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
+++ b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
@@ -5,6 +5,7 @@
  */
 
 #include <stdlib.h>
+#include <errno.h>
 
 #include <rte_memcpy.h>
 #include <rte_stdatomic.h>
@@ -291,15 +292,21 @@ power_get_available_freqs(struct amd_pstate_power_info *pi)
 	return ret;
 }
 
-/**
- * It is to fopen the sys file for the future setting the lcore frequency.
- */
+static inline unsigned long
+abs_diff(unsigned long a, unsigned long b)
+{
+	return (a > b) ? a - b : b - a;
+}
+
 static int
 power_init_for_setting_freq(struct amd_pstate_power_info *pi)
 {
-	FILE *f = NULL;
+	FILE *f;
 	char buf[BUFSIZ];
-	uint32_t i, freq;
+	char *endptr;
+	unsigned long freq, freq_conv;
+	unsigned long best_diff, diff;
+	uint32_t i, best_idx;
 	int ret;
 
 	open_core_sysfs_file(&f, "rw+", POWER_SYSFILE_SETSPEED, pi->lcore_id);
@@ -308,7 +315,6 @@ power_init_for_setting_freq(struct amd_pstate_power_info *pi)
 				POWER_SYSFILE_SETSPEED);
 		goto err;
 	}
-
 	ret = read_core_sysfs_s(f, buf, sizeof(buf));
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
@@ -316,25 +322,45 @@ power_init_for_setting_freq(struct amd_pstate_power_info *pi)
 		goto err;
 	}
 
-	freq = strtoul(buf, NULL, POWER_CONVERT_TO_DECIMAL);
+	errno = 0;
+	freq = strtoul(buf, &endptr, POWER_CONVERT_TO_DECIMAL);
+	if (errno != 0 || endptr == buf || freq == 0) {
+		POWER_LOG(ERR, "Failed to parse frequency '%s' for lcore %u",
+				buf, pi->lcore_id);
+		goto err;
+	}
 
 	/* convert the frequency to nearest 1000 value
 	 * Ex: if freq=1396789 then freq_conv=1397000
 	 * Ex: if freq=800030 then freq_conv=800000
 	 */
-	unsigned int freq_conv = 0;
-	freq_conv = (freq + FREQ_ROUNDING_DELTA)
-				/ ROUND_FREQ_TO_N_1000;
+	freq_conv = (freq + FREQ_ROUNDING_DELTA) / ROUND_FREQ_TO_N_1000;
 	freq_conv = freq_conv * ROUND_FREQ_TO_N_1000;
 
-	for (i = 0; i < pi->nb_freqs; i++) {
-		if (freq_conv == pi->freqs[i]) {
-			pi->curr_idx = i;
-			pi->f = f;
-			return 0;
+	/* Find the nearest frequency in the table.
+	 * With amd-pstate the CPU runs at continuously variable
+	 * frequencies so the current frequency will not exactly
+	 * match one of the synthesized frequency buckets.
+	 */
+	best_idx = 0;
+	best_diff = abs_diff(freq_conv, pi->freqs[0]);
+
+	for (i = 1; i < pi->nb_freqs; i++) {
+		diff = abs_diff(freq_conv, pi->freqs[i]);
+		if (diff < best_diff) {
+			best_diff = diff;
+			best_idx = i;
 		}
 	}
 
+	POWER_DEBUG_LOG("Freq %lu rounded to %lu matched bucket [%u] = %u "
+			"for lcore %u", freq, freq_conv, best_idx,
+			pi->freqs[best_idx], pi->lcore_id);
+
+	pi->curr_idx = best_idx;
+	pi->f = f;
+	return 0;
+
 err:
 	if (f != NULL)
 		fclose(f);
-- 
2.53.0

RE: [PATCH] power/amd_pstate: fix frequency matching for continuous scaling

[Tummala, Sivaprasad]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Sunday, March 29, 2026 1:04 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; stable@dpdk.org;
> Anatoly Burakov <anatoly.burakov@intel.com>; Tummala, Sivaprasad
> <Sivaprasad.Tummala@amd.com>
> Subject: [PATCH] power/amd_pstate: fix frequency matching for continuous scaling
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> The power_init_for_setting_freq() function fails on systems using the amd-pstate-
> epp driver because the current CPU frequency read from scaling_setspeed does
> not exactly match any of the synthesized frequency buckets. Unlike acpi_cpufreq
> which provides a discrete list of frequencies, amd-pstate operates with continuously
> variable frequencies, so an exact match will rarely succeed.
>
> For example, on a Ryzen 9 7945HX the sysfs file reports 2797172 which rounds to
> 2797000, but this value does not appear in the generated frequency table.
>
> Replace the exact match lookup with a nearest-frequency search.
>
> Also fix several issues in the same function:
>  - strtoul() was called with NULL endptr, making parse failures
>    undetectable
>  - errno was not checked, so stale errors could cause false positives
>  - freq was declared as uint32_t, truncating the unsigned long
>    return value of strtoul() on LP64 platforms
>  - no error was logged when frequency matching failed
>
> Bugzilla ID: 1915
> Fixes: 1ed04d33cf19 ("power: support amd-pstate cpufreq driver")
> Cc: stable@dpdk.org
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/power/amd_pstate/amd_pstate_cpufreq.c | 56 ++++++++++++++-----
>  1 file changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
> b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
> index 95495bff7d..14625ffd73 100644
> --- a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
> +++ b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include <stdlib.h>
> +#include <errno.h>
>
>  #include <rte_memcpy.h>
>  #include <rte_stdatomic.h>
> @@ -291,15 +292,21 @@ power_get_available_freqs(struct
> amd_pstate_power_info *pi)
>         return ret;
>  }
>
> -/**
> - * It is to fopen the sys file for the future setting the lcore frequency.
> - */
> +static inline unsigned long
> +abs_diff(unsigned long a, unsigned long b) {
> +       return (a > b) ? a - b : b - a;
> +}
> +
>  static int
>  power_init_for_setting_freq(struct amd_pstate_power_info *pi)  {
> -       FILE *f = NULL;
> +       FILE *f;
>         char buf[BUFSIZ];
> -       uint32_t i, freq;
> +       char *endptr;
> +       unsigned long freq, freq_conv;
> +       unsigned long best_diff, diff;
> +       uint32_t i, best_idx;
>         int ret;
>
>         open_core_sysfs_file(&f, "rw+", POWER_SYSFILE_SETSPEED, pi-
> >lcore_id); @@ -308,7 +315,6 @@ power_init_for_setting_freq(struct
> amd_pstate_power_info *pi)
>                                 POWER_SYSFILE_SETSPEED);
>                 goto err;
>         }
> -
>         ret = read_core_sysfs_s(f, buf, sizeof(buf));
>         if (ret < 0) {
>                 POWER_LOG(ERR, "Failed to read %s", @@ -316,25 +322,45 @@
> power_init_for_setting_freq(struct amd_pstate_power_info *pi)
>                 goto err;
>         }
>
> -       freq = strtoul(buf, NULL, POWER_CONVERT_TO_DECIMAL);
> +       errno = 0;
> +       freq = strtoul(buf, &endptr, POWER_CONVERT_TO_DECIMAL);
> +       if (errno != 0 || endptr == buf || freq == 0) {
> +               POWER_LOG(ERR, "Failed to parse frequency '%s' for lcore %u",
> +                               buf, pi->lcore_id);
> +               goto err;
> +       }
>
>         /* convert the frequency to nearest 1000 value
>          * Ex: if freq=1396789 then freq_conv=1397000
>          * Ex: if freq=800030 then freq_conv=800000
>          */
> -       unsigned int freq_conv = 0;
> -       freq_conv = (freq + FREQ_ROUNDING_DELTA)
> -                               / ROUND_FREQ_TO_N_1000;
> +       freq_conv = (freq + FREQ_ROUNDING_DELTA) /
> ROUND_FREQ_TO_N_1000;
>         freq_conv = freq_conv * ROUND_FREQ_TO_N_1000;
>
> -       for (i = 0; i < pi->nb_freqs; i++) {
> -               if (freq_conv == pi->freqs[i]) {
> -                       pi->curr_idx = i;
> -                       pi->f = f;
> -                       return 0;
> +       /* Find the nearest frequency in the table.
> +        * With amd-pstate the CPU runs at continuously variable
> +        * frequencies so the current frequency will not exactly
> +        * match one of the synthesized frequency buckets.
> +        */
> +       best_idx = 0;
> +       best_diff = abs_diff(freq_conv, pi->freqs[0]);
> +
> +       for (i = 1; i < pi->nb_freqs; i++) {
> +               diff = abs_diff(freq_conv, pi->freqs[i]);
> +               if (diff < best_diff) {
> +                       best_diff = diff;
> +                       best_idx = i;
>                 }
>         }
>
> +       POWER_DEBUG_LOG("Freq %lu rounded to %lu matched bucket [%u] =
> %u "
> +                       "for lcore %u", freq, freq_conv, best_idx,
> +                       pi->freqs[best_idx], pi->lcore_id);
> +
> +       pi->curr_idx = best_idx;
> +       pi->f = f;
> +       return 0;
> +
>  err:
>         if (f != NULL)
>                 fclose(f);
> --
> 2.53.0
LGTM
Acked-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>