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/panfrost: Check another bo field for cache option query Date: Fri, 05 Jun 2026 06:16:23 +1000 Message-ID: In-Reply-To: <20260604-claude-fixes-v2-1-57c6bd4c1655@collabora.com> References: <20260604-claude-fixes-v2-0-57c6bd4c1655@collabora.com> <20260604-claude-fixes-v2-1-57c6bd4c1655@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 **Verdict: Correct fix, but commit message could be clearer.** The existing code: ```c if (!bo->base.map_wc) args->create_flags |=3D PANFROST_BO_WB_MMAP; ``` This is indeed wrong. Looking at the source tree, `map_wc` is set by `shoul= d_map_wc()` which returns `false` for both coherent devices (where WC is im= possible) AND for `wb_mmap` BOs. So on a coherent device, `!map_wc` is alwa= ys true, meaning every BO would erroneously report `PANFROST_BO_WB_MMAP`. T= he fix to use `bo->wb_mmap` directly is correct =E2=80=94 it reflects the a= ctual creation flag: ```c if (bo->wb_mmap) args->create_flags |=3D PANFROST_BO_WB_MMAP; ``` **Nit:** The commit message says "is recorded in a Panfrost BO's wb_mmap fi= eld" but doesn't explain *why* `!map_wc` was wrong. On a coherent device, `= map_wc` is always false (line 460 in `panfrost_gem.c`), so `!map_wc` is alw= ays true =E2=80=94 the old code would report all BOs as `WB_MMAP` on cohere= nt devices. Explaining this would help reviewers. --- --- Generated by Claude Code Patch Reviewer