Re: [PATCHv1 7/8] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states

[Amit Kucheria]

On Fri, May 10, 2019 at 8:41 PM Marc Gonzalez <marc.w.gonzalez@xxxxxxx> wrote:
>
> On 10/05/2019 16:12, Amit Kucheria wrote:
>
> > On Fri, May 10, 2019 at 6:45 PM Marc Gonzalez wrote:
> >>
> >> On 10/05/2019 13:29, Amit Kucheria wrote:
> >>
> >>> Add device bindings for cpuidle states for cpu devices.
> >>>
> >>> Cc: Marc Gonzalez <marc.w.gonzalez@xxxxxxx>
> >>> Signed-off-by: Amit Kucheria <amit.kucheria@xxxxxxxxxx>
> >>> ---
> >>>   arch/arm64/boot/dts/qcom/msm8998.dtsi | 32 +++++++++++++++++++++++++++
> >>>   1 file changed, 32 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >>> index 3fd0769fe648..208281f318e2 100644
> >>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >>> @@ -78,6 +78,7 @@
> >>>                        compatible = "arm,armv8";
> >>>                        reg = <0x0 0x0>;
> >>>                        enable-method = "psci";
> >>> +                     cpu-idle-states = <&LITTLE_CPU_PD>;
> >>
> >> For some reason, I was expecting the big cores to come first, but according
> >> to /proc/cpuinfo, cores 0-3 are part 0x801, while cores 4-7 are part 0x800.
> >>
> >> According to https://github.com/pytorch/cpuinfo/blob/master/src/arm/uarch.c
> >>
> >> 0x801 = Low-power Kryo 260 / 280 "Silver" -> Cortex-A53
> >> 0x800 = High-performance Kryo 260 (r10p2) / Kryo 280 (r10p1) "Gold" -> Cortex-A73
> >
> > Hmm, did I mess up the order of the big and LITTLE cores?
> > I'll take a look again.
>
> Sorry for being unclear. I was saying I expected the opposite,
> but it appears the order in your patch is correct ;-)

OK :-)

> Little cores have higher latency (+5%) than big cores?

No, that is a result of me naively converting the downstream numbers
into cpuidle parameters for upstream. There is scope for tuning those
numbers with more instrumentation. My hope is that we will attract
more contributions once the basic idle states have landed upstream
i.e. change the story from "cpuidle isn't supported in upstream QC
platforms" to "cpuidle needs some tuning"

Regards,
Amit