From 55130fb22a1c396139c3da46f939bf5a6a92095e Mon Sep 17 00:00:00 2001 From: Punit Agrawal Date: Tue, 24 Nov 2020 08:59:51 +0900 Subject: ACPI: processor: Drop duplicate setting of shared_cpu_map 'shared_cpu_map', stored as part of the per-processor acpi_processor_performance structre, is used to store CPUs that share a performance domain. By definition it contains the owning CPU. While building the 'shared_cpu_map' it is being set twice - once while initialising the performance domains and again when matching CPUs belonging to the same domain. Drop the unnecessary initialisation. Signed-off-by: Punit Agrawal Signed-off-by: Rafael J. Wysocki --- drivers/acpi/processor_perflib.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c index b04a68950ff1..b0d320f18163 100644 --- a/drivers/acpi/processor_perflib.c +++ b/drivers/acpi/processor_perflib.c @@ -616,7 +616,6 @@ int acpi_processor_preregister_performance( continue; pr->performance = per_cpu_ptr(performance, i); - cpumask_set_cpu(i, pr->performance->shared_cpu_map); pdomain = &(pr->performance->domain_info); if (acpi_processor_get_psd(pr->handle, pdomain)) { retval = -EINVAL; -- cgit v1.2.3 From bca3e43c903f5c58daeab1fea0af566233ea003c Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Mon, 14 Dec 2020 12:07:40 +0000 Subject: ACPI: processor: fix NONE coordination for domain mapping failure For errors parsing the _PSD domains, a separate domain is returned for each CPU in the failed _PSD domain with no coordination (as per previous comment). But contrary to the intention, the code was setting CPUFREQ_SHARED_TYPE_ALL as coordination type. Change shared_type to CPUFREQ_SHARED_TYPE_NONE in case of errors parsing the domain information. The function still returns the error and the caller is free to bail out the domain initialisation altogether in that case. Given that both functions return domains with a single CPU, this change does not affect the functionality, but clarifies the intention. Signed-off-by: Ionela Voinescu Acked-by: Viresh Kumar [ rjw: Subject edit ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 2 +- drivers/acpi/processor_perflib.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 7a99b19bb893..9e335f0d2595 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -511,7 +511,7 @@ err_ret: /* Assume no coordination on any error parsing domain info */ cpumask_clear(pr->shared_cpu_map); cpumask_set_cpu(i, pr->shared_cpu_map); - pr->shared_type = CPUFREQ_SHARED_TYPE_ALL; + pr->shared_type = CPUFREQ_SHARED_TYPE_NONE; } out: free_cpumask_var(covered_cpus); diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c index b0d320f18163..1338925c9359 100644 --- a/drivers/acpi/processor_perflib.c +++ b/drivers/acpi/processor_perflib.c @@ -709,7 +709,7 @@ err_ret: if (retval) { cpumask_clear(pr->performance->shared_cpu_map); cpumask_set_cpu(i, pr->performance->shared_cpu_map); - pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL; + pr->performance->shared_type = CPUFREQ_SHARED_TYPE_NONE; } pr->performance = NULL; /* Will be set for real in register */ } -- cgit v1.2.3 From a28b2bfc099c6b9caa6ef697660408e076a32019 Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Mon, 14 Dec 2020 12:38:23 +0000 Subject: cppc_cpufreq: replace per-cpu data array with a list The cppc_cpudata per-cpu storage was inefficient (1) additional to causing functional issues (2) when CPUs are hotplugged out, due to per-cpu data being improperly initialised. (1) The amount of information needed for CPPC performance control in its cpufreq driver depends on the domain (PSD) coordination type: ANY: One set of CPPC control and capability data (e.g desired performance, highest/lowest performance, etc) applies to all CPUs in the domain. ALL: Same as ANY. To be noted that this type is not currently supported. When supported, information about which CPUs belong to a domain is needed in order for frequency change requests to be sent to each of them. HW: It's necessary to store CPPC control and capability information for all the CPUs. HW will then coordinate the performance state based on their limitations and requests. NONE: Same as HW. No HW coordination is expected. Despite this, the previous initialisation code would indiscriminately allocate memory for all CPUs (all_cpu_data) and unnecessarily duplicate performance capabilities and the domain sharing mask and type for each possible CPU. (2) With the current per-cpu structure, when having ANY coordination, the cppc_cpudata cpu information is not initialised (will remain 0) for all CPUs in a policy, other than policy->cpu. When policy->cpu is hotplugged out, the driver will incorrectly use the uninitialised (0) value of the other CPUs when making frequency changes. Additionally, the previous values stored in the perf_ctrls.desired_perf will be lost when policy->cpu changes. Therefore replace the array of per cpu data with a list. The memory for each structure is allocated at policy init, where a single structure can be allocated per policy, not per cpu. In order to accommodate the struct list_head node in the cppc_cpudata structure, the now unused cpu and cur_policy variables are removed. For example, on a arm64 Juno platform with 6 CPUs: (0, 1, 2, 3) in PSD1, (4, 5) in PSD2 - ANY coordination, the memory allocation comparison shows: Before patch: - ANY coordination: total slack req alloc/free caller 0 0 0 0/1 _kernel_size_le_hi32+0x0xffff800008ff7810 0 0 0 0/6 _kernel_size_le_hi32+0x0xffff800008ff7808 128 80 48 1/0 _kernel_size_le_hi32+0x0xffff800008ffc070 768 0 768 6/0 _kernel_size_le_hi32+0x0xffff800008ffc0e4 After patch: - ANY coordination: total slack req alloc/free caller 256 0 256 2/0 _kernel_size_le_hi32+0x0xffff800008fed410 0 0 0 0/2 _kernel_size_le_hi32+0x0xffff800008fed274 Additional notes: - A pointer to the policy's cppc_cpudata is stored in policy->driver_data - Driver registration is skipped if _CPC entries are not present. Signed-off-by: Ionela Voinescu Tested-by: Mian Yousaf Kaukab Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 141 ++++++++++++++++++++--------------------------- 1 file changed, 60 insertions(+), 81 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 9e335f0d2595..8c1d62e88c46 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -413,109 +413,88 @@ end: return result; } +bool acpi_cpc_valid(void) +{ + struct cpc_desc *cpc_ptr; + int cpu; + + for_each_possible_cpu(cpu) { + cpc_ptr = per_cpu(cpc_desc_ptr, cpu); + if (!cpc_ptr) + return false; + } + + return true; +} +EXPORT_SYMBOL_GPL(acpi_cpc_valid); + /** - * acpi_get_psd_map - Map the CPUs in a common freq domain. - * @all_cpu_data: Ptrs to CPU specific CPPC data including PSD info. + * acpi_get_psd_map - Map the CPUs in the freq domain of a given cpu + * @cpu: Find all CPUs that share a domain with cpu. + * @cpu_data: Pointer to CPU specific CPPC data including PSD info. * * Return: 0 for success or negative value for err. */ -int acpi_get_psd_map(struct cppc_cpudata **all_cpu_data) +int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data) { - int count_target; - int retval = 0; - unsigned int i, j; - cpumask_var_t covered_cpus; - struct cppc_cpudata *pr, *match_pr; - struct acpi_psd_package *pdomain; - struct acpi_psd_package *match_pdomain; struct cpc_desc *cpc_ptr, *match_cpc_ptr; - - if (!zalloc_cpumask_var(&covered_cpus, GFP_KERNEL)) - return -ENOMEM; + struct acpi_psd_package *match_pdomain; + struct acpi_psd_package *pdomain; + int count_target, i; /* * Now that we have _PSD data from all CPUs, let's setup P-state * domain info. */ - for_each_possible_cpu(i) { - if (cpumask_test_cpu(i, covered_cpus)) - continue; - - pr = all_cpu_data[i]; - cpc_ptr = per_cpu(cpc_desc_ptr, i); - if (!cpc_ptr) { - retval = -EFAULT; - goto err_ret; - } + cpc_ptr = per_cpu(cpc_desc_ptr, cpu); + if (!cpc_ptr) + return -EFAULT; - pdomain = &(cpc_ptr->domain_info); - cpumask_set_cpu(i, pr->shared_cpu_map); - cpumask_set_cpu(i, covered_cpus); - if (pdomain->num_processors <= 1) - continue; + pdomain = &(cpc_ptr->domain_info); + cpumask_set_cpu(cpu, cpu_data->shared_cpu_map); + if (pdomain->num_processors <= 1) + return 0; - /* Validate the Domain info */ - count_target = pdomain->num_processors; - if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL) - pr->shared_type = CPUFREQ_SHARED_TYPE_ALL; - else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL) - pr->shared_type = CPUFREQ_SHARED_TYPE_HW; - else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY) - pr->shared_type = CPUFREQ_SHARED_TYPE_ANY; - - for_each_possible_cpu(j) { - if (i == j) - continue; - - match_cpc_ptr = per_cpu(cpc_desc_ptr, j); - if (!match_cpc_ptr) { - retval = -EFAULT; - goto err_ret; - } + /* Validate the Domain info */ + count_target = pdomain->num_processors; + if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL) + cpu_data->shared_type = CPUFREQ_SHARED_TYPE_ALL; + else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL) + cpu_data->shared_type = CPUFREQ_SHARED_TYPE_HW; + else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY) + cpu_data->shared_type = CPUFREQ_SHARED_TYPE_ANY; - match_pdomain = &(match_cpc_ptr->domain_info); - if (match_pdomain->domain != pdomain->domain) - continue; + for_each_possible_cpu(i) { + if (i == cpu) + continue; - /* Here i and j are in the same domain */ - if (match_pdomain->num_processors != count_target) { - retval = -EFAULT; - goto err_ret; - } + match_cpc_ptr = per_cpu(cpc_desc_ptr, i); + if (!match_cpc_ptr) + goto err_fault; - if (pdomain->coord_type != match_pdomain->coord_type) { - retval = -EFAULT; - goto err_ret; - } + match_pdomain = &(match_cpc_ptr->domain_info); + if (match_pdomain->domain != pdomain->domain) + continue; - cpumask_set_cpu(j, covered_cpus); - cpumask_set_cpu(j, pr->shared_cpu_map); - } + /* Here i and cpu are in the same domain */ + if (match_pdomain->num_processors != count_target) + goto err_fault; - for_each_cpu(j, pr->shared_cpu_map) { - if (i == j) - continue; + if (pdomain->coord_type != match_pdomain->coord_type) + goto err_fault; - match_pr = all_cpu_data[j]; - match_pr->shared_type = pr->shared_type; - cpumask_copy(match_pr->shared_cpu_map, - pr->shared_cpu_map); - } + cpumask_set_cpu(i, cpu_data->shared_cpu_map); } - goto out; -err_ret: - for_each_possible_cpu(i) { - pr = all_cpu_data[i]; + return 0; - /* Assume no coordination on any error parsing domain info */ - cpumask_clear(pr->shared_cpu_map); - cpumask_set_cpu(i, pr->shared_cpu_map); - pr->shared_type = CPUFREQ_SHARED_TYPE_NONE; - } -out: - free_cpumask_var(covered_cpus); - return retval; +err_fault: + /* Assume no coordination on any error parsing domain info */ + cpumask_clear(cpu_data->shared_cpu_map); + cpumask_set_cpu(cpu, cpu_data->shared_cpu_map); + cpu_data->shared_type = CPUFREQ_SHARED_TYPE_NONE; + + return -EFAULT; } EXPORT_SYMBOL_GPL(acpi_get_psd_map); -- cgit v1.2.3