From 850337ec7b2c0f510772eeeef9c12c658b3f3603 Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Sat, 20 Aug 2022 23:58:18 +0800 Subject: tools/power/x86/intel-speed-select: Introduce struct isst_id SST control is power-domain based rather than cpu based, on all the systems including Sapphire Rapids and ealier. SST core APIs uses cpu id as parameter, and use the underlying pkg_id and die_id information to find a power domain, this is not straight forward and introduces obscure logics in the code. Introduce struct isst_id to represent a SST Power Domain. All core APIs are converted to use struct isst_id as parameter instead of using cpu id. Signed-off-by: Zhang Rui Signed-off-by: Srinivas Pandruvada --- tools/power/x86/intel-speed-select/isst-daemon.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'tools/power/x86/intel-speed-select/isst-daemon.c') diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c index d0400c6684ba..9bfe4e4a23fd 100644 --- a/tools/power/x86/intel-speed-select/isst-daemon.c +++ b/tools/power/x86/intel-speed-select/isst-daemon.c @@ -32,17 +32,17 @@ static void init_levels(void) per_package_levels_info[i][j] = -1; } -void process_level_change(int cpu) +void process_level_change(struct isst_id *id) { struct isst_pkg_ctdp_level_info ctdp_level; - int pkg_id = get_physical_package_id(cpu); - int die_id = get_physical_die_id(cpu); + int pkg_id = get_physical_package_id(id->cpu); + int die_id = get_physical_die_id(id->cpu); struct isst_pkg_ctdp pkg_dev; time_t tm; int ret; if (pkg_id >= MAX_PACKAGE_COUNT || die_id >= MAX_DIE_PER_PACKAGE) { - debug_printf("Invalid package/die info for cpu:%d\n", cpu); + debug_printf("Invalid package/die info for cpu:%d\n", id->cpu); return; } @@ -52,13 +52,13 @@ void process_level_change(int cpu) per_package_levels_tm[pkg_id][die_id] = tm; - ret = isst_get_ctdp_levels(cpu, &pkg_dev); + ret = isst_get_ctdp_levels(id, &pkg_dev); if (ret) { - debug_printf("Can't get tdp levels for cpu:%d\n", cpu); + debug_printf("Can't get tdp levels for cpu:%d\n", id->cpu); return; } - debug_printf("Get Config level %d pkg:%d die:%d current_level:%d \n", cpu, + debug_printf("Get Config level %d pkg:%d die:%d current_level:%d\n", id->cpu, pkg_id, die_id, pkg_dev.current_level); if (pkg_dev.locked) { @@ -70,17 +70,17 @@ void process_level_change(int cpu) return; debug_printf("**Config level change for cpu:%d pkg:%d die:%d from %d to %d\n", - cpu, pkg_id, die_id, per_package_levels_info[pkg_id][die_id], + id->cpu, pkg_id, die_id, per_package_levels_info[pkg_id][die_id], pkg_dev.current_level); per_package_levels_info[pkg_id][die_id] = pkg_dev.current_level; ctdp_level.core_cpumask_size = alloc_cpu_set(&ctdp_level.core_cpumask); - ret = isst_get_coremask_info(cpu, pkg_dev.current_level, &ctdp_level); + ret = isst_get_coremask_info(id, pkg_dev.current_level, &ctdp_level); if (ret) { free_cpu_set(ctdp_level.core_cpumask); - debug_printf("Can't get core_mask:%d\n", cpu); + debug_printf("Can't get core_mask:%d\n", id->cpu); return; } @@ -102,10 +102,10 @@ void process_level_change(int cpu) free_cpu_set(ctdp_level.core_cpumask); } -static void _poll_for_config_change(int cpu, void *arg1, void *arg2, +static void _poll_for_config_change(struct isst_id *id, void *arg1, void *arg2, void *arg3, void *arg4) { - process_level_change(cpu); + process_level_change(id); } static void poll_for_config_change(void) -- cgit v1.2.1 From 56d6469291f8b1e1ab2f78e401c564163d3ee0f0 Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Sat, 20 Aug 2022 23:58:21 +0800 Subject: tools/power/x86/intel-speed-select: Cleanup get_physical_id usage struct isst_id already contains package and die id information, thus there is no need to get the package and die id information, when struct isst_id is already available. Remove unneeded get_physical_package_id/get_physical_die_id usage. Signed-off-by: Zhang Rui Signed-off-by: Srinivas Pandruvada --- tools/power/x86/intel-speed-select/isst-daemon.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'tools/power/x86/intel-speed-select/isst-daemon.c') diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c index 9bfe4e4a23fd..4826625ce891 100644 --- a/tools/power/x86/intel-speed-select/isst-daemon.c +++ b/tools/power/x86/intel-speed-select/isst-daemon.c @@ -35,22 +35,20 @@ static void init_levels(void) void process_level_change(struct isst_id *id) { struct isst_pkg_ctdp_level_info ctdp_level; - int pkg_id = get_physical_package_id(id->cpu); - int die_id = get_physical_die_id(id->cpu); struct isst_pkg_ctdp pkg_dev; time_t tm; int ret; - if (pkg_id >= MAX_PACKAGE_COUNT || die_id >= MAX_DIE_PER_PACKAGE) { + if (id->pkg >= MAX_PACKAGE_COUNT || id->die >= MAX_DIE_PER_PACKAGE) { debug_printf("Invalid package/die info for cpu:%d\n", id->cpu); return; } tm = time(NULL); - if (tm - per_package_levels_tm[pkg_id][die_id] < 2 ) + if (tm - per_package_levels_tm[id->pkg][id->die] < 2) return; - per_package_levels_tm[pkg_id][die_id] = tm; + per_package_levels_tm[id->pkg][id->die] = tm; ret = isst_get_ctdp_levels(id, &pkg_dev); if (ret) { @@ -59,21 +57,21 @@ void process_level_change(struct isst_id *id) } debug_printf("Get Config level %d pkg:%d die:%d current_level:%d\n", id->cpu, - pkg_id, die_id, pkg_dev.current_level); + id->pkg, id->die, pkg_dev.current_level); if (pkg_dev.locked) { debug_printf("config TDP s locked \n"); return; } - if (per_package_levels_info[pkg_id][die_id] == pkg_dev.current_level) + if (per_package_levels_info[id->pkg][id->die] == pkg_dev.current_level) return; debug_printf("**Config level change for cpu:%d pkg:%d die:%d from %d to %d\n", - id->cpu, pkg_id, die_id, per_package_levels_info[pkg_id][die_id], + id->cpu, id->pkg, id->die, per_package_levels_info[id->pkg][id->die], pkg_dev.current_level); - per_package_levels_info[pkg_id][die_id] = pkg_dev.current_level; + per_package_levels_info[id->pkg][id->die] = pkg_dev.current_level; ctdp_level.core_cpumask_size = alloc_cpu_set(&ctdp_level.core_cpumask); @@ -87,7 +85,7 @@ void process_level_change(struct isst_id *id) if (ctdp_level.cpu_count) { int i, max_cpus = get_topo_max_cpus(); for (i = 0; i < max_cpus; ++i) { - if (pkg_id != get_physical_package_id(i) || die_id != get_physical_die_id(i)) + if (id->pkg != get_physical_package_id(i) || id->die != get_physical_die_id(i)) continue; if (CPU_ISSET_S(i, ctdp_level.core_cpumask_size, ctdp_level.core_cpumask)) { fprintf(stderr, "online cpu %d\n", i); -- cgit v1.2.1 From 00bb07db5a42c91f4a74e836c6ae70fff20f7f8a Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Sat, 20 Aug 2022 23:58:22 +0800 Subject: tools/power/x86/intel-speed-select: Introduce is_cpu_in_power_domain helper struct isst_id contains cpu, package and die info, and it can represent a specific SST power domain. Introduce is_cpu_in_power_domain() helper to identify if a cpu is in a specified power_domain. And cleanup the code to use the new helper. Signed-off-by: Zhang Rui Signed-off-by: Srinivas Pandruvada --- tools/power/x86/intel-speed-select/isst-daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/power/x86/intel-speed-select/isst-daemon.c') diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c index 4826625ce891..c5d978ecc443 100644 --- a/tools/power/x86/intel-speed-select/isst-daemon.c +++ b/tools/power/x86/intel-speed-select/isst-daemon.c @@ -85,7 +85,7 @@ void process_level_change(struct isst_id *id) if (ctdp_level.cpu_count) { int i, max_cpus = get_topo_max_cpus(); for (i = 0; i < max_cpus; ++i) { - if (id->pkg != get_physical_package_id(i) || id->die != get_physical_die_id(i)) + if (!is_cpu_in_power_domain(i, id)) continue; if (CPU_ISSET_S(i, ctdp_level.core_cpumask_size, ctdp_level.core_cpumask)) { fprintf(stderr, "online cpu %d\n", i); -- cgit v1.2.1 From 3ba6a27566a53030b3013f9f841ba5889a344cb8 Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Sat, 20 Aug 2022 23:58:24 +0800 Subject: tools/power/x86/intel-speed-select: Enforce isst_id value Enforce the pkg/die value in struct isst_id are either -1 or a valid value. This helps avoid inconsistent or redundant checks. Signed-off-by: Zhang Rui Signed-off-by: Srinivas Pandruvada --- tools/power/x86/intel-speed-select/isst-daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/power/x86/intel-speed-select/isst-daemon.c') diff --git a/tools/power/x86/intel-speed-select/isst-daemon.c b/tools/power/x86/intel-speed-select/isst-daemon.c index c5d978ecc443..0699137c0901 100644 --- a/tools/power/x86/intel-speed-select/isst-daemon.c +++ b/tools/power/x86/intel-speed-select/isst-daemon.c @@ -39,7 +39,7 @@ void process_level_change(struct isst_id *id) time_t tm; int ret; - if (id->pkg >= MAX_PACKAGE_COUNT || id->die >= MAX_DIE_PER_PACKAGE) { + if (id->pkg < 0 || id->die < 0) { debug_printf("Invalid package/die info for cpu:%d\n", id->cpu); return; } -- cgit v1.2.1