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: Allow IFPC with perfcntr stream Date: Tue, 05 May 2026 08:06:04 +1000 Message-ID: In-Reply-To: <20260504190751.61052-17-robin.clark@oss.qualcomm.com> References: <20260504190751.61052-1-robin.clark@oss.qualcomm.com> <20260504190751.61052-17-robin.clark@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review Changes `sysprof_setup()` signature to add `bool force_on`, then uses it in= `sample_worker` to inhibit IFPC while reading counter registers and re-ena= ble it afterwards. The `msm_gpu_sysprof_no_ifpc()` is decoupled from `msm_gpu_sysprof_no_perfc= ntr_zap()`: ```c static inline bool msm_gpu_sysprof_no_ifpc(struct msm_gpu *gpu) { return refcount_read(&gpu->sysprof_active) > 1; } ``` This means IFPC is now only disabled for legacy userspace SYSPROF usage, no= t for the new kernel-managed perfcntr stream. The perfcntr stream instead b= riefly inhibits IFPC during sampling via `sysprof_setup(gpu, true/false)`. = This is correct =E2=80=94 the pwrup reglist (patch 15) handles restoring SE= L registers on IFPC exit. **Minor: bitwise OR vs logical OR.** ```c bool sysprof =3D msm_gpu_sysprof_no_ifpc(gpu) | force_on; ``` This uses bitwise OR (`|`) on bools. While functionally correct, `||` would= be more conventional for boolean operands. Very minor. **Minor: diff artifact.** There appears to be a whitespace-damaged comment = in the `msm_gpu_sysprof_no_ifpc()` diff (leading `+` on the comment line th= at was part of the removed block). This may just be a display artifact from= the mbox formatting. --- Generated by Claude Code Patch Reviewer