mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices
@ 2026-02-27 14:12 Matt Coster
  2026-02-27 14:12 ` [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list Matt Coster
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Matt Coster @ 2026-02-27 14:12 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter
  Cc: Mark Brown, Geert Uytterhoeven, Frank Binns, Alessio Belle,
	Brajesh Gupta, Alexandru Dadu, dri-devel, linux-kernel,
	Matt Coster

The first patch here fixes the exact issue reported by Mark in the
linked issue. The remaining patches are related foot-gun-like issues
that were discovered in the process.

Although entirely fixes, this series is targetting drm-misc-next since
the underlying commit e19cc5ab347e ("drm/imagination: Use
dev_pm_domain_attach_list()") does not yet exist in any other trees (to
the best of my knowledge).

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
Matt Coster (3):
      drm/imagination: Check for NULL struct dev_pm_domain_list
      drm/imagination: Detach pm domains if linking fails
      drm/imagination: Ensure struct pvr_device->power is initialized

 drivers/gpu/drm/imagination/pvr_power.c | 52 ++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 17 deletions(-)
---
base-commit: 5ea5b6ff0d63aef1dc3fb25445acea183f61a934
change-id: 20260227-single-domain-power-fixes-d272d53589a9


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list
  2026-02-27 14:12 [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Matt Coster
@ 2026-02-27 14:12 ` Matt Coster
  2026-02-28 14:25   ` Mark Brown
  2026-02-27 14:12 ` [PATCH 2/3] drm/imagination: Detach pm domains if linking fails Matt Coster
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Matt Coster @ 2026-02-27 14:12 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter
  Cc: Mark Brown, Geert Uytterhoeven, Frank Binns, Alessio Belle,
	Brajesh Gupta, Alexandru Dadu, dri-devel, linux-kernel,
	Matt Coster

While dev_pm_domain_detach_list() itself contains the necessary NULL check,
the access to struct dev_pm_domain_list->num_pds does not and thus faults
on devices with <=1 power domains (where the struct dev_pm_domain_list
machinery is skipped for simplicity).

This can be reproduced on AM625, which produces the following log[1]:

