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: Expose GPU page sizes to UM Date: Sat, 14 Mar 2026 06:48:44 +1000 Message-ID: In-Reply-To: <20260313150956.1618635-2-adrian.larumbe@collabora.com> References: <20260313150956.1618635-1-adrian.larumbe@collabora.com> <20260313150956.1618635-2-adrian.larumbe@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 **UAPI concern: enum value insertion breaks ABI.** `DRM_PANTHOR_DEV_QUERY_MMU_INFO` is inserted *between* `DRM_PANTHOR_DEV_QUERY_CSIF_INFO` and `DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO` in the enum. Since these are auto-numbered C enum values, this shifts the values of `DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO` and `DRM_PANTHOR_DEV_QUERY_GROUP_PRIORITIES_INFO`, breaking existing userspace compiled against the old header. The new query should be added at the end of the enum (before any terminator/max, if any), or should use an explicit assigned value. ```c + /** @DRM_PANTHOR_DEV_QUERY_MMU_INFO: Query MMU information. */ + DRM_PANTHOR_DEV_QUERY_MMU_INFO, + /** @DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO: Query timestamp information. */ DRM_PANTHOR_DEV_QUERY_TIMESTAMP_INFO, ``` Similarly, in `panthor_drv.c`, the `DRM_PANTHOR_DEV_QUERY_MMU_INFO` case is inserted in the middle of the switch. While this is cosmetic and functionally fine (switch doesn't depend on ordering), it should match wherever the enum ends up. The struct `drm_panthor_mmu_info` has only a single `__u64` member. This is fine for now but consider whether any padding field is needed for future extensibility (it's a single u64 so no alignment issues). Otherwise the implementation is clean -- extracting the hardcoded `SZ_4K | SZ_2M` into a queryable parameter is a reasonable change. --- Generated by Claude Code Patch Reviewer