Merge tag 'platform-drivers-x86-v5.7-1' of git://git.infradead.org/linux-platform-drivers-x86

Pull x86 platform driver updates from Andy Shevchenko:

 - Fix for improper handling of fan_boost_mode in sysfs for ASUS
   laptops.

 - On newer ASUS laptops the 1st battery is named differently, here is a
   fix.

 - Fix Lex 2I385SW to allow both network cards to be used.

 - The power integrated circuit driver for Surface 3 has been added.

 - Refactor and clean up of Intel PMC driver and enable it on Intel
   Jasper Lake.

 - Clean up of Dell RBU driver.

 - Big update for Intel Speed Select technology support tool and driver.

* tag 'platform-drivers-x86-v5.7-1' of git://git.infradead.org/linux-platform-drivers-x86: (75 commits)
  platform/x86: surface3_power: Fix always true condition in mshw0011_space_handler()
  platform/x86: surface3_power: Fix Kconfig section ordering
  platform/x86: surface3_power: Add missed headers
  platform/x86: surface3_power: Reformat GUID assignment
  platform/x86: surface3_power: Drop useless macro ACPI_PTR()
  platform/x86: surface3_power: Prefix POLL_INTERVAL with SURFACE_3
  platform/x86: surface3_power: Simplify mshw0011_adp_psr() to one liner
  platform/x86: surface3_power: Use dev_err() instead of pr_err()
  platform/x86: surface3_power: Drop unused structure definition
  platform/x86: surface3_power: MSHW0011 rev-eng implementation
  platform/x86: intel_pmc_core: Make pmc_core_substate_res_show() generic
  platform/x86: intel_pmc_core: Make pmc_core_lpm_display() generic for platforms that support sub-states
  tools/power/x86/intel-speed-select: Fix a typo in error message
  tools/power/x86/intel-speed-select: Update version
  tools/power/x86/intel-speed-select: Avoid duplicate Package strings for json
  tools/power/x86/intel-speed-select: Add display for enabled cpus count
  tools/power/x86/intel-speed-select: Print friendly warning for bad command line
  tools/power/x86/intel-speed-select: Fix avx options for turbo-freq feature
  tools/power/x86/intel-speed-select: Improve CLX commands
  tools/power/x86/intel-speed-select: Show error for invalid CPUs in the options
  ...

4 files changed, 760 insertions, 230 deletions

diff --git a/tools/power/x86/intel-speed-select/isst-config.c b/tools/power/x86/intel-speed-select/isst-config.c
index 2b2b8167c65b..b73763489410 100644
--- a/tools/power/x86/intel-speed-select/isst-config.c
+++ b/tools/power/x86/intel-speed-select/isst-config.c
@@ -15,7 +15,7 @@ struct process_cmd_struct {
 	int arg;
 };
 
-static const char *version_str = "v1.2";
+static const char *version_str = "v1.3";
 static const int supported_api_ver = 1;
 static struct isst_if_platform_info isst_platform_info;
 static char *progname;
@@ -42,6 +42,7 @@ static int out_format_json;
 static int cmd_help;
 static int force_online_offline;
 static int auto_mode;
+static int fact_enable_fail;
 
 /* clos related */
 static int current_clos = -1;
@@ -61,6 +62,18 @@ struct _cpu_map {
 };
 struct _cpu_map *cpu_map;
 
+struct cpu_topology {
+	short cpu;
+	short core_id;
+	short pkg_id;
+	short die_id;
+};
+
+FILE *get_output_file(void)
+{
+	return outf;
+}
+
 void debug_printf(const char *format, ...)
 {
 	va_list args;
@@ -82,6 +95,14 @@ int is_clx_n_platform(void)
 	return 0;
 }
 
+int is_skx_based_platform(void)
+{
+	if (cpu_model == 0x55)
+		return 1;
+
+	return 0;
+}
+
 static int update_cpu_model(void)
 {
 	unsigned int ebx, ecx, edx;
@@ -175,25 +196,137 @@ int out_format_is_json(void)
 	return out_format_json;
 }
 
+static int get_stored_topology_info(int cpu, int *core_id, int *pkg_id, int *die_id)
+{
+	const char *pathname = "/tmp/isst_cpu_topology.dat";
+	struct cpu_topology cpu_top;
+	FILE *fp;
+	int ret;
+
+	fp = fopen(pathname, "rb");
+	if (!fp)
+		return -1;
+
+	ret = fseek(fp, cpu * sizeof(cpu_top), SEEK_SET);
+	if (ret)
+		goto err_ret;
+
+	ret = fread(&cpu_top, sizeof(cpu_top), 1, fp);
+	if (ret != 1) {
+		ret = -1;
+		goto err_ret;
+	}
+
+	*pkg_id = cpu_top.pkg_id;
+	*core_id = cpu_top.core_id;
+	*die_id = cpu_top.die_id;
+	ret = 0;
+
+err_ret:
+	fclose(fp);
+
+	return ret;
+}
+
+static void store_cpu_topology(void)
+{
+	const char *pathname = "/tmp/isst_cpu_topology.dat";
+	FILE *fp;
+	int i;
+
+	fp = fopen(pathname, "rb");
+	if (fp) {
+		/* Mapping already exists */
+		fclose(fp);
+		return;
+	}
+
+	fp = fopen(pathname, "wb");
+	if (!fp) {
+		fprintf(stderr, "Can't create file:%s\n", pathname);
+		return;
+	}
+
+	for (i = 0; i < topo_max_cpus; ++i) {
+		struct cpu_topology cpu_top;
+
+		cpu_top.core_id = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/core_id", i);
+		if (cpu_top.core_id < 0)
+			cpu_top.core_id = -1;
+
+		cpu_top.pkg_id = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/physical_package_id", i);
+		if (cpu_top.pkg_id < 0)
+			cpu_top.pkg_id = -1;
+
+		cpu_top.die_id = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/die_id", i);
+		if (cpu_top.die_id < 0)
+			cpu_top.die_id = -1;
+
+		cpu_top.cpu = i;
+
+		if (fwrite(&cpu_top, sizeof(cpu_top), 1, fp) != 1) {
+			fprintf(stderr, "Can't write to:%s\n", pathname);
+			break;
+		}
+	}
+
+	fclose(fp);
+}
+
 int get_physical_package_id(int cpu)
 {
-	return parse_int_file(
-		0, "/sys/devices/system/cpu/cpu%d/topology/physical_package_id",
-		cpu);
+	int ret;
+
+	ret = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/physical_package_id",
+			cpu);
+	if (ret < 0) {
+		int core_id, pkg_id, die_id;
+
+		ret = get_stored_topology_info(cpu, &core_id, &pkg_id, &die_id);
+		if (!ret)
+			return pkg_id;
+	}
+
+	return ret;
 }
 
 int get_physical_core_id(int cpu)
 {
-	return parse_int_file(
-		0, "/sys/devices/system/cpu/cpu%d/topology/core_id", cpu);
+	int ret;
+
+	ret = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/core_id",
+			cpu);
+	if (ret < 0) {
+		int core_id, pkg_id, die_id;
+
+		ret = get_stored_topology_info(cpu, &core_id, &pkg_id, &die_id);
+		if (!ret)
+			return core_id;
+	}
+
+	return ret;
 }
 
 int get_physical_die_id(int cpu)
 {
 	int ret;
 
-	ret = parse_int_file(0, "/sys/devices/system/cpu/cpu%d/topology/die_id",
-			     cpu);
+	ret = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/die_id",
+			cpu);
+	if (ret < 0) {
+		int core_id, pkg_id, die_id;
+
+		ret = get_stored_topology_info(cpu, &core_id, &pkg_id, &die_id);
+		if (!ret)
+			return die_id;
+	}
+
 	if (ret < 0)
 		ret = 0;
 
@@ -219,8 +352,14 @@ static void set_cpu_online_offline(int cpu, int state)
 		 "/sys/devices/system/cpu/cpu%d/online", cpu);
 
 	fd = open(buffer, O_WRONLY);
-	if (fd < 0)
+	if (fd < 0) {
+		if (!cpu && state) {
+			fprintf(stderr, "This system is not configured for CPU 0 online/offline\n");
+			fprintf(stderr, "Ignoring online request for CPU 0 as this is already online\n");
+			return;
+		}
 		err(-1, "%s open failed", buffer);
+	}
 
 	if (state)
 		ret = write(fd, "1\n", 2);
