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/a6xx+: Add support to configure perfcntrs Date: Mon, 25 May 2026 21:26:29 +1000 Message-ID: In-Reply-To: <20260520162454.18391-12-robin.clark@oss.qualcomm.com> References: <20260520162454.18391-1-robin.clark@oss.qualcomm.com> <20260520162454.18391-12-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 Patch Review The `a6xx_perfcntr_configure()` function writes SEL regs via the ringbuffer with pipe switching for BR/BV. The fence mechanism (`CP_MEM_WRITE` of `sel_fence` to `rbmemptrs->perfcntr_fence`) is a clean way to synchronize the sample_worker with SEL programming completion. One concern: the preempt_trigger at the end: ```c if (adreno_is_a8xx(to_adreno_gpu(gpu))) a8xx_preempt_trigger(gpu); else a6xx_preempt_trigger(gpu); ``` This runs unconditionally after configuring perfcntrs. Is it necessary? It seems like it's to kick the ringbuffer, but `a6xx_flush_yield()` already flushes. Possibly needed if preemption is pending, but worth a comment. --- Generated by Claude Code Patch Reviewer