[PATCH 0/6] cpupower: Add various feature control support for amd_pstate

[PATCH 0/6] cpupower: Add various feature control support for amd_pstate

[Wyes Karny]

In this series, three feature control support for amd_pstate has been
added.

 1. EPP: User can control Energy Performance Preference (EPP) value,
which conveys user's performance vs energy preference to platform. This
feature is present on amd_pstate and intel_pstate active mode.

 2. amd_pstate mode switch: amd_pstate has various working modes such as
'active', 'passive' and 'guided', which can be changed dynamically. 

 3. Turbo boost: Turbo boost feature is present in amd_pstate as well as
acpi-cpufreq. By default this is enabled, but user can control this
feature dynamically.

These features can be controlled with `cpupower set` command. The
options are respectively --epp, --amd-pstate-mode, --turbo-boost

Also, fix amd-pstate-epp driver name and make cpupower recognise
amd-pstate-epp driver.

Wyes Karny (6):
  amd-pstate: Make amd-pstate epp driver name hyphenated
  cpupower: Recognise amd-pstate active mode driver
  cpupower: Add is_sysfs_present API
  cpupower: Add EPP value change support
  cpupower: Add support for amd_pstate mode change
  cpupower: Add turbo-boost support in cpupower

 drivers/cpufreq/amd-pstate.c                 |  2 +-
 tools/power/cpupower/lib/cpupower.c          |  7 +++
 tools/power/cpupower/lib/cpupower_intern.h   |  1 +
 tools/power/cpupower/utils/cpupower-set.c    | 65 +++++++++++++++++++-
 tools/power/cpupower/utils/helpers/helpers.h | 11 ++++
 tools/power/cpupower/utils/helpers/misc.c    | 57 ++++++++++++++++-
 6 files changed, 139 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Wyes Karny]

amd-pstate passive mode driver is hyphenated. So make amd-pstate active
mode driver consistent with that rename "amd_pstate_epp" to
"amd-pstate-epp".

Cc: stable@vger.kernel.org
Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index ddd346a239e0..a5764946434c 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1356,7 +1356,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
 	.online		= amd_pstate_epp_cpu_online,
 	.suspend	= amd_pstate_epp_suspend,
 	.resume		= amd_pstate_epp_resume,
-	.name		= "amd_pstate_epp",
+	.name		= "amd-pstate-epp",
 	.attr		= amd_pstate_epp_attr,
 };
 
-- 
2.34.1

[PATCH 2/6] cpupower: Recognise amd-pstate active mode driver

[Wyes Karny]

amd-pstate active mode driver name is "amd-pstate-epp".  Add this to the
string matching condition to recognise amd-pstate active mode driver.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 tools/power/cpupower/utils/helpers/misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
index 9547b29254a7..21f653cd472c 100644
--- a/tools/power/cpupower/utils/helpers/misc.c
+++ b/tools/power/cpupower/utils/helpers/misc.c
@@ -95,7 +95,7 @@ bool cpupower_amd_pstate_enabled(void)
 	if (!driver)
 		return ret;
 
-	if (!strcmp(driver, "amd-pstate"))
+	if (!strcmp(driver, "amd-pstate") || !strcmp(driver, "amd-pstate-epp"))
 		ret = true;
 
 	cpufreq_put_driver(driver);
-- 
2.34.1

[PATCH 3/6] cpupower: Add is_sysfs_present API

[Wyes Karny]

Add is_sysfs_present API to check whether the sysfs file is present or not.

Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 tools/power/cpupower/lib/cpupower.c        | 7 +++++++
 tools/power/cpupower/lib/cpupower_intern.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/tools/power/cpupower/lib/cpupower.c b/tools/power/cpupower/lib/cpupower.c
index 3f7d0c0c5067..7a2ef691b20e 100644
--- a/tools/power/cpupower/lib/cpupower.c
+++ b/tools/power/cpupower/lib/cpupower.c
@@ -14,6 +14,13 @@
 #include "cpupower.h"
 #include "cpupower_intern.h"
 
+int is_valid_path(const char *path)
+{
+	if (access(path, F_OK) == -1)
+		return 0;
+	return 1;
+}
+
 unsigned int cpupower_read_sysfs(const char *path, char *buf, size_t buflen)
 {
 	ssize_t numread;
diff --git a/tools/power/cpupower/lib/cpupower_intern.h b/tools/power/cpupower/lib/cpupower_intern.h
index ac1112b956ec..5fdb8620d41b 100644
--- a/tools/power/cpupower/lib/cpupower_intern.h
+++ b/tools/power/cpupower/lib/cpupower_intern.h
@@ -7,5 +7,6 @@
 
 #define SYSFS_PATH_MAX 255
 
+int is_valid_path(const char *path);
 unsigned int cpupower_read_sysfs(const char *path, char *buf, size_t buflen);
 unsigned int cpupower_write_sysfs(const char *path, char *buf, size_t buflen);
-- 
2.34.1

[PATCH 4/6] cpupower: Add EPP value change support

[Wyes Karny]

amd_pstate and intel_pstate active mode drivers support energy
performance preference feature. Through this user can convey it's
energy/performance preference to platform. Add this value change
capability to cpupower.

To change the EPP value use below command:
cpupower set --epp performance

Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 tools/power/cpupower/utils/cpupower-set.c    | 23 +++++++++++++++++++-
 tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
 tools/power/cpupower/utils/helpers/misc.c    | 19 ++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
index 180d5ba877e6..a789b123dbd4 100644
--- a/tools/power/cpupower/utils/cpupower-set.c
+++ b/tools/power/cpupower/utils/cpupower-set.c
@@ -18,6 +18,7 @@
 
 static struct option set_opts[] = {
 	{"perf-bias", required_argument, NULL, 'b'},
+	{"epp", required_argument, NULL, 'e'},
 	{ },
 };
 
@@ -37,11 +38,13 @@ int cmd_set(int argc, char **argv)
 	union {
 		struct {
 			int perf_bias:1;
+			int epp:1;
 		};
 		int params;
 	} params;
 	int perf_bias = 0;
 	int ret = 0;
+	char epp[30];
 
 	ret = uname(&uts);
 	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
@@ -55,7 +58,7 @@ int cmd_set(int argc, char **argv)
 
 	params.params = 0;
 	/* parameter parsing */
-	while ((ret = getopt_long(argc, argv, "b:",
+	while ((ret = getopt_long(argc, argv, "b:e:",
 						set_opts, NULL)) != -1) {
 		switch (ret) {
 		case 'b':
@@ -69,6 +72,15 @@ int cmd_set(int argc, char **argv)
 			}
 			params.perf_bias = 1;
 			break;
+		case 'e':
+			if (params.epp)
+				print_wrong_arg_exit();
+			if (sscanf(optarg, "%29s", epp) != 1) {
+				print_wrong_arg_exit();
+				return -EINVAL;
+			}
+			params.epp = 1;
+			break;
 		default:
 			print_wrong_arg_exit();
 		}
@@ -102,6 +114,15 @@ int cmd_set(int argc, char **argv)
 				break;
 			}
 		}
+
+		if (params.epp) {
+			ret = cpupower_set_epp(cpu, epp);
+			if (ret) {
+				fprintf(stderr,
+					"Error setting epp value on CPU %d\n", cpu);
+				break;
+			}
+		}
 	}
 	return ret;
 }
diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
index 96e4bede078b..5d998de2d291 100644
--- a/tools/power/cpupower/utils/helpers/helpers.h
+++ b/tools/power/cpupower/utils/helpers/helpers.h
@@ -116,6 +116,8 @@ extern int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val);
 extern int cpupower_intel_get_perf_bias(unsigned int cpu);
 extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
 
+extern int cpupower_set_epp(unsigned int cpu, char *epp);
+
 /* Read/Write msr ****************************/
 
 /* PCI stuff ****************************/
@@ -173,6 +175,9 @@ static inline int cpupower_intel_get_perf_bias(unsigned int cpu)
 static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
 { return 0; };
 
+static inline int cpupower_set_epp(unsigned int cpu, char *epp)
+{ return -1; };
+
 /* Read/Write msr ****************************/
 
 static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
index 21f653cd472c..63c3f26ef874 100644
--- a/tools/power/cpupower/utils/helpers/misc.c
+++ b/tools/power/cpupower/utils/helpers/misc.c
@@ -87,6 +87,25 @@ int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val)
 	return 0;
 }
 
