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/panthor: Part ways with drm_gem_shmem_object Date: Tue, 31 Mar 2026 17:24:46 +1000 Message-ID: In-Reply-To: <20260330094848.2169422-6-boris.brezillon@collabora.com> References: <20260330094848.2169422-1-boris.brezillon@collabora.com> <20260330094848.2169422-6-boris.brezillon@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 This is the largest structural change. The fork from `drm_gem_shmem_object` to a custom `drm_gem_object` with manual page/sgt/vmap management is the key enabler for the shrinker. A few observations: 1. **`panthor_gem_create` uses `uint32_t`** in its signature. The v6 changelog says `s/uint32_t/u32/`, but the function signature still has `uint32_t flags`: ```c static struct panthor_gem_object * panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags, ``` This should be `u32 flags` for consistency with kernel style, though it's cosmetic. 2. **`panthor_gem_any_fault` in patch 5** uses `dma_resv_lock()` (blocking) in the fault handler. This is changed in patch 6 to a non-blocking trylock approach. As a standalone patch, blocking in the fault handler while holding the mmap lock could be problematic, but since patches 5 and 6 are applied together, this is a transient state. 3. **`panthor_gem_vm_open` WARN_ON** checks `!bo->backing.pages` - correct for patch 5, and properly removed in patch 6 when lazy allocation is added. 4. The `panthor_gem_free_object` acquires `dma_resv_lock` to clean up. This is the standard pattern. No issues. --- Generated by Claude Code Patch Reviewer