[PATCH] drm/msm/a6xx: Restore sysprof_active

[PATCH] drm/msm/a6xx: Restore sysprof_active

[Rob Clark]

This got lost in the shuffle somehow when moving the vfunc table to
catalogue.  Fixes inhibiting IFPC when userspace is collecting perfcntr
data.

Fixes: 491fadb2b818 ("drm/msm/adreno: Move adreno_gpu_func to catalogue")
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 85607706779b..14b4f4921991 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2751,6 +2751,7 @@ const struct adreno_gpu_funcs a6xx_gpu_funcs = {
 		.create_private_vm = a6xx_create_private_vm,
 		.get_rptr = a6xx_get_rptr,
 		.progress = a6xx_progress,
+		.sysprof_setup = a6xx_gmu_sysprof_setup,
 	},
 	.init = a6xx_gpu_init,
 	.get_timestamp = a6xx_gmu_get_timestamp,
@@ -2819,6 +2820,7 @@ const struct adreno_gpu_funcs a7xx_gpu_funcs = {
 		.create_private_vm = a6xx_create_private_vm,
 		.get_rptr = a6xx_get_rptr,
 		.progress = a6xx_progress,
+		.sysprof_setup = a6xx_gmu_sysprof_setup,
 	},
 	.init = a6xx_gpu_init,
 	.get_timestamp = a6xx_gmu_get_timestamp,
-- 
2.53.0

Re: [PATCH] drm/msm/a6xx: Restore sysprof_active

[Akhil P Oommen]

On 4/11/2026 8:33 PM, Rob Clark wrote:
> This got lost in the shuffle somehow when moving the vfunc table to
> catalogue.  Fixes inhibiting IFPC when userspace is collecting perfcntr
> data.
> 
> Fixes: 491fadb2b818 ("drm/msm/adreno: Move adreno_gpu_func to catalogue")
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>

Looks like this got messed up during a merge conflict resolution. :(

Reviewed-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>

-Akhil.

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 85607706779b..14b4f4921991 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2751,6 +2751,7 @@ const struct adreno_gpu_funcs a6xx_gpu_funcs = {
>  		.create_private_vm = a6xx_create_private_vm,
>  		.get_rptr = a6xx_get_rptr,
>  		.progress = a6xx_progress,
> +		.sysprof_setup = a6xx_gmu_sysprof_setup,
>  	},
>  	.init = a6xx_gpu_init,
>  	.get_timestamp = a6xx_gmu_get_timestamp,
> @@ -2819,6 +2820,7 @@ const struct adreno_gpu_funcs a7xx_gpu_funcs = {
>  		.create_private_vm = a6xx_create_private_vm,
>  		.get_rptr = a6xx_get_rptr,
>  		.progress = a6xx_progress,
> +		.sysprof_setup = a6xx_gmu_sysprof_setup,
>  	},
>  	.init = a6xx_gpu_init,
>  	.get_timestamp = a6xx_gmu_get_timestamp,

Claude review: drm/msm/a6xx: Restore sysprof_active

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm/a6xx: Restore sysprof_active
Author: Rob Clark <robin.clark@oss.qualcomm.com>
Patches: 2
Reviewed: 2026-04-12T08:58:12.216601

---

This is a single-patch fix that restores a `.sysprof_setup` vfunc assignment that was accidentally dropped during a refactoring (commit 491fadb2b818, "drm/msm/adreno: Move adreno_gpu_func to catalogue"). Without this assignment, `a6xx_gmu_sysprof_setup()` is never called when userspace starts/stops performance counter collection, meaning IFPC (Idle Frequency Power Collapse) is not inhibited, causing perfcounter select register values to be lost during collapse.

The fix is correct and straightforward. One observation worth raising: `a8xx_gpu_funcs` also uses the same GMU infrastructure but does not have `.sysprof_setup` either, which could be a similar gap (though possibly a pre-existing one rather than a regression from the catalogue move).

**Verdict:** The patch looks good. One question for the author regarding a8xx coverage.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/a6xx: Restore sysprof_active

[Claude Code Review Bot]

Patch Review

**Correctness: Good.** The `.sysprof_setup` field belongs to `struct msm_gpu_funcs` (defined at `msm_gpu.h:95`), and the assignment is correctly placed inside the `.base = { }` initializer block of both `a6xx_gpu_funcs` and `a7xx_gpu_funcs`. The function `a6xx_gmu_sysprof_setup()` exists in `a6xx_gmu.c:2037` and is declared in `a6xx_gpu.h:274`.

The call site in `msm_submitqueue.c:44-45`:
```c
if (gpu->funcs->sysprof_setup)
    gpu->funcs->sysprof_setup(gpu);
```
null-checks the pointer before calling, so the omission didn't cause a crash — it silently failed to block IFPC, causing perfcounter corruption during profiling.

**Placement within the initializer:** The new line is placed after `.progress` and before the closing `},` of the `.base` block, which is consistent with the existing ordering. Fine.

**Scope question:** The patch correctly skips `a6xx_gmuwrapper_funcs`, since that variant doesn't use the GMU and `a6xx_gmu_sysprof_setup()` accesses `a6xx_gpu->gmu` internals.

However, `a8xx_gpu_funcs` (at `a6xx_gpu.c:2820`) also uses the GMU (`pm_suspend = a6xx_gmu_pm_suspend`, `pm_resume = a6xx_gmu_pm_resume`) and has the same inline OOB handling in `a8xx_hw_init` (`a8xx_gpu.c:852-856`) that a6xx has in its hw_init (`a6xx_gpu.c:1610-1614`). The hw_init code only covers the resume/recovery path — it does not cover the dynamic case where userspace starts profiling while the GPU is already running, which is what `a6xx_gmu_sysprof_setup()` handles. Should `a8xx_gpu_funcs` also get `.sysprof_setup = a6xx_gmu_sysprof_setup`? If a8xx was added after the catalogue refactor, this would be a pre-existing gap rather than a regression, but it seems like the same bug.

**Fixes tag:** Correctly references the commit that caused the regression.

**Minor nit on subject line:** The subject says "Restore sysprof_active" but what's actually being restored is the `.sysprof_setup` vfunc assignment. `sysprof_active` is the refcount that the setup function reads — not the thing being restored. Very minor.

---
Generated by Claude Code Patch Reviewer