2023-06-12 11:53:15

by Wyes Karny

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

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



2023-06-12 11:53:15

by Wyes Karny

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

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: [email protected]
Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
Reviewed-by: Gautham R. Shenoy <[email protected]>
Signed-off-by: Wyes Karny <[email protected]>
---
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


2023-06-12 12:22:07

by Wyes Karny

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

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 <[email protected]>
Signed-off-by: Wyes Karny <[email protected]>
---
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


2023-06-12 12:24:04

by Wyes Karny

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

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 <[email protected]>
Signed-off-by: Wyes Karny <[email protected]>
---
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


2023-06-15 17:56:43

by Rafael J. Wysocki

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

On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <[email protected]> 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: [email protected]
> Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> Reviewed-by: Gautham R. Shenoy <[email protected]>
> Signed-off-by: Wyes Karny <[email protected]>

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
>

2023-06-15 18:07:10

by Wyes Karny

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

Hi Rafael,

On 15 Jun 19:31, Rafael J. Wysocki wrote:
> On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <[email protected]> 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: [email protected]
> > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > Reviewed-by: Gautham R. Shenoy <[email protected]>
> > Signed-off-by: Wyes Karny <[email protected]>
>
> 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
> >

2023-06-15 18:35:32

by Rafael J. Wysocki

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

On Thu, Jun 15, 2023 at 7:55 PM Wyes Karny <[email protected]> wrote:
>
> Hi Rafael,
>
> On 15 Jun 19:31, Rafael J. Wysocki wrote:
> > On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <[email protected]> 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: [email protected]
> > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > > Reviewed-by: Gautham R. Shenoy <[email protected]>
> > > Signed-off-by: Wyes Karny <[email protected]>
> >
> > 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.

2023-06-15 19:39:50

by Wyes Karny

[permalink] [raw]
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 <[email protected]> wrote:
> >
> > Hi Rafael,
> >
> > On 15 Jun 19:31, Rafael J. Wysocki wrote:
> > > On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <[email protected]> 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: [email protected]
> > > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > > > Reviewed-by: Gautham R. Shenoy <[email protected]>
> > > > Signed-off-by: Wyes Karny <[email protected]>
> > >
> > > 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

2023-06-16 07:40:22

by Huang Rui

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

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 <[email protected]>
> Signed-off-by: Wyes Karny <[email protected]>

Acked-by: Huang Rui <[email protected]>

> ---
> 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
>

2023-06-16 07:48:06

by Yuan, Perry

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

[AMD Official Use Only - General]

Hi Wyse,

> -----Original Message-----
> From: Karny, Wyes <[email protected]>
> Sent: Friday, June 16, 2023 3:05 AM
> To: Rafael J. Wysocki <[email protected]>
> Cc: Huang, Ray <[email protected]>; [email protected];
> [email protected]; [email protected]; Shenoy, Gautham Ranjal
> <[email protected]>; Limonciello, Mario
> <[email protected]>; Yuan, Perry <[email protected]>; linux-
> [email protected]; [email protected]; [email protected]
> 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 <[email protected]>
> wrote:
> > >
> > > Hi Rafael,
> > >
> > > On 15 Jun 19:31, Rafael J. Wysocki wrote:
> > > > On Mon, Jun 12, 2023 at 1:37 PM Wyes Karny <[email protected]>
> 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: [email protected]
> > > > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP
> > > > > support for the AMD processors")
> > > > > Reviewed-by: Gautham R. Shenoy <[email protected]>
> > > > > Signed-off-by: Wyes Karny <[email protected]>
> > > >
> > > > 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 <[email protected]>

2023-06-16 07:49:02

by Huang Rui

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

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: [email protected]
> Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> Reviewed-by: Gautham R. Shenoy <[email protected]>
> Signed-off-by: Wyes Karny <[email protected]>

Acked-by: Huang Rui <[email protected]>

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
>

2023-06-16 07:53:30

by Huang Rui

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

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 <[email protected]>
> Signed-off-by: Wyes Karny <[email protected]>

Acked-by: Huang Rui <[email protected]>

> ---
> 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
>

2023-06-16 13:00:01

by Rafael J. Wysocki

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

On Fri, Jun 16, 2023 at 9:08 AM Huang Rui <[email protected]> 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: [email protected]
> > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > Reviewed-by: Gautham R. Shenoy <[email protected]>
> > Signed-off-by: Wyes Karny <[email protected]>
>
> Acked-by: Huang Rui <[email protected]>
>
> 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.).

2023-06-20 13:13:14

by Huang Rui

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

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 <[email protected]> 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: [email protected]
> > > Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> > > Reviewed-by: Gautham R. Shenoy <[email protected]>
> > > Signed-off-by: Wyes Karny <[email protected]>
> >
> > Acked-by: Huang Rui <[email protected]>
> >
> > 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