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

[Matt Coster <Matt.Coster@imgtec.com>]

[-- 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 --]