diff mbox series

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

Message ID 1539781441-13076-3-git-send-email-smasetty@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series drm/msm: Revive GPU workload profiling | expand

Commit Message

Sharat Masetty Oct. 17, 2018, 1:04 p.m. UTC
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.

Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Jordan Crouse Oct. 17, 2018, 2:09 p.m. UTC | #1
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
>
diff mbox series

Patch

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 */
+	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;