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 <[email protected]> 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-284880d6e...@linaro.org?part=1
