[PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready

Frank Zhang posted 1 patch 2 weeks ago
drivers/pmdomain/rockchip/pm-domains.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
[PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
Posted by Frank Zhang 2 weeks ago
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;
Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
Posted by Sebastian Reichel 1 week, 3 days ago
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;
Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
Posted by rmxpzlb 1 week, 2 days ago
>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.
Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
Posted by Chaoyi Chen 1 week, 4 days ago
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
Re: [PATCH] pmdomain:rockchip: Fix init genpd as GENPD_STATE_ON before regulator ready
Posted by Quentin Schulz 1 week ago
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