Re: [PATCH v4 0/6] RK3588 and Rock 5B dts additions: thermal, OPP and fan

[Dragan Simic <dsimic@manjaro.org>]

Hello Quentin,

On 2024-05-28 18:08, Quentin Schulz wrote:
> On 5/28/24 5:42 PM, Alexey Charkov wrote:
>> On Tue, 28 May 2024 at 19:16, Heiko Stuebner <heiko@sntech.de> wrote:
>>> Am Dienstag, 28. Mai 2024, 17:01:48 CEST schrieb Dragan Simic:
>>>> On 2024-05-28 16:34, Heiko Stuebner wrote:
>>>>> Am Dienstag, 28. Mai 2024, 16:05:04 CEST schrieb Dragan Simic:
>>>>>> On 2024-05-28 11:49, Alexey Charkov wrote:
>>>>>>> Do you think this can be merged for 6.11? Looks like there hasn't
>>> been
>>>>>>> any new feedback in a while, and it would be good to have 
>>>>>>> frequency
>>>>>>> scaling in place for RK3588.
>>>>>>> 
>>>>>>> Please let me know if you have any reservations or if we need any
>>>>>>> broader discussion.
>>>>> 
>>>>> not really reservations, more like there was still discussion going 
>>>>> on
>>>>> around the OPPs. Meanwhile we had more discussions regarding the 
>>>>> whole
>>>>> speed binning Rockchip seems to do for rk3588 variants.
>>>>> 
>>>>> And waiting for the testing Dragan wanted to do ;-) .
>>>> 
>>>> I'm sorry for the delays.
>>> 
>>> Was definitly _not_ meant as blame ;-) .
>>> 
>>> The series has just too many discussions threads to unravel on half
>>> an afternoon.
>> 
>> FWIW, I think the latest exchange we had with Quentin regarding the 
>> OPPs
>> concluded in “false alarm”, given that this version of the series only
>> introduces a subset of them which should apply to all RK3588(s)
> 
> Correct.
> 
> However... I'm wondering if we shouldn't somehow follow the same
> pattern we have used for the rk3399 OPPs? We have a file for the
> "true" RK3399 OPPs, then the OP1 variant and the RK3399T.

If I'm not mistaken, the separate rk3399-*opp*.dtsi files were
added when the need arose.

> We already know there are a few variants of RK3588 with different
> OPPs: RK3588(S/S2?), RK3588J and RK3588M. I wouldn't be surprised if
> the RK3582 (though this one has already one big cluster (or two big
> cores) fewer than RK3588) has different OPPs as well?

Do we already have supported boards that use the RK3588J and
RK3588M variants of the RK3588 SoC?  If yes, we should separate
the relevant OPPs into the separate .dtsi files, but if not, we
should wait until the need arises.

> So. We have already discussed that the OPPs in that patch are valid
> for RK3588(S) but they aren't for the other variants.

... which applies currently if there are already other RK3588
variants in use on the supported boards.

> In the downstream kernel, any OPP whose opp-supported-hw has a first
> value masked by BIT(1) return non-0 is supported by RK3588M. In the
> downstream kernel, any OPP whose opp-supported-hw has a first value
> masked by BIT(2) return non-0 is supported by RK3588J.
> 
> This means that, for LITTLE clusters:
> - opp-1608000000 not supported on RK3588J
> - opp-1704000000 only supported on RK3588M (but already absent in this
> patch series)
> - opp-1800000000 only supported on RK3588(S), not RK3588J nor RK3588M
> 
> For big clusters:
> - opp-1800000000 not supported on RK3588J
> - opp-2016000000 not supported on RK3588J
> - opp-2208000000 only supported on RK3588(S), not RK3588J nor RK3588M
> - opp-2256000000 only supported on RK3588(S), not RK3588J nor RK3588M
> - opp-2304000000 only supported on RK3588(S), not RK3588J nor RK3588M
> - opp-2352000000 only supported on RK3588(S), not RK3588J nor RK3588M
> - opp-2400000000 only supported on RK3588(S), not RK3588J nor RK3588M
> 
> This is somehow also enforced in downstream kernel by removing the OPP
> nodes directly (hence, not even requiring the check of
> opp-supported-hw value), c.f.:
> https://git.theobroma-systems.com/tiger-linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> https://git.theobroma-systems.com/tiger-linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588m.dtsi
> 
> You'll not that the RK3588J also has less OPPs for the GPU and NPU
> (but those should also be masked by the opp-supported-hw value).