[PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
Dragan Simic
dsimic at manjaro.org
Sat Feb 15 13:26:23 PST 2025
Hello all,
On 2025-02-15 21:30, Heiko Stübner wrote:
> 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 at cherry.de> 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 at gmail.com>
>> > > ---
>> > > 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.
FWIW, as an additional data point, some other SoC manufacturers
officially state that the expected lifetime of their chips gets
reduced when they are run at higher temperatures, which are still
within the permitted temperature ranges.
> 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.
Totally agreed, "works for me" is simply not applicable here. As we
know, some samples of the same CPU or SoC may be luckier when it comes
to the silicon lottery that got the binned, meaning that only the most
conservative frequencies and voltages, as assigned by the manufacturer,
are what we can provide as the OPPs.
I'm having a more detailed look into this, and I'll come back with,
hopefully, some additional comments. :)
More information about the linux-arm-kernel
mailing list