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 sysprof accessors Date: Thu, 23 Apr 2026 09:13:15 +1000 Message-ID: In-Reply-To: <20260420222621.417276-8-robin.clark@oss.qualcomm.com> References: <20260420222621.417276-1-robin.clark@oss.qualcomm.com> <20260420222621.417276-8-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 Good refactor splitting the raw `refcount_read()` checks into two semantically distinct accessors: `msm_gpu_sysprof_no_perfcntr_zap()` and `msm_gpu_sysprof_no_ifpc()`. The comment on `msm_gpu_sysprof_no_ifpc()` clearly explains the planned future divergence. **Issue**: In `a8xx_gpu.c` `hw_init()`, the change uses `msm_gpu_sysprof_no_perfcntr_zap()`: ```c if (!ret && msm_gpu_sysprof_no_perfcntr_zap(gpu)) { ret = a6xx_gmu_set_oob(gmu, GMU_OOB_PERFCOUNTER_SET); ``` But the `a6xx_gpu.c` `hw_init()` uses `msm_gpu_sysprof_no_ifpc(gpu)` for the same `GMU_OOB_PERFCOUNTER_SET` call. Since this is about disabling IFPC, the a8xx path should also use `msm_gpu_sysprof_no_ifpc()` for consistency, even though the two are currently equivalent. Using the wrong accessor here means when the planned divergence happens, the a8xx path won't be updated correctly. Also: adding `.sysprof_setup = a6xx_gmu_sysprof_setup` to `a8xx_gpu_funcs` seems like a bug fix that was previously missing -- worth mentioning in the commit message. --- Generated by Claude Code Patch Reviewer