[PATCH 1/2] drm/amdgpu: explicitly check for s0ix when evicting resources

[PATCH 1/2] drm/amdgpu: explicitly check for s0ix when evicting resources

[Mario Limonciello]

This codepath should be running in both s0ix and s3, but only does
currently because s3 and s0ix are both set in the s0ix case.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ce93a304292c..412f377f80b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3956,8 +3956,8 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
  */
 static void amdgpu_device_evict_resources(struct amdgpu_device *adev)
 {
-	/* No need to evict vram on APUs for suspend to ram */
-	if (adev->in_s3 && (adev->flags & AMD_IS_APU))
+	/* No need to evict vram on APUs for suspend to ram or s2idle */
+	if ((adev->in_s3 || adev->in_s0ix) && (adev->flags & AMD_IS_APU))
 		return;
 
 	if (amdgpu_ttm_evict_resources(adev, TTM_PL_VRAM))
-- 
2.25.1

[PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

[Mario Limonciello]

This makes it clearer which codepaths are in use specifically in
one state or the other.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index db2a9dfd5918..413fecc89e6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2165,9 +2165,9 @@ static int amdgpu_pmops_suspend(struct device *dev)
 
 	if (amdgpu_acpi_is_s0ix_active(adev))
 		adev->in_s0ix = true;
-	adev->in_s3 = true;
+	else
+		adev->in_s3 = true;
 	r = amdgpu_device_suspend(drm_dev, true);
-	adev->in_s3 = false;
 	if (r)
 		return r;
 	if (!adev->in_s0ix)
@@ -2188,6 +2188,8 @@ static int amdgpu_pmops_resume(struct device *dev)
 	r = amdgpu_device_resume(drm_dev, true);
 	if (amdgpu_acpi_is_s0ix_active(adev))
 		adev->in_s0ix = false;
+	else
+		adev->in_s3 = false;
 	return r;
 }
 
-- 
2.25.1

RE: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

[Quan, Evan]

[AMD Official Use Only]

Series is acked-by: Evan Quan <evan.quan@amd.com>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Mario Limonciello
> Sent: Monday, January 3, 2022 11:23 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time
> 
> This makes it clearer which codepaths are in use specifically in
> one state or the other.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index db2a9dfd5918..413fecc89e6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2165,9 +2165,9 @@ static int amdgpu_pmops_suspend(struct device
> *dev)
> 
>  	if (amdgpu_acpi_is_s0ix_active(adev))
>  		adev->in_s0ix = true;
> -	adev->in_s3 = true;
> +	else
> +		adev->in_s3 = true;
>  	r = amdgpu_device_suspend(drm_dev, true);
> -	adev->in_s3 = false;
>  	if (r)
>  		return r;
>  	if (!adev->in_s0ix)
> @@ -2188,6 +2188,8 @@ static int amdgpu_pmops_resume(struct device
> *dev)
>  	r = amdgpu_device_resume(drm_dev, true);
>  	if (amdgpu_acpi_is_s0ix_active(adev))
>  		adev->in_s0ix = false;
> +	else
> +		adev->in_s3 = false;
>  	return r;
>  }
> 
> --
> 2.25.1

Re: [PATCH 1/2] drm/amdgpu: explicitly check for s0ix when evicting resources

[Deucher, Alexander]

[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]

[Public]

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Mario Limonciello <mario.limonciello@amd.com>
Sent: Monday, January 3, 2022 10:23 AM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Limonciello, Mario <Mario.Limonciello@amd.com>
Subject: [PATCH 1/2] drm/amdgpu: explicitly check for s0ix when evicting resources

