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

[Dietmar Eggemann]

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.

[...]