[   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

Fixes: e19cc5ab347e3 ("drm/imagination: Use dev_pm_domain_attach_list()")
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/r/c353fdef-9ccd-4a11-a527-ab4a792d8e70@sirena.org.uk/ [1]
Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_power.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index 006a72ed5064..be8018085b2d 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -668,14 +668,16 @@ void pvr_power_domains_fini(struct pvr_device *pvr_dev)
 {
 	struct pvr_device_power *pvr_power = &pvr_dev->power;
 
-	int i = (int)pvr_power->domains->num_pds - 1;
+	if (!pvr_power->domains)
+		goto out;
 
-	while (--i >= 0)
+	for (int i = (int)pvr_power->domains->num_pds - 2; i >= 0; --i)
 		device_link_del(pvr_power->domain_links[i]);
 
 	dev_pm_domain_detach_list(pvr_power->domains);
 
 	kfree(pvr_power->domain_links);
 
+out:
 	*pvr_power = (struct pvr_device_power){ 0 };
 }

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] drm/imagination: Detach pm domains if linking fails
  2026-02-27 14:12 [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Matt Coster
  2026-02-27 14:12 ` [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list Matt Coster
@ 2026-02-27 14:12 ` Matt Coster
  2026-02-27 14:12 ` [PATCH 3/3] drm/imagination: Ensure struct pvr_device->power is initialized Matt Coster
  2026-03-02 11:40 ` [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Alessio Belle
  3 siblings, 0 replies; 6+ messages in thread
From: Matt Coster @ 2026-02-27 14:12 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter
  Cc: Mark Brown, Geert Uytterhoeven, Frank Binns, Alessio Belle,
	Brajesh Gupta, Alexandru Dadu, dri-devel, linux-kernel,
	Matt Coster

There's a missing call to dev_pm_domain_detach_list() in the error path of
pvr_power_domains_init(); if creating the second stage of device links
fails then the struct dev_pm_domain_list will be left dangling.

Fixes: e19cc5ab347e3 ("drm/imagination: Use dev_pm_domain_attach_list()")
Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_power.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index be8018085b2d..5a1fda685f2c 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -661,6 +661,8 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 	while (--i >= 0)
 		device_link_del(domain_links[i]);
 
+	dev_pm_domain_detach_list(domains);
+
 	return err;
 }
 

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] drm/imagination: Ensure struct pvr_device->power is initialized
  2026-02-27 14:12 [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Matt Coster
  2026-02-27 14:12 ` [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list Matt Coster
  2026-02-27 14:12 ` [PATCH 2/3] drm/imagination: Detach pm domains if linking fails Matt Coster
@ 2026-02-27 14:12 ` Matt Coster
  2026-03-02 11:40 ` [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Alessio Belle
  3 siblings, 0 replies; 6+ messages in thread
From: Matt Coster @ 2026-02-27 14:12 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter
  Cc: Mark Brown, Geert Uytterhoeven, Frank Binns, Alessio Belle,
	Brajesh Gupta, Alexandru Dadu, dri-devel, linux-kernel,
	Matt Coster

When pvr_power_domains_init() handles <=1 power domains, the content of
struct pvr_device->power was previously left uninitialized.

Fixes: e19cc5ab347e3 ("drm/imagination: Use dev_pm_domain_attach_list()")
Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_power.c | 44 ++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index 5a1fda685f2c..7a8765c0c1ed 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -598,8 +598,8 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 	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 dev_pm_domain_list *domains = NULL;
+	struct device_link **domain_links = NULL;
 	int domain_count;
 	int link_count;
 
@@ -608,23 +608,30 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 
 	domain_count = of_count_phandle_with_args(dev->of_node, "power-domains",
 						  "#power-domain-cells");
-	if (domain_count < 0)
-		return domain_count;
+	if (domain_count < 0) {
+		err = domain_count;
+		goto out;
+	}
 
-	if (domain_count <= 1)
-		return 0;
+	if (domain_count <= 1) {
+		err = 0;
+		goto out;
+	}
 
 	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;
+		err = -EOPNOTSUPP;
+		goto out;
 	}
 
 	link_count = domain_count - 1;
 
 	domain_links = kzalloc_objs(*domain_links, link_count);
-	if (!domain_links)
-		return -ENOMEM;
+	if (!domain_links) {
+		err = -ENOMEM;
+		goto out;
+	}
 
 	const struct dev_pm_domain_attach_data pd_attach_data = {
 		.pd_names = ROGUE_PD_NAMES,
@@ -634,7 +641,7 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 
 	err = dev_pm_domain_attach_list(dev, &pd_attach_data, &domains);
 	if (err < 0)
-		return err;
+		goto err_free_links;
 
 	for (i = 0; i < link_count; i++) {
 		struct device_link *link;
@@ -650,18 +657,25 @@ int pvr_power_domains_init(struct pvr_device *pvr_dev)
 		domain_links[i] = link;
 	}
 
-	pvr_dev->power = (struct pvr_device_power){
-		.domains = domains,
-		.domain_links = no_free_ptr(domain_links),
-	};
-
-	return 0;
+	err = 0;
+	goto out;
 
 err_unlink:
 	while (--i >= 0)
 		device_link_del(domain_links[i]);
 
 	dev_pm_domain_detach_list(domains);
+	domains = NULL;
+
+err_free_links:
+	kfree(domain_links);
+	domain_links = NULL;
+
+out:
+	pvr_dev->power = (struct pvr_device_power){
+		.domains = domains,
+		.domain_links = domain_links,
+	};
 
 	return err;
 }

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list
  2026-02-27 14:12 ` [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list Matt Coster
@ 2026-02-28 14:25   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2026-02-28 14:25 UTC (permalink / raw)
  To: Matt Coster
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Geert Uytterhoeven, Frank Binns,
	Alessio Belle, Brajesh Gupta, Alexandru Dadu, dri-devel,
	linux-kernel

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

On Fri, Feb 27, 2026 at 02:12:47PM +0000, Matt Coster wrote:
> While dev_pm_domain_detach_list() itself contains the necessary NULL check,
> the access to struct dev_pm_domain_list->num_pds does not and thus faults
> on devices with <=1 power domains (where the struct dev_pm_domain_list
> machinery is skipped for simplicity).
> 
> This can be reproduced on AM625, which produces the following log[1]:

Tested-by: Mark Brown <broonie@kernel.org>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices
  2026-02-27 14:12 [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Matt Coster
                   ` (2 preceding siblings ...)
  2026-02-27 14:12 ` [PATCH 3/3] drm/imagination: Ensure struct pvr_device->power is initialized Matt Coster
@ 2026-03-02 11:40 ` Alessio Belle
  3 siblings, 0 replies; 6+ messages in thread
From: Alessio Belle @ 2026-03-02 11:40 UTC (permalink / raw)
  To: Matt Coster
  Cc: tzimmermann, simona, broonie, dri-devel, geert, airlied,
	Frank Binns, maarten.lankhorst, Brajesh Gupta, mripard,
	linux-kernel, Alexandru Dadu

On Fri, 2026-02-27 at 14:12 +0000, Matt Coster wrote:
> The first patch here fixes the exact issue reported by Mark in the
> linked issue. The remaining patches are related foot-gun-like issues
> that were discovered in the process.
> 
> Although entirely fixes, this series is targetting drm-misc-next since
> the underlying commit e19cc5ab347e ("drm/imagination: Use
> dev_pm_domain_attach_list()") does not yet exist in any other trees (to
> the best of my knowledge).
> 
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> ---
> Matt Coster (3):
>       drm/imagination: Check for NULL struct dev_pm_domain_list
>       drm/imagination: Detach pm domains if linking fails
>       drm/imagination: Ensure struct pvr_device->power is initialized
> 
>  drivers/gpu/drm/imagination/pvr_power.c | 52 ++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 17 deletions(-)
> ---
> base-commit: 5ea5b6ff0d63aef1dc3fb25445acea183f61a934
> change-id: 20260227-single-domain-power-fixes-d272d53589a9
> 

For the whole serie,

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

Thanks,
Alessio

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-02 11:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-27 14:12 [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Matt Coster
2026-02-27 14:12 ` [PATCH 1/3] drm/imagination: Check for NULL struct dev_pm_domain_list Matt Coster
2026-02-28 14:25   ` Mark Brown
2026-02-27 14:12 ` [PATCH 2/3] drm/imagination: Detach pm domains if linking fails Matt Coster
2026-02-27 14:12 ` [PATCH 3/3] drm/imagination: Ensure struct pvr_device->power is initialized Matt Coster
2026-03-02 11:40 ` [PATCH 0/3] drm/imagination: Fixes for power domain handling on single-domain devices Alessio Belle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®