diff mbox

drm/msm/gpu: Increase the pm runtime autosuspend for 5xx

Message ID 20180507224750.9383-1-jcrouse@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jordan Crouse May 7, 2018, 10:47 p.m. UTC 
Experimentation shows that resuming power quickly after suspending
ends up forcing a system hang for unknown reasons on 5xx targets.
To avoid cycling the power too much (especially during init)
turn up the autosuspend time for a5xx to 250ms and use
pm_runtime_put_autosuspend() when applicable.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 13 ++++++++++++-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c    |  3 ++-
 drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

Comments

Rob Clark May 8, 2018, 10:19 p.m. UTC | #1 
On Mon, May 7, 2018 at 6:47 PM, Jordan Crouse <jcrouse@codeaurora.org> wrote:
> Experimentation shows that resuming power quickly after suspending
> ends up forcing a system hang for unknown reasons on 5xx targets.
> To avoid cycling the power too much (especially during init)
> turn up the autosuspend time for a5xx to 250ms and use
> pm_runtime_put_autosuspend() when applicable.
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 13 ++++++++++++-
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c    |  3 ++-
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  1 +
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 8e0cb161754b..e8499fffbefb 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -35,6 +35,7 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_PFP] = "a300_pfp.fw",
>                 },
>                 .gmem  = SZ_256K,
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>                 .init  = a3xx_gpu_init,
>         }, {
>                 .rev   = ADRENO_REV(3, 0, 6, 0),
> @@ -45,6 +46,7 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_PFP] = "a300_pfp.fw",
>                 },
>                 .gmem  = SZ_128K,
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>                 .init  = a3xx_gpu_init,
>         }, {
>                 .rev   = ADRENO_REV(3, 2, ANY_ID, ANY_ID),
> @@ -55,6 +57,7 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_PFP] = "a300_pfp.fw",
>                 },
>                 .gmem  = SZ_512K,
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>                 .init  = a3xx_gpu_init,
>         }, {
>                 .rev   = ADRENO_REV(3, 3, 0, ANY_ID),
> @@ -65,6 +68,7 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_PFP] = "a330_pfp.fw",
>                 },
>                 .gmem  = SZ_1M,
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>                 .init  = a3xx_gpu_init,
>         }, {
>                 .rev   = ADRENO_REV(4, 2, 0, ANY_ID),
> @@ -75,6 +79,7 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_PFP] = "a420_pfp.fw",
>                 },
>                 .gmem  = (SZ_1M + SZ_512K),
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>                 .init  = a4xx_gpu_init,
>         }, {
>                 .rev   = ADRENO_REV(4, 3, 0, ANY_ID),
> @@ -85,6 +90,7 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_PFP] = "a420_pfp.fw",
>                 },
>                 .gmem  = (SZ_1M + SZ_512K),
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
>                 .init  = a4xx_gpu_init,
>         }, {
>                 .rev = ADRENO_REV(5, 3, 0, 2),
> @@ -96,6 +102,11 @@ static const struct adreno_info gpulist[] = {
>                         [ADRENO_FW_GPMU] = "a530v3_gpmu.fw2",
>                 },
>                 .gmem = SZ_1M,
> +               /*
> +                * Increase inactive period to 250 to avoid bouncing
> +                * the GDSC which appears to make it grumpy
> +                */
> +               .inactive_period = 250,

I suppose having inactive_period per-gpu is reasonable, in the sense
that I guess the time it takes to "boot" the gpu might differ (and I
guess the power cost too)..

Having an artificial high value for a530 seems a bit gross, although
not really against short-term hacks since it is a pita when things
don't work with upstream trees.  But it is kind of papering over the
issue, and no guarantee that userspace doesn't in some case wait long
enough for inactive to kick in, but not long enough after that to
anger gdsc[1].. so I think someone who knows the gdsc stuff really
needs to have a look at the root problem.

BR,
-R

[1] I'm assuming the issue is time between gdsc-off and gdsc-on... I
guess if it is time between gdsc-on and gdsc-on this might be
sufficent.  I remember in the early days of rc6 on intel, there were
similar sorts of issues, but it was really more motherboard specific
than chip specific, ie. some had power circuitry that simply wasn't up
to the task of handling the rapid toggles.

>                 .quirks = ADRENO_QUIRK_TWO_PASS_USE_WFI |
>                         ADRENO_QUIRK_FAULT_DETECT_MASK,
>                 .init = a5xx_gpu_init,
> @@ -158,7 +169,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
>         mutex_lock(&dev->struct_mutex);
>         ret = msm_gpu_hw_init(gpu);
>         mutex_unlock(&dev->struct_mutex);
> -       pm_runtime_put_sync(&pdev->dev);
> +       pm_runtime_put_autosuspend(&pdev->dev);
>         if (ret) {
>                 dev_err(dev->dev, "gpu hw init failed: %d\n", ret);
>                 return NULL;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 17d0506d058c..bcbf9f2a29f9 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -565,7 +565,8 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>
>         adreno_get_pwrlevels(&pdev->dev, gpu);
>
> -       pm_runtime_set_autosuspend_delay(&pdev->dev, DRM_MSM_INACTIVE_PERIOD);
> +       pm_runtime_set_autosuspend_delay(&pdev->dev,
> +               adreno_gpu->info->inactive_period);
>         pm_runtime_use_autosuspend(&pdev->dev);
>         pm_runtime_enable(&pdev->dev);
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index d6b0e7b813f4..bc9ec27e9ed8 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -84,6 +84,7 @@ struct adreno_info {
>         enum adreno_quirks quirks;
>         struct msm_gpu *(*init)(struct drm_device *dev);
>         const char *zapfw;
> +       u32 inactive_period;
>  };
>
>  const struct adreno_info *adreno_info(struct adreno_rev rev);
> --
> 2.17.0
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
diff mbox

Patch

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 8e0cb161754b..e8499fffbefb 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -35,6 +35,7 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_PFP] = "a300_pfp.fw",
 		},
 		.gmem  = SZ_256K,
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
 		.init  = a3xx_gpu_init,
 	}, {
 		.rev   = ADRENO_REV(3, 0, 6, 0),
@@ -45,6 +46,7 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_PFP] = "a300_pfp.fw",
 		},
 		.gmem  = SZ_128K,
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
 		.init  = a3xx_gpu_init,
 	}, {
 		.rev   = ADRENO_REV(3, 2, ANY_ID, ANY_ID),
@@ -55,6 +57,7 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_PFP] = "a300_pfp.fw",
 		},
 		.gmem  = SZ_512K,
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
 		.init  = a3xx_gpu_init,
 	}, {
 		.rev   = ADRENO_REV(3, 3, 0, ANY_ID),
@@ -65,6 +68,7 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_PFP] = "a330_pfp.fw",
 		},
 		.gmem  = SZ_1M,
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
 		.init  = a3xx_gpu_init,
 	}, {
 		.rev   = ADRENO_REV(4, 2, 0, ANY_ID),
@@ -75,6 +79,7 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_PFP] = "a420_pfp.fw",
 		},
 		.gmem  = (SZ_1M + SZ_512K),
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
 		.init  = a4xx_gpu_init,
 	}, {
 		.rev   = ADRENO_REV(4, 3, 0, ANY_ID),
@@ -85,6 +90,7 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_PFP] = "a420_pfp.fw",
 		},
 		.gmem  = (SZ_1M + SZ_512K),
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
 		.init  = a4xx_gpu_init,
 	}, {
 		.rev = ADRENO_REV(5, 3, 0, 2),
@@ -96,6 +102,11 @@  static const struct adreno_info gpulist[] = {
 			[ADRENO_FW_GPMU] = "a530v3_gpmu.fw2",
 		},
 		.gmem = SZ_1M,
+		/*
+		 * Increase inactive period to 250 to avoid bouncing
+		 * the GDSC which appears to make it grumpy
+		 */
+		.inactive_period = 250,
 		.quirks = ADRENO_QUIRK_TWO_PASS_USE_WFI |
 			ADRENO_QUIRK_FAULT_DETECT_MASK,
 		.init = a5xx_gpu_init,
@@ -158,7 +169,7 @@  struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 	mutex_lock(&dev->struct_mutex);
 	ret = msm_gpu_hw_init(gpu);
 	mutex_unlock(&dev->struct_mutex);
-	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
 	if (ret) {
 		dev_err(dev->dev, "gpu hw init failed: %d\n", ret);
 		return NULL;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 17d0506d058c..bcbf9f2a29f9 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -565,7 +565,8 @@  int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 	adreno_get_pwrlevels(&pdev->dev, gpu);
 
-	pm_runtime_set_autosuspend_delay(&pdev->dev, DRM_MSM_INACTIVE_PERIOD);
+	pm_runtime_set_autosuspend_delay(&pdev->dev,
+		adreno_gpu->info->inactive_period);
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index d6b0e7b813f4..bc9ec27e9ed8 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -84,6 +84,7 @@  struct adreno_info {
 	enum adreno_quirks quirks;
 	struct msm_gpu *(*init)(struct drm_device *dev);
 	const char *zapfw;
+	u32 inactive_period;
 };
 
 const struct adreno_info *adreno_info(struct adreno_rev rev);