Re: [PATCH V5 2/2] base/drivers/arch_topology: Default dmips-mhz if they are not set in DT

From: Dietmar Eggemann
Date: Mon Dec 03 2018 - 08:46:17 EST


Hi Daniel,

+cc: Russell King <linux@xxxxxxxxxxxxxxx>

On 11/27/18 2:24 PM, 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.

[...]

I'm afraid that this change is incompatible with the still existing cpu_efficiency interface we have in Arm32 for A15/A7 systems like Arm TC2:

In case you specify clock-frequency dt properties per cpu for such a system, the cpu_capacity values are determined via the cpu_efficiency code in arch/arm/kernel/topology.c.

So on Arm TC2 with clock-frequency = <1000000000> [A15] and <800000000> [A7] you get:

root@linaro-nano:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity
606
1441
1441
606
606

With your patches on top (cpu_capacity functionality in drivers/base/arch_topology.c does not have to be switched on by specifying capacity-dmips-mhz dt properties anymore) we end up scaling by max frequency again:

root@linaro-nano:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity
358
1024
1024
358
358

I tried to remove the cpu_efficiency based API a year ago but Russell pointed out that the compatibility has to be maintained for longer:

https://lore.kernel.org/lkml/20171024102718.16113-1-dietmar.eggemann@xxxxxxx/

I assume that the capacity-dmips-mhz dt property is like a switch to turn this functionality on for big.Little and so called gold/silver platforms, which have cores with the same uArch but in frequency domains with different max frequency values.

So what's wrong with specifying capacity-dmips-mhz = <1024> for all cores for those gold/silver platforms? I don't expect that there will be so many of them. And normal SMP platforms (w/o frequency domains w/o different max frequency values) don't have to execute this code.

IMHO, at least we should remove the cpu_efficiency bits before we do this change.

[...]