FW: [PATCH] drm/amdgpu: reduce amdgpu_device_resume() time

S, Shirish Shirish.S at amd.com
Thu May 11 04:59:25 UTC 2017


Re-spun patch with the corresponding changes.

-----Original Message-----
From: Alex Deucher [mailto:alexdeucher at gmail.com] 
Sent: Thursday, May 11, 2017 1:24 AM
To: S, Shirish <Shirish.S at amd.com>
Cc: amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Huang, Ray <Ray.Huang at amd.com>; Yu, Hui <Hui.Yu at amd.com>
Subject: Re: FW: [PATCH] drm/amdgpu: reduce amdgpu_device_resume() time

On Wed, May 10, 2017 at 5:20 AM, S, Shirish <Shirish.S at amd.com> wrote:
> From: Shirish S <shirish.s at amd.com>
>
> amdgpu_device_resume() has a high time consuming call of 
> amdgpu_late_init() which sets the clock_gating state of all IP blocks 
> and is blocking.
> This patch defers only this setting of clock gating state operation to 
> post resume of amdgpu driver but ideally before the UI comes up or in 
> some cases post ui as well.
>
> With this change the resume time of amdgpu_device comes down from 
> 1.299s to 0.199s which further helps in reducing the overall system 
> resume time.
>
> TEST:(For ChromiumOS on STONEY only)
> * UI comes up
> * S3 works multiple times
> * Resume time is consistent and lower
> * amdgpu_late_init() call gets called consistently and no errors reported.
>
> Signed-off-by: Shirish S <shirish.s at amd.com>
> Reviewed-by: Huang Rui <ray.huang at amd.com>

Please make sure this doesn't cause problems with S4.  A few comments below.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  3 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 57 
> +++++++++++++++++++++++-------
>  2 files changed, 47 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 1be8aed..addb204 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1615,6 +1615,9 @@ struct amdgpu_device {
>         /* amdkfd interface */
>         struct kfd_dev          *kfd;
>
> +       /* delayed work_func for deferring clockgating during resume */
> +       struct delayed_work     late_init_work;
> +
>         struct amdgpu_virt      virt;
>
>         /* link all shadow bo */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cfc650c..a6850cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -57,6 +57,8 @@
>
>  MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
>
> +#define AMDGPU_RESUME_MS               2000
> +
>  static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);  
> static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>
> @@ -1655,22 +1657,11 @@ static int amdgpu_init(struct amdgpu_device *adev)
>         return 0;
>  }
>
> -static int amdgpu_late_init(struct amdgpu_device *adev)
> +static int amdgpu_late_set_cg_state(struct amdgpu_device *adev)
>  {
>         int i = 0, r;
>
>         for (i = 0; i < adev->num_ip_blocks; i++) {
> -               if (!adev->ip_blocks[i].status.valid)
> -                       continue;
> -               if (adev->ip_blocks[i].version->funcs->late_init) {
> -                       r = adev->ip_blocks[i].version->funcs->late_init((void *)adev);
> -                       if (r) {
> -                               DRM_ERROR("late_init of IP block <%s> failed %d\n",
> -                                         adev->ip_blocks[i].version->funcs->name, r);
> -                               return r;
> -                       }
> -                       adev->ip_blocks[i].status.late_initialized = true;
> -               }
>                 /* skip CG for VCE/UVD, it's handled specially */
>                 if (adev->ip_blocks[i].version->type != AMD_IP_BLOCK_TYPE_UVD &&
>                     adev->ip_blocks[i].version->type != 
> AMD_IP_BLOCK_TYPE_VCE) { @@ -1688,6 +1679,27 @@ static int amdgpu_late_init(struct amdgpu_device *adev)
>         return 0;
>  }
>
> +static int amdgpu_late_init(struct amdgpu_device *adev) {
> +       int i = 0, r;
> +
> +       for (i = 0; i < adev->num_ip_blocks; i++) {
> +               if (!adev->ip_blocks[i].status.valid)
> +                       continue;
> +               if (adev->ip_blocks[i].version->funcs->late_init) {
> +                       r = adev->ip_blocks[i].version->funcs->late_init((void *)adev);
> +                       if (r) {
> +                               DRM_ERROR("late_init of IP block <%s> failed %d\n",
> +                                         adev->ip_blocks[i].version->funcs->name, r);
> +                               return r;
> +                       }
> +                       adev->ip_blocks[i].status.late_initialized = true;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static int amdgpu_fini(struct amdgpu_device *adev)  {
>         int i, r;
> @@ -1775,6 +1787,13 @@ static int amdgpu_fini(struct amdgpu_device *adev)
>         return 0;
>  }
>
> +static void amdgpu_late_init_func_handler(struct work_struct *work) {
> +       struct amdgpu_device *adev =
> +               container_of(work, struct amdgpu_device, late_init_work.work);
> +       amdgpu_late_set_cg_state(adev); }
> +
>  int amdgpu_suspend(struct amdgpu_device *adev)  {
>         int i, r;
> @@ -2074,6 +2093,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>         INIT_LIST_HEAD(&adev->gtt_list);
>         spin_lock_init(&adev->gtt_list_lock);
>
> +       INIT_DELAYED_WORK(&adev->late_init_work, 
> + amdgpu_late_init_func_handler);
> +
>         if (adev->asic_type >= CHIP_BONAIRE) {
>                 adev->rmmio_base = pci_resource_start(adev->pdev, 5);
>                 adev->rmmio_size = pci_resource_len(adev->pdev, 5); @@ 
> -2239,6 +2260,12 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>                 goto failed;
>         }
>
> +       r = amdgpu_late_set_cg_state(adev);
> +       if (r) {
> +               dev_err(adev->dev, "amdgpu_late_set_cg_state failed\n");
> +               goto failed;
> +       }

I think we can probably use the delayed work thread here as well to speed up initial driver load.

> +
>         return 0;
>
>  failed:
> @@ -2270,6 +2297,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>         amdgpu_fbdev_fini(adev);
>         r = amdgpu_fini(adev);
>         adev->accel_working = false;
> +       cancel_delayed_work_sync(&adev->late_init_work);
>         /* free i2c buses */
>         if (!amdgpu_device_has_dc_support(adev))
>                 amdgpu_i2c_fini(adev); @@ -2449,11 +2477,14 @@ int 
> amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon)
>                 if (r)
>                         DRM_ERROR("ib ring test failed (%d).\n", r);
>         }
> -
> +       /* defer the cg state setting to post resume */
>         r = amdgpu_late_init(adev);
>         if (r)
>                 goto unlock;
>
> +       mod_delayed_work(system_wq, &adev->late_init_work,
> +                       msecs_to_jiffies(AMDGPU_RESUME_MS));
> +
>         /* pin cursors */
>         list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>                 struct amdgpu_crtc *amdgpu_crtc = 
> to_amdgpu_crtc(crtc);
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list