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. -- viresh
