[PATCH 0/6] drm/vc4: Miscellaneous fixes and improvements

[PATCH 0/6] drm/vc4: Miscellaneous fixes and improvements

[Maíra Canal]

Hi,

This is a spin-off of the series "[PATCH 00/11] drm/vc4: Switch to DRM
GPU scheduler" [1]. While I was writing the series, I noticed some small
bugs in the driver code, so I compiled the fixes and code improvements
in this series.

Patches #1 to #4 are bug-fixes related to PM reference unbalance, memory
leaks, and race conditions. They will probably go through the
drm-misc-fixes branch. Finally, patches #5 and #6 are minor clean-ups
and common API adoption.

[1] https://lore.kernel.org/dri-devel/20260205-vc4-drm-scheduler-v1-0-c6174fd7bbc1@igalia.com/T/

Best regards,
- Maíra

---
Maíra Canal (6):
      drm/vc4: Release runtime PM reference after binding V3D
      drm/vc4: Fix memory leak of BO array in hang state
      drm/vc4: Fix a memory leak in hang state error path
      drm/vc4: Protect madv read in vc4_gem_object_mmap() with madv_lock
      drm/vc4: Use devm_request_irq() for automatic cleanup
      drm/vc4: Clean-up UAPI header inclusion

 drivers/gpu/drm/vc4/vc4_bo.c        |  4 +++-
 drivers/gpu/drm/vc4/vc4_drv.c       |  2 --
 drivers/gpu/drm/vc4/vc4_gem.c       | 20 +++++++++++---------
 drivers/gpu/drm/vc4/vc4_irq.c       | 33 ++++++++++++---------------------
 drivers/gpu/drm/vc4/vc4_plane.c     |  2 --
 drivers/gpu/drm/vc4/vc4_render_cl.c |  1 -
 drivers/gpu/drm/vc4/vc4_v3d.c       |  1 +
 drivers/gpu/drm/vc4/vc4_validate.c  |  1 -
 8 files changed, 27 insertions(+), 37 deletions(-)
---
base-commit: dac97a6bd6c4a24e60a147cb2cf750eddb7594a8
change-id: 20260330-vc4-misc-fixes-9b874f854fcf

[PATCH 1/6] drm/vc4: Release runtime PM reference after binding V3D

[Maíra Canal]

The vc4_v3d_bind() function acquires a runtime PM reference via
pm_runtime_resume_and_get() to access V3D registers during setup.
However, this reference is never released after a successful bind.
This prevents the device from ever runtime suspending, since the
reference count never reaches zero.

Release the runtime PM reference by adding pm_runtime_put_autosuspend()
after autosuspend is configured, allowing the device to runtime suspend
after the delay.

Fixes: 266cff37d7fc ("drm/vc4: v3d: Rework the runtime_pm setup")
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 3ffe09bc89d273c2ec598f391147425d9f6785bf..d31b906cb8e787517ba3ff72c236ffcf810522b1 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -481,6 +481,7 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
 
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_set_autosuspend_delay(dev, 40); /* a little over 2 frames. */
+	pm_runtime_put_autosuspend(dev);
 
 	return 0;
 

-- 
2.53.0

[PATCH 2/6] drm/vc4: Fix memory leak of BO array in hang state

[Maíra Canal]

The hang state's BO array is allocated separately with kzalloc() in
vc4_save_hang_state() but never freed in vc4_free_hang_state(). Add the
missing kfree() for the BO array before freeing the hang state struct.

Fixes: 214613656b51 ("drm/vc4: Add an interface for capturing the GPU state after a hang.")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 2d3df5e621c1a2192656efd6fc8940ebb1ce991f..248c47665a193fa639a481f97ac2af634f415384 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -62,6 +62,7 @@ vc4_free_hang_state(struct drm_device *dev, struct vc4_hang_state *state)
 	for (i = 0; i < state->user_state.bo_count; i++)
 		drm_gem_object_put(state->bo[i]);
 
+	kfree(state->bo);
 	kfree(state);
 }
 

-- 
2.53.0

[PATCH 3/6] drm/vc4: Fix a memory leak in hang state error path

[Maíra Canal]

When vc4_save_hang_state() encounters an early return condition, it
returns without freeing the previously allocated `kernel_state`,
leaking memory.

Add the missing kfree() calls by consolidating the early return paths
into a single place.

Fixes: 214613656b51 ("drm/vc4: Add an interface for capturing the GPU state after a hang.")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_gem.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 248c47665a193fa639a481f97ac2af634f415384..ab3c6d5d4eb446804877d6b8da01c3a9f65800df 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -171,10 +171,8 @@ vc4_save_hang_state(struct drm_device *dev)
 	spin_lock_irqsave(&vc4->job_lock, irqflags);
 	exec[0] = vc4_first_bin_job(vc4);
 	exec[1] = vc4_first_render_job(vc4);
