Re: [PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant OPPs

From: Diederik de Haas
Date: Sat Oct 12 2024 - 15:41:33 EST


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@xxxxxxxxxxx>
> ---
> .../{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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Attachment: signature.asc
Description: PGP signature