Re: [PATCH 2/3] drm/msm/a6xx: Move power counter selectable to resume()

Jordan Crouse
2018-10-17T14:09:43Z

On Wed, Oct 17, 2018 at 06:34:00PM +0530, Sharat Masetty wrote:
> Moving this to resume helps with both GPU DCVS and the performance
> sampling. For GPU DCVS this makes sure that the frequency does not scale
> when there are no GPU submissions. In the case of performance profiling,
> the GPU is UP, but its possible that the hw_init() was not called yet,
> so we need this to get sane perf values in all cases.

This description is worded oddly to me but I think the gist is that you
want the counters running before you start sampling. I agree which is why I
think that the /* FIXME: */ is right and this should go into in the GMU code
(a6xx_rpmh_start to be exact). The timing should still work out and the GMU code
can stay with the GMU code which is always a bonus.

> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index c0cd3ac..2c52b7c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -387,14 +387,6 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> /* Select CP0 to always count cycles */
> gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT);
>
> - /* FIXME: not sure if this should live here or in a6xx_gmu.c */
> - gmu_write(&a6xx_gpu->gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_MASK,
> - 0xff000000);
> - gmu_rmw(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_SELECT_0,
> - 0xff, 0x20);
> - gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_ENABLE,
> - 0x01);
> -
> gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1);
> gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1);
> gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1);
> @@ -655,6 +647,14 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
>
> gpu->needs_hw_init = true;
>
> + /* FIXME: not sure if this should live here or in a6xx_gmu.c */

If in case you don't move this, this line needs to go away.

> + gmu_write(&a6xx_gpu->gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_MASK,
> + 0xff000000);
> + gmu_rmw(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_SELECT_0,
> + 0xff, 0x20);
> + gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_ENABLE,
> + 0x01);
> +
> msm_gpu_resume_devfreq(gpu);
>
> return ret;
> --
> 1.9.1
>

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