-	if (!exec[0] && !exec[1]) {
-		spin_unlock_irqrestore(&vc4->job_lock, irqflags);
-		return;
-	}
+	if (!exec[0] && !exec[1])
+		goto err_free_state;
 
 	/* Get the bos from both binner and renderer into hang state. */
 	state->bo_count = 0;
@@ -191,10 +189,8 @@ vc4_save_hang_state(struct drm_device *dev)
 	kernel_state->bo = kzalloc_objs(*kernel_state->bo, state->bo_count,
 					GFP_ATOMIC);
 
-	if (!kernel_state->bo) {
-		spin_unlock_irqrestore(&vc4->job_lock, irqflags);
-		return;
-	}
+	if (!kernel_state->bo)
+		goto err_free_state;
 
 	k = 0;
 	for (i = 0; i < 2; i++) {
@@ -286,6 +282,12 @@ vc4_save_hang_state(struct drm_device *dev)
 		vc4->hang_state = kernel_state;
 		spin_unlock_irqrestore(&vc4->job_lock, irqflags);
 	}
+
+	return;
+
+err_free_state:
+	spin_unlock_irqrestore(&vc4->job_lock, irqflags);
+	kfree(kernel_state);
 }
 
 static void

-- 
2.53.0

[PATCH 4/6] drm/vc4: Protect madv read in vc4_gem_object_mmap() with madv_lock

[Maíra Canal]

The mmap callback reads bo->madv without holding madv_lock, racing with
concurrent DRM_IOCTL_VC4_GEM_MADVISE calls that modify the field under
the same lock. Add the missing locking to prevent the data race.

Fixes: b9f19259b84d ("drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_bo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index f45ba47b4ba8645f371215c083d3ead3ddd5ffe8..9377e58f9bc256ea81e19cc442d04139feec20fe 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -738,12 +738,15 @@ static int vc4_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct
 		return -EINVAL;
 	}
 
+	mutex_lock(&bo->madv_lock);
 	if (bo->madv != VC4_MADV_WILLNEED) {
 		DRM_DEBUG("mmapping of %s BO not allowed\n",
 			  bo->madv == VC4_MADV_DONTNEED ?
 			  "purgeable" : "purged");
+		mutex_unlock(&bo->madv_lock);
 		return -EINVAL;
 	}
+	mutex_unlock(&bo->madv_lock);
 
 	return drm_gem_dma_mmap(&bo->base, vma);
 }

-- 
2.53.0

[PATCH 5/6] drm/vc4: Use devm_request_irq() for automatic cleanup

[Maíra Canal]

Switch from request_irq()/free_irq() to the device-managed alternative
devm_request_irq(), letting device-managed resource cleanup handle IRQ
teardown automatically.

While here, inline vc4_irq_prepare() into vc4_irq_install() since it's
the only caller.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_irq.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index 63e88f90eef7c0720e32547f4e8b5ed289b2427b..8e5141bb50759e19e03c911f75b78354b240406f 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -47,7 +47,6 @@
 
 #include <linux/platform_device.h>
 
-#include <drm/drm_drv.h>
 #include <drm/drm_print.h>
 
 #include "vc4_drv.h"
@@ -242,23 +241,6 @@ vc4_irq(int irq, void *arg)
 	return status;
 }
 
-static void
-vc4_irq_prepare(struct drm_device *dev)
-{
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-
-	if (!vc4->v3d)
-		return;
-
-	init_waitqueue_head(&vc4->job_wait_queue);
-	INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work);
-
-	/* Clear any pending interrupts someone might have left around
-	 * for us.
-	 */
-	V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
-}
-
 void
 vc4_irq_enable(struct drm_device *dev)
 {
@@ -307,12 +289,22 @@ int vc4_irq_install(struct drm_device *dev, int irq)
 	if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4))
 		return -ENODEV;
 
+	if (!vc4->v3d)
+		return -ENODEV;
+
 	if (irq == IRQ_NOTCONNECTED)
 		return -ENOTCONN;
 
-	vc4_irq_prepare(dev);
+	init_waitqueue_head(&vc4->job_wait_queue);
+	INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work);
 
-	ret = request_irq(irq, vc4_irq, 0, dev->driver->name, dev);
+	/* Clear any pending interrupts someone might have left around
+	 * for us.
+	 */
+	V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
+
+	ret = devm_request_irq(dev->dev, irq, vc4_irq, 0,
+			       dev_name(dev->dev), dev);
 	if (ret)
 		return ret;
 
@@ -329,7 +321,6 @@ void vc4_irq_uninstall(struct drm_device *dev)
 		return;
 
 	vc4_irq_disable(dev);
