* [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
@ 2025-12-05 6:47 Frank Zhang
2025-12-08 3:17 ` Chaoyi Chen
2025-12-09 13:12 ` Sebastian Reichel
0 siblings, 2 replies; 5+ messages in thread
From: Frank Zhang @ 2025-12-05 6:47 UTC (permalink / raw)
To: ulf.hansson, heiko; +Cc: linux-rockchip, linux-pm, linux-kernel, Frank Zhang
RK3588_PD_NPU initialize as GENPD_STATE_ON before regulator ready.
rknn_iommu initlized success and suspend RK3588_PD_NPU. When rocket
driver register, it will resume rknn_iommu.
If regulator is still not ready at this point, rknn_iommu resume fail,
pm runtime status will be error: -EPROBE_DEFER.
This patch check regulator when pmdomain init, if regulator is not ready
or not enabled, power off pmdomain. Consumer device can power on it's
pmdomain after regulator ready
Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 1955c6d453e4..bc69f5d840e6 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -659,6 +659,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
return ret;
}
+static bool rockchip_pd_regulator_is_enabled(struct rockchip_pm_domain *pd)
+{
+ return IS_ERR_OR_NULL(pd->supply) ? false : regulator_is_enabled(pd->supply);
+}
+
static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
{
return IS_ERR_OR_NULL(pd->supply) ? 0 : regulator_disable(pd->supply);
@@ -861,6 +866,15 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
pd->genpd.name = pd->info->name;
else
pd->genpd.name = kbasename(node->full_name);
+
+ if (pd->info->need_regulator) {
+ if (IS_ERR_OR_NULL(pd->supply))
+ pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");
+
+ if (!rockchip_pd_regulator_is_enabled(pd))
+ rockchip_pd_power(pd, false);
+ }
+
pd->genpd.power_off = rockchip_pd_power_off;
pd->genpd.power_on = rockchip_pd_power_on;
pd->genpd.attach_dev = rockchip_pd_attach_dev;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
2025-12-05 6:47 [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready Frank Zhang
@ 2025-12-08 3:17 ` Chaoyi Chen
2025-12-12 12:19 ` Quentin Schulz
2025-12-09 13:12 ` Sebastian Reichel
1 sibling, 1 reply; 5+ messages in thread
From: Chaoyi Chen @ 2025-12-08 3:17 UTC (permalink / raw)
To: Frank Zhang
Cc: ulf.hansson, heiko, Quentin Schulz, linux-rockchip, linux-pm,
linux-kernel
On 12/5/2025 2:47 PM, Frank Zhang wrote:
> RK3588_PD_NPU initialize as GENPD_STATE_ON before regulator ready.
> rknn_iommu initlized success and suspend RK3588_PD_NPU. When rocket
> driver register, it will resume rknn_iommu.
>
> If regulator is still not ready at this point, rknn_iommu resume fail,
> pm runtime status will be error: -EPROBE_DEFER.
>
> This patch check regulator when pmdomain init, if regulator is not ready
> or not enabled, power off pmdomain. Consumer device can power on it's
> pmdomain after regulator ready
>
> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
> ---
> drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 1955c6d453e4..bc69f5d840e6 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -659,6 +659,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
> return ret;
> }
>
> +static bool rockchip_pd_regulator_is_enabled(struct rockchip_pm_domain *pd)
> +{
> + return IS_ERR_OR_NULL(pd->supply) ? false : regulator_is_enabled(pd->supply);
> +}
> +
> static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
> {
> return IS_ERR_OR_NULL(pd->supply) ? 0 : regulator_disable(pd->supply);
> @@ -861,6 +866,15 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
> pd->genpd.name = pd->info->name;
> else
> pd->genpd.name = kbasename(node->full_name);
> +
> + if (pd->info->need_regulator) {
> + if (IS_ERR_OR_NULL(pd->supply))
> + pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");
> +
> + if (!rockchip_pd_regulator_is_enabled(pd))
> + rockchip_pd_power(pd, false);
> + }
> +
> pd->genpd.power_off = rockchip_pd_power_off;
> pd->genpd.power_on = rockchip_pd_power_on;
> pd->genpd.attach_dev = rockchip_pd_attach_dev;
>
@Quentin, Could you please try this patch? This should resolve the
problem you're currently facing where the NPU fails to work in
built-in module.
Tested-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
--
Best,
Chaoyi
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
2025-12-05 6:47 [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready Frank Zhang
2025-12-08 3:17 ` Chaoyi Chen
@ 2025-12-09 13:12 ` Sebastian Reichel
2025-12-09 15:04 ` rmxpzlb
1 sibling, 1 reply; 5+ messages in thread
From: Sebastian Reichel @ 2025-12-09 13:12 UTC (permalink / raw)
To: Frank Zhang; +Cc: ulf.hansson, heiko, linux-rockchip, linux-pm, linux-kernel
Hi,
On Fri, Dec 05, 2025 at 02:47:39PM +0800, Frank Zhang wrote:
> RK3588_PD_NPU initialize as GENPD_STATE_ON before regulator ready.
> rknn_iommu initlized success and suspend RK3588_PD_NPU. When rocket
> driver register, it will resume rknn_iommu.
>
> If regulator is still not ready at this point, rknn_iommu resume fail,
> pm runtime status will be error: -EPROBE_DEFER.
>
> This patch check regulator when pmdomain init, if regulator is not ready
> or not enabled, power off pmdomain. Consumer device can power on it's
> pmdomain after regulator ready
>
> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
> ---
> drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 1955c6d453e4..bc69f5d840e6 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -659,6 +659,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
> return ret;
> }
>
> +static bool rockchip_pd_regulator_is_enabled(struct rockchip_pm_domain *pd)
> +{
> + return IS_ERR_OR_NULL(pd->supply) ? false : regulator_is_enabled(pd->supply);
> +}
> +
> static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
> {
> return IS_ERR_OR_NULL(pd->supply) ? 0 : regulator_disable(pd->supply);
> @@ -861,6 +866,15 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
> pd->genpd.name = pd->info->name;
> else
> pd->genpd.name = kbasename(node->full_name);
> +
> + if (pd->info->need_regulator) {
> + if (IS_ERR_OR_NULL(pd->supply))
> + pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");
> +
> + if (!rockchip_pd_regulator_is_enabled(pd))
> + rockchip_pd_power(pd, false);
> + }
It is extremly unlikely, that you will be able to get the regulator
at driver probe time. The typical regulator for NPU or GPU is
connected via SPI or I2C, which will only be available after the
power domain driver has been probed. So I suppose this could be
simplified as:
--------------------------------------------
/*
* power domain's needing a regulator should default to off, since
* the regulator state is unknown at probe time. Also the regulator
* state cannot be checked, since that usually requires IP needing
* (a different) power domain.
*/
if (pd->info->need_regulator)
rockchip_pd_power(pd, false);
--------------------------------------------
I think the proper fix would be to add support for registering the
regulator needing power-domain's delayed and then enforce requesting
the regulator at probe time. That's not trivial to implement, though.
Greetings,
-- Sebastian
> +
> pd->genpd.power_off = rockchip_pd_power_off;
> pd->genpd.power_on = rockchip_pd_power_on;
> pd->genpd.attach_dev = rockchip_pd_attach_dev;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
2025-12-09 13:12 ` Sebastian Reichel
@ 2025-12-09 15:04 ` rmxpzlb
0 siblings, 0 replies; 5+ messages in thread
From: rmxpzlb @ 2025-12-09 15:04 UTC (permalink / raw)
To: sebastian.reichel
Cc: heiko, linux-kernel, linux-pm, linux-rockchip, rmxpzlb,
ulf.hansson
>Hi,
>> On Fri, Dec 05, 2025 at 02:47:39PM +0800, Frank Zhang wrote:
>> RK3588_PD_NPU initialize as GENPD_STATE_ON before regulator ready.
>> rknn_iommu initlized success and suspend RK3588_PD_NPU. When rocket
>> driver register, it will resume rknn_iommu.
>>
>> If regulator is still not ready at this point, rknn_iommu resume fail,
>> pm runtime status will be error: -EPROBE_DEFER.
>>
>> This patch check regulator when pmdomain init, if regulator is not ready
>> or not enabled, power off pmdomain. Consumer device can power on it's
>> pmdomain after regulator ready
>>
>> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
>> ---
>> drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
>> index 1955c6d453e4..bc69f5d840e6 100644
>> --- a/drivers/pmdomain/rockchip/pm-domains.c
>> +++ b/drivers/pmdomain/rockchip/pm-domains.c
>> @@ -659,6 +659,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>> return ret;
>> }
>>
>> +static bool rockchip_pd_regulator_is_enabled(struct rockchip_pm_domain *pd)
>> +{
>> + return IS_ERR_OR_NULL(pd->supply) ? false : regulator_is_enabled(pd->supply);
>> +}
>> +
>> static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
>> {
>> return IS_ERR_OR_NULL(pd->supply) ? 0 : regulator_disable(pd->supply);
>> @@ -861,6 +866,15 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>> pd->genpd.name = pd->info->name;
>> else
>> pd->genpd.name = kbasename(node->full_name);
>> +
>> + if (pd->info->need_regulator) {
>> + if (IS_ERR_OR_NULL(pd->supply))
>> + pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");
>> +
>> + if (!rockchip_pd_regulator_is_enabled(pd))
>> + rockchip_pd_power(pd, false);
>> + }
>
>It is extremly unlikely, that you will be able to get the regulator
>at driver probe time. The typical regulator for NPU or GPU is
>connected via SPI or I2C, which will only be available after the
>power domain driver has been probed. So I suppose this could be
>simplified as:
>
>--------------------------------------------
>/*
> * power domain's needing a regulator should default to off, since
> * the regulator state is unknown at probe time. Also the regulator
> * state cannot be checked, since that usually requires IP needing
> * (a different) power domain.
> */
>if (pd->info->need_regulator)
> rockchip_pd_power(pd, false);
>--------------------------------------------
>
>I think the proper fix would be to add support for registering the
>regulator needing power-domain's delayed and then enforce requesting
>the regulator at probe time. That's not trivial to implement, though.
>
>Greetings,
>
>-- Sebastian
>
>> +
>> pd->genpd.power_off = rockchip_pd_power_off;
>> pd->genpd.power_on = rockchip_pd_power_on;
>> pd->genpd.attach_dev = rockchip_pd_attach_dev;
Thanks for your comment. This simplification is OK for me.
I will test and send new patch.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
2025-12-08 3:17 ` Chaoyi Chen
@ 2025-12-12 12:19 ` Quentin Schulz
0 siblings, 0 replies; 5+ messages in thread
From: Quentin Schulz @ 2025-12-12 12:19 UTC (permalink / raw)
To: Chaoyi Chen, Frank Zhang
Cc: ulf.hansson, heiko, linux-rockchip, linux-pm, linux-kernel
Hi Chaoyi, Frank,
On 12/8/25 4:17 AM, Chaoyi Chen wrote:
> On 12/5/2025 2:47 PM, Frank Zhang wrote:
>> RK3588_PD_NPU initialize as GENPD_STATE_ON before regulator ready.
>> rknn_iommu initlized success and suspend RK3588_PD_NPU. When rocket
>> driver register, it will resume rknn_iommu.
>>
>> If regulator is still not ready at this point, rknn_iommu resume fail,
>> pm runtime status will be error: -EPROBE_DEFER.
>>
>> This patch check regulator when pmdomain init, if regulator is not ready
>> or not enabled, power off pmdomain. Consumer device can power on it's
>> pmdomain after regulator ready
>>
>> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
>> ---
>> drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
>> index 1955c6d453e4..bc69f5d840e6 100644
>> --- a/drivers/pmdomain/rockchip/pm-domains.c
>> +++ b/drivers/pmdomain/rockchip/pm-domains.c
>> @@ -659,6 +659,11 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>> return ret;
>> }
>>
>> +static bool rockchip_pd_regulator_is_enabled(struct rockchip_pm_domain *pd)
>> +{
>> + return IS_ERR_OR_NULL(pd->supply) ? false : regulator_is_enabled(pd->supply);
>> +}
>> +
>> static int rockchip_pd_regulator_disable(struct rockchip_pm_domain *pd)
>> {
>> return IS_ERR_OR_NULL(pd->supply) ? 0 : regulator_disable(pd->supply);
>> @@ -861,6 +866,15 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>> pd->genpd.name = pd->info->name;
>> else
>> pd->genpd.name = kbasename(node->full_name);
>> +
>> + if (pd->info->need_regulator) {
>> + if (IS_ERR_OR_NULL(pd->supply))
>> + pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");
>> +
>> + if (!rockchip_pd_regulator_is_enabled(pd))
>> + rockchip_pd_power(pd, false);
>> + }
>> +
>> pd->genpd.power_off = rockchip_pd_power_off;
>> pd->genpd.power_on = rockchip_pd_power_on;
>> pd->genpd.attach_dev = rockchip_pd_attach_dev;
>>
>
> @Quentin, Could you please try this patch? This should resolve the
> problem you're currently facing where the NPU fails to work in
> built-in module.
>
Thanks for the heads up, Chaoyi!
I tested that for a few hours with a bootloop script (700+) and seems
like I couldn't reproduce the issue 4 reported in
https://lore.kernel.org/linux-rockchip/0b20d760-ad4f-41c0-b733-39db10d6cc41@cherry.de/
when building wih DRM_ACCEL_ROCKET=y.
So:
Tested-by: Quentin Schulz <quentin.schulz@cherry.de> # NPU on RK3588 Jaguar
@Frank, please add me in Cc if there's a v2 so I can recheck if you want.
Thanks for having a look!
Cheers,
Quentin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-12-12 12:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-05 6:47 [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready Frank Zhang
2025-12-08 3:17 ` Chaoyi Chen
2025-12-12 12:19 ` Quentin Schulz
2025-12-09 13:12 ` Sebastian Reichel
2025-12-09 15:04 ` rmxpzlb
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).