Re: [PATCH 2/2] drm/msm/a6xx: Use CX AO Counter register for timestamp on a750 GPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The lockless 64-bit counter read loop will spin indefinitely on A750 GPUs, causing a hard CPU hang, because the high and low register offsets for the CX AO counter are swapped in their XML definitions.
--

commit cf31c6e4a536e6c06c20aa83c73f0af1762a7d51
Author: Neil Armstrong <neil.armstrong@linaro.org>

drm/msm/a6xx: Use CX AO Counter register for timestamp on a750 GPUs

The a750 uses the CX AO Counters instead of the GMU_ALWAYS_ON_COUNTER
register on A6xx and other A7xx GPUs, use it when running a A750 GPU.

> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index f9de9329dee39..081e79ea46528 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -23,9 +23,15 @@ static u64 a6xx_gmu_get_timestamp(struct msm_gpu *gpu)
>  	u64 count_hi, count_lo, temp;
>  
>  	do {
> -		count_hi = gmu_read(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
> -		count_lo = gmu_read(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_L);
> -		temp = gmu_read(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
> +		if (adreno_is_a750_family(adreno_gpu)) {
> +			count_hi = gmu_read(&a6xx_gpu->gmu, REG_A7XX_GMU_CX_AO_COUNTER_H);
> +			count_lo = gmu_read(&a6xx_gpu->gmu, REG_A7XX_GMU_CX_AO_COUNTER_L);
> +			temp = gmu_read(&a6xx_gpu->gmu, REG_A7XX_GMU_CX_AO_COUNTER_H);
> +		} else {
> +			count_hi = gmu_read(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
> +			count_lo = gmu_read(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_L);
> +			temp = gmu_read(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
> +		}
>  	} while (unlikely(count_hi != temp));
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Critical]
Could this cause an infinite loop on A750 GPUs?

In drivers/gpu/drm/msm/registers/adreno/a6xx_gmu.xml, the register offsets
for the A750 CX AO counter appear to be swapped:

	<reg32 offset="0x1f880" name="GMU_CX_AO_COUNTER_H" variants="A7XX"/>
	<reg32 offset="0x1f881" name="GMU_CX_AO_COUNTER_L" variants="A7XX"/>

Since the H offset points to the lower 32 bits (0x1f880), this loop will
read the fast-changing lower 32 bits into count_hi and temp. Because these
bits change rapidly, won't the comparison (count_hi != temp) almost always
evaluate to true, causing a hard CPU hang in a6xx_gmu_get_timestamp()?

>  
>  	return (count_hi << 32) | count_lo;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-topic-sm8650-gmu-a750-timestamp-reg-v1-0-284880d6e68b@linaro.org?part=2