[PATCH 0/7] intel-speed-select: Add CascadeLake-N support

[PATCH 0/7] intel-speed-select: Add CascadeLake-N support

[Prarit Bhargava]

Add support for SST-BF on CascadeLake-N support.  The CascadeLake-N
processor only support SST-BF and not other SST functionality.

Prarit Bhargava (7):
  intel-speed-select: Add int argument to command functions
  intel-speed-select: Make process_command generic
  intel-speed-select: Add check for CascadeLake-N models
  intel-speed-select: Add configuration for CascadeLake-N
  intel-speed-select: Implement CascadeLake-N help and command functions
    structures
  intel-speed-select: Implement 'perf-profile info' on CascadeLake-N
  intel-speed-select: Implement base-freq commands on CascadeLake-N

 .../x86/intel-speed-select/isst-config.c      | 443 ++++++++++++------
 .../x86/intel-speed-select/isst-display.c     |  28 +-
 tools/power/x86/intel-speed-select/isst.h     |   8 +-
 3 files changed, 319 insertions(+), 160 deletions(-)

-- 
2.21.0

[PATCH 1/7] intel-speed-select: Add int argument to command functions

[Prarit Bhargava]

The current code structure has similar but separate command functions for
the enable and disable operations.  This can be improved by adding an int
argument to the command function structure, and interpreting 1 as enable
and 0 as disable.  This change results in the removal of the disable
command functions.

Add int argument to the command function structure.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 184 +++++++-----------
 1 file changed, 69 insertions(+), 115 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index 2a9890c8395a..9f2798caead9 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -11,7 +11,8 @@
 struct process_cmd_struct {
 	char *feature;
 	char *command;
-	void (*process_fn)(void);
+	void (*process_fn)(int arg);
+	int arg;
 };
 
 static const char *version_str = "v1.0";
@@ -678,7 +679,7 @@ static void exec_on_get_ctdp_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 }
 
 #define _get_tdp_level(desc, suffix, object, help)                                \
-	static void get_tdp_##object(void)                                        \
+	static void get_tdp_##object(int arg)                                    \
 	{                                                                         \
 		struct isst_pkg_ctdp ctdp;                                        \
 \
@@ -724,7 +725,7 @@ static void dump_isst_config_for_cpu(int cpu, void *arg1, void *arg2,
 	}
 }
 
-static void dump_isst_config(void)
+static void dump_isst_config(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr,
@@ -787,7 +788,7 @@ static void set_tdp_level_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	}
 }
 
-static void set_tdp_level(void)
+static void set_tdp_level(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr, "Set Config TDP level\n");
@@ -827,7 +828,7 @@ static void dump_pbf_config_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	}
 }
 
-static void dump_pbf_config(void)
+static void dump_pbf_config(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr,
@@ -871,43 +872,27 @@ static void set_pbf_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	}
 }
 
-static void set_pbf_enable(void)
-{
-	int status = 1;
-
-	if (cmd_help) {
-		fprintf(stderr,
-			"Enable Intel Speed Select Technology base frequency feature [No command arguments are required]\n");
-		exit(0);
-	}
-
-	isst_ctdp_display_information_start(outf);
-	if (max_target_cpus)
-		for_each_online_target_cpu_in_set(set_pbf_for_cpu, NULL, NULL,
-						  NULL, &status);
-	else
-		for_each_online_package_in_set(set_pbf_for_cpu, NULL, NULL,
-					       NULL, &status);
-	isst_ctdp_display_information_end(outf);
-}
-
-static void set_pbf_disable(void)
+static void set_pbf_enable(int arg)
 {
-	int status = 0;
+	int enable = arg;
 
 	if (cmd_help) {
-		fprintf(stderr,
-			"Disable Intel Speed Select Technology base frequency feature [No command arguments are required]\n");
+		if (enable)
+			fprintf(stderr,
+				"Enable Intel Speed Select Technology base frequency feature [No command arguments are required]\n");
+		else
+			fprintf(stderr,
+				"Disable Intel Speed Select Technology base frequency feature [No command arguments are required]\n");
 		exit(0);
 	}
 
 	isst_ctdp_display_information_start(outf);
 	if (max_target_cpus)
 		for_each_online_target_cpu_in_set(set_pbf_for_cpu, NULL, NULL,
-						  NULL, &status);
+						  NULL, &enable);
 	else
 		for_each_online_package_in_set(set_pbf_for_cpu, NULL, NULL,
-					       NULL, &status);
+					       NULL, &enable);
 	isst_ctdp_display_information_end(outf);
 }
 
@@ -925,7 +910,7 @@ static void dump_fact_config_for_cpu(int cpu, void *arg1, void *arg2,
 					      fact_avx, &fact_info);
 }
 
-static void dump_fact_config(void)
+static void dump_fact_config(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr,
@@ -985,35 +970,17 @@ static void set_fact_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	}
 }
 
-static void set_fact_enable(void)
+static void set_fact_enable(int arg)
 {
-	int status = 1;
+	int enable = arg;
 
 	if (cmd_help) {
-		fprintf(stderr,
-			"Enable Intel Speed Select Technology Turbo frequency feature\n");
-		fprintf(stderr,
-			"Optional: -t|--trl : Specify turbo ratio limit\n");
-		exit(0);
-	}
-
-	isst_ctdp_display_information_start(outf);
-	if (max_target_cpus)
-		for_each_online_target_cpu_in_set(set_fact_for_cpu, NULL, NULL,
-						  NULL, &status);
-	else
-		for_each_online_package_in_set(set_fact_for_cpu, NULL, NULL,
-					       NULL, &status);
-	isst_ctdp_display_information_end(outf);
-}
-
-static void set_fact_disable(void)
-{
-	int status = 0;
-
-	if (cmd_help) {
-		fprintf(stderr,
-			"Disable Intel Speed Select Technology turbo frequency feature\n");
+		if (enable)
+			fprintf(stderr,
+				"Enable Intel Speed Select Technology Turbo frequency feature\n");
+		else
+			fprintf(stderr,
+				"Disable Intel Speed Select Technology turbo frequency feature\n");
 		fprintf(stderr,
 			"Optional: -t|--trl : Specify turbo ratio limit\n");
 		exit(0);
@@ -1022,10 +989,10 @@ static void set_fact_disable(void)
 	isst_ctdp_display_information_start(outf);
 	if (max_target_cpus)
 		for_each_online_target_cpu_in_set(set_fact_for_cpu, NULL, NULL,
-						  NULL, &status);
+						  NULL, &enable);
 	else
 		for_each_online_package_in_set(set_fact_for_cpu, NULL, NULL,
-					       NULL, &status);
+					       NULL, &enable);
 	isst_ctdp_display_information_end(outf);
 }
 
@@ -1048,19 +1015,25 @@ static void enable_clos_qos_config(int cpu, void *arg1, void *arg2, void *arg3,
 	}
 }
 
-static void set_clos_enable(void)
+static void set_clos_enable(int arg)
 {
-	int status = 1;
+	int enable = arg;
 
 	if (cmd_help) {
-		fprintf(stderr, "Enable core-power for a package/die\n");
-		fprintf(stderr,
-			"\tClos Enable: Specify priority type with [--priority|-p]\n");
-		fprintf(stderr, "\t\t 0: Proportional, 1: Ordered\n");
+		if (enable) {
+			fprintf(stderr,
+				"Enable core-power for a package/die\n");
+			fprintf(stderr,
+				"\tClos Enable: Specify priority type with [--priority|-p]\n");
+			fprintf(stderr, "\t\t 0: Proportional, 1: Ordered\n");
+		} else {
+			fprintf(stderr,
+				"Disable core-power: [No command arguments are required]\n");
+		}
 		exit(0);
 	}
 
-	if (cpufreq_sysfs_present()) {
+	if (enable && cpufreq_sysfs_present()) {
 		fprintf(stderr,
 			"cpufreq subsystem and core-power enable will interfere with each other!\n");
 	}
@@ -1068,30 +1041,10 @@ static void set_clos_enable(void)
 	isst_ctdp_display_information_start(outf);
 	if (max_target_cpus)
 		for_each_online_target_cpu_in_set(enable_clos_qos_config, NULL,
-						  NULL, NULL, &status);
-	else
-		for_each_online_package_in_set(enable_clos_qos_config, NULL,
-					       NULL, NULL, &status);
-	isst_ctdp_display_information_end(outf);
-}
-
-static void set_clos_disable(void)
-{
-	int status = 0;
-
-	if (cmd_help) {
-		fprintf(stderr,
-			"Disable core-power: [No command arguments are required]\n");
-		exit(0);
-	}
-
-	isst_ctdp_display_information_start(outf);
-	if (max_target_cpus)
-		for_each_online_target_cpu_in_set(enable_clos_qos_config, NULL,
-						  NULL, NULL, &status);
+						  NULL, NULL, &enable);
 	else
 		for_each_online_package_in_set(enable_clos_qos_config, NULL,
-					       NULL, NULL, &status);
+					       NULL, NULL, &enable);
 	isst_ctdp_display_information_end(outf);
 }
 
@@ -1109,7 +1062,7 @@ static void dump_clos_config_for_cpu(int cpu, void *arg1, void *arg2,
 					      &clos_config);
 }
 
-static void dump_clos_config(void)
+static void dump_clos_config(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr,
@@ -1145,7 +1098,7 @@ static void get_clos_info_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 		isst_clos_display_clos_information(cpu, outf, enable, prio_type);
 }
 
-static void dump_clos_info(void)
+static void dump_clos_info(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr,
@@ -1188,7 +1141,7 @@ static void set_clos_config_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 		isst_display_result(cpu, outf, "core-power", "config", ret);
 }
 
-static void set_clos_config(void)
+static void set_clos_config(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr,
@@ -1252,7 +1205,7 @@ static void set_clos_assoc_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 		isst_display_result(cpu, outf, "core-power", "assoc", ret);
 }
 
-static void set_clos_assoc(void)
+static void set_clos_assoc(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr, "Associate a clos id to a CPU\n");
@@ -1286,7 +1239,7 @@ static void get_clos_assoc_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 		isst_clos_display_assoc_information(cpu, outf, clos);
 }
 
