Message-ID: <e55125ed-64fb-455e-b1e4-cebe2cf006e4@cherry.de>
Date: Wed, 12 Mar 2025 11:15:42 +0100
From: Quentin Schulz <quentin.schulz@...rry.de>
To: Alexey Charkov <alchark@...il.com>, Heiko Stübner
 <heiko@...ech.de>
Cc: Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Daniel Lezcano <daniel.lezcano@...aro.org>, Dragan Simic
 <dsimic@...jaro.org>, Viresh Kumar <viresh.kumar@...aro.org>,
 Chen-Yu Tsai <wens@...nel.org>, Diederik de Haas <didi.debian@...ow.org>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Kever Yang <kever.yang@...k-chips.com>
Subject: Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores
 on RK3588j

Hi all,

On 2/16/25 1:32 PM, Alexey Charkov wrote:
> Hi Heiko,
> 
> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@...ech.de> 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@...rry.de> wrote:
>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
[...]
>>>>> +     };
>>>>> +
>>>>> +     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 ;-) .
> 
> My key concern here was perhaps that we are looking at a file inside
> the Rockchip source tree which looks relevant by the name of it, but
> doesn't actually get included anywhere for any of the boards. This may
> or may not constitute an endorsement by Rockchip of any OPPs listed
> there :-D
> 

Rockchip support stated:

"""
If you run overdrive mode, you do not need to include rk3588j.dtsi, and 
you can change it to #incldue rk3588.dtsi to ensure that the maximum 
frequency can reach 2GHz

below picture from datasheet.
"""

The picture is the table 3-2 Recommended operating conditions, page 30 
from the RK3588J datasheet, e.g. 
https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf

What is overdrive?

"""
Overdrive mode brings higher frequency, and the voltage will increase 
accordingly. Under
the overdrive mode for a long time, the chipset may shorten the 
lifetime, especially in
high temperature condition
"""

according to the same datasheet, end of the same table, page 31.

so this seems like a turbo-mode frequency to me.

Additionally, the note for the "normal mode" (the one with the lower 
freqs) states:

"""
Normal mode means the chipset works under safety voltage and frequency. 
For the
industrial environment, highly recommend to keep in normal mode, the 
lifetime is
reasonably guaranteed.
"""

I believe "industrial environment" means RK3588J used in the 
temperatures that aren't OK for the standard RK3588 variant?

> I haven't seen a TRM for the J variant, nor do I have a board with
> RK3588J to run tests, so it would be great if Kever could chime in
> with how these OPPs are different from the others (or not).
> 
>> 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.
> 
> Fair enough. Let's just try and get another data point on whether
> these frequencies are vetted or not.
> 

So the higher freqs seems to be vetted (and used by default on 
Rockchip's BSP kernel), it just feels like you aren't really supposed to 
run those higher frequencies all the time? And especially not in 
"industrial environment"?

I would assume we want to stay on the safer side and remove those higher 
frequencies? Heiko?

Cheers,
Quentin

Powered by blists - more mailing lists