| Message ID | 20231027080400.56703-7-vincent.guittot@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | consolidate and cleanup CPU capacity | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-for-next-PR | fail | PR summary |
| conchuod/patch-6-test-1 | fail | .github/scripts/patches/build_rv32_defconfig.sh |
| conchuod/patch-6-test-2 | fail | .github/scripts/patches/build_rv64_clang_allmodconfig.sh |
| conchuod/patch-6-test-3 | fail | .github/scripts/patches/build_rv64_gcc_allmodconfig.sh |
| conchuod/patch-6-test-4 | fail | .github/scripts/patches/build_rv64_nommu_k210_defconfig.sh |
| conchuod/patch-6-test-5 | fail | .github/scripts/patches/build_rv64_nommu_virt_defconfig.sh |
| conchuod/patch-6-test-6 | warning | .github/scripts/patches/checkpatch.sh |
| conchuod/patch-6-test-7 | success | .github/scripts/patches/dtb_warn_rv64.sh |
| conchuod/patch-6-test-8 | success | .github/scripts/patches/header_inline.sh |
| conchuod/patch-6-test-9 | success | .github/scripts/patches/kdoc.sh |
| conchuod/patch-6-test-10 | success | .github/scripts/patches/module_param.sh |
| conchuod/patch-6-test-11 | success | .github/scripts/patches/verify_fixes.sh |
| conchuod/patch-6-test-12 | success | .github/scripts/patches/verify_signedoff.sh |
Hello Vincent, On 10/27/23 10:03, Vincent Guittot wrote: > Save the frequency associated to the performance that has been used when > initializing the capacity of CPUs. > Also, cppc cpufreq driver can register an artificial energy model. In such > case, it needs the frequency for this compute capacity. > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > --- > drivers/base/arch_topology.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index 9a073c2d2086..d4bef370feb3 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -349,6 +349,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > > void topology_init_cpu_capacity_cppc(void) > { > + u64 capacity, capacity_scale = 0; > struct cppc_perf_caps perf_caps; > int cpu; > > @@ -365,6 +366,10 @@ void topology_init_cpu_capacity_cppc(void) > (perf_caps.highest_perf >= perf_caps.nominal_perf) && > (perf_caps.highest_perf >= perf_caps.lowest_perf)) { > raw_capacity[cpu] = perf_caps.highest_perf; > + capacity_scale = max_t(u64, capacity_scale, raw_capacity[cpu]); > + > + per_cpu(capacity_ref_freq, cpu) = cppc_perf_to_khz(&perf_caps, raw_capacity[cpu]); > + To continue Beata's point, I think there is an issue with the following: cppc_perf_to_khz() and cppc_khz_to_perf() were previously used with a struct containing frequencies in KHz, cf. [1]. In the original _CPC object, frequencies are in MHz. It means that the perf_caps struct here contains frequencies in MHz, and per_cpu(capacity_ref_freq, cpu) is in MHz aswell. [1] https://github.com/torvalds/linux/blob/master/drivers/cpufreq/cppc_cpufreq.c#L682 > pr_debug("cpu_capacity: CPU%d cpu_capacity=%u (raw).\n", > cpu, raw_capacity[cpu]); > continue; > @@ -375,7 +380,15 @@ void topology_init_cpu_capacity_cppc(void) > goto exit; > } > > - topology_normalize_cpu_scale(); > + for_each_possible_cpu(cpu) { > + capacity = raw_capacity[cpu]; > + capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > + capacity_scale); > + topology_set_cpu_scale(cpu, capacity); > + pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n", > + cpu, topology_get_cpu_scale(cpu)); > + } > + > schedule_work(&update_topology_flags_work); > pr_debug("cpu_capacity: cpu_capacity initialization done\n"); >
Hi Pierre, On Thu, 2 Nov 2023 at 10:07, Pierre Gondois <pierre.gondois@arm.com> wrote: > > Hello Vincent, > > On 10/27/23 10:03, Vincent Guittot wrote: > > Save the frequency associated to the performance that has been used when > > initializing the capacity of CPUs. > > Also, cppc cpufreq driver can register an artificial energy model. In such > > case, it needs the frequency for this compute capacity. > > > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > > --- > > drivers/base/arch_topology.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > index 9a073c2d2086..d4bef370feb3 100644 > > --- a/drivers/base/arch_topology.c > > +++ b/drivers/base/arch_topology.c > > @@ -349,6 +349,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > > > > void topology_init_cpu_capacity_cppc(void) > > { > > + u64 capacity, capacity_scale = 0; > > struct cppc_perf_caps perf_caps; > > int cpu; > > > > @@ -365,6 +366,10 @@ void topology_init_cpu_capacity_cppc(void) > > (perf_caps.highest_perf >= perf_caps.nominal_perf) && > > (perf_caps.highest_perf >= perf_caps.lowest_perf)) { > > raw_capacity[cpu] = perf_caps.highest_perf; > > + capacity_scale = max_t(u64, capacity_scale, raw_capacity[cpu]); > > + > > + per_cpu(capacity_ref_freq, cpu) = cppc_perf_to_khz(&perf_caps, raw_capacity[cpu]); > > + > > To continue Beata's point, I think there is an issue with the following: > cppc_perf_to_khz() and cppc_khz_to_perf() were previously used with a struct containing > frequencies in KHz, cf. [1]. > In the original _CPC object, frequencies are in MHz. It means that the perf_caps struct > here contains frequencies in MHz, and per_cpu(capacity_ref_freq, cpu) is in MHz aswell. Yeah, I haven't noticed this intermediate step in cppc_cpufreq. I'm going to fix this > > [1] https://github.com/torvalds/linux/blob/master/drivers/cpufreq/cppc_cpufreq.c#L682 > > > > pr_debug("cpu_capacity: CPU%d cpu_capacity=%u (raw).\n", > > cpu, raw_capacity[cpu]); > > continue; > > @@ -375,7 +380,15 @@ void topology_init_cpu_capacity_cppc(void) > > goto exit; > > } > > > > - topology_normalize_cpu_scale(); > > + for_each_possible_cpu(cpu) { > > + capacity = raw_capacity[cpu]; > > + capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > > + capacity_scale); > > + topology_set_cpu_scale(cpu, capacity); > > + pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n", > > + cpu, topology_get_cpu_scale(cpu)); > > + } > > + > > schedule_work(&update_topology_flags_work); > > pr_debug("cpu_capacity: cpu_capacity initialization done\n"); > >
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 9a073c2d2086..d4bef370feb3 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -349,6 +349,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) void topology_init_cpu_capacity_cppc(void) { + u64 capacity, capacity_scale = 0; struct cppc_perf_caps perf_caps; int cpu; @@ -365,6 +366,10 @@ void topology_init_cpu_capacity_cppc(void) (perf_caps.highest_perf >= perf_caps.nominal_perf) && (perf_caps.highest_perf >= perf_caps.lowest_perf)) { raw_capacity[cpu] = perf_caps.highest_perf; + capacity_scale = max_t(u64, capacity_scale, raw_capacity[cpu]); + + per_cpu(capacity_ref_freq, cpu) = cppc_perf_to_khz(&perf_caps, raw_capacity[cpu]); + pr_debug("cpu_capacity: CPU%d cpu_capacity=%u (raw).\n", cpu, raw_capacity[cpu]); continue; @@ -375,7 +380,15 @@ void topology_init_cpu_capacity_cppc(void) goto exit; } - topology_normalize_cpu_scale(); + for_each_possible_cpu(cpu) { + capacity = raw_capacity[cpu]; + capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, + capacity_scale); + topology_set_cpu_scale(cpu, capacity); + pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n", + cpu, topology_get_cpu_scale(cpu)); + } + schedule_work(&update_topology_flags_work); pr_debug("cpu_capacity: cpu_capacity initialization done\n");
Save the frequency associated to the performance that has been used when initializing the capacity of CPUs. Also, cppc cpufreq driver can register an artificial energy model. In such case, it needs the frequency for this compute capacity. Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> --- drivers/base/arch_topology.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)