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/tyr: Set interconnect coherency during probe Date: Fri, 13 Mar 2026 14:38:51 +1000 Message-ID: In-Reply-To: <20260311-b4-tyr-use-register-macro-v2-v2-2-b936d9eb8f51@collabora.com> References: <20260311-b4-tyr-use-register-macro-v2-v2-0-b936d9eb8f51@collabora.com> <20260311-b4-tyr-use-register-macro-v2-v2-2-b936d9eb8f51@collabora.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 This patch writes `NO_COHERENCY` to the `COHERENCY_ENABLE` register during = probe, then reads it back for `GpuInfo`. The change is sound in principle =E2=80=94 it ensures the register is in a = known state and the `GpuInfo` struct reflects reality. However: **Potential issue with `into_raw()` for `selected_coherency`:** ```rust selected_coherency: io.read(COHERENCY_ENABLE).into_raw(), ``` The `COHERENCY_ENABLE` register has a single field `l2_cache_protocol_selec= t` spanning bits 31:0. Reading the full raw value should work since the fie= ld spans the whole register. But the UAPI constants (`DRM_PANTHOR_GPU_COHER= ENCY_NONE` =3D 0, `ACE_LITE` =3D 1, `ACE` =3D 2) are small values, so this = should be fine. **The `with_const_l2_cache_protocol_select` call is verbose but correct.** = Writing `NO_COHERENCY` (0) to a zeroed register is effectively a no-op writ= e, but it serves the purpose of establishing a known state. --- Generated by Claude Code Patch Reviewer