Hi Alexey,
On 5/8/24 11:43 AM, Alexey Charkov wrote:
Hi Quentin,
On Wed, May 8, 2024 at 1:12 PM Quentin Schulz <quentin.schulz@xxxxxxxxx> 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?
The one provided by Rockchip directly :) No intermediates.
I can give you the one we use on our products at the moment:
https://git.embedded.cherry.de/tiger-linux.git/ (or jaguar-linux,
doesn't matter).
The one that is (publicly) "maintained" by Rockchip is:
https://github.com/rockchip-linux/kernel/tree/develop-5.10
From Cherry's git repo:
"""
$ rg -B1 --color never -N opp-supported-hw
arch/arm64/boot/dts/rockchip/rk3588s.dtsi
opp-408000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-600000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-816000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1008000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1200000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1416000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1608000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-1704000000 {
opp-supported-hw = <0x02 0xffff>;
--
opp-1800000000 {
opp-supported-hw = <0xf9 0xffff>;
--
opp-408000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-600000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-816000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1008000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1200000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1416000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1608000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1800000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-2016000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-2208000000 {
opp-supported-hw = <0xf9 0xffff>;
--
opp-2256000000 {
opp-supported-hw = <0xf9 0x13>;
--
opp-2304000000 {
opp-supported-hw = <0xf9 0x24>;
--
opp-2352000000 {
opp-supported-hw = <0xf9 0x48>;
--
opp-2400000000 {
opp-supported-hw = <0xf9 0x80>;
--
opp-408000000 {
opp-supported-hw = <0xff 0x0ffff>;
--
opp-600000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-816000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1008000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1200000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1416000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1608000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-1800000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-2016000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-2208000000 {
opp-supported-hw = <0xf9 0xffff>;
--
opp-2256000000 {
opp-supported-hw = <0xf9 0x13>;
--
opp-2304000000 {
opp-supported-hw = <0xf9 0x24>;
--
opp-2352000000 {
opp-supported-hw = <0xf9 0x48>;
--
opp-2400000000 {
opp-supported-hw = <0xf9 0x80>;
--
opp-300000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-400000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-500000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-600000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-700000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-800000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-900000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-1000000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-300000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-400000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-500000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-600000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-700000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-800000000 {
opp-supported-hw = <0xff 0xffff>;
--
opp-900000000 {
opp-supported-hw = <0xfb 0xffff>;
--
opp-1000000000 {
opp-supported-hw = <0xfb 0xffff>;
"""
In order: LITTLE, big0, big1, DMC (memory), GPU and then NPU OPP table.
Looking at the 6.1 development branch from Rockchip
(https://github.com/JeffyCN/mirrors/blob/kernel-6.1). The LITTLE cluster
OPPs seem to all be using the same opp-supported-hw entry now (but
different from the one in 5.10). But, the big cluster OPPs in 6.1 are
matching the one in 5.10 (that is, not the ones from Radxa).
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)
Seems to be a change made by Radxa folks:
https://github.com/radxa/kernel/commit/cf277d5eb46ef55517afffa10d48dd71bdd00c61
(yay to no commit log \o/)
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 :)
An OPP from the DT is selected if _opp_is_supported returns true. This
is based on supported_hw member of the opp_table, which we set through
dev_pm_opp_set_supported_hw. This is called by
drivers/cpufreq/rockchip-cpufreq.c with two values: SoC Version and
Speed Grade. The SoC version is a bitmap set by rk3588_get_soc_info by
reading specification_serial_number region in the OTP and reading the
first byte. If it is anything but 0xd (RK3588M) or 0xa (RK3588J), it is
BIT(0).
To know if the opp is supported, you extract the first value of the
array and mask it with the value gotten from rk3588_get_soc_info (the
bitfield). This means that for RK3588 (and not the M or J variant), the
first value of the OPP opp-supported-hw is a match if it is an odd
number, so only opp-1704000000 in LITTLE cluster is excluded (on that
sole match).
The second value in opp-supported-hw seems to be derived somehow from
the cpu_leakage OTP. This is likely the same rabbit hole you dug two
months ago, so I'll trust your findings there to avoid getting my hands
dirty :)
In summary, false alarm (but still surprising changes made by Radxa
here, not that they matter if they only run their kernel on "pure"
RK3588). Sorry for the noise, and thanks for the explanations :)
I'm surprised that we removed the lowest frequencies at the same
voltage, are they not even allowing us to save a teeny tiny bit of power
consumption? (I'm asking because I'm pretty sure we'll eventually get
customers complaining the CPU freq doesn't go in super low frequency "so
this must be a way to consume less power in idle!").
Cheers,
Quentin
