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: set cx_misc_mmio regardless of if platform has LLCC Date: Tue, 05 May 2026 08:55:16 +1000 Message-ID: In-Reply-To: <20260502-adreno-810-v5-4-bc9fd2bb788d@pm.me> References: <20260502-adreno-810-v5-0-bc9fd2bb788d@pm.me> <20260502-adreno-810-v5-4-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: Mostly good, one observation** This is the key architectural fix. Previously, `a6xx_llc_slices_init()` did= three things: 1. Detected MMU500 2. Mapped `cx_misc_mmio` 3. Got LLCC slices 4. Set `cx_misc_mmio =3D ERR_PTR(-EINVAL)` if no LLCC slices available Step 4 was the problem: on platforms like Milos that have no LLCC, `cx_misc= _mmio` was clobbered with an error pointer, causing crashes when the GMU la= ter read TCM retention or software fuses via `cx_misc_read()`. The fix moves the MMU500 detection and `cx_misc_mmio` mapping into `a6xx_gp= u_init()`, after `a6xx_llc_slices_init()`, so the MMIO mapping is independe= nt of LLCC availability. The early-return guards in `a6xx_llc_activate()` a= nd `a7xx_llc_activate()` are changed from checking `IS_ERR(cx_misc_mmio)` t= o checking whether both LLCC slices are unavailable =E2=80=94 which is the = actual condition that matters for LLC activation. ```c - if (IS_ERR(a6xx_gpu->cx_misc_mmio)) + if (IS_ERR_OR_NULL(a6xx_gpu->llc_slice) && IS_ERR_OR_NULL(a6xx_gpu->htw_l= lc_slice)) return; ``` This is correct =E2=80=94 if there are no LLCC slices to activate, there's = nothing for the LLC activation functions to do. The `cx_misc_mmio` validity= is now a separate concern. **Minor observation**: After this patch, `cx_misc_mmio` could potentially b= e an `ERR_PTR` if `msm_ioremap()` fails, but the LLC activate functions no = longer check for that before the LLCC slice activate calls (which don't use= `cx_misc_mmio`). In `a6xx_llc_activate`, the `cx_misc_write/rmw` calls hap= pen later only in the `!have_mmu500` path, and `a7xx_llc_activate` doesn't = use cx_misc at all. For A8xx, `a8xx_llc_activate` also doesn't use cx_misc.= So this is safe, but callers of `a6xx_cx_misc_read/write` elsewhere (GMU c= ode for TCM retention, fuses, speedbin) should be careful =E2=80=94 a faile= d ioremap would cause a crash. In practice, if `cx_mem` is missing from the= DT, the GPU would likely fail earlier, so this is acceptable. Has `Reviewed-by` from Konrad Dybcio and Akhil P Oommen. --- --- Generated by Claude Code Patch Reviewer