On 22/11/2018 05:29, Viresh Kumar wrote: > On 21-11-18, 23:12, Daniel Lezcano wrote: >> On 30/10/2018 09:58, Viresh Kumar wrote: >>> s/dmpis/dmips/ in $subject >>> >>> On 29-10-18, 17:23, Daniel Lezcano wrote: >>>> In the case of assymetric SoC with the same micro-architecture, we >>> >>> asymmetric ? >>> >>>> 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 comutation triggered, so we need to write >>> >>> computation >>> >>>> 'capacity-dmips-mhz' for each CPU with the same value in order to >>>> force the scaled capacity computation. >>>> >>>> Fix this by setting a default capacity to SCHED_CAPACITY_SCALE, if no >>>> 'capacity-dmips-mhz' is defined in the DT. >>>> >>>> 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 | 27 ++++++++++++++++++++++++++- >>>> 1 file changed, 26 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >>>> index 7311641..7d594a6 100644 >>>> --- a/drivers/base/arch_topology.c >>>> +++ b/drivers/base/arch_topology.c >>>> @@ -205,6 +205,21 @@ static struct notifier_block >>>> init_cpu_capacity_notifier = { >>>> .notifier_call = init_cpu_capacity_callback, >>>> }; >>>> >>>> +static int topology_set_default_capacity(void) >>>> +{ >>>> + int cpu; >>>> + >>>> + raw_capacity = kzalloc(num_possible_cpus() * sizeof(*raw_capacity), >>>> + GFP_KERNEL); >>>> + if (!raw_capacity) >>>> + return -ENOMEM; >>>> + >>>> + for_each_possible_cpu(cpu) >>>> + raw_capacity[cpu] = SCHED_CAPACITY_SCALE; >>> >>> This isn't actually required as the value of raw_capacity isn't used >>> anymore after this point in code. Rather it is forcefully updated in >>> init_cpu_capacity_callback(): >>> >>> raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) * >>> policy->cpuinfo.max_freq / 1000UL; >>> >>> Maybe it is better to allocate raw_capacity once at boot and use >>> another global variable as flag (raw_capacity is used as a flag right >>> now at many places). >> >> Can we keep the proposed change as is to simply fix the default value? >> >> I want to do a separate change with a raw_capacity rewrite and remove >> the workqueue freeing it. > > Sure. But you still don't need to update raw_capacity[cpu] above as I pointed > out earlier. You can drop those lines at least.
Oh ... actually raw_capacity is not needed at all! -- <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
