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/adreno: add Adreno 810 GPU support Date: Tue, 05 May 2026 08:55:17 +1000 Message-ID: In-Reply-To: <20260502-adreno-810-v5-6-bc9fd2bb788d@pm.me> References: <20260502-adreno-810-v5-0-bc9fd2bb788d@pm.me> <20260502-adreno-810-v5-6-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: Looks good overall, a few observations** This is the main patch adding the A810 catalog entry with register tables. = Key observations: **Catalog entry structure:** ```c .chip_ids =3D ADRENO_CHIP_IDS(0x44010000), .family =3D ADRENO_8XX_GEN1, .fw =3D { [ADRENO_FW_SQE] =3D "gen80300_sqe.fw", [ADRENO_FW_GMU] =3D "gen80300_gmu.bin", }, .gmem =3D SZ_512K + SZ_64K, ``` - Family is `ADRENO_8XX_GEN1` (a830 family) =E2=80=94 this makes sense as A= 810 is the "first GPU to be released in the A8x family" per the cover lette= r, so it's in the earlier gen. - The existing A8xx GPUs (x285, a840) are `ADRENO_8XX_GEN2` with massive GM= EM (18-21MB). A810 has only 576KB =E2=80=94 much smaller, consistent with b= eing a lower-tier part. - Firmware is `gen80300` =E2=80=94 distinct from x285's `gen80100` and a840= 's `gen80200`. - No `[ADRENO_FW_AQE]` entry unlike A840, which is fine for a simpler GPU. **Quirks:** ```c .quirks =3D ADRENO_QUIRK_HAS_CACHED_COHERENT | ADRENO_QUIRK_HAS_HW_APRIV | ADRENO_QUIRK_PREEMPTION | ADRENO_QUIRK_IFPC, ``` This matches A840's quirk set. No `ADRENO_QUIRK_SOFTFUSE` (unlike x285), wh= ich means speedbin reads will use the nvmem fuse path rather than the cx_mi= sc softfuse path. **Protect table:** `DECLARE_ADRENO_PROTECT(a810_protect, 64)` =E2=80=94 47 entries out of 64 m= ax global slots. The `__build_asserts` check is properly added. **Register tables:** The `a810_nonctxt_regs`, `a810_pwrup_reglist_regs`, `a810_ifpc_reglist_regs= `, and `a810_dyn_pwrup_reglist_regs` are all A810-specific, which is expect= ed as register values differ between GPU generations. **`adreno_is_a810()` helper:** ```c +static inline int adreno_is_a810(struct adreno_gpu *gpu) +{ + return gpu->info->chip_ids[0] =3D=3D 0x44010000; +} ``` This is added but I don't see it used in this series. It follows the existi= ng pattern (e.g., `adreno_is_x285()`). Worth confirming it's needed =E2=80= =94 it may be used by future patches or by the dependent series. **`a810_dyn_pwrup_reglist_regs` includes `REG_A8XX_CP_PROTECT_PIPE(15)`:** ```c { REG_A8XX_CP_PROTECT_PIPE(15), 0, BIT(PIPE_BR) | BIT(PIPE_BV) }, ``` This is consistent with the hardcoded slot 15 from patch 5. **Minor**: The `a810_nonctxt_regs` has a comment `/* Avoid partial waves at= VFD */` on the BV-specific `PC_CHICKEN_BITS_1` entry, while the BR entry h= as no comment. This is fine as it explains why the BV value differs (0x23 v= s 0x03). **Observation on `max_slices =3D 1`**: A810 is a single-slice GPU, compared= to x285 (4 slices) and a840 (3 slices). This is consistent with it being a= lower-tier part. No blocking issues found. --- --- Generated by Claude Code Patch Reviewer