This codepath should be running in both s0ix and s3, but only does
currently because s3 and s0ix are both set in the s0ix case.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ce93a304292c..412f377f80b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3956,8 +3956,8 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
  */
 static void amdgpu_device_evict_resources(struct amdgpu_device *adev)
 {
-       /* No need to evict vram on APUs for suspend to ram */
-       if (adev->in_s3 && (adev->flags & AMD_IS_APU))
+       /* No need to evict vram on APUs for suspend to ram or s2idle */
+       if ((adev->in_s3 || adev->in_s0ix) && (adev->flags & AMD_IS_APU))
                 return;

         if (amdgpu_ttm_evict_resources(adev, TTM_PL_VRAM))
--
2.25.1


[-- Attachment #2: Type: text/html, Size: 2817 bytes --]

Re: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

[Deucher, Alexander]

[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]

[AMD Official Use Only]

I don't think this will work properly.  The in_s3 flag was mainly for runtime pm vs system suspend.  I'm not sure if in_s0ix is properly handled everywhere we check in_s3.

Alex

________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Mario Limonciello <mario.limonciello@amd.com>
Sent: Monday, January 3, 2022 10:23 AM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Limonciello, Mario <Mario.Limonciello@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

This makes it clearer which codepaths are in use specifically in
one state or the other.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index db2a9dfd5918..413fecc89e6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2165,9 +2165,9 @@ static int amdgpu_pmops_suspend(struct device *dev)

         if (amdgpu_acpi_is_s0ix_active(adev))
                 adev->in_s0ix = true;
-       adev->in_s3 = true;
+       else
+               adev->in_s3 = true;
         r = amdgpu_device_suspend(drm_dev, true);
-       adev->in_s3 = false;
         if (r)
                 return r;
         if (!adev->in_s0ix)
@@ -2188,6 +2188,8 @@ static int amdgpu_pmops_resume(struct device *dev)
         r = amdgpu_device_resume(drm_dev, true);
         if (amdgpu_acpi_is_s0ix_active(adev))
                 adev->in_s0ix = false;
+       else
+               adev->in_s3 = false;
         return r;
 }

--
2.25.1


[-- Attachment #2: Type: text/html, Size: 3973 bytes --]

RE: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

[Limonciello, Mario]

[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]

[AMD Official Use Only]

Maybe it was used more widely previously?
The only place that I found it in use was amdgpu_device_evict_resources.

From: Deucher, Alexander <Alexander.Deucher@amd.com>
Sent: Tuesday, January 4, 2022 11:24
To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time


[AMD Official Use Only]

I don't think this will work properly.  The in_s3 flag was mainly for runtime pm vs system suspend.  I'm not sure if in_s0ix is properly handled everywhere we check in_s3.

Alex

________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
Sent: Monday, January 3, 2022 10:23 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@amd.com>>
Subject: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

This makes it clearer which codepaths are in use specifically in
one state or the other.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index db2a9dfd5918..413fecc89e6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2165,9 +2165,9 @@ static int amdgpu_pmops_suspend(struct device *dev)

         if (amdgpu_acpi_is_s0ix_active(adev))
                 adev->in_s0ix = true;
-       adev->in_s3 = true;
+       else
+               adev->in_s3 = true;
         r = amdgpu_device_suspend(drm_dev, true);
-       adev->in_s3 = false;
         if (r)
                 return r;
         if (!adev->in_s0ix)
@@ -2188,6 +2188,8 @@ static int amdgpu_pmops_resume(struct device *dev)
         r = amdgpu_device_resume(drm_dev, true);
         if (amdgpu_acpi_is_s0ix_active(adev))
                 adev->in_s0ix = false;
+       else
+               adev->in_s3 = false;
         return r;
 }

--
2.25.1

[-- Attachment #2: Type: text/html, Size: 7100 bytes --]

Re: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time

[Alex Deucher]

On Tue, Jan 4, 2022 at 12:26 PM Limonciello, Mario
<Mario.Limonciello@amd.com> wrote:
>
> [AMD Official Use Only]
>
>
>
> Maybe it was used more widely previously?
>
> The only place that I found it in use was amdgpu_device_evict_resources.
>
>
>
> From: Deucher, Alexander <Alexander.Deucher@amd.com>
> Sent: Tuesday, January 4, 2022 11:24
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time
>
>
>
> [AMD Official Use Only]
>
>
>
> I don't think this will work properly.  The in_s3 flag was mainly for runtime pm vs system suspend.  I'm not sure if in_s0ix is properly handled everywhere we check in_s3.
>

In that case, Acked-by: Alex Deucher <alexander.deucher@amd.com>

>
>
> Alex
>
>
>
> ________________________________
>
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Mario Limonciello <mario.limonciello@amd.com>
> Sent: Monday, January 3, 2022 10:23 AM
> To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Cc: Limonciello, Mario <Mario.Limonciello@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu: don't set s3 and s0ix at the same time
>
>
>
> This makes it clearer which codepaths are in use specifically in
> one state or the other.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index db2a9dfd5918..413fecc89e6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2165,9 +2165,9 @@ static int amdgpu_pmops_suspend(struct device *dev)
>
>          if (amdgpu_acpi_is_s0ix_active(adev))
>                  adev->in_s0ix = true;
> -       adev->in_s3 = true;
> +       else
> +               adev->in_s3 = true;
>          r = amdgpu_device_suspend(drm_dev, true);
> -       adev->in_s3 = false;
>          if (r)
>                  return r;
>          if (!adev->in_s0ix)
> @@ -2188,6 +2188,8 @@ static int amdgpu_pmops_resume(struct device *dev)
>          r = amdgpu_device_resume(drm_dev, true);
>          if (amdgpu_acpi_is_s0ix_active(adev))
>                  adev->in_s0ix = false;
> +       else
> +               adev->in_s3 = false;
>          return r;
>  }
>
> --
> 2.25.1