On 3/24/2026 3:18 PM, Konrad Dybcio wrote:
> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>> GMU_ALWAYS_ON_COUNTER_* registers got moved in A8x, but currently, A6x
>> register offsets are used in the submit traces instead of A8x offsets.
>> To fix this, refactor a bit and use adreno_gpu->funcs->get_timestamp()
>> everywhere.
>>
>> While we are at it, update a8xx_gmu_get_timestamp() to use the GMU AO
>> counter.
>>
>> Fixes: 288a93200892 ("drm/msm/adreno: Introduce A8x GPU Support")
>> Signed-off-by: Akhil P Oommen <[email protected]>
>> ---
>
> [...]
>
>> -static int a6xx_gmu_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>> +static u64 a6xx_gmu_get_timestamp(struct msm_gpu *gpu)
>> {
>> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
>> struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
>>
>> - *value = read_gmu_ao_counter(a6xx_gpu);
>> -
>> - return 0;
>> + return read_gmu_ao_counter(a6xx_gpu);
>
> Can we instead make read_gmu_ao_counter() take a struct msm_gpu * and drop
> this wrapper? Other callers also already have a ptr of that type
>
> [...]
>
>> -int a8xx_gmu_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>> +static u64 read_gmu_ao_counter(struct a6xx_gpu *a6xx_gpu)
>
> Similarly here (also I know this is a static symbol, but keeping an
> a8xx_ prefix would be nice
>
> [...]
>
>> case MSM_PARAM_TIMESTAMP:
>> if (adreno_gpu->funcs->get_timestamp) {
>> - int ret;
>> -
>> pm_runtime_get_sync(&gpu->pdev->dev);
>> - ret = adreno_gpu->funcs->get_timestamp(gpu, value);
>> + *value = (uint64_t)
>> adreno_gpu->funcs->get_timestamp(gpu);
>
> "u64", I think checkpathch will also warn about whitespace after a typecast
I didn't see any checkpatch error (with b4 prep --check), but we can
just remove this unnecessary typecast here.
Ack on all other suggestions.
-Akhil.
>
> Konrad
