[PATCH] drm/imagination: Fix deadlock in soft reset sequence

[PATCH] drm/imagination: Fix deadlock in soft reset sequence

[Alessio Belle]

The soft reset sequence is currently executed from the threaded IRQ
handler, hence it cannot call disable_irq() which internally waits
for IRQ handlers, i.e. itself, to complete.

Use disable_irq_nosync() during a soft reset instead.

Fixes: cc1aeedb98ad ("drm/imagination: Implement firmware infrastructure and META FW support")
Cc: stable@vger.kernel.org
Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_power.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index 7a8765c0c1ed..046cce76498a 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -510,7 +510,16 @@ pvr_power_reset(struct pvr_device *pvr_dev, bool hard_reset)
 	}
 
 	/* Disable IRQs for the duration of the reset. */
-	disable_irq(pvr_dev->irq);
+	if (hard_reset) {
+		disable_irq(pvr_dev->irq);
+	} else {
+		/*
+		 * Soft reset is triggered as a response to a FW command to the Host and is
+		 * processed from the threaded IRQ handler. This code cannot (nor needs to)
+		 * wait for any IRQ processing to complete.
+		 */
+		disable_irq_nosync(pvr_dev->irq);
+	}
 
 	do {
 		if (hard_reset) {

---
base-commit: d2e20c8951e4bb5f4a828aed39813599980353b6
change-id: 20260309-fix-soft-reset-8f32c3783d3d

Best regards,
-- 
Alessio Belle <alessio.belle@imgtec.com>

Claude review: drm/imagination: Fix deadlock in soft reset sequence

[Claude Code Review Bot]

Overall Series Review

Subject: drm/imagination: Fix deadlock in soft reset sequence
Author: Alessio Belle <alessio.belle@imgtec.com>
Patches: 1
Reviewed: 2026-03-10T12:12:14.321041

---

This is a single-patch fix for a real deadlock in the PowerVR (Imagination) DRM driver. The analysis is sound: `pvr_power_reset(pvr_dev, false)` (soft reset) is called from `process_fwccb_command()` → `pvr_fwccb_process()` → `pvr_device_irq_thread_handler()`, which is the threaded IRQ handler. Calling `disable_irq()` from within that handler will deadlock because `disable_irq()` waits (via `synchronize_irq()`) for any running threaded handler to complete — i.e., it waits for itself.

The fix is correct and minimal: use `disable_irq_nosync()` for the soft reset path, which disables the IRQ without waiting for the current handler to finish. This is safe because we are *already inside* the only handler that could be running.

The patch is well-tagged with `Fixes:` and `Cc: stable`, appropriate for a deadlock fix.

**Verdict: This patch looks good and is ready to merge.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/imagination: Fix deadlock in soft reset sequence

[Claude Code Review Bot]

Patch Review

**Correctness: Good.**

The call chain is confirmed:
1. `pvr_device_irq_thread_handler()` (`pvr_device.c:203`) — threaded IRQ handler
2. → `pvr_fwccb_process()` (`pvr_device.c:217`)
3. → `process_fwccb_command()` (`pvr_ccb.c:196`)
4. → `pvr_power_reset(pvr_dev, false)` (`pvr_ccb.c:149`)

Inside `pvr_power_reset`, the original code unconditionally called `disable_irq()`, which invokes `synchronize_irq()` internally, deadlocking because the threaded handler is waiting for itself to complete.

The fix:
```c
if (hard_reset) {
    disable_irq(pvr_dev->irq);
} else {
    /*
     * Soft reset is triggered as a response to a FW command to the Host and is
     * processed from the threaded IRQ handler. This code cannot (nor needs to)
     * wait for any IRQ processing to complete.
     */
    disable_irq_nosync(pvr_dev->irq);
}
```

This is correct. `disable_irq_nosync()` still increments the IRQ disable depth counter, preventing new IRQs from being delivered, but doesn't call `synchronize_irq()`. Since we're already running *as* the threaded handler, there's no concurrent IRQ processing to wait for.

**Soft-to-hard escalation path: Correct.** When a soft reset fails (`pvr_power.c:559-562`), the code sets `hard_reset = true` and loops. The IRQ is already disabled (via `_nosync`), and the loop proceeds to do the hard reset with IRQs still disabled. At completion, `enable_irq()` at line 568 re-enables correctly regardless of which `disable_irq` variant was used, since both increment the same disable depth counter.

**Comment quality: Good.** The comment clearly explains why `disable_irq_nosync()` is used and why it's safe.

**Minor observation (non-blocking):** The hard reset callers (`pvr_power.c:209` in the watchdog, `pvr_mmu.c:169`) appear to run from non-IRQ context, so `disable_irq()` remains correct for that path.

No issues found. The patch is clean, minimal, and correct.

---
Generated by Claude Code Patch Reviewer