[PATCH] drm/amd: Set s0i3/s3 in prepare() callback instead of suspend() callback

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Mario
> Limonciello
> Sent: Tuesday, February 6, 2024 4:32 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Limonciello, Mario <Mario.Limonciello at amd.com>; Jürg Billeter
> <j at bitron.ch>
> Subject: [PATCH] drm/amd: Set s0i3/s3 in prepare() callback instead of
> suspend() callback
>
> 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.
>
> Move the s0i3/s3 setting flags into prepare() to ensure that they're set during
> eviction. Drop the existing call to return 1 in this case because the suspend()
> callback looks for the flags too.
>
> 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>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index b74f68a15802..190b2ee9e36b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2464,12 +2464,10 @@ static int amdgpu_pmops_prepare(struct device
> *dev)
>           pm_runtime_suspended(dev))
>               return 1;
>
> -     /* if we will not support s3 or s2i for the device
> -      *  then skip suspend
> -      */
> -     if (!amdgpu_acpi_is_s0ix_active(adev) &&
> -         !amdgpu_acpi_is_s3_active(adev))
> -             return 1;
> +     if (amdgpu_acpi_is_s0ix_active(adev))
> +             adev->in_s0ix = true;
> +     else if (amdgpu_acpi_is_s3_active(adev))
> +             adev->in_s3 = true;
>

Will resume always get called to clear these after after prepare?  Will these ever get set and then not unset?

Alex

>       return amdgpu_device_prepare(drm_dev);  } @@ -2484,10 +2482,6
> @@ static int amdgpu_pmops_suspend(struct device *dev)
>       struct drm_device *drm_dev = dev_get_drvdata(dev);
>       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>
> -     if (amdgpu_acpi_is_s0ix_active(adev))
> -             adev->in_s0ix = true;
> -     else if (amdgpu_acpi_is_s3_active(adev))
> -             adev->in_s3 = true;
>       if (!adev->in_s0ix && !adev->in_s3)
>               return 0;
>       return amdgpu_device_suspend(drm_dev, true);
> --
> 2.34.1