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 <smase...@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to