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@xxxxxxxxxxxxxx>
> ---
> 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
