On 26/11/2018 10:52, Quentin Perret wrote: > On Monday 26 Nov 2018 at 09:44:21 (+0100), Daniel Lezcano wrote: >> In the case of asymmetric SoC with the same micro-architecture, we >> have a group of CPUs with smaller OPPs than the other group. One >> example is the 96boards dragonboard 820c. There is no dmips/MHz >> difference between both groups, so no need to specify the values in >> the DT. Unfortunately, without these defined, there is no scaling >> capacity computation triggered, so we need to write >> 'capacity-dmips-mhz' for each CPU with the same value in order to >> force the scaled capacity computation. >> >> In order to fix this situation, allocate 'raw_capacity' so the pointer >> is set and the init_cpu_capacity_callback() function can be called. >> >> This was tested on db820c: >> - specified values in the DT (correct results) >> - partial values defined in the DT (error + fallback to defaults) >> - no specified values in the DT (correct results) >> >> correct results are: >> cat /sys/devices/system/cpu/cpu*/cpu_capacity >> 758 >> 758 >> 1024 >> 1024 >> >> ... respectively for CPU0, CPU1, CPU2 and CPU3. >> >> That reflects the capacity for the max frequencies 1593600 and 2150400. >> >> Cc: Chris Redpath <[email protected]> >> Cc: Quentin Perret <[email protected]> >> Cc: Viresh Kumar <[email protected]> >> Cc: Amit Kucheria <[email protected]> >> Cc: Nicolas Dechesne <[email protected]> >> Cc: Niklas Cassel <[email protected]> >> Signed-off-by: Daniel Lezcano <[email protected]> >> --- >> drivers/base/arch_topology.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> index fd5325b..e0c5b60 100644 >> --- a/drivers/base/arch_topology.c >> +++ b/drivers/base/arch_topology.c >> @@ -243,9 +243,20 @@ static int __init register_cpufreq_notifier(void) >> * until we have the necessary code to parse the cpu capacity, so >> * skip registering cpufreq notifier. >> */ >> - if (!acpi_disabled || !raw_capacity) >> + if (!acpi_disabled) >> return -EINVAL; >> >> + if (!raw_capacity) { >> + >> + pr_info("cpu_capacity: No capacity defined in DT, set default " >> + "values to %ld\n", SCHED_CAPACITY_SCALE); >> + >> + raw_capacity = kmalloc_array(num_possible_cpus(), >> + sizeof(*raw_capacity), GFP_KERNEL); >> + if (!raw_capacity) >> + return -ENOMEM; >> + } >> + >> if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) { >> pr_err("cpu_capacity: failed to allocate memory for >> cpus_to_visit\n"); >> return -ENOMEM; >> -- >> 2.7.4 > > With this, if the DT is partially filled, we will still do the frequency > scaling thing now right ?
Right, if the DT is partially filled. We end up with the error, the raw_capacity is free and set to NULL. register_cpufreq_notifier() will allocate it and the capacity is computed. > I'm not sure if this is the expected behaviour. If the DT is partially > filled, we probably want to have 1024 of capacity for all CPUs to match > the doc. Yes if they have the same number of OPP which is the case of 99% of the boards (excluding the big Little). Otherwise setting all CPU with a capacity of 1024 but having different OPP (like qcom gold-silver arch) does not make sense and the patch fix this. > Maybe you want to test 'if (!raw_capacity || cap_parsing_failed)' at the > top of topology_parse_cpu_capacity() ? I prefer to update the documentation, it makes more sense than adding more cumbersome tests in the current code. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
