[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9c817fca-dbf0-45a5-9c4f-f0022f756f8d@imgtec.com>
Date: Fri, 30 Jan 2026 11:58:40 +0000
From: Matt Coster <Matt.Coster@...tec.com>
To: Alessio Belle <Alessio.Belle@...tec.com>
CC: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard
<mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Geert Uytterhoeven
<geert@...ux-m68k.org>,
Frank Binns <Frank.Binns@...tec.com>,
Brajesh Gupta
<Brajesh.Gupta@...tec.com>,
Alexandru Dadu <Alexandru.Dadu@...tec.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()
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@...tec.com>
>
> Reviewed-by: Alessio Belle <alessio.belle@...tec.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@...tec.com
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists