[PATCH 3/3] drm/msm: re-factor devfreq code

[Jordan Crouse]

On Fri, May 25, 2018 at 04:00:46PM +0530, Sharat Masetty wrote:
> devfreq framework requires the drivers to provide busy time estimations.
> The GPU driver relies on the hardware performance counters for the busy time
> estimations, but different hardware revisions have counters which can be
> sourced from different clocks. So the busy time estimation will be target
> dependent.  Additionally on targets where the clocks are completely controlled
> by the on chip microcontroller, fetching and setting the current GPU frequency
> will be different. This patch aims to embrace these differences by re-factoring
> the devfreq code a bit.
> 
> Signed-off-by: Sharat Masetty <smasetty at codeaurora.org>
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 15 +++++++++++----
>  drivers/gpu/drm/msm/msm_gpu.c         | 23 ++++++++++++++---------
>  drivers/gpu/drm/msm/msm_gpu.h         |  4 +++-
>  3 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index d39400e..a35a840 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1219,12 +1219,19 @@ static struct msm_ringbuffer *a5xx_active_ring(struct msm_gpu *gpu)
>  	return a5xx_gpu->cur_ring;
>  }
>  
> -static int a5xx_gpu_busy(struct msm_gpu *gpu, uint64_t *value)
> +static u64 a5xx_gpu_busy(struct msm_gpu *gpu)
>  {
> -	*value = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO,
> -		REG_A5XX_RBBM_PERFCTR_RBBM_0_HI);
> +	u64 busy_cycles, busy_time;
>  
> -	return 0;
> +	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) /
> +		(clk_get_rate(gpu->core_clk) / 1000000);
> +
> +	gpu->devfreq.busy_cycles = busy_cycles;
> +
> +	return busy_time;
>  }
>  
>  static const struct adreno_gpu_funcs funcs = {
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index d8d4fc9..f437ee8 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -40,7 +40,11 @@ static int msm_devfreq_target(struct device *dev, unsigned long *freq,
>  	if (IS_ERR(opp))
>  		return PTR_ERR(opp);
>  
> -	clk_set_rate(gpu->core_clk, *freq);
> +	if (gpu->funcs->gpu_set_freq)
> +		gpu->funcs->gpu_set_freq(gpu, (u64)*freq);
> +	else
> +		clk_set_rate(gpu->core_clk, *freq);
> +
>  	dev_pm_opp_put(opp);
>  
>  	return 0;
> @@ -50,16 +54,14 @@ static int msm_devfreq_get_dev_status(struct device *dev,
>  		struct devfreq_dev_status *status)
>  {
>  	struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
> -	u64 cycles;
> -	u32 freq = ((u32) status->current_frequency) / 1000000;
>  	ktime_t time;
>  
> -	status->current_frequency = (unsigned long) clk_get_rate(gpu->core_clk);
> -	gpu->funcs->gpu_busy(gpu, &cycles);
> -
> -	status->busy_time = ((u32) (cycles - gpu->devfreq.busy_cycles)) / freq;
> +	if (gpu->funcs->gpu_get_freq)
> +		status->current_frequency = gpu->funcs->gpu_get_freq(gpu);
> +	else
> +		status->current_frequency = clk_get_rate(gpu->core_clk);
>  
> -	gpu->devfreq.busy_cycles = cycles;
> +	status->busy_time = gpu->funcs->gpu_busy(gpu);
>  
>  	time = ktime_get();
>  	status->total_time = ktime_us_delta(time, gpu->devfreq.time);
> @@ -72,7 +74,10 @@ static int msm_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
>  {
>  	struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
>  
> -	*freq = (unsigned long) clk_get_rate(gpu->core_clk);
> +	if (gpu->funcs->gpu_get_freq)
> +		*freq = gpu->funcs->gpu_get_freq(gpu);

This will get ugly on a 32 bit build.  I think that as long as the reset of the
subsystem uses unsigned long we might as well too. I can put on my fortune
tellers hat and guess we're probably not going to see a 4GB core running on a 32
bit target in the near future.

>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 1876b81..d3f02e7 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -68,7 +68,9 @@ struct msm_gpu_funcs {
>  	/* for generation specific debugfs: */
>  	int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
>  #endif
> -	int (*gpu_busy)(struct msm_gpu *gpu, uint64_t *value);
> +	u64 (*gpu_busy)(struct msm_gpu *gpu);
> +	u64 (*gpu_get_freq)(struct msm_gpu *gpu);
> +	int (*gpu_set_freq)(struct msm_gpu *gpu, u64 freq);

So go ahead and change both of these to an unsigned long and that should help
the casting.

Jordan

>  };
>  
>  struct msm_gpu {
> -- 
> 1.9.1
> 

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