Hi Dragan, On Sat Oct 12, 2024 at 9:45 PM CEST, Dragan Simic wrote: > On 2024-10-12 21:27, Diederik de Haas wrote: > > On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote: > >> Update the lower/upper voltage limits and the exact voltages for the > >> Rockchip > >> RK356x CPU OPPs, using the most conservative values (i.e. the highest > >> per-OPP > >> voltages) found in the vendor kernel source. [1] > >> > >> Using the most conservative per-OPP voltages ensures reliable CPU > >> operation > >> regardless of the actual CPU binning, with the downside of possibly > >> using > >> a bit more power for the CPU cores than absolutely needed. > >> > >> Additionally, fill in the missing "clock-latency-ns" CPU OPP > >> properties, using > >> the values found in the vendor kernel source. [1] > >> > >> [1] > >> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi > >> > >> Related-to: eb665b1c06bc ("arm64: dts: rockchip: Update GPU OPP > >> voltages in RK356x SoC dtsi") > >> Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx> > >> --- > >> arch/arm64/boot/dts/rockchip/rk3568.dtsi | 1 + > >> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 18 ++++++++++++------ > >> 2 files changed, 13 insertions(+), 6 deletions(-) > >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > >> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > >> index 0946310e8c12..5c54898f6ed1 100644 > >> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > >> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > >> @@ -273,6 +273,7 @@ &cpu0_opp_table { > >> opp-1992000000 { > >> opp-hz = /bits/ 64 <1992000000>; > >> opp-microvolt = <1150000 1150000 1150000>; > >> + clock-latency-ns = <40000>; > >> }; > >> }; > >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > >> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > >> index 0ee0ada6f0ab..534593f2ed0b 100644 > >> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > >> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > >> @@ -134,39 +134,45 @@ cpu0_opp_table: opp-table-0 { > >> > >> opp-408000000 { > >> opp-hz = /bits/ 64 <408000000>; > >> - opp-microvolt = <900000 900000 1150000>; > >> + opp-microvolt = <850000 850000 1150000>; > >> clock-latency-ns = <40000>; > >> }; > >> > >> opp-600000000 { > >> opp-hz = /bits/ 64 <600000000>; > >> - opp-microvolt = <900000 900000 1150000>; > >> + opp-microvolt = <850000 850000 1150000>; > >> + clock-latency-ns = <40000>; > >> }; > >> > >> opp-816000000 { > >> opp-hz = /bits/ 64 <816000000>; > >> - opp-microvolt = <900000 900000 1150000>; > >> + opp-microvolt = <850000 850000 1150000>; > >> + clock-latency-ns = <40000>; > >> opp-suspend; > >> }; > > > > While it felt a bit much to send a patch just to remove the blank lines > > between the opp nodes, this sounds like an excellent opportunity to > > make it consistent with the opp list in other DT files? > > Actually, my plan is to work on the SoC binning, which will involve > touching nearly every OPP in the Rockchip DTs, and will add much more > data to each OPP node. Thus, having empty lines as the separators > between the OPP nodes is something we should actually want, because As indicated in the "arm64: dts: rockchip: Add dtsi file for RK3399S SoC variant" patch series, I do prefer the separator lines ... > not having them will actually reduce the readability after the size > of the individual OPP nodes is increased. > > That's the reason why I opted for having the separator lines in this > patch series, i.e. because having them everywhere should be the final > outcome, and because in this case they were already present where the > OPPs were moved or copied from. ... but you actually removed those lines in the other patch set. While I'm looking forward to the extra data to the OPP nodes, I don't think the amount of properties should determine whether it should have a separator line or not. My 0.02 Cheers, Diederik
Attachment:
signature.asc
Description: PGP signature
