[PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Matt Coster]

This helper handles the attaching and linking of the entire list of power
domains. Besides making pvr_power_domains_init() simpler, this also lays
the groundwork to simplify supporting the varied power domain names used in
Volcanic GPU cores.

Note that we still need to create the links between power domains to ensure
they're brought up in a valid sequence.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
We've had this patch kicking around internally for a while; it's been
held up by discussions as to whether we actually need the dependencies
between domains for the hardware to behave currectly. As it turns out,
the answer is yes.

Geert sent a similar patch[1] yesterday which didn't retain the
inter-domain links and suggested we just send this one instead of
reworking his. Thank you for the kick up the backside to progress this
one! :)

[1]: https://lore.kernel.org/r/194465eda54d1f852a9226cf691ddc5aa208e0a3.1769097977.git.geert+renesas@glider.be/
---
 drivers/gpu/drm/imagination/pvr_device.h | 10 ++--
 drivers/gpu/drm/imagination/pvr_power.c  | 80 ++++++++++++--------------------
 2 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index cfda215e7428e..d51c57cf93323 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -152,15 +152,13 @@ struct pvr_device {
 	 * @power: Optional power domain devices.
 	 *
 	 * On platforms with more than one power domain for the GPU, they are
-	 * stored here in @domain_devs, along with links between them in
-	 * @domain_links. The size of @domain_devs is given by @domain_count,
-	 * while the size of @domain_links is (2 * @domain_count) - 1.
+	 * stored here in @domains, along with links between them in
+	 * @domain_links. The size of @domain_links is one less than
+	 * struct dev_pm_domain_list->num_pds in @domains.
 	 */
 	struct pvr_device_power {
-		struct device **domain_devs;
+		struct dev_pm_domain_list *domains;
 		struct device_link **domain_links;
-
-		u32 domain_count;
 	} power;
 
 	/**
diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index b9f801c63260c..a0834c550a852 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -593,14 +593,16 @@ pvr_watchdog_fini(struct pvr_device *pvr_dev)
 
 int pvr_power_domains_init(struct pvr_device *pvr_dev)
 {
-	struct device *dev = from_pvr_device(pvr_dev)->dev;
+	static const char *const ROGUE_PD_NAMES[] = { "a", "b", "c", "d", "e" };
+
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
+	struct device *dev = drm_dev->dev;
 
 	struct device_link **domain_links __free(kfree) = NULL;
-	struct device **domain_devs __free(kfree) = NULL;
+	struct dev_pm_domain_list *domains = NULL;
 	int domain_count;
 	int link_count;
 
-	char dev_name[2] = "a";
 	int err;
 	int i;
 
@@ -612,46 +614,33 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 	if (domain_count <= 1)
 		return 0;
 
-	link_count = domain_count + (domain_count - 1);
+	if (domain_count > ARRAY_SIZE(ROGUE_PD_NAMES)) {
+		drm_err(drm_dev, "%s() only supports %zu domains on Rogue",
+			__func__, ARRAY_SIZE(ROGUE_PD_NAMES));
+		return -EOPNOTSUPP;
+	}
 
-	domain_devs = kcalloc(domain_count, sizeof(*domain_devs), GFP_KERNEL);
-	if (!domain_devs)
-		return -ENOMEM;
+	link_count = domain_count - 1;
 
 	domain_links = kcalloc(link_count, sizeof(*domain_links), GFP_KERNEL);
 	if (!domain_links)
 		return -ENOMEM;
 
-	for (i = 0; i < domain_count; i++) {
-		struct device *domain_dev;
-
-		dev_name[0] = 'a' + i;
-		domain_dev = dev_pm_domain_attach_by_name(dev, dev_name);
-		if (IS_ERR_OR_NULL(domain_dev)) {
-			err = domain_dev ? PTR_ERR(domain_dev) : -ENODEV;
-			goto err_detach;
-		}
-
-		domain_devs[i] = domain_dev;
-	}
-
-	for (i = 0; i < domain_count; i++) {
-		struct device_link *link;
-
-		link = device_link_add(dev, domain_devs[i], DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
-		if (!link) {
-			err = -ENODEV;
-			goto err_unlink;
-		}
+	const struct dev_pm_domain_attach_data pd_attach_data = {
+		.pd_names = ROGUE_PD_NAMES,
+		.num_pd_names = domain_count,
+		.pd_flags = 0,
+	};
 
-		domain_links[i] = link;
-	}
+	err = dev_pm_domain_attach_list(dev, &pd_attach_data, &domains);
+	if (err < 0)
+		return err;
 
-	for (i = domain_count; i < link_count; i++) {
+	for (i = 0; i < link_count; i++) {
 		struct device_link *link;
 
-		link = device_link_add(domain_devs[i - domain_count + 1],
-				       domain_devs[i - domain_count],
+		link = device_link_add(domains->pd_devs[i + 1],
+				       domains->pd_devs[i],
 				       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
 		if (!link) {
 			err = -ENODEV;
@@ -662,9 +651,8 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 	}
 
 	pvr_dev->power = (struct pvr_device_power){
-		.domain_devs = no_free_ptr(domain_devs),
+		.domains = domains,
 		.domain_links = no_free_ptr(domain_links),
-		.domain_count = domain_count,
 	};
 
 	return 0;
@@ -673,31 +661,21 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 	while (--i >= 0)
 		device_link_del(domain_links[i]);
 
-	i = domain_count;
-
-err_detach:
-	while (--i >= 0)
-		dev_pm_domain_detach(domain_devs[i], true);
-
 	return err;
 }
 
 void pvr_power_domains_fini(struct pvr_device *pvr_dev)
 {
-	const int domain_count = pvr_dev->power.domain_count;
+	struct pvr_device_power *pvr_power = &pvr_dev->power;
 
-	int i = domain_count + (domain_count - 1);
+	int i = (int)pvr_power->domains->num_pds - 1;
 
 	while (--i >= 0)
-		device_link_del(pvr_dev->power.domain_links[i]);
-
-	i = domain_count;
+		device_link_del(pvr_power->domain_links[i]);
 
-	while (--i >= 0)
-		dev_pm_domain_detach(pvr_dev->power.domain_devs[i], true);
+	dev_pm_domain_detach_list(pvr_power->domains);
 
-	kfree(pvr_dev->power.domain_links);
-	kfree(pvr_dev->power.domain_devs);
+	kfree(pvr_power->domain_links);
 
-	pvr_dev->power = (struct pvr_device_power){ 0 };
+	*pvr_power = (struct pvr_device_power){ 0 };
 }

---
base-commit: 15bd2f5d52de890f745ac0c60a44cd27d095bb0d
change-id: 20251201-pm-domain-attach-list-ef4ec12a9271

Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Alessio Belle]

On Fri, 2026-01-23 at 14:44 +0000, Matt Coster wrote:
> This helper handles the attaching and linking of the entire list of power
> domains. Besides making pvr_power_domains_init() simpler, this also lays
> the groundwork to simplify supporting the varied power domain names used in
> Volcanic GPU cores.
> 
> Note that we still need to create the links between power domains to ensure
> they're brought up in a valid sequence.
> 
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>

Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks,
Alessio

> ---
> We've had this patch kicking around internally for a while; it's been
> held up by discussions as to whether we actually need the dependencies
> between domains for the hardware to behave currectly. As it turns out,
> the answer is yes.
> 
> Geert sent a similar patch[1] yesterday which didn't retain the
> inter-domain links and suggested we just send this one instead of
> reworking his. Thank you for the kick up the backside to progress this
> one! :)
> 
> [1]: https://lore.kernel.org/r/194465eda54d1f852a9226cf691ddc5aa208e0a3.1769097977.git.geert+renesas@glider.be/
> ---
>  drivers/gpu/drm/imagination/pvr_device.h | 10 ++--
>  drivers/gpu/drm/imagination/pvr_power.c  | 80 ++++++++++++--------------------
>  2 files changed, 33 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
> index cfda215e7428e..d51c57cf93323 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.h
> +++ b/drivers/gpu/drm/imagination/pvr_device.h
> @@ -152,15 +152,13 @@ struct pvr_device {
>  	 * @power: Optional power domain devices.
>  	 *
>  	 * On platforms with more than one power domain for the GPU, they are
> -	 * stored here in @domain_devs, along with links between them in
> -	 * @domain_links. The size of @domain_devs is given by @domain_count,
> -	 * while the size of @domain_links is (2 * @domain_count) - 1.
> +	 * stored here in @domains, along with links between them in
> +	 * @domain_links. The size of @domain_links is one less than
> +	 * struct dev_pm_domain_list->num_pds in @domains.
>  	 */
>  	struct pvr_device_power {
> -		struct device **domain_devs;
> +		struct dev_pm_domain_list *domains;
>  		struct device_link **domain_links;
> -
> -		u32 domain_count;
>  	} power;
>  
>  	/**
> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> index b9f801c63260c..a0834c550a852 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -593,14 +593,16 @@ pvr_watchdog_fini(struct pvr_device *pvr_dev)
>  
>  int pvr_power_domains_init(struct pvr_device *pvr_dev)
>  {
> -	struct device *dev = from_pvr_device(pvr_dev)->dev;
> +	static const char *const ROGUE_PD_NAMES[] = { "a", "b", "c", "d", "e" };
> +
> +	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> +	struct device *dev = drm_dev->dev;
>  
>  	struct device_link **domain_links __free(kfree) = NULL;
> -	struct device **domain_devs __free(kfree) = NULL;
> +	struct dev_pm_domain_list *domains = NULL;
>  	int domain_count;
>  	int link_count;
>  
> -	char dev_name[2] = "a";
>  	int err;
>  	int i;
>  
> @@ -612,46 +614,33 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
>  	if (domain_count <= 1)
>  		return 0;
>  
> -	link_count = domain_count + (domain_count - 1);
> +	if (domain_count > ARRAY_SIZE(ROGUE_PD_NAMES)) {
> +		drm_err(drm_dev, "%s() only supports %zu domains on Rogue",
> +			__func__, ARRAY_SIZE(ROGUE_PD_NAMES));
> +		return -EOPNOTSUPP;
> +	}
>  
> -	domain_devs = kcalloc(domain_count, sizeof(*domain_devs), GFP_KERNEL);
> -	if (!domain_devs)
> -		return -ENOMEM;
> +	link_count = domain_count - 1;
>  
>  	domain_links = kcalloc(link_count, sizeof(*domain_links), GFP_KERNEL);
>  	if (!domain_links)
>  		return -ENOMEM;
>  
> -	for (i = 0; i < domain_count; i++) {
> -		struct device *domain_dev;
> -
> -		dev_name[0] = 'a' + i;
> -		domain_dev = dev_pm_domain_attach_by_name(dev, dev_name);
> -		if (IS_ERR_OR_NULL(domain_dev)) {
> -			err = domain_dev ? PTR_ERR(domain_dev) : -ENODEV;
> -			goto err_detach;
> -		}
> -
> -		domain_devs[i] = domain_dev;
> -	}
> -
> -	for (i = 0; i < domain_count; i++) {
> -		struct device_link *link;
> -
> -		link = device_link_add(dev, domain_devs[i], DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
> -		if (!link) {
> -			err = -ENODEV;
> -			goto err_unlink;
> -		}
> +	const struct dev_pm_domain_attach_data pd_attach_data = {
> +		.pd_names = ROGUE_PD_NAMES,
> +		.num_pd_names = domain_count,
> +		.pd_flags = 0,
> +	};
>  
> -		domain_links[i] = link;
> -	}
> +	err = dev_pm_domain_attach_list(dev, &pd_attach_data, &domains);
> +	if (err < 0)
> +		return err;
>  
> -	for (i = domain_count; i < link_count; i++) {
> +	for (i = 0; i < link_count; i++) {
>  		struct device_link *link;
>  
> -		link = device_link_add(domain_devs[i - domain_count + 1],
> -				       domain_devs[i - domain_count],
> +		link = device_link_add(domains->pd_devs[i + 1],
> +				       domains->pd_devs[i],
>  				       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
>  		if (!link) {
>  			err = -ENODEV;
> @@ -662,9 +651,8 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
>  	}
>  
>  	pvr_dev->power = (struct pvr_device_power){
> -		.domain_devs = no_free_ptr(domain_devs),
> +		.domains = domains,
>  		.domain_links = no_free_ptr(domain_links),
> -		.domain_count = domain_count,
>  	};
>  
>  	return 0;
> @@ -673,31 +661,21 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
>  	while (--i >= 0)
>  		device_link_del(domain_links[i]);
>  
> -	i = domain_count;
> -
> -err_detach:
> -	while (--i >= 0)
> -		dev_pm_domain_detach(domain_devs[i], true);
> -
>  	return err;
>  }
>  
>  void pvr_power_domains_fini(struct pvr_device *pvr_dev)
>  {
> -	const int domain_count = pvr_dev->power.domain_count;
> +	struct pvr_device_power *pvr_power = &pvr_dev->power;
>  
> -	int i = domain_count + (domain_count - 1);
> +	int i = (int)pvr_power->domains->num_pds - 1;
>  
>  	while (--i >= 0)
> -		device_link_del(pvr_dev->power.domain_links[i]);
> -
> -	i = domain_count;
> +		device_link_del(pvr_power->domain_links[i]);
>  
> -	while (--i >= 0)
> -		dev_pm_domain_detach(pvr_dev->power.domain_devs[i], true);
> +	dev_pm_domain_detach_list(pvr_power->domains);
>  
> -	kfree(pvr_dev->power.domain_links);
> -	kfree(pvr_dev->power.domain_devs);
> +	kfree(pvr_power->domain_links);
>  
> -	pvr_dev->power = (struct pvr_device_power){ 0 };
> +	*pvr_power = (struct pvr_device_power){ 0 };
>  }
> 
> ---
> base-commit: 15bd2f5d52de890f745ac0c60a44cd27d095bb0d
> change-id: 20251201-pm-domain-attach-list-ef4ec12a9271
> 

Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 8207 bytes --]

On 30/01/2026 09:03, Alessio Belle wrote:
> On Fri, 2026-01-23 at 14:44 +0000, Matt Coster wrote:
>> This helper handles the attaching and linking of the entire list of power
>> domains. Besides making pvr_power_domains_init() simpler, this also lays
>> the groundwork to simplify supporting the varied power domain names used in
>> Volcanic GPU cores.
>>
>> Note that we still need to create the links between power domains to ensure
>> they're brought up in a valid sequence.
>>
>> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> 
> Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

I'll leave it until Monday or so to land this in drm-misc-next, just in
case anybody has any concerns or comments on this approach compared to
Geert's suggestion (linked below).

Cheers,
Matt

> 
> Thanks,
> Alessio
> 
>> ---
>> We've had this patch kicking around internally for a while; it's been
>> held up by discussions as to whether we actually need the dependencies
>> between domains for the hardware to behave currectly. As it turns out,
>> the answer is yes.
>>
>> Geert sent a similar patch[1] yesterday which didn't retain the
>> inter-domain links and suggested we just send this one instead of
>> reworking his. Thank you for the kick up the backside to progress this
>> one! :)
>>
>> [1]: https://lore.kernel.org/r/194465eda54d1f852a9226cf691ddc5aa208e0a3.1769097977.git.geert+renesas@glider.be/
>> ---
>>  drivers/gpu/drm/imagination/pvr_device.h | 10 ++--
>>  drivers/gpu/drm/imagination/pvr_power.c  | 80 ++++++++++++--------------------
>>  2 files changed, 33 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
>> index cfda215e7428e..d51c57cf93323 100644
>> --- a/drivers/gpu/drm/imagination/pvr_device.h
>> +++ b/drivers/gpu/drm/imagination/pvr_device.h
>> @@ -152,15 +152,13 @@ struct pvr_device {
>>        * @power: Optional power domain devices.
>>        *
>>        * On platforms with more than one power domain for the GPU, they are
>> -      * stored here in @domain_devs, along with links between them in
>> -      * @domain_links. The size of @domain_devs is given by @domain_count,
>> -      * while the size of @domain_links is (2 * @domain_count) - 1.
>> +      * stored here in @domains, along with links between them in
>> +      * @domain_links. The size of @domain_links is one less than
>> +      * struct dev_pm_domain_list->num_pds in @domains.
>>        */
>>       struct pvr_device_power {
>> -             struct device **domain_devs;
>> +             struct dev_pm_domain_list *domains;
>>               struct device_link **domain_links;
>> -
>> -             u32 domain_count;
>>       } power;
>>
>>       /**
>> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
>> index b9f801c63260c..a0834c550a852 100644
>> --- a/drivers/gpu/drm/imagination/pvr_power.c
>> +++ b/drivers/gpu/drm/imagination/pvr_power.c
>> @@ -593,14 +593,16 @@ pvr_watchdog_fini(struct pvr_device *pvr_dev)
>>
>>  int pvr_power_domains_init(struct pvr_device *pvr_dev)
>>  {
>> -     struct device *dev = from_pvr_device(pvr_dev)->dev;
>> +     static const char *const ROGUE_PD_NAMES[] = { "a", "b", "c", "d", "e" };
>> +
>> +     struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>> +     struct device *dev = drm_dev->dev;
>>
>>       struct device_link **domain_links __free(kfree) = NULL;
>> -     struct device **domain_devs __free(kfree) = NULL;
>> +     struct dev_pm_domain_list *domains = NULL;
>>       int domain_count;
>>       int link_count;
>>
>> -     char dev_name[2] = "a";
>>       int err;
>>       int i;
>>
>> @@ -612,46 +614,33 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
>>       if (domain_count <= 1)
>>               return 0;
>>
>> -     link_count = domain_count + (domain_count - 1);
>> +     if (domain_count > ARRAY_SIZE(ROGUE_PD_NAMES)) {
>> +             drm_err(drm_dev, "%s() only supports %zu domains on Rogue",
>> +                     __func__, ARRAY_SIZE(ROGUE_PD_NAMES));
>> +             return -EOPNOTSUPP;
>> +     }
>>
>> -     domain_devs = kcalloc(domain_count, sizeof(*domain_devs), GFP_KERNEL);
>> -     if (!domain_devs)
>> -             return -ENOMEM;
>> +     link_count = domain_count - 1;
>>
>>       domain_links = kcalloc(link_count, sizeof(*domain_links), GFP_KERNEL);
>>       if (!domain_links)
>>               return -ENOMEM;
>>
>> -     for (i = 0; i < domain_count; i++) {
>> -             struct device *domain_dev;
>> -
>> -             dev_name[0] = 'a' + i;
>> -             domain_dev = dev_pm_domain_attach_by_name(dev, dev_name);
>> -             if (IS_ERR_OR_NULL(domain_dev)) {
>> -                     err = domain_dev ? PTR_ERR(domain_dev) : -ENODEV;
>> -                     goto err_detach;
>> -             }
>> -
>> -             domain_devs[i] = domain_dev;
>> -     }
>> -
>> -     for (i = 0; i < domain_count; i++) {
>> -             struct device_link *link;
>> -
>> -             link = device_link_add(dev, domain_devs[i], DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
>> -             if (!link) {
>> -                     err = -ENODEV;
>> -                     goto err_unlink;
>> -             }
>> +     const struct dev_pm_domain_attach_data pd_attach_data = {
>> +             .pd_names = ROGUE_PD_NAMES,
>> +             .num_pd_names = domain_count,
>> +             .pd_flags = 0,
>> +     };
>>
>> -             domain_links[i] = link;
>> -     }
>> +     err = dev_pm_domain_attach_list(dev, &pd_attach_data, &domains);
>> +     if (err < 0)
>> +             return err;
>>
>> -     for (i = domain_count; i < link_count; i++) {
>> +     for (i = 0; i < link_count; i++) {
>>               struct device_link *link;
>>
>> -             link = device_link_add(domain_devs[i - domain_count + 1],
>> -                                    domain_devs[i - domain_count],
>> +             link = device_link_add(domains->pd_devs[i + 1],
>> +                                    domains->pd_devs[i],
>>                                      DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
>>               if (!link) {
>>                       err = -ENODEV;
>> @@ -662,9 +651,8 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
>>       }
>>
>>       pvr_dev->power = (struct pvr_device_power){
>> -             .domain_devs = no_free_ptr(domain_devs),
>> +             .domains = domains,
>>               .domain_links = no_free_ptr(domain_links),
>> -             .domain_count = domain_count,
>>       };
>>
>>       return 0;
>> @@ -673,31 +661,21 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
>>       while (--i >= 0)
>>               device_link_del(domain_links[i]);
>>
>> -     i = domain_count;
>> -
>> -err_detach:
>> -     while (--i >= 0)
>> -             dev_pm_domain_detach(domain_devs[i], true);
>> -
>>       return err;
>>  }
>>
>>  void pvr_power_domains_fini(struct pvr_device *pvr_dev)
>>  {
>> -     const int domain_count = pvr_dev->power.domain_count;
>> +     struct pvr_device_power *pvr_power = &pvr_dev->power;
>>
>> -     int i = domain_count + (domain_count - 1);
>> +     int i = (int)pvr_power->domains->num_pds - 1;
>>
>>       while (--i >= 0)
>> -             device_link_del(pvr_dev->power.domain_links[i]);
>> -
>> -     i = domain_count;
>> +             device_link_del(pvr_power->domain_links[i]);
>>
>> -     while (--i >= 0)
>> -             dev_pm_domain_detach(pvr_dev->power.domain_devs[i], true);
>> +     dev_pm_domain_detach_list(pvr_power->domains);
>>
>> -     kfree(pvr_dev->power.domain_links);
>> -     kfree(pvr_dev->power.domain_devs);
>> +     kfree(pvr_power->domain_links);
>>
>> -     pvr_dev->power = (struct pvr_device_power){ 0 };
>> +     *pvr_power = (struct pvr_device_power){ 0 };
>>  }
>>
>> ---
>> base-commit: 15bd2f5d52de890f745ac0c60a44cd27d095bb0d
>> change-id: 20251201-pm-domain-attach-list-ef4ec12a9271
>>
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Matt Coster]

On Fri, 23 Jan 2026 14:44:50 +0000, Matt Coster wrote:
> This helper handles the attaching and linking of the entire list of power
> domains. Besides making pvr_power_domains_init() simpler, this also lays
> the groundwork to simplify supporting the varied power domain names used in
> Volcanic GPU cores.
> 
> Note that we still need to create the links between power domains to ensure
> they're brought up in a valid sequence.
> 
> [...]

Applied, thanks!

[1/1] drm/imagination: Use dev_pm_domain_attach_list()
      commit: e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d

Best regards,
-- 
Matt Coster <matt.coster@imgtec.com>

Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 8854 bytes --]

On Fri, Jan 23, 2026 at 02:44:50PM +0000, Matt Coster wrote:
> This helper handles the attaching and linking of the entire list of power
> domains. Besides making pvr_power_domains_init() simpler, this also lays
> the groundwork to simplify supporting the varied power domain names used in
> Volcanic GPU cores.

I'm seeing oopses in -next on at least k3-am625-verdin-wifi-mallow which
bisect to this patch.  We get:

[   10.820056] powervr fd00000.gpu: Direct firmware load for powervr/rogue_33.15.11.3_v1.fw failed with error -2
[   10.831903] powervr fd00000.gpu: [drm] *ERROR* failed to load firmware powervr/rogue_33.15.11.3_v1.fw (err=-2)
...
[   10.844023] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000018
...
[   11.090162] Call trace:
[   11.092600]  pvr_power_domains_fini+0x18/0xa0 [powervr] (P)
[   11.098218]  pvr_probe+0x100/0x14c [powervr]
[   11.102505]  platform_probe+0x5c/0xa4

which does seem relevant to the changed code.

Full log:

   https://lava.sirena.org.uk/scheduler/job/2499326#L865

bisect log (with links to additional runtime logs):

# bad: [7d6661873f6b54c75195780a40d66bad3d482d8f] Add linux-next specific files for 20260226
# good: [d20332bddd695a63efdf0415f752bd25f4c69d9d] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
# good: [5c74a008ffc62fc57a041602b4517519c8bf9436] firmware: cs_dsp: Mark KUnit test suites KUNIT_SPEED_SLOW
# good: [260c3fff1fefc570d8f23e87953e181d7d248861] ASoC: cs-amp-lib-test: Stop including platform_device.h
# good: [ada32396f90951e12465224c04742607ca56a982] ASoC: SDCA: Add CS47L47 to class driver
# good: [bfd7db781e2e7a99b086d645a104d16e368f58ff] regulator: Kconfig: fix a typo
# good: [e02902dd493bf9c9b05353c761737ac514ad7a5c] spi: add devm_spi_new_ancillary_device()
# good: [7c12f6ead4672cb08b74e6f6115eb04dca8ccfa4] spi: tegra210-quad: Add runtime autosuspend support
# good: [37983fad7f3ef296fa0504c8e945987459dc5487] regmap: define cleanup helper for regmap_field
# good: [507a071d9868cb60e4e76f8a06fc8eb014f59ae4] spi: pxa2xx: use min() instead of min_t()
# good: [5ebc20921b7fff9feb44de465448e17a382c9965] ASoC: tas2552: Allow audio enable GPIO to sleep
# good: [fed6e5084894373d76270cad4a32eb6479ad8247] spi: atcspi200: Remove redundant assignment to .owner
# good: [fbb4c52ccdcb4a612d2b7f800aa57090eeee16d7] regulator: spacemit-p1: Update supply names
# good: [d075cef4af6327a5de4bee7bf77591e3201e54f4] ASoC: simple-card-utils: add sysclk ordering support
# good: [78dfbd4ad0be9f51de7b9a19388809254aeccd26] ASoC: Add quirk for Lecoo Bellator N176
# good: [0556bb42a84ee391a2145ddba86756f9747bc27f] regulator: pf0900: Make regu_irqs variable static const
# good: [c2bcf62ca75c541ec4297e6ff02a68ddc2e02029] regcache: Split regcache_count_cacheable_registers() helper
# good: [171b3663f33e1efdc97f5112f49be10b47b20fa8] ASoC: codecs: aw88261: Add firmware-name support
# good: [bf122191473e26a8f195308b1ba924c98424c8e1] ASoC: rt5677-spi: Add SPI device ID matching table
# good: [6d438685340df6ac8570326aaa51c3603a2fe25c] drm/fbdev-emulation: Remove empty placeholders
# good: [55473b60178060a4fdb4631bd0c91879cc7d18d8] drm/fbdev-emulation: Remove support for legacy emulation
# good: [cff3f89ffbdd4b6c43a117c01aaf5b290ff80803] drm/bridge: analogix_dp: Move &drm_bridge_funcs.mode_set to &drm_bridge_funcs.atomic_enable
# good: [779ec12c85c9e4547519e3903a371a3b26a289de] drm/komeda: fix integer overflow in AFBC framebuffer size check
git bisect start '7d6661873f6b54c75195780a40d66bad3d482d8f' 'd20332bddd695a63efdf0415f752bd25f4c69d9d' '5c74a008ffc62fc57a041602b4517519c8bf9436' '260c3fff1fefc570d8f23e87953e181d7d248861' 'ada32396f90951e12465224c04742607ca56a982' 'bfd7db781e2e7a99b086d645a104d16e368f58ff' 'e02902dd493bf9c9b05353c761737ac514ad7a5c' '7c12f6ead4672cb08b74e6f6115eb04dca8ccfa4' '37983fad7f3ef296fa0504c8e945987459dc5487' '507a071d9868cb60e4e76f8a06fc8eb014f59ae4' '5ebc20921b7fff9feb44de465448e17a382c9965' 'fed6e5084894373d76270cad4a32eb6479ad8247' 'fbb4c52ccdcb4a612d2b7f800aa57090eeee16d7' 'd075cef4af6327a5de4bee7bf77591e3201e54f4' '78dfbd4ad0be9f51de7b9a19388809254aeccd26' '0556bb42a84ee391a2145ddba86756f9747bc27f' 'c2bcf62ca75c541ec4297e6ff02a68ddc2e02029' '171b3663f33e1efdc97f5112f49be10b47b20fa8' 'bf122191473e26a8f195308b1ba924c98424c8e1' '6d438685340df6ac8570326aaa51c3603a2fe25c' '55473b60178060a4fdb4631bd0c91879cc7d18d8' 'cff3f89ffbdd4b6c43a117c01aaf5b290ff80803' '779ec12c85c9e4547519e3903a371a3b26a289de'
# test job: [5c74a008ffc62fc57a041602b4517519c8bf9436] https://lava.sirena.org.uk/scheduler/job/2496408
# test job: [260c3fff1fefc570d8f23e87953e181d7d248861] https://lava.sirena.org.uk/scheduler/job/2494139
# test job: [ada32396f90951e12465224c04742607ca56a982] https://lava.sirena.org.uk/scheduler/job/2489104
# test job: [bfd7db781e2e7a99b086d645a104d16e368f58ff] https://lava.sirena.org.uk/scheduler/job/2489537
# test job: [e02902dd493bf9c9b05353c761737ac514ad7a5c] https://lava.sirena.org.uk/scheduler/job/2489681
# test job: [7c12f6ead4672cb08b74e6f6115eb04dca8ccfa4] https://lava.sirena.org.uk/scheduler/job/2488641
# test job: [37983fad7f3ef296fa0504c8e945987459dc5487] https://lava.sirena.org.uk/scheduler/job/2489206
# test job: [507a071d9868cb60e4e76f8a06fc8eb014f59ae4] https://lava.sirena.org.uk/scheduler/job/2486373
# test job: [5ebc20921b7fff9feb44de465448e17a382c9965] https://lava.sirena.org.uk/scheduler/job/2485120
# test job: [fed6e5084894373d76270cad4a32eb6479ad8247] https://lava.sirena.org.uk/scheduler/job/2484679
# test job: [fbb4c52ccdcb4a612d2b7f800aa57090eeee16d7] https://lava.sirena.org.uk/scheduler/job/2482458
# test job: [d075cef4af6327a5de4bee7bf77591e3201e54f4] https://lava.sirena.org.uk/scheduler/job/2483472
# test job: [78dfbd4ad0be9f51de7b9a19388809254aeccd26] https://lava.sirena.org.uk/scheduler/job/2483100
# test job: [0556bb42a84ee391a2145ddba86756f9747bc27f] https://lava.sirena.org.uk/scheduler/job/2483206
# test job: [c2bcf62ca75c541ec4297e6ff02a68ddc2e02029] https://lava.sirena.org.uk/scheduler/job/2483259
# test job: [171b3663f33e1efdc97f5112f49be10b47b20fa8] https://lava.sirena.org.uk/scheduler/job/2482602
# test job: [bf122191473e26a8f195308b1ba924c98424c8e1] https://lava.sirena.org.uk/scheduler/job/2482819
# test job: [6d438685340df6ac8570326aaa51c3603a2fe25c] https://lava.sirena.org.uk/scheduler/job/2498159
# test job: [55473b60178060a4fdb4631bd0c91879cc7d18d8] https://lava.sirena.org.uk/scheduler/job/2498392
# test job: [cff3f89ffbdd4b6c43a117c01aaf5b290ff80803] https://lava.sirena.org.uk/scheduler/job/2497735
# test job: [779ec12c85c9e4547519e3903a371a3b26a289de] https://lava.sirena.org.uk/scheduler/job/2498023
# test job: [7d6661873f6b54c75195780a40d66bad3d482d8f] https://lava.sirena.org.uk/scheduler/job/2499326
# bad: [7d6661873f6b54c75195780a40d66bad3d482d8f] Add linux-next specific files for 20260226
git bisect bad 7d6661873f6b54c75195780a40d66bad3d482d8f
# test job: [fe0b1b8269eba268a7dee293b32d6c008a6c3fec] https://lava.sirena.org.uk/scheduler/job/2497668
# bad: [fe0b1b8269eba268a7dee293b32d6c008a6c3fec] drm/mcde: dsi: mcde_dsi_bind: break when a panel or bridge is found
git bisect bad fe0b1b8269eba268a7dee293b32d6c008a6c3fec
# test job: [3c2d28f4a67af7ada8f3332270b0d349967e6aa1] https://lava.sirena.org.uk/scheduler/job/2497695
# bad: [3c2d28f4a67af7ada8f3332270b0d349967e6aa1] drm/colorop: Use destroy callback for color pipeline teardown
git bisect bad 3c2d28f4a67af7ada8f3332270b0d349967e6aa1
# test job: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] https://lava.sirena.org.uk/scheduler/job/2498321
# bad: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] drm/imagination: Use dev_pm_domain_attach_list()
git bisect bad e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d
# first bad commit: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] drm/imagination: Use dev_pm_domain_attach_list()
# test job: [f81455b2d3327a5685623e7db4050dbbe5513bc3] https://lava.sirena.org.uk/scheduler/job/2497837
# bad: [f81455b2d3327a5685623e7db4050dbbe5513bc3] drm: bridge: anx7625: implement minimal Type-C support
git bisect bad f81455b2d3327a5685623e7db4050dbbe5513bc3
# test job: [1d7532444a32b53ff7344dc52019bab5a4b5ed66] https://lava.sirena.org.uk/scheduler/job/2499703
# bad: [1d7532444a32b53ff7344dc52019bab5a4b5ed66] dt-bindings: drm/bridge: anx7625: describe Type-C connector
git bisect bad 1d7532444a32b53ff7344dc52019bab5a4b5ed66
# test job: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] https://lava.sirena.org.uk/scheduler/job/2498321
# bad: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] drm/imagination: Use dev_pm_domain_attach_list()
git bisect bad e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d
# first bad commit: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] drm/imagination: Use dev_pm_domain_attach_list()

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Geert Uytterhoeven]

Hi Mark,

On Thu, 26 Feb 2026 at 18:24, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jan 23, 2026 at 02:44:50PM +0000, Matt Coster wrote:
> > This helper handles the attaching and linking of the entire list of power
> > domains. Besides making pvr_power_domains_init() simpler, this also lays
> > the groundwork to simplify supporting the varied power domain names used in
> > Volcanic GPU cores.
>
> I'm seeing oopses in -next on at least k3-am625-verdin-wifi-mallow which
> bisect to this patch.  We get:
>
> [   10.820056] powervr fd00000.gpu: Direct firmware load for powervr/rogue_33.15.11.3_v1.fw failed with error -2
> [   10.831903] powervr fd00000.gpu: [drm] *ERROR* failed to load firmware powervr/rogue_33.15.11.3_v1.fw (err=-2)
> ...
> [   10.844023] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000018
> ...
> [   11.090162] Call trace:
> [   11.092600]  pvr_power_domains_fini+0x18/0xa0 [powervr] (P)
> [   11.098218]  pvr_probe+0x100/0x14c [powervr]
> [   11.102505]  platform_probe+0x5c/0xa4
>
> which does seem relevant to the changed code.
>
> Full log:
>
>    https://lava.sirena.org.uk/scheduler/job/2499326#L865
>
> bisect log (with links to additional runtime logs):

> # first bad commit: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] drm/imagination: Use dev_pm_domain_attach_list()

Thanks for your report!

I can confirm this crash on e.g. R-Car H3 ES2.0 (after adding a
GPU node).  Actually I had seen it earlier this week, but didn't pay
enough attention.  Hence  I just assumed it was the known race condition
when removing multiple PM Domains, and thus missed it is a new bug.

The issue is that pvr_power->domains is a NULL pointer, thus causing
a crash when dereferencing that pointer:

    int i = (int)pvr_power->domains->num_pds - 1;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 2241 bytes --]

On 27/02/2026 08:10, Geert Uytterhoeven wrote:
> Hi Mark,
> 
> On Thu, 26 Feb 2026 at 18:24, Mark Brown <broonie@kernel.org> wrote:
>> On Fri, Jan 23, 2026 at 02:44:50PM +0000, Matt Coster wrote:
>>> This helper handles the attaching and linking of the entire list of power
>>> domains. Besides making pvr_power_domains_init() simpler, this also lays
>>> the groundwork to simplify supporting the varied power domain names used in
>>> Volcanic GPU cores.
>>
>> I'm seeing oopses in -next on at least k3-am625-verdin-wifi-mallow which
>> bisect to this patch.  We get:
>>
>> [   10.820056] powervr fd00000.gpu: Direct firmware load for powervr/rogue_33.15.11.3_v1.fw failed with error -2
>> [   10.831903] powervr fd00000.gpu: [drm] *ERROR* failed to load firmware powervr/rogue_33.15.11.3_v1.fw (err=-2)
>> ...
>> [   10.844023] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000018
>> ...
>> [   11.090162] Call trace:
>> [   11.092600]  pvr_power_domains_fini+0x18/0xa0 [powervr] (P)
>> [   11.098218]  pvr_probe+0x100/0x14c [powervr]
>> [   11.102505]  platform_probe+0x5c/0xa4
>>
>> which does seem relevant to the changed code.
>>
>> Full log:
>>
>>    https://lava.sirena.org.uk/scheduler/job/2499326*L865
>>
>> bisect log (with links to additional runtime logs):
> 
>> # first bad commit: [e19cc5ab347e3cdcc21c97ea5d11af8da7f1358d] drm/imagination: Use dev_pm_domain_attach_list()
> 
> Thanks for your report!

+1 from us :)

> 
> I can confirm this crash on e.g. R-Car H3 ES2.0 (after adding a
> GPU node).  Actually I had seen it earlier this week, but didn't pay
> enough attention.  Hence  I just assumed it was the known race condition
> when removing multiple PM Domains, and thus missed it is a new bug.
> 
> The issue is that pvr_power->domains is a NULL pointer, thus causing
> a crash when dereferencing that pointer:
> 
>     int i = (int)pvr_power->domains->num_pds - 1;

We've confirmed the same on AM625; there's patches coming today for this
fix and another possible foot-gun we spotted in the process.

Cheers,
Matt

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]