* [PATCH v2 0/6] Fix RK3588 GPU domain
@ 2024-09-19 9:12 Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 1/6] pmdomain: rockchip: cleanup mutex handling in rockchip_pd_power Sebastian Reichel
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
Hi,
I got a report, that the Linux kernel crashes on Rock 5B when the panthor
driver is loaded late after booting. The crash starts with the following
shortened error print:
rockchip-pm-domain fd8d8000.power-management:power-controller: failed to set domain 'gpu', val=0
rockchip-pm-domain fd8d8000.power-management:power-controller: failed to get ack on domain 'gpu', val=0xa9fff
SError Interrupt on CPU4, code 0x00000000be000411 -- SError
This series first does some cleanups in the Rockchip power domain
driver and changes the driver, so that it no longer tries to continue
when it fails to enable a domain. This gets rid of the SError interrupt
and long backtraces. But the kernel still hangs when it fails to enable
a power domain. I have not done further analysis to check if that can
be avoided.
Last but not least this provides a fix for the GPU power domain failing
to get enabled - after some testing from my side it seems to require the
GPU voltage supply to be enabled.
I'm not really happy about the hack to get a regulator for a sub-node,
which I took over from the Mediatek driver. I discussed this with
Chen-Yu Tsai and Heiko Stübner at OSS EU and the plan is:
1. Merge Rockchip PM domain driver with this hack for now, since DRM CI
people need it
2. Chen-Yu will work on a series, which fixes the hack in Mediatek by
introducing a new devm_regulator_get function taking an DT node as
additional argument
3. Rockchip PM domain later will switch to that once it has landed
Changes since PATCHv1:
* https://lore.kernel.org/all/20240910180530.47194-1-sebastian.reichel@collabora.com/
* Collect Reviewed-by/Acked-by/Tested-by
* swap first and second patch to avoid introducing and directly removing a mutex_unlock
* fix spelling of indentation
* fix double empty line after rockchip_pd_regulator_disable()
Greetings,
-- Sebastian
Sebastian Reichel (6):
pmdomain: rockchip: cleanup mutex handling in rockchip_pd_power
pmdomain: rockchip: forward rockchip_do_pmu_set_power_domain errors
pmdomain: rockchip: reduce indentation in rockchip_pd_power
dt-bindings: power: rockchip: add regulator support
pmdomain: rockchip: add regulator support
arm64: dts: rockchip: Add GPU power domain regulator dependency for
RK3588
.../power/rockchip,power-controller.yaml | 3 +
.../boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 +
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 2 +-
.../boot/dts/rockchip/rk3588-coolpi-cm5.dtsi | 4 +
.../rockchip/rk3588-friendlyelec-cm3588.dtsi | 4 +
.../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 4 +
.../boot/dts/rockchip/rk3588-ok3588-c.dts | 4 +
.../boot/dts/rockchip/rk3588-rock-5-itx.dts | 4 +
.../boot/dts/rockchip/rk3588-rock-5b.dts | 4 +
.../arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 4 +
.../boot/dts/rockchip/rk3588s-coolpi-4b.dts | 4 +
.../dts/rockchip/rk3588s-khadas-edge2.dts | 4 +
.../boot/dts/rockchip/rk3588s-orangepi-5.dts | 4 +
drivers/pmdomain/rockchip/pm-domains.c | 129 +++++++++++++-----
14 files changed, 143 insertions(+), 35 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/6] pmdomain: rockchip: cleanup mutex handling in rockchip_pd_power
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
@ 2024-09-19 9:12 ` Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 2/6] pmdomain: rockchip: forward rockchip_do_pmu_set_power_domain errors Sebastian Reichel
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
Use the cleanup infrastructure to handle the mutex, which
slightly improve code readability for this function.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 9b76b62869d0..4f7021f47261 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -529,13 +529,12 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
struct rockchip_pmu *pmu = pd->pmu;
int ret;
- mutex_lock(&pmu->mutex);
+ guard(mutex)(&pmu->mutex);
if (rockchip_pmu_domain_is_on(pd) != power_on) {
ret = clk_bulk_enable(pd->num_clks, pd->clks);
if (ret < 0) {
dev_err(pmu->dev, "failed to enable clocks\n");
- mutex_unlock(&pmu->mutex);
return ret;
}
@@ -558,7 +557,6 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
clk_bulk_disable(pd->num_clks, pd->clks);
}
- mutex_unlock(&pmu->mutex);
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/6] pmdomain: rockchip: forward rockchip_do_pmu_set_power_domain errors
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 1/6] pmdomain: rockchip: cleanup mutex handling in rockchip_pd_power Sebastian Reichel
@ 2024-09-19 9:12 ` Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 3/6] pmdomain: rockchip: reduce indentation in rockchip_pd_power Sebastian Reichel
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
Currently rockchip_do_pmu_set_power_domain prints a warning if there
have been errors turning on the power domain, but it does not return
any errors and rockchip_pd_power() tries to continue setting up the
QOS registers. This usually results in accessing unpowered registers,
which triggers an SError and a full system hang.
This improves the error handling by forwarding the error to avoid
kernel panics.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 34 +++++++++++++++++---------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 4f7021f47261..5e5291dedd28 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -488,16 +488,17 @@ static int rockchip_pmu_domain_mem_reset(struct rockchip_pm_domain *pd)
return ret;
}
-static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
- bool on)
+static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
+ bool on)
{
struct rockchip_pmu *pmu = pd->pmu;
struct generic_pm_domain *genpd = &pd->genpd;
u32 pd_pwr_offset = pd->info->pwr_offset;
bool is_on, is_mem_on = false;
+ int ret;
if (pd->info->pwr_mask == 0)
- return;
+ return 0;
if (on && pd->info->mem_status_mask)
is_mem_on = rockchip_pmu_domain_is_mem_on(pd);
@@ -512,16 +513,21 @@ static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
wmb();
- if (is_mem_on && rockchip_pmu_domain_mem_reset(pd))
- return;
+ if (is_mem_on) {
+ ret = rockchip_pmu_domain_mem_reset(pd);
+ if (ret)
+ return ret;
+ }
- if (readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
- is_on == on, 0, 10000)) {
- dev_err(pmu->dev,
- "failed to set domain '%s', val=%d\n",
- genpd->name, is_on);
- return;
+ ret = readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
+ is_on == on, 0, 10000);
+ if (ret) {
+ dev_err(pmu->dev, "failed to set domain '%s' %s, val=%d\n",
+ genpd->name, on ? "on" : "off", is_on);
+ return ret;
}
+
+ return 0;
}
static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
@@ -545,7 +551,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
rockchip_pmu_set_idle_request(pd, true);
}
- rockchip_do_pmu_set_power_domain(pd, power_on);
+ ret = rockchip_do_pmu_set_power_domain(pd, power_on);
+ if (ret < 0) {
+ clk_bulk_disable(pd->num_clks, pd->clks);
+ return ret;
+ }
if (power_on) {
/* if powering up, leave idle mode */
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/6] pmdomain: rockchip: reduce indentation in rockchip_pd_power
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 1/6] pmdomain: rockchip: cleanup mutex handling in rockchip_pd_power Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 2/6] pmdomain: rockchip: forward rockchip_do_pmu_set_power_domain errors Sebastian Reichel
@ 2024-09-19 9:12 ` Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 4/6] dt-bindings: power: rockchip: add regulator support Sebastian Reichel
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
Rework the logic, so that the function exits early when the
power domain state is already correct to reduce code indentation.
No functional change intended.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 45 +++++++++++++-------------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 5e5291dedd28..663d390faaeb 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -537,36 +537,37 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
guard(mutex)(&pmu->mutex);
- if (rockchip_pmu_domain_is_on(pd) != power_on) {
- ret = clk_bulk_enable(pd->num_clks, pd->clks);
- if (ret < 0) {
- dev_err(pmu->dev, "failed to enable clocks\n");
- return ret;
- }
+ if (rockchip_pmu_domain_is_on(pd) == power_on)
+ return 0;
- if (!power_on) {
- rockchip_pmu_save_qos(pd);
+ ret = clk_bulk_enable(pd->num_clks, pd->clks);
+ if (ret < 0) {
+ dev_err(pmu->dev, "failed to enable clocks\n");
+ return ret;
+ }
- /* if powering down, idle request to NIU first */
- rockchip_pmu_set_idle_request(pd, true);
- }
+ if (!power_on) {
+ rockchip_pmu_save_qos(pd);
- ret = rockchip_do_pmu_set_power_domain(pd, power_on);
- if (ret < 0) {
- clk_bulk_disable(pd->num_clks, pd->clks);
- return ret;
- }
+ /* if powering down, idle request to NIU first */
+ rockchip_pmu_set_idle_request(pd, true);
+ }
- if (power_on) {
- /* if powering up, leave idle mode */
- rockchip_pmu_set_idle_request(pd, false);
+ ret = rockchip_do_pmu_set_power_domain(pd, power_on);
+ if (ret < 0) {
+ clk_bulk_disable(pd->num_clks, pd->clks);
+ return ret;
+ }
- rockchip_pmu_restore_qos(pd);
- }
+ if (power_on) {
+ /* if powering up, leave idle mode */
+ rockchip_pmu_set_idle_request(pd, false);
- clk_bulk_disable(pd->num_clks, pd->clks);
+ rockchip_pmu_restore_qos(pd);
}
+ clk_bulk_disable(pd->num_clks, pd->clks);
+
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/6] dt-bindings: power: rockchip: add regulator support
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
` (2 preceding siblings ...)
2024-09-19 9:12 ` [PATCH v2 3/6] pmdomain: rockchip: reduce indentation in rockchip_pd_power Sebastian Reichel
@ 2024-09-19 9:12 ` Sebastian Reichel
2024-10-02 14:02 ` Robin Murphy
2024-09-19 9:12 ` [PATCH v2 5/6] pmdomain: " Sebastian Reichel
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
Add optional support for a voltage supply required to enable a
power domain. The binding follows the way it is handled by the
Mediatek binding to keep things consistent.
This will initially be used by the RK3588 GPU power domain, which
fails to be enabled when the GPU regulator is not enabled.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
.../devicetree/bindings/power/rockchip,power-controller.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
index 0d5e999a58f1..0b4c5b174812 100644
--- a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -131,6 +131,9 @@ $defs:
A number of phandles to clocks that need to be enabled
while power domain switches state.
+ domain-supply:
+ description: domain regulator supply.
+
pm_qos:
$ref: /schemas/types.yaml#/definitions/phandle-array
items:
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 5/6] pmdomain: rockchip: add regulator support
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
` (3 preceding siblings ...)
2024-09-19 9:12 ` [PATCH v2 4/6] dt-bindings: power: rockchip: add regulator support Sebastian Reichel
@ 2024-09-19 9:12 ` Sebastian Reichel
2024-09-20 23:03 ` Chen-Yu Tsai
2024-09-19 9:12 ` [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588 Sebastian Reichel
2024-10-02 10:59 ` [PATCH v2 0/6] Fix RK3588 GPU domain Ulf Hansson
6 siblings, 1 reply; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
Some power domains require extra voltages to be applied. For example
trying to enable the GPU domain on RK3588 fails when the SoC does not
have VDD GPU enabled.
The solution to temporarily change the device's device tree node has
been taken over from the Mediatek power domain driver.
The regulator is not acquired at probe time, since that creates circular
dependencies. The power domain driver must be probed early, since SoC
peripherals need it. Regulators on the other hand depend on SoC
peripherals like SPI, I2C or GPIO.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 56 +++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 663d390faaeb..4bc17b588419 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -18,6 +18,7 @@
#include <linux/of_clk.h>
#include <linux/clk.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/mfd/syscon.h>
#include <soc/rockchip/pm_domains.h>
#include <dt-bindings/power/px30-power.h>
@@ -89,6 +90,8 @@ struct rockchip_pm_domain {
u32 *qos_save_regs[MAX_QOS_REGS_NUM];
int num_clks;
struct clk_bulk_data *clks;
+ struct device_node *node;
+ struct regulator *supply;
};
struct rockchip_pmu {
@@ -571,18 +574,66 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
return 0;
}
+static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
+{
+ return pd->supply ? regulator_disable(pd->supply) : 0;
+}
+
+static int rockchip_pd_regulator_enable(struct rockchip_pm_domain *pd)
+{
+ struct rockchip_pmu *pmu = pd->pmu;
+ struct device_node *main_node;
+
+ if (!pd->supply) {
+ /*
+ * Find regulator in current power domain node.
+ * devm_regulator_get() finds regulator in a node and its child
+ * node, so set of_node to current power domain node then change
+ * back to original node after regulator is found for current
+ * power domain node.
+ */
+ main_node = pmu->dev->of_node;
+ pmu->dev->of_node = pd->node;
+ pd->supply = devm_regulator_get(pmu->dev, "domain");
+ pmu->dev->of_node = main_node;
+ if (IS_ERR(pd->supply)) {
+ pd->supply = NULL;
+ return 0;
+ }
+ }
+
+ return regulator_enable(pd->supply);
+}
+
static int rockchip_pd_power_on(struct generic_pm_domain *domain)
{
struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
+ int ret;
+
+ ret = rockchip_pd_regulator_enable(pd);
+ if (ret) {
+ dev_err(pd->pmu->dev, "Failed to enable supply: %d\n", ret);
+ return ret;
+ }
- return rockchip_pd_power(pd, true);
+ ret = rockchip_pd_power(pd, true);
+ if (ret)
+ rockchip_pd_regulator_disable(pd);
+
+ return ret;
}
static int rockchip_pd_power_off(struct generic_pm_domain *domain)
{
struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
+ int ret;
- return rockchip_pd_power(pd, false);
+ ret = rockchip_pd_power(pd, false);
+ if (ret)
+ return ret;
+
+ rockchip_pd_regulator_disable(pd);
+ return ret;
}
static int rockchip_pd_attach_dev(struct generic_pm_domain *genpd,
@@ -663,6 +714,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
pd->info = pd_info;
pd->pmu = pmu;
+ pd->node = node;
pd->num_clks = of_clk_get_parent_count(node);
if (pd->num_clks > 0) {
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
` (4 preceding siblings ...)
2024-09-19 9:12 ` [PATCH v2 5/6] pmdomain: " Sebastian Reichel
@ 2024-09-19 9:12 ` Sebastian Reichel
2024-09-19 11:33 ` Jonas Karlman
2024-10-02 10:59 ` [PATCH v2 0/6] Fix RK3588 GPU domain Ulf Hansson
6 siblings, 1 reply; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 9:12 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
Sebastian Reichel, kernel
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(-)
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 {
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588
2024-09-19 9:12 ` [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588 Sebastian Reichel
@ 2024-09-19 11:33 ` Jonas Karlman
2024-09-19 19:27 ` Sebastian Reichel
0 siblings, 1 reply; 14+ messages in thread
From: Jonas Karlman @ 2024-09-19 11:33 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson, Elaine Zhang, Adrián Martínez Larumbe,
Boris Brezillon, Chen-Yu Tsai, devicetree, linux-rockchip,
linux-kernel, linux-pm, kernel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588
2024-09-19 11:33 ` Jonas Karlman
@ 2024-09-19 19:27 ` Sebastian Reichel
2024-09-19 21:00 ` Jonas Karlman
0 siblings, 1 reply; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-19 19:27 UTC (permalink / raw)
To: Jonas Karlman
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson, Elaine Zhang, Adrián Martínez Larumbe,
Boris Brezillon, Chen-Yu Tsai, devicetree, linux-rockchip,
linux-kernel, linux-pm, kernel
[-- Attachment #1: Type: text/plain, Size: 3288 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588
2024-09-19 19:27 ` Sebastian Reichel
@ 2024-09-19 21:00 ` Jonas Karlman
0 siblings, 0 replies; 14+ messages in thread
From: Jonas Karlman @ 2024-09-19 21:00 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson, Elaine Zhang, Adrián Martínez Larumbe,
Boris Brezillon, Chen-Yu Tsai, devicetree, linux-rockchip,
linux-kernel, linux-pm, kernel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/6] pmdomain: rockchip: add regulator support
2024-09-19 9:12 ` [PATCH v2 5/6] pmdomain: " Sebastian Reichel
@ 2024-09-20 23:03 ` Chen-Yu Tsai
0 siblings, 0 replies; 14+ messages in thread
From: Chen-Yu Tsai @ 2024-09-20 23:03 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Ulf Hansson, Elaine Zhang, Adrián Martínez Larumbe,
Boris Brezillon, devicetree, linux-rockchip, linux-kernel,
linux-pm, kernel
On Thu, Sep 19, 2024 at 11:27 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Some power domains require extra voltages to be applied. For example
> trying to enable the GPU domain on RK3588 fails when the SoC does not
> have VDD GPU enabled.
>
> The solution to temporarily change the device's device tree node has
> been taken over from the Mediatek power domain driver.
>
> The regulator is not acquired at probe time, since that creates circular
> dependencies. The power domain driver must be probed early, since SoC
> peripherals need it. Regulators on the other hand depend on SoC
> peripherals like SPI, I2C or GPIO.
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Adrian Larumbe <adrian.larumbe@collabora.com> # On Rock 5B
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/pmdomain/rockchip/pm-domains.c | 56 +++++++++++++++++++++++++-
> 1 file changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 663d390faaeb..4bc17b588419 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -18,6 +18,7 @@
> #include <linux/of_clk.h>
> #include <linux/clk.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/mfd/syscon.h>
> #include <soc/rockchip/pm_domains.h>
> #include <dt-bindings/power/px30-power.h>
> @@ -89,6 +90,8 @@ struct rockchip_pm_domain {
> u32 *qos_save_regs[MAX_QOS_REGS_NUM];
> int num_clks;
> struct clk_bulk_data *clks;
> + struct device_node *node;
> + struct regulator *supply;
> };
>
> struct rockchip_pmu {
> @@ -571,18 +574,66 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
> return 0;
> }
>
> +static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
> +{
> + return pd->supply ? regulator_disable(pd->supply) : 0;
> +}
> +
> +static int rockchip_pd_regulator_enable(struct rockchip_pm_domain *pd)
> +{
> + struct rockchip_pmu *pmu = pd->pmu;
> + struct device_node *main_node;
> +
> + if (!pd->supply) {
> + /*
> + * Find regulator in current power domain node.
> + * devm_regulator_get() finds regulator in a node and its child
> + * node, so set of_node to current power domain node then change
> + * back to original node after regulator is found for current
> + * power domain node.
> + */
> + main_node = pmu->dev->of_node;
> + pmu->dev->of_node = pd->node;
> + pd->supply = devm_regulator_get(pmu->dev, "domain");
How do you differentiate between a power domain not needing a supply,
vs a power domain that is missing its supply in DT?
You're using the normal "devm_regulator_get()" here (no "_optional),
which gives the dummy supply if no supply is specified in the DT, and
I think that wouldn't work properly.
I'm trying to work out if having "devm_of_regulator_get()" is needed or
not.
Also, this could return -EPROBE_DEFER. I guess it works with the initial
pm_domain_attach() from the driver core?
Thanks
ChenYu
> + pmu->dev->of_node = main_node;
> + if (IS_ERR(pd->supply)) {
> + pd->supply = NULL;
> + return 0;
> + }
> + }
> +
> + return regulator_enable(pd->supply);
> +}
> +
> static int rockchip_pd_power_on(struct generic_pm_domain *domain)
> {
> struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
> + int ret;
> +
> + ret = rockchip_pd_regulator_enable(pd);
> + if (ret) {
> + dev_err(pd->pmu->dev, "Failed to enable supply: %d\n", ret);
> + return ret;
> + }
>
> - return rockchip_pd_power(pd, true);
> + ret = rockchip_pd_power(pd, true);
> + if (ret)
> + rockchip_pd_regulator_disable(pd);
> +
> + return ret;
> }
>
> static int rockchip_pd_power_off(struct generic_pm_domain *domain)
> {
> struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
> + int ret;
>
> - return rockchip_pd_power(pd, false);
> + ret = rockchip_pd_power(pd, false);
> + if (ret)
> + return ret;
> +
> + rockchip_pd_regulator_disable(pd);
> + return ret;
> }
>
> static int rockchip_pd_attach_dev(struct generic_pm_domain *genpd,
> @@ -663,6 +714,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>
> pd->info = pd_info;
> pd->pmu = pmu;
> + pd->node = node;
>
> pd->num_clks = of_clk_get_parent_count(node);
> if (pd->num_clks > 0) {
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/6] Fix RK3588 GPU domain
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
` (5 preceding siblings ...)
2024-09-19 9:12 ` [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588 Sebastian Reichel
@ 2024-10-02 10:59 ` Ulf Hansson
2024-10-02 13:12 ` Chen-Yu Tsai
6 siblings, 1 reply; 14+ messages in thread
From: Ulf Hansson @ 2024-10-02 10:59 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
kernel
On Thu, 19 Sept 2024 at 11:18, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> I got a report, that the Linux kernel crashes on Rock 5B when the panthor
> driver is loaded late after booting. The crash starts with the following
> shortened error print:
>
> rockchip-pm-domain fd8d8000.power-management:power-controller: failed to set domain 'gpu', val=0
> rockchip-pm-domain fd8d8000.power-management:power-controller: failed to get ack on domain 'gpu', val=0xa9fff
> SError Interrupt on CPU4, code 0x00000000be000411 -- SError
>
> This series first does some cleanups in the Rockchip power domain
> driver and changes the driver, so that it no longer tries to continue
> when it fails to enable a domain. This gets rid of the SError interrupt
> and long backtraces. But the kernel still hangs when it fails to enable
> a power domain. I have not done further analysis to check if that can
> be avoided.
>
> Last but not least this provides a fix for the GPU power domain failing
> to get enabled - after some testing from my side it seems to require the
> GPU voltage supply to be enabled.
>
> I'm not really happy about the hack to get a regulator for a sub-node,
> which I took over from the Mediatek driver. I discussed this with
> Chen-Yu Tsai and Heiko Stübner at OSS EU and the plan is:
>
> 1. Merge Rockchip PM domain driver with this hack for now, since DRM CI
> people need it
> 2. Chen-Yu will work on a series, which fixes the hack in Mediatek by
> introducing a new devm_regulator_get function taking an DT node as
> additional argument
> 3. Rockchip PM domain later will switch to that once it has landed
I have just queued up 2) on my next branch.
My suggestion is to skip the intermediate step in 1) and go directly
for 3) instead, unless you think there is a problem with that, of
course?
[...]
Kind regards
Uffe
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/6] Fix RK3588 GPU domain
2024-10-02 10:59 ` [PATCH v2 0/6] Fix RK3588 GPU domain Ulf Hansson
@ 2024-10-02 13:12 ` Chen-Yu Tsai
0 siblings, 0 replies; 14+ messages in thread
From: Chen-Yu Tsai @ 2024-10-02 13:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Elaine Zhang, Adrián Martínez Larumbe,
Boris Brezillon, devicetree, linux-rockchip, linux-kernel,
linux-pm, kernel
On Wed, Oct 2, 2024 at 7:00 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 19 Sept 2024 at 11:18, Sebastian Reichel
> <sebastian.reichel@collabora.com> wrote:
> >
> > Hi,
> >
> > I got a report, that the Linux kernel crashes on Rock 5B when the panthor
> > driver is loaded late after booting. The crash starts with the following
> > shortened error print:
> >
> > rockchip-pm-domain fd8d8000.power-management:power-controller: failed to set domain 'gpu', val=0
> > rockchip-pm-domain fd8d8000.power-management:power-controller: failed to get ack on domain 'gpu', val=0xa9fff
> > SError Interrupt on CPU4, code 0x00000000be000411 -- SError
> >
> > This series first does some cleanups in the Rockchip power domain
> > driver and changes the driver, so that it no longer tries to continue
> > when it fails to enable a domain. This gets rid of the SError interrupt
> > and long backtraces. But the kernel still hangs when it fails to enable
> > a power domain. I have not done further analysis to check if that can
> > be avoided.
> >
> > Last but not least this provides a fix for the GPU power domain failing
> > to get enabled - after some testing from my side it seems to require the
> > GPU voltage supply to be enabled.
> >
> > I'm not really happy about the hack to get a regulator for a sub-node,
> > which I took over from the Mediatek driver. I discussed this with
> > Chen-Yu Tsai and Heiko Stübner at OSS EU and the plan is:
> >
> > 1. Merge Rockchip PM domain driver with this hack for now, since DRM CI
> > people need it
> > 2. Chen-Yu will work on a series, which fixes the hack in Mediatek by
> > introducing a new devm_regulator_get function taking an DT node as
> > additional argument
> > 3. Rockchip PM domain later will switch to that once it has landed
>
> I have just queued up 2) on my next branch.
>
> My suggestion is to skip the intermediate step in 1) and go directly
> for 3) instead, unless you think there is a problem with that, of
> course?
I don't think we were expecting things to get merged so soon. And IIRC
Sebastian went on vacation after Plumbers for two weeks.
ChenYu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/6] dt-bindings: power: rockchip: add regulator support
2024-09-19 9:12 ` [PATCH v2 4/6] dt-bindings: power: rockchip: add regulator support Sebastian Reichel
@ 2024-10-02 14:02 ` Robin Murphy
0 siblings, 0 replies; 14+ messages in thread
From: Robin Murphy @ 2024-10-02 14:02 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Ulf Hansson
Cc: Elaine Zhang, Adrián Martínez Larumbe, Boris Brezillon,
Chen-Yu Tsai, devicetree, linux-rockchip, linux-kernel, linux-pm,
kernel
On 2024-09-19 10:12 am, Sebastian Reichel wrote:
> Add optional support for a voltage supply required to enable a
> power domain. The binding follows the way it is handled by the
> Mediatek binding to keep things consistent.
>
> This will initially be used by the RK3588 GPU power domain, which
> fails to be enabled when the GPU regulator is not enabled.
Note that this applies equally to RK3399 and quite possibly others too,
it's just that so far it's always been bodged by making the relevant
regulator always-on (e.g. [1]).
Cheers,
Robin.
[1] https://lore.kernel.org/all/20210619121446.7802-1-knaerzche@gmail.com/
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> .../devicetree/bindings/power/rockchip,power-controller.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> index 0d5e999a58f1..0b4c5b174812 100644
> --- a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> @@ -131,6 +131,9 @@ $defs:
> A number of phandles to clocks that need to be enabled
> while power domain switches state.
>
> + domain-supply:
> + description: domain regulator supply.
> +
> pm_qos:
> $ref: /schemas/types.yaml#/definitions/phandle-array
> items:
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-10-02 14:02 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-19 9:12 [PATCH v2 0/6] Fix RK3588 GPU domain Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 1/6] pmdomain: rockchip: cleanup mutex handling in rockchip_pd_power Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 2/6] pmdomain: rockchip: forward rockchip_do_pmu_set_power_domain errors Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 3/6] pmdomain: rockchip: reduce indentation in rockchip_pd_power Sebastian Reichel
2024-09-19 9:12 ` [PATCH v2 4/6] dt-bindings: power: rockchip: add regulator support Sebastian Reichel
2024-10-02 14:02 ` Robin Murphy
2024-09-19 9:12 ` [PATCH v2 5/6] pmdomain: " Sebastian Reichel
2024-09-20 23:03 ` Chen-Yu Tsai
2024-09-19 9:12 ` [PATCH v2 6/6] arm64: dts: rockchip: Add GPU power domain regulator dependency for RK3588 Sebastian Reichel
2024-09-19 11:33 ` Jonas Karlman
2024-09-19 19:27 ` Sebastian Reichel
2024-09-19 21:00 ` Jonas Karlman
2024-10-02 10:59 ` [PATCH v2 0/6] Fix RK3588 GPU domain Ulf Hansson
2024-10-02 13:12 ` Chen-Yu Tsai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).