Re: [PATCH 1/2] drm/msm/a6xx: Add CX AO Counter registers used for a750 GPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Swapped _H and _L register offsets for GMU_CX_AO_COUNTER cause incorrect timestamp calculation and potential infinite loop.
--

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

drm/msm/a6xx: Add CX AO Counter registers used for a750 GPUs

This commit adds the CX AO Counters to the a6xx GMU XML definitions for
use on a750 GPUs. These replace the GMU_ALWAYS_ON_COUNTER register used
on other A6xx and A7xx variants.

> diff --git a/drivers/gpu/drm/msm/registers/adreno/a6xx_gmu.xml b/drivers/gpu/drm/msm/registers/adreno/a6xx_gmu.xml
> index 33404eb18fd02..3fe7bc3ea93d1 100644
> --- a/drivers/gpu/drm/msm/registers/adreno/a6xx_gmu.xml
> +++ b/drivers/gpu/drm/msm/registers/adreno/a6xx_gmu.xml
> @@ -141,6 +141,8 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
>  	<reg32 offset="0x1f9f0" name="GMU_BOOT_KMD_LM_HANDSHAKE"/>
>  	<reg32 offset="0x1f957" name="GMU_LLM_GLM_SLEEP_CTRL"/>
>  	<reg32 offset="0x1f958" name="GMU_LLM_GLM_SLEEP_STATUS"/>
> +	<reg32 offset="0x1f880" name="GMU_CX_AO_COUNTER_H" variants="A7XX"/>
> +	<reg32 offset="0x1f881" name="GMU_CX_AO_COUNTER_L" variants="A7XX"/>

[Severity: High]
Are the offsets for GMU_CX_AO_COUNTER_H and GMU_CX_AO_COUNTER_L swapped
here? 

Usually, standard little-endian hardware counters place the lower 32 bits
(_L) at the lower offset (0x1f880) and the upper 32 bits (_H) at the higher
offset (0x1f881).

If GMU_CX_AO_COUNTER_H points to the fast-changing lower 32 bits of the
counter, it could cause an infinite loop in a6xx_gmu_get_timestamp(). That
function relies on a read-retry loop to ensure the high bits haven't rolled
over:

    do {
        count_hi = read(_H);
        count_lo = read(_L);
        temp = read(_H);
    } while (unlikely(count_hi != temp));

If _H is mapped to the lower bits, which increment rapidly at 19.2 MHz, the
two reads of _H will almost always yield different values due to MMIO
latency. This would cause the loop to spin indefinitely and potentially lock
up the CPU, returning corrupted timestamps if it ever manages to exit.

>  	<reg32 offset="0x1f888" name="GMU_ALWAYS_ON_COUNTER_L" variants="A6XX-A7XX"/>
>  	<reg32 offset="0x1f840" name="GMU_ALWAYS_ON_COUNTER_L" variants="A8XX-"/>
>  	<reg32 offset="0x1f889" name="GMU_ALWAYS_ON_COUNTER_H" variants="A6XX-A7XX"/>

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