diff mbox series

[v2,6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588

Message ID 20240919091834.83572-7-sebastian.reichel@collabora.com (mailing list archive)
State New
Headers show
Series Fix RK3588 GPU domain | expand

Commit Message

Sebastian Reichel Sept. 19, 2024, 9:12 a.m. UTC
Enabling the GPU power domain requires that the GPU regulator is
enabled. The regulator is enabled at boot time, but automatically
gets disabled when there are no users.

If the GPU driver is not probed at boot time or rebound while
the system is running the system will try to enable the power
domain before the regulator is enabled resulting in a failure
hanging the whole system. Avoid this by adding an explicit
dependency.

Reported-by: Adrián Martínez Larumbe <adrian.larumbe@collabora.com>
Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts         | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi                | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi          | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts               | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts             | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts           | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts              | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi               | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts           | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts        | 4 ++++
 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts          | 4 ++++
 12 files changed, 45 insertions(+), 1 deletion(-)

Comments

Jonas Karlman Sept. 19, 2024, 11:33 a.m. UTC | #1
Hi Sebastian,

On 2024-09-19 11:12, Sebastian Reichel wrote:
> Enabling the GPU power domain requires that the GPU regulator is
> enabled. The regulator is enabled at boot time, but automatically
> gets disabled when there are no users.
> 
> If the GPU driver is not probed at boot time or rebound while
> the system is running the system will try to enable the power
> domain before the regulator is enabled resulting in a failure
> hanging the whole system. Avoid this by adding an explicit
> dependency.
> 
> Reported-by: Adrián Martínez Larumbe <adrian.larumbe@collabora.com>
> Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts         | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi                | 2 +-
>  arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi          | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts               | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts             | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts           | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts              | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi               | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts           | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts        | 4 ++++
>  arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts          | 4 ++++
>  12 files changed, 45 insertions(+), 1 deletion(-)
> 

Any reason why following rk3588 DTs was not updated?

rk3588-evb1-v10.dts
rk3588-nanopc-t6.dtsi
rk3588-quartzpro64.dts
rk3588s-gameforce-ace.dts
rk3588s-odroid-m2.dts

I also expect we may need to define domain-supply for the npu on rk3588
and also both gpu and npu on rk356x in a future series.

Similar freeze issue has been observed on rk356x when booting vendor
kernel with npu support enabled using mainline U-Boot and DT [1].

To work around that issue on rk356x the npu regulator could be changed
to always-on/boot-on to get past the kernel freeze [2].

[1] https://github.com/armbian/build/pull/7025#issuecomment-2291067748
[2] https://github.com/Kwiboo/u-boot-rockchip/commit/da31da4b68f858f54364a21b0dd00fef2ab0d0d6

Regards,
Jonas
Sebastian Reichel Sept. 19, 2024, 7:27 p.m. UTC | #2
Hi,

On Thu, Sep 19, 2024 at 01:33:25PM GMT, Jonas Karlman wrote:
> On 2024-09-19 11:12, Sebastian Reichel wrote:
> > Enabling the GPU power domain requires that the GPU regulator is
> > enabled. The regulator is enabled at boot time, but automatically
> > gets disabled when there are no users.
> > 
> > If the GPU driver is not probed at boot time or rebound while
> > the system is running the system will try to enable the power
> > domain before the regulator is enabled resulting in a failure
> > hanging the whole system. Avoid this by adding an explicit
> > dependency.
> > 
> > Reported-by: Adrián Martínez Larumbe <adrian.larumbe@collabora.com>
> > Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts         | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi                | 2 +-
> >  arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi          | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts               | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts             | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts           | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts              | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi               | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts           | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts        | 4 ++++
> >  arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts          | 4 ++++
> >  12 files changed, 45 insertions(+), 1 deletion(-)
> 
> Any reason why following rk3588 DTs was not updated?
> 
> rk3588-evb1-v10.dts
> rk3588-quartzpro64.dts

These two I skipped initially, since they have the GPU regulators
always enabled due to the coupling. I'm not 100% sure if the GPU
or the GPU-MEM regulator (or both) are required for the GPU power
domain.

> rk3588-nanopc-t6.dtsi
> rk3588s-gameforce-ace.dts
> rk3588s-odroid-m2.dts

... And these I missed, since they are new.

I don't have enough time to prepare a v3 before my vacation.
Note, that not describing the regulator just keeps the current
behaviour.

> I also expect we may need to define domain-supply for the npu on
> rk3588 and also both gpu and npu on rk356x in a future series.

Yes, I already discussed that in Vienna with Heiko and Tomeu. The
binding change also allows adding a regulator to the NPU power
domain.

> Similar freeze issue has been observed on rk356x when booting vendor
> kernel with npu support enabled using mainline U-Boot and DT [1].
> 
> To work around that issue on rk356x the npu regulator could be changed
> to always-on/boot-on to get past the kernel freeze [2].
> 
> [1] https://github.com/armbian/build/pull/7025#issuecomment-2291067748
> [2] https://github.com/Kwiboo/u-boot-rockchip/commit/da31da4b68f858f54364a21b0dd00fef2ab0d0d6

Yes, that looks like the same issue and I guess the changes to the Rockchip
power-domain driver should also work for rk356x. I don't have one, though.

-- Sebastian
Jonas Karlman Sept. 19, 2024, 9 p.m. UTC | #3
Hi,

On 2024-09-19 21:27, Sebastian Reichel wrote:
> Hi,
> 
> On Thu, Sep 19, 2024 at 01:33:25PM GMT, Jonas Karlman wrote:
>> On 2024-09-19 11:12, Sebastian Reichel wrote:
>>> Enabling the GPU power domain requires that the GPU regulator is
>>> enabled. The regulator is enabled at boot time, but automatically
>>> gets disabled when there are no users.
>>>
>>> If the GPU driver is not probed at boot time or rebound while
>>> the system is running the system will try to enable the power
>>> domain before the regulator is enabled resulting in a failure
>>> hanging the whole system. Avoid this by adding an explicit
>>> dependency.
>>>
>>> Reported-by: Adrián Martínez Larumbe <adrian.larumbe@collabora.com>
>>> Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts         | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi                | 2 +-
>>>  arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi          | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts               | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts             | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts           | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts              | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi               | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts           | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts        | 4 ++++
>>>  arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts          | 4 ++++
>>>  12 files changed, 45 insertions(+), 1 deletion(-)
>>
>> Any reason why following rk3588 DTs was not updated?
>>
>> rk3588-evb1-v10.dts
>> rk3588-quartzpro64.dts
> 
> These two I skipped initially, since they have the GPU regulators
> always enabled due to the coupling. I'm not 100% sure if the GPU
> or the GPU-MEM regulator (or both) are required for the GPU power
> domain.
> 
>> rk3588-nanopc-t6.dtsi
>> rk3588s-gameforce-ace.dts
>> rk3588s-odroid-m2.dts
> 
> ... And these I missed, since they are new.

Great, so no technical reason :-)

