[PATCH v4 2/3] drm/amd: Stop evicting resources on APUs in suspend

[Alex Deucher]

On Thu, Feb 8, 2024 at 1:48 AM Mario Limonciello
<mario.limonciello at amd.com> wrote:
>
> commit 5095d5418193 ("drm/amd: Evict resources during PM ops prepare()
> callback") intentionally moved the eviction of resources to earlier in
> the suspend process, but this introduced a subtle change that it occurs
> before adev->in_s0ix or adev->in_s3 are set. This meant that APUs
> actually started to evict resources at suspend time as well.
>
> Explicitly set s0ix or s3 in the prepare() stage, and unset them if the
> prepare() stage failed.
>
> Reported-by: Jürg Billeter <j at bitron.ch>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3132#note_2271038
> Fixes: 5095d5418193 ("drm/amd: Evict resources during PM ops prepare() callback")
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>

Acked-by: Alex Deucher <alexander.deucher at amd.com>

> ---
> v3->v4:
>     * New function to set s0ix/s3 and explicitly unset in cleanup
> v2->v3:
>     * Whitespace
> v1->v2:
>     * Add and use new in_prepare member
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c   | 15 +++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++--
>  3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3d8a48f46b01..f6c38a974bae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1547,9 +1547,11 @@ static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev,
>  #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
>  bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev);
>  bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev);
> +void amdgpu_choose_low_power_state(struct amdgpu_device *adev);
>  #else
>  static inline bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) { return false; }
>  static inline bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev) { return false; }
> +static void amdgpu_choose_low_power_state(struct amdgpu_device *adev) { }
>  #endif
>
>  #if defined(CONFIG_DRM_AMD_DC)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 2deebece810e..cc21ed67a330 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1519,4 +1519,19 @@ bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
>  #endif /* CONFIG_AMD_PMC */
>  }
>
> +/**
> + * amdgpu_choose_low_power_state
> + *
> + * @adev: amdgpu_device_pointer
> + *
> + * Choose the target low power state for the GPU
> + */
> +void amdgpu_choose_low_power_state(struct amdgpu_device *adev)
> +{
> +       if (amdgpu_acpi_is_s0ix_active(adev))
> +               adev->in_s0ix = true;
> +       else if (amdgpu_acpi_is_s3_active(adev))
> +               adev->in_s3 = true;
> +}
> +
>  #endif /* CONFIG_SUSPEND */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2bc460cb993d..dab03865c827 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4518,13 +4518,15 @@ int amdgpu_device_prepare(struct drm_device *dev)
>         struct amdgpu_device *adev = drm_to_adev(dev);
>         int i, r;
>
> +       amdgpu_choose_low_power_state(adev);
> +
>         if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>                 return 0;
>
>         /* Evict the majority of BOs before starting suspend sequence */
>         r = amdgpu_device_evict_resources(adev);
>         if (r)
> -               return r;
> +               goto unprepare;
>
>         for (i = 0; i < adev->num_ip_blocks; i++) {
>                 if (!adev->ip_blocks[i].status.valid)
> @@ -4533,10 +4535,15 @@ int amdgpu_device_prepare(struct drm_device *dev)
>                         continue;
>                 r = adev->ip_blocks[i].version->funcs->prepare_suspend((void *)adev);
>                 if (r)
> -                       return r;
> +                       goto unprepare;
>         }
>
>         return 0;
> +
> +unprepare:
> +       adev->in_s0ix = adev->in_s3 = false;
> +
> +       return r;
>  }
>
>  /**
> --
> 2.34.1
>