-	free_irq(vc4->irq, dev);
 }
 
 /** Reinitializes interrupt registers when a GPU reset is performed. */

-- 
2.53.0

[PATCH 6/6] drm/vc4: Clean-up UAPI header inclusion

[Maíra Canal]

"uapi/drm/vc4_drm.h" is already included through "vc4_drv.h". Therefore,
remove its direct inclusion from several files.

Suggested-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_bo.c        | 1 -
 drivers/gpu/drm/vc4/vc4_drv.c       | 2 --
 drivers/gpu/drm/vc4/vc4_gem.c       | 1 -
 drivers/gpu/drm/vc4/vc4_plane.c     | 2 --
 drivers/gpu/drm/vc4/vc4_render_cl.c | 1 -
 drivers/gpu/drm/vc4/vc4_validate.c  | 1 -
 6 files changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 9377e58f9bc256ea81e19cc442d04139feec20fe..2161761b1f2215c3ce7054caf21019d4be5c70d9 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -22,7 +22,6 @@
 #include <drm/drm_print.h>
 
 #include "vc4_drv.h"
-#include "uapi/drm/vc4_drm.h"
 
 static const struct drm_gem_object_funcs vc4_gem_object_funcs;
 
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index a14ecb7694613f15df3cfdb0195825d961fe1707..616caf9d9915b9ed7f614d6f0339fa6ee2cb80e3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -41,8 +41,6 @@
 
 #include <soc/bcm2835/raspberrypi-firmware.h>
 
-#include "uapi/drm/vc4_drm.h"
-
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index ab3c6d5d4eb446804877d6b8da01c3a9f65800df..e231c906709c1b15bd19e3f478ad86048700ba55 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -33,7 +33,6 @@
 #include <drm/drm_print.h>
 #include <drm/drm_syncobj.h>
 
-#include "uapi/drm/vc4_drm.h"
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 #include "vc4_trace.h"
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 91d499fefba2189131ff96270a9b001d694ab2ae..a81bad3519ccf3e8056a2ae37ddcfeccc2306a14 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -26,8 +26,6 @@
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_print.h>
 
-#include "uapi/drm/vc4_drm.h"
-
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c
index edc471e71c0eb1729c0dcf823ca6c9e0a7e207ed..c264d21bc3fe357ec28048760243b27b3dc26b1b 100644
--- a/drivers/gpu/drm/vc4/vc4_render_cl.c
+++ b/drivers/gpu/drm/vc4/vc4_render_cl.c
@@ -37,7 +37,6 @@
 
 #include <drm/drm_print.h>
 
-#include "uapi/drm/vc4_drm.h"
 #include "vc4_drv.h"
 #include "vc4_packet.h"
 
diff --git a/drivers/gpu/drm/vc4/vc4_validate.c b/drivers/gpu/drm/vc4/vc4_validate.c
index 545c4c3608f5683209ea1462d61d9d0dd68d0bd7..7f2fadfde7a8ee63a531a98474a8447b37faaac1 100644
--- a/drivers/gpu/drm/vc4/vc4_validate.c
+++ b/drivers/gpu/drm/vc4/vc4_validate.c
@@ -45,7 +45,6 @@
 
 #include <drm/drm_print.h>
 
-#include "uapi/drm/vc4_drm.h"
 #include "vc4_drv.h"
 #include "vc4_packet.h"
 

-- 
2.53.0

Claude review: drm/vc4: Miscellaneous fixes and improvements

[Claude Code Review Bot]

Overall Series Review

Subject: drm/vc4: Miscellaneous fixes and improvements
Author: =?utf-8?q?Ma=C3=ADra_Canal?= <mcanal@igalia.com>
Patches: 7
Reviewed: 2026-03-31T17:01:59.537429

---

This is a well-structured series of 6 patches from Maíra Canal addressing real bugs and performing minor cleanups in the VC4 DRM driver. Patches 1-4 are legitimate bugfixes (PM refcount leak, memory leaks, race condition), while patches 5-6 are cleanups. The patches are individually small, well-scoped, and correctly ordered. The series quality is good overall with one minor concern noted below.

**Recommendation: Series looks good for merging.** Patches 1-4 to drm-misc-fixes, patches 5-6 to drm-misc-next.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vc4: Release runtime PM reference after binding V3D

[Claude Code Review Bot]

Patch Review

**Verdict: Good fix.**

The `pm_runtime_resume_and_get()` at line 459 acquires a reference that is only released on the error path (`err_put_runtime_pm`) but never on the success path. Adding `pm_runtime_put_autosuspend(dev)` after configuring autosuspend is correct — the device will remain active for the 40ms autosuspend delay and then suspend if no other references are held.

```c
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, 40); /* a little over 2 frames. */
+pm_runtime_put_autosuspend(dev);
```

