Am Sonntag, 30. Juni 2024, 00:01:41 CEST schrieb Diederik de Haas: > 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 issue I had was more about the #ifdef'ery and then having a board define a constant to enable one or the other. As far as I understood the description, the OPP itself is the same in terms of frequency and voltage, just the regulator can't fully realize that target voltage, so the solution is to allow a voltage range, to also support the less-exact regulator. On the rk3588 on the other hand the soc variants have different OPP tables themselfs, because the soc itself only supports different frequencies+voltages. So the solution here is the split of the OPPs so that we don't mess around with /delete-node/ edits of one OPP table. So TL;DR separate OPP tables are the way to go if the user needs different freq+voltage values and voltage ranges allows boards to use less-adapted regulators. > > 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. Correct. Values from some "random" Radxa kernel would also not be my selection of choice. In the mainline-kernel we always want the save choice - which in for me is Rockchip's. If people want to experiment with other values on their own boards to sort of overclock their chips, that's their prerogative. Heiko > > 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 >