Re: [PATCH 5/8] drm/msm/a6xx: Add IFPC tables for Adreno 630

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/msm/a6xx: perfcntr select regs lost across IFPC on A630
- [Medium] drm/msm/a6xx: shutdown idle timeout in a6xx_gmu_shutdown() on A630
--

commit e4f747633a2babaf9194dc940b8cefe42bf15c0f
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

drm/msm/a6xx: Add IFPC tables for Adreno 630

This commit adds the IFPC and power-up register list tables for Adreno 630
and enables ADRENO_QUIRK_IFPC in its catalog entry.

> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> index 4ef2bd6e7ad26..5e1d91a73a4e1 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c

[ ... ]

> @@ -1014,7 +1092,8 @@ static const struct adreno_info a6xx_gpus[] = {
>  		},
>  		.gmem = SZ_1M,
>  		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> -			  ADRENO_QUIRK_4GB_VA,
> +			  ADRENO_QUIRK_4GB_VA |
> +			  ADRENO_QUIRK_IFPC,

[Severity: Medium]
Does enabling ADRENO_QUIRK_IFPC on Adreno 630 cause a6xx_gmu_shutdown() to
time out waiting for idle when the GPU is runtime-resumed and suspended
without running hw_init()?

On Adreno 630 (ADRENO_6XX_GEN1), a6xx_gmu_init() sets gmu->legacy = true, and
with ADRENO_QUIRK_IFPC set it now also configures gmu->idle_level =
GMU_IDLE_STATE_IFPC. When the GPU is resumed via
a6xx_gmu_pm_resume() -> a6xx_gmu_resume() -> a6xx_gmu_fw_start(),
a6xx_gmu_gfx_rail_on() asserts GMU_OOB_BOOT_SLUMBER:

drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_gmu_gfx_rail_on() {
	...
	/* Let the GMU know the boot sequence has started */
	return a6xx_gmu_set_oob(gmu, GMU_OOB_BOOT_SLUMBER);
}

Normally, GMU_OOB_BOOT_SLUMBER is cleared at the end of hw_init():

drivers/gpu/drm/msm/adreno/a6xx_gpu.c:hw_init() {
	...
	if (a6xx_gpu->gmu.legacy) {
		/* Take the GMU out of its special boot mode */
		a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_BOOT_SLUMBER);
	}
}

However, if the GPU is woken without running msm_gpu_hw_init() (for example,
when userspace queries MSM_PARAM_TIMESTAMP in adreno_get_param() or
configures sysprof in msm_context_set_sysprof()), adreno_gpu->base.needs_hw_init
remains true when runtime suspend calls a6xx_gmu_shutdown():

drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_gmu_shutdown() {
	...
	if (adreno_gpu->base.needs_hw_init) {
		if (a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET))
			goto force_off;

		a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
	}

	if (test_and_clear_bit(GMU_STATUS_OOB_PERF_SET, &gmu->status))
		a6xx_gmu_clear_oob(gmu, GMU_OOB_PERFCOUNTER_SET);

	ret = a6xx_gmu_wait_for_idle(gmu);
	if (ret)
		goto force_off;
}

Because a6xx_gmu_shutdown() does not clear GMU_OOB_BOOT_SLUMBER when
needs_hw_init is true on a legacy GMU, the GX rail stays powered on while
a6xx_gmu_wait_for_idle() -> a6xx_gmu_check_idle_level() polls for
GMU_IDLE_STATE_IFPC and !adreno_gpu->funcs->gx_is_on(adreno_gpu), which times
out and falls back to force_off.

>  		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
>  		.funcs = &a6xx_gpu_funcs,
>  		.zapfw = "a630_zap.mdt",
> @@ -1023,6 +1102,8 @@ static const struct adreno_info a6xx_gpus[] = {
>  			.protect = &a630_protect,
>  			.gmu_cgc_mode = 0x00020202,
>  			.prim_fifo_threshold = 0x00180000,
> +			.ifpc_reglist = &a630_ifpc_reglist,
> +			.pwrup_reglist = &a630_pwrup_reglist,

[Severity: Medium]
With IFPC enabled on Adreno 630, will performance counter select registers
configured for global performance counter streaming (MSM_PERFCNTR_STREAM) be
lost across IFPC power collapse?

Commit 632ad5510114 ("drm/msm/a6xx: Restore perfcntr Selects on IFPC") changed
msm_gpu_sysprof_no_ifpc() to only check refcount_read(&gpu->sysprof_active) > 1,
allowing IFPC while gpu->perfcntrs->stream is active and relying on
a6xx_perfcntr_configure() to append the configured select registers to
pwrup_reglist so the CP restores them on IFPC exit.

However, a6xx_perfcntr_configure() only populates reglist for ADRENO_7XX_GEN1
and newer:

drivers/gpu/drm/msm/adreno/a6xx_gpu.c:a6xx_perfcntr_configure() {
	...
	if (to_adreno_gpu(gpu)->info->family >= ADRENO_7XX_GEN1) {
		WARN_ON(!a6xx_gpu->pwrup_reglist_emitted);

		struct cpu_gpu_lock *lock = a6xx_gpu->pwrup_reglist_ptr;
		int off = (2 * lock->ifpc_list_len) +
			  (2 * lock->preemption_list_len) +
			  (3 * a6xx_gpu->dynamic_sel_reglist_offset);

		reglist = (uint32_t *)&lock->regs[0];
		reglist += off;
		reglist_sel_start = reglist;
		...
	}
}

Similarly, a6xx_patch_pwrup_reglist() populates the A6xx 2-dword
(offset, value) list in lock->list_length without tracking dynamic select
register entries. On Adreno 630 (ADRENO_6XX_GEN1), reglist remains NULL in
a6xx_perfcntr_configure(), so entering IFPC between sample intervals will
reset the PERFCTR_*_SEL registers without restoring them on wakeup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-topic-a6xx_ifpc-v1-0-6992841a9a32@oss.qualcomm.com?part=5