@@ -259,7 +398,12 @@ static void for_each_online_package_in_set(void (*callback)(int, void *, void *,
 		die_id = get_physical_die_id(i);
 		if (die_id < 0)
 			die_id = 0;
-		pkg_id = get_physical_package_id(i);
+
+		pkg_id = parse_int_file(0,
+			"/sys/devices/system/cpu/cpu%d/topology/physical_package_id", i);
+		if (pkg_id < 0)
+			continue;
+
 		/* Create an unique id for package, die combination to store */
 		pkg_id = (MAX_PACKAGE_COUNT * pkg_id + die_id);
 
@@ -281,7 +425,7 @@ static void for_each_online_target_cpu_in_set(
 	void (*callback)(int, void *, void *, void *, void *), void *arg1,
 	void *arg2, void *arg3, void *arg4)
 {
-	int i;
+	int i, found = 0;
 
 	for (i = 0; i < topo_max_cpus; ++i) {
 		int online;
@@ -295,9 +439,14 @@ static void for_each_online_target_cpu_in_set(
 			online =
 				1; /* online entry for CPU 0 needs some special configs */
 
-		if (online && callback)
+		if (online && callback) {
 			callback(i, arg1, arg2, arg3, arg4);
+			found = 1;
+		}
 	}
+
+	if (!found)
+		fprintf(stderr, "No valid CPU in the list\n");
 }
 
 #define BITMASK_SIZE 32
@@ -305,14 +454,27 @@ static void set_max_cpu_num(void)
 {
 	FILE *filep;
 	unsigned long dummy;
+	int i;
 
 	topo_max_cpus = 0;
-	filep = fopen_or_exit(
-		"/sys/devices/system/cpu/cpu0/topology/thread_siblings", "r");
+	for (i = 0; i < 256; ++i) {
+		char path[256];
+
+		snprintf(path, sizeof(path),
+			 "/sys/devices/system/cpu/cpu%d/topology/thread_siblings", i);
+		filep = fopen(path, "r");
+		if (filep)
+			break;
+	}
+
+	if (!filep) {
+		fprintf(stderr, "Can't get max cpu number\n");
+		exit(0);
+	}
+
 	while (fscanf(filep, "%lx,", &dummy) == 1)
 		topo_max_cpus += BITMASK_SIZE;
 	fclose(filep);
-	topo_max_cpus--; /* 0 based */
 
 	debug_printf("max cpus %d\n", topo_max_cpus);
 }
@@ -362,6 +524,10 @@ static void set_cpu_present_cpu_mask(void)
 				die_id = 0;
 
 			pkg_id = get_physical_package_id(i);
+			if (pkg_id < 0) {
+				fprintf(stderr, "Failed to get package id, CPU %d may be offline\n", i);
+				continue;
+			}
 			if (pkg_id < MAX_PACKAGE_COUNT &&
 			    die_id < MAX_DIE_PER_PACKAGE) {
 				int core_id = get_physical_core_id(i);
@@ -542,7 +708,11 @@ static int isst_send_mmio_command(unsigned int cpu, unsigned int reg, int write,
 	}
 
 	if (ioctl(fd, cmd, &io_regs) == -1) {
-		perror("ISST_IF_IO_CMD");
+		if (errno == ENOTTY) {
+			perror("ISST_IF_IO_COMMAND\n");
+			fprintf(stderr, "Check presence of kernel modules: isst_if_mmio\n");
+			exit(0);
+		}
 		fprintf(outf, "Error: mmio_cmd cpu:%d reg:%x read_write:%x\n",
 			cpu, reg, write);
 	} else {
@@ -571,7 +741,8 @@ int isst_send_mbox_command(unsigned int cpu, unsigned char command,
 		"mbox_send: cpu:%d command:%x sub_command:%x parameter:%x req_data:%x\n",
 		cpu, command, sub_command, parameter, req_data);
 
-	if (isst_platform_info.mmio_supported && command == CONFIG_CLOS) {
+	if (!is_skx_based_platform() && command == CONFIG_CLOS &&
+	    sub_command != CLOS_PM_QOS_CONFIG) {
 		unsigned int value;
 		int write = 0;
 		int clos_id, core_id, ret = 0;
@@ -620,10 +791,14 @@ int isst_send_mbox_command(unsigned int cpu, unsigned char command,
 		err(-1, "%s open failed", pathname);
 
 	if (ioctl(fd, ISST_IF_MBOX_COMMAND, &mbox_cmds) == -1) {
-		perror("ISST_IF_MBOX_COMMAND");
-		fprintf(outf,
-			"Error: mbox_cmd cpu:%d command:%x sub_command:%x parameter:%x req_data:%x\n",
-			cpu, command, sub_command, parameter, req_data);
+		if (errno == ENOTTY) {
+			perror("ISST_IF_MBOX_COMMAND\n");
+			fprintf(stderr, "Check presence of kernel modules: isst_if_mbox_pci or isst_if_mbox_msr\n");
+			exit(0);
+		}
+		debug_printf(
+			"Error: mbox_cmd cpu:%d command:%x sub_command:%x parameter:%x req_data:%x errorno:%d\n",
+			cpu, command, sub_command, parameter, req_data, errno);
 		return -1;
 	} else {
 		*resp = mbox_cmds.mbox_cmd[0].resp_data;
@@ -656,7 +831,7 @@ int isst_send_msr_command(unsigned int cpu, unsigned int msr, int write,
 		msr_cmds.msr_cmd[0].data = *req_resp;
 
 	if (ioctl(fd, ISST_IF_MSR_COMMAND, &msr_cmds) == -1) {
-		perror("ISST_IF_MSR_COMMAD");
+		perror("ISST_IF_MSR_COMMAND");
 		fprintf(outf, "Error: msr_cmd cpu:%d msr:%x read_write:%d\n",
 			cpu, msr, write);
 	} else {
@@ -697,12 +872,85 @@ static int isst_fill_platform_info(void)
 	return 0;
 }
 
+static void isst_print_extended_platform_info(void)
+{
+	int cp_state, cp_cap, fact_support = 0, pbf_support = 0;
+	struct isst_pkg_ctdp_level_info ctdp_level;
+	struct isst_pkg_ctdp pkg_dev;
+	int ret, i, j;
+	FILE *filep;
+
+	for (i = 0; i < 256; ++i) {
+		char path[256];
+
+		snprintf(path, sizeof(path),
+			 "/sys/devices/system/cpu/cpu%d/topology/thread_siblings", i);
+		filep = fopen(path, "r");
+		if (filep)
+			break;
+	}
+
+	if (!filep)
+		return;
+
+	fclose(filep);
+
+	ret = isst_get_ctdp_levels(i, &pkg_dev);
+	if (ret)
+		return;
+
+	if (pkg_dev.enabled) {
+		fprintf(outf, "Intel(R) SST-PP (feature perf-profile) is supported\n");
+	} else {
+		fprintf(outf, "Intel(R) SST-PP (feature perf-profile) is not supported\n");
+		fprintf(outf, "Only performance level 0 (base level) is present\n");
+	}
+
+	if (pkg_dev.locked)
+		fprintf(outf, "TDP level change control is locked\n");
+	else
+		fprintf(outf, "TDP level change control is unlocked, max level: %d \n", pkg_dev.levels);
+
+	for (j = 0; j <= pkg_dev.levels; ++j) {
+		ret = isst_get_ctdp_control(i, j, &ctdp_level);
+		if (ret)
+			continue;
+
+		if (!fact_support && ctdp_level.fact_support)
+			fact_support = 1;
+
+		if (!pbf_support && ctdp_level.pbf_support)
+			pbf_support = 1;
+	}
+
+	if (fact_support)
+		fprintf(outf, "Intel(R) SST-TF (feature turbo-freq) is supported\n");
+	else
+		fprintf(outf, "Intel(R) SST-TF (feature turbo-freq) is not supported\n");
+
+	if (pbf_support)
+		fprintf(outf, "Intel(R) SST-BF (feature base-freq) is supported\n");
+	else
+		fprintf(outf, "Intel(R) SST-BF (feature base-freq) is not supported\n");
+
+	ret = isst_read_pm_config(i, &cp_state, &cp_cap);
+	if (cp_cap)
+		fprintf(outf, "Intel(R) SST-CP (feature core-power) is supported\n");
+	else
+		fprintf(outf, "Intel(R) SST-CP (feature core-power) is not supported\n");
+}
+
 static void isst_print_platform_information(void)
 {
 	struct isst_if_platform_info platform_info;
 	const char *pathname = "/dev/isst_interface";
 	int fd;
 
+	if (is_clx_n_platform()) {
+		fprintf(stderr, "\nThis option in not supported on this platform\n");
+		exit(0);
+	}
+
 	fd = open(pathname, O_RDWR);
 	if (fd < 0)
 		err(-1, "%s open failed", pathname);
@@ -718,6 +966,7 @@ static void isst_print_platform_information(void)
 			platform_info.mbox_supported);
 		fprintf(outf, "Platform: mmio supported : %d\n",
 			platform_info.mmio_supported);
+		isst_print_extended_platform_info();
 	}
 
 	close(fd);
@@ -725,6 +974,7 @@ static void isst_print_platform_information(void)
 	exit(0);
 }
 
+static char *local_str0, *local_str1;
 static void exec_on_get_ctdp_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 				 void *arg4)
 {
@@ -734,13 +984,14 @@ static void exec_on_get_ctdp_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	fn_ptr = arg1;
 	ret = fn_ptr(cpu, arg2);
 	if (ret)
-		perror("get_tdp_*");
+		isst_display_error_info_message(1, "get_tdp_* failed", 0, 0);
 	else
 		isst_ctdp_display_core_info(cpu, outf, arg3,
-					    *(unsigned int *)arg4);
+					    *(unsigned int *)arg4,
+					    local_str0, local_str1);
 }
 
-#define _get_tdp_level(desc, suffix, object, help)                                \
+#define _get_tdp_level(desc, suffix, object, help, str0, str1)			\
 	static void get_tdp_##object(int arg)                                    \
 	{                                                                         \
 		struct isst_pkg_ctdp ctdp;                                        \
@@ -751,6 +1002,8 @@ static void exec_on_get_ctdp_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 				help);                                            \
 			exit(0);                                                  \
 		}                                                                 \
+		local_str0 = str0;						  \
+		local_str1 = str1;						  \
 		isst_ctdp_display_information_start(outf);                        \
 		if (max_target_cpus)                                              \
 			for_each_online_target_cpu_in_set(                        \
@@ -764,12 +1017,12 @@ static void exec_on_get_ctdp_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 		isst_ctdp_display_information_end(outf);                          \
 	}
 
-_get_tdp_level("get-config-levels", levels, levels, "TDP levels");
-_get_tdp_level("get-config-version", levels, version, "TDP version");
-_get_tdp_level("get-config-enabled", levels, enabled, "TDP enable status");
+_get_tdp_level("get-config-levels", levels, levels, "Max TDP level", NULL, NULL);
+_get_tdp_level("get-config-version", levels, version, "TDP version", NULL, NULL);
+_get_tdp_level("get-config-enabled", levels, enabled, "perf-profile enable status", "disabled", "enabled");
 _get_tdp_level("get-config-current_level", levels, current_level,
-	       "Current TDP Level");
-_get_tdp_level("get-lock-status", levels, locked, "TDP lock status");
+	       "Current TDP Level", NULL, NULL);
+_get_tdp_level("get-lock-status", levels, locked, "TDP lock status", "unlocked", "locked");
 
 struct isst_pkg_ctdp clx_n_pkg_dev;
 
@@ -902,9 +1155,14 @@ static void dump_clx_n_config_for_cpu(int cpu, void *arg1, void *arg2,
 {
 	int ret;
 
+	if (tdp_level != 0xff && tdp_level != 0) {
+		isst_display_error_info_message(1, "Invalid level", 1, tdp_level);
+		exit(0);
+	}
+
 	ret = clx_n_config(cpu);
 	if (ret) {
-		perror("isst_get_process_ctdp");
+		debug_printf("clx_n_config failed");
 	} else {
 		struct isst_pkg_ctdp_level_info *ctdp_level;
 		struct isst_pbf_info *pbf_info;
@@ -926,7 +1184,9 @@ static void dump_isst_config_for_cpu(int cpu, void *arg1, void *arg2,
 	memset(&pkg_dev, 0, sizeof(pkg_dev));
 	ret = isst_get_process_ctdp(cpu, tdp_level, &pkg_dev);
 	if (ret) {
-		perror("isst_get_process_ctdp");
+		isst_display_error_info_message(1, "Failed to get perf-profile info on cpu", 1, cpu);
+		isst_ctdp_display_information_end(outf);
+		exit(1);
 	} else {
 		isst_ctdp_display_information(cpu, outf, tdp_level, &pkg_dev);
 		isst_get_process_ctdp_complete(cpu, &pkg_dev);
@@ -969,9 +1229,11 @@ static void set_tdp_level_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	int ret;
 
 	ret = isst_set_tdp_level(cpu, tdp_level);
-	if (ret)
-		perror("set_tdp_level_for_cpu");
-	else {
+	if (ret) {
+		isst_display_error_info_message(1, "Set TDP level failed", 0, 0);
+		isst_ctdp_display_information_end(outf);
+		exit(1);
+	} else {
 		isst_display_result(cpu, outf, "perf-profile", "set_tdp_level",
 				    ret);
 		if (force_online_offline) {
@@ -1009,11 +1271,13 @@ static void set_tdp_level(int arg)
 			"\t Arguments: -l|--level : Specify tdp level\n");
 		fprintf(stderr,
 			"\t Optional Arguments: -o | online : online/offline for the tdp level\n");
+		fprintf(stderr,
+			"\t  online/offline operation has limitations, refer to Linux hotplug documentation\n");
 		exit(0);
 	}
 
 	if (tdp_level == 0xff) {
-		fprintf(outf, "Invalid command: specify tdp_level\n");
+		isst_display_error_info_message(1, "Invalid command: specify tdp_level", 0, 0);
 		exit(1);
 	}
 	isst_ctdp_display_information_start(outf);
@@ -1033,7 +1297,7 @@ static void clx_n_dump_pbf_config_for_cpu(int cpu, void *arg1, void *arg2,
 
 	ret = clx_n_config(cpu);
 	if (ret) {
-		perror("isst_get_process_ctdp");
+		isst_display_error_info_message(1, "clx_n_config failed", 0, 0);
 	} else {
 		struct isst_pkg_ctdp_level_info *ctdp_level;
 		struct isst_pbf_info *pbf_info;
@@ -1054,7 +1318,9 @@ static void dump_pbf_config_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 
 	ret = isst_get_pbf_info(cpu, tdp_level, &pbf_info);
 	if (ret) {
-		perror("isst_get_pbf_info");
+		isst_display_error_info_message(1, "Failed to get base-freq info at this level", 1, tdp_level);
+		isst_ctdp_display_information_end(outf);
+		exit(1);
 	} else {
 		isst_pbf_display_information(cpu, outf, tdp_level, &pbf_info);
 		isst_get_pbf_info_complete(&pbf_info);
@@ -1074,7 +1340,7 @@ static void dump_pbf_config(int arg)
 	}
 
 	if (tdp_level == 0xff) {
-		fprintf(outf, "Invalid command: specify tdp_level\n");
+		isst_display_error_info_message(1, "Invalid command: specify tdp_level", 0, 0);
 		exit(1);
 	}
 
@@ -1100,7 +1366,7 @@ static int set_clos_param(int cpu, int clos, int epp, int wt, int min, int max)
 
 	ret = isst_pm_get_clos(cpu, clos, &clos_config);
 	if (ret) {
-		perror("isst_pm_get_clos");
+		isst_display_error_info_message(1, "isst_pm_get_clos failed", 0, 0);
 		return ret;
 	}
 	clos_config.clos_min = min;
@@ -1109,7 +1375,7 @@ static int set_clos_param(int cpu, int clos, int epp, int wt, int min, int max)
 	clos_config.clos_prop_prio = wt;
 	ret = isst_set_clos(cpu, clos, &clos_config);
 	if (ret) {
-		perror("isst_pm_set_clos");
+		isst_display_error_info_message(1, "isst_set_clos failed", 0, 0);
 		return ret;
 	}
 
@@ -1153,7 +1419,7 @@ static int set_clx_pbf_cpufreq_scaling_min_max(int cpu)
 
 	ret = clx_n_config(cpu);
 	if (ret) {
-		perror("set_clx_pbf_cpufreq_scaling_min_max");
+		debug_printf("cpufreq_scaling_min_max failed for CLX");
 		return ret;
 	}
 
@@ -1316,7 +1582,7 @@ static int set_core_priority_and_min(int cpu, int mask_size,
 		debug_printf("Associate cpu: %d clos: %d\n", i, clos);
 		ret = isst_clos_associate(i, clos);
 		if (ret) {
-			perror("isst_clos_associate");
+			isst_display_error_info_message(1, "isst_clos_associate failed", 0, 0);
 			return ret;
 		}
 	}
@@ -1332,14 +1598,14 @@ static int set_pbf_core_power(int cpu)
 
 	ret = isst_get_ctdp_levels(cpu, &pkg_dev);
 	if (ret) {
-		perror("isst_get_ctdp_levels");
+		debug_printf("isst_get_ctdp_levels failed");
 		return ret;
 	}
 	debug_printf("Current_level: %d\n", pkg_dev.current_level);
 
 	ret = isst_get_pbf_info(cpu, pkg_dev.current_level, &pbf_info);
 	if (ret) {
-		perror("isst_get_pbf_info");
+		debug_printf("isst_get_pbf_info failed");
 		return ret;
 	}
 	debug_printf("p1_high: %d p1_low: %d\n", pbf_info.p1_high,
@@ -1349,13 +1615,13 @@ static int set_pbf_core_power(int cpu)
 					pbf_info.core_cpumask,
 					pbf_info.p1_high, pbf_info.p1_low);
 	if (ret) {
-		perror("set_core_priority_and_min");
+		debug_printf("set_core_priority_and_min failed");
 		return ret;
 	}
 
 	ret = isst_pm_qos_config(cpu, 1, 1);
 	if (ret) {
-		perror("isst_pm_qos_config");
+		debug_printf("isst_pm_qos_config failed");
 		return ret;
 	}
 
@@ -1369,17 +1635,13 @@ static void set_pbf_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	int status = *(int *)arg4;
 
 	if (is_clx_n_platform()) {
+		ret = 0;
 		if (status) {
-			ret = 0;
-			if (auto_mode)
-				set_clx_pbf_cpufreq_scaling_min_max(cpu);
+			set_clx_pbf_cpufreq_scaling_min_max(cpu);
 
 		} else {
-			ret = -1;
-			if (auto_mode) {
-				set_scaling_max_to_cpuinfo_max(cpu);
-				set_scaling_min_to_cpuinfo_min(cpu);
-			}
+			set_scaling_max_to_cpuinfo_max(cpu);
+			set_scaling_min_to_cpuinfo_min(cpu);
 		}
 		goto disp_result;
 	}
@@ -1392,7 +1654,7 @@ static void set_pbf_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 
 	ret = isst_set_pbf_fact_status(cpu, 1, status);
 	if (ret) {
-		perror("isst_set_pbf");
+		debug_printf("isst_set_pbf_fact_status failed");
 		if (auto_mode)
 			isst_pm_qos_config(cpu, 0, 0);
 	} else {
@@ -1405,7 +1667,7 @@ static void set_pbf_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	}
 
 	if (auto_mode && !status)
-		isst_pm_qos_config(cpu, 0, 0);
+		isst_pm_qos_config(cpu, 0, 1);
 
 disp_result:
 	if (status)
@@ -1424,10 +1686,25 @@ static void set_pbf_enable(int arg)
 		if (enable) {
 			fprintf(stderr,
 				"Enable Intel Speed Select Technology base frequency feature\n");
+			if (is_clx_n_platform()) {
+				fprintf(stderr,
+					"\tOn this platform this command doesn't enable feature in the hardware.\n");
+				fprintf(stderr,
+					"\tIt updates the cpufreq scaling_min_freq to match cpufreq base_frequency.\n");
+				exit(0);
+
+			}
 			fprintf(stderr,
 				"\tOptional Arguments: -a|--auto : Use priority of cores to set core-power associations\n");
 		} else {
 
+			if (is_clx_n_platform()) {
+				fprintf(stderr,
+					"\tOn this platform this command doesn't disable feature in the hardware.\n");
+				fprintf(stderr,
+					"\tIt updates the cpufreq scaling_min_freq to match cpuinfo_min_freq\n");
+				exit(0);
+			}
 			fprintf(stderr,
 				"Disable Intel Speed Select Technology base frequency feature\n");
 			fprintf(stderr,
@@ -1452,12 +1729,15 @@ static void dump_fact_config_for_cpu(int cpu, void *arg1, void *arg2,
 	struct isst_fact_info fact_info;
 	int ret;
 
-	ret = isst_get_fact_info(cpu, tdp_level, &fact_info);
-	if (ret)
-		perror("isst_get_fact_bucket_info");
-	else
+	ret = isst_get_fact_info(cpu, tdp_level, fact_bucket, &fact_info);
+	if (ret) {
+		isst_display_error_info_message(1, "Failed to get turbo-freq info at this level", 1, tdp_level);
+		isst_ctdp_display_information_end(outf);
+		exit(1);
+	} else {
 		isst_fact_display_information(cpu, outf, tdp_level, fact_bucket,
 					      fact_avx, &fact_info);
+	}
 }
 
 static void dump_fact_config(int arg)
@@ -1475,7 +1755,7 @@ static void dump_fact_config(int arg)
 	}
 
 	if (tdp_level == 0xff) {
-		fprintf(outf, "Invalid command: specify tdp_level\n");
+		isst_display_error_info_message(1, "Invalid command: specify tdp_level\n", 0, 0);
 		exit(1);
 	}
 
@@ -1503,7 +1783,7 @@ static void set_fact_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 
 	ret = isst_set_pbf_fact_status(cpu, 0, status);
 	if (ret) {
-		perror("isst_set_fact");
+		debug_printf("isst_set_pbf_fact_status failed");
 		if (auto_mode)
 			isst_pm_qos_config(cpu, 0, 0);
 
@@ -1527,6 +1807,8 @@ static void set_fact_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 disp_results:
 	if (status) {
 		isst_display_result(cpu, outf, "turbo-freq", "enable", ret);
+		if (ret)
+			fact_enable_fail = ret;
 	} else {
 		/* Since we modified TRL during Fact enable, restore it */
 		isst_set_trl_from_current_tdp(cpu, fact_trl);
@@ -1568,7 +1850,7 @@ static void set_fact_enable(int arg)
 					       NULL, &enable);
 	isst_ctdp_display_information_end(outf);
 
-	if (enable && auto_mode) {
+	if (!fact_enable_fail && enable && auto_mode) {
 		/*
 		 * When we adjust CLOS param, we have to set for siblings also.
 		 * So for the each user specified CPU, also add the sibling
@@ -1652,9 +1934,12 @@ static void enable_clos_qos_config(int cpu, void *arg1, void *arg2, void *arg3,
 	int ret;
 	int status = *(int *)arg4;
 
+	if (is_skx_based_platform())
+		clos_priority_type = 1;
+
 	ret = isst_pm_qos_config(cpu, status, clos_priority_type);
 	if (ret)
-		perror("isst_pm_qos_config");
+		isst_display_error_info_message(1, "isst_pm_qos_config failed", 0, 0);
 
 	if (status)
 		isst_display_result(cpu, outf, "core-power", "enable",
@@ -1672,9 +1957,11 @@ static void set_clos_enable(int arg)
 		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");
+			if (!is_skx_based_platform()) {
+				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");
@@ -1705,7 +1992,7 @@ static void dump_clos_config_for_cpu(int cpu, void *arg1, void *arg2,
 
 	ret = isst_pm_get_clos(cpu, current_clos, &clos_config);
 	if (ret)
-		perror("isst_pm_get_clos");
+		isst_display_error_info_message(1, "isst_pm_get_clos failed", 0, 0);
 	else
 		isst_clos_display_information(cpu, outf, current_clos,
 					      &clos_config);
@@ -1721,7 +2008,8 @@ static void dump_clos_config(int arg)
 		exit(0);
 	}
 	if (current_clos < 0 || current_clos > 3) {
-		fprintf(stderr, "Invalid clos id\n");
+		isst_display_error_info_message(1, "Invalid clos id\n", 0, 0);
+		isst_ctdp_display_information_end(outf);
 		exit(0);
 	}
 
@@ -1742,9 +2030,14 @@ static void get_clos_info_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 
 	ret = isst_clos_get_clos_information(cpu, &enable, &prio_type);
 	if (ret)
-		perror("isst_clos_get_info");
-	else
-		isst_clos_display_clos_information(cpu, outf, enable, prio_type);
+		isst_display_error_info_message(1, "isst_clos_get_info failed", 0, 0);
+	else {
+		int cp_state, cp_cap;
+
+		isst_read_pm_config(cpu, &cp_state, &cp_cap);
+		isst_clos_display_clos_information(cpu, outf, enable, prio_type,
+						   cp_state, cp_cap);
+	}
 }
 
 static void dump_clos_info(int arg)
@@ -1752,19 +2045,17 @@ static void dump_clos_info(int arg)
 	if (cmd_help) {
 		fprintf(stderr,
 			"Print Intel Speed Select Technology core power information\n");
-		fprintf(stderr, "\tSpecify targeted cpu id with [--cpu|-c]\n");
-		exit(0);
-	}
-
-	if (!max_target_cpus) {
-		fprintf(stderr,
-			"Invalid target cpu. Specify with [-c|--cpu]\n");
+		fprintf(stderr, "\t Optionally specify targeted cpu id with [--cpu|-c]\n");
 		exit(0);
 	}
 
 	isst_ctdp_display_information_start(outf);
-	for_each_online_target_cpu_in_set(get_clos_info_for_cpu, NULL,
-					  NULL, NULL, NULL);
+	if (max_target_cpus)
+		for_each_online_target_cpu_in_set(get_clos_info_for_cpu, NULL,
+						  NULL, NULL, NULL);
+	else
+		for_each_online_package_in_set(get_clos_info_for_cpu, NULL,
+					       NULL, NULL, NULL);
 	isst_ctdp_display_information_end(outf);
 
 }
@@ -1785,7 +2076,7 @@ static void set_clos_config_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 	clos_config.clos_desired = clos_desired;
 	ret = isst_set_clos(cpu, current_clos, &clos_config);
 	if (ret)
-		perror("isst_set_clos");
+		isst_display_error_info_message(1, "isst_set_clos failed", 0, 0);
 	else
 		isst_display_result(cpu, outf, "core-power", "config", ret);
 }
@@ -1797,26 +2088,27 @@ static void set_clos_config(int arg)
 			"Set core-power configuration for one of the four clos ids\n");
 		fprintf(stderr,
 			"\tSpecify targeted clos id with [--clos|-c]\n");
-		fprintf(stderr, "\tSpecify clos EPP with [--epp|-e]\n");
-		fprintf(stderr,
-			"\tSpecify clos Proportional Priority [--weight|-w]\n");
+		if (!is_skx_based_platform()) {
+			fprintf(stderr, "\tSpecify clos EPP with [--epp|-e]\n");
+			fprintf(stderr,
+				"\tSpecify clos Proportional Priority [--weight|-w]\n");
+		}
 		fprintf(stderr, "\tSpecify clos min in MHz with [--min|-n]\n");
 		fprintf(stderr, "\tSpecify clos max in MHz with [--max|-m]\n");
-		fprintf(stderr, "\tSpecify clos desired in MHz with [--desired|-d]\n");
 		exit(0);
 	}
 
 	if (current_clos < 0 || current_clos > 3) {
-		fprintf(stderr, "Invalid clos id\n");
+		isst_display_error_info_message(1, "Invalid clos id\n", 0, 0);
 		exit(0);
 	}
-	if (clos_epp < 0 || clos_epp > 0x0F) {
-		fprintf(stderr, "clos epp is not specified, default: 0\n");
+	if (!is_skx_based_platform() && (clos_epp < 0 || clos_epp > 0x0F)) {
+		fprintf(stderr, "clos epp is not specified or invalid, default: 0\n");
 		clos_epp = 0;
 	}
-	if (clos_prop_prio < 0 || clos_prop_prio > 0x0F) {
+	if (!is_skx_based_platform() && (clos_prop_prio < 0 || clos_prop_prio > 0x0F)) {
 		fprintf(stderr,
-			"clos frequency weight is not specified, default: 0\n");
+			"clos frequency weight is not specified or invalid, default: 0\n");
 		clos_prop_prio = 0;
 	}
 	if (clos_min < 0) {
@@ -1824,11 +2116,11 @@ static void set_clos_config(int arg)
 		clos_min = 0;
 	}
 	if (clos_max < 0) {
-		fprintf(stderr, "clos max is not specified, default: 25500 MHz\n");
+		fprintf(stderr, "clos max is not specified, default: Max frequency (ratio 0xff)\n");
 		clos_max = 0xff;
 	}
-	if (clos_desired < 0) {
-		fprintf(stderr, "clos desired is not specified, default: 0\n");
+	if (clos_desired) {
+		fprintf(stderr, "clos desired is not supported on this platform\n");
 		clos_desired = 0x00;
 	}
 
@@ -1849,7 +2141,7 @@ static void set_clos_assoc_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 
 	ret = isst_clos_associate(cpu, current_clos);
 	if (ret)
-		perror("isst_clos_associate");
+		debug_printf("isst_clos_associate failed");
 	else
 		isst_display_result(cpu, outf, "core-power", "assoc", ret);
 }
@@ -1860,19 +2152,22 @@ static void set_clos_assoc(int arg)
 		fprintf(stderr, "Associate a clos id to a CPU\n");
 		fprintf(stderr,
 			"\tSpecify targeted clos id with [--clos|-c]\n");
+		fprintf(stderr,
+			"\tFor example to associate clos 1 to CPU 0: issue\n");
+		fprintf(stderr,
+			"\tintel-speed-select --cpu 0 core-power assoc --clos 1\n");
 		exit(0);
 	}
 
 	if (current_clos < 0 || current_clos > 3) {
-		fprintf(stderr, "Invalid clos id\n");
+		isst_display_error_info_message(1, "Invalid clos id\n", 0, 0);
 		exit(0);
 	}
 	if (max_target_cpus)
 		for_each_online_target_cpu_in_set(set_clos_assoc_for_cpu, NULL,
 						  NULL, NULL, NULL);
 	else {
-		fprintf(stderr,
-			"Invalid target cpu. Specify with [-c|--cpu]\n");
+		isst_display_error_info_message(1, "Invalid target cpu. Specify with [-c|--cpu]", 0, 0);
 	}
 }
 
@@ -1883,7 +2178,7 @@ static void get_clos_assoc_for_cpu(int cpu, void *arg1, void *arg2, void *arg3,
 
 	ret = isst_clos_get_assoc_status(cpu, &clos);
 	if (ret)
-		perror("isst_clos_get_assoc_status");
+		isst_display_error_info_message(1, "isst_clos_get_assoc_status failed", 0, 0);
 	else
 		isst_clos_display_assoc_information(cpu, outf, clos);
 }
@@ -1897,8 +2192,7 @@ static void get_clos_assoc(int arg)
 	}
 
 	if (!max_target_cpus) {
-		fprintf(stderr,
-			"Invalid target cpu. Specify with [-c|--cpu]\n");
+		isst_display_error_info_message(1, "Invalid target cpu. Specify with [-c|--cpu]", 0, 0);
 		exit(0);
 	}
 
@@ -2035,7 +2329,7 @@ static void parse_cmd_args(int argc, int start, char **argv)
 	option_index = start;
 
 	optind = start + 1;
-	while ((opt = getopt_long(argc, argv, "b:l:t:c:d:e:n:m:p:w:hoa",
+	while ((opt = getopt_long(argc, argv, "b:l:t:c:d:e:n:m:p:w:r:hoa",
 				  long_options, &option_index)) != -1) {
 		switch (opt) {
 		case 'a':
@@ -2061,7 +2355,7 @@ static void parse_cmd_args(int argc, int start, char **argv)
 				fact_avx = 0x01;
 			} else if (!strncmp(optarg, "avx2", 4)) {
 				fact_avx = 0x02;
-			} else if (!strncmp(optarg, "avx512", 4)) {
+			} else if (!strncmp(optarg, "avx512", 6)) {
 				fact_avx = 0x04;
 			} else {
 				fprintf(outf, "Invalid sse,avx options\n");
@@ -2078,6 +2372,10 @@ static void parse_cmd_args(int argc, int start, char **argv)
 			break;
 		case 'e':
 			clos_epp = atoi(optarg);
+			if (is_skx_based_platform()) {
+				isst_display_error_info_message(1, "epp can't be specified on this platform", 0, 0);
+				exit(0);
+			}
 			break;
 		case 'n':
 			clos_min = atoi(optarg);
@@ -2089,14 +2387,25 @@ static void parse_cmd_args(int argc, int start, char **argv)
 			break;
 		case 'p':
 			clos_priority_type = atoi(optarg);
+			if (is_skx_based_platform() && !clos_priority_type) {
+				isst_display_error_info_message(1, "Invalid clos priority type: proportional for this platform", 0, 0);
+				exit(0);
+			}
 			break;
 		case 'w':
 			clos_prop_prio = atoi(optarg);
+			if (is_skx_based_platform()) {
+				isst_display_error_info_message(1, "weight can't be specified on this platform", 0, 0);
+				exit(0);
+			}
 			break;
 		default:
-			printf("no match\n");
+			printf("Unknown option: ignore\n");
 		}
 	}
+
+	if (argv[optind])
+		printf("Garbage at the end of command: ignore\n");
 }
 
 static void isst_help(void)
@@ -2214,11 +2523,18 @@ void process_command(int argc, char **argv,
 
 static void usage(void)
 {
-	printf("Intel(R) Speed Select Technology\n");
+	if (is_clx_n_platform()) {
+		fprintf(stderr, "\nThere is limited support of Intel Speed Select features on this platform.\n");
+		fprintf(stderr, "Everything is pre-configured using BIOS options, this tool can't enable any feature in the hardware.\n\n");
+	}
+
 	printf("\nUsage:\n");
 	printf("intel-speed-select [OPTIONS] FEATURE COMMAND COMMAND_ARGUMENTS\n");
-	printf("\nUse this tool to enumerate and control the Intel Speed Select Technology features,\n");
-	printf("\nFEATURE : [perf-profile|base-freq|turbo-freq|core-power]\n");
+	printf("\nUse this tool to enumerate and control the Intel Speed Select Technology features:\n");
+	if (is_clx_n_platform())
+		printf("\nFEATURE : [perf-profile|base-freq]\n");
+	else
+		printf("\nFEATURE : [perf-profile|base-freq|turbo-freq|core-power]\n");
 	printf("\nFor help on each feature, use -h|--help\n");
 	printf("\tFor example:  intel-speed-select perf-profile -h\n");
 
@@ -2231,17 +2547,29 @@ static void usage(void)
 	printf("\t\tDefault: Die scoped for all dies in the system with multiple dies/package\n");
 	printf("\t\t\t Or Package scoped for all Packages when each package contains one die\n");
 	printf("\t[-d|--debug] : Debug mode\n");
+	printf("\t[-f|--format] : output format [json|text]. Default: text\n");
 	printf("\t[-h|--help] : Print help\n");
 	printf("\t[-i|--info] : Print platform information\n");
 	printf("\t[-o|--out] : Output file\n");
 	printf("\t\t\tDefault : stderr\n");
-	printf("\t[-f|--format] : output format [json|text]. Default: text\n");
 	printf("\t[-v|--version] : Print version\n");
 
 	printf("\nResult format\n");
 	printf("\tResult display uses a common format for each command:\n");
 	printf("\tResults are formatted in text/JSON with\n");
 	printf("\t\tPackage, Die, CPU, and command specific results.\n");
+
+	printf("\nExamples\n");
+	printf("\tTo get platform information:\n");
+	printf("\t\tintel-speed-select --info\n");
+	printf("\tTo get full perf-profile information dump:\n");
+	printf("\t\tintel-speed-select perf-profile info\n");
+	printf("\tTo get full base-freq information dump:\n");
+	printf("\t\tintel-speed-select base-freq info -l 0\n");
+	if (!is_clx_n_platform()) {
+		printf("\tTo get full turbo-freq information dump:\n");
+		printf("\t\tintel-speed-select turbo-freq info -l 0\n");
+	}
 	exit(1);
 }
 
@@ -2254,6 +2582,8 @@ static void print_version(void)
 
 static void cmdline(int argc, char **argv)
 {
+	const char *pathname = "/dev/isst_interface";
+	FILE *fp;
 	int opt;
 	int option_index = 0;
 	int ret;
@@ -2269,6 +2599,28 @@ static void cmdline(int argc, char **argv)
 		{ 0, 0, 0, 0 }
 	};
 
+	if (geteuid() != 0) {
+		fprintf(stderr, "Must run as root\n");
+		exit(0);
+	}
+
+	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);
+
+	if (!is_clx_n_platform()) {
+		fp = fopen(pathname, "rb");
+		if (!fp) {
+			fprintf(stderr, "Intel speed select drivers are not loaded on this system.\n");
+			fprintf(stderr, "Verify that kernel config includes CONFIG_INTEL_SPEED_SELECT_INTERFACE.\n");
+			fprintf(stderr, "If the config is included then this is not a supported platform.\n");
+			exit(0);
+		}
+		fclose(fp);
+	}
+
 	progname = argv[0];
 	while ((opt = getopt_long_only(argc, argv, "+c:df:hio:v", long_options,
 				       &option_index)) != -1) {
@@ -2303,21 +2655,12 @@ static void cmdline(int argc, char **argv)
 		}
 	}
 
-	if (geteuid() != 0) {
-		fprintf(stderr, "Must run as root\n");
-		exit(0);
-	}
-
 	if (optind > (argc - 2)) {
-		fprintf(stderr, "Feature name and|or command not specified\n");
+		usage();
 		exit(0);
 	}
-	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();
+	store_cpu_topology();
 	set_cpu_present_cpu_mask();
 	set_cpu_target_cpu_mask();
 
diff --git a/tools/power/x86/intel-speed-select/isst-core.c b/tools/power/x86/intel-speed-select/isst-core.c
index 81a119f688a3..67c9b1139631 100644
--- a/tools/power/x86/intel-speed-select/isst-core.c
+++ b/tools/power/x86/intel-speed-select/isst-core.c
@@ -114,8 +114,10 @@ int isst_get_tdp_info(int cpu, int config_index,
 
 	ret = isst_send_mbox_command(cpu, CONFIG_TDP, CONFIG_TDP_GET_TDP_INFO,
 				     0, config_index, &resp);
-	if (ret)
+	if (ret) {
+		isst_display_error_info_message(1, "Invalid level, Can't get TDP information at level", 1, config_index);
 		return ret;
+	}
 
 	ctdp_level->pkg_tdp = resp & GENMASK(14, 0);
 	ctdp_level->tdp_ratio = (resp & GENMASK(23, 16)) >> 16;
@@ -352,7 +354,7 @@ int isst_set_tdp_level_msr(int cpu, int tdp_level)
 	debug_printf("cpu: tdp_level via MSR %d\n", cpu, tdp_level);
 
 	if (isst_get_config_tdp_lock_status(cpu)) {
-		debug_printf("cpu: tdp_locked %d\n", cpu);
+		isst_display_error_info_message(1, "tdp_locked", 0, 0);
 		return -1;
 	}
 
@@ -373,19 +375,50 @@ int isst_set_tdp_level(int cpu, int tdp_level)
 	unsigned int resp;
 	int ret;
 
+
+	if (isst_get_config_tdp_lock_status(cpu)) {
+		isst_display_error_info_message(1, "TDP is locked", 0, 0);
+		return -1;
+
+	}
+
 	ret = isst_send_mbox_command(cpu, CONFIG_TDP, CONFIG_TDP_SET_LEVEL, 0,
 				     tdp_level, &resp);
-	if (ret)
-		return isst_set_tdp_level_msr(cpu, tdp_level);
+	if (ret) {
+		isst_display_error_info_message(1, "Set TDP level failed for level", 1, tdp_level);
+		return ret;
+	}
 
 	return 0;
 }
 
 int isst_get_pbf_info(int cpu, int level, struct isst_pbf_info *pbf_info)
 {
+	struct isst_pkg_ctdp_level_info ctdp_level;
+	struct isst_pkg_ctdp pkg_dev;
 	int i, ret, core_cnt, max;
 	unsigned int req, resp;
 
+	ret = isst_get_ctdp_levels(cpu, &pkg_dev);
+	if (ret) {
+		isst_display_error_info_message(1, "Failed to get number of levels", 0, 0);
+		return ret;
+	}
+
+	if (level > pkg_dev.levels) {
+		isst_display_error_info_message(1, "Invalid level", 1, level);
+		return -1;
+	}
+
+	ret = isst_get_ctdp_control(cpu, level, &ctdp_level);
+	if (ret)
+		return ret;
+
+	if (!ctdp_level.pbf_support) {
+		isst_display_error_info_message(1, "base-freq feature is not present at this level", 1, level);
+		return -1;
+	}
+
 	pbf_info->core_cpumask_size = alloc_cpu_set(&pbf_info->core_cpumask);
 
 	core_cnt = get_core_count(get_physical_package_id(cpu), get_physical_die_id(cpu));
@@ -481,6 +514,10 @@ int isst_set_pbf_fact_status(int cpu, int pbf, int enable)
 		else
 			req &= ~BIT(17);
 	} else {
+
+		if (enable && !ctdp_level.sst_cp_enabled)
+			isst_display_error_info_message(0, "Make sure to execute before: core-power enable", 0, 0);
+
 		if (ctdp_level.pbf_enabled)
 			req = BIT(17);
 
@@ -566,10 +603,32 @@ int isst_get_fact_bucket_info(int cpu, int level,
 	return 0;
 }
 
-int isst_get_fact_info(int cpu, int level, struct isst_fact_info *fact_info)
+int isst_get_fact_info(int cpu, int level, int fact_bucket, struct isst_fact_info *fact_info)
 {
+	struct isst_pkg_ctdp_level_info ctdp_level;
+	struct isst_pkg_ctdp pkg_dev;
 	unsigned int resp;
-	int ret;
+	int j, ret, print;
+
+	ret = isst_get_ctdp_levels(cpu, &pkg_dev);
+	if (ret) {
+		isst_display_error_info_message(1, "Failed to get number of levels", 0, 0);
+		return ret;
+	}
+
+	if (level > pkg_dev.levels) {
+		isst_display_error_info_message(1, "Invalid level", 1, level);
+		return -1;
+	}
+
+	ret = isst_get_ctdp_control(cpu, level, &ctdp_level);
+	if (ret)
+		return ret;
+
+	if (!ctdp_level.fact_support) {
+		isst_display_error_info_message(1, "turbo-freq feature is not present at this level", 1, level);
+		return -1;
+	}
 
 	ret = isst_send_mbox_command(cpu, CONFIG_TDP,
 				     CONFIG_TDP_GET_FACT_LP_CLIPPING_RATIO, 0,
@@ -585,8 +644,25 @@ int isst_get_fact_info(int cpu, int level, struct isst_fact_info *fact_info)
 	fact_info->lp_clipping_ratio_license_avx512 = (resp >> 16) & 0xff;
 
 	ret = isst_get_fact_bucket_info(cpu, level, fact_info->bucket_info);
+	if (ret)
+		return ret;
 
-	return ret;
+	print = 0;
+	for (j = 0; j < ISST_FACT_MAX_BUCKETS; ++j) {
+		if (fact_bucket != 0xff && fact_bucket != j)
+			continue;
+
+		if (!fact_info->bucket_info[j].high_priority_cores_count)
+			break;
+
+		print = 1;
+	}
+	if (!print) {
+		isst_display_error_info_message(1, "Invalid bucket", 0, 0);
+		return -1;
+	}
+
+	return 0;
 }
 
 int isst_set_trl(int cpu, unsigned long long trl)
@@ -671,7 +747,7 @@ void isst_get_process_ctdp_complete(int cpu, struct isst_pkg_ctdp *pkg_dev)
 
 int isst_get_process_ctdp(int cpu, int tdp_level, struct isst_pkg_ctdp *pkg_dev)
 {
-	int i, ret;
+	int i, ret, valid = 0;
 
 	if (pkg_dev->processed)
 		return 0;
@@ -684,6 +760,14 @@ int isst_get_process_ctdp(int cpu, int tdp_level, struct isst_pkg_ctdp *pkg_dev)
 		     cpu, pkg_dev->enabled, pkg_dev->current_level,
 		     pkg_dev->levels);
 
+	if (tdp_level != 0xff && tdp_level > pkg_dev->levels) {
+		isst_display_error_info_message(1, "Invalid level", 0, 0);
+		return -1;
+	}
+
+	if (!pkg_dev->enabled)
+		isst_display_error_info_message(0, "perf-profile feature is not supported, just base-config level 0 is valid", 0, 0);
+
 	for (i = 0; i <= pkg_dev->levels; ++i) {
 		struct isst_pkg_ctdp_level_info *ctdp_level;
 
@@ -703,6 +787,7 @@ int isst_get_process_ctdp(int cpu, int tdp_level, struct isst_pkg_ctdp *pkg_dev)
 		if (ret)
 			continue;
 
+		valid = 1;
 		pkg_dev->processed = 1;
 		ctdp_level->processed = 1;
 
@@ -713,7 +798,7 @@ int isst_get_process_ctdp(int cpu, int tdp_level, struct isst_pkg_ctdp *pkg_dev)
 		}
 
 		if (ctdp_level->fact_support) {
-			ret = isst_get_fact_info(cpu, i,
+			ret = isst_get_fact_info(cpu, i, 0xff,
 						 &ctdp_level->fact_info);
 			if (ret)
 				return ret;
@@ -775,6 +860,9 @@ int isst_get_process_ctdp(int cpu, int tdp_level, struct isst_pkg_ctdp *pkg_dev)
 		isst_get_uncore_mem_freq(cpu, i, ctdp_level);
 	}
 
+	if (!valid)
+		isst_display_error_info_message(0, "Invalid level, Can't get TDP control information at specified levels on cpu", 1, cpu);
+
 	return 0;
 }
 
@@ -829,17 +917,19 @@ int isst_pm_qos_config(int cpu, int enable_clos, int priority_type)
 		}
 		ret = isst_write_pm_config(cpu, 0);
 		if (ret)
-			perror("isst_write_pm_config\n");
+			isst_display_error_info_message(0, "WRITE_PM_CONFIG command failed, ignoring error\n", 0, 0);
 	} else {
 		ret = isst_write_pm_config(cpu, 1);
 		if (ret)
-			perror("isst_write_pm_config\n");
+			isst_display_error_info_message(0, "WRITE_PM_CONFIG command failed, ignoring error\n", 0, 0);
 	}
 
 	ret = isst_send_mbox_command(cpu, CONFIG_CLOS, CLOS_PM_QOS_CONFIG, 0, 0,
 				     &resp);
-	if (ret)
+	if (ret) {
+		isst_display_error_info_message(1, "CLOS_PM_QOS_CONFIG command failed", 0, 0);
 		return ret;
+	}
 
 	debug_printf("cpu:%d CLOS_PM_QOS_CONFIG resp:%x\n", cpu, resp);
 
@@ -850,6 +940,9 @@ int isst_pm_qos_config(int cpu, int enable_clos, int priority_type)
 	else
 		req = req & ~BIT(1);
 
+	if (priority_type > 1)
+		isst_display_error_info_message(1, "Invalid priority type: Changing type to ordered", 0, 0);
+
 	if (priority_type)
 		req = req | BIT(2);
 	else
diff --git a/tools/power/x86/intel-speed-select/isst-display.c b/tools/power/x86/intel-speed-select/isst-display.c
index 4fb0c1d49d64..51dbaa5f02ec 100644
--- a/tools/power/x86/intel-speed-select/isst-display.c
+++ b/tools/power/x86/intel-speed-select/isst-display.c
@@ -158,10 +158,17 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
 	last_level = level;
 }
 
-static void print_package_info(int cpu, FILE *outf)
+static int print_package_info(int cpu, FILE *outf)
 {
 	char header[256];
 
+	if (out_format_is_json()) {
+		snprintf(header, sizeof(header), "package-%d:die-%d:cpu-%d",
+			 get_physical_package_id(cpu), get_physical_die_id(cpu),
+			 cpu);
+		format_and_print(outf, 1, header, NULL);
+		return 1;
+	}
 	snprintf(header, sizeof(header), "package-%d",
 		 get_physical_package_id(cpu));
 	format_and_print(outf, 1, header, NULL);
@@ -169,6 +176,8 @@ static void print_package_info(int cpu, FILE *outf)
 	format_and_print(outf, 2, header, NULL);
 	snprintf(header, sizeof(header), "cpu-%d", cpu);
 	format_and_print(outf, 3, header, NULL);
+
+	return 3;
 }
 
 static void _isst_pbf_display_information(int cpu, FILE *outf, int level,
@@ -178,7 +187,7 @@ static void _isst_pbf_display_information(int cpu, FILE *outf, int level,
 	char header[256];
 	char value[256];
 
-	snprintf(header, sizeof(header), "speed-select-base-freq");
+	snprintf(header, sizeof(header), "speed-select-base-freq-properties");
 	format_and_print(outf, disp_level, header, NULL);
 
 	snprintf(header, sizeof(header), "high-priority-base-frequency(MHz)");
@@ -222,9 +231,23 @@ static void _isst_fact_display_information(int cpu, FILE *outf, int level,
 	struct isst_fact_bucket_info *bucket_info = fact_info->bucket_info;
 	char header[256];
 	char value[256];
-	int j;
+	int print = 0, j;
+
+	for (j = 0; j < ISST_FACT_MAX_BUCKETS; ++j) {
+		if (fact_bucket != 0xff && fact_bucket != j)
+			continue;
+
+		if (!bucket_info[j].high_priority_cores_count)
+			break;
 
-	snprintf(header, sizeof(header), "speed-select-turbo-freq");
+		print = 1;
+	}
+	if (!print) {
+		fprintf(stderr, "Invalid bucket\n");
+		return;
+	}
+
+	snprintf(header, sizeof(header), "speed-select-turbo-freq-properties");
 	format_and_print(outf, base_level, header, NULL);
 	for (j = 0; j < ISST_FACT_MAX_BUCKETS; ++j) {
 		if (fact_bucket != 0xff && fact_bucket != j)
@@ -289,7 +312,7 @@ static void _isst_fact_display_information(int cpu, FILE *outf, int level,
 }
 
 void isst_ctdp_display_core_info(int cpu, FILE *outf, char *prefix,
-				 unsigned int val)
+				 unsigned int val, char *str0, char *str1)
 {
 	char header[256];
 	char value[256];
@@ -301,8 +324,12 @@ void isst_ctdp_display_core_info(int cpu, FILE *outf, char *prefix,
 	format_and_print(outf, 2, header, NULL);
 	snprintf(header, sizeof(header), "cpu-%d", cpu);
 	format_and_print(outf, 3, header, NULL);
-
-	snprintf(value, sizeof(value), "%u", val);
+	if (str0 && !val)
+		snprintf(value, sizeof(value), "%s", str0);
+	else if (str1 && val)
+		snprintf(value, sizeof(value), "%s", str1);
+	else
+		snprintf(value, sizeof(value), "%u", val);
 	format_and_print(outf, 4, prefix, value);
 
 	format_and_print(outf, 1, NULL, NULL);
@@ -313,10 +340,11 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 {
 	char header[256];
 	char value[256];
-	int i, base_level = 1;
+	static int level;
+	int i;
 
 	if (pkg_dev->processed)
-		print_package_info(cpu, outf);
+		level = print_package_info(cpu, outf);
 
 	for (i = 0; i <= pkg_dev->levels; ++i) {
 		struct isst_pkg_ctdp_level_info *ctdp_level;
@@ -328,72 +356,80 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 
 		snprintf(header, sizeof(header), "perf-profile-level-%d",
 			 ctdp_level->level);
-		format_and_print(outf, base_level + 3, header, NULL);
+		format_and_print(outf, level + 1, header, NULL);
 
 		snprintf(header, sizeof(header), "cpu-count");
 		j = get_cpu_count(get_physical_die_id(cpu),
 				  get_physical_die_id(cpu));
 		snprintf(value, sizeof(value), "%d", j);
-		format_and_print(outf, base_level + 4, header, value);
+		format_and_print(outf, level + 2, header, value);
+
+		j = CPU_COUNT_S(ctdp_level->core_cpumask_size,
+				ctdp_level->core_cpumask);
+		if (j) {
+			snprintf(header, sizeof(header), "enable-cpu-count");
+			snprintf(value, sizeof(value), "%d", j);
+			format_and_print(outf, level + 2, header, value);
+		}
 
 		if (ctdp_level->core_cpumask_size) {
 			snprintf(header, sizeof(header), "enable-cpu-mask");
 			printcpumask(sizeof(value), value,
 				     ctdp_level->core_cpumask_size,
 				     ctdp_level->core_cpumask);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 
 			snprintf(header, sizeof(header), "enable-cpu-list");
 			printcpulist(sizeof(value), value,
 				     ctdp_level->core_cpumask_size,
 				     ctdp_level->core_cpumask);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		snprintf(header, sizeof(header), "thermal-design-power-ratio");
 		snprintf(value, sizeof(value), "%d", ctdp_level->tdp_ratio);
-		format_and_print(outf, base_level + 4, header, value);
+		format_and_print(outf, level + 2, header, value);
 
 		snprintf(header, sizeof(header), "base-frequency(MHz)");
 		if (!ctdp_level->sse_p1)
 			ctdp_level->sse_p1 = ctdp_level->tdp_ratio;
 		snprintf(value, sizeof(value), "%d",
 			  ctdp_level->sse_p1 * DISP_FREQ_MULTIPLIER);
-		format_and_print(outf, base_level + 4, header, value);
+		format_and_print(outf, level + 2, header, value);
 
 		if (ctdp_level->avx2_p1) {
 			snprintf(header, sizeof(header), "base-frequency-avx2(MHz)");
 			snprintf(value, sizeof(value), "%d",
 				 ctdp_level->avx2_p1 * DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		if (ctdp_level->avx512_p1) {
 			snprintf(header, sizeof(header), "base-frequency-avx512(MHz)");
 			snprintf(value, sizeof(value), "%d",
 				 ctdp_level->avx512_p1 * DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		if (ctdp_level->uncore_p1) {
 			snprintf(header, sizeof(header), "uncore-frequency-min(MHz)");
 			snprintf(value, sizeof(value), "%d",
 				 ctdp_level->uncore_p1 * DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		if (ctdp_level->uncore_p0) {
 			snprintf(header, sizeof(header), "uncore-frequency-max(MHz)");
 			snprintf(value, sizeof(value), "%d",
 				 ctdp_level->uncore_p0 * DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		if (ctdp_level->mem_freq) {
 			snprintf(header, sizeof(header), "mem-frequency(MHz)");
 			snprintf(value, sizeof(value), "%d",
 				 ctdp_level->mem_freq * DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		snprintf(header, sizeof(header),
@@ -405,7 +441,7 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 				snprintf(value, sizeof(value), "disabled");
 		} else
 			snprintf(value, sizeof(value), "unsupported");
-		format_and_print(outf, base_level + 4, header, value);
+		format_and_print(outf, level + 2, header, value);
 
 		snprintf(header, sizeof(header),
 			 "speed-select-base-freq");
@@ -416,7 +452,7 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 				snprintf(value, sizeof(value), "disabled");
 		} else
 			snprintf(value, sizeof(value), "unsupported");
-		format_and_print(outf, base_level + 4, header, value);
+		format_and_print(outf, level + 2, header, value);
 
 		snprintf(header, sizeof(header),
 			 "speed-select-core-power");
@@ -427,110 +463,115 @@ void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 				snprintf(value, sizeof(value), "disabled");
 		} else
 			snprintf(value, sizeof(value), "unsupported");
-		format_and_print(outf, base_level + 4, header, value);
+		format_and_print(outf, level + 2, header, value);
 
 		if (is_clx_n_platform()) {
 			if (ctdp_level->pbf_support)
 				_isst_pbf_display_information(cpu, outf,
 							      tdp_level,
 							  &ctdp_level->pbf_info,
-							      base_level + 4);
+							      level + 1);
 			continue;
 		}
 
 		if (ctdp_level->pkg_tdp) {
 			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);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		if (ctdp_level->t_proc_hot) {
 			snprintf(header, sizeof(header), "tjunction-max(C)");
 			snprintf(value, sizeof(value), "%d", ctdp_level->t_proc_hot);
-			format_and_print(outf, base_level + 4, header, value);
+			format_and_print(outf, level + 2, header, value);
 		}
 
 		snprintf(header, sizeof(header), "turbo-ratio-limits-sse");
-		format_and_print(outf, base_level + 4, header, NULL);
+		format_and_print(outf, level + 2, header, NULL);
 		for (j = 0; j < 8; ++j) {
 			snprintf(header, sizeof(header), "bucket-%d", j);
-			format_and_print(outf, base_level + 5, header, NULL);
+			format_and_print(outf, level + 3, header, NULL);
 
 			snprintf(header, sizeof(header), "core-count");
 			snprintf(value, sizeof(value), "%llu", (ctdp_level->buckets_info >> (j * 8)) & 0xff);
-			format_and_print(outf, base_level + 6, header, value);
+			format_and_print(outf, level + 4, header, value);
 
 			snprintf(header, sizeof(header),
 				"max-turbo-frequency(MHz)");
 			snprintf(value, sizeof(value), "%d",
 				 ctdp_level->trl_sse_active_cores[j] *
 				  DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 6, header, value);
+			format_and_print(outf, level + 4, header, value);
 		}
 
 		if (ctdp_level->trl_avx_active_cores[0]) {
 			snprintf(header, sizeof(header), "turbo-ratio-limits-avx2");
-			format_and_print(outf, base_level + 4, header, NULL);
+			format_and_print(outf, level + 2, header, NULL);
 			for (j = 0; j < 8; ++j) {
 				snprintf(header, sizeof(header), "bucket-%d", j);
-				format_and_print(outf, base_level + 5, header, NULL);
+				format_and_print(outf, level + 3, header, NULL);
 
 				snprintf(header, sizeof(header), "core-count");
 				snprintf(value, sizeof(value), "%llu", (ctdp_level->buckets_info >> (j * 8)) & 0xff);
-				format_and_print(outf, base_level + 6, header, value);
+				format_and_print(outf, level + 4, header, value);
 
 				snprintf(header, sizeof(header), "max-turbo-frequency(MHz)");
 				snprintf(value, sizeof(value), "%d", ctdp_level->trl_avx_active_cores[j] * DISP_FREQ_MULTIPLIER);
-				format_and_print(outf, base_level + 6, header, value);
+				format_and_print(outf, level + 4, header, value);
 			}
 		}
 
 		if (ctdp_level->trl_avx_512_active_cores[0]) {
 			snprintf(header, sizeof(header), "turbo-ratio-limits-avx512");
-			format_and_print(outf, base_level + 4, header, NULL);
+			format_and_print(outf, level + 2, header, NULL);
 			for (j = 0; j < 8; ++j) {
 				snprintf(header, sizeof(header), "bucket-%d", j);
-				format_and_print(outf, base_level + 5, header, NULL);
+				format_and_print(outf, level + 3, header, NULL);
 
 				snprintf(header, sizeof(header), "core-count");
 				snprintf(value, sizeof(value), "%llu", (ctdp_level->buckets_info >> (j * 8)) & 0xff);
-				format_and_print(outf, base_level + 6, header, value);
+				format_and_print(outf, level + 4, header, value);
 
 				snprintf(header, sizeof(header), "max-turbo-frequency(MHz)");
 				snprintf(value, sizeof(value), "%d", ctdp_level->trl_avx_512_active_cores[j] * DISP_FREQ_MULTIPLIER);
-			format_and_print(outf, base_level + 6, header, value);
+				format_and_print(outf, level + 4, header, value);
 			}
 		}
 
 		if (ctdp_level->pbf_support)
 			_isst_pbf_display_information(cpu, outf, i,
 						      &ctdp_level->pbf_info,
-						      base_level + 4);
+						      level + 2);
 		if (ctdp_level->fact_support)
 			_isst_fact_display_information(cpu, outf, i, 0xff, 0xff,
 						       &ctdp_level->fact_info,
-						       base_level + 4);
+						       level + 2);
 	}
 
 	format_and_print(outf, 1, NULL, NULL);
 }
 
+static int start;
 void isst_ctdp_display_information_start(FILE *outf)
 {
 	last_level = 0;
 	format_and_print(outf, 0, "start", NULL);
+	start = 1;
 }
 
 void isst_ctdp_display_information_end(FILE *outf)
 {
 	format_and_print(outf, 0, NULL, NULL);
+	start = 0;
 }
 
 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);
+	int _level;
+
+	_level = print_package_info(cpu, outf);
+	_isst_pbf_display_information(cpu, outf, level, pbf_info, _level + 1);
 	format_and_print(outf, 1, NULL, NULL);
 }
 
@@ -538,9 +579,11 @@ void isst_fact_display_information(int cpu, FILE *outf, int level,
 				   int fact_bucket, int fact_avx,
 				   struct isst_fact_info *fact_info)
 {
-	print_package_info(cpu, outf);
+	int _level;
+
+	_level = print_package_info(cpu, outf);
 	_isst_fact_display_information(cpu, outf, level, fact_bucket, fact_avx,
-				       fact_info, 4);
+				       fact_info, _level + 1);
 	format_and_print(outf, 1, NULL, NULL);
 }
 
@@ -549,94 +592,103 @@ void isst_clos_display_information(int cpu, FILE *outf, int clos,
 {
 	char header[256];
 	char value[256];
+	int level;
 
-	snprintf(header, sizeof(header), "package-%d",
-		 get_physical_package_id(cpu));
-	format_and_print(outf, 1, header, NULL);
-	snprintf(header, sizeof(header), "die-%d", get_physical_die_id(cpu));
-	format_and_print(outf, 2, header, NULL);
-	snprintf(header, sizeof(header), "cpu-%d", cpu);
-	format_and_print(outf, 3, header, NULL);
+	level = print_package_info(cpu, outf);
 
 	snprintf(header, sizeof(header), "core-power");
-	format_and_print(outf, 4, header, NULL);
+	format_and_print(outf, level + 1, header, NULL);
 
 	snprintf(header, sizeof(header), "clos");
 	snprintf(value, sizeof(value), "%d", clos);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "epp");
 	snprintf(value, sizeof(value), "%d", clos_config->epp);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "clos-proportional-priority");
 	snprintf(value, sizeof(value), "%d", clos_config->clos_prop_prio);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "clos-min");
 	snprintf(value, sizeof(value), "%d MHz", clos_config->clos_min * DISP_FREQ_MULTIPLIER);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "clos-max");
-	snprintf(value, sizeof(value), "%d MHz", clos_config->clos_max * DISP_FREQ_MULTIPLIER);
-	format_and_print(outf, 5, header, value);
+	if (clos_config->clos_max == 0xff)
+		snprintf(value, sizeof(value), "Max Turbo frequency");
+	else
+		snprintf(value, sizeof(value), "%d MHz", clos_config->clos_max * DISP_FREQ_MULTIPLIER);
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "clos-desired");
 	snprintf(value, sizeof(value), "%d MHz", clos_config->clos_desired * DISP_FREQ_MULTIPLIER);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
 
-	format_and_print(outf, 1, NULL, NULL);
+	format_and_print(outf, level, NULL, NULL);
 }
 
 void isst_clos_display_clos_information(int cpu, FILE *outf,
-					int clos_enable, int type)
+					int clos_enable, int type,
+					int state, int cap)
 {
 	char header[256];
 	char value[256];
+	int level;
 
-	snprintf(header, sizeof(header), "package-%d",
-		 get_physical_package_id(cpu));
-	format_and_print(outf, 1, header, NULL);
-	snprintf(header, sizeof(header), "die-%d", get_physical_die_id(cpu));
-	format_and_print(outf, 2, header, NULL);
-	snprintf(header, sizeof(header), "cpu-%d", cpu);
-	format_and_print(outf, 3, header, NULL);
+	level = print_package_info(cpu, outf);
 
 	snprintf(header, sizeof(header), "core-power");
-	format_and_print(outf, 4, header, NULL);
+	format_and_print(outf, level + 1, header, NULL);
+
+	snprintf(header, sizeof(header), "support-status");
+	if (cap)
+		snprintf(value, sizeof(value), "supported");
+	else
+		snprintf(value, sizeof(value), "unsupported");
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "enable-status");
-	snprintf(value, sizeof(value), "%d", clos_enable);
-	format_and_print(outf, 5, header, value);
+	if (state)
+		snprintf(value, sizeof(value), "enabled");
+	else
+		snprintf(value, sizeof(value), "disabled");
+	format_and_print(outf, level + 2, header, value);
+
+	snprintf(header, sizeof(header), "clos-enable-status");
+	if (clos_enable)
+		snprintf(value, sizeof(value), "enabled");
+	else
+		snprintf(value, sizeof(value), "disabled");
+	format_and_print(outf, level + 2, header, value);
 
 	snprintf(header, sizeof(header), "priority-type");
-	snprintf(value, sizeof(value), "%d", type);
-	format_and_print(outf, 5, header, value);
+	if (type)
+		snprintf(value, sizeof(value), "ordered");
+	else
+		snprintf(value, sizeof(value), "proportional");
+	format_and_print(outf, level + 2, header, value);
 
-	format_and_print(outf, 1, NULL, NULL);
+	format_and_print(outf, level, NULL, NULL);
 }
 
 void isst_clos_display_assoc_information(int cpu, FILE *outf, int clos)
 {
 	char header[256];
 	char value[256];
+	int level;
 
-	snprintf(header, sizeof(header), "package-%d",
-		 get_physical_package_id(cpu));
-	format_and_print(outf, 1, header, NULL);
-	snprintf(header, sizeof(header), "die-%d", get_physical_die_id(cpu));
-	format_and_print(outf, 2, header, NULL);
-	snprintf(header, sizeof(header), "cpu-%d", cpu);
-	format_and_print(outf, 3, header, NULL);
+	level = print_package_info(cpu, outf);
 
 	snprintf(header, sizeof(header), "get-assoc");
-	format_and_print(outf, 4, header, NULL);
+	format_and_print(outf, level + 1, header, NULL);
 
 	snprintf(header, sizeof(header), "clos");
 	snprintf(value, sizeof(value), "%d", clos);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
 
-	format_and_print(outf, 1, NULL, NULL);
+	format_and_print(outf, level, NULL, NULL);
 }
 
 void isst_display_result(int cpu, FILE *outf, char *feature, char *cmd,
@@ -644,24 +696,60 @@ void isst_display_result(int cpu, FILE *outf, char *feature, char *cmd,
 {
 	char header[256];
 	char value[256];
+	int level = 3;
+
+	if (cpu >= 0)
+		level = print_package_info(cpu, outf);
 
-	if (cpu >= 0) {
-		snprintf(header, sizeof(header), "package-%d",
-			 get_physical_package_id(cpu));
-		format_and_print(outf, 1, header, NULL);
-		snprintf(header, sizeof(header), "die-%d", get_physical_die_id(cpu));
-		format_and_print(outf, 2, header, NULL);
-		snprintf(header, sizeof(header), "cpu-%d", cpu);
-		format_and_print(outf, 3, header, NULL);
-	}
 	snprintf(header, sizeof(header), "%s", feature);
-	format_and_print(outf, 4, header, NULL);
+	format_and_print(outf, level + 1, header, NULL);
 	snprintf(header, sizeof(header), "%s", cmd);
 	if (!result)
 		snprintf(value, sizeof(value), "success");
 	else
 		snprintf(value, sizeof(value), "failed(error %d)", result);
-	format_and_print(outf, 5, header, value);
+	format_and_print(outf, level + 2, header, value);
+
+	format_and_print(outf, level, NULL, NULL);
+}
+
+void isst_display_error_info_message(int error, char *msg, int arg_valid, int arg)
+{
+	FILE *outf = get_output_file();
+	static int error_index;
+	char header[256];
+	char value[256];
+
+	if (!out_format_is_json()) {
+		if (arg_valid)
+			snprintf(value, sizeof(value), "%s %d", msg, arg);
+		else
+			snprintf(value, sizeof(value), "%s", msg);
+
+		if (error)
+			fprintf(outf, "Error: %s\n", value);
+		else
+			fprintf(outf, "Information: %s\n", value);
+		return;
+	}
+
+	if (!start)
+		format_and_print(outf, 0, "start", NULL);
+
+	if (error)
+		snprintf(header, sizeof(header), "Error%d", error_index++);
+	else
+		snprintf(header, sizeof(header), "Information:%d", error_index++);
+	format_and_print(outf, 1, header, NULL);
+
+	snprintf(header, sizeof(header), "message");
+	if (arg_valid)
+		snprintf(value, sizeof(value), "%s %d", msg, arg);
+	else
+		snprintf(value, sizeof(value), "%s", msg);
 
+	format_and_print(outf, 2, header, value);
 	format_and_print(outf, 1, NULL, NULL);
+	if (!start)
+		format_and_print(outf, 0, NULL, NULL);
 }
diff --git a/tools/power/x86/intel-speed-select/isst.h b/tools/power/x86/intel-speed-select/isst.h
index ad5aa6341d0f..2e1afd856a78 100644
--- a/tools/power/x86/intel-speed-select/isst.h
+++ b/tools/power/x86/intel-speed-select/isst.h
@@ -172,6 +172,7 @@ extern int get_cpu_count(int pkg_id, int die_id);
 extern int get_core_count(int pkg_id, int die_id);
 
 /* Common interfaces */
+FILE *get_output_file(void);
 extern void debug_printf(const char *format, ...);
 extern int out_format_is_json(void);
 extern int get_physical_package_id(int cpu);
@@ -196,6 +197,8 @@ extern int isst_send_msr_command(unsigned int cpu, unsigned int command,
 				 int write, unsigned long long *req_resp);
 
 extern int isst_get_ctdp_levels(int cpu, struct isst_pkg_ctdp *pkg_dev);
+extern int isst_get_ctdp_control(int cpu, int config_index,
+				 struct isst_pkg_ctdp_level_info *ctdp_level);
 extern int isst_get_coremask_info(int cpu, int config_index,
 			   struct isst_pkg_ctdp_level_info *ctdp_level);
 extern int isst_get_process_ctdp(int cpu, int tdp_level,
@@ -205,7 +208,7 @@ extern void isst_get_process_ctdp_complete(int cpu,
 extern void isst_ctdp_display_information(int cpu, FILE *outf, int tdp_level,
 					  struct isst_pkg_ctdp *pkg_dev);
 extern void isst_ctdp_display_core_info(int cpu, FILE *outf, char *prefix,
-					unsigned int val);
+					unsigned int val, char *str0, char *str1);
 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,
@@ -216,7 +219,7 @@ extern int isst_set_pbf_fact_status(int cpu, int pbf, int enable);
 extern int isst_get_pbf_info(int cpu, int level,
 			     struct isst_pbf_info *pbf_info);
 extern void isst_get_pbf_info_complete(struct isst_pbf_info *pbf_info);
-extern int isst_get_fact_info(int cpu, int level,
+extern int isst_get_fact_info(int cpu, int level, int fact_bucket,
 			      struct isst_fact_info *fact_info);
 extern int isst_get_fact_bucket_info(int cpu, int level,
 				     struct isst_fact_bucket_info *bucket_info);
@@ -245,7 +248,10 @@ extern void isst_display_result(int cpu, FILE *outf, char *feature, char *cmd,
 
 extern int isst_clos_get_clos_information(int cpu, int *enable, int *type);
 extern void isst_clos_display_clos_information(int cpu, FILE *outf,
-					       int clos_enable, int type);
+					       int clos_enable, int type,
+					       int state, int cap);
 extern int is_clx_n_platform(void);
 extern int get_cpufreq_base_freq(int cpu);
+extern int isst_read_pm_config(int cpu, int *cp_state, int *cp_cap);
+extern void isst_display_error_info_message(int error, char *msg, int arg_valid, int arg);
 #endif