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 <[email protected]> > > Reviewed-by: Alessio Belle <[email protected]>
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+rene...@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: [email protected]
OpenPGP_signature.asc
Description: OpenPGP digital signature
