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/msm: Add PERFCNTR_CONFIG ioctl Date: Tue, 05 May 2026 08:06:01 +1000 Message-ID: In-Reply-To: <20260504190751.61052-1-robin.clark@oss.qualcomm.com> References: <20260504190751.61052-1-robin.clark@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: drm/msm: Add PERFCNTR_CONFIG ioctl Author: Rob Clark Patches: 16 Reviewed: 2026-05-05T08:06:01.354257 --- This is a well-structured 16-patch series from Rob Clark adding a new `PERFCNTR_CONFIG` ioctl to the MSM/Adreno DRM driver. The series cleanly replaces the old debugfs-based performance counter infrastructure with a proper UAPI for both global (kernel-sampled) and local (per-cmdstream, UMD-managed) counter collection. The architecture is sound: global counter streaming via an fd with circular buffer, local reservations tracked per-context, conflict detection between the two modes, and proper IFPC handling via the pwrup reglist for a7xx+. The code is v3 and has already had some review (Dmitry's R-b on patches 1-2). The series is generally in good shape, but I have a few issues ranging from a behavioral change introduced by a refactoring to some locking/error-path concerns. **Key concerns:** 1. The `a6xx_flush_yield` refactoring silently changes behavior in the `preempt_start` paths (dword[3] changes from 0x00 to 0x01). 2. The `nr_regs` calculation in patch 15 has the subtraction operands reversed. 3. `guard(pm_runtime_active_auto)` does not check the return value of `pm_runtime_get_sync()`, which can fail. 4. The IOCTL has no upper bound on `bufsz_shift`, allowing potentially very large kernel allocations. 5. Missing `NULL` check on `gpu->perfcntrs` in `msm_gpu_sysprof_no_perfcntr_zap()` before the series finishes initializing perfcntrs for all GPU generations. --- --- Generated by Claude Code Patch Reviewer