The placement after `pm_runtime_use_autosuspend()` and `pm_runtime_set_autosuspend_delay()` is correct — autosuspend must be configured before calling `put_autosuspend`. Already has Melissa's Reviewed-by.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vc4: Fix memory leak of BO array in hang state

[Claude Code Review Bot]

Patch Review

**Verdict: Good fix.**

The `state->bo` array is allocated via `kzalloc_objs()` in `vc4_save_hang_state()` but `vc4_free_hang_state()` only freed the individual BO references and the top-level `state` struct, leaking the `bo` array itself.

```c
for (i = 0; i < state->user_state.bo_count; i++)
    drm_gem_object_put(state->bo[i]);

+kfree(state->bo);
 kfree(state);
```

Straightforward and correct. The `kfree(state->bo)` must come after the loop that puts the BO references and before `kfree(state)`.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vc4: Fix a memory leak in hang state error path

[Claude Code Review Bot]

Patch Review

**Verdict: Good fix.**

Two early returns in `vc4_save_hang_state()` leaked the `kernel_state` allocation. The patch consolidates them into a single `err_free_state` label that unlocks the spinlock and frees `kernel_state`.

```c
-if (!exec[0] && !exec[1]) {
-    spin_unlock_irqrestore(&vc4->job_lock, irqflags);
-    return;
-}
+if (!exec[0] && !exec[1])
+    goto err_free_state;
```

And:
```c
+err_free_state:
+    spin_unlock_irqrestore(&vc4->job_lock, irqflags);
+    kfree(kernel_state);
```

Both error paths hold the spinlock, and the label correctly releases it before freeing. Note this patch depends on patch 2 being applied first (the `kfree(state->bo)` in `vc4_free_hang_state`), but the second early return (`!kernel_state->bo`) happens before `bo` is allocated, so `kfree(kernel_state)` alone is sufficient at the error label — `kernel_state->bo` is NULL in both error cases (either never assigned, or the allocation failed). Correct.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vc4: Protect madv read in vc4_gem_object_mmap() with madv_lock

[Claude Code Review Bot]

Patch Review

**Verdict: Mostly good, minor observation.**

The fix adds `mutex_lock(&bo->madv_lock)` around the `bo->madv` check to prevent a data race with `DRM_IOCTL_VC4_GEM_MADVISE`.

```c
+mutex_lock(&bo->madv_lock);
 if (bo->madv != VC4_MADV_WILLNEED) {
     ...
+    mutex_unlock(&bo->madv_lock);
     return -EINVAL;
 }
+mutex_unlock(&bo->madv_lock);
 
 return drm_gem_dma_mmap(&bo->base, vma);
```

There is a TOCTOU window here: `madv` could change between dropping the lock and calling `drm_gem_dma_mmap()`. However, this is a pre-existing design pattern in the driver — the same TOCTOU exists in other madv-checking paths, and the comment in `vc4_save_hang_state` acknowledges that BO consistency "cannot be guaranteed" when userspace acts concurrently. The fix is consistent with how the rest of the driver handles this, and closes the data race on the read itself, which is the actual bug (unsynchronized access to a shared variable).

No blocking issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vc4: Use devm_request_irq() for automatic cleanup

[Claude Code Review Bot]

Patch Review

**Verdict: Good cleanup, one behavioral note.**

The patch switches from `request_irq()`/`free_irq()` to `devm_request_irq()` and inlines the single-caller `vc4_irq_prepare()`.

**Behavioral change with `!vc4->v3d` check:** The old `vc4_irq_prepare()` would silently return if `!vc4->v3d`. The new code returns `-ENODEV` in that case. This is fine because the caller `vc4_v3d_bind()` only calls `vc4_irq_install()` after successfully setting up `vc4->v3d`, so this path should not be reached in normal operation. The explicit error return is actually better for catching programming errors.

**IRQ name change:** The patch changes the IRQ name from `dev->driver->name` to `dev_name(dev->dev)`. This is a cosmetic difference visible in `/proc/interrupts` but is the idiomatic approach when using devm APIs.

**Removal of `free_irq()` in `vc4_irq_uninstall()`:** Correct since `devm_request_irq()` handles cleanup automatically. The `vc4_irq_disable()` call remains to ensure interrupts are masked before teardown.

**Removed `#include <drm/drm_drv.h>`:** This was only needed for `dev->driver->name` which is no longer used. Fine.

No blocking issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vc4: Clean-up UAPI header inclusion

[Claude Code Review Bot]

Patch Review

**Verdict: Good cleanup.**

Removes redundant `#include "uapi/drm/vc4_drm.h"` from 6 files since it's already included transitively via `vc4_drv.h`. Straightforward and mechanical.

No issues.

---
Generated by Claude Code Patch Reviewer