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

[Konrad Dybcio]

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

Konrad