| Message ID | cc2aed3116a57dd50e2bb15ab41b12784adfafe3.1728752527.git.dsimic@manjaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Update, encapsulate and expand the RK356x SoC dtsi files | expand |
Hi Dragan, On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote: > Rename the Rockchip RK356x SoC dtsi files and, consequently, adjust their > contents appropriately, to prepare them for the ability to specify different > CPU and GPU OPPs for each of the supported RK356x SoC variants. > > The first new RK356x SoC variant to be introduced is the RK3566T, which the > Pine64 Quartz64 Zero SBC is officially based on. [1] Some other SBCs are > also based on the RK3566T variant, including Radxa ROCK 3C and ZERO 3E/3W, > but the slight trouble is that Radxa doesn't state that officially. Though, > it's rather easy to spot the RK3566T on such boards, because their official > specifications state that the maximum frequency for the Cortex-A55 cores is > lower than the "full-fat" RK3566's 1.8 GHz. [2][3][4] I think we changed terminology from "full-fat" to something else in the rk3588 variant? Would be nice to be consisten. > These changes follow the approach used for the Rockchip RK3588 SoC variants, > which was introduced and described further in commit def88eb4d836 ("arm64: > dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs"). Please > see that commit for a more detailed explanation. > > No functional changes are introduced, which was validated by decompiling and No functional changes ... > comparing all affected board dtb files before and after these changes. In > more detail, the affected dtb files have some of their blocks shuffled around > a bit and some of their phandles have different values, as a result of the > changes to the order in which the building blocks from the parent dtsi files > are included, but they effectively remain the same as the originals. > > [1] https://wiki.pine64.org/wiki/Quartz64 > [2] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf > [3] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf > [4] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf > > Related-to: def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs") > Signed-off-by: Dragan Simic <dsimic@manjaro.org> > --- > .../{rk3566.dtsi => rk3566-base.dtsi} | 2 +- > arch/arm64/boot/dts/rockchip/rk3566.dtsi | 116 ++++++++++++++---- > arch/arm64/boot/dts/rockchip/rk3568.dtsi | 114 +++++++++++++++-- > .../{rk356x.dtsi => rk356x-base.dtsi} | 87 ------------- > 4 files changed, 202 insertions(+), 117 deletions(-) > copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} (95%) > rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} (96%) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi > similarity index 95% > copy from arch/arm64/boot/dts/rockchip/rk3566.dtsi > copy to arch/arm64/boot/dts/rockchip/rk3566-base.dtsi > index 6c4b17d27bdc..e56e0b6ba941 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > -#include "rk356x.dtsi" > +#include "rk356x-base.dtsi" > > / { > compatible = "rockchip,rk3566"; > diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi > index 6c4b17d27bdc..3fcca79279f7 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi > @@ -1,35 +1,107 @@ > // SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > -#include "rk356x.dtsi" > +#include "rk3566-base.dtsi" > > / { > - compatible = "rockchip,rk3566"; > + cpu0_opp_table: opp-table-0 { > + compatible = "operating-points-v2"; > + opp-shared; > + > + opp-408000000 { > + opp-hz = /bits/ 64 <408000000>; > + opp-microvolt = <850000 850000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-600000000 { > + opp-hz = /bits/ 64 <600000000>; > + opp-microvolt = <850000 850000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-816000000 { > + opp-hz = /bits/ 64 <816000000>; > + opp-microvolt = <850000 850000 1150000>; > + clock-latency-ns = <40000>; > + opp-suspend; > + }; Just like with patch 1 of this series, drop the blank line? > + > + opp-1104000000 { > + opp-hz = /bits/ 64 <1104000000>; > + opp-microvolt = <900000 900000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1416000000 { > + opp-hz = /bits/ 64 <1416000000>; > + opp-microvolt = <1025000 1025000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1608000000 { > + opp-hz = /bits/ 64 <1608000000>; > + opp-microvolt = <1100000 1100000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1800000000 { > + opp-hz = /bits/ 64 <1800000000>; > + opp-microvolt = <1150000 1150000 1150000>; > + clock-latency-ns = <40000>; > + }; > + }; > + > + gpu_opp_table: opp-table-1 { > + compatible = "operating-points-v2"; > + > + opp-200000000 { > + opp-hz = /bits/ 64 <200000000>; > + opp-microvolt = <850000 850000 1000000>; > + }; > + > + opp-300000000 { > + opp-hz = /bits/ 64 <300000000>; > + opp-microvolt = <850000 850000 1000000>; > + }; > + > + opp-400000000 { > + opp-hz = /bits/ 64 <400000000>; > + opp-microvolt = <850000 850000 1000000>; > + }; > + > + opp-600000000 { > + opp-hz = /bits/ 64 <600000000>; > + opp-microvolt = <900000 900000 1000000>; > + }; > + > + opp-700000000 { > + opp-hz = /bits/ 64 <700000000>; > + opp-microvolt = <950000 950000 1000000>; > + }; > + > + opp-800000000 { > + opp-hz = /bits/ 64 <800000000>; > + opp-microvolt = <1000000 1000000 1000000>; > + }; > + }; > }; > > -&pipegrf { > - compatible = "rockchip,rk3566-pipe-grf", "syscon"; This seems unrelated? > +&cpu0 { > + operating-points-v2 = <&cpu0_opp_table>; > }; > > -&power { > - power-domain@RK3568_PD_PIPE { > - reg = <RK3568_PD_PIPE>; > - clocks = <&cru PCLK_PIPE>; > - pm_qos = <&qos_pcie2x1>, > - <&qos_sata1>, > - <&qos_sata2>, > - <&qos_usb3_0>, > - <&qos_usb3_1>; > - #power-domain-cells = <0>; > - }; This seems unrelated to me and possibly a functional change? If this was intended, then a description in the commit message would be nice why this is appropriate and possibly moved to a separate patch? > +&cpu1 { > + operating-points-v2 = <&cpu0_opp_table>; > +}; > + > +&cpu2 { > + operating-points-v2 = <&cpu0_opp_table>; > }; > > -&usb_host0_xhci { > - phys = <&usb2phy0_otg>; > - phy-names = "usb2-phy"; > - extcon = <&usb2phy0>; > - maximum-speed = "high-speed"; This also looks unrelated and a functional change? > +&cpu3 { > + operating-points-v2 = <&cpu0_opp_table>; > }; > > -&vop { > - compatible = "rockchip,rk3566-vop"; This also looks unrelated? Cheers, Diederik > +&gpu { > + operating-points-v2 = <&gpu_opp_table>; > }; > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > index 5c54898f6ed1..ecaefe208e3e 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi > @@ -3,11 +3,99 @@ > * Copyright (c) 2021 Rockchip Electronics Co., Ltd. > */ > > -#include "rk356x.dtsi" > +#include "rk356x-base.dtsi" > > / { > compatible = "rockchip,rk3568"; > > + cpu0_opp_table: opp-table-0 { > + compatible = "operating-points-v2"; > + opp-shared; > + > + opp-408000000 { > + opp-hz = /bits/ 64 <408000000>; > + opp-microvolt = <850000 850000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-600000000 { > + opp-hz = /bits/ 64 <600000000>; > + opp-microvolt = <850000 850000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-816000000 { > + opp-hz = /bits/ 64 <816000000>; > + opp-microvolt = <850000 850000 1150000>; > + clock-latency-ns = <40000>; > + opp-suspend; > + }; > + > + opp-1104000000 { > + opp-hz = /bits/ 64 <1104000000>; > + opp-microvolt = <900000 900000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1416000000 { > + opp-hz = /bits/ 64 <1416000000>; > + opp-microvolt = <1025000 1025000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1608000000 { > + opp-hz = /bits/ 64 <1608000000>; > + opp-microvolt = <1100000 1100000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1800000000 { > + opp-hz = /bits/ 64 <1800000000>; > + opp-microvolt = <1150000 1150000 1150000>; > + clock-latency-ns = <40000>; > + }; > + > + opp-1992000000 { > + opp-hz = /bits/ 64 <1992000000>; > + opp-microvolt = <1150000 1150000 1150000>; > + clock-latency-ns = <40000>; > + }; > + }; > + > + gpu_opp_table: opp-table-1 { > + compatible = "operating-points-v2"; > + > + opp-200000000 { > + opp-hz = /bits/ 64 <200000000>; > + opp-microvolt = <850000 850000 1000000>; > + }; > + > + opp-300000000 { > + opp-hz = /bits/ 64 <300000000>; > + opp-microvolt = <850000 850000 1000000>; > + }; > + > + opp-400000000 { > + opp-hz = /bits/ 64 <400000000>; > + opp-microvolt = <850000 850000 1000000>; > + }; > + > + opp-600000000 { > + opp-hz = /bits/ 64 <600000000>; > + opp-microvolt = <900000 900000 1000000>; > + }; > + > + opp-700000000 { > + opp-hz = /bits/ 64 <700000000>; > + opp-microvolt = <950000 950000 1000000>; > + }; > + > + opp-800000000 { > + opp-hz = /bits/ 64 <800000000>; > + opp-microvolt = <1000000 1000000 1000000>; > + }; > + }; > + > sata0: sata@fc000000 { > compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci"; > reg = <0 0xfc000000 0 0x1000>; > @@ -269,12 +357,24 @@ combphy0: phy@fe820000 { > }; > }; > > -&cpu0_opp_table { > - opp-1992000000 { > - opp-hz = /bits/ 64 <1992000000>; > - opp-microvolt = <1150000 1150000 1150000>; > - clock-latency-ns = <40000>; > - }; > +&cpu0 { > + operating-points-v2 = <&cpu0_opp_table>; > +}; > + > +&cpu1 { > + operating-points-v2 = <&cpu0_opp_table>; > +}; > + > +&cpu2 { > + operating-points-v2 = <&cpu0_opp_table>; > +}; > + > +&cpu3 { > + operating-points-v2 = <&cpu0_opp_table>; > +}; > + > +&gpu { > + operating-points-v2 = <&gpu_opp_table>; > }; > > &pipegrf { > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi > similarity index 96% > rename from arch/arm64/boot/dts/rockchip/rk356x.dtsi > rename to arch/arm64/boot/dts/rockchip/rk356x-base.dtsi > index 534593f2ed0b..62be06f3b863 100644 > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi > @@ -56,7 +56,6 @@ cpu0: cpu@0 { > clocks = <&scmi_clk 0>; > #cooling-cells = <2>; > enable-method = "psci"; > - operating-points-v2 = <&cpu0_opp_table>; > i-cache-size = <0x8000>; > i-cache-line-size = <64>; > i-cache-sets = <128>; > @@ -72,7 +71,6 @@ cpu1: cpu@100 { > reg = <0x0 0x100>; > #cooling-cells = <2>; > enable-method = "psci"; > - operating-points-v2 = <&cpu0_opp_table>; > i-cache-size = <0x8000>; > i-cache-line-size = <64>; > i-cache-sets = <128>; > @@ -88,7 +86,6 @@ cpu2: cpu@200 { > reg = <0x0 0x200>; > #cooling-cells = <2>; > enable-method = "psci"; > - operating-points-v2 = <&cpu0_opp_table>; > i-cache-size = <0x8000>; > i-cache-line-size = <64>; > i-cache-sets = <128>; > @@ -104,7 +101,6 @@ cpu3: cpu@300 { > reg = <0x0 0x300>; > #cooling-cells = <2>; > enable-method = "psci"; > - operating-points-v2 = <&cpu0_opp_table>; > i-cache-size = <0x8000>; > i-cache-line-size = <64>; > i-cache-sets = <128>; > @@ -128,54 +124,6 @@ l3_cache: l3-cache { > cache-sets = <512>; > }; > > - cpu0_opp_table: opp-table-0 { > - compatible = "operating-points-v2"; > - opp-shared; > - > - opp-408000000 { > - opp-hz = /bits/ 64 <408000000>; > - opp-microvolt = <850000 850000 1150000>; > - clock-latency-ns = <40000>; > - }; > - > - opp-600000000 { > - opp-hz = /bits/ 64 <600000000>; > - opp-microvolt = <850000 850000 1150000>; > - clock-latency-ns = <40000>; > - }; > - > - opp-816000000 { > - opp-hz = /bits/ 64 <816000000>; > - opp-microvolt = <850000 850000 1150000>; > - clock-latency-ns = <40000>; > - opp-suspend; > - }; > - > - opp-1104000000 { > - opp-hz = /bits/ 64 <1104000000>; > - opp-microvolt = <900000 900000 1150000>; > - clock-latency-ns = <40000>; > - }; > - > - opp-1416000000 { > - opp-hz = /bits/ 64 <1416000000>; > - opp-microvolt = <1025000 1025000 1150000>; > - clock-latency-ns = <40000>; > - }; > - > - opp-1608000000 { > - opp-hz = /bits/ 64 <1608000000>; > - opp-microvolt = <1100000 1100000 1150000>; > - clock-latency-ns = <40000>; > - }; > - > - opp-1800000000 { > - opp-hz = /bits/ 64 <1800000000>; > - opp-microvolt = <1150000 1150000 1150000>; > - clock-latency-ns = <40000>; > - }; > - }; > - > display_subsystem: display-subsystem { > compatible = "rockchip,display-subsystem"; > ports = <&vop_out>; > @@ -196,40 +144,6 @@ scmi_clk: protocol@14 { > }; > }; > > - gpu_opp_table: opp-table-1 { > - compatible = "operating-points-v2"; > - > - opp-200000000 { > - opp-hz = /bits/ 64 <200000000>; > - opp-microvolt = <850000 850000 1000000>; > - }; > - > - opp-300000000 { > - opp-hz = /bits/ 64 <300000000>; > - opp-microvolt = <850000 850000 1000000>; > - }; > - > - opp-400000000 { > - opp-hz = /bits/ 64 <400000000>; > - opp-microvolt = <850000 850000 1000000>; > - }; > - > - opp-600000000 { > - opp-hz = /bits/ 64 <600000000>; > - opp-microvolt = <900000 900000 1000000>; > - }; > - > - opp-700000000 { > - opp-hz = /bits/ 64 <700000000>; > - opp-microvolt = <950000 950000 1000000>; > - }; > - > - opp-800000000 { > - opp-hz = /bits/ 64 <800000000>; > - opp-microvolt = <1000000 1000000 1000000>; > - }; > - }; > - > hdmi_sound: hdmi-sound { > compatible = "simple-audio-card"; > simple-audio-card,name = "HDMI"; > @@ -635,7 +549,6 @@ gpu: gpu@fde60000 { > clocks = <&scmi_clk 1>, <&cru CLK_GPU>; > clock-names = "gpu", "bus"; > #cooling-cells = <2>; > - operating-points-v2 = <&gpu_opp_table>; > power-domains = <&power RK3568_PD_GPU>; > status = "disabled"; > }; > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hello Diederik, On 2024-10-12 21:41, Diederik de Haas wrote: > On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote: >> Rename the Rockchip RK356x SoC dtsi files and, consequently, adjust >> their >> contents appropriately, to prepare them for the ability to specify >> different >> CPU and GPU OPPs for each of the supported RK356x SoC variants. >> >> The first new RK356x SoC variant to be introduced is the RK3566T, >> which the >> Pine64 Quartz64 Zero SBC is officially based on. [1] Some other SBCs >> are >> also based on the RK3566T variant, including Radxa ROCK 3C and ZERO >> 3E/3W, >> but the slight trouble is that Radxa doesn't state that officially. >> Though, >> it's rather easy to spot the RK3566T on such boards, because their >> official >> specifications state that the maximum frequency for the Cortex-A55 >> cores is >> lower than the "full-fat" RK3566's 1.8 GHz. [2][3][4] > > I think we changed terminology from "full-fat" to something else in the > rk3588 variant? Would be nice to be consisten. Back then, it was about the naming of one of the dtsi files, [*] not about using "full-fat" in the commit description. Using "full-fat" in one of the file names was just part of the RFC, as a temporary solution. OTOH, frankly, I don't feel like using "full-fat" in this commit description is inappropriate or inconsistent. [*] https://lore.kernel.org/linux-rockchip/673dcf47596e7bc8ba065034e339bb1bbf9cdcb0.1716948159.git.dsimic@manjaro.org/T/#u >> These changes follow the approach used for the Rockchip RK3588 SoC >> variants, >> which was introduced and described further in commit def88eb4d836 >> ("arm64: >> dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs"). >> Please >> see that commit for a more detailed explanation. >> >> No functional changes are introduced, which was validated by >> decompiling and > > No functional changes ... This will be covered later in my response... >> comparing all affected board dtb files before and after these changes. >> In >> more detail, the affected dtb files have some of their blocks shuffled >> around >> a bit and some of their phandles have different values, as a result of >> the >> changes to the order in which the building blocks from the parent dtsi >> files >> are included, but they effectively remain the same as the originals. >> >> [1] https://wiki.pine64.org/wiki/Quartz64 >> [2] >> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf >> [3] >> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf >> [4] >> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf >> >> Related-to: def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC >> dtsi files for per-variant OPPs") >> Signed-off-by: Dragan Simic <dsimic@manjaro.org> >> --- >> .../{rk3566.dtsi => rk3566-base.dtsi} | 2 +- >> arch/arm64/boot/dts/rockchip/rk3566.dtsi | 116 >> ++++++++++++++---- >> arch/arm64/boot/dts/rockchip/rk3568.dtsi | 114 +++++++++++++++-- >> .../{rk356x.dtsi => rk356x-base.dtsi} | 87 ------------- >> 4 files changed, 202 insertions(+), 117 deletions(-) >> copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} >> (95%) >> rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} >> (96%) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi >> b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi >> similarity index 95% >> copy from arch/arm64/boot/dts/rockchip/rk3566.dtsi >> copy to arch/arm64/boot/dts/rockchip/rk3566-base.dtsi >> index 6c4b17d27bdc..e56e0b6ba941 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi >> @@ -1,6 +1,6 @@ >> // SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> >> -#include "rk356x.dtsi" >> +#include "rk356x-base.dtsi" >> >> / { >> compatible = "rockchip,rk3566"; >> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi >> b/arch/arm64/boot/dts/rockchip/rk3566.dtsi >> index 6c4b17d27bdc..3fcca79279f7 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi >> @@ -1,35 +1,107 @@ >> // SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> >> -#include "rk356x.dtsi" >> +#include "rk3566-base.dtsi" >> >> / { >> - compatible = "rockchip,rk3566"; >> + cpu0_opp_table: opp-table-0 { >> + compatible = "operating-points-v2"; >> + opp-shared; >> + >> + opp-408000000 { >> + opp-hz = /bits/ 64 <408000000>; >> + opp-microvolt = <850000 850000 1150000>; >> + clock-latency-ns = <40000>; >> + }; >> + >> + opp-600000000 { >> + opp-hz = /bits/ 64 <600000000>; >> + opp-microvolt = <850000 850000 1150000>; >> + clock-latency-ns = <40000>; >> + }; >> + >> + opp-816000000 { >> + opp-hz = /bits/ 64 <816000000>; >> + opp-microvolt = <850000 850000 1150000>; >> + clock-latency-ns = <40000>; >> + opp-suspend; >> + }; > > Just like with patch 1 of this series, drop the blank line? I believe I've already explained the reasoning behind that. [**] [**] https://lore.kernel.org/linux-rockchip/0a1f13d06ec3668c136997e72d0aea44@manjaro.org/ >> + >> + opp-1104000000 { >> + opp-hz = /bits/ 64 <1104000000>; >> + opp-microvolt = <900000 900000 1150000>; >> + clock-latency-ns = <40000>; >> + }; >> + >> + opp-1416000000 { >> + opp-hz = /bits/ 64 <1416000000>; >> + opp-microvolt = <1025000 1025000 1150000>; >> + clock-latency-ns = <40000>; >> + }; >> + >> + opp-1608000000 { >> + opp-hz = /bits/ 64 <1608000000>; >> + opp-microvolt = <1100000 1100000 1150000>; >> + clock-latency-ns = <40000>; >> + }; >> + >> + opp-1800000000 { >> + opp-hz = /bits/ 64 <1800000000>; >> + opp-microvolt = <1150000 1150000 1150000>; >> + clock-latency-ns = <40000>; >> + }; >> + }; >> + >> + gpu_opp_table: opp-table-1 { >> + compatible = "operating-points-v2"; >> + >> + opp-200000000 { >> + opp-hz = /bits/ 64 <200000000>; >> + opp-microvolt = <850000 850000 1000000>; >> + }; >> + >> + opp-300000000 { >> + opp-hz = /bits/ 64 <300000000>; >> + opp-microvolt = <850000 850000 1000000>; >> + }; >> + >> + opp-400000000 { >> + opp-hz = /bits/ 64 <400000000>; >> + opp-microvolt = <850000 850000 1000000>; >> + }; >> + >> + opp-600000000 { >> + opp-hz = /bits/ 64 <600000000>; >> + opp-microvolt = <900000 900000 1000000>; >> + }; >> + >> + opp-700000000 { >> + opp-hz = /bits/ 64 <700000000>; >> + opp-microvolt = <950000 950000 1000000>; >> + }; >> + >> + opp-800000000 { >> + opp-hz = /bits/ 64 <800000000>; >> + opp-microvolt = <1000000 1000000 1000000>; >> + }; >> + }; >> }; >> >> -&pipegrf { >> - compatible = "rockchip,rk3566-pipe-grf", "syscon"; > > This seems unrelated? Yes, it looks completely out of place, but it's just the way this diff ended up looking like. It's actually fine. >> +&cpu0 { >> + operating-points-v2 = <&cpu0_opp_table>; >> }; >> >> -&power { >> - power-domain@RK3568_PD_PIPE { >> - reg = <RK3568_PD_PIPE>; >> - clocks = <&cru PCLK_PIPE>; >> - pm_qos = <&qos_pcie2x1>, >> - <&qos_sata1>, >> - <&qos_sata2>, >> - <&qos_usb3_0>, >> - <&qos_usb3_1>; >> - #power-domain-cells = <0>; >> - }; > > This seems unrelated to me and possibly a functional change? > If this was intended, then a description in the commit message would be > nice why this is appropriate and possibly moved to a separate patch? Just another instance of the diff ending up looking strange, while there are actually no such changes. >> +&cpu1 { >> + operating-points-v2 = <&cpu0_opp_table>; >> +}; >> + >> +&cpu2 { >> + operating-points-v2 = <&cpu0_opp_table>; >> }; >> >> -&usb_host0_xhci { >> - phys = <&usb2phy0_otg>; >> - phy-names = "usb2-phy"; >> - extcon = <&usb2phy0>; >> - maximum-speed = "high-speed"; > > This also looks unrelated and a functional change? Already explained above. >> +&cpu3 { >> + operating-points-v2 = <&cpu0_opp_table>; >> }; >> >> -&vop { >> - compatible = "rockchip,rk3566-vop"; > > This also looks unrelated? Already explained above.
Hi Dragan, On Sat Oct 12, 2024 at 9:41 PM CEST, Diederik de Haas wrote: > On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote: > > > > -&pipegrf { > > - compatible = "rockchip,rk3566-pipe-grf", "syscon"; > > This seems unrelated? > > > +&cpu0 { > > + operating-points-v2 = <&cpu0_opp_table>; > > }; > > > > -&power { > > - power-domain@RK3568_PD_PIPE { > > - reg = <RK3568_PD_PIPE>; > > - clocks = <&cru PCLK_PIPE>; > > - pm_qos = <&qos_pcie2x1>, > > - <&qos_sata1>, > > - <&qos_sata2>, > > - <&qos_usb3_0>, > > - <&qos_usb3_1>; > > - #power-domain-cells = <0>; > > - }; > > This seems unrelated to me and possibly a functional change? > If this was intended, then a description in the commit message would be > nice why this is appropriate and possibly moved to a separate patch? > > > +&cpu1 { > > + operating-points-v2 = <&cpu0_opp_table>; > > +}; > > + > > +&cpu2 { > > + operating-points-v2 = <&cpu0_opp_table>; > > }; > > > > -&usb_host0_xhci { > > - phys = <&usb2phy0_otg>; > > - phy-names = "usb2-phy"; > > - extcon = <&usb2phy0>; > > - maximum-speed = "high-speed"; > > This also looks unrelated and a functional change? > > > +&cpu3 { > > + operating-points-v2 = <&cpu0_opp_table>; > > }; > > > > -&vop { > > - compatible = "rockchip,rk3566-vop"; > > This also looks unrelated? It turns out I was wrong. The elements I thought were removed, aren't removed. Sorry for the noise. Diederik
Hello Diederik, On 2024-10-19 20:09, Diederik de Haas wrote: > On Sat Oct 12, 2024 at 9:41 PM CEST, Diederik de Haas wrote: >> On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote: >> > >> > -&pipegrf { >> > - compatible = "rockchip,rk3566-pipe-grf", "syscon"; >> >> This seems unrelated? >> >> > +&cpu0 { >> > + operating-points-v2 = <&cpu0_opp_table>; >> > }; >> > >> > -&power { >> > - power-domain@RK3568_PD_PIPE { >> > - reg = <RK3568_PD_PIPE>; >> > - clocks = <&cru PCLK_PIPE>; >> > - pm_qos = <&qos_pcie2x1>, >> > - <&qos_sata1>, >> > - <&qos_sata2>, >> > - <&qos_usb3_0>, >> > - <&qos_usb3_1>; >> > - #power-domain-cells = <0>; >> > - }; >> >> This seems unrelated to me and possibly a functional change? >> If this was intended, then a description in the commit message would >> be >> nice why this is appropriate and possibly moved to a separate patch? >> >> > +&cpu1 { >> > + operating-points-v2 = <&cpu0_opp_table>; >> > +}; >> > + >> > +&cpu2 { >> > + operating-points-v2 = <&cpu0_opp_table>; >> > }; >> > >> > -&usb_host0_xhci { >> > - phys = <&usb2phy0_otg>; >> > - phy-names = "usb2-phy"; >> > - extcon = <&usb2phy0>; >> > - maximum-speed = "high-speed"; >> >> This also looks unrelated and a functional change? >> >> > +&cpu3 { >> > + operating-points-v2 = <&cpu0_opp_table>; >> > }; >> > >> > -&vop { >> > - compatible = "rockchip,rk3566-vop"; >> >> This also looks unrelated? > > It turns out I was wrong. > The elements I thought were removed, aren't removed. > > Sorry for the noise. No worries. I tried to tune and adjust the patch generation parameters as best as possible, but some parts of the produced patches still remained slightly confusing. By the way, a few months ago there was a discussion on the Git mailing list about making Git perform such parameter adjustment magic itself, which back then seemed like a good idea to me, but after dealing with a few rather complex patches myself, I no longer think that's actually possible.
diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi similarity index 95% copy from arch/arm64/boot/dts/rockchip/rk3566.dtsi copy to arch/arm64/boot/dts/rockchip/rk3566-base.dtsi index 6c4b17d27bdc..e56e0b6ba941 100644 --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi @@ -1,6 +1,6 @@ // SPDX-License-Identifier: (GPL-2.0+ OR MIT) -#include "rk356x.dtsi" +#include "rk356x-base.dtsi" / { compatible = "rockchip,rk3566"; diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi index 6c4b17d27bdc..3fcca79279f7 100644 --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi @@ -1,35 +1,107 @@ // SPDX-License-Identifier: (GPL-2.0+ OR MIT) -#include "rk356x.dtsi" +#include "rk3566-base.dtsi" / { - compatible = "rockchip,rk3566"; + cpu0_opp_table: opp-table-0 { + compatible = "operating-points-v2"; + opp-shared; + + opp-408000000 { + opp-hz = /bits/ 64 <408000000>; + opp-microvolt = <850000 850000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <850000 850000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-816000000 { + opp-hz = /bits/ 64 <816000000>; + opp-microvolt = <850000 850000 1150000>; + clock-latency-ns = <40000>; + opp-suspend; + }; + + opp-1104000000 { + opp-hz = /bits/ 64 <1104000000>; + opp-microvolt = <900000 900000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1416000000 { + opp-hz = /bits/ 64 <1416000000>; + opp-microvolt = <1025000 1025000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1608000000 { + opp-hz = /bits/ 64 <1608000000>; + opp-microvolt = <1100000 1100000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1800000000 { + opp-hz = /bits/ 64 <1800000000>; + opp-microvolt = <1150000 1150000 1150000>; + clock-latency-ns = <40000>; + }; + }; + + gpu_opp_table: opp-table-1 { + compatible = "operating-points-v2"; + + opp-200000000 { + opp-hz = /bits/ 64 <200000000>; + opp-microvolt = <850000 850000 1000000>; + }; + + opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + opp-microvolt = <850000 850000 1000000>; + }; + + opp-400000000 { + opp-hz = /bits/ 64 <400000000>; + opp-microvolt = <850000 850000 1000000>; + }; + + opp-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <900000 900000 1000000>; + }; + + opp-700000000 { + opp-hz = /bits/ 64 <700000000>; + opp-microvolt = <950000 950000 1000000>; + }; + + opp-800000000 { + opp-hz = /bits/ 64 <800000000>; + opp-microvolt = <1000000 1000000 1000000>; + }; + }; }; -&pipegrf { - compatible = "rockchip,rk3566-pipe-grf", "syscon"; +&cpu0 { + operating-points-v2 = <&cpu0_opp_table>; }; -&power { - power-domain@RK3568_PD_PIPE { - reg = <RK3568_PD_PIPE>; - clocks = <&cru PCLK_PIPE>; - pm_qos = <&qos_pcie2x1>, - <&qos_sata1>, - <&qos_sata2>, - <&qos_usb3_0>, - <&qos_usb3_1>; - #power-domain-cells = <0>; - }; +&cpu1 { + operating-points-v2 = <&cpu0_opp_table>; +}; + +&cpu2 { + operating-points-v2 = <&cpu0_opp_table>; }; -&usb_host0_xhci { - phys = <&usb2phy0_otg>; - phy-names = "usb2-phy"; - extcon = <&usb2phy0>; - maximum-speed = "high-speed"; +&cpu3 { + operating-points-v2 = <&cpu0_opp_table>; }; -&vop { - compatible = "rockchip,rk3566-vop"; +&gpu { + operating-points-v2 = <&gpu_opp_table>; }; diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi index 5c54898f6ed1..ecaefe208e3e 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi @@ -3,11 +3,99 @@ * Copyright (c) 2021 Rockchip Electronics Co., Ltd. */ -#include "rk356x.dtsi" +#include "rk356x-base.dtsi" / { compatible = "rockchip,rk3568"; + cpu0_opp_table: opp-table-0 { + compatible = "operating-points-v2"; + opp-shared; + + opp-408000000 { + opp-hz = /bits/ 64 <408000000>; + opp-microvolt = <850000 850000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <850000 850000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-816000000 { + opp-hz = /bits/ 64 <816000000>; + opp-microvolt = <850000 850000 1150000>; + clock-latency-ns = <40000>; + opp-suspend; + }; + + opp-1104000000 { + opp-hz = /bits/ 64 <1104000000>; + opp-microvolt = <900000 900000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1416000000 { + opp-hz = /bits/ 64 <1416000000>; + opp-microvolt = <1025000 1025000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1608000000 { + opp-hz = /bits/ 64 <1608000000>; + opp-microvolt = <1100000 1100000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1800000000 { + opp-hz = /bits/ 64 <1800000000>; + opp-microvolt = <1150000 1150000 1150000>; + clock-latency-ns = <40000>; + }; + + opp-1992000000 { + opp-hz = /bits/ 64 <1992000000>; + opp-microvolt = <1150000 1150000 1150000>; + clock-latency-ns = <40000>; + }; + }; + + gpu_opp_table: opp-table-1 { + compatible = "operating-points-v2"; + + opp-200000000 { + opp-hz = /bits/ 64 <200000000>; + opp-microvolt = <850000 850000 1000000>; + }; + + opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + opp-microvolt = <850000 850000 1000000>; + }; + + opp-400000000 { + opp-hz = /bits/ 64 <400000000>; + opp-microvolt = <850000 850000 1000000>; + }; + + opp-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <900000 900000 1000000>; + }; + + opp-700000000 { + opp-hz = /bits/ 64 <700000000>; + opp-microvolt = <950000 950000 1000000>; + }; + + opp-800000000 { + opp-hz = /bits/ 64 <800000000>; + opp-microvolt = <1000000 1000000 1000000>; + }; + }; + sata0: sata@fc000000 { compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci"; reg = <0 0xfc000000 0 0x1000>; @@ -269,12 +357,24 @@ combphy0: phy@fe820000 { }; }; -&cpu0_opp_table { - opp-1992000000 { - opp-hz = /bits/ 64 <1992000000>; - opp-microvolt = <1150000 1150000 1150000>; - clock-latency-ns = <40000>; - }; +&cpu0 { + operating-points-v2 = <&cpu0_opp_table>; +}; + +&cpu1 { + operating-points-v2 = <&cpu0_opp_table>; +}; + +&cpu2 { + operating-points-v2 = <&cpu0_opp_table>; +}; + +&cpu3 { + operating-points-v2 = <&cpu0_opp_table>; +}; + +&gpu { + operating-points-v2 = <&gpu_opp_table>; }; &pipegrf { diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi similarity index 96% rename from arch/arm64/boot/dts/rockchip/rk356x.dtsi rename to arch/arm64/boot/dts/rockchip/rk356x-base.dtsi index 534593f2ed0b..62be06f3b863 100644 --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi @@ -56,7 +56,6 @@ cpu0: cpu@0 { clocks = <&scmi_clk 0>; #cooling-cells = <2>; enable-method = "psci"; - operating-points-v2 = <&cpu0_opp_table>; i-cache-size = <0x8000>; i-cache-line-size = <64>; i-cache-sets = <128>; @@ -72,7 +71,6 @@ cpu1: cpu@100 { reg = <0x0 0x100>; #cooling-cells = <2>; enable-method = "psci"; - operating-points-v2 = <&cpu0_opp_table>; i-cache-size = <0x8000>; i-cache-line-size = <64>; i-cache-sets = <128>; @@ -88,7 +86,6 @@ cpu2: cpu@200 { reg = <0x0 0x200>; #cooling-cells = <2>; enable-method = "psci"; - operating-points-v2 = <&cpu0_opp_table>; i-cache-size = <0x8000>; i-cache-line-size = <64>; i-cache-sets = <128>; @@ -104,7 +101,6 @@ cpu3: cpu@300 { reg = <0x0 0x300>; #cooling-cells = <2>; enable-method = "psci"; - operating-points-v2 = <&cpu0_opp_table>; i-cache-size = <0x8000>; i-cache-line-size = <64>; i-cache-sets = <128>; @@ -128,54 +124,6 @@ l3_cache: l3-cache { cache-sets = <512>; }; - cpu0_opp_table: opp-table-0 { - compatible = "operating-points-v2"; - opp-shared; - - opp-408000000 { - opp-hz = /bits/ 64 <408000000>; - opp-microvolt = <850000 850000 1150000>; - clock-latency-ns = <40000>; - }; - - opp-600000000 { - opp-hz = /bits/ 64 <600000000>; - opp-microvolt = <850000 850000 1150000>; - clock-latency-ns = <40000>; - }; - - opp-816000000 { - opp-hz = /bits/ 64 <816000000>; - opp-microvolt = <850000 850000 1150000>; - clock-latency-ns = <40000>; - opp-suspend; - }; - - opp-1104000000 { - opp-hz = /bits/ 64 <1104000000>; - opp-microvolt = <900000 900000 1150000>; - clock-latency-ns = <40000>; - }; - - opp-1416000000 { - opp-hz = /bits/ 64 <1416000000>; - opp-microvolt = <1025000 1025000 1150000>; - clock-latency-ns = <40000>; - }; - - opp-1608000000 { - opp-hz = /bits/ 64 <1608000000>; - opp-microvolt = <1100000 1100000 1150000>; - clock-latency-ns = <40000>; - }; - - opp-1800000000 { - opp-hz = /bits/ 64 <1800000000>; - opp-microvolt = <1150000 1150000 1150000>; - clock-latency-ns = <40000>; - }; - }; - display_subsystem: display-subsystem { compatible = "rockchip,display-subsystem"; ports = <&vop_out>; @@ -196,40 +144,6 @@ scmi_clk: protocol@14 { }; }; - gpu_opp_table: opp-table-1 { - compatible = "operating-points-v2"; - - opp-200000000 { - opp-hz = /bits/ 64 <200000000>; - opp-microvolt = <850000 850000 1000000>; - }; - - opp-300000000 { - opp-hz = /bits/ 64 <300000000>; - opp-microvolt = <850000 850000 1000000>; - }; - - opp-400000000 { - opp-hz = /bits/ 64 <400000000>; - opp-microvolt = <850000 850000 1000000>; - }; - - opp-600000000 { - opp-hz = /bits/ 64 <600000000>; - opp-microvolt = <900000 900000 1000000>; - }; - - opp-700000000 { - opp-hz = /bits/ 64 <700000000>; - opp-microvolt = <950000 950000 1000000>; - }; - - opp-800000000 { - opp-hz = /bits/ 64 <800000000>; - opp-microvolt = <1000000 1000000 1000000>; - }; - }; - hdmi_sound: hdmi-sound { compatible = "simple-audio-card"; simple-audio-card,name = "HDMI"; @@ -635,7 +549,6 @@ gpu: gpu@fde60000 { clocks = <&scmi_clk 1>, <&cru CLK_GPU>; clock-names = "gpu", "bus"; #cooling-cells = <2>; - operating-points-v2 = <&gpu_opp_table>; power-domains = <&power RK3568_PD_GPU>; status = "disabled"; };
Rename the Rockchip RK356x SoC dtsi files and, consequently, adjust their contents appropriately, to prepare them for the ability to specify different CPU and GPU OPPs for each of the supported RK356x SoC variants. The first new RK356x SoC variant to be introduced is the RK3566T, which the Pine64 Quartz64 Zero SBC is officially based on. [1] Some other SBCs are also based on the RK3566T variant, including Radxa ROCK 3C and ZERO 3E/3W, but the slight trouble is that Radxa doesn't state that officially. Though, it's rather easy to spot the RK3566T on such boards, because their official specifications state that the maximum frequency for the Cortex-A55 cores is lower than the "full-fat" RK3566's 1.8 GHz. [2][3][4] These changes follow the approach used for the Rockchip RK3588 SoC variants, which was introduced and described further in commit def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs"). Please see that commit for a more detailed explanation. No functional changes are introduced, which was validated by decompiling and comparing all affected board dtb files before and after these changes. In more detail, the affected dtb files have some of their blocks shuffled around a bit and some of their phandles have different values, as a result of the changes to the order in which the building blocks from the parent dtsi files are included, but they effectively remain the same as the originals. [1] https://wiki.pine64.org/wiki/Quartz64 [2] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf [3] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf [4] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf Related-to: def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs") Signed-off-by: Dragan Simic <dsimic@manjaro.org> --- .../{rk3566.dtsi => rk3566-base.dtsi} | 2 +- arch/arm64/boot/dts/rockchip/rk3566.dtsi | 116 ++++++++++++++---- arch/arm64/boot/dts/rockchip/rk3568.dtsi | 114 +++++++++++++++-- .../{rk356x.dtsi => rk356x-base.dtsi} | 87 ------------- 4 files changed, 202 insertions(+), 117 deletions(-) copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} (95%) rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} (96%)