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