[PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Geert Uytterhoeven]

Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
helpers instead of open-coding multi PM Domain handling.

This changes behavior slightly:
  - The new handling is also applied in case of a single PM Domain,
  - PM Domains are now referred to by index instead of by name, but
    "make dtbs_check" enforces the actual naming and ordering anyway,
  - There are no longer device links created between virtual domain
    devices, only between virtual devices and the parent device.
None of this should have an actual impact on functionality.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Tested lightly on R-Car M3-W: driver probes and firmware is loaded.
---
 drivers/gpu/drm/imagination/pvr_device.h |  13 +--
 drivers/gpu/drm/imagination/pvr_power.c  | 105 ++---------------------
 2 files changed, 9 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index 491718fb87a1b608..a823f6f7e0b659c6 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -148,19 +148,12 @@ struct pvr_device {
 	struct clk *mem_clk;
 
 	/**
-	 * @power: Optional power domain devices.
+	 * @pds: 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, along with links between them.
 	 */
-	struct pvr_device_power {
-		struct device **domain_devs;
-		struct device_link **domain_links;
-
-		u32 domain_count;
-	} power;
+	struct dev_pm_domain_list *pds;
 
 	/**
 	 * @reset: Optional reset line.
diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index b9f801c63260cb81..cc6efab3c8b015ce 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -594,110 +594,17 @@ 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;
+	int ret;
 
-	struct device_link **domain_links __free(kfree) = NULL;
-	struct device **domain_devs __free(kfree) = NULL;
-	int domain_count;
-	int link_count;
-
-	char dev_name[2] = "a";
-	int err;
-	int i;
-
-	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 <= 1)
-		return 0;
-
-	link_count = domain_count + (domain_count - 1);
-
-	domain_devs = kcalloc(domain_count, sizeof(*domain_devs), GFP_KERNEL);
-	if (!domain_devs)
-		return -ENOMEM;
-
-	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;
-		}
-
-		domain_links[i] = link;
-	}
-
-	for (i = domain_count; i < link_count; i++) {
-		struct device_link *link;
-
-		link = device_link_add(domain_devs[i - domain_count + 1],
-				       domain_devs[i - domain_count],
-				       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
-		if (!link) {
-			err = -ENODEV;
-			goto err_unlink;
-		}
-
-		domain_links[i] = link;
-	}
-
-	pvr_dev->power = (struct pvr_device_power){
-		.domain_devs = no_free_ptr(domain_devs),
-		.domain_links = no_free_ptr(domain_links),
-		.domain_count = domain_count,
-	};
+	ret = dev_pm_domain_attach_list(dev, NULL, &pvr_dev->pds);
+	if (ret < 0)
+		return ret;
 
 	return 0;
-
-err_unlink:
-	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;
-
-	int i = domain_count + (domain_count - 1);
-
-	while (--i >= 0)
-		device_link_del(pvr_dev->power.domain_links[i]);
-
-	i = domain_count;
-
-	while (--i >= 0)
-		dev_pm_domain_detach(pvr_dev->power.domain_devs[i], true);
-
-	kfree(pvr_dev->power.domain_links);
-	kfree(pvr_dev->power.domain_devs);
-
-	pvr_dev->power = (struct pvr_device_power){ 0 };
+	dev_pm_domain_detach_list(pvr_dev->pds);
+	pvr_dev->pds = NULL;
 }
-- 
2.43.0

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster]

[-- Attachment #1.1.1: Type: text/plain, Size: 6265 bytes --]

On 22/01/2026 16:08, Geert Uytterhoeven wrote:
> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
> helpers instead of open-coding multi PM Domain handling.
> 
> This changes behavior slightly:
>   - The new handling is also applied in case of a single PM Domain,
>   - PM Domains are now referred to by index instead of by name, but
>     "make dtbs_check" enforces the actual naming and ordering anyway,
>   - There are no longer device links created between virtual domain
>     devices, only between virtual devices and the parent device.

We still need this guarantee, both at start and end of day. In the
current implementation dev_pm_domain_attach_list() iterates forwards,
but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
prefer not to rely on the implementation details when we can declare the
dependencies explicitly.

We had/have a patch (attached) kicking around internally to use the
*_list() functions but keep the inter-domain links in place; it got held
up by discussions as to whether we actually need those dependencies for
the hardware to behave correctly. Your patch spurred me to run around
the office and nag people a bit, and it seems we really do need to care
about the ordering.

Can you add the links back in for a V2 or I can properly send the
attached patch instead, I don't mind either way.

Cheers,
Matt

> None of this should have an actual impact on functionality.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Tested lightly on R-Car M3-W: driver probes and firmware is loaded.
> ---
>  drivers/gpu/drm/imagination/pvr_device.h |  13 +--
>  drivers/gpu/drm/imagination/pvr_power.c  | 105 ++---------------------
>  2 files changed, 9 insertions(+), 109 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
> index 491718fb87a1b608..a823f6f7e0b659c6 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.h
> +++ b/drivers/gpu/drm/imagination/pvr_device.h
> @@ -148,19 +148,12 @@ struct pvr_device {
>  	struct clk *mem_clk;
>  
>  	/**
> -	 * @power: Optional power domain devices.
> +	 * @pds: 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, along with links between them.
>  	 */
> -	struct pvr_device_power {
> -		struct device **domain_devs;
> -		struct device_link **domain_links;
> -
> -		u32 domain_count;
> -	} power;
> +	struct dev_pm_domain_list *pds;
>  
>  	/**
>  	 * @reset: Optional reset line.
> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> index b9f801c63260cb81..cc6efab3c8b015ce 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -594,110 +594,17 @@ 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;
> +	int ret;
>  
> -	struct device_link **domain_links __free(kfree) = NULL;
> -	struct device **domain_devs __free(kfree) = NULL;
> -	int domain_count;
> -	int link_count;
> -
> -	char dev_name[2] = "a";
> -	int err;
> -	int i;
> -
> -	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 <= 1)
> -		return 0;
> -
> -	link_count = domain_count + (domain_count - 1);
> -
> -	domain_devs = kcalloc(domain_count, sizeof(*domain_devs), GFP_KERNEL);
> -	if (!domain_devs)
> -		return -ENOMEM;
> -
> -	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;
> -		}
> -
> -		domain_links[i] = link;
> -	}
> -
> -	for (i = domain_count; i < link_count; i++) {
> -		struct device_link *link;
> -
> -		link = device_link_add(domain_devs[i - domain_count + 1],
> -				       domain_devs[i - domain_count],
> -				       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
> -		if (!link) {
> -			err = -ENODEV;
> -			goto err_unlink;
> -		}
> -
> -		domain_links[i] = link;
> -	}
> -
> -	pvr_dev->power = (struct pvr_device_power){
> -		.domain_devs = no_free_ptr(domain_devs),
> -		.domain_links = no_free_ptr(domain_links),
> -		.domain_count = domain_count,
> -	};
> +	ret = dev_pm_domain_attach_list(dev, NULL, &pvr_dev->pds);
> +	if (ret < 0)
> +		return ret;
>  
>  	return 0;
> -
> -err_unlink:
> -	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;
> -
> -	int i = domain_count + (domain_count - 1);
> -
> -	while (--i >= 0)
> -		device_link_del(pvr_dev->power.domain_links[i]);
> -
> -	i = domain_count;
> -
> -	while (--i >= 0)
> -		dev_pm_domain_detach(pvr_dev->power.domain_devs[i], true);
> -
> -	kfree(pvr_dev->power.domain_links);
> -	kfree(pvr_dev->power.domain_devs);
> -
> -	pvr_dev->power = (struct pvr_device_power){ 0 };
> +	dev_pm_domain_detach_list(pvr_dev->pds);
> +	pvr_dev->pds = NULL;
>  }


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #1.1.2: 0001-drm-imagination-Use-dev_pm_domain_attach_list.patch --]
[-- Type: text/plain, Size: 5741 bytes --]

From 0ff3dc3a8c8364574e6bdbeeb3067e70bd0e8c0a Mon Sep 17 00:00:00 2001
From: Matt Coster <matt.coster@imgtec.com>
Date: Tue, 25 Nov 2025 09:51:51 +0000
Subject: [PATCH] drm/imagination: Use dev_pm_domain_attach_list()

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>
---
 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 ec53ff2755418..759c639aafa2d 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -150,15 +150,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 };
 }
-- 
2.52.0


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Geert Uytterhoeven]

Hi Matt,

On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com> wrote:
> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
> > Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
> > helpers instead of open-coding multi PM Domain handling.
> >
> > This changes behavior slightly:
> >   - The new handling is also applied in case of a single PM Domain,
> >   - PM Domains are now referred to by index instead of by name, but
> >     "make dtbs_check" enforces the actual naming and ordering anyway,
> >   - There are no longer device links created between virtual domain
> >     devices, only between virtual devices and the parent device.
>
> We still need this guarantee, both at start and end of day. In the
> current implementation dev_pm_domain_attach_list() iterates forwards,
> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
> prefer not to rely on the implementation details when we can declare the
> dependencies explicitly.

Note that on R-Car, the PM Domains are nested (see e.g. r8a7795_areas[]),
so they are always (un)powered in the correct order.  But that may not
be the case in the integration on other SoCs.

> We had/have a patch (attached) kicking around internally to use the
> *_list() functions but keep the inter-domain links in place; it got held
> up by discussions as to whether we actually need those dependencies for
> the hardware to behave correctly. Your patch spurred me to run around
> the office and nag people a bit, and it seems we really do need to care
> about the ordering.

OK.

> Can you add the links back in for a V2 or I can properly send the
> attached patch instead, I don't mind either way.

Please move forward with your patch, you are the expert.
I prefer not to be blamed for any breakage ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Marek Vasut]

On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:

Hello everyone,

> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com> wrote:
>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>> helpers instead of open-coding multi PM Domain handling.
>>>
>>> This changes behavior slightly:
>>>    - The new handling is also applied in case of a single PM Domain,
>>>    - PM Domains are now referred to by index instead of by name, but
>>>      "make dtbs_check" enforces the actual naming and ordering anyway,
>>>    - There are no longer device links created between virtual domain
>>>      devices, only between virtual devices and the parent device.
>>
>> We still need this guarantee, both at start and end of day. In the
>> current implementation dev_pm_domain_attach_list() iterates forwards,
>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>> prefer not to rely on the implementation details when we can declare the
>> dependencies explicitly.
> 
> Note that on R-Car, the PM Domains are nested (see e.g. r8a7795_areas[]),
> so they are always (un)powered in the correct order.  But that may not
> be the case in the integration on other SoCs.
> 
>> We had/have a patch (attached) kicking around internally to use the
>> *_list() functions but keep the inter-domain links in place; it got held
>> up by discussions as to whether we actually need those dependencies for
>> the hardware to behave correctly. Your patch spurred me to run around
>> the office and nag people a bit, and it seems we really do need to care
>> about the ordering.
> 
> OK.
> 
>> Can you add the links back in for a V2 or I can properly send the
>> attached patch instead, I don't mind either way.
> 
> Please move forward with your patch, you are the expert.
> I prefer not to be blamed for any breakage ;-)

Has there been any progress on fixing this kernel crash ?

There are already two proposed solutions, but no fix is upstream.

-- 
Best regards,
Marek Vasut

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 2618 bytes --]

On 11/02/2026 19:17, Marek Vasut wrote:
> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
> 
> Hello everyone,
> 
>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com> wrote:
>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>> helpers instead of open-coding multi PM Domain handling.
>>>>
>>>> This changes behavior slightly:
>>>>    - The new handling is also applied in case of a single PM Domain,
>>>>    - PM Domains are now referred to by index instead of by name, but
>>>>      "make dtbs_check" enforces the actual naming and ordering anyway,
>>>>    - There are no longer device links created between virtual domain
>>>>      devices, only between virtual devices and the parent device.
>>>
>>> We still need this guarantee, both at start and end of day. In the
>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>> prefer not to rely on the implementation details when we can declare the
>>> dependencies explicitly.
>>
>> Note that on R-Car, the PM Domains are nested (see e.g. r8a7795_areas[]),
>> so they are always (un)powered in the correct order.  But that may not
>> be the case in the integration on other SoCs.
>>
>>> We had/have a patch (attached) kicking around internally to use the
>>> *_list() functions but keep the inter-domain links in place; it got held
>>> up by discussions as to whether we actually need those dependencies for
>>> the hardware to behave correctly. Your patch spurred me to run around
>>> the office and nag people a bit, and it seems we really do need to care
>>> about the ordering.
>>
>> OK.
>>
>>> Can you add the links back in for a V2 or I can properly send the
>>> attached patch instead, I don't mind either way.
>>
>> Please move forward with your patch, you are the expert.
>> I prefer not to be blamed for any breakage ;-)
> 
> Has there been any progress on fixing this kernel crash ?
> 
> There are already two proposed solutions, but no fix is upstream.
> 

Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use
dev_pm_domain_attach_list()"), but this does not fix the underlying
issue of missing synchronization in the PM core[1] is still unresolved
as far as I'm aware.

Cheers,
Matt

[1]: https://lore.kernel.org/r/CAMuHMdVOUzanEufhWqOL0nv81xCYh4YNAX_waG6y9PyUZ030tg@mail.gmail.com/

-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Marek Vasut]

On 2/12/26 10:00 AM, Matt Coster wrote:
> On 11/02/2026 19:17, Marek Vasut wrote:
>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>
>> Hello everyone,
>>
>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com> wrote:
>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>
>>>>> This changes behavior slightly:
>>>>>     - The new handling is also applied in case of a single PM Domain,
>>>>>     - PM Domains are now referred to by index instead of by name, but
>>>>>       "make dtbs_check" enforces the actual naming and ordering anyway,
>>>>>     - There are no longer device links created between virtual domain
>>>>>       devices, only between virtual devices and the parent device.
>>>>
>>>> We still need this guarantee, both at start and end of day. In the
>>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>>> prefer not to rely on the implementation details when we can declare the
>>>> dependencies explicitly.
>>>
>>> Note that on R-Car, the PM Domains are nested (see e.g. r8a7795_areas[]),
>>> so they are always (un)powered in the correct order.  But that may not
>>> be the case in the integration on other SoCs.
>>>
>>>> We had/have a patch (attached) kicking around internally to use the
>>>> *_list() functions but keep the inter-domain links in place; it got held
>>>> up by discussions as to whether we actually need those dependencies for
>>>> the hardware to behave correctly. Your patch spurred me to run around
>>>> the office and nag people a bit, and it seems we really do need to care
>>>> about the ordering.
>>>
>>> OK.
>>>
>>>> Can you add the links back in for a V2 or I can properly send the
>>>> attached patch instead, I don't mind either way.
>>>
>>> Please move forward with your patch, you are the expert.
>>> I prefer not to be blamed for any breakage ;-)
>>
>> Has there been any progress on fixing this kernel crash ?
>>
>> There are already two proposed solutions, but no fix is upstream.
>>
> 
> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use
> dev_pm_domain_attach_list()"), but this does not fix the underlying
> issue of missing synchronization in the PM core[1] is still unresolved
> as far as I'm aware.

OK, but the pvr driver can currently easily crash the kernel on boot if 
firmware is missing, so that should be fixed soon, right ?

I added the regressions list onto CC, because this seems like a problem 
worth tracking.

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Thorsten Leemhuis]

On 2/12/26 15:38, Marek Vasut wrote:
> On 2/12/26 10:00 AM, Matt Coster wrote:
>> On 11/02/2026 19:17, Marek Vasut wrote:
>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>> wrote:
>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>
>>>>>> This changes behavior slightly:
>>>>>>     - The new handling is also applied in case of a single PM Domain,
>>>>>>     - PM Domains are now referred to by index instead of by name, but
>>>>>>       "make dtbs_check" enforces the actual naming and ordering
>>>>>> anyway,
>>>>>>     - There are no longer device links created between virtual domain
>>>>>>       devices, only between virtual devices and the parent device.
>>>>>
>>>>> We still need this guarantee, both at start and end of day. In the
>>>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>>>> prefer not to rely on the implementation details when we can
>>>>> declare the
>>>>> dependencies explicitly.
>>>>
>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>> r8a7795_areas[]),
>>>> so they are always (un)powered in the correct order.  But that may not
>>>> be the case in the integration on other SoCs.
>>>>
>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>> held
>>>>> up by discussions as to whether we actually need those dependencies
>>>>> for
>>>>> the hardware to behave correctly. Your patch spurred me to run around
>>>>> the office and nag people a bit, and it seems we really do need to
>>>>> care
>>>>> about the ordering.
>>>>
>>>> OK.
>>>>
>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>> attached patch instead, I don't mind either way.
>>>>
>>>> Please move forward with your patch, you are the expert.
>>>> I prefer not to be blamed for any breakage ;-)
>>>
>>> Has there been any progress on fixing this kernel crash ?
>>>
>>> There are already two proposed solutions, but no fix is upstream.
>>
>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>> issue of missing synchronization in the PM core[1] is still unresolved
>> as far as I'm aware.
> 
> OK, but the pvr driver can currently easily crash the kernel on boot if
> firmware is missing, so that should be fixed soon, right ?

Well, drm-misc-next afaik means that the above mentioned fix would only
be merged in 7.1, which is ~4 months away, which is not really "soon"
I'd say. Or did I misjudge this?

> I added the regressions list onto CC, because this seems like a problem
> worth tracking.

Noticed that and wondered what change caused the regression. Did not
find a answer in a quick search on lore[1]. Because if it's a
regression, we maybe should just revert the culprit for now according to
Linus:
https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/

Ciao, Thorsten

[1] I guess this was the initial report from Geert?
https://lore.kernel.org/all/CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Marek Vasut]

On 2/12/26 4:56 PM, Thorsten Leemhuis wrote:
> On 2/12/26 15:38, Marek Vasut wrote:
>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>> wrote:
>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>
>>>>>>> This changes behavior slightly:
>>>>>>>      - The new handling is also applied in case of a single PM Domain,
>>>>>>>      - PM Domains are now referred to by index instead of by name, but
>>>>>>>        "make dtbs_check" enforces the actual naming and ordering
>>>>>>> anyway,
>>>>>>>      - There are no longer device links created between virtual domain
>>>>>>>        devices, only between virtual devices and the parent device.
>>>>>>
>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>>>>> prefer not to rely on the implementation details when we can
>>>>>> declare the
>>>>>> dependencies explicitly.
>>>>>
>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>> r8a7795_areas[]),
>>>>> so they are always (un)powered in the correct order.  But that may not
>>>>> be the case in the integration on other SoCs.
>>>>>
>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>> held
>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>> for
>>>>>> the hardware to behave correctly. Your patch spurred me to run around
>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>> care
>>>>>> about the ordering.
>>>>>
>>>>> OK.
>>>>>
>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>> attached patch instead, I don't mind either way.
>>>>>
>>>>> Please move forward with your patch, you are the expert.
>>>>> I prefer not to be blamed for any breakage ;-)
>>>>
>>>> Has there been any progress on fixing this kernel crash ?
>>>>
>>>> There are already two proposed solutions, but no fix is upstream.
>>>
>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>> issue of missing synchronization in the PM core[1] is still unresolved
>>> as far as I'm aware.
>>
>> OK, but the pvr driver can currently easily crash the kernel on boot if
>> firmware is missing, so that should be fixed soon, right ?
> 
> Well, drm-misc-next afaik means that the above mentioned fix would only
> be merged in 7.1, which is ~4 months away, which is not really "soon"
> I'd say. Or did I misjudge this?

The PM domain issue here crashes the kernel, so I think this would be 
material for drm-misc-fixes .

>> I added the regressions list onto CC, because this seems like a problem
>> worth tracking.
> 
> Noticed that and wondered what change caused the regression.

I think this one:

330e76d31697 ("drm/imagination: Add power domain control")

> Did not
> find a answer in a quick search on lore[1]. Because if it's a
> regression, we maybe should just revert the culprit for now according to
> Linus:
> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/
> 
> Ciao, Thorsten
> 
> [1] I guess this was the initial report from Geert?
> https://lore.kernel.org/all/CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/

It is.

I think there are other SoCs which depend on the power domain commit, so 
revert is not so clear cut anymore. But SoCs which have hierarchical 
power domains and which manage to probe this driver without having a 
firmware available for the GPU will simply end with crashed kernel, 
which is really not good.

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Thorsten Leemhuis]

On 2/13/26 23:52, Marek Vasut wrote:
> On 2/12/26 4:56 PM, Thorsten Leemhuis wrote:
>> On 2/12/26 15:38, Marek Vasut wrote:
>>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>>> wrote:
>>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>>> Call the dev_pm_domain_attach_list() and
>>>>>>>> dev_pm_domain_detach_list()
>>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>>
>>>>>>>> This changes behavior slightly:
>>>>>>>>      - The new handling is also applied in case of a single PM
>>>>>>>> Domain,
>>>>>>>>      - PM Domains are now referred to by index instead of by
>>>>>>>> name, but
>>>>>>>>        "make dtbs_check" enforces the actual naming and ordering
>>>>>>>> anyway,
>>>>>>>>      - There are no longer device links created between virtual
>>>>>>>> domain
>>>>>>>>        devices, only between virtual devices and the parent device.
>>>>>>>
>>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>>> current implementation dev_pm_domain_attach_list() iterates
>>>>>>> forwards,
>>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that,
>>>>>>> I'd
>>>>>>> prefer not to rely on the implementation details when we can
>>>>>>> declare the
>>>>>>> dependencies explicitly.
>>>>>>
>>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>>> r8a7795_areas[]),
>>>>>> so they are always (un)powered in the correct order.  But that may
>>>>>> not
>>>>>> be the case in the integration on other SoCs.
>>>>>>
>>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>>> held
>>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>>> for
>>>>>>> the hardware to behave correctly. Your patch spurred me to run
>>>>>>> around
>>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>>> care
>>>>>>> about the ordering.
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>>> attached patch instead, I don't mind either way.
>>>>>>
>>>>>> Please move forward with your patch, you are the expert.
>>>>>> I prefer not to be blamed for any breakage ;-)
>>>>>
>>>>> Has there been any progress on fixing this kernel crash ?
>>>>>
>>>>> There are already two proposed solutions, but no fix is upstream.
>>>>
>>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use>>
>>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>>> issue of missing synchronization in the PM core[1] is still unresolved
>>>> as far as I'm aware.
>>>
>>> OK, but the pvr driver can currently easily crash the kernel on boot if
>>> firmware is missing, so that should be fixed soon, right ?
>>
>> Well, drm-misc-next afaik means that the above mentioned fix would only
>> be merged in 7.1, which is ~4 months away, which is not really "soon"
>> I'd say. Or did I misjudge this?
> 
> The PM domain issue here crashes the kernel, so I think this would be
> material for drm-misc-fixes .

Yeah, sounds a lot like it.

>>> I added the regressions list onto CC, because this seems like a problem
>>> worth tracking.
>>
>> Noticed that and wondered what change caused the regression.
> 
> I think this one:
> 
> 330e76d31697 ("drm/imagination: Add power domain control")

Thx; FWIW, that was merged for v6.16-rc1.

>> Did not
>> find a answer in a quick search on lore[1]. Because if it's a
>> regression, we maybe should just revert the culprit for now according to
>> Linus:
>> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-
>> yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/
>>
>> [1] I guess this was the initial report from Geert?
>> https://lore.kernel.org/all/
>> CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/
> 
> It is.
> 
> I think there are other SoCs which depend on the power domain commit, so
> revert is not so clear cut anymore.

Well, it's a judgement call. 330e76d31697 was merged less then a year
ago, so I'd not be surprised at all if Linus would revert it in a case
like this. But it seems it doesn't revert clearly anymore, which
complicates things.

> But SoCs which have hierarchical
> power domains and which manage to probe this driver without having a
> firmware available for the GPU will simply end with crashed kernel,
> which is really not good.

Does the patch Matt mentioned fix the crash? His "this does not fix the
underlying issue [...]" (see quote earlier) makes it sound like the
crash or some other problem (theoretical or practical? regression or
not?) remains. If that's the case and no quick fix in sight I guess it
would be best if someone affected could post a revert and then we can
ask Linus if he wants to pick it up.

Ciao, Thorsten

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Geert Uytterhoeven]

Hi Thorsten,

On Sat, 14 Feb 2026 at 13:38, Thorsten Leemhuis
<regressions@leemhuis.info> wrote:
> On 2/13/26 23:52, Marek Vasut wrote:
> > On 2/12/26 4:56 PM, Thorsten Leemhuis wrote:
> >> On 2/12/26 15:38, Marek Vasut wrote:
> >>> On 2/12/26 10:00 AM, Matt Coster wrote:
> >>>> On 11/02/2026 19:17, Marek Vasut wrote:
> >>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
> >>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
> >>>>>> wrote:
> >>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
> >>>>>>>> Call the dev_pm_domain_attach_list() and
> >>>>>>>> dev_pm_domain_detach_list()
> >>>>>>>> helpers instead of open-coding multi PM Domain handling.
> >>>>>>>>
> >>>>>>>> This changes behavior slightly:
> >>>>>>>>      - The new handling is also applied in case of a single PM
> >>>>>>>> Domain,
> >>>>>>>>      - PM Domains are now referred to by index instead of by
> >>>>>>>> name, but
> >>>>>>>>        "make dtbs_check" enforces the actual naming and ordering
> >>>>>>>> anyway,
> >>>>>>>>      - There are no longer device links created between virtual
> >>>>>>>> domain
> >>>>>>>>        devices, only between virtual devices and the parent device.
> >>>>>>>
> >>>>>>> We still need this guarantee, both at start and end of day. In the
> >>>>>>> current implementation dev_pm_domain_attach_list() iterates
> >>>>>>> forwards,
> >>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that,
> >>>>>>> I'd
> >>>>>>> prefer not to rely on the implementation details when we can
> >>>>>>> declare the
> >>>>>>> dependencies explicitly.
> >>>>>>
> >>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
> >>>>>> r8a7795_areas[]),
> >>>>>> so they are always (un)powered in the correct order.  But that may
> >>>>>> not
> >>>>>> be the case in the integration on other SoCs.
> >>>>>>
> >>>>>>> We had/have a patch (attached) kicking around internally to use the
> >>>>>>> *_list() functions but keep the inter-domain links in place; it got
> >>>>>>> held
> >>>>>>> up by discussions as to whether we actually need those dependencies
> >>>>>>> for
> >>>>>>> the hardware to behave correctly. Your patch spurred me to run
> >>>>>>> around
> >>>>>>> the office and nag people a bit, and it seems we really do need to
> >>>>>>> care
> >>>>>>> about the ordering.
> >>>>>>
> >>>>>> OK.
> >>>>>>
> >>>>>>> Can you add the links back in for a V2 or I can properly send the
> >>>>>>> attached patch instead, I don't mind either way.
> >>>>>>
> >>>>>> Please move forward with your patch, you are the expert.
> >>>>>> I prefer not to be blamed for any breakage ;-)
> >>>>>
> >>>>> Has there been any progress on fixing this kernel crash ?
> >>>>>
> >>>>> There are already two proposed solutions, but no fix is upstream.
> >>>>
> >>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
> >>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use>>
> >>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
> >>>> issue of missing synchronization in the PM core[1] is still unresolved
> >>>> as far as I'm aware.
> >>>
> >>> OK, but the pvr driver can currently easily crash the kernel on boot if
> >>> firmware is missing, so that should be fixed soon, right ?
> >>
> >> Well, drm-misc-next afaik means that the above mentioned fix would only
> >> be merged in 7.1, which is ~4 months away, which is not really "soon"
> >> I'd say. Or did I misjudge this?
> >
> > The PM domain issue here crashes the kernel, so I think this would be
> > material for drm-misc-fixes .
>
> Yeah, sounds a lot like it.
>
> >>> I added the regressions list onto CC, because this seems like a problem
> >>> worth tracking.
> >>
> >> Noticed that and wondered what change caused the regression.
> >
> > I think this one:
> >
> > 330e76d31697 ("drm/imagination: Add power domain control")
>
> Thx; FWIW, that was merged for v6.16-rc1.
>
> >> Did not
> >> find a answer in a quick search on lore[1]. Because if it's a
> >> regression, we maybe should just revert the culprit for now according to
> >> Linus:
> >> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-
> >> yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/
> >>
> >> [1] I guess this was the initial report from Geert?
> >> https://lore.kernel.org/all/
> >> CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/
> >
> > It is.
> >
> > I think there are other SoCs which depend on the power domain commit, so
> > revert is not so clear cut anymore.
>
> Well, it's a judgement call. 330e76d31697 was merged less then a year
> ago, so I'd not be surprised at all if Linus would revert it in a case
> like this. But it seems it doesn't revert clearly anymore, which
> complicates things.
>
> > But SoCs which have hierarchical
> > power domains and which manage to probe this driver without having a
> > firmware available for the GPU will simply end with crashed kernel,
> > which is really not good.
>
> Does the patch Matt mentioned fix the crash? His "this does not fix the
> underlying issue [...]" (see quote earlier) makes it sound like the
> crash or some other problem (theoretical or practical? regression or
> not?) remains. If that's the case and no quick fix in sight I guess it
> would be best if someone affected could post a revert and then we can
> ask Linus if he wants to pick it up.

I don't think that patch would fix the crash.  The Adreno and Panfrost
GPU drivers do similar things (explicit multi-PM Domain handling),
so I am wondering if the issue can be triggered with them too (e.g. on
unbind).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Thorsten Leemhuis]

[top-posting to make sure everybody is aware that involving Linus for a
judgement call is on the table]

Geert: Many thx for the clarification. In that case:

Geert or Marek: could one of you two please submit a revert for
330e76d3169721 ("drm/imagination: Add power domain control") [v6.16-rc1]?

With a bit of luck the responsible drm maintainers will pick it up
quickly -- or it might lead to a different solution for the problem. If
neither happens within a few days, I'll point Linus to this thread and
the revert. Then he can make the judgement call if he wants to.

Ciao, Thorsten

On 2/16/26 10:00, Geert Uytterhoeven wrote:
> On Sat, 14 Feb 2026 at 13:38, Thorsten Leemhuis
> <regressions@leemhuis.info> wrote:
>> On 2/13/26 23:52, Marek Vasut wrote:
>>> On 2/12/26 4:56 PM, Thorsten Leemhuis wrote:
>>>> On 2/12/26 15:38, Marek Vasut wrote:
>>>>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>>>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>>>>> wrote:
>>>>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>>>>> Call the dev_pm_domain_attach_list() and
>>>>>>>>>> dev_pm_domain_detach_list()
>>>>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>>>>
>>>>>>>>>> This changes behavior slightly:
>>>>>>>>>>      - The new handling is also applied in case of a single PM
>>>>>>>>>> Domain,
>>>>>>>>>>      - PM Domains are now referred to by index instead of by
>>>>>>>>>> name, but
>>>>>>>>>>        "make dtbs_check" enforces the actual naming and ordering
>>>>>>>>>> anyway,
>>>>>>>>>>      - There are no longer device links created between virtual
>>>>>>>>>> domain
>>>>>>>>>>        devices, only between virtual devices and the parent device.
>>>>>>>>>
>>>>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>>>>> current implementation dev_pm_domain_attach_list() iterates
>>>>>>>>> forwards,
>>>>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that,
>>>>>>>>> I'd
>>>>>>>>> prefer not to rely on the implementation details when we can
>>>>>>>>> declare the
>>>>>>>>> dependencies explicitly.
>>>>>>>>
>>>>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>>>>> r8a7795_areas[]),
>>>>>>>> so they are always (un)powered in the correct order.  But that may
>>>>>>>> not
>>>>>>>> be the case in the integration on other SoCs.
>>>>>>>>
>>>>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>>>>> held
>>>>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>>>>> for
>>>>>>>>> the hardware to behave correctly. Your patch spurred me to run
>>>>>>>>> around
>>>>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>>>>> care
>>>>>>>>> about the ordering.
>>>>>>>>
>>>>>>>> OK.
>>>>>>>>
>>>>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>>>>> attached patch instead, I don't mind either way.
>>>>>>>>
>>>>>>>> Please move forward with your patch, you are the expert.
>>>>>>>> I prefer not to be blamed for any breakage ;-)
>>>>>>>
>>>>>>> Has there been any progress on fixing this kernel crash ?
>>>>>>>
>>>>>>> There are already two proposed solutions, but no fix is upstream.
>>>>>>
>>>>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>>>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use>>
>>>>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>>>>> issue of missing synchronization in the PM core[1] is still unresolved
>>>>>> as far as I'm aware.
>>>>>
>>>>> OK, but the pvr driver can currently easily crash the kernel on boot if
>>>>> firmware is missing, so that should be fixed soon, right ?
>>>>
>>>> Well, drm-misc-next afaik means that the above mentioned fix would only
>>>> be merged in 7.1, which is ~4 months away, which is not really "soon"
>>>> I'd say. Or did I misjudge this?
>>>
>>> The PM domain issue here crashes the kernel, so I think this would be
>>> material for drm-misc-fixes .
>>
>> Yeah, sounds a lot like it.
>>
>>>>> I added the regressions list onto CC, because this seems like a problem
>>>>> worth tracking.
>>>>
>>>> Noticed that and wondered what change caused the regression.
>>>
>>> I think this one:
>>>
>>> 330e76d31697 ("drm/imagination: Add power domain control")
>>
>> Thx; FWIW, that was merged for v6.16-rc1.
>>
>>>> Did not
>>>> find a answer in a quick search on lore[1]. Because if it's a
>>>> regression, we maybe should just revert the culprit for now according to
>>>> Linus:
>>>> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-
>>>> yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/
>>>>
>>>> [1] I guess this was the initial report from Geert?
>>>> https://lore.kernel.org/all/
>>>> CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/
>>>
>>> It is.
>>>
>>> I think there are other SoCs which depend on the power domain commit, so
>>> revert is not so clear cut anymore.
>>
>> Well, it's a judgement call. 330e76d31697 was merged less then a year
>> ago, so I'd not be surprised at all if Linus would revert it in a case
>> like this. But it seems it doesn't revert clearly anymore, which
>> complicates things.
>>
>>> But SoCs which have hierarchical
>>> power domains and which manage to probe this driver without having a
>>> firmware available for the GPU will simply end with crashed kernel,
>>> which is really not good.
>>
>> Does the patch Matt mentioned fix the crash? His "this does not fix the
>> underlying issue [...]" (see quote earlier) makes it sound like the
>> crash or some other problem (theoretical or practical? regression or
>> not?) remains. If that's the case and no quick fix in sight I guess it
>> would be best if someone affected could post a revert and then we can
>> ask Linus if he wants to pick it up.
> 
> I don't think that patch would fix the crash.  The Adreno and Panfrost
> GPU drivers do similar things (explicit multi-PM Domain handling),
> so I am wondering if the issue can be triggered with them too (e.g. on
> unbind).

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 10557 bytes --]

On 16/02/2026 10:11, Thorsten Leemhuis wrote:
> [top-posting to make sure everybody is aware that involving Linus for a
> judgement call is on the table]
> 
> Geert: Many thx for the clarification. In that case:
> 
> Geert or Marek: could one of you two please submit a revert for
> 330e76d3169721 ("drm/imagination: Add power domain control") [v6.16-rc1]?
> 
> With a bit of luck the responsible drm maintainers will pick it up
> quickly -- or it might lead to a different solution for the problem. If
> neither happens within a few days, I'll point Linus to this thread and
> the revert. Then he can make the judgement call if he wants to.

Hi all,

Apologies for not replying to this sooner, but please see some thoughts
below on various parts of this discussion.

We're currently trying to force this issue to reproduce on hardware we
have on hand; we'd like to see it fixed properly as much as anyone.

> 
> Ciao, Thorsten
> 
> On 2/16/26 10:00, Geert Uytterhoeven wrote:
>> On Sat, 14 Feb 2026 at 13:38, Thorsten Leemhuis
>> <regressions@leemhuis.info> wrote:
>>> On 2/13/26 23:52, Marek Vasut wrote:
>>>> On 2/12/26 4:56 PM, Thorsten Leemhuis wrote:
>>>>> On 2/12/26 15:38, Marek Vasut wrote:
>>>>>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>>>>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>>>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>>>>>> wrote:
>>>>>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>>>>>> Call the dev_pm_domain_attach_list() and
>>>>>>>>>>> dev_pm_domain_detach_list()
>>>>>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>>>>>
>>>>>>>>>>> This changes behavior slightly:
>>>>>>>>>>>      - The new handling is also applied in case of a single PM
>>>>>>>>>>> Domain,
>>>>>>>>>>>      - PM Domains are now referred to by index instead of by
>>>>>>>>>>> name, but
>>>>>>>>>>>        "make dtbs_check" enforces the actual naming and ordering
>>>>>>>>>>> anyway,
>>>>>>>>>>>      - There are no longer device links created between virtual
>>>>>>>>>>> domain
>>>>>>>>>>>        devices, only between virtual devices and the parent device.
>>>>>>>>>>
>>>>>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>>>>>> current implementation dev_pm_domain_attach_list() iterates
>>>>>>>>>> forwards,
>>>>>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that,
>>>>>>>>>> I'd
>>>>>>>>>> prefer not to rely on the implementation details when we can
>>>>>>>>>> declare the
>>>>>>>>>> dependencies explicitly.
>>>>>>>>>
>>>>>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>>>>>> r8a7795_areas[]),
>>>>>>>>> so they are always (un)powered in the correct order.  But that may
>>>>>>>>> not
>>>>>>>>> be the case in the integration on other SoCs.
>>>>>>>>>
>>>>>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>>>>>> held
>>>>>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>>>>>> for
>>>>>>>>>> the hardware to behave correctly. Your patch spurred me to run
>>>>>>>>>> around
>>>>>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>>>>>> care
>>>>>>>>>> about the ordering.
>>>>>>>>>
>>>>>>>>> OK.
>>>>>>>>>
>>>>>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>>>>>> attached patch instead, I don't mind either way.
>>>>>>>>>
>>>>>>>>> Please move forward with your patch, you are the expert.
>>>>>>>>> I prefer not to be blamed for any breakage ;-)
>>>>>>>>
>>>>>>>> Has there been any progress on fixing this kernel crash ?
>>>>>>>>
>>>>>>>> There are already two proposed solutions, but no fix is upstream.
>>>>>>>
>>>>>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>>>>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use>>
>>>>>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>>>>>> issue of missing synchronization in the PM core[1] is still unresolved
>>>>>>> as far as I'm aware.
>>>>>>
>>>>>> OK, but the pvr driver can currently easily crash the kernel on boot if
>>>>>> firmware is missing, so that should be fixed soon, right ?
>>>>>
>>>>> Well, drm-misc-next afaik means that the above mentioned fix would only
>>>>> be merged in 7.1, which is ~4 months away, which is not really "soon"
>>>>> I'd say. Or did I misjudge this?

The above isn't really a "fix" per se, it's just an enhancement. The
underlying crash can still happen. We could still pick it into
drm-misc-fixes and have it in the next -rc plus backported to stable,
but I'm not sure I see the value.

>>>>
>>>> The PM domain issue here crashes the kernel, so I think this would be
>>>> material for drm-misc-fixes .
>>>
>>> Yeah, sounds a lot like it.
>>>
>>>>>> I added the regressions list onto CC, because this seems like a problem
>>>>>> worth tracking.
>>>>>
>>>>> Noticed that and wondered what change caused the regression.

From our side at least, I don't believe this is a regression at all. We
haven't been able to reproduce this issue on any of the platforms we
have available (although we did stumble on a somewhat related bugfix
while trying).

>>>>
>>>> I think this one:
>>>>
>>>> 330e76d31697 ("drm/imagination: Add power domain control")

This commit added support for multiple power domains on Imagination GPUs
in essentially the same manner as other drivers (as mentioned by Geert
below). Nothing in there is specific to the Renesas platforms where this
bug can be produces and it is required to support other fully-functional
platforms such as TI AM68.

>>>
>>> Thx; FWIW, that was merged for v6.16-rc1.
>>>
>>>>> Did not
>>>>> find a answer in a quick search on lore[1]. Because if it's a
>>>>> regression, we maybe should just revert the culprit for now according to
>>>>> Linus:
>>>>> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66- 
>>>>> yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/
>>>>>
>>>>> [1] I guess this was the initial report from Geert?
>>>>> https://lore.kernel.org/all/ 
>>>>> CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/
>>>>
>>>> It is.
>>>>
>>>> I think there are other SoCs which depend on the power domain commit, so
>>>> revert is not so clear cut anymore.
>>>
>>> Well, it's a judgement call. 330e76d31697 was merged less then a year
>>> ago, so I'd not be surprised at all if Linus would revert it in a case
>>> like this. But it seems it doesn't revert clearly anymore, which
>>> complicates things.

Reverting this change feels backwards, we're saying that Imagination's
support for multiple power domains is broken because one platform has
issues when we attempt to control its power domains. I fully agree that
we should be working towards resolving this issue, but I don't agree
that ripping out this handling (that, again, is essentially the same as
handling in other GPU drivers) is a reasonable solution (even in the
short term).

As it currently stands, we're only declaring two GPUs as "fully
supported": AXE-1-16M and BXS-4-64 as made explicit in commit
1c21f240fbc1e ("drm/imagination: Warn or error on unsupported
hardware"). In order to reach the stage where that check occurs, the GPU
must be powered on so registers containing the GPU ID can be read.
However, the original error report happens long after this and would now
require the exp_hw_support module parameter to be set for this point to
be reached (as the driver would not attempt to initialise an
"unsupported" device without this explicit opt-in).

It's not currently clear to me whether the crash can be reproduced on
the affected Renesas platform without this opt-in; it's feasable that
this is the case but we know it occurs during an error unwind after
device firmware fails to initialise and the exp_hw_support check happens
much earlier than this.

Either way, we should consider adding additional "supported" checks
based on compatibile strings earlier during initialization to prevent
fundamental issues like this one from even being possible on unsupported
and/or unvalidated hardware.

All that being said, ripping out power domain handling will completely
break one of the only two platforms we currently fully support: BXS-4-64
which also has two power domains and is present in several TI SoCs (and
their DTs).

>>>
>>>> But SoCs which have hierarchical
>>>> power domains and which manage to probe this driver without having a
>>>> firmware available for the GPU will simply end with crashed kernel,
>>>> which is really not good.
>>>
>>> Does the patch Matt mentioned fix the crash? His "this does not fix the
>>> underlying issue [...]" (see quote earlier) makes it sound like the
>>> crash or some other problem (theoretical or practical? regression or
>>> not?) remains. If that's the case and no quick fix in sight I guess it
>>> would be best if someone affected could post a revert and then we can
>>> ask Linus if he wants to pick it up.

The patch I posted (and applied) is somewhat orthogonal to the issue at
hand. Geert originally suggested using the _attach_list() helper as a
way to mitigate the issue, and we agree that it's a reasonable cleanup
effort to make use of it, but I believe the "fix" in the originally
proposed patch came from removing the device links that are required on
our platform to ensure the GPU power domains come up (and down) in the
correct sequence.

>>
>> I don't think that patch would fix the crash.  The Adreno and Panfrost
>> GPU drivers do similar things (explicit multi-PM Domain handling),
>> so I am wondering if the issue can be triggered with them too (e.g. on
>> unbind).

My current understanding of the situation is that the fix proposed by
Marek in the Reneasas driver[2] works, but is not suitable since
pm_runtime_barrier() should be inserted by the caller, not the power
driver. But it seems that's not always possible (particularly when using
devm), so I don't really understand where we go from here. I still don't
see anything we're doing substantially differently (before or after the
commit I mentioned above) from anybody else.

Cheers,
Matt

[2]: https://lore.kernel.org/r/0e9f963b-00e0-43d1-b567-cb10b8f66df1@mailbox.org/

-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Thorsten Leemhuis]

On 2/16/26 11:58, Matt Coster wrote:
> On 16/02/2026 10:11, Thorsten Leemhuis wrote:
> 
> We're currently trying to force this issue to reproduce on hardware we
> have on hand; we'd like to see it fixed properly as much as anyone.

Yeah, no worries, I never doubted that. But getting things properly fixed
can mean "revert, fix, reapply" when it comes to regressions in Linux --
which is something that should not be seen as something bad, as Linus said
himself (see below)!

> From our side at least, I don't believe this is a regression at all.
In the end what matters is: some change afaics caused systems to not work
anymore that used to be working -- that makes it a regression my the Linux
kernels standards. And those by the same standards must be fixed, ideally
quickly. Find a few quotes on that from Linus below that explains this
better. 

Ciao, Thorsten
---


On how quickly regressions should be fixed
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/>`_::

    But a user complaining should basically result in an immediate fix -
    possibly a "revert and rethink".

  With a later clarification on `2026-01-28 <https://lore.kernel.org/all/CAHk-%3Dwi86AosXs66-yi54%2BmpQjPu0upxB8ZAfG%2BLsMyJmcuMSA@mail.gmail.com/>`_::

    It's also worth noting that "immediate" obviously doesn't mean "right
    this *second* when the problem has been reported".

    But if it's a regression with a known commit that caused it, I think
    the rule of thumb should generally be "within a week", preferably
    before the next rc.

* From `2023-04-21 <https://lore.kernel.org/all/CAHk-=wgD98pmSK3ZyHk_d9kZ2bhgN6DuNZMAJaV0WTtbkf=RDw@mail.gmail.com/>`_::

    Known-broken commits either
     (a) get a timely fix that doesn't have other questions
    or
     (b) get reverted

* From `2021-09-20(2) <https://lore.kernel.org/all/CAHk-=wgOvmtRw1TNbMC1rn5YqyTKyn0hz+sc4k0DGNn++u9aYw@mail.gmail.com/>`_::

    [...] review shouldn't hold up reported regressions of existing code. That's
    just basic _testing_ - either the fix should be applied, or - if the fix is
    too invasive or too ugly - the problematic source of the regression should
    be reverted.

    Review should be about new code, it shouldn't be holding up "there's a
    bug report, here's the obvious fix".

* From `2023-05-08 <https://lore.kernel.org/all/CAHk-=wgzU8_dGn0Yg+DyX7ammTkDUCyEJ4C=NvnHRhxKWC7Wpw@mail.gmail.com/>`_::

    If something doesn't even build, it should damn well be fixed ASAP.


On how fixing regressions with reverts can help prevent maintainer burnout
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* From `2026-01-28 <https://lore.kernel.org/all/CAHk-%3Dwi86AosXs66-yi54%2BmpQjPu0upxB8ZAfG%2BLsMyJmcuMSA@mail.gmail.com/>`_::

    > So how can I/we make "immediate fixes" happen more often without
    > contributing to maintainer burnout?

    [...] the "revert and rethink" model [...] often a good idea in general
    unless there's just an obvious fix for an obvious bug [...]

    Exactly so that maintainers don't get stressed out over having a pending
    problem report that people keep pestering them about.

    I think people are sometimes a bit too bought into whatever changes
    they made, and reverting is seen as "too drastic", but I think it's
    often the quick and easy solution for when there isn't some obvious
    response to a regression report.


On why the "no regressions" rule exists
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/>`_::

    But the basic rule is: be so good about backwards compatibility that
    users never have to worry about upgrading. They should absolutely feel
    confident that any kernel-reported problem will either be solved, or
    have an easy solution that is appropriate for *them* (ie a
    non-technical user shouldn't be expected to be able to do a lot).

    Because the last thing we want is people holding back from trying new
    kernels.

* From `2024-05-28 <https://lore.kernel.org/all/CAHk-=wgtb7y-bEh7tPDvDWru7ZKQ8-KMjZ53Tsk37zsPPdwXbA@mail.gmail.com/>`_::

    I introduced that "no regressions" rule something like two decades
    ago, because people need to be able to update their kernel without
    fear of something they relied on suddenly stopping to work.

* From `2018-08-03 <https://lore.kernel.org/all/CA+55aFwWZX=CXmWDTkDGb36kf12XmTehmQjbiMPCqCRG2hi9kw@mail.gmail.com/>`_::

    The whole point of "we do not regress" is so that people can upgrade
    the kernel and never have to worry about it.

    [...]

    Because the only thing that matters IS THE USER.

* From `2017-10-26(1) <https://lore.kernel.org/lkml/CA+55aFxW7NMAMvYhkvz1UPbUTUJewRt6Yb51QAx5RtrWOwjebg@mail.gmail.com/>`_::

    If the kernel used to work for you, the rule is that it continues to work
    for you.

    [...]

    People should basically always feel like they can update their kernel
    and simply not have to worry about it.

    I refuse to introduce "you can only update the kernel if you also
    update that other program" kind of limitations. If the kernel used to
    work for you, the rule is that it continues to work for you.


On exceptions to the "no regressions" rule
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/>`_::

    There are _very_ few exceptions to that rule, the main one being "the
    problem was a fundamental huge and gaping security issue and we *had* to
    make that change, and we couldn't even make your limited use-case just
    continue to work".

    The other exception is "the problem was reported years after it was
    introduced, and now most people rely on the new behavior".

    [...]

    Now, if it's one or two users and you can just get them to recompile,
    that's one thing. Niche hardware and odd use-cases can sometimes be
    solved that way, and regressions can sometimes be fixed by handholding
    every single reporter if the reporter is willing and able to change
    his or her workflow.

* From `2023-04-20 <https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/>`_::

    And yes, I do consider "regression in an earlier release" to be a
    regression that needs fixing.

    There's obviously a time limit: if that "regression in an earlier
    release" was a year or more ago, and just took forever for people to
    notice, and it had semantic changes that now mean that fixing the
    regression could cause a _new_ regression, then that can cause me to
    go "Oh, now the new semantics are what we have to live with".

* From `2021-09-20(3) <https://lore.kernel.org/all/CAHk-=wi7DB2SJ-wngVvsJ7Ak2cM556Q8437sOXo4EJt2BWPdEg@mail.gmail.com/>`_::

    Yes, we have situations where even regressions don't matter - like
    major security issues that simply cannot be fixed other ways, because
    the regression _was_ the security hole.

* From `2017-10-26(2) <https://lore.kernel.org/lkml/CA+55aFxW7NMAMvYhkvz1UPbUTUJewRt6Yb51QAx5RtrWOwjebg@mail.gmail.com/>`_::

    There have been exceptions, but they are few and far between, and they
    generally have some major and fundamental reasons for having happened,
    that were basically entirely unavoidable, and people _tried_hard_ to
    avoid them. Maybe we can't practically support the hardware any more
    after it is decades old and nobody uses it with modern kernels any
    more. Maybe there's a serious security issue with how we did things,
    and people actually depended on that fundamentally broken model. Maybe
    there was some fundamental other breakage that just _had_ to have a
    flag day for very core and fundamental reasons.


On accepting when a regression occurred
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/>`_::

    But starting to argue about users reporting breaking changes is
    basically the final line for me. I have a couple of people that I have
    in my spam block-list and refuse to have anything to do with, and they
    have generally been about exactly that.

    Note how it's not about making mistakes and _causing_ the regression.
    That's normal. That's development. But then arguing about it is a
    no-no.

* From `2024-06-23 <https://lore.kernel.org/all/CAHk-=wi_KMO_rJ6OCr8mAWBRg-irziM=T9wxGC+J1VVoQb39gw@mail.gmail.com/>`_::

    We don't introduce regressions and then blame others.

    There's a very clear rule in kernel development: things that break
    other things ARE NOT FIXES.

    EVER.

    They get reverted, or the thing they broke gets fixed.

* From `2021-06-05 <https://lore.kernel.org/all/CAHk-=wiUVqHN76YUwhkjZzwTdjMMJf_zN4+u7vEJjmEGh3recw@mail.gmail.com/>`_::

    THERE ARE NO VALID ARGUMENTS FOR REGRESSIONS.

    Honestly, security people need to understand that "not working" is not
    a success case of security. It's a failure case.

    Yes, "not working" may be secure. But security in that case is *pointless*.

* From `2017-10-26(5) <https://lore.kernel.org/lkml/CA+55aFwiiQYJ+YoLKCXjN_beDVfu38mg=Ggg5LFOcqHE8Qi7Zw@mail.gmail.com/>`_::

    [...] when regressions *do* occur, we admit to them and fix them, instead of
    blaming user space.

    The fact that you have apparently been denying the regression now for
    three weeks means that I will revert, and I will stop pulling apparmor
    requests until the people involved understand how kernel development
    is done.


On back-and-forth
~~~~~~~~~~~~~~~~~

* From `2024-05-28 <https://lore.kernel.org/all/CAHk-=wgtb7y-bEh7tPDvDWru7ZKQ8-KMjZ53Tsk37zsPPdwXbA@mail.gmail.com/>`_::

    The "no regressions" rule is that we do not introduce NEW bugs.

    It *literally* came about because we had an endless dance of "fix two
    bugs, introduce one new one", and that then resulted in a system that
    you cannot TRUST.

* From `2021-09-20(1) <https://lore.kernel.org/all/CAHk-=wi7DB2SJ-wngVvsJ7Ak2cM556Q8437sOXo4EJt2BWPdEg@mail.gmail.com/>`_::


    And the thing that makes regressions special is that back when I
    wasn't so strict about these things, we'd end up in endless "seesaw
    situations" where somebody would fix something, it would break
    something else, then that something else would break, and it would
    never actually converge on anything reliable at all.

* From `2015-08-13 <https://lore.kernel.org/all/CA+55aFxk8-BsiKwr_S-c+4G6wihKPQVMLE34H9wOZpeua6W9+Q@mail.gmail.com/>`_::

    The strict policy of no regressions actually originally started mainly wrt
    suspend/resume issues, where the "fix one machine, break another" kind of
    back-and-forth caused endless problems, and meant that we didn't actually
    necessarily make any forward progress, just moving a problem around.


On regressions caused by bugfixes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* From `2018-08-03 <https://lore.kernel.org/all/CA+55aFwWZX=CXmWDTkDGb36kf12XmTehmQjbiMPCqCRG2hi9kw@mail.gmail.com/>`_::

    > Kernel had a bug which has been fixed

    That is *ENTIRELY* immaterial.

    Guys, whether something was buggy or not DOES NOT MATTER.

    [...]

    It's basically saying "I took something that worked, and I broke it,
    but now it's better". Do you not see how f*cking insane that statement
    is?

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 14329 bytes --]

On 16/02/2026 11:38, Thorsten Leemhuis wrote:
> On 2/16/26 11:58, Matt Coster wrote:
>> On 16/02/2026 10:11, Thorsten Leemhuis wrote:
>>
>> We're currently trying to force this issue to reproduce on hardware we
>> have on hand; we'd like to see it fixed properly as much as anyone.
> 
> Yeah, no worries, I never doubted that. But getting things properly fixed
> can mean "revert, fix, reapply" when it comes to regressions in Linux --
> which is something that should not be seen as something bad, as Linus said
> himself (see below)!
> 
>> From our side at least, I don't believe this is a regression at all.
> In the end what matters is: some change afaics caused systems to not work
> anymore that used to be working -- that makes it a regression my the Linux
> kernels standards. And those by the same standards must be fixed, ideally
> quickly. Find a few quotes on that from Linus below that explains this
> better. 

I feel like I should reiterate that the commit we're talking about
reverting is fundamental to support for one of the only two platforms
currently supported. And that the changes to add "support" (just
bindings and DT) for the affected Renesas platforms came several months
*after* this.

The "regression" here is that we allowed DTS changes to land for
unsupported platforms in the interest of allowing further development to
happen incrementally upstream. There has been no further progress on
that front beyond the DTS patches, however. We have never declared that
these platforms should be functional and error-free, and have taken
measures to ensure this is clear to users[1].

There are currently two platforms on which this has been reproduced:

 - Renesas Gray Hawk Single (R-Car V4M) -- this was the original report
   from Geert, and it should be noted that there are no bindings or DTS
   support for the GPU in this platform in tree at this time.
 - Renesas Salvator-X (R-Car M3-W) -- this was Geert's follow-up
   reproduction case, and the upstream bindings and DTS do contain the
   GPU, but it required adding delays to PM core code to trigger the
   race condition(?) that causes the crash.

As far as we know, there are no other situations where this crash
occurs.

Would you consider a suitable "revert" to be fully gating support for
these platforms (or even the entire group of Renesas platforms added in
this "experimental" manner just to be safe) behind the exp_hw_support
paramater until they can be properly tested? Specifically, I'm talking
about masking them off at the of_match level so that no hardware
interaction is even attempted without explicit user opt-in to
experimental hardware.

Cheers,
Matt

[1]: commit 1c21f240fbc1 ("drm/imagination: Warn or error on unsupported hardware")

> 
> Ciao, Thorsten
> ---
> 
> 
> On how quickly regressions should be fixed
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> * From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/ >`_::
> 
>     But a user complaining should basically result in an immediate fix -
>     possibly a "revert and rethink".
> 
>   With a later clarification on `2026-01-28 <https://lore.kernel.org/all/CAHk-%3Dwi86AosXs66-yi54%2BmpQjPu0upxB8ZAfG%2BLsMyJmcuMSA@mail.gmail.com/ >`_::
> 
>     It's also worth noting that "immediate" obviously doesn't mean "right
>     this *second* when the problem has been reported".
> 
>     But if it's a regression with a known commit that caused it, I think
>     the rule of thumb should generally be "within a week", preferably
>     before the next rc.
> 
> * From `2023-04-21 <https://lore.kernel.org/all/CAHk-=wgD98pmSK3ZyHk_d9kZ2bhgN6DuNZMAJaV0WTtbkf=RDw@mail.gmail.com/ >`_::
> 
>     Known-broken commits either
>      (a) get a timely fix that doesn't have other questions
>     or
>      (b) get reverted
> 
> * From `2021-09-20(2) <https://lore.kernel.org/all/CAHk-=wgOvmtRw1TNbMC1rn5YqyTKyn0hz+sc4k0DGNn++u9aYw@mail.gmail.com/ >`_::
> 
>     [...] review shouldn't hold up reported regressions of existing code. That's
>     just basic _testing_ - either the fix should be applied, or - if the fix is
>     too invasive or too ugly - the problematic source of the regression should
>     be reverted.
> 
>     Review should be about new code, it shouldn't be holding up "there's a
>     bug report, here's the obvious fix".
> 
> * From `2023-05-08 <https://lore.kernel.org/all/CAHk-=wgzU8_dGn0Yg+DyX7ammTkDUCyEJ4C=NvnHRhxKWC7Wpw@mail.gmail.com/ >`_::
> 
>     If something doesn't even build, it should damn well be fixed ASAP.
> 
> 
> On how fixing regressions with reverts can help prevent maintainer burnout
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> * From `2026-01-28 <https://lore.kernel.org/all/CAHk-%3Dwi86AosXs66-yi54%2BmpQjPu0upxB8ZAfG%2BLsMyJmcuMSA@mail.gmail.com/ >`_::
> 
>     > So how can I/we make "immediate fixes" happen more often without
>     > contributing to maintainer burnout?
> 
>     [...] the "revert and rethink" model [...] often a good idea in general
>     unless there's just an obvious fix for an obvious bug [...]
> 
>     Exactly so that maintainers don't get stressed out over having a pending
>     problem report that people keep pestering them about.
> 
>     I think people are sometimes a bit too bought into whatever changes
>     they made, and reverting is seen as "too drastic", but I think it's
>     often the quick and easy solution for when there isn't some obvious
>     response to a regression report.
> 
> 
> On why the "no regressions" rule exists
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> * From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/ >`_::
> 
>     But the basic rule is: be so good about backwards compatibility that
>     users never have to worry about upgrading. They should absolutely feel
>     confident that any kernel-reported problem will either be solved, or
>     have an easy solution that is appropriate for *them* (ie a
>     non-technical user shouldn't be expected to be able to do a lot).
> 
>     Because the last thing we want is people holding back from trying new
>     kernels.
> 
> * From `2024-05-28 <https://lore.kernel.org/all/CAHk-=wgtb7y-bEh7tPDvDWru7ZKQ8-KMjZ53Tsk37zsPPdwXbA@mail.gmail.com/ >`_::
> 
>     I introduced that "no regressions" rule something like two decades
>     ago, because people need to be able to update their kernel without
>     fear of something they relied on suddenly stopping to work.
> 
> * From `2018-08-03 <https://lore.kernel.org/all/CA+55aFwWZX=CXmWDTkDGb36kf12XmTehmQjbiMPCqCRG2hi9kw@mail.gmail.com/ >`_::
> 
>     The whole point of "we do not regress" is so that people can upgrade
>     the kernel and never have to worry about it.
> 
>     [...]
> 
>     Because the only thing that matters IS THE USER.
> 
> * From `2017-10-26(1) <https://lore.kernel.org/lkml/CA+55aFxW7NMAMvYhkvz1UPbUTUJewRt6Yb51QAx5RtrWOwjebg@mail.gmail.com/ >`_::
> 
>     If the kernel used to work for you, the rule is that it continues to work
>     for you.
> 
>     [...]
> 
>     People should basically always feel like they can update their kernel
>     and simply not have to worry about it.
> 
>     I refuse to introduce "you can only update the kernel if you also
>     update that other program" kind of limitations. If the kernel used to
>     work for you, the rule is that it continues to work for you.
> 
> 
> On exceptions to the "no regressions" rule
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> * From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/ >`_::
> 
>     There are _very_ few exceptions to that rule, the main one being "the
>     problem was a fundamental huge and gaping security issue and we *had* to
>     make that change, and we couldn't even make your limited use-case just
>     continue to work".
> 
>     The other exception is "the problem was reported years after it was
>     introduced, and now most people rely on the new behavior".
> 
>     [...]
> 
>     Now, if it's one or two users and you can just get them to recompile,
>     that's one thing. Niche hardware and odd use-cases can sometimes be
>     solved that way, and regressions can sometimes be fixed by handholding
>     every single reporter if the reporter is willing and able to change
>     his or her workflow.
> 
> * From `2023-04-20 <https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/ >`_::
> 
>     And yes, I do consider "regression in an earlier release" to be a
>     regression that needs fixing.
> 
>     There's obviously a time limit: if that "regression in an earlier
>     release" was a year or more ago, and just took forever for people to
>     notice, and it had semantic changes that now mean that fixing the
>     regression could cause a _new_ regression, then that can cause me to
>     go "Oh, now the new semantics are what we have to live with".
> 
> * From `2021-09-20(3) <https://lore.kernel.org/all/CAHk-=wi7DB2SJ-wngVvsJ7Ak2cM556Q8437sOXo4EJt2BWPdEg@mail.gmail.com/ >`_::
> 
>     Yes, we have situations where even regressions don't matter - like
>     major security issues that simply cannot be fixed other ways, because
>     the regression _was_ the security hole.
> 
> * From `2017-10-26(2) <https://lore.kernel.org/lkml/CA+55aFxW7NMAMvYhkvz1UPbUTUJewRt6Yb51QAx5RtrWOwjebg@mail.gmail.com/ >`_::
> 
>     There have been exceptions, but they are few and far between, and they
>     generally have some major and fundamental reasons for having happened,
>     that were basically entirely unavoidable, and people _tried_hard_ to
>     avoid them. Maybe we can't practically support the hardware any more
>     after it is decades old and nobody uses it with modern kernels any
>     more. Maybe there's a serious security issue with how we did things,
>     and people actually depended on that fundamentally broken model. Maybe
>     there was some fundamental other breakage that just _had_ to have a
>     flag day for very core and fundamental reasons.
> 
> 
> On accepting when a regression occurred
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> * From `2026-01-22 <https://lore.kernel.org/all/CAHk-=wheQNiW_WtHGO7bKkT7Uib-p+ai2JP9M+z+FYcZ6CAxYA@mail.gmail.com/ >`_::
> 
>     But starting to argue about users reporting breaking changes is
>     basically the final line for me. I have a couple of people that I have
>     in my spam block-list and refuse to have anything to do with, and they
>     have generally been about exactly that.
> 
>     Note how it's not about making mistakes and _causing_ the regression.
>     That's normal. That's development. But then arguing about it is a
>     no-no.
> 
> * From `2024-06-23 <https://lore.kernel.org/all/CAHk-=wi_KMO_rJ6OCr8mAWBRg-irziM=T9wxGC+J1VVoQb39gw@mail.gmail.com/ >`_::
> 
>     We don't introduce regressions and then blame others.
> 
>     There's a very clear rule in kernel development: things that break
>     other things ARE NOT FIXES.
> 
>     EVER.
> 
>     They get reverted, or the thing they broke gets fixed.
> 
> * From `2021-06-05 <https://lore.kernel.org/all/CAHk-=wiUVqHN76YUwhkjZzwTdjMMJf_zN4+u7vEJjmEGh3recw@mail.gmail.com/ >`_::
> 
>     THERE ARE NO VALID ARGUMENTS FOR REGRESSIONS.
> 
>     Honestly, security people need to understand that "not working" is not
>     a success case of security. It's a failure case.
> 
>     Yes, "not working" may be secure. But security in that case is *pointless*.
> 
> * From `2017-10-26(5) <https://lore.kernel.org/lkml/CA+55aFwiiQYJ+YoLKCXjN_beDVfu38mg=Ggg5LFOcqHE8Qi7Zw@mail.gmail.com/ >`_::
> 
>     [...] when regressions *do* occur, we admit to them and fix them, instead of
>     blaming user space.
> 
>     The fact that you have apparently been denying the regression now for
>     three weeks means that I will revert, and I will stop pulling apparmor
>     requests until the people involved understand how kernel development
>     is done.
> 
> 
> On back-and-forth
> ~~~~~~~~~~~~~~~~~
> 
> * From `2024-05-28 <https://lore.kernel.org/all/CAHk-=wgtb7y-bEh7tPDvDWru7ZKQ8-KMjZ53Tsk37zsPPdwXbA@mail.gmail.com/ >`_::
> 
>     The "no regressions" rule is that we do not introduce NEW bugs.
> 
>     It *literally* came about because we had an endless dance of "fix two
>     bugs, introduce one new one", and that then resulted in a system that
>     you cannot TRUST.
> 
> * From `2021-09-20(1) <https://lore.kernel.org/all/CAHk-=wi7DB2SJ-wngVvsJ7Ak2cM556Q8437sOXo4EJt2BWPdEg@mail.gmail.com/ >`_::
> 
> 
>     And the thing that makes regressions special is that back when I
>     wasn't so strict about these things, we'd end up in endless "seesaw
>     situations" where somebody would fix something, it would break
>     something else, then that something else would break, and it would
>     never actually converge on anything reliable at all.
> 
> * From `2015-08-13 <https://lore.kernel.org/all/CA+55aFxk8-BsiKwr_S-c+4G6wihKPQVMLE34H9wOZpeua6W9+Q@mail.gmail.com/ >`_::
> 
>     The strict policy of no regressions actually originally started mainly wrt
>     suspend/resume issues, where the "fix one machine, break another" kind of
>     back-and-forth caused endless problems, and meant that we didn't actually
>     necessarily make any forward progress, just moving a problem around.
> 
> 
> On regressions caused by bugfixes
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> * From `2018-08-03 <https://lore.kernel.org/all/CA+55aFwWZX=CXmWDTkDGb36kf12XmTehmQjbiMPCqCRG2hi9kw@mail.gmail.com/ >`_::
> 
>     > Kernel had a bug which has been fixed
> 
>     That is *ENTIRELY* immaterial.
> 
>     Guys, whether something was buggy or not DOES NOT MATTER.
> 
>     [...]
> 
>     It's basically saying "I took something that worked, and I broke it,
>     but now it's better". Do you not see how f*cking insane that statement
>     is?


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Thorsten Leemhuis]

On 2/16/26 14:37, Matt Coster wrote:
> On 16/02/2026 11:38, Thorsten Leemhuis wrote:
>> On 2/16/26 11:58, Matt Coster wrote:
>>> On 16/02/2026 10:11, Thorsten Leemhuis wrote:
>>>
>>> We're currently trying to force this issue to reproduce on hardware we
>>> have on hand; we'd like to see it fixed properly as much as anyone.
>>
>> Yeah, no worries, I never doubted that. But getting things properly fixed
>> can mean "revert, fix, reapply" when it comes to regressions in Linux --
>> which is something that should not be seen as something bad, as Linus said
>> himself (see below)!
>>
>>> From our side at least, I don't believe this is a regression at all.
>> In the end what matters is: some change afaics caused systems to not work
>> anymore that used to be working -- that makes it a regression my the Linux
>> kernels standards. And those by the same standards must be fixed, ideally
>> quickly. Find a few quotes on that from Linus below that explains this
>> better. 
> I feel like I should reiterate that the commit we're talking about
> reverting is fundamental to support for one of the only two platforms
> currently supported.

That might or might not be relevant, see the "back and forth" section
from the Linus quotes.

> And that the changes to add "support" (just
> bindings and DT) for the affected Renesas platforms came several months
> *after* this.

Ohh? That might change things then. I relied on the info from Geert and
Marek – and would be glad if you guys could sort this out, as you are
the experts here (and I already got myself way deeper involved then I
wanted to).

> [...]

Ciao, Thorsten

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Marek Vasut]

On 2/16/26 6:28 PM, Thorsten Leemhuis wrote:
> On 2/16/26 14:37, Matt Coster wrote:
>> On 16/02/2026 11:38, Thorsten Leemhuis wrote:
>>> On 2/16/26 11:58, Matt Coster wrote:
>>>> On 16/02/2026 10:11, Thorsten Leemhuis wrote:
>>>>
>>>> We're currently trying to force this issue to reproduce on hardware we
>>>> have on hand; we'd like to see it fixed properly as much as anyone.
>>>
>>> Yeah, no worries, I never doubted that. But getting things properly fixed
>>> can mean "revert, fix, reapply" when it comes to regressions in Linux --
>>> which is something that should not be seen as something bad, as Linus said
>>> himself (see below)!
>>>
>>>>  From our side at least, I don't believe this is a regression at all.
>>> In the end what matters is: some change afaics caused systems to not work
>>> anymore that used to be working -- that makes it a regression my the Linux
>>> kernels standards. And those by the same standards must be fixed, ideally
>>> quickly. Find a few quotes on that from Linus below that explains this
>>> better.
>> I feel like I should reiterate that the commit we're talking about
>> reverting is fundamental to support for one of the only two platforms
>> currently supported.
> 
> That might or might not be relevant, see the "back and forth" section
> from the Linus quotes.
> 
>> And that the changes to add "support" (just
>> bindings and DT) for the affected Renesas platforms came several months
>> *after* this.
> 
> Ohh? That might change things then. I relied on the info from Geert and
> Marek – and would be glad if you guys could sort this out, as you are
> the experts here (and I already got myself way deeper involved then I
> wanted to).

I already replied to Matt on this part -- I don't think the kernel crash 
is related to any specific platform. That the R-Car platform triggers it 
only means, that the crash is real and should be fixed.

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Marek Vasut]

On 2/16/26 2:37 PM, Matt Coster wrote:
> On 16/02/2026 11:38, Thorsten Leemhuis wrote:
>> On 2/16/26 11:58, Matt Coster wrote:
>>> On 16/02/2026 10:11, Thorsten Leemhuis wrote:
>>>
>>> We're currently trying to force this issue to reproduce on hardware we
>>> have on hand; we'd like to see it fixed properly as much as anyone.
>>
>> Yeah, no worries, I never doubted that. But getting things properly fixed
>> can mean "revert, fix, reapply" when it comes to regressions in Linux --
>> which is something that should not be seen as something bad, as Linus said
>> himself (see below)!
>>
>>>  From our side at least, I don't believe this is a regression at all.
>> In the end what matters is: some change afaics caused systems to not work
>> anymore that used to be working -- that makes it a regression my the Linux
>> kernels standards. And those by the same standards must be fixed, ideally
>> quickly. Find a few quotes on that from Linus below that explains this
>> better.
> 
> I feel like I should reiterate that the commit we're talking about
> reverting is fundamental to support for one of the only two platforms
> currently supported. And that the changes to add "support" (just
> bindings and DT) for the affected Renesas platforms came several months
> *after* this.

I would argue, that the problem at hand is not related to any specific 
platform, this is a driver bug. That some platform triggers it means, 
that the driver bug is real and has to be fixed. Whether the bug is in 
this driver or PM core.

> The "regression" here is that we allowed DTS changes to land for
> unsupported platforms in the interest of allowing further development to
> happen incrementally upstream. There has been no further progress on
> that front beyond the DTS patches, however.

Those specific DTS patches were put on hold, they couldn't be applied 
because they would lead to kernel crash in this driver, so the hold is 
to be expected.

> We have never declared that
> these platforms should be functional and error-free, and have taken
> measures to ensure this is clear to users[1].

I would argue, we should not mix functional issues with outright kernel 
crashes. If the GPU misrenders something, that is a functional issue. If 
the GPU driver crashes the kernel, that is a kernel bug and should be fixed.

And in this case, it is the later, the driver can trigger a kernel crash.

> There are currently two platforms on which this has been reproduced:
> 
>   - Renesas Gray Hawk Single (R-Car V4M) -- this was the original report
>     from Geert, and it should be noted that there are no bindings or DTS
>     support for the GPU in this platform in tree at this time.
>   - Renesas Salvator-X (R-Car M3-W) -- this was Geert's follow-up
>     reproduction case, and the upstream bindings and DTS do contain the
>     GPU, but it required adding delays to PM core code to trigger the
>     race condition(?) that causes the crash.
> 
> As far as we know, there are no other situations where this crash
> occurs.

It seems the crash would occur on any platform with hierarchical power 
domains.

> Would you consider a suitable "revert" to be fully gating support for
> these platforms (or even the entire group of Renesas platforms added in
> this "experimental" manner just to be safe) behind the exp_hw_support
> paramater until they can be properly tested? Specifically, I'm talking
> about masking them off at the of_match level so that no hardware
> interaction is even attempted without explicit user opt-in to
> experimental hardware.

No, that is only hiding the kernel crash without actually fixing it. 
This is not good.

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Marek Vasut]

On 2/16/26 11:58 AM, Matt Coster wrote:

[...]

>>>>>>> I added the regressions list onto CC, because this seems like a problem
>>>>>>> worth tracking.
>>>>>>
>>>>>> Noticed that and wondered what change caused the regression.
> 
>  From our side at least, I don't believe this is a regression at all. We
> haven't been able to reproduce this issue on any of the platforms we
> have available (although we did stumble on a somewhat related bugfix
> while trying).

There are already two proposed fixes available, and both Geert and me 
are available for testing any other fixes too.

And yes, this is a regression, because this causes a kernel crash which 
did not happen before.

>>>>>
>>>>> I think this one:
>>>>>
>>>>> 330e76d31697 ("drm/imagination: Add power domain control")
> 
> This commit added support for multiple power domains on Imagination GPUs
> in essentially the same manner as other drivers (as mentioned by Geert
> below). Nothing in there is specific to the Renesas platforms where this
> bug can be produces and it is required to support other fully-functional
> platforms such as TI AM68.

That does not imply, that the code is not buggy.

[...]

>>>> Well, it's a judgement call. 330e76d31697 was merged less then a year
>>>> ago, so I'd not be surprised at all if Linus would revert it in a case
>>>> like this. But it seems it doesn't revert clearly anymore, which
>>>> complicates things.
> 
> Reverting this change feels backwards, we're saying that Imagination's
> support for multiple power domains is broken because one platform has
> issues when we attempt to control its power domains. I fully agree that
> we should be working towards resolving this issue, but I don't agree
> that ripping out this handling (that, again, is essentially the same as
> handling in other GPU drivers) is a reasonable solution (even in the
> short term).
> 
> As it currently stands, we're only declaring two GPUs as "fully
> supported": AXE-1-16M and BXS-4-64 as made explicit in commit
> 1c21f240fbc1e ("drm/imagination: Warn or error on unsupported
> hardware").

Actually, this change landed only very recently and completely broke the 
GX6250 on R-Car M3W, which used to probe and work before. Now the driver 
does not even probe with "Unsupported GPU" output, which is clearly bogus.

I would say the aforementioned commit introduced another new regression 
-- something that used to work before no longer works, functionality was 
lost because of the change.

> In order to reach the stage where that check occurs, the GPU
> must be powered on so registers containing the GPU ID can be read.
> However, the original error report happens long after this and would now
> require the exp_hw_support module parameter to be set for this point to
> be reached (as the driver would not attempt to initialise an
> "unsupported" device without this explicit opt-in).

I don't think it is a good solution to selectively block specific 
hardware, only to avoid triggering the power domain issue. The power 
domain issue should be fixed, not hidden.

> It's not currently clear to me whether the crash can be reproduced on
> the affected Renesas platform without this opt-in; it's feasable that
> this is the case but we know it occurs during an error unwind after
> device firmware fails to initialise and the exp_hw_support check happens
> much earlier than this.
> 
> Either way, we should consider adding additional "supported" checks
> based on compatibile strings earlier during initialization to prevent
> fundamental issues like this one from even being possible on unsupported
> and/or unvalidated hardware.

Actually no, the hardware support should be as broad as possible, 
without imposing any artificial limitations like that. This allows users 
to test the driver and find errors, which if they are fixed, ultimately 
improve quality of the driver.

> All that being said, ripping out power domain handling will completely
> break one of the only two platforms we currently fully support: BXS-4-64
> which also has two power domains and is present in several TI SoCs (and
> their DTs).

I am not saying the code should be removed, I am saying the regression 
should be fixed. There are already two proposed fixes available, so it 
would be good to move them forward.

1c21f240fbc1e ("drm/imagination: Warn or error on unsupported hardware") 
should be reverted, this is not helping anything. Warning message would 
be fine though.

>>>>> But SoCs which have hierarchical
>>>>> power domains and which manage to probe this driver without having a
>>>>> firmware available for the GPU will simply end with crashed kernel,
>>>>> which is really not good.
>>>>
>>>> Does the patch Matt mentioned fix the crash? His "this does not fix the
>>>> underlying issue [...]" (see quote earlier) makes it sound like the
>>>> crash or some other problem (theoretical or practical? regression or
>>>> not?) remains. If that's the case and no quick fix in sight I guess it
>>>> would be best if someone affected could post a revert and then we can
>>>> ask Linus if he wants to pick it up.
> 
> The patch I posted (and applied) is somewhat orthogonal to the issue at
> hand. Geert originally suggested using the _attach_list() helper as a
> way to mitigate the issue, and we agree that it's a reasonable cleanup
> effort to make use of it, but I believe the "fix" in the originally
> proposed patch came from removing the device links that are required on
> our platform to ensure the GPU power domains come up (and down) in the
> correct sequence.
> 
>>>
>>> I don't think that patch would fix the crash.  The Adreno and Panfrost
>>> GPU drivers do similar things (explicit multi-PM Domain handling),
>>> so I am wondering if the issue can be triggered with them too (e.g. on
>>> unbind).
> 
> My current understanding of the situation is that the fix proposed by
> Marek in the Reneasas driver[2] works, but is not suitable since
> pm_runtime_barrier() should be inserted by the caller, not the power
> driver. But it seems that's not always possible (particularly when using
> devm), so I don't really understand where we go from here. I still don't
> see anything we're doing substantially differently (before or after the
> commit I mentioned above) from anybody else.

As far as I understand that, Geert was waiting for input from PM people?

At the beginning of this thread, almost a month ago, there was the 
following exchange, hence I was under the impression that a fix is coming:

"
 >> Can you add the links back in for a V2 or I can properly send the
 >> attached patch instead, I don't mind either way.
 >
 > Please move forward with your patch, you are the expert.
 > I prefer not to be blamed for any breakage ;-)
"

Re: [EXTERNAL] Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 4801 bytes --]

On 12/02/2026 15:56, Thorsten Leemhuis wrote:
> On 2/12/26 15:38, Marek Vasut wrote:
>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>> wrote:
>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>
>>>>>>> This changes behavior slightly:
>>>>>>>     - The new handling is also applied in case of a single PM Domain,
>>>>>>>     - PM Domains are now referred to by index instead of by name, but
>>>>>>>       "make dtbs_check" enforces the actual naming and ordering
>>>>>>> anyway,
>>>>>>>     - There are no longer device links created between virtual domain
>>>>>>>       devices, only between virtual devices and the parent device.
>>>>>>
>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>>>>> prefer not to rely on the implementation details when we can
>>>>>> declare the
>>>>>> dependencies explicitly.
>>>>>
>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>> r8a7795_areas[]),
>>>>> so they are always (un)powered in the correct order.  But that may not
>>>>> be the case in the integration on other SoCs.
>>>>>
>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>> held
>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>> for
>>>>>> the hardware to behave correctly. Your patch spurred me to run around
>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>> care
>>>>>> about the ordering.
>>>>>
>>>>> OK.
>>>>>
>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>> attached patch instead, I don't mind either way.
>>>>>
>>>>> Please move forward with your patch, you are the expert.
>>>>> I prefer not to be blamed for any breakage ;-)
>>>>
>>>> Has there been any progress on fixing this kernel crash ?
>>>>
>>>> There are already two proposed solutions, but no fix is upstream.
>>>
>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use
>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>> issue of missing synchronization in the PM core[1] is still unresolved
>>> as far as I'm aware.
>>
>> OK, but the pvr driver can currently easily crash the kernel on boot if
>> firmware is missing, so that should be fixed soon, right ?
> 
> Well, drm-misc-next afaik means that the above mentioned fix would only
> be merged in 7.1, which is ~4 months away, which is not really "soon"
> I'd say. Or did I misjudge this?

The above isn't really a "fix" per se, it's just an enhancement. The
underlying crash can still happen. We could still pick it into
drm-misc-fixes and have it in the next -rc plus backported to stable,
but I'm not sure I see the value.

> 
>> I added the regressions list onto CC, because this seems like a problem
>> worth tracking.
> 
> Noticed that and wondered what change caused the regression. Did not
> find a answer in a quick search on lore[1]. Because if it's a
> regression, we maybe should just revert the culprit for now according to
> Linus:
> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/ 

From our side at least, I don't believe this is a regression at all. We
haven't been able to reproduce this issue on any of the platforms we
have available (although we did stumble on a related bugfix[2] while
trying).

My current understanding of the situation is that the fix proposed by
Marek in the Reneasas driver[3] works, but is not suitable since
pm_runtime_barrier() should be inserted by the caller, not the power
driver. But it seems that's not always possible (particularly when using
devm), so I don't really understand where we go from here. I don't see
anything we're doing substantially differently (before or after the
commit I mentioned above) from anybody else.

Cheers,
Matt

[2]: TBC
[3]: https://lore.kernel.org/r/0e9f963b-00e0-43d1-b567-cb10b8f66df1@mailbox.org/

> 
> Ciao, Thorsten
> 
> [1] I guess this was the initial report from Geert?
> https://lore.kernel.org/all/CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/ 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH] drm/imagination: Convert to dev_pm_domain_{at,de}tach_list()

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 5151 bytes --]

Apologies, sent the wrong draft. Please disregard and see [1] instead.

Cheers,
Matt

[1]: https://lore.kernel.org/r/21b1fd77-252e-4fb3-aa65-1c26043c5412@imgtec.com/

On 16/02/2026 10:57, Matt Coster wrote:
> On 12/02/2026 15:56, Thorsten Leemhuis wrote:
>> On 2/12/26 15:38, Marek Vasut wrote:
>>> On 2/12/26 10:00 AM, Matt Coster wrote:
>>>> On 11/02/2026 19:17, Marek Vasut wrote:
>>>>> On 1/23/26 2:50 PM, Geert Uytterhoeven wrote:
>>>>>> On Fri, 23 Jan 2026 at 14:36, Matt Coster <Matt.Coster@imgtec.com>
>>>>>> wrote:
>>>>>>> On 22/01/2026 16:08, Geert Uytterhoeven wrote:
>>>>>>>> Call the dev_pm_domain_attach_list() and dev_pm_domain_detach_list()
>>>>>>>> helpers instead of open-coding multi PM Domain handling.
>>>>>>>>
>>>>>>>> This changes behavior slightly:
>>>>>>>>     - The new handling is also applied in case of a single PM Domain,
>>>>>>>>     - PM Domains are now referred to by index instead of by name, but
>>>>>>>>       "make dtbs_check" enforces the actual naming and ordering
>>>>>>>> anyway,
>>>>>>>>     - There are no longer device links created between virtual domain
>>>>>>>>       devices, only between virtual devices and the parent device.
>>>>>>>
>>>>>>> We still need this guarantee, both at start and end of day. In the
>>>>>>> current implementation dev_pm_domain_attach_list() iterates forwards,
>>>>>>> but so does dev_pm_domain_detach_list(). Even if we changed that, I'd
>>>>>>> prefer not to rely on the implementation details when we can
>>>>>>> declare the
>>>>>>> dependencies explicitly.
>>>>>>
>>>>>> Note that on R-Car, the PM Domains are nested (see e.g.
>>>>>> r8a7795_areas[]),
>>>>>> so they are always (un)powered in the correct order.  But that may not
>>>>>> be the case in the integration on other SoCs.
>>>>>>
>>>>>>> We had/have a patch (attached) kicking around internally to use the
>>>>>>> *_list() functions but keep the inter-domain links in place; it got
>>>>>>> held
>>>>>>> up by discussions as to whether we actually need those dependencies
>>>>>>> for
>>>>>>> the hardware to behave correctly. Your patch spurred me to run around
>>>>>>> the office and nag people a bit, and it seems we really do need to
>>>>>>> care
>>>>>>> about the ordering.
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>>> Can you add the links back in for a V2 or I can properly send the
>>>>>>> attached patch instead, I don't mind either way.
>>>>>>
>>>>>> Please move forward with your patch, you are the expert.
>>>>>> I prefer not to be blamed for any breakage ;-)
>>>>>
>>>>> Has there been any progress on fixing this kernel crash ?
>>>>>
>>>>> There are already two proposed solutions, but no fix is upstream.
>>>>
>>>> Yes and no. Our patch to use dev_pm_domain_attach_list() has landed in
>>>> drm-misc-next as commit e19cc5ab347e3 ("drm/imagination: Use
>>>> dev_pm_domain_attach_list()"), but this does not fix the underlying
>>>> issue of missing synchronization in the PM core[1] is still unresolved
>>>> as far as I'm aware.
>>>
>>> OK, but the pvr driver can currently easily crash the kernel on boot if
>>> firmware is missing, so that should be fixed soon, right ?
>>
>> Well, drm-misc-next afaik means that the above mentioned fix would only
>> be merged in 7.1, which is ~4 months away, which is not really "soon"
>> I'd say. Or did I misjudge this?
> 
> The above isn't really a "fix" per se, it's just an enhancement. The
> underlying crash can still happen. We could still pick it into
> drm-misc-fixes and have it in the next -rc plus backported to stable,
> but I'm not sure I see the value.
> 
>>
>>> I added the regressions list onto CC, because this seems like a problem
>>> worth tracking.
>>
>> Noticed that and wondered what change caused the regression. Did not
>> find a answer in a quick search on lore[1]. Because if it's a
>> regression, we maybe should just revert the culprit for now according to
>> Linus:
>> https://lore.kernel.org/lkml/CAHk-=wi86AosXs66-yi54+mpQjPu0upxB8ZAfG+LsMyJmcuMSA@mail.gmail.com/ 
> 
> From our side at least, I don't believe this is a regression at all. We
> haven't been able to reproduce this issue on any of the platforms we
> have available (although we did stumble on a related bugfix[2] while
> trying).
> 
> My current understanding of the situation is that the fix proposed by
> Marek in the Reneasas driver[3] works, but is not suitable since
> pm_runtime_barrier() should be inserted by the caller, not the power
> driver. But it seems that's not always possible (particularly when using
> devm), so I don't really understand where we go from here. I don't see
> anything we're doing substantially differently (before or after the
> commit I mentioned above) from anybody else.
> 
> Cheers,
> Matt
> 
> [2]: TBC
> [3]: https://lore.kernel.org/r/0e9f963b-00e0-43d1-b567-cb10b8f66df1@mailbox.org/
> 
>>
>> Ciao, Thorsten
>>
>> [1] I guess this was the initial report from Geert?
>> https://lore.kernel.org/all/CAMuHMdWapT40hV3c+CSBqFOW05aWcV1a6v_NiJYgoYi0i9_PDQ@mail.gmail.com/ 
> 
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]