From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/v3d: Skip CSD when it has zeroed workgroups Date: Thu, 04 Jun 2026 12:49:58 +1000 Message-ID: In-Reply-To: <20260602-v3d-fix-indirect-csd-v3-2-cc79e06e543c@igalia.com> References: <20260602-v3d-fix-indirect-csd-v3-0-cc79e06e543c@igalia.com> <20260602-v3d-fix-indirect-csd-v3-2-cc79e06e543c@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Status: Good** This patch adds a check in `v3d_csd_job_run()` to skip hardware submission = when any workgroup dimension is zero. The check is placed after the existin= g error check and before setting `active_job`: ```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; ``` The use of `V3D_GET_FIELD` with the proper field macros (bits [31:16] per `= v3d_regs.h:416-429`) is the right approach, extracting only the workgroup c= ount bits rather than checking the whole register value. This correctly han= dles `cfg[]` values that might have non-zero bits in the lower half but a z= ero workgroup count in the upper half. The comment explaining the hardware behavior (0 is interpreted as 65536) is= accurate and provides good justification for why this check is needed. The placement before `v3d->queue[V3D_CSD].active_job =3D &job->base` is cor= rect =E2=80=94 by returning `NULL` before setting `active_job`, the functio= n avoids leaving the queue in a state where it thinks a job is active but n= o fence was created and no hardware submission occurred. This matches the e= xisting pattern used by the error-path check above it (lines 350-353 in the= current tree). The `Fixes` tag references the original CSD support commit, which is approp= riate since the zero-workgroup case was always a latent bug =E2=80=94 it ju= st wasn't reachable until indirect CSD was added. --- Generated by Claude Code Patch Reviewer