Re: [PATCH v2] arm64: dts: rockchip: Add GPU OPP voltage ranges to RK356x SoC dtsi

[Diederik de Haas <didi.debian@cknow.org>]

[-- Attachment #1: Type: text/plain, Size: 2676 bytes --]

On Saturday, 29 June 2024 18:39:02 CEST Dragan Simic wrote:
> Add support for voltage ranges to the GPU OPPs defined in the SoC dtsi for
> RK356x.  These voltage ranges are useful for RK356x-based boards that are
> designed to use the same power supply for the GPU and NPU portions of the
> SoC, which is described further in the following documents:
> 
>   - Rockchip RK3566 Hardware Design Guide, version 1.1.0, page 37
>   - Rockchip RK3568 Hardware Design Guide, version 1.2, page 78

That was interesting to read, thanks.
Now I understand the difference between rk809(-5) and rk817(-5).

But AFAIUI the above description described why there were separate tables for 
rk809 and rk817 in v1. But that was dropped in v2. So it seems to me the 
(commit) message should be updated accordingly?

I also expected that (for v1) there would be a similar construct as was 
recently added for rk3588. But I should interpret Heiko's comments as that 
strategy should not be applied to rk356x?

> The values for the exact GPU OPP voltages and the lower limits for the GPU
> OPP voltage ranges differ from the values found in the vendor kernel source
> (cf. downstream commit f8b9431ee38e ("arm64: dts: rockchip: rk3568: support
> adjust opp-table by otp")). [1][2]  

Why? In their latest update Rockchip changed it to the values as specified in 
the links. My assumption is that based on extensive testing they did and/or 
the feedback they got from the client/customers, they felt the need to change 
it to the values they did.

I think we should follow their values unless we have an explicit and very good 
reason to deviate from that.

> However, our values have served us well so far, so let's keep them for now,

And I don't think that qualifies as a (very) good reason.
I think it's reasonable to assume that far more (stress) testing has been done 
with the downstream code, then has happened with the upstream code.
Hopefully that'll change in the future, but I don't think we're there yet.

When we/upstream adds npu support, I think we should also follow downstream's 
OPP values, unless we have a very good reason to deviate from that.

> until we actually start supporting the CPU and GPU binning, together with
> the related voltage adjustments.

I may not fully understand what you mean by that, but I think it's (again) 
reasonable to assume that Rockchip has far more insight into this then we do.

Cheers,
  Diederik

> [1]
> https://github.com/rockchip-linux/kernel/commit/f8b9431ee38ed561650be7092ab
> 93f564598daa9 [2]
> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650
> be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]