Re: [PATCH v4 6/6] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588

[Alexey Charkov <alchark@gmail.com>]

Hi Quentin,

On Wed, May 8, 2024 at 1:12 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi Alexey,
>
> On 5/6/24 11:36 AM, Alexey Charkov wrote:
> > By default the CPUs on RK3588 start up in a conservative performance
> > mode. Add frequency and voltage mappings to the device tree to enable
> > dynamic scaling via cpufreq.
> >
> > OPP values are adapted from Radxa's downstream kernel for Rock 5B [1],
> > stripping them down to the minimum frequency and voltage combinations
> > as expected by the generic upstream cpufreq-dt driver, and also dropping
> > those OPPs that don't differ in voltage but only in frequency (keeping
> > the top frequency OPP in each case).
> >
> > Note that this patch ignores voltage scaling for the CPU memory
> > interface which the downstream kernel does through a custom cpufreq
> > driver, and which is why the downstream version has two sets of voltage
> > values for each OPP (the second one being meant for the memory
> > interface supply regulator). This is done instead via regulator
> > coupling between CPU and memory interface supplies on affected boards.
> >
>
> I'm not sure this is everything we need though.
>
> For the LITTLE cores cluster, all OPPs up to 1.416GHz are using the same
> opp-supported-hw, however the ones above, aren't.

Thanks a lot for pointing this out - could you please elaborate which
downstream kernel you referred to?

> 1.608GHz, 1.704GHz and 1.8GHz are all using different opp-supported-hw.

In Radxa's downstream kernel source that I looked at [1] the LITTLE
core cluster has all OPPs listed with opp-supported-hw = <0xff
0xffff>;

> Similarly, for the big cores clusters, all OPPs up to 1.608GHz are using
> the same opp-supported-hw, but not the ones above.
>
> 1.8GHz and 2.016GHz, 2.208GHz, 2.256GHz, 2.304GHz, 2.352GHz and 2.4GHz
> all have a different opp-supported-hw.

Hmm, only 2.256GHz, 2.304GHz and 2.352GHz in the sources I'm looking
at have a different opp-supported-hw = <0xff 0x0>; (but note that I
dropped them all from my patch here)

> The values in that array are coming from cpu leakage (different for
> LITTLE, big0 and big1 clusters) and "specification serial number"
> (whatever that means), those are coming from the SoC OTP. In the
> downstream kernel from Rockchip, the former value is called "SoC
> Version" and the latter "Speed Grade".

From what I understood by studying Radxa's downstream kernel sources
and TF-A sources [2], the "leakage" in NVMEM cells drives the
selection of power-optimized voltage levels (opp-microvolt-L1 through
opp-microvolt-L7) for each OPP depending on a OTP-programmed silicon
quality metric, whereas in my patch I only kept the most conservative
voltage values for each OPP (i.e. highest-voltage default ones) and
not the power-optimized ones.

So the proposed patch should (supposedly?) work on any silicon, only
the heat death of the universe becomes marginally closer :)

> I think this may have something to do with "binning" and I would see the
> ones above the "common" OPPs as "overclocking". Not all CPUs would
> support them and some may not run stable at some lower frequency than
> their stable max. Adding Kever from Rockchip in Cc to have some input on
> the need to support those.

Would be great to understand those in more detail, indeed!

Thanks a lot,
Alexey

[1] https://github.com/radxa/kernel/blob/c428536281d69aeb2b3480f65b2b227210b61535/arch/arm64/boot/dts/rockchip/rk3588s.dtsi#L588
[2] https://lore.kernel.org/linux-rockchip/CABjd4YzTL=5S7cS8ACNAYVa730WA3iGd5L_wP1Vn9=f83RCORA@mail.gmail.com/