diff mbox series

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

Message ID	20230612113615.205353-5-wyes.karny@amd.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C From: Wyes Karny <wyes.karny@amd.com> To: <rafael@kernel.org>, <ray.huang@amd.com>, <viresh.kumar@linaro.org>, <trenn@suse.com>, <shuah@kernel.org> CC: <gautham.shenoy@amd.com>, <Mario.Limonciello@amd.com>, <Perry.Yuan@amd.com>, <linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Wyes Karny <wyes.karny@amd.com> Subject: [PATCH 4/6] cpupower: Add EPP value change support Date: Mon, 12 Jun 2023 11:36:13 +0000 Message-ID: <20230612113615.205353-5-wyes.karny@amd.com> In-Reply-To: <20230612113615.205353-1-wyes.karny@amd.com> References: <20230612113615.205353-1-wyes.karny@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain Precedence: bulk
Series	cpupower: Add various feature control support for amd_pstate \| [0/6] cpupower: Add various feature control support for amd_pstate [1/6] amd-pstate: Make amd-pstate epp driver name hyphenated [2/6] cpupower: Recognise amd-pstate active mode driver [3/6] cpupower: Add is_sysfs_present API [4/6] cpupower: Add EPP value change support [5/6] cpupower: Add support for amd_pstate mode change [6/6] cpupower: Add turbo-boost support in cpupower

Commit Message

Wyes Karny June 12, 2023, 11:36 a.m. UTC

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

Comments

Huang Rui June 16, 2023, 7:22 a.m. UTC | #1

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
>

Wyes Karny June 16, 2023, 9:03 a.m. UTC | #2

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

Huang Rui June 20, 2023, 12:58 p.m. UTC | #3

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

Wyes Karny June 20, 2023, 1:20 p.m. UTC | #4

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

Huang Rui June 20, 2023, 1:49 p.m. UTC | #5

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

diff mbox series

Patch

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