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: Wed, 27 May 2026 15:22:49 +1000 Message-ID: In-Reply-To: <20260526-claude-fixes-v1-1-16e92eaa4949@collabora.com> References: <20260526-claude-fixes-v1-0-16e92eaa4949@collabora.com> <20260526-claude-fixes-v1-1-16e92eaa4949@collabora.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Status: Looks correct, good fix.** The change in `panfrost_drv.c`: ```c - if (!bo->base.map_wc) + if (bo->wb_mmap) ``` The old code infers WB_MMAP by checking `!map_wc`. But `map_wc` is derived from *two* inputs, as seen in `panfrost_gem.c:should_map_wc()`: ```c if (pfdev->coherent) return false; if (bo->wb_mmap) return false; return true; ``` On a coherent device, `map_wc` is always `false` regardless of `wb_mmap`. So `!bo->base.map_wc` would be `true` for *every* BO on a coherent device, incorrectly reporting `PANFROST_BO_WB_MMAP` for BOs that were never created with that flag. The fix to check `bo->wb_mmap` directly is the right thing to do -- it queries the user's original intent rather than a derived value. **Minor**: The commit message could be clearer about *why* `map_wc` is wrong -- specifically mentioning the coherent-device case would help reviewers understand the concrete bug. --- --- Generated by Claude Code Patch Reviewer