Re: [PATCH 3/3] drm/msm: gpu Add new gpu register read/write functions

[Stephen Boyd <sboyd@codeaurora.org>]

On 11/22/2016 07:47 AM, Jordan Crouse wrote:
> Add some new functions to manipulate GPU registers.  gpu_read64 and
> gpu_write64 can read/write a 64 bit value to two 32 bit registers.
> For 4XX and older these are normally perfcounter registers, but
> future targets will use 64 bit addressing so there will be many
> more spots where a 64 bit read and write are needed.
>
> gpu_rmw() does a read/modify/write on a 32 bit register given a mask
> and bits to OR in.
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 12 ++---------
>  drivers/gpu/drm/msm/msm_gpu.h         | 39 +++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index ba16507..b82210c 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -513,16 +513,8 @@ static int a4xx_pm_suspend(struct msm_gpu *gpu) {
>  
>  static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>  {
> -	uint32_t hi, lo, tmp;
> -
> -	tmp = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_HI);
> -	do {
> -		hi = tmp;
> -		lo = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO);
> -		tmp = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_HI);
> -	} while (tmp != hi);
> -
> -	*value = (((uint64_t)hi) << 32) | lo;
> +	*value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO,
> +		REG_A4XX_RBBM_PERFCTR_CP_0_HI);

Did we stop caring about the case where the high bits changed while the
low bits were read? gpu_read64 (pretty poor name by the way considering
how many GPUs there are supported in the kernel) doesn't look to check
for or handle that case.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project