Re: [PATCH 01/16] drm/msm/a8xx: Fix the ticks used in submit traces

[Akhil P Oommen]

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 <akhilpo@xxxxxxxxxxxxxxxx>
>> ---
> 
> [...]
> 
>> -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