Re: [PATCH] arm64: dts: rockchip: Remove overdrive-mode OPPs from RK3588J SoC dtsi

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Dragan,

On 3/23/25 11:19 AM, Dragan Simic wrote:
> Hello Quentin,
> 
> Thanks for your comments!  Please see some responses below.
> 
> On 2025-03-21 10:53, Quentin Schulz wrote:
>> On 3/21/25 4:28 AM, Dragan Simic wrote:
>>> The differences in the vendor-approved CPU and GPU OPPs for the standard
>>> Rockchip RK3588 variant [1] and the industrial Rockchip RK3588J 
>>> variant [2]
>>> come from the latter, presumably, supporting an extended temperature 
>>> range
>>> that's usually associated with industrial applications, despite the 
>>> two SoC
>>> variant datasheets specifying the same upper limit for the allowed 
>>> ambient
>>> temperature for both variants.  However, the lower temperature limit is
>>
>> RK3588 is rated for 0-80°C, RK3588J for -40-85°C, c.f. Recommended
>> Operating Conditions, Table 3-2, Ambient Operating Temperature.
> 
> Indeed, which is why I specifically wrote "specifying the same upper
> limit", because having a lower negative temperature limit could hardly
> put the RK3588J in danger of overheating or running hotter. :)
> 

"""
despite the two SoC variant datasheets specifying the same upper limit 
for the allowed temperature for both variants
"""

is incorrect. The whole range is different, yes it's only a 5°C 
difference for the upper limit, but they still are different.

>>> specified much lower for the RK3588J variant. [1][2]
>>>
>>> To be on the safe side and to ensure maximum longevity of the RK3588J 
>>> SoCs,
>>> only the CPU and GPU OPPs that are declared by the vendor to be 
>>> always safe
>>> for this SoC variant may be provided.  As explained by the vendor [3] 
>>> and
>>> according to its datasheet, [2] the RK3588J variant can actually run 
>>> safely
>>> at higher CPU and GPU OPPs as well, but only when not enjoying the 
>>> assumed
>>> extended temperature range that the RK3588J, as an SoC variant targeted
>>
>> "only when not enjoying the assumed extended temperature range" is
>> extrapolated by me/us and not confirmed by Rockchip themselves. I've
>> asked for a statement on what "industrial environment" they specify in
>> the Normal Mode explanation means since it's the only time they use
>> the term. I've yet to receive an answer. The only thing Rockchip in
>> their datasheet is that the overdrive mode will shorten lifetime when
>> used for a long time, especially in high temperature conditions. It's
>> not clear whether we can use the overdrive mode even within the RK3588
>> typical range of operation.
> 
> True.  I'll see to rephrase the patch description a bit in the v2,
> to avoid this kind of speculation.  I mean, perhaps the speculation
> is right, but it hasn't been confirmed officially by Rockchip.
> 

Speculation is fine, but it should be worded as such.

[...]

>>> The provided RK3588J CPU OPPs follow the slightly debatable "provide 
>>> only
>>> the highest-frequency OPP from the same-voltage group" approach 
>>> that's been
>>
>> Interesting that we went for a different strategy for the GPU OPPs :)
> 
> Good point, and I'm fully aware of that. :)  Actually, I'm rather
> sure that omitting the additional CPU OPPs does no good to us, but
> I didn't want to argue about that when they were dropped originally,
> before I can have some hard numbers to prove it in a repeatable way.
> 

I assume we'll have some patch in the future with those added and those 
hard numbers you're talking about, so looking forward to seeing it on 
the ML :)

[...]

>>> Helped-by: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> Reported-by/Suggested-by?
>>
>> I don't see Helped-by in
>> https://eur02.safelinks.protection.outlook.com/? 
>> url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Flatest%2Fprocess%2Fsubmitting-patches.html%23using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cdc754791b6844506b11c08dd69f444a7%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638783220330058516%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=4bv9pUh6aSD0GVLJ4Zvuyvox1K0xxwf83KXX86QsvMo%3D&reserved=0
>>
>> I see 2496b2aaacf137250f4ca449f465e2cadaabb0e8 got the Helped-by
>> replaced by a Suggested-by for example, but I see other patches with
>> Helped-by... if that is a standard trailer for kernel patches, then
>> maybe we should add it to that doc?
> 
> Actually, I already tried to get the Helped-by tag added to the
> kernel documentation, by submitting a small patch series. [*]
> Unfortunately, it got rejected. :/
> 
> However, Heiko accepts Helped-by tags and nobody higher up the
> tree seems to complain, so we should be fine. :)  It isn't the
> case with all maintainers, though.
> 
> [*] https://eur02.safelinks.protection.outlook.com/? 
> url=https%3A%2F%2Flore.kernel.org%2Fall%2Fcover.1730874296.git.dsimic%40manjaro.org%2FT%2F%23u&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cdc754791b6844506b11c08dd69f444a7%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638783220330070422%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=3dZgSG%2FBT6f%2Ffqs7D30HvEl18SzqYPwNeUGWBZfMAqM%3D&reserved=0
> 

Are you trying to up the numbers of Helped-by in commit logs to make it 
a reasonable request to add the trailer in the documentation :) ?

Cheers,
Quentin