Message-ID: <2cf3889741f84584d8f23a4407fed08e@manjaro.org>
Date: Thu, 13 Mar 2025 11:42:17 +0100
From: Dragan Simic <dsimic@...jaro.org>
To: Quentin Schulz <quentin.schulz@...rry.de>
Cc: Alexey Charkov <alchark@...il.com>, Heiko Stübner
 <heiko@...ech.de>, 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>, 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

Hello all,

On 2025-03-12 11:34, Dragan Simic wrote:
> On 2025-03-12 11:15, Quentin Schulz wrote:
>> On 2/16/25 1:32 PM, Alexey Charkov wrote:
>>> 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?
> 
> Thanks a lot for obtaining these clarifications!
> 
> Yes, I'd say that in this case "industrial environment" boils down
> to the extended temperature range for the RK3588J 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?
> 
> I agree, we should remove the higher-frequency OPPs.  I'd like
> to go through everything once again in detail and prepare a patch
> that removes the unsafe OPPs, and you could review it once it's
> on the ML, to make sure it's fine.

Just as a note, everything above (and even a bit more) is confirmed
and clearly described in the publicly available RK3588J datasheet,
which I'll provide as a reference in my upcoming patch.

Powered by blists - more mailing lists