[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <848896564152798716484225d49df14255cde64e.camel@imgtec.com>
Date: Fri, 30 Jan 2026 09:03:34 +0000
From: Alessio Belle <Alessio.Belle@...tec.com>
To: Matt Coster <Matt.Coster@...tec.com>
CC: "tzimmermann@...e.de" <tzimmermann@...e.de>,
"simona@...ll.ch"
<simona@...ll.ch>,
"dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>,
"geert@...ux-m68k.org"
<geert@...ux-m68k.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
Frank Binns <Frank.Binns@...tec.com>,
"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
Brajesh Gupta <Brajesh.Gupta@...tec.com>,
"mripard@...nel.org"
<mripard@...nel.org>,
"airlied@...il.com" <airlied@...il.com>,
Alexandru Dadu
<Alexandru.Dadu@...tec.com>
Subject: Re: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()
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>
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
>
Powered by blists - more mailing lists