+int cpupower_set_epp(unsigned int cpu, char *epp)
+{
+	char path[SYSFS_PATH_MAX];
+	char linebuf[30] = {};
+
+	snprintf(path, sizeof(path),
+		PATH_TO_CPU "cpu%u/cpufreq/energy_performance_preference", cpu);
+
+	if (!is_valid_path(path))
+		return -1;
+
+	snprintf(linebuf, sizeof(linebuf), "%s", epp);
+
+	if (cpupower_write_sysfs(path, linebuf, 30) <= 0)
+		return -1;
+
+	return 0;
+}
+
 bool cpupower_amd_pstate_enabled(void)
 {
 	char *driver = cpufreq_get_driver(0);
-- 
2.34.1

[PATCH 5/6] cpupower: Add support for amd_pstate mode change

[Wyes Karny]

amd_pstate supports changing of its mode dynamically via `status` sysfs
file. Add the same capability in cpupower. To change the mode to active
mode use below command:

cpupower set --amd-pstate-mode active

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 tools/power/cpupower/utils/cpupower-set.c    | 24 ++++++++++++++++++--
 tools/power/cpupower/utils/helpers/helpers.h |  3 +++
 tools/power/cpupower/utils/helpers/misc.c    | 18 +++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
index a789b123dbd4..c2ba69b7ea54 100644
--- a/tools/power/cpupower/utils/cpupower-set.c
+++ b/tools/power/cpupower/utils/cpupower-set.c
@@ -19,6 +19,7 @@
 static struct option set_opts[] = {
 	{"perf-bias", required_argument, NULL, 'b'},
 	{"epp", required_argument, NULL, 'e'},
+	{"amd-pstate-mode", required_argument, NULL, 'm'},
 	{ },
 };
 
@@ -39,12 +40,13 @@ int cmd_set(int argc, char **argv)
 		struct {
 			int perf_bias:1;
 			int epp:1;
+			int mode:1;
 		};
 		int params;
 	} params;
 	int perf_bias = 0;
 	int ret = 0;
-	char epp[30];
+	char epp[30], mode[20];
 
 	ret = uname(&uts);
 	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
@@ -58,7 +60,7 @@ int cmd_set(int argc, char **argv)
 
 	params.params = 0;
 	/* parameter parsing */
-	while ((ret = getopt_long(argc, argv, "b:e:",
+	while ((ret = getopt_long(argc, argv, "b:e:m:",
 						set_opts, NULL)) != -1) {
 		switch (ret) {
 		case 'b':
@@ -81,6 +83,17 @@ int cmd_set(int argc, char **argv)
 			}
 			params.epp = 1;
 			break;
+		case 'm':
+			if (cpupower_cpu_info.vendor != X86_VENDOR_AMD)
+				print_wrong_arg_exit();
+			if (params.mode)
+				print_wrong_arg_exit();
+			if (sscanf(optarg, "%19s", mode) != 1) {
+				print_wrong_arg_exit();
+				return -EINVAL;
+			}
+			params.mode = 1;
+			break;
 		default:
 			print_wrong_arg_exit();
 		}
@@ -89,6 +102,12 @@ int cmd_set(int argc, char **argv)
 	if (!params.params)
 		print_wrong_arg_exit();
 
+	if (params.mode) {
+		ret = cpupower_set_amd_pstate_mode(mode);
+		if (ret)
+			fprintf(stderr, "Error setting mode\n");
+	}
+
 	/* Default is: set all CPUs */
 	if (bitmask_isallclear(cpus_chosen))
 		bitmask_setall(cpus_chosen);
@@ -123,6 +142,7 @@ int cmd_set(int argc, char **argv)
 				break;
 			}
 		}
+
 	}
 	return ret;
 }
diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
index 5d998de2d291..d35596631eef 100644
--- a/tools/power/cpupower/utils/helpers/helpers.h
+++ b/tools/power/cpupower/utils/helpers/helpers.h
@@ -117,6 +117,7 @@ extern int cpupower_intel_get_perf_bias(unsigned int cpu);
 extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
 
 extern int cpupower_set_epp(unsigned int cpu, char *epp);
+extern int cpupower_set_amd_pstate_mode(char *mode);
 
 /* Read/Write msr ****************************/
 
@@ -177,6 +178,8 @@ static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
 
 static inline int cpupower_set_epp(unsigned int cpu, char *epp)
 { return -1; };
+static inline int cpupower_set_amd_pstate_mode(char *mode)
+{ return -1; };
 
 /* Read/Write msr ****************************/
 
diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
index 63c3f26ef874..9df9af8a969e 100644
--- a/tools/power/cpupower/utils/helpers/misc.c
+++ b/tools/power/cpupower/utils/helpers/misc.c
@@ -106,6 +106,24 @@ int cpupower_set_epp(unsigned int cpu, char *epp)
 	return 0;
 }
 
+int cpupower_set_amd_pstate_mode(char *mode)
+{
+	char path[SYSFS_PATH_MAX];
+	char linebuf[20] = {};
+
+	snprintf(path, sizeof(path), PATH_TO_CPU "amd_pstate/status");
+
+	if (!is_valid_path(path))
+		return -1;
+
+	snprintf(linebuf, sizeof(linebuf), "%s\n", mode);
+
+	if (cpupower_write_sysfs(path, linebuf, 20) <= 0)
+		return -1;
+
+	return 0;
+}
+
 bool cpupower_amd_pstate_enabled(void)
 {
 	char *driver = cpufreq_get_driver(0);
-- 
2.34.1

[PATCH 6/6] cpupower: Add turbo-boost support in cpupower

[Wyes Karny]

If boost sysfs (/sys/devices/system/cpu/cpufreq/boost) file is present
turbo-boost is feature is supported in the hardware. By default this
feature should be enabled. But to disable/enable it write to the sysfs
file. Use the same to control this feature via cpupower.

To enable:
cpupower set --turbo-boost 1

To disable:
cpupower set --turbo-boost 0

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
 tools/power/cpupower/utils/cpupower-set.c    | 22 +++++++++++++++++++-
 tools/power/cpupower/utils/helpers/helpers.h |  3 +++
 tools/power/cpupower/utils/helpers/misc.c    | 18 ++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
index c2ba69b7ea54..0677b58374ab 100644
--- a/tools/power/cpupower/utils/cpupower-set.c
+++ b/tools/power/cpupower/utils/cpupower-set.c
@@ -20,6 +20,7 @@ static struct option set_opts[] = {
 	{"perf-bias", required_argument, NULL, 'b'},
 	{"epp", required_argument, NULL, 'e'},
 	{"amd-pstate-mode", required_argument, NULL, 'm'},
+	{"turbo-boost", required_argument, NULL, 't'},
 	{ },
 };
 
@@ -41,10 +42,11 @@ int cmd_set(int argc, char **argv)
 			int perf_bias:1;
 			int epp:1;
 			int mode:1;
+			int turbo_boost:1;
 		};
 		int params;
 	} params;
-	int perf_bias = 0;
+	int perf_bias = 0, turbo_boost = 1;
 	int ret = 0;
 	char epp[30], mode[20];
 
@@ -94,6 +96,18 @@ int cmd_set(int argc, char **argv)
 			}
 			params.mode = 1;
 			break;
+		case 't':
+			if (params.turbo_boost)
+				print_wrong_arg_exit();
+			turbo_boost = atoi(optarg);
+			if (turbo_boost < 0 || turbo_boost > 1) {
+				printf("--turbo-boost param out of range [0-1]\n");
+				print_wrong_arg_exit();
+			}
+			params.turbo_boost = 1;
+			break;
+
+
 		default:
 			print_wrong_arg_exit();
 		}
@@ -108,6 +122,12 @@ int cmd_set(int argc, char **argv)
 			fprintf(stderr, "Error setting mode\n");
 	}
 
+	if (params.turbo_boost) {
+		ret = cpupower_set_turbo_boost(turbo_boost);
+		if (ret)
+			fprintf(stderr, "Error setting turbo-boost\n");
+	}
+
 	/* Default is: set all CPUs */
 	if (bitmask_isallclear(cpus_chosen))
 		bitmask_setall(cpus_chosen);
diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
index d35596631eef..95749b8ee475 100644
--- a/tools/power/cpupower/utils/helpers/helpers.h
+++ b/tools/power/cpupower/utils/helpers/helpers.h
@@ -118,6 +118,7 @@ extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
 
 extern int cpupower_set_epp(unsigned int cpu, char *epp);
 extern int cpupower_set_amd_pstate_mode(char *mode);
+extern int cpupower_set_turbo_boost(int turbo_boost);
 
 /* Read/Write msr ****************************/
 
@@ -180,6 +181,8 @@ static inline int cpupower_set_epp(unsigned int cpu, char *epp)
 { return -1; };
 static inline int cpupower_set_amd_pstate_mode(char *mode)
 { return -1; };
+static inline int cpupower_set_turbo_boost(int turbo_boost)
+{ return -1; };
 
 /* Read/Write msr ****************************/
 
diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
index 9df9af8a969e..fc822a0e6b7b 100644
--- a/tools/power/cpupower/utils/helpers/misc.c
+++ b/tools/power/cpupower/utils/helpers/misc.c
@@ -124,6 +124,24 @@ int cpupower_set_amd_pstate_mode(char *mode)
 	return 0;
 }
 
+int cpupower_set_turbo_boost(int turbo_boost)
+{
+	char path[SYSFS_PATH_MAX];
+	char linebuf[2] = {};
+
+	snprintf(path, sizeof(path), PATH_TO_CPU "cpufreq/boost");
+
+	if (!is_valid_path(path))
+		return -1;
+
+	snprintf(linebuf, sizeof(linebuf), "%d", turbo_boost);
+
+	if (cpupower_write_sysfs(path, linebuf, 2) <= 0)
+		return -1;
+
+	return 0;
+}
+
 bool cpupower_amd_pstate_enabled(void)
 {
 	char *driver = cpufreq_get_driver(0);
-- 
2.34.1

[PATCH v1.1 3/6] cpupower: Add is_valid_path API

[Wyes Karny]

Add is_valid_path API to check whether the sysfs file is present or not.

Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
v1 -> v1.1
- s/is_sysfs_present/is_valid_path

 tools/power/cpupower/lib/cpupower.c        | 7 +++++++
 tools/power/cpupower/lib/cpupower_intern.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/tools/power/cpupower/lib/cpupower.c b/tools/power/cpupower/lib/cpupower.c
index 3f7d0c0c5067..7a2ef691b20e 100644
--- a/tools/power/cpupower/lib/cpupower.c
+++ b/tools/power/cpupower/lib/cpupower.c
@@ -14,6 +14,13 @@
 #include "cpupower.h"
 #include "cpupower_intern.h"
 
+int is_valid_path(const char *path)
+{
+	if (access(path, F_OK) == -1)
+		return 0;
+	return 1;
+}
+
 unsigned int cpupower_read_sysfs(const char *path, char *buf, size_t buflen)
 {
 	ssize_t numread;
diff --git a/tools/power/cpupower/lib/cpupower_intern.h b/tools/power/cpupower/lib/cpupower_intern.h
index ac1112b956ec..5fdb8620d41b 100644
--- a/tools/power/cpupower/lib/cpupower_intern.h
+++ b/tools/power/cpupower/lib/cpupower_intern.h
@@ -7,5 +7,6 @@
 
 #define SYSFS_PATH_MAX 255
 
+int is_valid_path(const char *path);
 unsigned int cpupower_read_sysfs(const char *path, char *buf, size_t buflen);
 unsigned int cpupower_write_sysfs(const char *path, char *buf, size_t buflen);
-- 
2.34.1

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Rafael J. Wysocki]

On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <wyes.karny@amd.com> wrote:
>
> amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> mode driver consistent with that rename "amd_pstate_epp" to
> "amd-pstate-epp".
>
> Cc: stable@vger.kernel.org
> Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>

How much does the rest of the series depend on this patch?

I can apply it right away and push it out to a forward-only branch if
that helps.

> ---
>  drivers/cpufreq/amd-pstate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index ddd346a239e0..a5764946434c 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1356,7 +1356,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
>         .online         = amd_pstate_epp_cpu_online,
>         .suspend        = amd_pstate_epp_suspend,
>         .resume         = amd_pstate_epp_resume,
> -       .name           = "amd_pstate_epp",
> +       .name           = "amd-pstate-epp",
>         .attr           = amd_pstate_epp_attr,
>  };
>
> --
> 2.34.1
>

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Wyes Karny]

Hi Rafael,

On 15 Jun 19:31, Rafael J. Wysocki wrote:
> On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <wyes.karny@amd.com> wrote:
> >
> > amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> > mode driver consistent with that rename "amd_pstate_epp" to
> > "amd-pstate-epp".
> >
> > Cc: stable@vger.kernel.org
> > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> 
> How much does the rest of the series depend on this patch?

The rest of the series is independent of this patch.

> 
> I can apply it right away and push it out to a forward-only branch if
> that helps.

Sure, that helps!

Thanks & Regards,
Wyes


> 
> > ---
> >  drivers/cpufreq/amd-pstate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index ddd346a239e0..a5764946434c 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -1356,7 +1356,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> >         .online         = amd_pstate_epp_cpu_online,
> >         .suspend        = amd_pstate_epp_suspend,
> >         .resume         = amd_pstate_epp_resume,
> > -       .name           = "amd_pstate_epp",
> > +       .name           = "amd-pstate-epp",
> >         .attr           = amd_pstate_epp_attr,
> >  };
> >
> > --
> > 2.34.1
> >

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Rafael J. Wysocki]

On Thu, Jun 15, 2023 at 7:55 PM Wyes Karny <wyes.karny@amd.com> wrote:
>
> Hi Rafael,
>
> On 15 Jun 19:31, Rafael J. Wysocki wrote:
> > On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <wyes.karny@amd.com> wrote:
> > >
> > > amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> > > mode driver consistent with that rename "amd_pstate_epp" to
> > > "amd-pstate-epp".
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> >
> > How much does the rest of the series depend on this patch?
>
> The rest of the series is independent of this patch.

So it should have been posted separately as an individual fix.

Please resend the rest of the series without it to avoid confusion and
I'll apply it for 6.5 tomorrow.

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Wyes Karny]

On 15 Jun 20:30, Rafael J. Wysocki wrote:
> On Thu, Jun 15, 2023 at 7:55 PM Wyes Karny <wyes.karny@amd.com> wrote:
> >
> > Hi Rafael,
> >
> > On 15 Jun 19:31, Rafael J. Wysocki wrote:
> > > On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <wyes.karny@amd.com> wrote:
> > > >
> > > > amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> > > > mode driver consistent with that rename "amd_pstate_epp" to
> > > > "amd-pstate-epp".
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > >
> > > How much does the rest of the series depend on this patch?
> >
> > The rest of the series is independent of this patch.
> 
> So it should have been posted separately as an individual fix.
> 
> Please resend the rest of the series without it to avoid confusion and
> I'll apply it for 6.5 tomorrow.

Sure, I'll send rest of the series separately.

Thanks,
Wyes

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Huang Rui]

On Mon, Jun 12, 2023 at 07:36:10PM +0800, Karny, Wyes wrote:
> amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> mode driver consistent with that rename "amd_pstate_epp" to
> "amd-pstate-epp".
> 
> Cc: stable@vger.kernel.org
> Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>

Acked-by: Huang Rui <ray.huang@amd.com>

And yes, we should seprate it from cpupower as Rafael mentioned. cpupower
tool may go to another repo.

Thanks,
Ray

> ---
>  drivers/cpufreq/amd-pstate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index ddd346a239e0..a5764946434c 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1356,7 +1356,7 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
>  	.online		= amd_pstate_epp_cpu_online,
>  	.suspend	= amd_pstate_epp_suspend,
>  	.resume		= amd_pstate_epp_resume,
> -	.name		= "amd_pstate_epp",
> +	.name		= "amd-pstate-epp",
>  	.attr		= amd_pstate_epp_attr,
>  };
>  
> -- 
> 2.34.1
> 

Re: [PATCH 4/6] cpupower: Add EPP value change support

[Huang Rui]

