Claude review: drm/v3d: Allocate all resources before enabling the clock

Claude Code Review Bot
2026-03-31T21:45:10Z

Patch Review

**Status: Looks good with minor notes.**

This patch reorders probe so that all resource allocations (register mappings, reset control, clock lookup, IRQ setup, perfmon init, DMA allocation, gem init) happen before `clk_prepare_enable()`. This is a prerequisite for patch 3 where the clock enable moves into the runtime PM resume callback.

Key observations:

1. **IRQ init vs. IRQ enable separation**: The patch splits `v3d_irq_init()` — it moves the interrupt clearing and `v3d_irq_enable()` call out of `v3d_irq_init()` into a new explicit `v3d_irq_enable()` call after the clock is enabled. The clearing of pending interrupts is moved into `v3d_irq_enable()`. This is correct since you can't touch HW registers before the clock is on, while `request_irq()` itself doesn't need the HW powered.

2. **`v3d_init_hw_state()` made non-static**: The function is moved in `v3d_gem.c` and its declaration is added to `v3d_drv.h`. There's a spurious extra blank line after the function definition:

```c
+void
+v3d_init_hw_state(struct v3d_dev *v3d)
+{
+ v3d_init_core(v3d, 0);
+}
+
+
static void
```

Very minor formatting nit — two blank lines instead of one.

3. **Remove order in `v3d_platform_drm_remove()`**: The patch correctly reorders removal to power off SMS and disable clock *before* destroying gem and freeing DMA — the reverse of the new probe order. Good.

4. **Error path labels**: The error cleanup path now has `clk_disable` placed between `irq_disable` and `gem_destroy`, matching the reverse allocation order. Correct.