Claude review: drm/v3d: Skip CSD when it has zeroed workgroups

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Status: Good, with one minor style observation.**

This patch addresses two things:

**Part A — Always rewrite CFG[0..2] from the indirect buffer.** In the rewrite function, the cfg writes are now done unconditionally *before* the zero check:

```c
 	args->cfg[0] = wg_counts[0] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[1] = wg_counts[1] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[2] = wg_counts[2] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 
+	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
+		goto unmap_bo;
```

This is the v3→v4 improvement. It ensures CFG[0..2] always reflect the actual indirect buffer contents rather than preserving stale userspace values. The `goto unmap_bo` still correctly skips the `num_batches` calculation, which would produce `0 - 1 = 0xFFFFFFFF` (triggering the existing `WARN_ON(args->cfg[4] == ~0)`) if any count is zero.

**Part B — Skip hardware dispatch for zero workgroups.** In `v3d_csd_job_run()`:

```c
+	if (!V3D_GET_FIELD(job->args.cfg[0], V3D_CSD_QUEUED_CFG0_NUM_WGS_X) ||
+	    !V3D_GET_FIELD(job->args.cfg[1], V3D_CSD_QUEUED_CFG1_NUM_WGS_Y) ||
+	    !V3D_GET_FIELD(job->args.cfg[2], V3D_CSD_QUEUED_CFG2_NUM_WGS_Z))
+		return NULL;
```

Using `V3D_GET_FIELD` with the proper register field macros is the right approach — it extracts bits [31:16] from each CFG register, which is exactly where the workgroup counts live (confirmed: `V3D_CSD_QUEUED_CFG0_NUM_WGS_X_SHIFT = 16` in `v3d_regs.h` matches `V3D_CSD_CFG012_WG_COUNT_SHIFT = 16`). This works for both indirect and direct CSD submissions. The comment explaining the hardware's 0→65536 interpretation is useful.

**Minor observation:** The existing error path in this function does `v3d->queue[V3D_CSD].active_job = NULL` before returning NULL:

```c
	if (unlikely(job->base.base.s_fence->finished.error)) {
		v3d->queue[V3D_CSD].active_job = NULL;
		return NULL;
	}
```

The new zero-workgroup path returns NULL without this. Since `active_job` is set only on the *next* line (`v3d->queue[V3D_CSD].active_job = &job->base`), the value at this point should already be NULL from the previous job's IRQ handler, so this is not a correctness issue. However, for consistency with the existing error path and defensive coding, adding `v3d->queue[V3D_CSD].active_job = NULL;` before the return would be slightly cleaner. Not a blocker.

No other issues. The Fixes tag (`d223f98f0209`) correctly references the original CSD dispatch support, as the zero-workgroup check belongs there at the hardware-facing level.

---
Generated by Claude Code Patch Reviewer