On Mon, Jun 12, 2023 at 07:36:13PM +0800, Karny, Wyes wrote:
> amd_pstate and intel_pstate active mode drivers support energy
> performance preference feature. Through this user can convey it's
> energy/performance preference to platform. Add this value change
> capability to cpupower.
> 
> To change the EPP value use below command:
> cpupower set --epp performance
> 
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> ---
>  tools/power/cpupower/utils/cpupower-set.c    | 23 +++++++++++++++++++-
>  tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
>  tools/power/cpupower/utils/helpers/misc.c    | 19 ++++++++++++++++
>  3 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> index 180d5ba877e6..a789b123dbd4 100644
> --- a/tools/power/cpupower/utils/cpupower-set.c
> +++ b/tools/power/cpupower/utils/cpupower-set.c
> @@ -18,6 +18,7 @@
>  
>  static struct option set_opts[] = {
>  	{"perf-bias", required_argument, NULL, 'b'},
> +	{"epp", required_argument, NULL, 'e'},

How about re-using the "perf-bias", I think it should be the simliar
function with Intel's processor?

Thanks,
Ray

>  	{ },
>  };
>  
> @@ -37,11 +38,13 @@ int cmd_set(int argc, char **argv)
>  	union {
>  		struct {
>  			int perf_bias:1;
> +			int epp:1;
>  		};
>  		int params;
>  	} params;
>  	int perf_bias = 0;
>  	int ret = 0;
> +	char epp[30];
>  
>  	ret = uname(&uts);
>  	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
> @@ -55,7 +58,7 @@ int cmd_set(int argc, char **argv)
>  
>  	params.params = 0;
>  	/* parameter parsing */
> -	while ((ret = getopt_long(argc, argv, "b:",
> +	while ((ret = getopt_long(argc, argv, "b:e:",
>  						set_opts, NULL)) != -1) {
>  		switch (ret) {
>  		case 'b':
> @@ -69,6 +72,15 @@ int cmd_set(int argc, char **argv)
>  			}
>  			params.perf_bias = 1;
>  			break;
> +		case 'e':
> +			if (params.epp)
> +				print_wrong_arg_exit();
> +			if (sscanf(optarg, "%29s", epp) != 1) {
> +				print_wrong_arg_exit();
> +				return -EINVAL;
> +			}
> +			params.epp = 1;
> +			break;
>  		default:
>  			print_wrong_arg_exit();
>  		}
> @@ -102,6 +114,15 @@ int cmd_set(int argc, char **argv)
>  				break;
>  			}
>  		}
> +
> +		if (params.epp) {
> +			ret = cpupower_set_epp(cpu, epp);
> +			if (ret) {
> +				fprintf(stderr,
> +					"Error setting epp value on CPU %d\n", cpu);
> +				break;
> +			}
> +		}
>  	}
>  	return ret;
>  }
> diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> index 96e4bede078b..5d998de2d291 100644
> --- a/tools/power/cpupower/utils/helpers/helpers.h
> +++ b/tools/power/cpupower/utils/helpers/helpers.h
> @@ -116,6 +116,8 @@ extern int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val);
>  extern int cpupower_intel_get_perf_bias(unsigned int cpu);
>  extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
>  
> +extern int cpupower_set_epp(unsigned int cpu, char *epp);
> +
>  /* Read/Write msr ****************************/
>  
>  /* PCI stuff ****************************/
> @@ -173,6 +175,9 @@ static inline int cpupower_intel_get_perf_bias(unsigned int cpu)
>  static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
>  { return 0; };
>  
> +static inline int cpupower_set_epp(unsigned int cpu, char *epp)
> +{ return -1; };
> +
>  /* Read/Write msr ****************************/
>  
>  static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
> diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> index 21f653cd472c..63c3f26ef874 100644
> --- a/tools/power/cpupower/utils/helpers/misc.c
> +++ b/tools/power/cpupower/utils/helpers/misc.c
> @@ -87,6 +87,25 @@ int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val)
>  	return 0;
>  }
>  
> +int cpupower_set_epp(unsigned int cpu, char *epp)
> +{
> +	char path[SYSFS_PATH_MAX];
> +	char linebuf[30] = {};
> +
> +	snprintf(path, sizeof(path),
> +		PATH_TO_CPU "cpu%u/cpufreq/energy_performance_preference", cpu);
> +
> +	if (!is_valid_path(path))
> +		return -1;
> +
> +	snprintf(linebuf, sizeof(linebuf), "%s", epp);
> +
> +	if (cpupower_write_sysfs(path, linebuf, 30) <= 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  bool cpupower_amd_pstate_enabled(void)
>  {
>  	char *driver = cpufreq_get_driver(0);
> -- 
> 2.34.1
> 

Re: [PATCH 5/6] cpupower: Add support for amd_pstate mode change

[Huang Rui]

On Mon, Jun 12, 2023 at 07:36:14PM +0800, Karny, Wyes wrote:
> amd_pstate supports changing of its mode dynamically via `status` sysfs
> file. Add the same capability in cpupower. To change the mode to active
> mode use below command:
> 
> cpupower set --amd-pstate-mode active
> 
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>

Acked-by: Huang Rui <ray.huang@amd.com>

> ---
>  tools/power/cpupower/utils/cpupower-set.c    | 24 ++++++++++++++++++--
>  tools/power/cpupower/utils/helpers/helpers.h |  3 +++
>  tools/power/cpupower/utils/helpers/misc.c    | 18 +++++++++++++++
>  3 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> index a789b123dbd4..c2ba69b7ea54 100644
> --- a/tools/power/cpupower/utils/cpupower-set.c
> +++ b/tools/power/cpupower/utils/cpupower-set.c
> @@ -19,6 +19,7 @@
>  static struct option set_opts[] = {
>  	{"perf-bias", required_argument, NULL, 'b'},
>  	{"epp", required_argument, NULL, 'e'},
> +	{"amd-pstate-mode", required_argument, NULL, 'm'},
>  	{ },
>  };
>  
> @@ -39,12 +40,13 @@ int cmd_set(int argc, char **argv)
>  		struct {
>  			int perf_bias:1;
>  			int epp:1;
> +			int mode:1;
>  		};
>  		int params;
>  	} params;
>  	int perf_bias = 0;
>  	int ret = 0;
> -	char epp[30];
> +	char epp[30], mode[20];
>  
>  	ret = uname(&uts);
>  	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
> @@ -58,7 +60,7 @@ int cmd_set(int argc, char **argv)
>  
>  	params.params = 0;
>  	/* parameter parsing */
> -	while ((ret = getopt_long(argc, argv, "b:e:",
> +	while ((ret = getopt_long(argc, argv, "b:e:m:",
>  						set_opts, NULL)) != -1) {
>  		switch (ret) {
>  		case 'b':
> @@ -81,6 +83,17 @@ int cmd_set(int argc, char **argv)
>  			}
>  			params.epp = 1;
>  			break;
> +		case 'm':
> +			if (cpupower_cpu_info.vendor != X86_VENDOR_AMD)
> +				print_wrong_arg_exit();
> +			if (params.mode)
> +				print_wrong_arg_exit();
> +			if (sscanf(optarg, "%19s", mode) != 1) {
> +				print_wrong_arg_exit();
> +				return -EINVAL;
> +			}
> +			params.mode = 1;
> +			break;
>  		default:
>  			print_wrong_arg_exit();
>  		}
> @@ -89,6 +102,12 @@ int cmd_set(int argc, char **argv)
>  	if (!params.params)
>  		print_wrong_arg_exit();
>  
> +	if (params.mode) {
> +		ret = cpupower_set_amd_pstate_mode(mode);
> +		if (ret)
> +			fprintf(stderr, "Error setting mode\n");
> +	}
> +
>  	/* Default is: set all CPUs */
>  	if (bitmask_isallclear(cpus_chosen))
>  		bitmask_setall(cpus_chosen);
> @@ -123,6 +142,7 @@ int cmd_set(int argc, char **argv)
>  				break;
>  			}
>  		}
> +
>  	}
>  	return ret;
>  }
> diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> index 5d998de2d291..d35596631eef 100644
> --- a/tools/power/cpupower/utils/helpers/helpers.h
> +++ b/tools/power/cpupower/utils/helpers/helpers.h
> @@ -117,6 +117,7 @@ extern int cpupower_intel_get_perf_bias(unsigned int cpu);
>  extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
>  
>  extern int cpupower_set_epp(unsigned int cpu, char *epp);
> +extern int cpupower_set_amd_pstate_mode(char *mode);
>  
>  /* Read/Write msr ****************************/
>  
> @@ -177,6 +178,8 @@ static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
>  
>  static inline int cpupower_set_epp(unsigned int cpu, char *epp)
>  { return -1; };
> +static inline int cpupower_set_amd_pstate_mode(char *mode)
> +{ return -1; };
>  
>  /* Read/Write msr ****************************/
>  
> diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> index 63c3f26ef874..9df9af8a969e 100644
> --- a/tools/power/cpupower/utils/helpers/misc.c
> +++ b/tools/power/cpupower/utils/helpers/misc.c
> @@ -106,6 +106,24 @@ int cpupower_set_epp(unsigned int cpu, char *epp)
>  	return 0;
>  }
>  
> +int cpupower_set_amd_pstate_mode(char *mode)
> +{
> +	char path[SYSFS_PATH_MAX];
> +	char linebuf[20] = {};
> +
> +	snprintf(path, sizeof(path), PATH_TO_CPU "amd_pstate/status");
> +
> +	if (!is_valid_path(path))
> +		return -1;
> +
> +	snprintf(linebuf, sizeof(linebuf), "%s\n", mode);
> +
> +	if (cpupower_write_sysfs(path, linebuf, 20) <= 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  bool cpupower_amd_pstate_enabled(void)
>  {
>  	char *driver = cpufreq_get_driver(0);
> -- 
> 2.34.1
> 

RE: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Wyse,

> -----Original Message-----
> From: Karny, Wyes <Wyes.Karny@amd.com>
> Sent: Friday, June 16, 2023 3:05 AM
> To: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Huang, Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org;
> trenn@suse.com; shuah@kernel.org; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>; linux-
> pm@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name
> hyphenated
>
> On 15 Jun 20:30, Rafael J. Wysocki wrote:
> > On Thu, Jun 15, 2023 at 7:55 PM Wyes Karny <wyes.karny@amd.com>
> wrote:
> > >
> > > Hi Rafael,
> > >
> > > On 15 Jun 19:31, Rafael J. Wysocki wrote:
> > > > On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <wyes.karny@amd.com>
> wrote:
> > > > >
> > > > > amd-pstate passive mode driver is hyphenated. So make amd-pstate
> > > > > active mode driver consistent with that rename "amd_pstate_epp"
> > > > > to "amd-pstate-epp".
> > > > >
> > > > > Cc: stable@vger.kernel.org
> > > > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP
> > > > > support for the AMD processors")
> > > > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > > >
> > > > How much does the rest of the series depend on this patch?
> > >
> > > The rest of the series is independent of this patch.
> >
> > So it should have been posted separately as an individual fix.
> >
> > Please resend the rest of the series without it to avoid confusion and
> > I'll apply it for 6.5 tomorrow.
>
> Sure, I'll send rest of the series separately.
>
> Thanks,
> Wyes

LGTM,
Reviewed-by: Perry Yuan <Perry.Yuan@amd.com>

Re: [PATCH 6/6] cpupower: Add turbo-boost support in cpupower

[Huang Rui]

On Mon, Jun 12, 2023 at 07:36:15PM +0800, Karny, Wyes wrote:
> If boost sysfs (/sys/devices/system/cpu/cpufreq/boost) file is present
> turbo-boost is feature is supported in the hardware. By default this
> feature should be enabled. But to disable/enable it write to the sysfs
> file. Use the same to control this feature via cpupower.
> 
> To enable:
> cpupower set --turbo-boost 1
> 
> To disable:
> cpupower set --turbo-boost 0
> 
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>

Acked-by: Huang Rui <ray.huang@amd.com>

> ---
>  tools/power/cpupower/utils/cpupower-set.c    | 22 +++++++++++++++++++-
>  tools/power/cpupower/utils/helpers/helpers.h |  3 +++
>  tools/power/cpupower/utils/helpers/misc.c    | 18 ++++++++++++++++
>  3 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> index c2ba69b7ea54..0677b58374ab 100644
> --- a/tools/power/cpupower/utils/cpupower-set.c
> +++ b/tools/power/cpupower/utils/cpupower-set.c
> @@ -20,6 +20,7 @@ static struct option set_opts[] = {
>  	{"perf-bias", required_argument, NULL, 'b'},
>  	{"epp", required_argument, NULL, 'e'},
>  	{"amd-pstate-mode", required_argument, NULL, 'm'},
> +	{"turbo-boost", required_argument, NULL, 't'},
>  	{ },
>  };
>  
> @@ -41,10 +42,11 @@ int cmd_set(int argc, char **argv)
>  			int perf_bias:1;
>  			int epp:1;
>  			int mode:1;
> +			int turbo_boost:1;
>  		};
>  		int params;
>  	} params;
> -	int perf_bias = 0;
> +	int perf_bias = 0, turbo_boost = 1;
>  	int ret = 0;
>  	char epp[30], mode[20];
>  
> @@ -94,6 +96,18 @@ int cmd_set(int argc, char **argv)
>  			}
>  			params.mode = 1;
>  			break;
> +		case 't':
> +			if (params.turbo_boost)
> +				print_wrong_arg_exit();
> +			turbo_boost = atoi(optarg);
> +			if (turbo_boost < 0 || turbo_boost > 1) {
> +				printf("--turbo-boost param out of range [0-1]\n");
> +				print_wrong_arg_exit();
> +			}
> +			params.turbo_boost = 1;
> +			break;
> +
> +
>  		default:
>  			print_wrong_arg_exit();
>  		}
> @@ -108,6 +122,12 @@ int cmd_set(int argc, char **argv)
>  			fprintf(stderr, "Error setting mode\n");
>  	}
>  
> +	if (params.turbo_boost) {
> +		ret = cpupower_set_turbo_boost(turbo_boost);
> +		if (ret)
> +			fprintf(stderr, "Error setting turbo-boost\n");
> +	}
> +
>  	/* Default is: set all CPUs */
>  	if (bitmask_isallclear(cpus_chosen))
>  		bitmask_setall(cpus_chosen);
> diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> index d35596631eef..95749b8ee475 100644
> --- a/tools/power/cpupower/utils/helpers/helpers.h
> +++ b/tools/power/cpupower/utils/helpers/helpers.h
> @@ -118,6 +118,7 @@ extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
>  
>  extern int cpupower_set_epp(unsigned int cpu, char *epp);
>  extern int cpupower_set_amd_pstate_mode(char *mode);
> +extern int cpupower_set_turbo_boost(int turbo_boost);
>  
>  /* Read/Write msr ****************************/
>  
> @@ -180,6 +181,8 @@ static inline int cpupower_set_epp(unsigned int cpu, char *epp)
>  { return -1; };
>  static inline int cpupower_set_amd_pstate_mode(char *mode)
>  { return -1; };
> +static inline int cpupower_set_turbo_boost(int turbo_boost)
> +{ return -1; };
>  
>  /* Read/Write msr ****************************/
>  
> diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> index 9df9af8a969e..fc822a0e6b7b 100644
> --- a/tools/power/cpupower/utils/helpers/misc.c
> +++ b/tools/power/cpupower/utils/helpers/misc.c
> @@ -124,6 +124,24 @@ int cpupower_set_amd_pstate_mode(char *mode)
>  	return 0;
>  }
>  
> +int cpupower_set_turbo_boost(int turbo_boost)
> +{
> +	char path[SYSFS_PATH_MAX];
> +	char linebuf[2] = {};
> +
> +	snprintf(path, sizeof(path), PATH_TO_CPU "cpufreq/boost");
> +
> +	if (!is_valid_path(path))
> +		return -1;
> +
> +	snprintf(linebuf, sizeof(linebuf), "%d", turbo_boost);
> +
> +	if (cpupower_write_sysfs(path, linebuf, 2) <= 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  bool cpupower_amd_pstate_enabled(void)
>  {
>  	char *driver = cpufreq_get_driver(0);
> -- 
> 2.34.1
> 

Re: [PATCH 4/6] cpupower: Add EPP value change support

[Wyes Karny]

Hi Ray,

On 16 Jun 15:22, Huang Rui wrote:
> On Mon, Jun 12, 2023 at 07:36:13PM +0800, Karny, Wyes wrote:
> > amd_pstate and intel_pstate active mode drivers support energy
> > performance preference feature. Through this user can convey it's
> > energy/performance preference to platform. Add this value change
> > capability to cpupower.
> > 
> > To change the EPP value use below command:
> > cpupower set --epp performance
> > 
> > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > ---
> >  tools/power/cpupower/utils/cpupower-set.c    | 23 +++++++++++++++++++-
> >  tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
> >  tools/power/cpupower/utils/helpers/misc.c    | 19 ++++++++++++++++
> >  3 files changed, 46 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> > index 180d5ba877e6..a789b123dbd4 100644
> > --- a/tools/power/cpupower/utils/cpupower-set.c
> > +++ b/tools/power/cpupower/utils/cpupower-set.c
> > @@ -18,6 +18,7 @@
> >  
> >  static struct option set_opts[] = {
> >  	{"perf-bias", required_argument, NULL, 'b'},
> > +	{"epp", required_argument, NULL, 'e'},
> 
> How about re-using the "perf-bias", I think it should be the simliar
> function with Intel's processor?

AFAIU 'perf-bias' is not same as 'epp'. Perf-bias sysfs file expects
0-15 integer value, whereas epp sysfs file expects one of the string
form energy_performance_available_preferences file.

Thanks,
Wyes
> 
> Thanks,
> Ray
> 
> >  	{ },
> >  };
> >  
> > @@ -37,11 +38,13 @@ int cmd_set(int argc, char **argv)
> >  	union {
> >  		struct {
> >  			int perf_bias:1;
> > +			int epp:1;
> >  		};
> >  		int params;
> >  	} params;
> >  	int perf_bias = 0;
> >  	int ret = 0;
> > +	char epp[30];
> >  
> >  	ret = uname(&uts);
> >  	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
> > @@ -55,7 +58,7 @@ int cmd_set(int argc, char **argv)
> >  
> >  	params.params = 0;
> >  	/* parameter parsing */
> > -	while ((ret = getopt_long(argc, argv, "b:",
> > +	while ((ret = getopt_long(argc, argv, "b:e:",
> >  						set_opts, NULL)) != -1) {
> >  		switch (ret) {
> >  		case 'b':
> > @@ -69,6 +72,15 @@ int cmd_set(int argc, char **argv)
> >  			}
> >  			params.perf_bias = 1;
> >  			break;
> > +		case 'e':
> > +			if (params.epp)
> > +				print_wrong_arg_exit();
> > +			if (sscanf(optarg, "%29s", epp) != 1) {
> > +				print_wrong_arg_exit();
> > +				return -EINVAL;
> > +			}
> > +			params.epp = 1;
> > +			break;
> >  		default:
> >  			print_wrong_arg_exit();
> >  		}
> > @@ -102,6 +114,15 @@ int cmd_set(int argc, char **argv)
> >  				break;
> >  			}
> >  		}
> > +
> > +		if (params.epp) {
> > +			ret = cpupower_set_epp(cpu, epp);
> > +			if (ret) {
> > +				fprintf(stderr,
> > +					"Error setting epp value on CPU %d\n", cpu);
> > +				break;
> > +			}
> > +		}
> >  	}
> >  	return ret;
> >  }
> > diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> > index 96e4bede078b..5d998de2d291 100644
> > --- a/tools/power/cpupower/utils/helpers/helpers.h
> > +++ b/tools/power/cpupower/utils/helpers/helpers.h
> > @@ -116,6 +116,8 @@ extern int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val);
> >  extern int cpupower_intel_get_perf_bias(unsigned int cpu);
> >  extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
> >  
> > +extern int cpupower_set_epp(unsigned int cpu, char *epp);
> > +
> >  /* Read/Write msr ****************************/
> >  
> >  /* PCI stuff ****************************/
> > @@ -173,6 +175,9 @@ static inline int cpupower_intel_get_perf_bias(unsigned int cpu)
> >  static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
> >  { return 0; };
> >  
> > +static inline int cpupower_set_epp(unsigned int cpu, char *epp)
> > +{ return -1; };
> > +
> >  /* Read/Write msr ****************************/
> >  
> >  static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
> > diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> > index 21f653cd472c..63c3f26ef874 100644
> > --- a/tools/power/cpupower/utils/helpers/misc.c
> > +++ b/tools/power/cpupower/utils/helpers/misc.c
> > @@ -87,6 +87,25 @@ int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val)
> >  	return 0;
> >  }
> >  
> > +int cpupower_set_epp(unsigned int cpu, char *epp)
> > +{
> > +	char path[SYSFS_PATH_MAX];
> > +	char linebuf[30] = {};
> > +
> > +	snprintf(path, sizeof(path),
> > +		PATH_TO_CPU "cpu%u/cpufreq/energy_performance_preference", cpu);
> > +
> > +	if (!is_valid_path(path))
> > +		return -1;
> > +
> > +	snprintf(linebuf, sizeof(linebuf), "%s", epp);
> > +
> > +	if (cpupower_write_sysfs(path, linebuf, 30) <= 0)
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> >  bool cpupower_amd_pstate_enabled(void)
> >  {
> >  	char *driver = cpufreq_get_driver(0);
> > -- 
> > 2.34.1
> > 

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Rafael J. Wysocki]

On Fri, Jun 16, 2023 at 9:08 AM Huang Rui <ray.huang@amd.com> wrote:
>
> On Mon, Jun 12, 2023 at 07:36:10PM +0800, Karny, Wyes wrote:
> > amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> > mode driver consistent with that rename "amd_pstate_epp" to
> > "amd-pstate-epp".
> >
> > Cc: stable@vger.kernel.org
> > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
>
> Acked-by: Huang Rui <ray.huang@amd.com>
>
> And yes, we should seprate it from cpupower as Rafael mentioned. cpupower
> tool may go to another repo.

Not only that.

It is generally better to send individual fixes that don't depend on
anything else as separate patches, because this allows them to be
picked up and fast-tracked at multiple levels (mainline, stable,
distro kernels etc.).

Re: [PATCH 1/6] amd-pstate: Make amd-pstate epp driver name hyphenated

[Huang Rui]

On Fri, Jun 16, 2023 at 08:33:30PM +0800, Rafael J. Wysocki wrote:
> On Fri, Jun 16, 2023 at 9:08 AM Huang Rui <ray.huang@amd.com> wrote:
> >
> > On Mon, Jun 12, 2023 at 07:36:10PM +0800, Karny, Wyes wrote:
> > > amd-pstate passive mode driver is hyphenated. So make amd-pstate active
> > > mode driver consistent with that rename "amd_pstate_epp" to
> > > "amd-pstate-epp".
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> >
> > Acked-by: Huang Rui <ray.huang@amd.com>
> >
> > And yes, we should seprate it from cpupower as Rafael mentioned. cpupower
> > tool may go to another repo.
> 
> Not only that.
> 
> It is generally better to send individual fixes that don't depend on
> anything else as separate patches, because this allows them to be
> picked up and fast-tracked at multiple levels (mainline, stable,
> distro kernels etc.).

Get it. Thanks for the clarification.

Best Regards,
Ray

Re: [PATCH 4/6] cpupower: Add EPP value change support

[Huang Rui]

On Fri, Jun 16, 2023 at 05:03:46PM +0800, Karny, Wyes wrote:
> Hi Ray,
> 
> On 16 Jun 15:22, Huang Rui wrote:
> > On Mon, Jun 12, 2023 at 07:36:13PM +0800, Karny, Wyes wrote:
> > > amd_pstate and intel_pstate active mode drivers support energy
> > > performance preference feature. Through this user can convey it's
> > > energy/performance preference to platform. Add this value change
> > > capability to cpupower.
> > > 
> > > To change the EPP value use below command:
> > > cpupower set --epp performance
> > > 
> > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > > ---
> > >  tools/power/cpupower/utils/cpupower-set.c    | 23 +++++++++++++++++++-
> > >  tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
> > >  tools/power/cpupower/utils/helpers/misc.c    | 19 ++++++++++++++++
> > >  3 files changed, 46 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> > > index 180d5ba877e6..a789b123dbd4 100644
> > > --- a/tools/power/cpupower/utils/cpupower-set.c
> > > +++ b/tools/power/cpupower/utils/cpupower-set.c
> > > @@ -18,6 +18,7 @@
> > >  
> > >  static struct option set_opts[] = {
> > >  	{"perf-bias", required_argument, NULL, 'b'},
> > > +	{"epp", required_argument, NULL, 'e'},
> > 
> > How about re-using the "perf-bias", I think it should be the simliar
> > function with Intel's processor?
> 
> AFAIU 'perf-bias' is not same as 'epp'. Perf-bias sysfs file expects
> 0-15 integer value, whereas epp sysfs file expects one of the string
> form energy_performance_available_preferences file.

I am thinking whether we can have a unify energy performance preferences
input in cpupower for both intel and amd processors. But it seems not easy
to align with them because different hardware interfaces here.

Anyway, it's not a big problem. Path looks good for me.

Thanks,
Ray

> 
> Thanks,
> Wyes
> > 
> > Thanks,
> > Ray
> > 
> > >  	{ },
> > >  };
> > >  
> > > @@ -37,11 +38,13 @@ int cmd_set(int argc, char **argv)
> > >  	union {
> > >  		struct {
> > >  			int perf_bias:1;
> > > +			int epp:1;
> > >  		};
> > >  		int params;
> > >  	} params;
> > >  	int perf_bias = 0;
> > >  	int ret = 0;
> > > +	char epp[30];
> > >  
> > >  	ret = uname(&uts);
> > >  	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
> > > @@ -55,7 +58,7 @@ int cmd_set(int argc, char **argv)
> > >  
> > >  	params.params = 0;
> > >  	/* parameter parsing */
> > > -	while ((ret = getopt_long(argc, argv, "b:",
> > > +	while ((ret = getopt_long(argc, argv, "b:e:",
> > >  						set_opts, NULL)) != -1) {
> > >  		switch (ret) {
> > >  		case 'b':
> > > @@ -69,6 +72,15 @@ int cmd_set(int argc, char **argv)
> > >  			}
> > >  			params.perf_bias = 1;
> > >  			break;
> > > +		case 'e':
> > > +			if (params.epp)
> > > +				print_wrong_arg_exit();
> > > +			if (sscanf(optarg, "%29s", epp) != 1) {
> > > +				print_wrong_arg_exit();
> > > +				return -EINVAL;
> > > +			}
> > > +			params.epp = 1;
> > > +			break;
> > >  		default:
> > >  			print_wrong_arg_exit();
> > >  		}
> > > @@ -102,6 +114,15 @@ int cmd_set(int argc, char **argv)
> > >  				break;
> > >  			}
> > >  		}
> > > +
> > > +		if (params.epp) {
> > > +			ret = cpupower_set_epp(cpu, epp);
> > > +			if (ret) {
> > > +				fprintf(stderr,
> > > +					"Error setting epp value on CPU %d\n", cpu);
> > > +				break;
> > > +			}
> > > +		}
> > >  	}
> > >  	return ret;
> > >  }
> > > diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> > > index 96e4bede078b..5d998de2d291 100644
> > > --- a/tools/power/cpupower/utils/helpers/helpers.h
> > > +++ b/tools/power/cpupower/utils/helpers/helpers.h
> > > @@ -116,6 +116,8 @@ extern int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val);
> > >  extern int cpupower_intel_get_perf_bias(unsigned int cpu);
> > >  extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
> > >  
> > > +extern int cpupower_set_epp(unsigned int cpu, char *epp);
> > > +
> > >  /* Read/Write msr ****************************/
> > >  
> > >  /* PCI stuff ****************************/
> > > @@ -173,6 +175,9 @@ static inline int cpupower_intel_get_perf_bias(unsigned int cpu)
> > >  static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
> > >  { return 0; };
> > >  
> > > +static inline int cpupower_set_epp(unsigned int cpu, char *epp)
> > > +{ return -1; };
> > > +
> > >  /* Read/Write msr ****************************/
> > >  
> > >  static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
> > > diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> > > index 21f653cd472c..63c3f26ef874 100644
> > > --- a/tools/power/cpupower/utils/helpers/misc.c
> > > +++ b/tools/power/cpupower/utils/helpers/misc.c
> > > @@ -87,6 +87,25 @@ int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val)
> > >  	return 0;
> > >  }
> > >  
> > > +int cpupower_set_epp(unsigned int cpu, char *epp)
> > > +{
> > > +	char path[SYSFS_PATH_MAX];
> > > +	char linebuf[30] = {};
> > > +
> > > +	snprintf(path, sizeof(path),
> > > +		PATH_TO_CPU "cpu%u/cpufreq/energy_performance_preference", cpu);
> > > +
> > > +	if (!is_valid_path(path))
> > > +		return -1;
> > > +
> > > +	snprintf(linebuf, sizeof(linebuf), "%s", epp);
> > > +
> > > +	if (cpupower_write_sysfs(path, linebuf, 30) <= 0)
> > > +		return -1;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  bool cpupower_amd_pstate_enabled(void)
> > >  {
> > >  	char *driver = cpufreq_get_driver(0);
> > > -- 
> > > 2.34.1
> > > 

Re: [PATCH 4/6] cpupower: Add EPP value change support

[Wyes Karny]

Hi Ray,

On 20 Jun 20:58, Huang Rui wrote:
> On Fri, Jun 16, 2023 at 05:03:46PM +0800, Karny, Wyes wrote:
> > Hi Ray,
> > 
> > On 16 Jun 15:22, Huang Rui wrote:
> > > On Mon, Jun 12, 2023 at 07:36:13PM +0800, Karny, Wyes wrote:
> > > > amd_pstate and intel_pstate active mode drivers support energy
> > > > performance preference feature. Through this user can convey it's
> > > > energy/performance preference to platform. Add this value change
> > > > capability to cpupower.
> > > > 
> > > > To change the EPP value use below command:
> > > > cpupower set --epp performance
> > > > 
> > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > > > ---
> > > >  tools/power/cpupower/utils/cpupower-set.c    | 23 +++++++++++++++++++-
> > > >  tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
> > > >  tools/power/cpupower/utils/helpers/misc.c    | 19 ++++++++++++++++
> > > >  3 files changed, 46 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> > > > index 180d5ba877e6..a789b123dbd4 100644
> > > > --- a/tools/power/cpupower/utils/cpupower-set.c
> > > > +++ b/tools/power/cpupower/utils/cpupower-set.c
> > > > @@ -18,6 +18,7 @@
> > > >  
> > > >  static struct option set_opts[] = {
> > > >  	{"perf-bias", required_argument, NULL, 'b'},
> > > > +	{"epp", required_argument, NULL, 'e'},
> > > 
> > > How about re-using the "perf-bias", I think it should be the simliar
> > > function with Intel's processor?
> > 
> > AFAIU 'perf-bias' is not same as 'epp'. Perf-bias sysfs file expects
> > 0-15 integer value, whereas epp sysfs file expects one of the string
> > form energy_performance_available_preferences file.
> 
> I am thinking whether we can have a unify energy performance preferences
> input in cpupower for both intel and amd processors. But it seems not easy
> to align with them because different hardware interfaces here.

It will work for intel_pstate as well. Perf_bias is differnet interface
for some intel processors IIUC.

Thanks,
Wyes
> Anyway, it's not a big problem. Path looks good for me.
> 
> Thanks,
> Ray
> 
> > 
> > Thanks,
> > Wyes
> > > 
> > > Thanks,
> > > Ray
> > > 
> > > >  	{ },
> > > >  };
> > > >  
> > > > @@ -37,11 +38,13 @@ int cmd_set(int argc, char **argv)
> > > >  	union {
> > > >  		struct {
> > > >  			int perf_bias:1;
> > > > +			int epp:1;
> > > >  		};
> > > >  		int params;
> > > >  	} params;
> > > >  	int perf_bias = 0;
> > > >  	int ret = 0;
> > > > +	char epp[30];
> > > >  
> > > >  	ret = uname(&uts);
> > > >  	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
> > > > @@ -55,7 +58,7 @@ int cmd_set(int argc, char **argv)
> > > >  
> > > >  	params.params = 0;
> > > >  	/* parameter parsing */
> > > > -	while ((ret = getopt_long(argc, argv, "b:",
> > > > +	while ((ret = getopt_long(argc, argv, "b:e:",
> > > >  						set_opts, NULL)) != -1) {
> > > >  		switch (ret) {
> > > >  		case 'b':
> > > > @@ -69,6 +72,15 @@ int cmd_set(int argc, char **argv)
> > > >  			}
> > > >  			params.perf_bias = 1;
> > > >  			break;
> > > > +		case 'e':
> > > > +			if (params.epp)
> > > > +				print_wrong_arg_exit();
> > > > +			if (sscanf(optarg, "%29s", epp) != 1) {
> > > > +				print_wrong_arg_exit();
> > > > +				return -EINVAL;
> > > > +			}
> > > > +			params.epp = 1;
> > > > +			break;
> > > >  		default:
> > > >  			print_wrong_arg_exit();
> > > >  		}
> > > > @@ -102,6 +114,15 @@ int cmd_set(int argc, char **argv)
> > > >  				break;
> > > >  			}
> > > >  		}
> > > > +
> > > > +		if (params.epp) {
> > > > +			ret = cpupower_set_epp(cpu, epp);
> > > > +			if (ret) {
> > > > +				fprintf(stderr,
> > > > +					"Error setting epp value on CPU %d\n", cpu);
> > > > +				break;
> > > > +			}
> > > > +		}
> > > >  	}
> > > >  	return ret;
> > > >  }
> > > > diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> > > > index 96e4bede078b..5d998de2d291 100644
> > > > --- a/tools/power/cpupower/utils/helpers/helpers.h
> > > > +++ b/tools/power/cpupower/utils/helpers/helpers.h
> > > > @@ -116,6 +116,8 @@ extern int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val);
> > > >  extern int cpupower_intel_get_perf_bias(unsigned int cpu);
> > > >  extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
> > > >  
> > > > +extern int cpupower_set_epp(unsigned int cpu, char *epp);
> > > > +
> > > >  /* Read/Write msr ****************************/
> > > >  
> > > >  /* PCI stuff ****************************/
> > > > @@ -173,6 +175,9 @@ static inline int cpupower_intel_get_perf_bias(unsigned int cpu)
> > > >  static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
> > > >  { return 0; };
> > > >  
> > > > +static inline int cpupower_set_epp(unsigned int cpu, char *epp)
> > > > +{ return -1; };
> > > > +
> > > >  /* Read/Write msr ****************************/
> > > >  
> > > >  static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
> > > > diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> > > > index 21f653cd472c..63c3f26ef874 100644
> > > > --- a/tools/power/cpupower/utils/helpers/misc.c
> > > > +++ b/tools/power/cpupower/utils/helpers/misc.c
> > > > @@ -87,6 +87,25 @@ int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val)
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +int cpupower_set_epp(unsigned int cpu, char *epp)
> > > > +{
> > > > +	char path[SYSFS_PATH_MAX];
> > > > +	char linebuf[30] = {};
> > > > +
> > > > +	snprintf(path, sizeof(path),
> > > > +		PATH_TO_CPU "cpu%u/cpufreq/energy_performance_preference", cpu);
> > > > +
> > > > +	if (!is_valid_path(path))
> > > > +		return -1;
> > > > +
> > > > +	snprintf(linebuf, sizeof(linebuf), "%s", epp);
> > > > +
> > > > +	if (cpupower_write_sysfs(path, linebuf, 30) <= 0)
> > > > +		return -1;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  bool cpupower_amd_pstate_enabled(void)
> > > >  {
> > > >  	char *driver = cpufreq_get_driver(0);
> > > > -- 
> > > > 2.34.1
> > > > 

Re: [PATCH 4/6] cpupower: Add EPP value change support

[Huang Rui]

On Tue, Jun 20, 2023 at 09:20:52PM +0800, Karny, Wyes wrote:
> Hi Ray,
> 
> On 20 Jun 20:58, Huang Rui wrote:
> > On Fri, Jun 16, 2023 at 05:03:46PM +0800, Karny, Wyes wrote:
> > > Hi Ray,
> > > 
> > > On 16 Jun 15:22, Huang Rui wrote:
> > > > On Mon, Jun 12, 2023 at 07:36:13PM +0800, Karny, Wyes wrote:
> > > > > amd_pstate and intel_pstate active mode drivers support energy
> > > > > performance preference feature. Through this user can convey it's
> > > > > energy/performance preference to platform. Add this value change
> > > > > capability to cpupower.
> > > > > 
> > > > > To change the EPP value use below command:
> > > > > cpupower set --epp performance
> > > > > 
> > > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > > > > ---
> > > > >  tools/power/cpupower/utils/cpupower-set.c    | 23 +++++++++++++++++++-
> > > > >  tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
> > > > >  tools/power/cpupower/utils/helpers/misc.c    | 19 ++++++++++++++++
> > > > >  3 files changed, 46 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> > > > > index 180d5ba877e6..a789b123dbd4 100644
> > > > > --- a/tools/power/cpupower/utils/cpupower-set.c
> > > > > +++ b/tools/power/cpupower/utils/cpupower-set.c
> > > > > @@ -18,6 +18,7 @@
> > > > >  
> > > > >  static struct option set_opts[] = {
> > > > >  	{"perf-bias", required_argument, NULL, 'b'},
> > > > > +	{"epp", required_argument, NULL, 'e'},
> > > > 
> > > > How about re-using the "perf-bias", I think it should be the simliar
> > > > function with Intel's processor?
> > > 
> > > AFAIU 'perf-bias' is not same as 'epp'. Perf-bias sysfs file expects
> > > 0-15 integer value, whereas epp sysfs file expects one of the string
> > > form energy_performance_available_preferences file.
> > 
> > I am thinking whether we can have a unify energy performance preferences
> > input in cpupower for both intel and amd processors. But it seems not easy
> > to align with them because different hardware interfaces here.
> 
> It will work for intel_pstate as well. Perf_bias is differnet interface
> for some intel processors IIUC.
> 

Sorry, I mixed up the energy performance preference (epp) and energy
performance bias (epb) in the intel pstate driver. You're right!

Best Regards,
Ray

> Thanks,
> Wyes
> > Anyway, it's not a big problem. Path looks good for me.
> > 
> > Thanks,
> > Ray
> > 
> > > 
> > > Thanks,
> > > Wyes
> > > > 
> > > > Thanks,
> > > > Ray
> > > > 
> > > > >  	{ },
> > > > >  };
> > > > >  
> > > > > @@ -37,11 +38,13 @@ int cmd_set(int argc, char **argv)
> > > > >  	union {
> > > > >  		struct {
> > > > >  			int perf_bias:1;
> > > > > +			int epp:1;
> > > > >  		};
> > > > >  		int params;
> > > > >  	} params;
> > > > >  	int perf_bias = 0;
> > > > >  	int ret = 0;
> > > > > +	char epp[30];
> > > > >  
> > > > >  	ret = uname(&uts);
> > > > >  	if (!ret && (!strcmp(uts.machine, "ppc64le") ||
> > > > > @@ -55,7 +58,7 @@ int cmd_set(int argc, char **argv)
> > > > >  
> > > > >  	params.params = 0;
> > > > >  	/* parameter parsing */
> > > > > -	while ((ret = getopt_long(argc, argv, "b:",
> > > > > +	while ((ret = getopt_long(argc, argv, "b:e:",
> > > > >  						set_opts, NULL)) != -1) {
> > > > >  		switch (ret) {
> > > > >  		case 'b':
> > > > > @@ -69,6 +72,15 @@ int cmd_set(int argc, char **argv)
> > > > >  			}
> > > > >  			params.perf_bias = 1;
> > > > >  			break;
> > > > > +		case 'e':
> > > > > +			if (params.epp)
> > > > > +				print_wrong_arg_exit();
> > > > > +			if (sscanf(optarg, "%29s", epp) != 1) {
> > > > > +				print_wrong_arg_exit();
> > > > > +				return -EINVAL;
> > > > > +			}
> > > > > +			params.epp = 1;
> > > > > +			break;
> > > > >  		default:
> > > > >  			print_wrong_arg_exit();
> > > > >  		}
> > > > > @@ -102,6 +114,15 @@ int cmd_set(int argc, char **argv)
> > > > >  				break;
> > > > >  			}
> > > > >  		}
> > > > > +
> > > > > +		if (params.epp) {
> > > > > +			ret = cpupower_set_epp(cpu, epp);
> > > > > +			if (ret) {
> > > > > +				fprintf(stderr,
> > > > > +					"Error setting epp value on CPU %d\n", cpu);
> > > > > +				break;
> > > > > +			}
> > > > > +		}
> > > > >  	}
> > > > >  	return ret;
> > > > >  }
> > > > > diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> > > > > index 96e4bede078b..5d998de2d291 100644
> > > > > --- a/tools/power/cpupower/utils/helpers/helpers.h
> > > > > +++ b/tools/power/cpupower/utils/helpers/helpers.h
> > > > > @@ -116,6 +116,8 @@ extern int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val);
> > > > >  extern int cpupower_intel_get_perf_bias(unsigned int cpu);
> > > > >  extern unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu);
> > > > >  
> > > > > +extern int cpupower_set_epp(unsigned int cpu, char *epp);
> > > > > +
> > > > >  /* Read/Write msr ****************************/
> > > > >  
> > > > >  /* PCI stuff ****************************/
> > > > > @@ -173,6 +175,9 @@ static inline int cpupower_intel_get_perf_bias(unsigned int cpu)
> > > > >  static inline unsigned long long msr_intel_get_turbo_ratio(unsigned int cpu)
> > > > >  { return 0; };
> > > > >  
> > > > > +static inline int cpupower_set_epp(unsigned int cpu, char *epp)
> > > > > +{ return -1; };
> > > > > +
> > > > >  /* Read/Write msr ****************************/
> > > > >  
> > > > >  static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
> > > > > diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> > > > > index 21f653cd472c..63c3f26ef874 100644
> > > > > --- a/tools/power/cpupower/utils/helpers/misc.c
> > > > > +++ b/tools/power/cpupower/utils/helpers/misc.c
> > > > > @@ -87,6 +87,25 @@ int cpupower_intel_set_perf_bias(unsigned int cpu, unsigned int val)
> > > > >  	return 0;
> > > > >  }
> > > > >  
> > > > > +int cpupower_set_epp(unsigned int cpu, char *epp)
> > > > > +{
> > > > > +	char path[SYSFS_PATH_MAX];
> > > > > +	char linebuf[30] = {};
> > > > > +
> > > > > +	snprintf(path, sizeof(path),
> > > > > +		PATH_TO_CPU "cpu%u/cpufreq/energy_performance_preference", cpu);
> > > > > +
> > > > > +	if (!is_valid_path(path))
> > > > > +		return -1;
> > > > > +
> > > > > +	snprintf(linebuf, sizeof(linebuf), "%s", epp);
> > > > > +
> > > > > +	if (cpupower_write_sysfs(path, linebuf, 30) <= 0)
> > > > > +		return -1;
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > >  bool cpupower_amd_pstate_enabled(void)
> > > > >  {
> > > > >  	char *driver = cpufreq_get_driver(0);
> > > > > -- 
> > > > > 2.34.1
> > > > >