-static void get_clos_assoc(void)
+static void get_clos_assoc(int arg)
 {
 	if (cmd_help) {
 		fprintf(stderr, "Get associate clos id to a CPU\n");
@@ -1307,26 +1260,27 @@ static void get_clos_assoc(void)
 }
 
 static struct process_cmd_struct isst_cmds[] = {
-	{ "perf-profile", "get-lock-status", get_tdp_locked },
-	{ "perf-profile", "get-config-levels", get_tdp_levels },
-	{ "perf-profile", "get-config-version", get_tdp_version },
-	{ "perf-profile", "get-config-enabled", get_tdp_enabled },
-	{ "perf-profile", "get-config-current-level", get_tdp_current_level },
-	{ "perf-profile", "set-config-level", set_tdp_level },
-	{ "perf-profile", "info", dump_isst_config },
-	{ "base-freq", "info", dump_pbf_config },
-	{ "base-freq", "enable", set_pbf_enable },
-	{ "base-freq", "disable", set_pbf_disable },
-	{ "turbo-freq", "info", dump_fact_config },
-	{ "turbo-freq", "enable", set_fact_enable },
-	{ "turbo-freq", "disable", set_fact_disable },
-	{ "core-power", "info", dump_clos_info },
-	{ "core-power", "enable", set_clos_enable },
-	{ "core-power", "disable", set_clos_disable },
-	{ "core-power", "config", set_clos_config },
-	{ "core-power", "get-config", dump_clos_config },
-	{ "core-power", "assoc", set_clos_assoc },
-	{ "core-power", "get-assoc", get_clos_assoc },
+	{ "perf-profile", "get-lock-status", get_tdp_locked, 0 },
+	{ "perf-profile", "get-config-levels", get_tdp_levels, 0 },
+	{ "perf-profile", "get-config-version", get_tdp_version, 0 },
+	{ "perf-profile", "get-config-enabled", get_tdp_enabled, 0 },
+	{ "perf-profile", "get-config-current-level", get_tdp_current_level,
+	 0 },
+	{ "perf-profile", "set-config-level", set_tdp_level, 0 },
+	{ "perf-profile", "info", dump_isst_config, 0 },
+	{ "base-freq", "info", dump_pbf_config, 0 },
+	{ "base-freq", "enable", set_pbf_enable, 1 },
+	{ "base-freq", "disable", set_pbf_enable, 0 },
+	{ "turbo-freq", "info", dump_fact_config, 0 },
+	{ "turbo-freq", "enable", set_fact_enable, 1 },
+	{ "turbo-freq", "disable", set_fact_enable, 0 },
+	{ "core-power", "info", dump_clos_info, 0 },
+	{ "core-power", "enable", set_clos_enable, 1 },
+	{ "core-power", "disable", set_clos_enable, 0 },
+	{ "core-power", "config", set_clos_config, 0 },
+	{ "core-power", "get-config", dump_clos_config, 0 },
+	{ "core-power", "assoc", set_clos_assoc, 0 },
+	{ "core-power", "get-assoc", get_clos_assoc, 0 },
 	{ NULL, NULL, NULL }
 };
 
@@ -1571,7 +1525,7 @@ void process_command(int argc, char **argv)
 		if (!strcmp(isst_cmds[i].feature, feature) &&
 		    !strcmp(isst_cmds[i].command, cmd)) {
 			parse_cmd_args(argc, optind + 1, argv);
-			isst_cmds[i].process_fn();
+			isst_cmds[i].process_fn(isst_cmds[i].arg);
 			matched = 1;
 			break;
 		}
-- 
2.21.0

Re: [PATCH 1/7] intel-speed-select: Add int argument to command functions

[Srinivas Pandruvada]

On Thu, 2019-09-26 at 08:54 -0400, Prarit Bhargava wrote:
> The current code structure has similar but separate command functions
> for
> the enable and disable operations.  This can be improved by adding an
> int
> argument to the command function structure, and interpreting 1 as
> enable
> and 0 as disable.  This change results in the removal of the disable
> command functions.
> 
> Add int argument to the command function structure.
Does this brings in any significant reduction in data or code size?
My focus is to add features first which helps users.

Thanks,
Srinivas


> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  .../x86/intel-speed-select/isst-config.c      | 184 +++++++---------
> --
>  1 file changed, 69 insertions(+), 115 deletions(-)
> 
> diff --git a/tools/power/x86/intel-speed-select/isst-config.c
> b/tools/power/x86/intel-speed-select/isst-config.c
> index 2a9890c8395a..9f2798caead9 100644
> --- a/tools/power/x86/intel-speed-select/isst-config.c
> +++ b/tools/power/x86/intel-speed-select/isst-config.c
> @@ -11,7 +11,8 @@
>  struct process_cmd_struct {
>  	char *feature;
>  	char *command;
> -	void (*process_fn)(void);
> +	void (*process_fn)(int arg);
> +	int arg;
>  };
>  
>  static const char *version_str = "v1.0";
> @@ -678,7 +679,7 @@ static void exec_on_get_ctdp_cpu(int cpu, void
> *arg1, void *arg2, void *arg3,
>  }
>  
>  #define _get_tdp_level(desc, suffix, object,
> help)                                \
> -	static void
> get_tdp_##object(void)                                        \
> +	static void get_tdp_##object(int
> arg)                                    \
>  	{                                                              
>            \
>  		struct isst_pkg_ctdp
> ctdp;                                        \
>  \
> @@ -724,7 +725,7 @@ static void dump_isst_config_for_cpu(int cpu,
> void *arg1, void *arg2,
>  	}
>  }
>  
> -static void dump_isst_config(void)
> +static void dump_isst_config(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr,
> @@ -787,7 +788,7 @@ static void set_tdp_level_for_cpu(int cpu, void
> *arg1, void *arg2, void *arg3,
>  	}
>  }
>  
> -static void set_tdp_level(void)
> +static void set_tdp_level(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr, "Set Config TDP level\n");
> @@ -827,7 +828,7 @@ static void dump_pbf_config_for_cpu(int cpu, void
> *arg1, void *arg2, void *arg3,
>  	}
>  }
>  
> -static void dump_pbf_config(void)
> +static void dump_pbf_config(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr,
> @@ -871,43 +872,27 @@ static void set_pbf_for_cpu(int cpu, void
> *arg1, void *arg2, void *arg3,
>  	}
>  }
>  
> -static void set_pbf_enable(void)
> -{
> -	int status = 1;
> -
> -	if (cmd_help) {
> -		fprintf(stderr,
> -			"Enable Intel Speed Select Technology base
> frequency feature [No command arguments are required]\n");
> -		exit(0);
> -	}
> -
> -	isst_ctdp_display_information_start(outf);
> -	if (max_target_cpus)
> -		for_each_online_target_cpu_in_set(set_pbf_for_cpu,
> NULL, NULL,
> -						  NULL, &status);
> -	else
> -		for_each_online_package_in_set(set_pbf_for_cpu, NULL,
> NULL,
> -					       NULL, &status);
> -	isst_ctdp_display_information_end(outf);
> -}
> -
> -static void set_pbf_disable(void)
> +static void set_pbf_enable(int arg)
>  {
> -	int status = 0;
> +	int enable = arg;
>  
>  	if (cmd_help) {
> -		fprintf(stderr,
> -			"Disable Intel Speed Select Technology base
> frequency feature [No command arguments are required]\n");
> +		if (enable)
> +			fprintf(stderr,
> +				"Enable Intel Speed Select Technology
> base frequency feature [No command arguments are required]\n");
> +		else
> +			fprintf(stderr,
> +				"Disable Intel Speed Select Technology
> base frequency feature [No command arguments are required]\n");
>  		exit(0);
>  	}
>  
>  	isst_ctdp_display_information_start(outf);
>  	if (max_target_cpus)
>  		for_each_online_target_cpu_in_set(set_pbf_for_cpu,
> NULL, NULL,
> -						  NULL, &status);
> +						  NULL, &enable);
>  	else
>  		for_each_online_package_in_set(set_pbf_for_cpu, NULL,
> NULL,
> -					       NULL, &status);
> +					       NULL, &enable);
>  	isst_ctdp_display_information_end(outf);
>  }
>  
> @@ -925,7 +910,7 @@ static void dump_fact_config_for_cpu(int cpu,
> void *arg1, void *arg2,
>  					      fact_avx, &fact_info);
>  }
>  
> -static void dump_fact_config(void)
> +static void dump_fact_config(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr,
> @@ -985,35 +970,17 @@ static void set_fact_for_cpu(int cpu, void
> *arg1, void *arg2, void *arg3,
>  	}
>  }
>  
> -static void set_fact_enable(void)
> +static void set_fact_enable(int arg)
>  {
> -	int status = 1;
> +	int enable = arg;
>  
>  	if (cmd_help) {
> -		fprintf(stderr,
> -			"Enable Intel Speed Select Technology Turbo
> frequency feature\n");
> -		fprintf(stderr,
> -			"Optional: -t|--trl : Specify turbo ratio
> limit\n");
> -		exit(0);
> -	}
> -
> -	isst_ctdp_display_information_start(outf);
> -	if (max_target_cpus)
> -		for_each_online_target_cpu_in_set(set_fact_for_cpu,
> NULL, NULL,
> -						  NULL, &status);
> -	else
> -		for_each_online_package_in_set(set_fact_for_cpu, NULL,
> NULL,
> -					       NULL, &status);
> -	isst_ctdp_display_information_end(outf);
> -}
> -
> -static void set_fact_disable(void)
> -{
> -	int status = 0;
> -
> -	if (cmd_help) {
> -		fprintf(stderr,
> -			"Disable Intel Speed Select Technology turbo
> frequency feature\n");
> +		if (enable)
> +			fprintf(stderr,
> +				"Enable Intel Speed Select Technology
> Turbo frequency feature\n");
> +		else
> +			fprintf(stderr,
> +				"Disable Intel Speed Select Technology
> turbo frequency feature\n");
>  		fprintf(stderr,
>  			"Optional: -t|--trl : Specify turbo ratio
> limit\n");
>  		exit(0);
> @@ -1022,10 +989,10 @@ static void set_fact_disable(void)
>  	isst_ctdp_display_information_start(outf);
>  	if (max_target_cpus)
>  		for_each_online_target_cpu_in_set(set_fact_for_cpu,
> NULL, NULL,
> -						  NULL, &status);
> +						  NULL, &enable);
>  	else
>  		for_each_online_package_in_set(set_fact_for_cpu, NULL,
> NULL,
> -					       NULL, &status);
> +					       NULL, &enable);
>  	isst_ctdp_display_information_end(outf);
>  }
>  
> @@ -1048,19 +1015,25 @@ static void enable_clos_qos_config(int cpu,
> void *arg1, void *arg2, void *arg3,
>  	}
>  }
>  
> -static void set_clos_enable(void)
> +static void set_clos_enable(int arg)
>  {
> -	int status = 1;
> +	int enable = arg;
>  
>  	if (cmd_help) {
> -		fprintf(stderr, "Enable core-power for a
> package/die\n");
> -		fprintf(stderr,
> -			"\tClos Enable: Specify priority type with [
> --priority|-p]\n");
> -		fprintf(stderr, "\t\t 0: Proportional, 1: Ordered\n");
> +		if (enable) {
> +			fprintf(stderr,
> +				"Enable core-power for a
> package/die\n");
> +			fprintf(stderr,
> +				"\tClos Enable: Specify priority type
> with [--priority|-p]\n");
> +			fprintf(stderr, "\t\t 0: Proportional, 1:
> Ordered\n");
> +		} else {
> +			fprintf(stderr,
> +				"Disable core-power: [No command
> arguments are required]\n");
> +		}
>  		exit(0);
>  	}
>  
> -	if (cpufreq_sysfs_present()) {
> +	if (enable && cpufreq_sysfs_present()) {
>  		fprintf(stderr,
>  			"cpufreq subsystem and core-power enable will
> interfere with each other!\n");
>  	}
> @@ -1068,30 +1041,10 @@ static void set_clos_enable(void)
>  	isst_ctdp_display_information_start(outf);
>  	if (max_target_cpus)
>  		for_each_online_target_cpu_in_set(enable_clos_qos_confi
> g, NULL,
> -						  NULL, NULL, &status);
> -	else
> -		for_each_online_package_in_set(enable_clos_qos_config,
> NULL,
> -					       NULL, NULL, &status);
> -	isst_ctdp_display_information_end(outf);
> -}
> -
> -static void set_clos_disable(void)
> -{
> -	int status = 0;
> -
> -	if (cmd_help) {
> -		fprintf(stderr,
> -			"Disable core-power: [No command arguments are
> required]\n");
> -		exit(0);
> -	}
> -
> -	isst_ctdp_display_information_start(outf);
> -	if (max_target_cpus)
> -		for_each_online_target_cpu_in_set(enable_clos_qos_confi
> g, NULL,
> -						  NULL, NULL, &status);
> +						  NULL, NULL, &enable);
>  	else
>  		for_each_online_package_in_set(enable_clos_qos_config,
> NULL,
> -					       NULL, NULL, &status);
> +					       NULL, NULL, &enable);
>  	isst_ctdp_display_information_end(outf);
>  }
>  
> @@ -1109,7 +1062,7 @@ static void dump_clos_config_for_cpu(int cpu,
> void *arg1, void *arg2,
>  					      &clos_config);
>  }
>  
> -static void dump_clos_config(void)
> +static void dump_clos_config(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr,
> @@ -1145,7 +1098,7 @@ static void get_clos_info_for_cpu(int cpu, void
> *arg1, void *arg2, void *arg3,
>  		isst_clos_display_clos_information(cpu, outf, enable,
> prio_type);
>  }
>  
> -static void dump_clos_info(void)
> +static void dump_clos_info(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr,
> @@ -1188,7 +1141,7 @@ static void set_clos_config_for_cpu(int cpu,
> void *arg1, void *arg2, void *arg3,
>  		isst_display_result(cpu, outf, "core-power", "config",
> ret);
>  }
>  
> -static void set_clos_config(void)
> +static void set_clos_config(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr,
> @@ -1252,7 +1205,7 @@ static void set_clos_assoc_for_cpu(int cpu,
> void *arg1, void *arg2, void *arg3,
>  		isst_display_result(cpu, outf, "core-power", "assoc",
> ret);
>  }
>  
> -static void set_clos_assoc(void)
> +static void set_clos_assoc(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr, "Associate a clos id to a CPU\n");
> @@ -1286,7 +1239,7 @@ static void get_clos_assoc_for_cpu(int cpu,
> void *arg1, void *arg2, void *arg3,
>  		isst_clos_display_assoc_information(cpu, outf, clos);
>  }
>  
> -static void get_clos_assoc(void)
> +static void get_clos_assoc(int arg)
>  {
>  	if (cmd_help) {
>  		fprintf(stderr, "Get associate clos id to a CPU\n");
> @@ -1307,26 +1260,27 @@ static void get_clos_assoc(void)
>  }
>  
>  static struct process_cmd_struct isst_cmds[] = {
> -	{ "perf-profile", "get-lock-status", get_tdp_locked },
> -	{ "perf-profile", "get-config-levels", get_tdp_levels },
> -	{ "perf-profile", "get-config-version", get_tdp_version },
> -	{ "perf-profile", "get-config-enabled", get_tdp_enabled },
> -	{ "perf-profile", "get-config-current-level",
> get_tdp_current_level },
> -	{ "perf-profile", "set-config-level", set_tdp_level },
> -	{ "perf-profile", "info", dump_isst_config },
> -	{ "base-freq", "info", dump_pbf_config },
> -	{ "base-freq", "enable", set_pbf_enable },
> -	{ "base-freq", "disable", set_pbf_disable },
> -	{ "turbo-freq", "info", dump_fact_config },
> -	{ "turbo-freq", "enable", set_fact_enable },
> -	{ "turbo-freq", "disable", set_fact_disable },
> -	{ "core-power", "info", dump_clos_info },
> -	{ "core-power", "enable", set_clos_enable },
> -	{ "core-power", "disable", set_clos_disable },
> -	{ "core-power", "config", set_clos_config },
> -	{ "core-power", "get-config", dump_clos_config },
> -	{ "core-power", "assoc", set_clos_assoc },
> -	{ "core-power", "get-assoc", get_clos_assoc },
> +	{ "perf-profile", "get-lock-status", get_tdp_locked, 0 },
> +	{ "perf-profile", "get-config-levels", get_tdp_levels, 0 },
> +	{ "perf-profile", "get-config-version", get_tdp_version, 0 },
> +	{ "perf-profile", "get-config-enabled", get_tdp_enabled, 0 },
> +	{ "perf-profile", "get-config-current-level",
> get_tdp_current_level,
> +	 0 },
> +	{ "perf-profile", "set-config-level", set_tdp_level, 0 },
> +	{ "perf-profile", "info", dump_isst_config, 0 },
> +	{ "base-freq", "info", dump_pbf_config, 0 },
> +	{ "base-freq", "enable", set_pbf_enable, 1 },
> +	{ "base-freq", "disable", set_pbf_enable, 0 },
> +	{ "turbo-freq", "info", dump_fact_config, 0 },
> +	{ "turbo-freq", "enable", set_fact_enable, 1 },
> +	{ "turbo-freq", "disable", set_fact_enable, 0 },
> +	{ "core-power", "info", dump_clos_info, 0 },
> +	{ "core-power", "enable", set_clos_enable, 1 },
> +	{ "core-power", "disable", set_clos_enable, 0 },
> +	{ "core-power", "config", set_clos_config, 0 },
> +	{ "core-power", "get-config", dump_clos_config, 0 },
> +	{ "core-power", "assoc", set_clos_assoc, 0 },
> +	{ "core-power", "get-assoc", get_clos_assoc, 0 },
>  	{ NULL, NULL, NULL }
>  };
>  
> @@ -1571,7 +1525,7 @@ void process_command(int argc, char **argv)
>  		if (!strcmp(isst_cmds[i].feature, feature) &&
>  		    !strcmp(isst_cmds[i].command, cmd)) {
>  			parse_cmd_args(argc, optind + 1, argv);
> -			isst_cmds[i].process_fn();
> +			isst_cmds[i].process_fn(isst_cmds[i].arg);
>  			matched = 1;
>  			break;
>  		}

Re: [PATCH 1/7] intel-speed-select: Add int argument to command functions

[Prarit Bhargava]

On 9/26/19 4:21 PM, Srinivas Pandruvada wrote:
> On Thu, 2019-09-26 at 08:54 -0400, Prarit Bhargava wrote:
>> The current code structure has similar but separate command functions
>> for
>> the enable and disable operations.  This can be improved by adding an
>> int
>> argument to the command function structure, and interpreting 1 as
>> enable
>> and 0 as disable.  This change results in the removal of the disable
>> command functions.
>>
>> Add int argument to the command function structure.
> Does this brings in any significant reduction in data or code size?

It reduces code size.  Right now you have separate functions for enable &
disable.  These are unified into single functions.

P.

> My focus is to add features first which helps users.
> 
> Thanks,
> Srinivas
> 
> 
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> ---
>>  .../x86/intel-speed-select/isst-config.c      | 184 +++++++---------
>> --
>>  1 file changed, 69 insertions(+), 115 deletions(-)
>>
>> diff --git a/tools/power/x86/intel-speed-select/isst-config.c
>> b/tools/power/x86/intel-speed-select/isst-config.c
>> index 2a9890c8395a..9f2798caead9 100644
>> --- a/tools/power/x86/intel-speed-select/isst-config.c
>> +++ b/tools/power/x86/intel-speed-select/isst-config.c
>> @@ -11,7 +11,8 @@
>>  struct process_cmd_struct {
>>  	char *feature;
>>  	char *command;
>> -	void (*process_fn)(void);
>> +	void (*process_fn)(int arg);
>> +	int arg;
>>  };
>>  
>>  static const char *version_str = "v1.0";
>> @@ -678,7 +679,7 @@ static void exec_on_get_ctdp_cpu(int cpu, void
>> *arg1, void *arg2, void *arg3,
>>  }
>>  
>>  #define _get_tdp_level(desc, suffix, object,
>> help)                                \
>> -	static void
>> get_tdp_##object(void)                                        \
>> +	static void get_tdp_##object(int
>> arg)                                    \
>>  	{                                                              
>>            \
>>  		struct isst_pkg_ctdp
>> ctdp;                                        \
>>  \
>> @@ -724,7 +725,7 @@ static void dump_isst_config_for_cpu(int cpu,
>> void *arg1, void *arg2,
>>  	}
>>  }
>>  
>> -static void dump_isst_config(void)
>> +static void dump_isst_config(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr,
>> @@ -787,7 +788,7 @@ static void set_tdp_level_for_cpu(int cpu, void
>> *arg1, void *arg2, void *arg3,
>>  	}
>>  }
>>  
>> -static void set_tdp_level(void)
>> +static void set_tdp_level(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr, "Set Config TDP level\n");
>> @@ -827,7 +828,7 @@ static void dump_pbf_config_for_cpu(int cpu, void
>> *arg1, void *arg2, void *arg3,
>>  	}
>>  }
>>  
>> -static void dump_pbf_config(void)
>> +static void dump_pbf_config(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr,
>> @@ -871,43 +872,27 @@ static void set_pbf_for_cpu(int cpu, void
>> *arg1, void *arg2, void *arg3,
>>  	}
>>  }
>>  
>> -static void set_pbf_enable(void)
>> -{
>> -	int status = 1;
>> -
>> -	if (cmd_help) {
>> -		fprintf(stderr,
>> -			"Enable Intel Speed Select Technology base
>> frequency feature [No command arguments are required]\n");
>> -		exit(0);
>> -	}
>> -
>> -	isst_ctdp_display_information_start(outf);
>> -	if (max_target_cpus)
>> -		for_each_online_target_cpu_in_set(set_pbf_for_cpu,
>> NULL, NULL,
>> -						  NULL, &status);
>> -	else
>> -		for_each_online_package_in_set(set_pbf_for_cpu, NULL,
>> NULL,
>> -					       NULL, &status);
>> -	isst_ctdp_display_information_end(outf);
>> -}
>> -
>> -static void set_pbf_disable(void)
>> +static void set_pbf_enable(int arg)
>>  {
>> -	int status = 0;
>> +	int enable = arg;
>>  
>>  	if (cmd_help) {
>> -		fprintf(stderr,
>> -			"Disable Intel Speed Select Technology base
>> frequency feature [No command arguments are required]\n");
>> +		if (enable)
>> +			fprintf(stderr,
>> +				"Enable Intel Speed Select Technology
>> base frequency feature [No command arguments are required]\n");
>> +		else
>> +			fprintf(stderr,
>> +				"Disable Intel Speed Select Technology
>> base frequency feature [No command arguments are required]\n");
>>  		exit(0);
>>  	}
>>  
>>  	isst_ctdp_display_information_start(outf);
>>  	if (max_target_cpus)
>>  		for_each_online_target_cpu_in_set(set_pbf_for_cpu,
>> NULL, NULL,
>> -						  NULL, &status);
>> +						  NULL, &enable);
>>  	else
>>  		for_each_online_package_in_set(set_pbf_for_cpu, NULL,
>> NULL,
>> -					       NULL, &status);
>> +					       NULL, &enable);
>>  	isst_ctdp_display_information_end(outf);
>>  }
>>  
>> @@ -925,7 +910,7 @@ static void dump_fact_config_for_cpu(int cpu,
>> void *arg1, void *arg2,
>>  					      fact_avx, &fact_info);
>>  }
>>  
>> -static void dump_fact_config(void)
>> +static void dump_fact_config(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr,
>> @@ -985,35 +970,17 @@ static void set_fact_for_cpu(int cpu, void
>> *arg1, void *arg2, void *arg3,
>>  	}
>>  }
>>  
>> -static void set_fact_enable(void)
>> +static void set_fact_enable(int arg)
>>  {
>> -	int status = 1;
>> +	int enable = arg;
>>  
>>  	if (cmd_help) {
>> -		fprintf(stderr,
>> -			"Enable Intel Speed Select Technology Turbo
>> frequency feature\n");
>> -		fprintf(stderr,
>> -			"Optional: -t|--trl : Specify turbo ratio
>> limit\n");
>> -		exit(0);
>> -	}
>> -
>> -	isst_ctdp_display_information_start(outf);
>> -	if (max_target_cpus)
>> -		for_each_online_target_cpu_in_set(set_fact_for_cpu,
>> NULL, NULL,
>> -						  NULL, &status);
>> -	else
>> -		for_each_online_package_in_set(set_fact_for_cpu, NULL,
>> NULL,
>> -					       NULL, &status);
>> -	isst_ctdp_display_information_end(outf);
>> -}
>> -
>> -static void set_fact_disable(void)
>> -{
>> -	int status = 0;
>> -
>> -	if (cmd_help) {
>> -		fprintf(stderr,
>> -			"Disable Intel Speed Select Technology turbo
>> frequency feature\n");
>> +		if (enable)
>> +			fprintf(stderr,
>> +				"Enable Intel Speed Select Technology
>> Turbo frequency feature\n");
>> +		else
>> +			fprintf(stderr,
>> +				"Disable Intel Speed Select Technology
>> turbo frequency feature\n");
>>  		fprintf(stderr,
>>  			"Optional: -t|--trl : Specify turbo ratio
>> limit\n");
>>  		exit(0);
>> @@ -1022,10 +989,10 @@ static void set_fact_disable(void)
>>  	isst_ctdp_display_information_start(outf);
>>  	if (max_target_cpus)
>>  		for_each_online_target_cpu_in_set(set_fact_for_cpu,
>> NULL, NULL,
>> -						  NULL, &status);
>> +						  NULL, &enable);
>>  	else
>>  		for_each_online_package_in_set(set_fact_for_cpu, NULL,
>> NULL,
>> -					       NULL, &status);
>> +					       NULL, &enable);
>>  	isst_ctdp_display_information_end(outf);
>>  }
>>  
>> @@ -1048,19 +1015,25 @@ static void enable_clos_qos_config(int cpu,
>> void *arg1, void *arg2, void *arg3,
>>  	}
>>  }
>>  
>> -static void set_clos_enable(void)
>> +static void set_clos_enable(int arg)
>>  {
>> -	int status = 1;
>> +	int enable = arg;
>>  
>>  	if (cmd_help) {
>> -		fprintf(stderr, "Enable core-power for a
>> package/die\n");
>> -		fprintf(stderr,
>> -			"\tClos Enable: Specify priority type with [
>> --priority|-p]\n");
>> -		fprintf(stderr, "\t\t 0: Proportional, 1: Ordered\n");
>> +		if (enable) {
>> +			fprintf(stderr,
>> +				"Enable core-power for a
>> package/die\n");
>> +			fprintf(stderr,
>> +				"\tClos Enable: Specify priority type
>> with [--priority|-p]\n");
>> +			fprintf(stderr, "\t\t 0: Proportional, 1:
>> Ordered\n");
>> +		} else {
>> +			fprintf(stderr,
>> +				"Disable core-power: [No command
>> arguments are required]\n");
>> +		}
>>  		exit(0);
>>  	}
>>  
>> -	if (cpufreq_sysfs_present()) {
>> +	if (enable && cpufreq_sysfs_present()) {
>>  		fprintf(stderr,
>>  			"cpufreq subsystem and core-power enable will
>> interfere with each other!\n");
>>  	}
>> @@ -1068,30 +1041,10 @@ static void set_clos_enable(void)
>>  	isst_ctdp_display_information_start(outf);
>>  	if (max_target_cpus)
>>  		for_each_online_target_cpu_in_set(enable_clos_qos_confi
>> g, NULL,
>> -						  NULL, NULL, &status);
>> -	else
>> -		for_each_online_package_in_set(enable_clos_qos_config,
>> NULL,
>> -					       NULL, NULL, &status);
>> -	isst_ctdp_display_information_end(outf);
>> -}
>> -
>> -static void set_clos_disable(void)
>> -{
>> -	int status = 0;
>> -
>> -	if (cmd_help) {
>> -		fprintf(stderr,
>> -			"Disable core-power: [No command arguments are
>> required]\n");
>> -		exit(0);
>> -	}
>> -
>> -	isst_ctdp_display_information_start(outf);
>> -	if (max_target_cpus)
>> -		for_each_online_target_cpu_in_set(enable_clos_qos_confi
>> g, NULL,
>> -						  NULL, NULL, &status);
>> +						  NULL, NULL, &enable);
>>  	else
>>  		for_each_online_package_in_set(enable_clos_qos_config,
>> NULL,
>> -					       NULL, NULL, &status);
>> +					       NULL, NULL, &enable);
>>  	isst_ctdp_display_information_end(outf);
>>  }
>>  
>> @@ -1109,7 +1062,7 @@ static void dump_clos_config_for_cpu(int cpu,
>> void *arg1, void *arg2,
>>  					      &clos_config);
>>  }
>>  
>> -static void dump_clos_config(void)
>> +static void dump_clos_config(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr,
>> @@ -1145,7 +1098,7 @@ static void get_clos_info_for_cpu(int cpu, void
>> *arg1, void *arg2, void *arg3,
>>  		isst_clos_display_clos_information(cpu, outf, enable,
>> prio_type);
>>  }
>>  
>> -static void dump_clos_info(void)
>> +static void dump_clos_info(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr,
>> @@ -1188,7 +1141,7 @@ static void set_clos_config_for_cpu(int cpu,
>> void *arg1, void *arg2, void *arg3,
>>  		isst_display_result(cpu, outf, "core-power", "config",
>> ret);
>>  }
>>  
>> -static void set_clos_config(void)
>> +static void set_clos_config(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr,
>> @@ -1252,7 +1205,7 @@ static void set_clos_assoc_for_cpu(int cpu,
>> void *arg1, void *arg2, void *arg3,
>>  		isst_display_result(cpu, outf, "core-power", "assoc",
>> ret);
>>  }
>>  
>> -static void set_clos_assoc(void)
>> +static void set_clos_assoc(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr, "Associate a clos id to a CPU\n");
>> @@ -1286,7 +1239,7 @@ static void get_clos_assoc_for_cpu(int cpu,
>> void *arg1, void *arg2, void *arg3,
>>  		isst_clos_display_assoc_information(cpu, outf, clos);
>>  }
>>  
>> -static void get_clos_assoc(void)
>> +static void get_clos_assoc(int arg)
>>  {
>>  	if (cmd_help) {
>>  		fprintf(stderr, "Get associate clos id to a CPU\n");
>> @@ -1307,26 +1260,27 @@ static void get_clos_assoc(void)
>>  }
>>  
>>  static struct process_cmd_struct isst_cmds[] = {
>> -	{ "perf-profile", "get-lock-status", get_tdp_locked },
>> -	{ "perf-profile", "get-config-levels", get_tdp_levels },
>> -	{ "perf-profile", "get-config-version", get_tdp_version },
>> -	{ "perf-profile", "get-config-enabled", get_tdp_enabled },
>> -	{ "perf-profile", "get-config-current-level",
>> get_tdp_current_level },
>> -	{ "perf-profile", "set-config-level", set_tdp_level },
>> -	{ "perf-profile", "info", dump_isst_config },
>> -	{ "base-freq", "info", dump_pbf_config },
>> -	{ "base-freq", "enable", set_pbf_enable },
>> -	{ "base-freq", "disable", set_pbf_disable },
>> -	{ "turbo-freq", "info", dump_fact_config },
>> -	{ "turbo-freq", "enable", set_fact_enable },
>> -	{ "turbo-freq", "disable", set_fact_disable },
>> -	{ "core-power", "info", dump_clos_info },
>> -	{ "core-power", "enable", set_clos_enable },
>> -	{ "core-power", "disable", set_clos_disable },
>> -	{ "core-power", "config", set_clos_config },
>> -	{ "core-power", "get-config", dump_clos_config },
>> -	{ "core-power", "assoc", set_clos_assoc },
>> -	{ "core-power", "get-assoc", get_clos_assoc },
>> +	{ "perf-profile", "get-lock-status", get_tdp_locked, 0 },
>> +	{ "perf-profile", "get-config-levels", get_tdp_levels, 0 },
>> +	{ "perf-profile", "get-config-version", get_tdp_version, 0 },
>> +	{ "perf-profile", "get-config-enabled", get_tdp_enabled, 0 },
>> +	{ "perf-profile", "get-config-current-level",
>> get_tdp_current_level,
>> +	 0 },
>> +	{ "perf-profile", "set-config-level", set_tdp_level, 0 },
>> +	{ "perf-profile", "info", dump_isst_config, 0 },
>> +	{ "base-freq", "info", dump_pbf_config, 0 },
>> +	{ "base-freq", "enable", set_pbf_enable, 1 },
>> +	{ "base-freq", "disable", set_pbf_enable, 0 },
>> +	{ "turbo-freq", "info", dump_fact_config, 0 },
>> +	{ "turbo-freq", "enable", set_fact_enable, 1 },
>> +	{ "turbo-freq", "disable", set_fact_enable, 0 },
>> +	{ "core-power", "info", dump_clos_info, 0 },
>> +	{ "core-power", "enable", set_clos_enable, 1 },
>> +	{ "core-power", "disable", set_clos_enable, 0 },
>> +	{ "core-power", "config", set_clos_config, 0 },
>> +	{ "core-power", "get-config", dump_clos_config, 0 },
>> +	{ "core-power", "assoc", set_clos_assoc, 0 },
>> +	{ "core-power", "get-assoc", get_clos_assoc, 0 },
>>  	{ NULL, NULL, NULL }
>>  };
>>  
>> @@ -1571,7 +1525,7 @@ void process_command(int argc, char **argv)
>>  		if (!strcmp(isst_cmds[i].feature, feature) &&
>>  		    !strcmp(isst_cmds[i].command, cmd)) {
>>  			parse_cmd_args(argc, optind + 1, argv);
>> -			isst_cmds[i].process_fn();
>> +			isst_cmds[i].process_fn(isst_cmds[i].arg);
>>  			matched = 1;
>>  			break;
>>  		}
> 

Re: [PATCH 1/7] intel-speed-select: Add int argument to command functions

[Srinivas Pandruvada]

On Mon, 2019-09-30 at 11:18 -0400, Prarit Bhargava wrote:
> 
> On 9/26/19 4:21 PM, Srinivas Pandruvada wrote:
> > On Thu, 2019-09-26 at 08:54 -0400, Prarit Bhargava wrote:
> > > The current code structure has similar but separate command
> > > functions
> > > for
> > > the enable and disable operations.  This can be improved by
> > > adding an
> > > int
> > > argument to the command function structure, and interpreting 1 as
> > > enable
> > > and 0 as disable.  This change results in the removal of the
> > > disable
> > > command functions.
> > > 
> > > Add int argument to the command function structure.
> > 
> > Does this brings in any significant reduction in data or code size?
> 
> It reduces code size.  Right now you have separate functions for
> enable &
> disable.  These are unified into single functions.
It reduce by 300 bytes.
My logic is the command processor functions are responsible for doing
what is required. After 5 years someone adding a command, don't need to
understand the meaning of additional argument.

IMO let's first focus on adding CLX-N support, this is I guess the aim
of this series. Then we will work on some cleanup.

I can't apply these patches for test. If you can use

http://git.infradead.org/linux-platform-drivers-x86.git/shortlog/refs/heads/review-andy

as the baseline.


Thanks,
Srinivas

> 
> P.
> 
> > My focus is to add features first which helps users.
> > 
> > Thanks,
> > Srinivas
> > 
> > 
> > > 
> > > Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> > > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > ---
> > >  .../x86/intel-speed-select/isst-config.c      | 184 +++++++-----
> > > ----
> > > --
> > >  1 file changed, 69 insertions(+), 115 deletions(-)
> > > 
> > > diff --git a/tools/power/x86/intel-speed-select/isst-config.c
> > > b/tools/power/x86/intel-speed-select/isst-config.c
> > > index 2a9890c8395a..9f2798caead9 100644
> > > --- a/tools/power/x86/intel-speed-select/isst-config.c
> > > +++ b/tools/power/x86/intel-speed-select/isst-config.c
> > > @@ -11,7 +11,8 @@
> > >  struct process_cmd_struct {
> > >  	char *feature;
> > >  	char *command;
> > > -	void (*process_fn)(void);
> > > +	void (*process_fn)(int arg);
> > > +	int arg;
> > >  };
> > >  
> > >  static const char *version_str = "v1.0";
> > > @@ -678,7 +679,7 @@ static void exec_on_get_ctdp_cpu(int cpu,
> > > void
> > > *arg1, void *arg2, void *arg3,
> > >  }
> > >  
> > >  #define _get_tdp_level(desc, suffix, object,
> > > help)                                \
> > > -	static void
> > > get_tdp_##object(void)                                        \
> > > +	static void get_tdp_##object(int
> > > arg)                                    \
> > >  	{                                                              
> > >            \
> > >  		struct isst_pkg_ctdp
> > > ctdp;                                        \
> > >  \
> > > @@ -724,7 +725,7 @@ static void dump_isst_config_for_cpu(int cpu,
> > > void *arg1, void *arg2,
> > >  	}
> > >  }
> > >  
> > > -static void dump_isst_config(void)
> > > +static void dump_isst_config(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr,
> > > @@ -787,7 +788,7 @@ static void set_tdp_level_for_cpu(int cpu,
> > > void
> > > *arg1, void *arg2, void *arg3,
> > >  	}
> > >  }
> > >  
> > > -static void set_tdp_level(void)
> > > +static void set_tdp_level(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr, "Set Config TDP level\n");
> > > @@ -827,7 +828,7 @@ static void dump_pbf_config_for_cpu(int cpu,
> > > void
> > > *arg1, void *arg2, void *arg3,
> > >  	}
> > >  }
> > >  
> > > -static void dump_pbf_config(void)
> > > +static void dump_pbf_config(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr,
> > > @@ -871,43 +872,27 @@ static void set_pbf_for_cpu(int cpu, void
> > > *arg1, void *arg2, void *arg3,
> > >  	}
> > >  }
> > >  
> > > -static void set_pbf_enable(void)
> > > -{
> > > -	int status = 1;
> > > -
> > > -	if (cmd_help) {
> > > -		fprintf(stderr,
> > > -			"Enable Intel Speed Select Technology base
> > > frequency feature [No command arguments are required]\n");
> > > -		exit(0);
> > > -	}
> > > -
> > > -	isst_ctdp_display_information_start(outf);
> > > -	if (max_target_cpus)
> > > -		for_each_online_target_cpu_in_set(set_pbf_for_cpu,
> > > NULL, NULL,
> > > -						  NULL, &status);
> > > -	else
> > > -		for_each_online_package_in_set(set_pbf_for_cpu, NULL,
> > > NULL,
> > > -					       NULL, &status);
> > > -	isst_ctdp_display_information_end(outf);
> > > -}
> > > -
> > > -static void set_pbf_disable(void)
> > > +static void set_pbf_enable(int arg)
> > >  {
> > > -	int status = 0;
> > > +	int enable = arg;
> > >  
> > >  	if (cmd_help) {
> > > -		fprintf(stderr,
> > > -			"Disable Intel Speed Select Technology base
> > > frequency feature [No command arguments are required]\n");
> > > +		if (enable)
> > > +			fprintf(stderr,
> > > +				"Enable Intel Speed Select Technology
> > > base frequency feature [No command arguments are required]\n");
> > > +		else
> > > +			fprintf(stderr,
> > > +				"Disable Intel Speed Select Technology
> > > base frequency feature [No command arguments are required]\n");
> > >  		exit(0);
> > >  	}
> > >  
> > >  	isst_ctdp_display_information_start(outf);
> > >  	if (max_target_cpus)
> > >  		for_each_online_target_cpu_in_set(set_pbf_for_cpu,
> > > NULL, NULL,
> > > -						  NULL, &status);
> > > +						  NULL, &enable);
> > >  	else
> > >  		for_each_online_package_in_set(set_pbf_for_cpu, NULL,
> > > NULL,
> > > -					       NULL, &status);
> > > +					       NULL, &enable);
> > >  	isst_ctdp_display_information_end(outf);
> > >  }
> > >  
> > > @@ -925,7 +910,7 @@ static void dump_fact_config_for_cpu(int cpu,
> > > void *arg1, void *arg2,
> > >  					      fact_avx, &fact_info);
> > >  }
> > >  
> > > -static void dump_fact_config(void)
> > > +static void dump_fact_config(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr,
> > > @@ -985,35 +970,17 @@ static void set_fact_for_cpu(int cpu, void
> > > *arg1, void *arg2, void *arg3,
> > >  	}
> > >  }
> > >  
> > > -static void set_fact_enable(void)
> > > +static void set_fact_enable(int arg)
> > >  {
> > > -	int status = 1;
> > > +	int enable = arg;
> > >  
> > >  	if (cmd_help) {
> > > -		fprintf(stderr,
> > > -			"Enable Intel Speed Select Technology Turbo
> > > frequency feature\n");
> > > -		fprintf(stderr,
> > > -			"Optional: -t|--trl : Specify turbo ratio
> > > limit\n");
> > > -		exit(0);
> > > -	}
> > > -
> > > -	isst_ctdp_display_information_start(outf);
> > > -	if (max_target_cpus)
> > > -		for_each_online_target_cpu_in_set(set_fact_for_cpu,
> > > NULL, NULL,
> > > -						  NULL, &status);
> > > -	else
> > > -		for_each_online_package_in_set(set_fact_for_cpu, NULL,
> > > NULL,
> > > -					       NULL, &status);
> > > -	isst_ctdp_display_information_end(outf);
> > > -}
> > > -
> > > -static void set_fact_disable(void)
> > > -{
> > > -	int status = 0;
> > > -
> > > -	if (cmd_help) {
> > > -		fprintf(stderr,
> > > -			"Disable Intel Speed Select Technology turbo
> > > frequency feature\n");
> > > +		if (enable)
> > > +			fprintf(stderr,
> > > +				"Enable Intel Speed Select Technology
> > > Turbo frequency feature\n");
> > > +		else
> > > +			fprintf(stderr,
> > > +				"Disable Intel Speed Select Technology
> > > turbo frequency feature\n");
> > >  		fprintf(stderr,
> > >  			"Optional: -t|--trl : Specify turbo ratio
> > > limit\n");
> > >  		exit(0);
> > > @@ -1022,10 +989,10 @@ static void set_fact_disable(void)
> > >  	isst_ctdp_display_information_start(outf);
> > >  	if (max_target_cpus)
> > >  		for_each_online_target_cpu_in_set(set_fact_for_cpu,
> > > NULL, NULL,
> > > -						  NULL, &status);
> > > +						  NULL, &enable);
> > >  	else
> > >  		for_each_online_package_in_set(set_fact_for_cpu, NULL,
> > > NULL,
> > > -					       NULL, &status);
> > > +					       NULL, &enable);
> > >  	isst_ctdp_display_information_end(outf);
> > >  }
> > >  
> > > @@ -1048,19 +1015,25 @@ static void enable_clos_qos_config(int
> > > cpu,
> > > void *arg1, void *arg2, void *arg3,
> > >  	}
> > >  }
> > >  
> > > -static void set_clos_enable(void)
> > > +static void set_clos_enable(int arg)
> > >  {
> > > -	int status = 1;
> > > +	int enable = arg;
> > >  
> > >  	if (cmd_help) {
> > > -		fprintf(stderr, "Enable core-power for a
> > > package/die\n");
> > > -		fprintf(stderr,
> > > -			"\tClos Enable: Specify priority type with [
> > > --priority|-p]\n");
> > > -		fprintf(stderr, "\t\t 0: Proportional, 1: Ordered\n");
> > > +		if (enable) {
> > > +			fprintf(stderr,
> > > +				"Enable core-power for a
> > > package/die\n");
> > > +			fprintf(stderr,
> > > +				"\tClos Enable: Specify priority type
> > > with [--priority|-p]\n");
> > > +			fprintf(stderr, "\t\t 0: Proportional, 1:
> > > Ordered\n");
> > > +		} else {
> > > +			fprintf(stderr,
> > > +				"Disable core-power: [No command
> > > arguments are required]\n");
> > > +		}
> > >  		exit(0);
> > >  	}
> > >  
> > > -	if (cpufreq_sysfs_present()) {
> > > +	if (enable && cpufreq_sysfs_present()) {
> > >  		fprintf(stderr,
> > >  			"cpufreq subsystem and core-power enable will
> > > interfere with each other!\n");
> > >  	}
> > > @@ -1068,30 +1041,10 @@ static void set_clos_enable(void)
> > >  	isst_ctdp_display_information_start(outf);
> > >  	if (max_target_cpus)
> > >  		for_each_online_target_cpu_in_set(enable_clos_qos_confi
> > > g, NULL,
> > > -						  NULL, NULL, &status);
> > > -	else
> > > -		for_each_online_package_in_set(enable_clos_qos_config,
> > > NULL,
> > > -					       NULL, NULL, &status);
> > > -	isst_ctdp_display_information_end(outf);
> > > -}
> > > -
> > > -static void set_clos_disable(void)
> > > -{
> > > -	int status = 0;
> > > -
> > > -	if (cmd_help) {
> > > -		fprintf(stderr,
> > > -			"Disable core-power: [No command arguments are
> > > required]\n");
> > > -		exit(0);
> > > -	}
> > > -
> > > -	isst_ctdp_display_information_start(outf);
> > > -	if (max_target_cpus)
> > > -		for_each_online_target_cpu_in_set(enable_clos_qos_confi
> > > g, NULL,
> > > -						  NULL, NULL, &status);
> > > +						  NULL, NULL, &enable);
> > >  	else
> > >  		for_each_online_package_in_set(enable_clos_qos_config,
> > > NULL,
> > > -					       NULL, NULL, &status);
> > > +					       NULL, NULL, &enable);
> > >  	isst_ctdp_display_information_end(outf);
> > >  }
> > >  
> > > @@ -1109,7 +1062,7 @@ static void dump_clos_config_for_cpu(int
> > > cpu,
> > > void *arg1, void *arg2,
> > >  					      &clos_config);
> > >  }
> > >  
> > > -static void dump_clos_config(void)
> > > +static void dump_clos_config(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr,
> > > @@ -1145,7 +1098,7 @@ static void get_clos_info_for_cpu(int cpu,
> > > void
> > > *arg1, void *arg2, void *arg3,
> > >  		isst_clos_display_clos_information(cpu, outf, enable,
> > > prio_type);
> > >  }
> > >  
> > > -static void dump_clos_info(void)
> > > +static void dump_clos_info(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr,
> > > @@ -1188,7 +1141,7 @@ static void set_clos_config_for_cpu(int
> > > cpu,
> > > void *arg1, void *arg2, void *arg3,
> > >  		isst_display_result(cpu, outf, "core-power", "config",
> > > ret);
> > >  }
> > >  
> > > -static void set_clos_config(void)
> > > +static void set_clos_config(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr,
> > > @@ -1252,7 +1205,7 @@ static void set_clos_assoc_for_cpu(int cpu,
> > > void *arg1, void *arg2, void *arg3,
> > >  		isst_display_result(cpu, outf, "core-power", "assoc",
> > > ret);
> > >  }
> > >  
> > > -static void set_clos_assoc(void)
> > > +static void set_clos_assoc(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr, "Associate a clos id to a CPU\n");
> > > @@ -1286,7 +1239,7 @@ static void get_clos_assoc_for_cpu(int cpu,
> > > void *arg1, void *arg2, void *arg3,
> > >  		isst_clos_display_assoc_information(cpu, outf, clos);
> > >  }
> > >  
> > > -static void get_clos_assoc(void)
> > > +static void get_clos_assoc(int arg)
> > >  {
> > >  	if (cmd_help) {
> > >  		fprintf(stderr, "Get associate clos id to a CPU\n");
> > > @@ -1307,26 +1260,27 @@ static void get_clos_assoc(void)
> > >  }
> > >  
> > >  static struct process_cmd_struct isst_cmds[] = {
> > > -	{ "perf-profile", "get-lock-status", get_tdp_locked },
> > > -	{ "perf-profile", "get-config-levels", get_tdp_levels },
> > > -	{ "perf-profile", "get-config-version", get_tdp_version },
> > > -	{ "perf-profile", "get-config-enabled", get_tdp_enabled },
> > > -	{ "perf-profile", "get-config-current-level",
> > > get_tdp_current_level },
> > > -	{ "perf-profile", "set-config-level", set_tdp_level },
> > > -	{ "perf-profile", "info", dump_isst_config },
> > > -	{ "base-freq", "info", dump_pbf_config },
> > > -	{ "base-freq", "enable", set_pbf_enable },
> > > -	{ "base-freq", "disable", set_pbf_disable },
> > > -	{ "turbo-freq", "info", dump_fact_config },
> > > -	{ "turbo-freq", "enable", set_fact_enable },
> > > -	{ "turbo-freq", "disable", set_fact_disable },
> > > -	{ "core-power", "info", dump_clos_info },
> > > -	{ "core-power", "enable", set_clos_enable },
> > > -	{ "core-power", "disable", set_clos_disable },
> > > -	{ "core-power", "config", set_clos_config },
> > > -	{ "core-power", "get-config", dump_clos_config },
> > > -	{ "core-power", "assoc", set_clos_assoc },
> > > -	{ "core-power", "get-assoc", get_clos_assoc },
> > > +	{ "perf-profile", "get-lock-status", get_tdp_locked, 0 },
> > > +	{ "perf-profile", "get-config-levels", get_tdp_levels, 0 },
> > > +	{ "perf-profile", "get-config-version", get_tdp_version, 0 },
> > > +	{ "perf-profile", "get-config-enabled", get_tdp_enabled, 0 },
> > > +	{ "perf-profile", "get-config-current-level",
> > > get_tdp_current_level,
> > > +	 0 },
> > > +	{ "perf-profile", "set-config-level", set_tdp_level, 0 },
> > > +	{ "perf-profile", "info", dump_isst_config, 0 },
> > > +	{ "base-freq", "info", dump_pbf_config, 0 },
> > > +	{ "base-freq", "enable", set_pbf_enable, 1 },
> > > +	{ "base-freq", "disable", set_pbf_enable, 0 },
> > > +	{ "turbo-freq", "info", dump_fact_config, 0 },
> > > +	{ "turbo-freq", "enable", set_fact_enable, 1 },
> > > +	{ "turbo-freq", "disable", set_fact_enable, 0 },
> > > +	{ "core-power", "info", dump_clos_info, 0 },
> > > +	{ "core-power", "enable", set_clos_enable, 1 },
> > > +	{ "core-power", "disable", set_clos_enable, 0 },
> > > +	{ "core-power", "config", set_clos_config, 0 },
> > > +	{ "core-power", "get-config", dump_clos_config, 0 },
> > > +	{ "core-power", "assoc", set_clos_assoc, 0 },
> > > +	{ "core-power", "get-assoc", get_clos_assoc, 0 },
> > >  	{ NULL, NULL, NULL }
> > >  };
> > >  
> > > @@ -1571,7 +1525,7 @@ void process_command(int argc, char **argv)
> > >  		if (!strcmp(isst_cmds[i].feature, feature) &&
> > >  		    !strcmp(isst_cmds[i].command, cmd)) {
> > >  			parse_cmd_args(argc, optind + 1, argv);
> > > -			isst_cmds[i].process_fn();
> > > +			isst_cmds[i].process_fn(isst_cmds[i].arg);
> > >  			matched = 1;
> > >  			break;
> > >  		}

[PATCH 2/7] intel-speed-select: Make process_command generic

[Prarit Bhargava]

Make the process_command take any help command and command list.  This
will make it easier to help commands and a command list for CascadeLake-N.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index 9f2798caead9..bb6f8f5986c9 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -1498,7 +1498,9 @@ static struct process_cmd_help_struct isst_help_cmds[] = {
 	{ NULL, NULL }
 };
 
-void process_command(int argc, char **argv)
+void process_command(int argc, char **argv,
+		     struct process_cmd_help_struct *help_cmds,
+		     struct process_cmd_struct *cmds)
 {
 	int i = 0, matched = 0;
 	char *feature = argv[optind];
@@ -1509,9 +1511,9 @@ void process_command(int argc, char **argv)
 
 	debug_printf("feature name [%s] command [%s]\n", feature, cmd);
 	if (!strcmp(cmd, "-h") || !strcmp(cmd, "--help")) {
-		while (isst_help_cmds[i].feature) {
-			if (!strcmp(isst_help_cmds[i].feature, feature)) {
-				isst_help_cmds[i].process_fn();
+		while (help_cmds[i].feature) {
+			if (!strcmp(help_cmds[i].feature, feature)) {
+				help_cmds[i].process_fn();
 				exit(0);
 			}
 			++i;
@@ -1521,11 +1523,11 @@ void process_command(int argc, char **argv)
 	create_cpu_map();
 
 	i = 0;
-	while (isst_cmds[i].feature) {
-		if (!strcmp(isst_cmds[i].feature, feature) &&
-		    !strcmp(isst_cmds[i].command, cmd)) {
+	while (cmds[i].feature) {
+		if (!strcmp(cmds[i].feature, feature) &&
+		    !strcmp(cmds[i].command, cmd)) {
 			parse_cmd_args(argc, optind + 1, argv);
-			isst_cmds[i].process_fn(isst_cmds[i].arg);
+			cmds[i].process_fn(cmds[i].arg);
 			matched = 1;
 			break;
 		}
@@ -1646,7 +1648,7 @@ static void cmdline(int argc, char **argv)
 	if (ret)
 		goto out;
 
-	process_command(argc, argv);
+	process_command(argc, argv, isst_help_cmds, isst_cmds);
 out:
 	free_cpu_set(present_cpumask);
 	free_cpu_set(target_cpumask);
-- 
2.21.0

[PATCH 3/7] intel-speed-select: Add check for CascadeLake-N models

[Prarit Bhargava]

Three CascadeLake-N models (6252N, 6230N, and 5218N) have SST-PBF support.

Return an error if the CascadeLake processor is not one of these specific
models.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index bb6f8f5986c9..ae8e3b5153ad 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -71,7 +71,7 @@ void debug_printf(const char *format, ...)
 	va_end(args);
 }
 
-static void update_cpu_model(void)
+static int update_cpu_model(void)
 {
 	unsigned int ebx, ecx, edx;
 	unsigned int fms, family;
@@ -81,6 +81,32 @@ static void update_cpu_model(void)
 	cpu_model = (fms >> 4) & 0xf;
 	if (family == 6 || family == 0xf)
 		cpu_model += ((fms >> 16) & 0xf) << 4;
+
+	/* only three CascadeLake-N models are supported */
+	if (cpu_model == 0x55) {
+		FILE *fp;
+		size_t n;
+		char *line;
+		int ret = 1;
+
+		fp = fopen("/proc/cpuinfo", "r");
+		if (!fp)
+			err(-1, "cannot open /proc/cpuinfo\n");
+
+		while (getline(&line, &n, fp) > 0) {
+			if (strstr(line, "model name")) {
+				if (strstr(line, "6252N") ||
+				    strstr(line, "6230N") ||
+				    strstr(line, "5218N"))
+					ret = 0;
+				break;
+			}
+		}
+		free(line);
+		fclose(fp);
+		return ret;
+	}
+	return 0;
 }
 
 /* Open a file, and exit on failure */
@@ -1638,7 +1664,9 @@ static void cmdline(int argc, char **argv)
 		fprintf(stderr, "Feature name and|or command not specified\n");
 		exit(0);
 	}
-	update_cpu_model();
+	ret = update_cpu_model();
+	if (ret)
+		err(-1, "Invalid CPU model (%d)\n", cpu_model);
 	printf("Intel(R) Speed Select Technology\n");
 	printf("Executing on CPU model:%d[0x%x]\n", cpu_model, cpu_model);
 	set_max_cpu_num();
-- 
2.21.0

[PATCH 4/7] intel-speed-select: Add configuration for CascadeLake-N

[Prarit Bhargava]

Create a 'dummy' pkg_dev struct for use by CascadeLake-N processors.  This
struct will be used in later patches to implement info and status calls
for CascadeLake-N SST-BF.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 98 ++++++++++++++++++-
 .../x86/intel-speed-select/isst-display.c     |  2 -
 tools/power/x86/intel-speed-select/isst.h     |  2 +
 3 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index ae8e3b5153ad..33f328d971d3 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -735,6 +735,94 @@ _get_tdp_level("get-config-current_level", levels, current_level,
 	       "Current TDP Level");
 _get_tdp_level("get-lock-status", levels, locked, "TDP lock status");
 
+struct isst_pkg_ctdp cn_pkg_dev;
+
+static int cn_get_base_ratio(void)
+{
+	FILE *fp;
+	char *begin, *end, *line;
+	char number[5];
+	float value = 0;
+	size_t n;
+
+	fp = fopen("/proc/cpuinfo", "r");
+	if (!fp)
+		err(-1, "cannot open /proc/cpuinfo\n");
+
+	while (getline(&line, &n, fp) > 0) {
+		if (strstr(line, "model name")) {
+			/* this is true for CascadeLake-N */
+			begin = strstr(line, "@ ") + 2;
+			end = strstr(line, "GHz");
+			strncpy(number, begin, end - begin);
+			value = atof(number) * 10;
+			break;
+		}
+	}
+	free(line);
+	fclose(fp);
+
+	return (int)(value);
+}
+
+static void cascadelaken_config(void)
+{
+	int i;
+	unsigned long cpu_bf;
+	struct isst_pkg_ctdp_level_info *ctdp_level;
+	struct isst_pbf_info *pbf_info;
+
+	ctdp_level = &cn_pkg_dev.ctdp_level[0];
+	pbf_info = &ctdp_level->pbf_info;
+
+	/* find the frequency base ratio */
+	ctdp_level->tdp_ratio = cn_get_base_ratio();
+	if (ctdp_level->tdp_ratio == 0)
+		err(-1, "cn base ratio is zero\n");
+
+	/* find the high and low priority frequencies */
+	pbf_info->p1_high = 0;
+	pbf_info->p1_low = ~0;
+	for (i = 0; i < topo_max_cpus; i++) {
+		cpu_bf = parse_int_file(1,
+			"/sys/devices/system/cpu/cpu%d/cpufreq/base_frequency",
+					i);
+		if (cpu_bf > pbf_info->p1_high)
+			pbf_info->p1_high = cpu_bf;
+		if (cpu_bf < pbf_info->p1_low)
+			pbf_info->p1_low = cpu_bf;
+	}
+
+	if (pbf_info->p1_high == ~0UL)
+		err(-1, "maximum base frequency not set\n");
+
+	if (pbf_info->p1_low == 0)
+		err(-1, "minimum base frequency not set\n");
+
+	/* convert frequencies back to ratios */
+	pbf_info->p1_high = pbf_info->p1_high / DISP_FREQ_MULTIPLIER;
+	pbf_info->p1_low = pbf_info->p1_low / DISP_FREQ_MULTIPLIER;
+
+	/* create high priority cpu mask */
+	pbf_info->core_cpumask_size = alloc_cpu_set(&pbf_info->core_cpumask);
+	for (i = 0; i < topo_max_cpus; i++) {
+		cpu_bf = parse_int_file(1,
+			"/sys/devices/system/cpu/cpu%d/cpufreq/base_frequency",
+					i);
+		cpu_bf = cpu_bf / DISP_FREQ_MULTIPLIER;
+		if (cpu_bf == pbf_info->p1_high)
+			CPU_SET_S(i, pbf_info->core_cpumask_size,
+				  pbf_info->core_cpumask);
+	}
+
+	/* extra ctdp & pbf struct parameters */
+	ctdp_level->processed = 1;
+	ctdp_level->pbf_support = 1; /* PBF is always supported and enabled */
+	ctdp_level->pbf_enabled = 1;
+	ctdp_level->fact_support = 0; /* FACT is never supported */
+	ctdp_level->fact_enabled = 0;
+}
+
 static void dump_isst_config_for_cpu(int cpu, void *arg1, void *arg2,
 				     void *arg3, void *arg4)
 {
@@ -1546,7 +1634,10 @@ void process_command(int argc, char **argv,
 		}
 	}
 
-	create_cpu_map();
+	if (cpu_model != 0x55)
+		create_cpu_map();
+	else
+		cascadelaken_config();
 
 	i = 0;
 	while (cmds[i].feature) {
@@ -1562,6 +1653,11 @@ void process_command(int argc, char **argv,
 
 	if (!matched)
 		fprintf(stderr, "Invalid command\n");
+
+	if (cpu_model != 0x55) {
+		/* this was allocated in cascadelaken_config() */
+		free_cpu_set(cn_pkg_dev.ctdp_level[0].pbf_info.core_cpumask);
+	}
 }
 
 static void usage(void)
diff --git a/tools/power/x86/intel-speed-select/isst-display.c b/tools/power/x86/intel-speed-select/isst-display.c
index 40346d534f78..1caa7ae25245 100644
--- a/tools/power/x86/intel-speed-select/isst-display.c
+++ b/tools/power/x86/intel-speed-select/isst-display.c
@@ -6,8 +6,6 @@
 
 #include "isst.h"
 
-#define DISP_FREQ_MULTIPLIER 100
-
 static void printcpulist(int str_len, char *str, int mask_size,
 			 cpu_set_t *cpu_mask)
 {
diff --git a/tools/power/x86/intel-speed-select/isst.h b/tools/power/x86/intel-speed-select/isst.h
index d280b27d600d..0dcae17b3945 100644
--- a/tools/power/x86/intel-speed-select/isst.h
+++ b/tools/power/x86/intel-speed-select/isst.h
@@ -69,6 +69,8 @@
 #define PM_CLOS_OFFSET				0x08
 #define PQR_ASSOC_OFFSET			0x20
 
+#define DISP_FREQ_MULTIPLIER 100
+
 struct isst_clos_config {
 	int pkg_id;
 	int die_id;
-- 
2.21.0

[PATCH 5/7] intel-speed-select: Implement CascadeLake-N help and command functions structures

[Prarit Bhargava]

CascadeLake-N only supports SST-BF and needs some of the perf-profile
commands, and the base-freq commands.

Add help functions, and create an empty command structures (the functions
will be implemented later in this patchset).  Call these functions
when running on CascadeLake-N.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 37 ++++++++++++++-----
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index 33f328d971d3..a82005825990 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -1373,6 +1373,10 @@ static void get_clos_assoc(int arg)
 	isst_ctdp_display_information_end(outf);
 }
 
+static struct process_cmd_struct cn_cmds[] = {
+	{ NULL, NULL, NULL, 0 }
+};
+
 static struct process_cmd_struct isst_cmds[] = {
 	{ "perf-profile", "get-lock-status", get_tdp_locked, 0 },
 	{ "perf-profile", "get-config-levels", get_tdp_levels, 0 },
@@ -1557,12 +1561,15 @@ static void isst_help(void)
 		TDP, etc.\n");
 	printf("\nCommands : For feature=perf-profile\n");
 	printf("\tinfo\n");
-	printf("\tget-lock-status\n");
-	printf("\tget-config-levels\n");
-	printf("\tget-config-version\n");
-	printf("\tget-config-enabled\n");
-	printf("\tget-config-current-level\n");
-	printf("\tset-config-level\n");
+
+	if (cpu_model != 0x55) {
+		printf("\tget-lock-status\n");
+		printf("\tget-config-levels\n");
+		printf("\tget-config-version\n");
+		printf("\tget-config-enabled\n");
+		printf("\tget-config-current-level\n");
+		printf("\tset-config-level\n");
+	}
 }
 
 static void pbf_help(void)
@@ -1612,6 +1619,12 @@ static struct process_cmd_help_struct isst_help_cmds[] = {
 	{ NULL, NULL }
 };
 
+static struct process_cmd_help_struct cn_help_cmds[] = {
+	{ "perf-profile", isst_help },
+	{ "base-freq", pbf_help },
+	{ NULL, NULL }
+};
+
 void process_command(int argc, char **argv,
 		     struct process_cmd_help_struct *help_cmds,
 		     struct process_cmd_struct *cmds)
@@ -1768,11 +1781,15 @@ static void cmdline(int argc, char **argv)
 	set_max_cpu_num();
 	set_cpu_present_cpu_mask();
 	set_cpu_target_cpu_mask();
-	ret = isst_fill_platform_info();
-	if (ret)
-		goto out;
 
-	process_command(argc, argv, isst_help_cmds, isst_cmds);
+	if (cpu_model != 0x55) {
+		ret = isst_fill_platform_info();
+		if (ret)
+			goto out;
+		process_command(argc, argv, isst_help_cmds, isst_cmds);
+	} else {
+		process_command(argc, argv, cn_help_cmds, cn_cmds);
+	}
 out:
 	free_cpu_set(present_cpumask);
 	free_cpu_set(target_cpumask);
-- 
2.21.0

[PATCH 6/7] intel-speed-select: Implement 'perf-profile info' on CascadeLake-N

[Prarit Bhargava]

Add functionality for perf-profile info on CascadeLake-N.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 24 +++++++++++++++----
 .../x86/intel-speed-select/isst-display.c     | 22 +++++++++++++----
 tools/power/x86/intel-speed-select/isst.h     |  3 ++-
 3 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index a82005825990..0c66012fc5a1 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -823,6 +823,13 @@ static void cascadelaken_config(void)
 	ctdp_level->fact_enabled = 0;
 }
 
+static void dump_cn_config_for_cpu(int cpu, void *arg1, void *arg2,
+				   void *arg3, void *arg4)
+{
+	isst_ctdp_display_information(cpu, outf, tdp_level, &cn_pkg_dev,
+				      cpu_model);
+}
+
 static void dump_isst_config_for_cpu(int cpu, void *arg1, void *arg2,
 				     void *arg3, void *arg4)
 {
@@ -834,13 +841,16 @@ static void dump_isst_config_for_cpu(int cpu, void *arg1, void *arg2,
 	if (ret) {
 		perror("isst_get_process_ctdp");
 	} else {
-		isst_ctdp_display_information(cpu, outf, tdp_level, &pkg_dev);
+		isst_ctdp_display_information(cpu, outf, tdp_level, &pkg_dev,
+					      cpu_model);
 		isst_get_process_ctdp_complete(cpu, &pkg_dev);
 	}
 }
 
 static void dump_isst_config(int arg)
 {
+	void *fn;
+
 	if (cmd_help) {
 		fprintf(stderr,
 			"Print Intel(R) Speed Select Technology Performance profile configuration\n");
@@ -852,14 +862,17 @@ static void dump_isst_config(int arg)
 		exit(0);
 	}
 
+	if (cpu_model != 0x55)
+		fn = dump_isst_config_for_cpu;
+	else
+		fn = dump_cn_config_for_cpu;
+
 	isst_ctdp_display_information_start(outf);
 
 	if (max_target_cpus)
-		for_each_online_target_cpu_in_set(dump_isst_config_for_cpu,
-						  NULL, NULL, NULL, NULL);
+		for_each_online_target_cpu_in_set(fn, NULL, NULL, NULL, NULL);
 	else
-		for_each_online_package_in_set(dump_isst_config_for_cpu, NULL,
-					       NULL, NULL, NULL);
+		for_each_online_package_in_set(fn, NULL, NULL, NULL, NULL);
 
 	isst_ctdp_display_information_end(outf);
 }
@@ -1374,6 +1387,7 @@ static void get_clos_assoc(int arg)
 }
 
 static struct process_cmd_struct cn_cmds[] = {
+	{ "perf-profile", "info", dump_isst_config, 0 },
 	{ NULL, NULL, NULL, 0 }
 };
 
diff --git a/tools/power/x86/intel-speed-select/isst-display.c b/tools/power/x86/intel-speed-select/isst-display.c
index 1caa7ae25245..7d91a5de1b85 100644
--- a/tools/power/x86/intel-speed-select/isst-display.c
+++ b/tools/power/x86/intel-speed-select/isst-display.c
@@ -173,7 +173,7 @@ static void print_package_info(int cpu, FILE *outf)
 
 static void _isst_pbf_display_information(int cpu, FILE *outf, int level,
 					  struct isst_pbf_info *pbf_info,
-					  int disp_level)
+					  int disp_level, int cpu_model)
 {
 	char header[256];
 	char value[256];
@@ -202,6 +202,9 @@ static void _isst_pbf_display_information(int cpu, FILE *outf, int level,
 		 pbf_info->p1_low * DISP_FREQ_MULTIPLIER);
 	format_and_print(outf, disp_level + 1, header, value);
 
+	if (cpu_model == 0x55)
+		return;
+
 	snprintf(header, sizeof(header), "tjunction-temperature(C)");
 	snprintf(value, sizeof(value), "%d", pbf_info->t_prochot);
 	format_and_print(outf, disp_level + 1, header, value);
@@ -306,7 +309,7 @@ void isst_ctdp_display_core_info(int cpu, FILE *outf, char *prefix,
 }
 
 void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
-				   struct isst_pkg_ctdp *pkg_dev)
+				   struct isst_pkg_ctdp *pkg_dev, int cpu_model)
 {
 	char header[256];
 	char value[256];
@@ -375,6 +378,16 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 			snprintf(value, sizeof(value), "unsupported");
 		format_and_print(outf, base_level + 4, header, value);
 
+		if (cpu_model == 0x55) {
+			if (ctdp_level->pbf_support)
+				_isst_pbf_display_information(cpu, outf,
+							      tdp_level,
+							  &ctdp_level->pbf_info,
+							      base_level + 4,
+							      cpu_model);
+			continue;
+		}
+
 		snprintf(header, sizeof(header), "thermal-design-power(W)");
 		snprintf(value, sizeof(value), "%d", ctdp_level->pkg_tdp);
 		format_and_print(outf, base_level + 4, header, value);
@@ -438,7 +451,8 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 		if (ctdp_level->pbf_support)
 			_isst_pbf_display_information(cpu, outf, i,
 						      &ctdp_level->pbf_info,
-						      base_level + 4);
+						      base_level + 4,
+						      cpu_model);
 		if (ctdp_level->fact_support)
 			_isst_fact_display_information(cpu, outf, i, 0xff, 0xff,
 						       &ctdp_level->fact_info,
@@ -463,7 +477,7 @@ void isst_pbf_display_information(int cpu, FILE *outf, int level,
 				  struct isst_pbf_info *pbf_info)
 {
 	print_package_info(cpu, outf);
-	_isst_pbf_display_information(cpu, outf, level, pbf_info, 4);
+	_isst_pbf_display_information(cpu, outf, level, pbf_info, 4, 0);
 	format_and_print(outf, 1, NULL, NULL);
 }
 
diff --git a/tools/power/x86/intel-speed-select/isst.h b/tools/power/x86/intel-speed-select/isst.h
index 0dcae17b3945..35b0fec44d35 100644
--- a/tools/power/x86/intel-speed-select/isst.h
+++ b/tools/power/x86/intel-speed-select/isst.h
@@ -196,7 +196,8 @@ extern int isst_get_process_ctdp(int cpu, int tdp_level,
 extern void isst_get_process_ctdp_complete(int cpu,
 					   struct isst_pkg_ctdp *pkg_dev);
 extern void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
-					  struct isst_pkg_ctdp *pkg_dev);
+					  struct isst_pkg_ctdp *pkg_dev,
+					  int cpu_model);
 extern void isst_ctdp_display_core_info(int cpu, FILE *outf, char *prefix,
 					unsigned int val);
 extern void isst_ctdp_display_information_start(FILE *outf);
-- 
2.21.0

[PATCH 7/7] intel-speed-select: Implement base-freq commands on CascadeLake-N

[Prarit Bhargava]

Add functionality for base-freq info|enable|disable info on CascadeLake-N.

The enable command always returns success, and the disable command always
returns failed because SST-BF cannot be enabled or disabled from the OS on
CascadeLake-N.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../x86/intel-speed-select/isst-config.c      | 56 ++++++++++++++-----
 .../x86/intel-speed-select/isst-display.c     |  6 +-
 tools/power/x86/intel-speed-select/isst.h     |  3 +-
 3 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index 0c66012fc5a1..24e6084a36b4 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -940,6 +940,18 @@ static void set_tdp_level(int arg)
 	isst_ctdp_display_information_end(outf);
 }
 
+static void cn_dump_pbf_config_for_cpu(int cpu, void *arg1, void *arg2,
+				       void *arg3, void *arg4)
+{
+	struct isst_pbf_info *pbf_info;
+	struct isst_pkg_ctdp_level_info *ctdp_level;
+
+	ctdp_level = &cn_pkg_dev.ctdp_level[0];
+	pbf_info = &ctdp_level->pbf_info;
+
+	isst_pbf_display_information(cpu, outf, tdp_level, pbf_info, cpu_model);
+}
+
 static void dump_pbf_config_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 				    void *arg4)
 {
@@ -950,13 +962,16 @@ static void dump_pbf_config_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	if (ret) {
 		perror("isst_get_pbf_info");
 	} else {
-		isst_pbf_display_information(cpu, outf, tdp_level, &pbf_info);
+		isst_pbf_display_information(cpu, outf, tdp_level, &pbf_info,
+					     cpu_model);
 		isst_get_pbf_info_complete(&pbf_info);
 	}
 }
 
 static void dump_pbf_config(int arg)
 {
+	void *fn;
+
 	if (cmd_help) {
 		fprintf(stderr,
 			"Print Intel(R) Speed Select Technology base frequency configuration for a TDP level\n");
@@ -970,13 +985,18 @@ static void dump_pbf_config(int arg)
 		exit(1);
 	}
 
+	if (cpu_model != 0x55)
+		fn = dump_pbf_config_for_cpu;
+	else
+		fn = cn_dump_pbf_config_for_cpu;
+
 	isst_ctdp_display_information_start(outf);
+
 	if (max_target_cpus)
-		for_each_online_target_cpu_in_set(dump_pbf_config_for_cpu, NULL,
-						  NULL, NULL, NULL);
+		for_each_online_target_cpu_in_set(fn, NULL, NULL, NULL, NULL);
 	else
-		for_each_online_package_in_set(dump_pbf_config_for_cpu, NULL,
-					       NULL, NULL, NULL);
+		for_each_online_package_in_set(fn, NULL, NULL, NULL, NULL);
+
 	isst_ctdp_display_information_end(outf);
 }
 
@@ -986,17 +1006,24 @@ static void set_pbf_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	int ret;
 	int status = *(int *)arg4;
 
-	ret = isst_set_pbf_fact_status(cpu, 1, status);
-	if (ret) {
-		perror("isst_set_pbf");
+	if (cpu_model != 0x55) {
+		ret = isst_set_pbf_fact_status(cpu, 1, status);
+		if (ret) {
+			perror("isst_set_pbf");
+			return;
+		}
+
 	} else {
-		if (status)
-			isst_display_result(cpu, outf, "base-freq", "enable",
-					    ret);
+		if (status == 0)
+			ret = -1;
 		else
-			isst_display_result(cpu, outf, "base-freq", "disable",
-					    ret);
+			ret = 0;
 	}
+
+	if (status)
+		isst_display_result(cpu, outf, "base-freq", "enable", ret);
+	else
+		isst_display_result(cpu, outf, "base-freq", "disable", ret);
 }
 
 static void set_pbf_enable(int arg)
@@ -1388,6 +1415,9 @@ static void get_clos_assoc(int arg)
 
 static struct process_cmd_struct cn_cmds[] = {
 	{ "perf-profile", "info", dump_isst_config, 0 },
+	{ "base-freq", "info", dump_pbf_config, 0 },
+	{ "base-freq", "enable", set_pbf_enable, 1 },
+	{ "base-freq", "disable", set_pbf_enable, 0 },
 	{ NULL, NULL, NULL, 0 }
 };
 
diff --git a/tools/power/x86/intel-speed-select/isst-display.c b/tools/power/x86/intel-speed-select/isst-display.c
index 7d91a5de1b85..8cc8d963a55e 100644
--- a/tools/power/x86/intel-speed-select/isst-display.c
+++ b/tools/power/x86/intel-speed-select/isst-display.c
@@ -474,10 +474,12 @@ void isst_ctdp_display_information_end(FILE *outf)
 }
 
 void isst_pbf_display_information(int cpu, FILE *outf, int level,
-				  struct isst_pbf_info *pbf_info)
+				  struct isst_pbf_info *pbf_info,
+				  int cpu_model)
 {
 	print_package_info(cpu, outf);
-	_isst_pbf_display_information(cpu, outf, level, pbf_info, 4, 0);
+	_isst_pbf_display_information(cpu, outf, level, pbf_info, 4,
+				      cpu_model);
 	format_and_print(outf, 1, NULL, NULL);
 }
 
diff --git a/tools/power/x86/intel-speed-select/isst.h b/tools/power/x86/intel-speed-select/isst.h
index 35b0fec44d35..d94b46272ce2 100644
--- a/tools/power/x86/intel-speed-select/isst.h
+++ b/tools/power/x86/intel-speed-select/isst.h
@@ -203,7 +203,8 @@ extern void isst_ctdp_display_core_info(int cpu, FILE *outf, char *prefix,
 extern void isst_ctdp_display_information_start(FILE *outf);
 extern void isst_ctdp_display_information_end(FILE *outf);
 extern void isst_pbf_display_information(int cpu, FILE *outf, int level,
-					 struct isst_pbf_info *info);
+					 struct isst_pbf_info *info,
+					 int cpu_model);
 extern int isst_set_tdp_level(int cpu, int tdp_level);
 extern int isst_set_tdp_level_msr(int cpu, int tdp_level);
 extern int isst_set_pbf_fact_status(int cpu, int pbf, int enable);
-- 
2.21.0