[PATCH 0/4] drm/vc4: Reset the V3D block through the reset controller

[PATCH 0/4] drm/vc4: Reset the V3D block through the reset controller

[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. It can be dropped from the series if it turns out
to be contentious.

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

Best regards,
- Maíra

---
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: 09b47186a4164f3aaa3591313f80794443117342
change-id: 20260812-vc4-reset-control-cddf3eb8a9e9

[PATCH 1/4] drm/vc4: Drop the undocumented brcm,vc4-v3d compatible

[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.

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 d31b906cb8e7..379ab77243ad 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

[PATCH 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line

[Maíra Canal]

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."), which allows the GPU to be reset without
power-cycling its domain. Document it so that the vc4 driver can use it to
recover from a GPU hang.

Keep the property optional, as existing device trees do not describe it.

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

[PATCH 3/4] drm/vc4: Use the reset controller to recover from a GPU hang

[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.

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 0f5958c1e6b6..86e0e7c12901 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 8e5141bb5075..90c5194a1c93 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 379ab77243ad..d2da8d2f8eeb 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

[PATCH 4/4] ARM: dts: bcm2835: Add the V3D reset line

[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.

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

Re: [PATCH 2/4] dt-bindings: display: bcm2835-v3d: Add an optional reset line

[Stefan Wahren]

Hi Maíra,

Am 16.08.26 um 16:05 schrieb Maíra Canal:
> 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."), which allows the GPU to be reset without
> power-cycling its domain. Document it so that the vc4 driver can use it to
> recover from a GPU hang.
not sure the last sentence is necessary for a DT binding. Except of this:

Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
>
> Keep the property optional, as existing device trees do not describe it.
>
> 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
>

Re: [PATCH 4/4] ARM: dts: bcm2835: Add the V3D reset line

[Stefan Wahren]

Am 16.08.26 um 16:05 schrieb 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.
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
> ---
>   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>;
>   		};
>   
>   	};
>