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

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

Patch Review

**Assessment: Correct in principle, one minor concern.**

The core idea is sound. The V3D CSD hardware treats a zero workgroup count as 65536, so submitting a zero-count dispatch causes an illegitimate execution. Returning `NULL` from `run_job` is the correct mechanism — the DRM scheduler treats a `NULL` return as "job completed synchronously with success" (calls `drm_sched_job_done(sched_job, 0)`), which is the right semantic for a no-op dispatch. No fences, perf counters, or IRQ infrastructure are set up, which is all appropriate since no hardware work occurs.

The placement of the check is correct — it's after the error-fence check but before `active_job` is set and before any hardware interaction:

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

if (!job->args.cfg[0] || !job->args.cfg[1] || !job->args.cfg[2])
    return NULL;

v3d->queue[V3D_CSD].active_job = &job->base;
```

**Minor concern — cfg[0..2] may contain more than the workgroup count.** Since `V3D_CSD_CFG012_WG_COUNT_SHIFT` is 16, the workgroup count occupies bits [31:16] of each cfg register. The check `!job->args.cfg[0]` tests the entire 32-bit value, not just the count field. This works for the indirect CSD path because `v3d_rewrite_csd_job_wg_counts_from_indirect()` does a full assignment (`args->cfg[0] = wg_counts[0] << 16`), so the lower 16 bits are always zero after rewrite. And in the zero-count case (with patch 1), the rewrite is skipped entirely, so cfg[0..2] retain their userspace-provided values — which must be zero-initialized for this check to trigger.

This is presumably fine because Mesa would initialize cfg[0..2] to zero for indirect CSD (the real values come from the indirect buffer), but it might be worth a brief comment or, alternatively, masking to check only the count field explicitly:

```c
#define V3D_CSD_CFG012_WG_COUNT_MASK  GENMASK(31, V3D_CSD_CFG012_WG_COUNT_SHIFT)

if (!(job->args.cfg[0] & V3D_CSD_CFG012_WG_COUNT_MASK) || ...)
```

This would make the check robust regardless of what userspace puts in the lower bits. That said, given the rewrite function's full assignment behavior, the current check works correctly in practice.

**Nit — `Fixes:` tag.** The tag references `d223f98f0209 ("drm/v3d: Add support for compute shader dispatch.")` (the original CSD commit), while the actual trigger scenario (zero counts from an indirect buffer) was only introduced later with indirect CSD support (`18b8413b25b7`). The Fixes tag is defensible since the hardware's 0-as-65536 behavior was always a latent issue, but it does stretch the stable backport scope. Not wrong, just worth noting.

---
Generated by Claude Code Patch Reviewer