Re: [Freedreno] [PATCH] drm/msm/gpu: Increase the pm runtime autosuspend for 5xx

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