Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov: > On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@xxxxxxxxx> wrote: > > On 6/17/24 8:28 PM, Alexey Charkov wrote: > > > RK3588j is the 'industrial' variant of RK3588, and it uses a different > > > set of OPPs both in terms of allowed frequencies and in terms of > > > applicable voltages at each frequency setpoint. > > > > > > Add the OPPs that apply to RK3588j (and apparently RK3588m too) to > > > enable dynamic CPU frequency scaling. > > > > > > OPP values are derived from Rockchip downstream sources [1] by taking > > > only those OPPs which have the highest frequency for a given voltage > > > level and dropping the rest (if they are included, the kernel complains > > > at boot time about them being inefficient) > > > > > > [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > > > > > > > Funny stuff actually. Rockchip have their own downstream cpufreq driver > > which autodetects at runtime the SoC variant and filter out OPPs based > > on that info. And this patch is based on those values and filters. > > > > However, they also decided that maybe this isn't the best way to do it > > and they decided to have an rk3588j.dtsi where they remove some of those > > OPPs instead of just updating the mask/filter in their base dtsi > > (rk3588s.dtsi downstream). See > > https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi > > Funny stuff indeed! Judging by the comments in the file you > referenced, those higher OPP values constitute an 'overdrive' mode, > and apparently the chip shouldn't stay in those for prolonged periods > of time. However, I couldn't locate any dts file inside Rockchip > sources that would include this rk3588j.dtsi - so not sure if we > should follow what it says too zealously. > > > So... > > > > > Signed-off-by: Alexey Charkov <alchark@xxxxxxxxx> > > > --- > > > arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++ > > > 1 file changed, 108 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi > > > index 0bbeee399a63..b7e69553857b 100644 > > > --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi > > > +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi > > > @@ -5,3 +5,111 @@ > > > */ > > > > > > #include "rk3588-extra.dtsi" > > > + > > > +/ { > > > + cluster0_opp_table: opp-table-cluster0 { > > > + compatible = "operating-points-v2"; > > > + opp-shared; > > > + > > > + opp-1416000000 { > > > + opp-hz = /bits/ 64 <1416000000>; > > > + opp-microvolt = <750000 750000 950000>; > > > + clock-latency-ns = <40000>; > > > + opp-suspend; > > > + }; > > > + opp-1608000000 { > > > + opp-hz = /bits/ 64 <1608000000>; > > > + opp-microvolt = <887500 887500 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-1704000000 { > > > + opp-hz = /bits/ 64 <1704000000>; > > > + opp-microvolt = <937500 937500 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > > None of those are valid according to Rockchip, we should have > > Well, valid but more taxing on the hardware apparently. > > > opp-1296000000 { > > opp-hz = /bits/ 64 <1296000000>; > > opp-microvolt = <750000 750000 950000>; > > clock-latency-ns = <40000>; > > opp-suspend; > > }; > > > > instead? > > I dropped this one because it uses a lower frequency but same voltage > as the 1.416 GHz one, thus being 'inefficient' as the thermal > subsystem says in the logs. It can be added back if we decide to > remove opp-1416000000. > > > and... > > > > > + }; > > > + > > > + cluster1_opp_table: opp-table-cluster1 { > > > + compatible = "operating-points-v2"; > > > + opp-shared; > > > + > > > + opp-1416000000 { > > > + opp-hz = /bits/ 64 <1416000000>; > > > + opp-microvolt = <750000 750000 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-1608000000 { > > > + opp-hz = /bits/ 64 <1608000000>; > > > + opp-microvolt = <787500 787500 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-1800000000 { > > > + opp-hz = /bits/ 64 <1800000000>; > > > + opp-microvolt = <875000 875000 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-2016000000 { > > > + opp-hz = /bits/ 64 <2016000000>; > > > + opp-microvolt = <950000 950000 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > > opp-1800000000 and opp-2016000000 should be removed. > > > > and... > > > > > + }; > > > + > > > + cluster2_opp_table: opp-table-cluster2 { > > > + compatible = "operating-points-v2"; > > > + opp-shared; > > > + > > > + opp-1416000000 { > > > + opp-hz = /bits/ 64 <1416000000>; > > > + opp-microvolt = <750000 750000 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-1608000000 { > > > + opp-hz = /bits/ 64 <1608000000>; > > > + opp-microvolt = <787500 787500 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-1800000000 { > > > + opp-hz = /bits/ 64 <1800000000>; > > > + opp-microvolt = <875000 875000 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > + opp-2016000000 { > > > + opp-hz = /bits/ 64 <2016000000>; > > > + opp-microvolt = <950000 950000 950000>; > > > + clock-latency-ns = <40000>; > > > + }; > > > > opp-1800000000 and opp-2016000000 should be removed as well. > > > > Did I misunderstand what Rockchip did here? Adding Kever in Cc at least > > for awareness on Rockchip side :) > > > > I guess another option could be to mark those above as > > > > turbo-mode; > > > > though no clue what this would entail. From a glance at cpufreq, it > > seems that for Rockchip since we use the default cpufreq-dt, it would > > mark those as unusable, so essentially a no-op, so better remove them > > entirely? > > I believe the opp core just marks 'turbo-mode' opps as > CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the > cpufreq core. But then to actually use those boost frequencies I would > guess the governor needs to somehow know the power/thermal constraints > of the chip?.. Don't know where that is defined. personally I don't believe in "boost" frequencies - except when they are declared officially. Rockchip for the longest time maintains that running the chip outside the declared power/rate envelope can reduce its overall lifetime. So for Rockchip in mainline a "it runs stable for me" has never been a suitable reasoning ;-) . So while the situation might be strange for the rk3588j, I really only want correct frequencies here please - not any pseudo "turbo freqs". I don't care that much what people do on their own device{s/trees}, but the devicetrees we supply centrally (and to u-boot, etc) should only contain frequencies vetted by the manufacturer. Heiko
