Re: [PATCH 1/3] drm/msm: Change gpu_busy() function

[Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On Wed, Oct 17, 2018 at 06:33:59PM +0530, Sharat Masetty wrote:
> The gpu_busy() function turns out is useful for performance profiling as
> well. So we change the input params to pass in the pointer to the previous
> busy cycles, this makes the function more generic and removes the dependency
> on the gpu devfreq.

This patch is good in theory.

> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 6 +++---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +++---
>  drivers/gpu/drm/msm/msm_gpu.c         | 3 ++-
>  drivers/gpu/drm/msm/msm_gpu.h         | 2 +-
>  4 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 236a6ed..b5df80c 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1428,7 +1428,7 @@ static struct msm_ringbuffer *a5xx_active_ring(struct msm_gpu *gpu)
>  	return a5xx_gpu->cur_ring;
>  }
>  
> -static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu)
> +static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu, u64 *prev_cycles)
>  {
>  	u64 busy_cycles;
>  	unsigned long busy_time;
> @@ -1436,10 +1436,10 @@ static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu)
>  	busy_cycles = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO,
>  			REG_A5XX_RBBM_PERFCTR_RBBM_0_HI);
>  
> -	busy_time = (busy_cycles - gpu->devfreq.busy_cycles) /
> +	busy_time = (busy_cycles - *prev_cycles) /
>  		(clk_get_rate(gpu->core_clk) / 1000000);
>  
> -	gpu->devfreq.busy_cycles = busy_cycles;
> +	*prev_cycles = busy_cycles;
>  
>  	return busy_time;
>  }
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 07c3c3c..c0cd3ac 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -722,7 +722,7 @@ static void a6xx_destroy(struct msm_gpu *gpu)
>  	kfree(a6xx_gpu);
>  }
>  
> -static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
> +static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu, u64 *prev_cycles)
>  {
>  	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
>  	struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> @@ -733,9 +733,9 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
>  			REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_L,
>  			REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_H);
>  
> -	busy_time = ((busy_cycles - gpu->devfreq.busy_cycles) * 10) / 192;
> +	busy_time = ((busy_cycles - *prev_cycles) * 10) / 192;

You must be using older code  - this is missing the fixes from Sean Paul.

> -	gpu->devfreq.busy_cycles = busy_cycles;
> +	*prev_cycles = busy_cycles;
>  
>  	return busy_time;
>  }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index ca573f6..e9b5426 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -62,7 +62,8 @@ static int msm_devfreq_get_dev_status(struct device *dev,
>  	else
>  		status->current_frequency = clk_get_rate(gpu->core_clk);
>  
> -	status->busy_time = gpu->funcs->gpu_busy(gpu);
> +	status->busy_time = gpu->funcs->gpu_busy(gpu,
> +			&gpu->devfreq.busy_cycles);
>  
>  	time = ktime_get();
>  	status->total_time = ktime_us_delta(time, gpu->devfreq.time);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 9df48e3..0ff23ca 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -71,7 +71,7 @@ struct msm_gpu_funcs {
>  	/* for generation specific debugfs: */
>  	int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
>  #endif
> -	unsigned long (*gpu_busy)(struct msm_gpu *gpu);
> +	unsigned long (*gpu_busy)(struct msm_gpu *gpu, u64 *busy_cycles);
>  	struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);
>  	int (*gpu_state_put)(struct msm_gpu_state *state);
>  	unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
> -- 
> 1.9.1
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno