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: Append SEL regs to dyn pwrup reglist Date: Sat, 16 May 2026 15:20:25 +1000 Message-ID: In-Reply-To: <20260511130017.96867-16-robin.clark@oss.qualcomm.com> References: <20260511130017.96867-1-robin.clark@oss.qualcomm.com> <20260511130017.96867-16-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 Changes `cpu_gpu_lock::regs` from fixed `uint64_t regs[62]` to flex array `uint64_t regs[]`. Combined with patch 14's buffer size increase, this allows significantly more register entries. The `perfcntr_select()` helper writes to both the ringbuffer (for immediate effect) and the pwrup reglist (for IFPC restore). The memory barrier sequencing looks correct: ```c /* Clear entries, update dynamic_list_len down first: */ lock->dynamic_list_len = a6xx_gpu->dynamic_sel_reglist_offset; wmb(); /* ... write new entries ... */ wmb(); /* Then update dynamic_list_len up: */ lock->dynamic_list_len = ... + nr_regs; ``` The comment about skipping the spinlock dance with SQE (unlike kgsl) is well-reasoned. One concern: the `regs[]` flex array change means `sizeof(struct cpu_gpu_lock)` drops from 512 to 16 bytes. Verify no code uses `sizeof(struct cpu_gpu_lock)` for allocation or offset calculations. --- Generated by Claude Code Patch Reviewer