> 
> I don't have enough time to prepare a v3 before my vacation.
> Note, that not describing the regulator just keeps the current
> behaviour.

Yeah, remaining boards can be updated in a separate/follow-up series.

> 
>> I also expect we may need to define domain-supply for the npu on
>> rk3588 and also both gpu and npu on rk356x in a future series.
> 
> Yes, I already discussed that in Vienna with Heiko and Tomeu. The
> binding change also allows adding a regulator to the NPU power
> domain.

Great and good to know.

> 
>> Similar freeze issue has been observed on rk356x when booting vendor
>> kernel with npu support enabled using mainline U-Boot and DT [1].
>>
>> To work around that issue on rk356x the npu regulator could be changed
>> to always-on/boot-on to get past the kernel freeze [2].
>>
>> [1] https://github.com/armbian/build/pull/7025#issuecomment-2291067748
>> [2] https://github.com/Kwiboo/u-boot-rockchip/commit/da31da4b68f858f54364a21b0dd00fef2ab0d0d6
> 
> Yes, that looks like the same issue and I guess the changes to the Rockchip
> power-domain driver should also work for rk356x. I don't have one, though.

I will test this series and try to add domain-supply for GPU and NPU on
a few rk356x boards, should help avoid possible issues in future.

Will also send a U-Boot series to enable RK806 PMIC support on a few
more rk3588 boards, should help ensure always-on/boot-on PMIC regulators
are enabled before Linux is started.

Regards,
Jonas

> 
> -- Sebastian
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts b/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts
index c667704ba985..00a1cd96781d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts
@@ -286,6 +286,10 @@  &pcie3x4 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	hym8563 {
 		hym8563_int: hym8563-int {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 905f37876c23..d82ac5a481b4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -861,7 +861,7 @@  power-domain@RK3588_PD_NPU2 {
 				};
 			};
 			/* These power domains are grouped by VD_GPU */
-			power-domain@RK3588_PD_GPU {
+			pd_gpu: power-domain@RK3588_PD_GPU {
 				reg = <RK3588_PD_GPU>;
 				clocks = <&cru CLK_GPU>,
 					 <&cru CLK_GPU_COREGROUP>,
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi
index fde8b228f2c7..cf9d75159ba6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi
@@ -277,6 +277,10 @@  &pcie2x1l2 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	hym8563 {
 		hym8563_int: hym8563-int {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi
index e3a9598b99fc..1af0a30866f6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588.dtsi
@@ -256,6 +256,10 @@  &pcie2x1l2 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	gpio-leds {
 		led_sys_pin: led-sys-pin {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 31d2f8994f85..3cefaf830229 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -403,6 +403,10 @@  &pcie3x4 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	emmc {
 		emmc_reset: emmc-reset {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts b/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts
index c2a08bdf09e8..a9c1fed929fd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts
@@ -312,6 +312,10 @@  &pcie3x4 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	pcie2 {
 		pcie2_0_rst: pcie2-0-rst {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
index d0b922b8d67e..0eadf4fb4ba4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
@@ -530,6 +530,10 @@  &pcie3x4 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	hym8563 {
 		rtc_int: rtc-int {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index 8f7a59918db7..717504383d46 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -465,6 +465,10 @@  &pcie3x4 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	hdmirx {
 		hdmirx_hpd: hdmirx-5v-detection {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index 615094bb8ba3..1b5c4a7fd5c6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
@@ -317,6 +317,10 @@  &pcie3x4 {
 	reset-gpios = <&gpio3 RK_PB6 GPIO_ACTIVE_HIGH>;
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	emmc {
 		emmc_reset: emmc-reset {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts b/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
index 074c316a9a69..d938db0e2239 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
@@ -329,6 +329,10 @@  &pcie2x1l2 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	hym8563 {
 		hym8563_int: hym8563-int {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
index dbddfc3bb464..d29d404417ee 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
@@ -233,6 +233,10 @@  hym8563: rtc@51 {
 	};
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	vdd_sd {
 		vdd_sd_en: vdd-sd-en {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
index feea6b20a6bf..ef3a721d1fc7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
@@ -297,6 +297,10 @@  &pcie2x1l2 {
 	status = "okay";
 };
 
+&pd_gpu {
+	domain-supply = <&vdd_gpu_s0>;
+};
+
 &pinctrl {
 	gpio-func {
 		leds_gpio: leds-gpio {