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/msm/a8xx: use pipe protect slot 15 for last-span-unbound feature Date: Tue, 05 May 2026 08:55:17 +1000 Message-ID: In-Reply-To: <20260502-adreno-810-v5-5-bc9fd2bb788d@pm.me> References: <20260502-adreno-810-v5-0-bc9fd2bb788d@pm.me> <20260502-adreno-810-v5-5-bc9fd2bb788d@pm.me> 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: Correct fix, but deserves a note about the underlying issue** The existing code uses `protect->count_max` as the pipe protect slot index: ```c a8xx_write_pipe(gpu, PIPE_BR, REG_A8XX_CP_PROTECT_PIPE(protect->count_max),= final_cfg); ``` For the existing A8xx GPUs (x285 and a840), `count_max` happens to be 15 (v= ia `DECLARE_ADRENO_PROTECT(..., 15)`). But for A810, `count_max` is 64 beca= use A810 has 64 global protect slots. Since pipe protect only has 16 slots = (0-15), using `count_max=3D64` as a pipe slot index would be out of bounds. The fix hardcodes slot 15, matching the downstream driver. This is correct = behavior =E2=80=94 the last-span-unbound feature should always target the l= ast pipe slot. ```c + a8xx_write_pipe(gpu, PIPE_BR, REG_A8XX_CP_PROTECT_PIPE(15), final_cfg); + a8xx_write_pipe(gpu, PIPE_BV, REG_A8XX_CP_PROTECT_PIPE(15), final_cfg); ``` **Suggestion**: This patch should ideally come *before* patch 6 (which intr= oduces `DECLARE_ADRENO_PROTECT(a810_protect, 64)`) to avoid any window wher= e the code is broken =E2=80=94 though since these patches are applied as a = series, bisectability is maintained because the A810 chip ID won't match on= any hardware until patch 6 adds the catalog entry. **Design note**: The fact that `count_max` was accidentally overloaded with= two meanings (global slot capacity AND pipe slot index) is a latent bug. I= t might be worth a follow-up to add a `#define A8XX_PIPE_PROTECT_LAST_SLOT = 15` or similar, and perhaps add a comment in `DECLARE_ADRENO_PROTECT` notin= g that `count_max` for A8xx is the global protect count, not the pipe slot.= But that's cleanup, not a blocker. Has `Reviewed-by` from Konrad Dybcio and Akhil P Oommen. --- --- Generated by Claude Code Patch Reviewer