Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller
@ 2026-09-15 20:51 Maíra Canal
  2026-09-15 20:51 ` [PATCH v2 1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible Maíra Canal
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Maíra Canal @ 2026-09-15 20:51 UTC (permalink / raw)
  To: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list
  Cc: kernel-dev, dri-devel, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Krzysztof Kozlowski, Maíra Canal

vc4_reset() recovers a hung GPU by dropping the runtime PM usage count to
zero so that the V3D power domain goes down, and then taking it again.
Forcing the count to zero only works if the driver knows exactly how many
references it holds, which is why vc4 unconventionaly wraps every
pm_runtime_get_sync() call in a private refcount and mutex, making the
code cumbersome.

The BCM2835 PM block has exposed a reset line for the V3D block since
commit 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains
under a new binding."), added for exactly this reason, but vc4 never
picked it up. This series switches vc4 over to it and drops the private
refcount, leaving vc4_v3d_pm_get/put() as plain runtime PM wrappers.

The reset line is optional, so device trees that do not describe one keep
working: they get the driver-side recovery in vc4_irq_reset() while the
hardware is left untouched. Of the two in-tree platforms,

  1. BCM2835 gains the property in PATCH 4.
  2. Cygnus has no V3D power domain to begin with, so the power-cycle
     only ever gated its clock there.

PATCH 1 is an independent cleanup. "brcm,vc4-v3d" was never documented
and was never used by any in-tree device tree, so it is a candidate for
removal on its own.

This series was tested on the Raspberry Pi 3B and 3B+.

Best regards,
- Maíra

---
v1 -> v2: https://lore.kernel.org/r/20260816-vc4-reset-control-v1-0-896444373716@igalia.com

- [1/4, 2/4, 3/4, 4/4] Add R-b and A-b tags from Krzysztof, Florian, and
  Stefan.
- [2/4] Rewrite commit message (Stefan, Krzysztof)

---
Maíra Canal (4):
      drm/vc4: Drop the undocumented brcm,vc4-v3d compatible
      dt-bindings: display: bcm2835-v3d: Add an optional reset line
      drm/vc4: Use the reset controller to recover from a GPU hang
      ARM: dts: bcm2835: Add the V3D reset line

 .../bindings/display/brcm,bcm2835-v3d.yaml         |  3 ++
 arch/arm/boot/dts/broadcom/bcm2835-common.dtsi     |  1 +
 drivers/gpu/drm/vc4/vc4_drv.c                      |  1 -
 drivers/gpu/drm/vc4/vc4_drv.h                      | 13 ++++---
 drivers/gpu/drm/vc4/vc4_gem.c                      | 40 +++++++++++++---------
 drivers/gpu/drm/vc4/vc4_irq.c                      |  7 ++--
 drivers/gpu/drm/vc4/vc4_v3d.c                      | 37 +++++++-------------
 7 files changed, 51 insertions(+), 51 deletions(-)
---
base-commit: 5363e01e4ff03a204cab85292b89cf90c2d1b101
change-id: 20260812-vc4-reset-control-cddf3eb8a9e9


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

* [PATCH v2 1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible
  2026-09-15 20:51 [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
@ 2026-09-15 20:51 ` Maíra Canal
  2026-09-15 20:51 ` [PATCH v2 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line Maíra Canal
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Maíra Canal @ 2026-09-15 20:51 UTC (permalink / raw)
  To: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list
  Cc: kernel-dev, dri-devel, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Krzysztof Kozlowski, Maíra Canal

"brcm,vc4-v3d" is the compatible string V3D support originally shipped
with in commit d3f5168a0810 ("drm/vc4: Bind and initialize the V3D
engine."). Commit 90d7116061f8 ("drm/vc4: Recognize a more specific
compatible string for V3D.") added "brcm,bcm2835-v3d" in v4.6 and kept
the original one around, because the Raspberry Pi firmware was shipping
device trees that used it.

That string was never part of the binding: the documentation added in
commit 4653f22e9ab0 ("dt-bindings: Add binding docs for V3D.") describes
only "brcm,bcm2835-v3d", and no upstream device tree has ever used
"brcm,vc4-v3d": the first in-tree V3D node, from commit 49ac67e0c39c
("ARM: bcm2835: Add VC4 to the device tree."), already carried the
specific name. The firmware device trees the compatible was kept for
predate v4.6, released in 2016.

Drop it from both match tables.

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 1 -
 drivers/gpu/drm/vc4/vc4_v3d.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 616caf9d9915..eb859c65a453 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -279,7 +279,6 @@ static const struct of_device_id vc4_dma_range_matches[] = {
 	{ .compatible = "brcm,bcm2835-hvs" },
 	{ .compatible = "brcm,bcm2835-v3d" },
 	{ .compatible = "brcm,cygnus-v3d" },
-	{ .compatible = "brcm,vc4-v3d" },
 	{}
 };
 
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f32410420d3e..a86739873e05 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -531,7 +531,6 @@ static void vc4_v3d_dev_remove(struct platform_device *pdev)
 const struct of_device_id vc4_v3d_dt_match[] = {
 	{ .compatible = "brcm,bcm2835-v3d" },
 	{ .compatible = "brcm,cygnus-v3d" },
-	{ .compatible = "brcm,vc4-v3d" },
 	{}
 };
 

-- 
2.55.0


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

* [PATCH v2 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line
  2026-09-15 20:51 [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
  2026-09-15 20:51 ` [PATCH v2 1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible Maíra Canal
@ 2026-09-15 20:51 ` Maíra Canal
  2026-09-18  8:55   ` Krzysztof Kozlowski
  2026-09-15 20:51 ` [PATCH v2 3/4] drm/vc4: Use the reset controller to recover from a GPU hang Maíra Canal
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Maíra Canal @ 2026-09-15 20:51 UTC (permalink / raw)
  To: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list
  Cc: kernel-dev, dri-devel, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Maíra Canal

The BCM2835 power management block drives a reset line for the V3D block
that is independent of the graphics power domain, allowing the GPU to be
reset without power-cycling the domain. Expose this reset line in the V3D
binding.

Keep the property optional, as devices like the Broadcom Cygnus don't have
a reset line.

Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml
index c55a8217de25..1063e686647c 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml
+++ b/Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml
@@ -27,6 +27,9 @@ properties:
   power-domains:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
 required:
   - compatible
   - reg

-- 
2.55.0


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

* [PATCH v2 3/4] drm/vc4: Use the reset controller to recover from a GPU hang
  2026-09-15 20:51 [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
  2026-09-15 20:51 ` [PATCH v2 1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible Maíra Canal
  2026-09-15 20:51 ` [PATCH v2 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line Maíra Canal
@ 2026-09-15 20:51 ` Maíra Canal
  2026-09-15 21:04   ` sashiko-bot
  2026-09-15 20:51 ` [PATCH v2 4/4] ARM: dts: bcm2835: Add the V3D reset line Maíra Canal
  2026-09-18 18:55 ` [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
  4 siblings, 1 reply; 9+ messages in thread
From: Maíra Canal @ 2026-09-15 20:51 UTC (permalink / raw)
  To: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list
  Cc: kernel-dev, dri-devel, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Maíra Canal

vc4_reset() recovers a hung GPU by dropping the runtime PM usage count to
zero so that the power domain goes down, and then taking it again. Such an
unconventional approach only works if the driver knows exactly how many
references it holds, which is why vc4 wrapped every pm_runtime_get_sync()
call in a private refcount and mutex.

Commit 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains
under a new binding.") exposed a V3D reset line for exactly this reason,
so that the block can be reset without power-cycling its domain, but the
vc4 driver never picked it up. Use it now, which removes the need for the
private refcount and leaves vc4_v3d_pm_get/put() as plain runtime PM
wrappers.

The reset line is optional, to accommodate older device trees. Device
trees that do not describe one still get the driver-side recovery in
vc4_irq_reset(), but the hardware is left untouched. Two in-tree platforms
use the VC4 V3D block: BCM2835 gains the resets property in the next
commit, and Cygnus is no worse off than it was, as its V3D node has no
power domain and the power-cycle only ever gated its clock.

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h | 13 ++++++++-----
 drivers/gpu/drm/vc4/vc4_gem.c | 40 ++++++++++++++++++++++++----------------
 drivers/gpu/drm/vc4/vc4_irq.c |  7 +++----
 drivers/gpu/drm/vc4/vc4_v3d.c | 36 ++++++++++++------------------------
 4 files changed, 47 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 649032174dbd..695bc6a29d30 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -212,14 +212,9 @@ struct vc4_dev {
 
 	struct work_struct overflow_mem_work;
 
-	int power_refcount;
-
 	/* Set to true when the load tracker is active. */
 	bool load_tracker_enabled;
 
-	/* Mutex controlling the power refcount. */
-	struct mutex power_lock;
-
 	struct {
 		struct timer_list timer;
 		struct work_struct reset_work;
@@ -294,6 +289,13 @@ struct vc4_v3d {
 	struct platform_device *pdev;
 	void __iomem *regs;
 	struct clk *clk;
+
+	/* Reset line for the V3D block, used to recover from a GPU hang.
+	 * NULL if the device tree does not describe one, in which case the
+	 * GPU cannot be reset.
+	 */
+	struct reset_control *reset;
+
 	struct debugfs_regset32 regset;
 };
 
@@ -1056,6 +1058,7 @@ int vc4_v3d_bin_bo_get(struct vc4_dev *vc4, bool *used);
 void vc4_v3d_bin_bo_put(struct vc4_dev *vc4);
 int vc4_v3d_pm_get(struct vc4_dev *vc4);
 void vc4_v3d_pm_put(struct vc4_dev *vc4);
+void vc4_v3d_init_hw(struct drm_device *dev);
 int vc4_v3d_debugfs_init(struct drm_minor *minor);
 
 /* vc4_validate.c */
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index e231c906709c..3212b9167620 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -23,7 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
+#include <linux/reset.h>
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/sched/signal.h>
@@ -292,19 +292,22 @@ vc4_save_hang_state(struct drm_device *dev)
 static void
 vc4_reset(struct drm_device *dev)
 {
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct vc4_v3d *v3d = to_vc4_dev(dev)->v3d;
+	int ret;
 
-	DRM_INFO("Resetting GPU.\n");
+	vc4_irq_disable(dev);
 
-	mutex_lock(&vc4->power_lock);
-	if (vc4->power_refcount) {
-		/* Power the device off and back on the by dropping the
-		 * reference on runtime PM.
-		 */
-		pm_runtime_put_sync_suspend(&vc4->v3d->pdev->dev);
-		pm_runtime_get_sync(&vc4->v3d->pdev->dev);
+	if (v3d->reset) {
+		drm_info(dev, "Resetting GPU.\n");
+
+		ret = reset_control_reset(v3d->reset);
+		if (ret)
+			drm_err(dev, "Failed to reset the GPU: %d\n", ret);
+
+		vc4_v3d_init_hw(dev);
+	} else {
+		drm_info_once(dev, "No reset line; GPU state is not reset.\n");
 	}
-	mutex_unlock(&vc4->power_lock);
 
 	vc4_irq_reset(dev);
 
@@ -320,10 +323,19 @@ vc4_reset_work(struct work_struct *work)
 {
 	struct vc4_dev *vc4 =
 		container_of(work, struct vc4_dev, hangcheck.reset_work);
+	int ret;
+
+	/* Make sure the device is not suspended during the reset. */
+	ret = vc4_v3d_pm_get(vc4);
+	if (ret) {
+		drm_err(&vc4->base, "Failed to resume V3D for GPU reset: %d\n", ret);
+		return;
+	}
 
 	vc4_save_hang_state(&vc4->base);
-
 	vc4_reset(&vc4->base);
+
+	vc4_v3d_pm_put(vc4);
 }
 
 static void
@@ -1177,10 +1189,6 @@ int vc4_gem_init(struct drm_device *dev)
 
 	INIT_WORK(&vc4->job_done_work, vc4_job_done_work);
 
-	ret = drmm_mutex_init(dev, &vc4->power_lock);
-	if (ret)
-		return ret;
-
 	INIT_LIST_HEAD(&vc4->purgeable.list);
 
 	ret = drmm_mutex_init(dev, &vc4->purgeable.lock);
diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index 7877d493d80e..3a3ea1e62dcb 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -336,10 +336,9 @@ void vc4_irq_reset(struct drm_device *dev)
 	V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
 
 	/*
-	 * Turn all our interrupts on.  Binner out of memory is the
-	 * only one we expect to trigger at this point, since we've
-	 * just come from poweron and haven't supplied any overflow
-	 * memory yet.
+	 * Turn all our interrupts on. Binner out of memory is the only
+	 * one we expect to trigger at this point, since the reset cleared
+	 * the overflow memory address and none has been supplied yet.
 	 */
 	V3D_WRITE(V3D_INTENA, V3D_DRIVER_IRQS);
 
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index a86739873e05..b40d98c9d1d2 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -9,6 +9,7 @@
 #include <linux/component.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include <drm/drm_print.h>
 
@@ -122,29 +123,13 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 	return 0;
 }
 
-/*
- * Wraps pm_runtime_get_sync() in a refcount, so that we can reliably
- * get the pm_runtime refcount to 0 in vc4_reset().
- */
 int
 vc4_v3d_pm_get(struct vc4_dev *vc4)
 {
 	if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4))
 		return -ENODEV;
 
-	mutex_lock(&vc4->power_lock);
-	if (vc4->power_refcount++ == 0) {
-		int ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-
-		if (ret < 0) {
-			vc4->power_refcount--;
-			mutex_unlock(&vc4->power_lock);
-			return ret;
-		}
-	}
-	mutex_unlock(&vc4->power_lock);
-
-	return 0;
+	return pm_runtime_resume_and_get(&vc4->v3d->pdev->dev);
 }
 
 void
@@ -153,15 +138,10 @@ vc4_v3d_pm_put(struct vc4_dev *vc4)
 	if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4))
 		return;
 
-	mutex_lock(&vc4->power_lock);
-	if (--vc4->power_refcount == 0) {
-		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
-		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
-	}
-	mutex_unlock(&vc4->power_lock);
+	pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
 }
 
-static void vc4_v3d_init_hw(struct drm_device *dev)
+void vc4_v3d_init_hw(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
@@ -447,6 +427,14 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
 	if (IS_ERR(v3d->clk))
 		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
 
+	v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(v3d->reset))
+		return dev_err_probe(dev, PTR_ERR(v3d->reset),
+				     "Failed to get reset control\n");
+
+	if (!v3d->reset)
+		drm_warn(drm, "No V3D reset line in the device tree");
+
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
 		return ret;

-- 
2.55.0


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

* [PATCH v2 4/4] ARM: dts: bcm2835: Add the V3D reset line
  2026-09-15 20:51 [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
                   ` (2 preceding siblings ...)
  2026-09-15 20:51 ` [PATCH v2 3/4] drm/vc4: Use the reset controller to recover from a GPU hang Maíra Canal
@ 2026-09-15 20:51 ` Maíra Canal
  2026-09-15 21:03   ` sashiko-bot
  2026-09-18 18:55 ` [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
  4 siblings, 1 reply; 9+ messages in thread
From: Maíra Canal @ 2026-09-15 20:51 UTC (permalink / raw)
  To: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list
  Cc: kernel-dev, dri-devel, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Maíra Canal

The PM block exposes a reset line for the V3D block, which the vc4 driver
uses to reset the GPU when recovering from a hang instead of forcing its
power domain down. Wire it up in the shared V3D node, covering BCM2835,
BCM2836 and BCM2837.

BCM2711 already describes the same reset line for its V3D node.

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 arch/arm/boot/dts/broadcom/bcm2835-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
index 1e76b290510d..39eea8ce1058 100644
--- a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
@@ -139,6 +139,7 @@ v3d: v3d@7ec00000 {
 			compatible = "brcm,bcm2835-v3d";
 			reg = <0x7ec00000 0x1000>;
 			interrupts = <1 10>;
+			resets = <&pm BCM2835_RESET_V3D>;
 		};
 
 	};

-- 
2.55.0


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

* Re: [PATCH v2 4/4] ARM: dts: bcm2835: Add the V3D reset line
  2026-09-15 20:51 ` [PATCH v2 4/4] ARM: dts: bcm2835: Add the V3D reset line Maíra Canal
@ 2026-09-15 21:03   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-15 21:03 UTC (permalink / raw)
  To: Maíra Canal; +Cc: devicetree, robh, dri-devel, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: reference leak and underflow in bcm2835_reset_reset via bypassed genpd
--

commit a62442494a68213d73ba03025de353d2162a2d5f
Author: Maíra Canal <mcanal@igalia.com>

ARM: dts: bcm2835: Add the V3D reset line

This commit wires up the V3D reset line to the shared V3D node in the
device tree for BCM2835, BCM2836, and BCM2837. This enables the vc4
driver to use the reset line when recovering from a hang instead of
forcing the power domain down.

> diff --git a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
> index 1e76b290510d3..39eea8ce10583 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
> +++ b/arch/arm/boot/dts/broadcom/bcm2835-common.dtsi
> @@ -139,6 +139,7 @@ v3d: v3d@7ec00000 {
>  			compatible = "brcm,bcm2835-v3d";
>  			reg = <0x7ec00000 0x1000>;
>  			interrupts = <1 10>;
> +			resets = <&pm BCM2835_RESET_V3D>;

[Severity: Medium]
Will this reset configuration conflict with the firmware power domain on
Raspberry Pi?

On boards where the V3D device uses a firmware-managed power domain, the
native bcm2835-power genpd is left uninitialized and its native clock
(BCM2835_CLOCK_V3D) enable count is 0.

When the vc4 driver detects a GPU hang, it triggers a reset that calls
into bcm2835_reset_reset(). This unconditionally cycles the native power
domain off and on:

drivers/pmdomain/bcm/bcm2835-power.c:bcm2835_reset_reset() {
	...
	ret = bcm2835_power_pd_power_off(&pd->base);
	...
}

drivers/pmdomain/bcm/bcm2835-power.c:bcm2835_asb_power_off() {
	...
	clk_disable_unprepare(pd->clk);
	...
}

Does calling clk_disable_unprepare() here trigger a kernel WARN_ON by
underflowing the uninitialized 0 count? And subsequently, when the reset
path calls bcm2835_power_pd_power_on(), does clk_prepare_enable()
increment the count from 0 to 1, permanently leaking a clock reference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-vc4-reset-control-v2-0-cb3a25b07822@igalia.com?part=4

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

* Re: [PATCH v2 3/4] drm/vc4: Use the reset controller to recover from a GPU hang
  2026-09-15 20:51 ` [PATCH v2 3/4] drm/vc4: Use the reset controller to recover from a GPU hang Maíra Canal
@ 2026-09-15 21:04   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-15 21:04 UTC (permalink / raw)
  To: Maíra Canal; +Cc: robh, conor+dt, devicetree, dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/vc4: UAF in vc4_job_done_work() due to missing hardware reset
- [Medium] drm/vc4: component re-bind fails due to devm_reset_control in bind
--

commit 8ebe4ef2282246bb3aa856ccf103a302ccfae13f
Author: Maíra Canal <mcanal@igalia.com>

drm/vc4: Use the reset controller to recover from a GPU hang

This patch switches the GPU recovery mechanism from a runtime PM power cycle
to using a dedicated V3D reset line. It also leaves the runtime PM wrapper
functions as simple passthroughs and makes the reset line optional for older
device trees.

> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> index e231c906709c1..3212b91676201 100644
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
[ ... ]
> @@ -292,19 +292,22 @@ vc4_save_hang_state(struct drm_device *dev)
>  static void
>  vc4_reset(struct drm_device *dev)
>  {
> -	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +	struct vc4_v3d *v3d = to_vc4_dev(dev)->v3d;
> +	int ret;
>  
> -	DRM_INFO("Resetting GPU.\n");
> +	vc4_irq_disable(dev);
>  
> -	mutex_lock(&vc4->power_lock);
> -	if (vc4->power_refcount) {
> -		/* Power the device off and back on the by dropping the
> -		 * reference on runtime PM.
> -		 */
> -		pm_runtime_put_sync_suspend(&vc4->v3d->pdev->dev);
> -		pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> +	if (v3d->reset) {
> +		drm_info(dev, "Resetting GPU.\n");
> +
> +		ret = reset_control_reset(v3d->reset);
> +		if (ret)
> +			drm_err(dev, "Failed to reset the GPU: %d\n", ret);
> +
> +		vc4_v3d_init_hw(dev);
> +	} else {
> +		drm_info_once(dev, "No reset line; GPU state is not reset.\n");
>  	}
> -	mutex_unlock(&vc4->power_lock);
>  
>  	vc4_irq_reset(dev);

[Severity: High]
Does this create a use-after-free on older device trees where v3d->reset is
NULL? 

By removing the runtime PM suspend/resume cycle, the hardware is no longer
reset on older platforms. However, the driver continues to unconditionally
call vc4_irq_reset() which cancels jobs and finishes render jobs:

vc4_irq_reset()
  vc4_cancel_bin_job(dev)
  vc4_irq_finish_render_job(dev)

These routines signal the job fences and schedule vc4_job_done_work, which
frees the GEM memory. If the GPU hardware is still executing because the
reset was skipped, won't it asynchronously access this freed memory when it
eventually makes progress?

[ ... ]
> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
> index a86739873e05b..b40d98c9d1d28 100644
> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
[ ... ]
> @@ -447,6 +427,14 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
>  	if (IS_ERR(v3d->clk))
>  		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
>  
> +	v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(v3d->reset))
> +		return dev_err_probe(dev, PTR_ERR(v3d->reset),
> +				     "Failed to get reset control\n");
> +
> +	if (!v3d->reset)
> +		drm_warn(drm, "No V3D reset line in the device tree");
> +
>  	ret = platform_get_irq(pdev, 0);

[Severity: Medium]
Will this cause component re-bind operations to fail?

Since devm allocations are tied to the device's initial probe lifecycle rather
than the component bind/unbind lifecycle, the exclusive reset control is not
released when the component is unbound. 

If the DRM master device is unbound and then rebound, wouldn't a subsequent
bind attempt fail with -EBUSY because the leaked initial allocation still
holds the exclusive lock on the reset line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-vc4-reset-control-v2-0-cb3a25b07822@igalia.com?part=3

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

* Re: [PATCH v2 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line
  2026-09-15 20:51 ` [PATCH v2 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line Maíra Canal
@ 2026-09-18  8:55   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-18  8:55 UTC (permalink / raw)
  To: Maíra Canal
  Cc: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list, kernel-dev, dri-devel,
	devicetree, linux-rpi-kernel, linux-arm-kernel

On Tue, Sep 15, 2026 at 05:51:25PM -0300, Maíra Canal wrote:
> The BCM2835 power management block drives a reset line for the V3D block
> that is independent of the graphics power domain, allowing the GPU to be
> reset without power-cycling the domain. Expose this reset line in the V3D
> binding.
> 
> Keep the property optional, as devices like the Broadcom Cygnus don't have
> a reset line.
> 
> Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


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

* Re: [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller
  2026-09-15 20:51 [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
                   ` (3 preceding siblings ...)
  2026-09-15 20:51 ` [PATCH v2 4/4] ARM: dts: bcm2835: Add the V3D reset line Maíra Canal
@ 2026-09-18 18:55 ` Maíra Canal
  4 siblings, 0 replies; 9+ messages in thread
From: Maíra Canal @ 2026-09-18 18:55 UTC (permalink / raw)
  To: Maxime Ripard, Dave Stevenson, Raspberry Pi Kernel Maintenance,
	Stefan Wahren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list, Maíra Canal
  Cc: kernel-dev, dri-devel, devicetree, linux-rpi-kernel,
	linux-arm-kernel, Krzysztof Kozlowski


On Tue, 15 Sep 2026 17:51:23 -0300, Maíra Canal wrote:
> vc4_reset() recovers a hung GPU by dropping the runtime PM usage count to
> zero so that the V3D power domain goes down, and then taking it again.
> Forcing the count to zero only works if the driver knows exactly how many
> references it holds, which is why vc4 unconventionaly wraps every
> pm_runtime_get_sync() call in a private refcount and mutex, making the
> code cumbersome.
> 
> [...]

Applied, thanks!

[1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible
      commit: fb825a46e7982460bd620f5e6a71924c647f4280
[2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line
      commit: 6ab4aad3481903ac3743513517c290a3ebd39232
[3/4] drm/vc4: Use the reset controller to recover from a GPU hang
      commit: b4448823f151d0b4a3f3f4c5622c6ade51422c4e
[4/4] ARM: dts: bcm2835: Add the V3D reset line
      commit: 3afbeb6b99ddbe082a238a659b7e7a7239f49e0e

Best regards,
- Maíra

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

end of thread, other threads:[~2026-09-18 18:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 20:51 [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal
2026-09-15 20:51 ` [PATCH v2 1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible Maíra Canal
2026-09-15 20:51 ` [PATCH v2 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line Maíra Canal
2026-09-18  8:55   ` Krzysztof Kozlowski
2026-09-15 20:51 ` [PATCH v2 3/4] drm/vc4: Use the reset controller to recover from a GPU hang Maíra Canal
2026-09-15 21:04   ` sashiko-bot
2026-09-15 20:51 ` [PATCH v2 4/4] ARM: dts: bcm2835: Add the V3D reset line Maíra Canal
2026-09-15 21:03   ` sashiko-bot
2026-09-18 18:55 ` [PATCH v2 0/4] drm/vc4: Reset the V3D block through the reset controller Maíra Canal

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