[PATCH v5 0/7] Better handle memory pressure at suspend

[PATCH v5 0/7] Better handle memory pressure at suspend

[Mario Limonciello]

At suspend time if there is memory pressure then dynamically allocating
memory will cause failures that don't clean up properly when trying
suspend a second time.

Move the bigger memory allocations into Linux PM prepare() callback, drop
allocations that aren't really needed in DC code and report failures
in dm_suspend() up.

v1: https://lore.kernel.org/amd-gfx/20230925143359.14932-1-mario.limonciello@amd.com/
v2: https://lore.kernel.org/amd-gfx/20231002224449.95565-1-mario.limonciello@amd.com/T/#mc800319a05df821cd1875234b09bf212e2e3282b
v3: https://lore.kernel.org/amd-gfx/20231003205437.123426-1-mario.limonciello@amd.com/T/#m00a49b75cd2638bf8a0ebd549d6a6010bfb7328b
v4: https://lore.kernel.org/amd-gfx/20231004171838.168215-1-mario.limonciello@amd.com/T/#m0921840868295ec19abbe5ecbaa0aee75356b9e1

v4->v5:
  * Call amdgpu_device_prepare() from other callers to amdgpu_device_suspend()
  * 3x evict calls -> 2x evict calls
  * Add IP block specific prepare path
  * Fix issue in UVD
  * Raise warnings for issues that could happen in amdgpu_switcheroo_set_state()
  * Catch problem that could happen in dm_suspend()
  * Rebase on top of DML2 series in amd-staging-drm-next
v3->v4:
 * Combine patches 1/2
 * Drop adev->in_suspend references
v2->v3:
 * Handle adev->in_suspend in prepare() and complete()
 * Add missing scratch variable in dc_resource_state_destruct()
 * Revert error code propagation in same series
v1->v2:
 * Handle DC code too
 * Add prepare callback rather than moving symbol calls
Mario Limonciello (3):
  drm/amd: Evict resources during PM ops prepare() callback
  drm/amd/display: Destroy DC context while keeping DML
  drm/amd/display: make dc_set_power_state() return type `void` again

Mario Limonciello (7):
  drm/amd: Evict resources during PM ops prepare() callback
  drm/amd: Add concept of running prepare() sequence for IP blocks
  drm/amd: Split up UVD suspend into prepare and suspend steps
  drm/amd: Capture errors in amdgpu_switcheroo_set_state()
  drm/amd/display: Catch errors from drm_atomic_helper_suspend()
  drm/amd/display: Destroy DC context while keeping DML and DML2
  drm/amd/display: make dc_set_power_state() return type `void` again

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 75 ++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 10 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c       | 12 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h       |  1 +
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c         |  9 +++
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c         |  9 +++
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c         |  9 +++
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c         |  9 +++
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c         |  9 +++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++---
 drivers/gpu/drm/amd/display/dc/core/dc.c      | 43 +----------
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 12 +++
 drivers/gpu/drm/amd/display/dc/dc.h           |  2 +-
 drivers/gpu/drm/amd/include/amd_shared.h      |  1 +
 15 files changed, 149 insertions(+), 72 deletions(-)

-- 
2.34.1

[PATCH v5 1/7] drm/amd: Evict resources during PM ops prepare() callback

[Mario Limonciello]

Linux PM core has a prepare() callback run before suspend.

If the system is under high memory pressure, the resources may need
to be evicted into swap instead.  If the storage backing for swap
is offlined during the suspend() step then such a call may fail.

So move this step into prepare() to move evict majority of
resources and update all non-pmops callers to call the same callback.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4->v5:
 * Call amdgpu_device_prepare() from other callers to amdgpu_device_suspend()
 * 3x evict calls -> 2x evict calls
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 31 ++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 10 ++++---
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4cc78e0e4304..fdb2e9ae13e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1409,6 +1409,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
 void amdgpu_driver_release_kms(struct drm_device *dev);
 
 int amdgpu_device_ip_suspend(struct amdgpu_device *adev);
+int amdgpu_device_prepare(struct drm_device *dev);
 int amdgpu_device_suspend(struct drm_device *dev, bool fbcon);
 int amdgpu_device_resume(struct drm_device *dev, bool fbcon);
 u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0cb702c3046a..cb334dc57c59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1760,6 +1760,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
 	} else {
 		pr_info("switched off\n");
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
+		amdgpu_device_prepare(dev);
 		amdgpu_device_suspend(dev, true);
 		amdgpu_device_cache_pci_state(pdev);
 		/* Shut down the device */
@@ -4335,6 +4336,31 @@ static int amdgpu_device_evict_resources(struct amdgpu_device *adev)
 /*
  * Suspend & resume.
  */
+/**
+ * amdgpu_device_prepare - prepare for device suspend
+ *
+ * @dev: drm dev pointer
+ *
+ * Prepare to put the hw in the suspend state (all asics).
+ * Returns 0 for success or an error on failure.
+ * Called at driver suspend.
+ */
+int amdgpu_device_prepare(struct drm_device *dev)
+{
+	struct amdgpu_device *adev = drm_to_adev(dev);
+	int r;
+
+	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;
+
+	return 0;
+}
+
 /**
  * amdgpu_device_suspend - initiate device suspend
  *
@@ -4355,11 +4381,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 
 	adev->in_suspend = true;
 
-	/* Evict the majority of BOs before grabbing the full access */
-	r = amdgpu_device_evict_resources(adev);
-	if (r)
-		return r;
-
 	if (amdgpu_sriov_vf(adev)) {
 		amdgpu_virt_fini_data_exchange(adev);
 		r = amdgpu_virt_request_full_gpu(adev, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 81affdf7c0c3..420196a17e22 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2427,8 +2427,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
 	/* Return a positive number here so
 	 * DPM_FLAG_SMART_SUSPEND works properly
 	 */
-	if (amdgpu_device_supports_boco(drm_dev))
-		return pm_runtime_suspended(dev);
+	if (amdgpu_device_supports_boco(drm_dev) &&
+	    pm_runtime_suspended(dev))
+		return 1;
 
 	/* if we will not support s3 or s2i for the device
 	 *  then skip suspend
@@ -2437,7 +2438,7 @@ static int amdgpu_pmops_prepare(struct device *dev)
 	    !amdgpu_acpi_is_s3_active(adev))
 		return 1;
 
-	return 0;
+	return amdgpu_device_prepare(drm_dev);
 }
 
 static void amdgpu_pmops_complete(struct device *dev)
@@ -2637,6 +2638,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	if (amdgpu_device_supports_boco(drm_dev))
 		adev->mp1_state = PP_MP1_STATE_UNLOAD;
 
+	ret = amdgpu_device_prepare(drm_dev);
+	if (ret)
+		return ret;
 	ret = amdgpu_device_suspend(drm_dev, false);
 	if (ret) {
 		adev->in_runpm = false;
-- 
2.34.1

Re: [PATCH v5 1/7] drm/amd: Evict resources during PM ops prepare() callback

[Christian König]

Am 06.10.23 um 20:50 schrieb Mario Limonciello:
> Linux PM core has a prepare() callback run before suspend.
>
> If the system is under high memory pressure, the resources may need
> to be evicted into swap instead.  If the storage backing for swap
> is offlined during the suspend() step then such a call may fail.
>
> So move this step into prepare() to move evict majority of
> resources and update all non-pmops callers to call the same callback.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for this one.

> ---
> v4->v5:
>   * Call amdgpu_device_prepare() from other callers to amdgpu_device_suspend()
>   * 3x evict calls -> 2x evict calls
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 31 ++++++++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 10 ++++---
>   3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4cc78e0e4304..fdb2e9ae13e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1409,6 +1409,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>   void amdgpu_driver_release_kms(struct drm_device *dev);
>   
>   int amdgpu_device_ip_suspend(struct amdgpu_device *adev);
> +int amdgpu_device_prepare(struct drm_device *dev);
>   int amdgpu_device_suspend(struct drm_device *dev, bool fbcon);
>   int amdgpu_device_resume(struct drm_device *dev, bool fbcon);
>   u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 0cb702c3046a..cb334dc57c59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1760,6 +1760,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
>   	} else {
>   		pr_info("switched off\n");
>   		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> +		amdgpu_device_prepare(dev);
>   		amdgpu_device_suspend(dev, true);
>   		amdgpu_device_cache_pci_state(pdev);
>   		/* Shut down the device */
> @@ -4335,6 +4336,31 @@ static int amdgpu_device_evict_resources(struct amdgpu_device *adev)
>   /*
>    * Suspend & resume.
>    */
> +/**
> + * amdgpu_device_prepare - prepare for device suspend
> + *
> + * @dev: drm dev pointer
> + *
> + * Prepare to put the hw in the suspend state (all asics).
> + * Returns 0 for success or an error on failure.
> + * Called at driver suspend.
> + */
> +int amdgpu_device_prepare(struct drm_device *dev)
> +{
> +	struct amdgpu_device *adev = drm_to_adev(dev);
> +	int r;
> +
> +	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;
> +
> +	return 0;
> +}
> +
>   /**
>    * amdgpu_device_suspend - initiate device suspend
>    *
> @@ -4355,11 +4381,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
>   
>   	adev->in_suspend = true;
>   
> -	/* Evict the majority of BOs before grabbing the full access */
> -	r = amdgpu_device_evict_resources(adev);
> -	if (r)
> -		return r;
> -
>   	if (amdgpu_sriov_vf(adev)) {
>   		amdgpu_virt_fini_data_exchange(adev);
>   		r = amdgpu_virt_request_full_gpu(adev, false);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 81affdf7c0c3..420196a17e22 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2427,8 +2427,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
>   	/* Return a positive number here so
>   	 * DPM_FLAG_SMART_SUSPEND works properly
>   	 */
> -	if (amdgpu_device_supports_boco(drm_dev))
> -		return pm_runtime_suspended(dev);
> +	if (amdgpu_device_supports_boco(drm_dev) &&
> +	    pm_runtime_suspended(dev))
> +		return 1;
>   
>   	/* if we will not support s3 or s2i for the device
>   	 *  then skip suspend
> @@ -2437,7 +2438,7 @@ static int amdgpu_pmops_prepare(struct device *dev)
>   	    !amdgpu_acpi_is_s3_active(adev))
>   		return 1;
>   
> -	return 0;
> +	return amdgpu_device_prepare(drm_dev);
>   }
>   
>   static void amdgpu_pmops_complete(struct device *dev)
> @@ -2637,6 +2638,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>   	if (amdgpu_device_supports_boco(drm_dev))
>   		adev->mp1_state = PP_MP1_STATE_UNLOAD;
>   
> +	ret = amdgpu_device_prepare(drm_dev);
> +	if (ret)
> +		return ret;
>   	ret = amdgpu_device_suspend(drm_dev, false);
>   	if (ret) {
>   		adev->in_runpm = false;

Re: [PATCH v5 1/7] drm/amd: Evict resources during PM ops prepare() callback

[Alex Deucher]

On Fri, Oct 6, 2023 at 3:17 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Linux PM core has a prepare() callback run before suspend.
>
> If the system is under high memory pressure, the resources may need
> to be evicted into swap instead.  If the storage backing for swap
> is offlined during the suspend() step then such a call may fail.
>
> So move this step into prepare() to move evict majority of
> resources and update all non-pmops callers to call the same callback.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
> v4->v5:
>  * Call amdgpu_device_prepare() from other callers to amdgpu_device_suspend()
>  * 3x evict calls -> 2x evict calls
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 31 ++++++++++++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 10 ++++---
>  3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4cc78e0e4304..fdb2e9ae13e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1409,6 +1409,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>  void amdgpu_driver_release_kms(struct drm_device *dev);
>
>  int amdgpu_device_ip_suspend(struct amdgpu_device *adev);
> +int amdgpu_device_prepare(struct drm_device *dev);
>  int amdgpu_device_suspend(struct drm_device *dev, bool fbcon);
>  int amdgpu_device_resume(struct drm_device *dev, bool fbcon);
>  u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 0cb702c3046a..cb334dc57c59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1760,6 +1760,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
>         } else {
>                 pr_info("switched off\n");
>                 dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> +               amdgpu_device_prepare(dev);
>                 amdgpu_device_suspend(dev, true);
>                 amdgpu_device_cache_pci_state(pdev);
>                 /* Shut down the device */
> @@ -4335,6 +4336,31 @@ static int amdgpu_device_evict_resources(struct amdgpu_device *adev)
>  /*
>   * Suspend & resume.
>   */
> +/**
> + * amdgpu_device_prepare - prepare for device suspend
> + *
> + * @dev: drm dev pointer
> + *
> + * Prepare to put the hw in the suspend state (all asics).
> + * Returns 0 for success or an error on failure.
> + * Called at driver suspend.
> + */
> +int amdgpu_device_prepare(struct drm_device *dev)
> +{
> +       struct amdgpu_device *adev = drm_to_adev(dev);
> +       int r;
> +
> +       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;
> +
> +       return 0;
> +}
> +
>  /**
>   * amdgpu_device_suspend - initiate device suspend
>   *
> @@ -4355,11 +4381,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
>
>         adev->in_suspend = true;
>
> -       /* Evict the majority of BOs before grabbing the full access */
> -       r = amdgpu_device_evict_resources(adev);
> -       if (r)
> -               return r;
> -
>         if (amdgpu_sriov_vf(adev)) {
>                 amdgpu_virt_fini_data_exchange(adev);
>                 r = amdgpu_virt_request_full_gpu(adev, false);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 81affdf7c0c3..420196a17e22 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2427,8 +2427,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
>         /* Return a positive number here so
>          * DPM_FLAG_SMART_SUSPEND works properly
>          */
> -       if (amdgpu_device_supports_boco(drm_dev))
> -               return pm_runtime_suspended(dev);
> +       if (amdgpu_device_supports_boco(drm_dev) &&
> +           pm_runtime_suspended(dev))
> +               return 1;
>
>         /* if we will not support s3 or s2i for the device
>          *  then skip suspend
> @@ -2437,7 +2438,7 @@ static int amdgpu_pmops_prepare(struct device *dev)
>             !amdgpu_acpi_is_s3_active(adev))
>                 return 1;
>
> -       return 0;
> +       return amdgpu_device_prepare(drm_dev);
>  }
>
>  static void amdgpu_pmops_complete(struct device *dev)
> @@ -2637,6 +2638,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>         if (amdgpu_device_supports_boco(drm_dev))
>                 adev->mp1_state = PP_MP1_STATE_UNLOAD;
>
> +       ret = amdgpu_device_prepare(drm_dev);
> +       if (ret)
> +               return ret;
>         ret = amdgpu_device_suspend(drm_dev, false);
>         if (ret) {
>                 adev->in_runpm = false;
> --
> 2.34.1
>

[PATCH v5 2/7] drm/amd: Add concept of running prepare() sequence for IP blocks

[Mario Limonciello]

If any IP blocks allocate memory during their sw_fini() sequence
this can cause the suspend to fail under memory pressure.  Introduce
a new phase that IP blocks can use to allocate memory before suspend
starts so that it can potentially be evicted into swap instead.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4->v5:
 * New patch
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++++-
 drivers/gpu/drm/amd/include/amd_shared.h   |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index cb334dc57c59..a362152cd0da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4348,7 +4348,7 @@ static int amdgpu_device_evict_resources(struct amdgpu_device *adev)
 int amdgpu_device_prepare(struct drm_device *dev)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	int r;
+	int i, r;
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
@@ -4358,6 +4358,16 @@ int amdgpu_device_prepare(struct drm_device *dev)
 	if (r)
 		return 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->prepare)
+			continue;
+		r = adev->ip_blocks[i].version->funcs->prepare((void *)adev);
+		if (r)
+			return r;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
index ce75351204bb..1f831cb747e0 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -299,6 +299,7 @@ struct amd_ip_funcs {
 	int (*hw_init)(void *handle);
 	int (*hw_fini)(void *handle);
 	void (*late_fini)(void *handle);
+	int (*prepare)(void *prepare);
 	int (*suspend)(void *handle);
 	int (*resume)(void *handle);
 	bool (*is_idle)(void *handle);
-- 
2.34.1

Re: [PATCH v5 2/7] drm/amd: Add concept of running prepare() sequence for IP blocks

[Christian König]

Am 06.10.23 um 20:50 schrieb Mario Limonciello:
> If any IP blocks allocate memory during their sw_fini() sequence

hw_fini instead of sw_fini? sw_fini should only be called on driver unload.

> this can cause the suspend to fail under memory pressure.  Introduce
> a new phase that IP blocks can use to allocate memory before suspend
> starts so that it can potentially be evicted into swap instead.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Apart from the commit message Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

> ---
> v4->v5:
>   * New patch
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++++-
>   drivers/gpu/drm/amd/include/amd_shared.h   |  1 +
>   2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb334dc57c59..a362152cd0da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4348,7 +4348,7 @@ static int amdgpu_device_evict_resources(struct amdgpu_device *adev)
>   int amdgpu_device_prepare(struct drm_device *dev)
>   {
>   	struct amdgpu_device *adev = drm_to_adev(dev);
> -	int r;
> +	int i, r;
>   
>   	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>   		return 0;
> @@ -4358,6 +4358,16 @@ int amdgpu_device_prepare(struct drm_device *dev)
>   	if (r)
>   		return 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->prepare)
> +			continue;
> +		r = adev->ip_blocks[i].version->funcs->prepare((void *)adev);
> +		if (r)
> +			return r;
> +	}
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index ce75351204bb..1f831cb747e0 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -299,6 +299,7 @@ struct amd_ip_funcs {
>   	int (*hw_init)(void *handle);
>   	int (*hw_fini)(void *handle);
>   	void (*late_fini)(void *handle);
> +	int (*prepare)(void *prepare);
>   	int (*suspend)(void *handle);
>   	int (*resume)(void *handle);
>   	bool (*is_idle)(void *handle);

Re: [PATCH v5 2/7] drm/amd: Add concept of running prepare() sequence for IP blocks

[Alex Deucher]

On Fri, Oct 6, 2023 at 2:51 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> If any IP blocks allocate memory during their sw_fini() sequence
> this can cause the suspend to fail under memory pressure.  Introduce
> a new phase that IP blocks can use to allocate memory before suspend
> starts so that it can potentially be evicted into swap instead.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v4->v5:
>  * New patch
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++++-
>  drivers/gpu/drm/amd/include/amd_shared.h   |  1 +
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb334dc57c59..a362152cd0da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4348,7 +4348,7 @@ static int amdgpu_device_evict_resources(struct amdgpu_device *adev)
>  int amdgpu_device_prepare(struct drm_device *dev)
>  {
>         struct amdgpu_device *adev = drm_to_adev(dev);
> -       int r;
> +       int i, r;
>
>         if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>                 return 0;
> @@ -4358,6 +4358,16 @@ int amdgpu_device_prepare(struct drm_device *dev)
>         if (r)
>                 return 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->prepare)
> +                       continue;
> +               r = adev->ip_blocks[i].version->funcs->prepare((void *)adev);
> +               if (r)
> +                       return r;
> +       }
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index ce75351204bb..1f831cb747e0 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -299,6 +299,7 @@ struct amd_ip_funcs {
>         int (*hw_init)(void *handle);
>         int (*hw_fini)(void *handle);
>         void (*late_fini)(void *handle);
> +       int (*prepare)(void *prepare);

Prepare is a little vague.  How about prepare_suspend()?  Also *handle
for consistency.  Could possibly use this for DC as well if we end up
needing to allocate DML structures.

Alex

>         int (*suspend)(void *handle);
>         int (*resume)(void *handle);
>         bool (*is_idle)(void *handle);
> --
> 2.34.1
>

[PATCH v5 3/7] drm/amd: Split up UVD suspend into prepare and suspend steps

[Mario Limonciello]

amdgpu_uvd_suspend() allocates memory and copies objects into that
allocated memory.  This fails under memory pressure.  Instead move
majority of this code into a prepare step when swap can still be
allocated.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4->v5:
 * New patch
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 12 ++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c   |  9 +++++++++
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c   |  9 +++++++++
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c   |  9 +++++++++
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   |  9 +++++++++
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   |  9 +++++++++
 7 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index b7441654e6fa..a53c4ba8b3fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -418,12 +418,11 @@ int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
 	return 0;
 }
 
-int amdgpu_uvd_suspend(struct amdgpu_device *adev)
+int amdgpu_uvd_prepare(struct amdgpu_device *adev)
 {
 	unsigned int size;
 	void *ptr;
 	int i, j, idx;
-	bool in_ras_intr = amdgpu_ras_intr_triggered();
 
 	cancel_delayed_work_sync(&adev->uvd.idle_work);
 
@@ -452,7 +451,7 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
 
 		if (drm_dev_enter(adev_to_drm(adev), &idx)) {
 			/* re-write 0 since err_event_athub will corrupt VCPU buffer */
-			if (in_ras_intr)
+			if (amdgpu_ras_intr_triggered())
 				memset(adev->uvd.inst[j].saved_bo, 0, size);
 			else
 				memcpy_fromio(adev->uvd.inst[j].saved_bo, ptr, size);
@@ -461,7 +460,12 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
 		}
 	}
 
-	if (in_ras_intr)
+	return 0;
+}
+
+int amdgpu_uvd_suspend(struct amdgpu_device *adev)
+{
+	if (amdgpu_ras_intr_triggered())
 		DRM_WARN("UVD VCPU state may lost due to RAS ERREVENT_ATHUB_INTERRUPT\n");
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index 9f89bb7cd60b..72228425e021 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -74,6 +74,7 @@ struct amdgpu_uvd {
 int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
 int amdgpu_uvd_sw_fini(struct amdgpu_device *adev);
 int amdgpu_uvd_entity_init(struct amdgpu_device *adev);
+int amdgpu_uvd_prepare(struct amdgpu_device *adev);
 int amdgpu_uvd_suspend(struct amdgpu_device *adev);
 int amdgpu_uvd_resume(struct amdgpu_device *adev);
 int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
index 5534c769b655..869e9948fa36 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
@@ -706,6 +706,14 @@ static int uvd_v3_1_hw_fini(void *handle)
 	return 0;
 }
 
+static int uvd_v3_1_prepare(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v3_1_suspend(void *handle)
 {
 	int r;
@@ -806,6 +814,7 @@ static const struct amd_ip_funcs uvd_v3_1_ip_funcs = {
 	.sw_fini = uvd_v3_1_sw_fini,
 	.hw_init = uvd_v3_1_hw_init,
 	.hw_fini = uvd_v3_1_hw_fini,
+	.prepare = uvd_v3_1_prepare,
 	.suspend = uvd_v3_1_suspend,
 	.resume = uvd_v3_1_resume,
 	.is_idle = uvd_v3_1_is_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index c108b8381795..e589c17af371 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -220,6 +220,14 @@ static int uvd_v4_2_hw_fini(void *handle)
 	return 0;
 }
 
+static int uvd_v4_2_prepare(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v4_2_suspend(void *handle)
 {
 	int r;
@@ -756,6 +764,7 @@ static const struct amd_ip_funcs uvd_v4_2_ip_funcs = {
 	.sw_fini = uvd_v4_2_sw_fini,
 	.hw_init = uvd_v4_2_hw_init,
 	.hw_fini = uvd_v4_2_hw_fini,
+	.prepare = uvd_v4_2_prepare,
 	.suspend = uvd_v4_2_suspend,
 	.resume = uvd_v4_2_resume,
 	.is_idle = uvd_v4_2_is_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
index d7e31e48a2b8..65aa23bc2d91 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
@@ -218,6 +218,14 @@ static int uvd_v5_0_hw_fini(void *handle)
 	return 0;
 }
 
+static int uvd_v5_0_prepare(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v5_0_suspend(void *handle)
 {
 	int r;
@@ -863,6 +871,7 @@ static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
 	.sw_fini = uvd_v5_0_sw_fini,
 	.hw_init = uvd_v5_0_hw_init,
 	.hw_fini = uvd_v5_0_hw_fini,
+	.prepare = uvd_v5_0_prepare,
 	.suspend = uvd_v5_0_suspend,
 	.resume = uvd_v5_0_resume,
 	.is_idle = uvd_v5_0_is_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 5fe872f4bea7..a4035aeb973b 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -542,6 +542,14 @@ static int uvd_v6_0_hw_fini(void *handle)
 	return 0;
 }
 
+static int uvd_v6_0_prepare(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v6_0_suspend(void *handle)
 {
 	int r;
@@ -1528,6 +1536,7 @@ static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
 	.sw_fini = uvd_v6_0_sw_fini,
 	.hw_init = uvd_v6_0_hw_init,
 	.hw_fini = uvd_v6_0_hw_fini,
+	.prepare = uvd_v6_0_prepare,
 	.suspend = uvd_v6_0_suspend,
 	.resume = uvd_v6_0_resume,
 	.is_idle = uvd_v6_0_is_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index 86d1d46e1e5e..3228bf1a379c 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -612,6 +612,14 @@ static int uvd_v7_0_hw_fini(void *handle)
 	return 0;
 }
 
+static int uvd_v7_0_prepare(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	return amdgpu_uvd_prepare(adev);
+
+}
+
 static int uvd_v7_0_suspend(void *handle)
 {
 	int r;
@@ -1787,6 +1795,7 @@ const struct amd_ip_funcs uvd_v7_0_ip_funcs = {
 	.sw_fini = uvd_v7_0_sw_fini,
 	.hw_init = uvd_v7_0_hw_init,
 	.hw_fini = uvd_v7_0_hw_fini,
+	.prepare = uvd_v7_0_prepare,
 	.suspend = uvd_v7_0_suspend,
 	.resume = uvd_v7_0_resume,
 	.is_idle = NULL /* uvd_v7_0_is_idle */,
-- 
2.34.1

Re: [PATCH v5 3/7] drm/amd: Split up UVD suspend into prepare and suspend steps

[Alex Deucher]

On Fri, Oct 6, 2023 at 3:07 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> amdgpu_uvd_suspend() allocates memory and copies objects into that
> allocated memory.  This fails under memory pressure.  Instead move
> majority of this code into a prepare step when swap can still be
> allocated.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v4->v5:
>  * New patch
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 12 ++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c   |  9 +++++++++
>  drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c   |  9 +++++++++
>  drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c   |  9 +++++++++
>  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   |  9 +++++++++
>  drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   |  9 +++++++++
>  7 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index b7441654e6fa..a53c4ba8b3fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -418,12 +418,11 @@ int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
>         return 0;
>  }
>
> -int amdgpu_uvd_suspend(struct amdgpu_device *adev)
> +int amdgpu_uvd_prepare(struct amdgpu_device *adev)

amdgpu_uvd_prepare_suspend() so it's more obvious what this is for.
Other than that, looks good to me.

Alex

>  {
>         unsigned int size;
>         void *ptr;
>         int i, j, idx;
> -       bool in_ras_intr = amdgpu_ras_intr_triggered();
>
>         cancel_delayed_work_sync(&adev->uvd.idle_work);
>
> @@ -452,7 +451,7 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
>
>                 if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>                         /* re-write 0 since err_event_athub will corrupt VCPU buffer */
> -                       if (in_ras_intr)
> +                       if (amdgpu_ras_intr_triggered())
>                                 memset(adev->uvd.inst[j].saved_bo, 0, size);
>                         else
>                                 memcpy_fromio(adev->uvd.inst[j].saved_bo, ptr, size);
> @@ -461,7 +460,12 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
>                 }
>         }
>
> -       if (in_ras_intr)
> +       return 0;
> +}
> +
> +int amdgpu_uvd_suspend(struct amdgpu_device *adev)
> +{
> +       if (amdgpu_ras_intr_triggered())
>                 DRM_WARN("UVD VCPU state may lost due to RAS ERREVENT_ATHUB_INTERRUPT\n");
>
>         return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> index 9f89bb7cd60b..72228425e021 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> @@ -74,6 +74,7 @@ struct amdgpu_uvd {
>  int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
>  int amdgpu_uvd_sw_fini(struct amdgpu_device *adev);
>  int amdgpu_uvd_entity_init(struct amdgpu_device *adev);
> +int amdgpu_uvd_prepare(struct amdgpu_device *adev);
>  int amdgpu_uvd_suspend(struct amdgpu_device *adev);
>  int amdgpu_uvd_resume(struct amdgpu_device *adev);
>  int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> index 5534c769b655..869e9948fa36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> @@ -706,6 +706,14 @@ static int uvd_v3_1_hw_fini(void *handle)
>         return 0;
>  }
>
> +static int uvd_v3_1_prepare(void *handle)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +       return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v3_1_suspend(void *handle)
>  {
>         int r;
> @@ -806,6 +814,7 @@ static const struct amd_ip_funcs uvd_v3_1_ip_funcs = {
>         .sw_fini = uvd_v3_1_sw_fini,
>         .hw_init = uvd_v3_1_hw_init,
>         .hw_fini = uvd_v3_1_hw_fini,
> +       .prepare = uvd_v3_1_prepare,
>         .suspend = uvd_v3_1_suspend,
>         .resume = uvd_v3_1_resume,
>         .is_idle = uvd_v3_1_is_idle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> index c108b8381795..e589c17af371 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> @@ -220,6 +220,14 @@ static int uvd_v4_2_hw_fini(void *handle)
>         return 0;
>  }
>
> +static int uvd_v4_2_prepare(void *handle)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +       return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v4_2_suspend(void *handle)
>  {
>         int r;
> @@ -756,6 +764,7 @@ static const struct amd_ip_funcs uvd_v4_2_ip_funcs = {
>         .sw_fini = uvd_v4_2_sw_fini,
>         .hw_init = uvd_v4_2_hw_init,
>         .hw_fini = uvd_v4_2_hw_fini,
> +       .prepare = uvd_v4_2_prepare,
>         .suspend = uvd_v4_2_suspend,
>         .resume = uvd_v4_2_resume,
>         .is_idle = uvd_v4_2_is_idle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index d7e31e48a2b8..65aa23bc2d91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -218,6 +218,14 @@ static int uvd_v5_0_hw_fini(void *handle)
>         return 0;
>  }
>
> +static int uvd_v5_0_prepare(void *handle)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +       return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v5_0_suspend(void *handle)
>  {
>         int r;
> @@ -863,6 +871,7 @@ static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
>         .sw_fini = uvd_v5_0_sw_fini,
>         .hw_init = uvd_v5_0_hw_init,
>         .hw_fini = uvd_v5_0_hw_fini,
> +       .prepare = uvd_v5_0_prepare,
>         .suspend = uvd_v5_0_suspend,
>         .resume = uvd_v5_0_resume,
>         .is_idle = uvd_v5_0_is_idle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 5fe872f4bea7..a4035aeb973b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -542,6 +542,14 @@ static int uvd_v6_0_hw_fini(void *handle)
>         return 0;
>  }
>
> +static int uvd_v6_0_prepare(void *handle)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +       return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v6_0_suspend(void *handle)
>  {
>         int r;
> @@ -1528,6 +1536,7 @@ static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
>         .sw_fini = uvd_v6_0_sw_fini,
>         .hw_init = uvd_v6_0_hw_init,
>         .hw_fini = uvd_v6_0_hw_fini,
> +       .prepare = uvd_v6_0_prepare,
>         .suspend = uvd_v6_0_suspend,
>         .resume = uvd_v6_0_resume,
>         .is_idle = uvd_v6_0_is_idle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> index 86d1d46e1e5e..3228bf1a379c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> @@ -612,6 +612,14 @@ static int uvd_v7_0_hw_fini(void *handle)
>         return 0;
>  }
>
> +static int uvd_v7_0_prepare(void *handle)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +       return amdgpu_uvd_prepare(adev);
> +
> +}
> +
>  static int uvd_v7_0_suspend(void *handle)
>  {
>         int r;
> @@ -1787,6 +1795,7 @@ const struct amd_ip_funcs uvd_v7_0_ip_funcs = {
>         .sw_fini = uvd_v7_0_sw_fini,
>         .hw_init = uvd_v7_0_hw_init,
>         .hw_fini = uvd_v7_0_hw_fini,
> +       .prepare = uvd_v7_0_prepare,
>         .suspend = uvd_v7_0_suspend,
>         .resume = uvd_v7_0_resume,
>         .is_idle = NULL /* uvd_v7_0_is_idle */,
> --
> 2.34.1
>

[PATCH v5 4/7] drm/amd: Capture errors in amdgpu_switcheroo_set_state()

[Mario Limonciello]

amdgpu_switcheroo_set_state() calls lots of functions that could
fail under memory pressure or for other reasons.  Don't assume
everything can successfully run sequentially, and check return codes
for everything that returns one.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a362152cd0da..8dfcff783dab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1749,23 +1749,45 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
 		/* don't suspend or resume card normally */
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-		pci_set_power_state(pdev, PCI_D0);
-		amdgpu_device_load_pci_state(pdev);
+		r = pci_set_power_state(pdev, PCI_D0);
+		if (r) {
+			DRM_WARN("pci_set_power_state failed (%d)\n", r);
+			return;
+		}
+		if (!amdgpu_device_load_pci_state(pdev))
+			return;
 		r = pci_enable_device(pdev);
 		if (r)
 			DRM_WARN("pci_enable_device failed (%d)\n", r);
-		amdgpu_device_resume(dev, true);
+		r = amdgpu_device_resume(dev, true);
+		if (r) {
+			DRM_WARN("amdgpu_device_resume failed (%d)\n", r);
+			return;
+		}
 
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	} else {
 		pr_info("switched off\n");
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
-		amdgpu_device_prepare(dev);
-		amdgpu_device_suspend(dev, true);
-		amdgpu_device_cache_pci_state(pdev);
+		r = amdgpu_device_prepare(dev);
+		if (r) {
+			DRM_WARN("amdgpu_device_prepare failed (%d)\n", r);
+			return;
+		}
+		r = amdgpu_device_suspend(dev, true);
+		if (r) {
+			DRM_WARN("amdgpu_device_suspend failed (%d)\n", r);
+			return;
+		}
+		if (!amdgpu_device_cache_pci_state(pdev))
+			return;
 		/* Shut down the device */
 		pci_disable_device(pdev);
-		pci_set_power_state(pdev, PCI_D3cold);
+		r = pci_set_power_state(pdev, PCI_D3cold);
+		if (r) {
+			DRM_WARN("pci_set_power_state failed (%d)\n", r);
+			return;
+		}
 		dev->switch_power_state = DRM_SWITCH_POWER_OFF;
 	}
 }
-- 
2.34.1

Re: [PATCH v5 4/7] drm/amd: Capture errors in amdgpu_switcheroo_set_state()

[Christian König]

Am 06.10.23 um 20:50 schrieb Mario Limonciello:
> amdgpu_switcheroo_set_state() calls lots of functions that could
> fail under memory pressure or for other reasons.  Don't assume
> everything can successfully run sequentially, and check return codes
> for everything that returns one.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 +++++++++++++++++-----
>   1 file changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index a362152cd0da..8dfcff783dab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1749,23 +1749,45 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
>   		/* don't suspend or resume card normally */
>   		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>   
> -		pci_set_power_state(pdev, PCI_D0);
> -		amdgpu_device_load_pci_state(pdev);
> +		r = pci_set_power_state(pdev, PCI_D0);
> +		if (r) {
> +			DRM_WARN("pci_set_power_state failed (%d)\n", r);
> +			return;
> +		}
> +		if (!amdgpu_device_load_pci_state(pdev))
> +			return;
>   		r = pci_enable_device(pdev);
>   		if (r)
>   			DRM_WARN("pci_enable_device failed (%d)\n", r);
> -		amdgpu_device_resume(dev, true);
> +		r = amdgpu_device_resume(dev, true);
> +		if (r) {
> +			DRM_WARN("amdgpu_device_resume failed (%d)\n", r);
> +			return;
> +		}
>   
>   		dev->switch_power_state = DRM_SWITCH_POWER_ON;
>   	} else {
>   		pr_info("switched off\n");
>   		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> -		amdgpu_device_prepare(dev);
> -		amdgpu_device_suspend(dev, true);
> -		amdgpu_device_cache_pci_state(pdev);
> +		r = amdgpu_device_prepare(dev);
> +		if (r) {
> +			DRM_WARN("amdgpu_device_prepare failed (%d)\n", r);
> +			return;
> +		}
> +		r = amdgpu_device_suspend(dev, true);
> +		if (r) {
> +			DRM_WARN("amdgpu_device_suspend failed (%d)\n", r);
> +			return;
> +		}
> +		if (!amdgpu_device_cache_pci_state(pdev))
> +			return;
>   		/* Shut down the device */
>   		pci_disable_device(pdev);
> -		pci_set_power_state(pdev, PCI_D3cold);
> +		r = pci_set_power_state(pdev, PCI_D3cold);
> +		if (r) {
> +			DRM_WARN("pci_set_power_state failed (%d)\n", r);
> +			return;
> +		}
>   		dev->switch_power_state = DRM_SWITCH_POWER_OFF;
>   	}
>   }

[PATCH v5 5/7] drm/amd/display: Catch errors from drm_atomic_helper_suspend()

[Mario Limonciello]

drm_atomic_helper_suspend() can return PTR_ERR(), in which case the
error gets stored into `dm->cached_state`.  This can cause failures
during resume.  Catch the error during suspend and fail the suspend
instead.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4->v5:
 * New patch
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a59a11ae42db..63944d3b9e8c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2716,6 +2716,8 @@ static int dm_suspend(void *handle)
 
 	WARN_ON(adev->dm.cached_state);
 	adev->dm.cached_state = drm_atomic_helper_suspend(adev_to_drm(adev));
+	if (IS_ERR(adev->dm.cached_state))
+		return PTR_ERR(adev->dm.cached_state);
 
 	s3_handle_mst(adev_to_drm(adev), true);
 
-- 
2.34.1

Re: [PATCH v5 5/7] drm/amd/display: Catch errors from drm_atomic_helper_suspend()

[Christian König]

Am 06.10.23 um 20:50 schrieb Mario Limonciello:
> drm_atomic_helper_suspend() can return PTR_ERR(), in which case the
> error gets stored into `dm->cached_state`.  This can cause failures
> during resume.  Catch the error during suspend and fail the suspend
> instead.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
> v4->v5:
>   * New patch
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index a59a11ae42db..63944d3b9e8c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2716,6 +2716,8 @@ static int dm_suspend(void *handle)
>   
>   	WARN_ON(adev->dm.cached_state);
>   	adev->dm.cached_state = drm_atomic_helper_suspend(adev_to_drm(adev));
> +	if (IS_ERR(adev->dm.cached_state))
> +		return PTR_ERR(adev->dm.cached_state);
>   
>   	s3_handle_mst(adev_to_drm(adev), true);
>   

Re: [PATCH v5 5/7] drm/amd/display: Catch errors from drm_atomic_helper_suspend()

[Alex Deucher]

On Sun, Oct 8, 2023 at 6:57 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> drm_atomic_helper_suspend() can return PTR_ERR(), in which case the
> error gets stored into `dm->cached_state`.  This can cause failures
> during resume.  Catch the error during suspend and fail the suspend
> instead.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

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

> ---
> v4->v5:
>  * New patch
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index a59a11ae42db..63944d3b9e8c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2716,6 +2716,8 @@ static int dm_suspend(void *handle)
>
>         WARN_ON(adev->dm.cached_state);
>         adev->dm.cached_state = drm_atomic_helper_suspend(adev_to_drm(adev));
> +       if (IS_ERR(adev->dm.cached_state))
> +               return PTR_ERR(adev->dm.cached_state);
>
>         s3_handle_mst(adev_to_drm(adev), true);
>
> --
> 2.34.1
>

[PATCH v5 6/7] drm/amd/display: Destroy DC context while keeping DML and DML2

[Mario Limonciello]

If there is memory pressure at suspend time then dynamically
allocating a large structure as part of DC suspend code will
fail.

Instead re-use the same structures and clear all members except
those that should be maintained.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4->v5:
 * Rebase for DML2
---
 drivers/gpu/drm/amd/display/dc/core/dc.c      | 37 -------------------
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 12 ++++++
 2 files changed, 12 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 2436a293931b..55e7b5a8ec8e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -4704,12 +4704,6 @@ bool dc_set_power_state(
 	struct dc *dc,
 	enum dc_acpi_cm_power_state power_state)
 {
-	struct kref refcount;
-	struct display_mode_lib *dml;
-#ifdef CONFIG_DRM_AMD_DC_FP
-	struct dml2_context *dml2 = NULL;
-#endif
-
 	if (!dc->current_state)
 		return true;
 
@@ -4728,40 +4722,9 @@ bool dc_set_power_state(
 
 		break;
 	default:
-#ifdef CONFIG_DRM_AMD_DC_FP
-		if (dc->debug.using_dml2)
-			dml2 = dc->current_state->bw_ctx.dml2;
-#endif
 		ASSERT(dc->current_state->stream_count == 0);
-		/* Zero out the current context so that on resume we start with
-		 * clean state, and dc hw programming optimizations will not
-		 * cause any trouble.
-		 */
-		dml = kzalloc(sizeof(struct display_mode_lib),
-				GFP_KERNEL);
-
-		ASSERT(dml);
-		if (!dml)
-			return false;
-
-		/* Preserve refcount */
-		refcount = dc->current_state->refcount;
-		/* Preserve display mode lib */
-		memcpy(dml, &dc->current_state->bw_ctx.dml, sizeof(struct display_mode_lib));
 
 		dc_resource_state_destruct(dc->current_state);
-		memset(dc->current_state, 0,
-				sizeof(*dc->current_state));
-
-		dc->current_state->refcount = refcount;
-		dc->current_state->bw_ctx.dml = *dml;
-
-		kfree(dml);
-
-#ifdef CONFIG_DRM_AMD_DC_FP
-		if (dc->debug.using_dml2)
-			dc->current_state->bw_ctx.dml2 = dml2;
-#endif
 
 		break;
 	}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index c1e2f0e10ab2..e2c7acdff301 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -4357,6 +4357,18 @@ void dc_resource_state_destruct(struct dc_state *context)
 		context->streams[i] = NULL;
 	}
 	context->stream_count = 0;
+	context->stream_mask = 0;
+	memset(&context->res_ctx, 0, sizeof(context->res_ctx));
+	memset(&context->pp_display_cfg, 0, sizeof(context->pp_display_cfg));
+	memset(&context->dcn_bw_vars, 0, sizeof(context->dcn_bw_vars));
+	context->clk_mgr = NULL;
+	memset(&context->bw_ctx.bw, 0, sizeof(context->bw_ctx.bw));
+	memset(context->block_sequence, 0, sizeof(context->block_sequence));
+	context->block_sequence_steps = 0;
+	memset(context->dc_dmub_cmd, 0, sizeof(context->dc_dmub_cmd));
+	context->dmub_cmd_count = 0;
+	memset(&context->perf_params, 0, sizeof(context->perf_params));
+	memset(&context->scratch, 0, sizeof(context->scratch));
 }
 
 void dc_resource_state_copy_construct(
-- 
2.34.1

Re: [PATCH v5 6/7] drm/amd/display: Destroy DC context while keeping DML and DML2

[Alex Deucher]

On Sat, Oct 7, 2023 at 3:47 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> If there is memory pressure at suspend time then dynamically
> allocating a large structure as part of DC suspend code will
> fail.
>
> Instead re-use the same structures and clear all members except
> those that should be maintained.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2362
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

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

> ---
> v4->v5:
>  * Rebase for DML2
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc.c      | 37 -------------------
>  .../gpu/drm/amd/display/dc/core/dc_resource.c | 12 ++++++
>  2 files changed, 12 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 2436a293931b..55e7b5a8ec8e 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -4704,12 +4704,6 @@ bool dc_set_power_state(
>         struct dc *dc,
>         enum dc_acpi_cm_power_state power_state)
>  {
> -       struct kref refcount;
> -       struct display_mode_lib *dml;
> -#ifdef CONFIG_DRM_AMD_DC_FP
> -       struct dml2_context *dml2 = NULL;
> -#endif
> -
>         if (!dc->current_state)
>                 return true;
>
> @@ -4728,40 +4722,9 @@ bool dc_set_power_state(
>
>                 break;
>         default:
> -#ifdef CONFIG_DRM_AMD_DC_FP
> -               if (dc->debug.using_dml2)
> -                       dml2 = dc->current_state->bw_ctx.dml2;
> -#endif
>                 ASSERT(dc->current_state->stream_count == 0);
> -               /* Zero out the current context so that on resume we start with
> -                * clean state, and dc hw programming optimizations will not
> -                * cause any trouble.
> -                */
> -               dml = kzalloc(sizeof(struct display_mode_lib),
> -                               GFP_KERNEL);
> -
> -               ASSERT(dml);
> -               if (!dml)
> -                       return false;
> -
> -               /* Preserve refcount */
> -               refcount = dc->current_state->refcount;
> -               /* Preserve display mode lib */
> -               memcpy(dml, &dc->current_state->bw_ctx.dml, sizeof(struct display_mode_lib));
>
>                 dc_resource_state_destruct(dc->current_state);
> -               memset(dc->current_state, 0,
> -                               sizeof(*dc->current_state));
> -
> -               dc->current_state->refcount = refcount;
> -               dc->current_state->bw_ctx.dml = *dml;
> -
> -               kfree(dml);
> -
> -#ifdef CONFIG_DRM_AMD_DC_FP
> -               if (dc->debug.using_dml2)
> -                       dc->current_state->bw_ctx.dml2 = dml2;
> -#endif
>
>                 break;
>         }
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index c1e2f0e10ab2..e2c7acdff301 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -4357,6 +4357,18 @@ void dc_resource_state_destruct(struct dc_state *context)
>                 context->streams[i] = NULL;
>         }
>         context->stream_count = 0;
> +       context->stream_mask = 0;
> +       memset(&context->res_ctx, 0, sizeof(context->res_ctx));
> +       memset(&context->pp_display_cfg, 0, sizeof(context->pp_display_cfg));
> +       memset(&context->dcn_bw_vars, 0, sizeof(context->dcn_bw_vars));
> +       context->clk_mgr = NULL;
> +       memset(&context->bw_ctx.bw, 0, sizeof(context->bw_ctx.bw));
> +       memset(context->block_sequence, 0, sizeof(context->block_sequence));
> +       context->block_sequence_steps = 0;
> +       memset(context->dc_dmub_cmd, 0, sizeof(context->dc_dmub_cmd));
> +       context->dmub_cmd_count = 0;
> +       memset(&context->perf_params, 0, sizeof(context->perf_params));
> +       memset(&context->scratch, 0, sizeof(context->scratch));
>  }
>
>  void dc_resource_state_copy_construct(
> --
> 2.34.1
>

[PATCH v5 7/7] drm/amd/display: make dc_set_power_state() return type `void` again

[Mario Limonciello]

As dc_set_power_state() no longer allocates memory, it's not necessary
to have return types and check return code as it can't fail anymore.

Change it back to `void`.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4->v5:
 * Pick up tag
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 +++++------------
 drivers/gpu/drm/amd/display/dc/core/dc.c        |  6 ++----
 drivers/gpu/drm/amd/display/dc/dc.h             |  2 +-
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 63944d3b9e8c..a293edf1ee40 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2685,11 +2685,6 @@ static void hpd_rx_irq_work_suspend(struct amdgpu_display_manager *dm)
 	}
 }
 
-static int dm_set_power_state(struct dc *dc, enum dc_acpi_cm_power_state power_state)
-{
-	return dc_set_power_state(dc, power_state) ? 0 : -ENOMEM;
-}
-
 static int dm_suspend(void *handle)
 {
 	struct amdgpu_device *adev = handle;
@@ -2725,7 +2720,9 @@ static int dm_suspend(void *handle)
 
 	hpd_rx_irq_work_suspend(dm);
 
-	return dm_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D3);
+	dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D3);
+
+	return 0;
 }
 
 struct drm_connector *
@@ -2919,9 +2916,7 @@ static int dm_resume(void *handle)
 		if (r)
 			DRM_ERROR("DMUB interface failed to initialize: status=%d\n", r);
 
-		r = dm_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
-		if (r)
-			return r;
+		dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
 
 		dc_resume(dm->dc);
 
@@ -2971,9 +2966,7 @@ static int dm_resume(void *handle)
 	}
 
 	/* power on hardware */
-	r = dm_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
-	if (r)
-		return r;
+	 dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
 
 	/* program HPD filter */
 	dc_resume(dm->dc);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 55e7b5a8ec8e..2e5e44f289a3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -4700,12 +4700,12 @@ void dc_power_down_on_boot(struct dc *dc)
 		dc->hwss.power_down_on_boot(dc);
 }
 
-bool dc_set_power_state(
+void dc_set_power_state(
 	struct dc *dc,
 	enum dc_acpi_cm_power_state power_state)
 {
 	if (!dc->current_state)
-		return true;
+		return;
 
 	switch (power_state) {
 	case DC_ACPI_CM_POWER_STATE_D0:
@@ -4728,8 +4728,6 @@ bool dc_set_power_state(
 
 		break;
 	}
-
-	return true;
 }
 
 void dc_resume(struct dc *dc)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 885f272a54c1..59f429064da2 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -2340,7 +2340,7 @@ void dc_notify_vsync_int_state(struct dc *dc, struct dc_stream_state *stream, bo
 
 /* Power Interfaces */
 
-bool dc_set_power_state(
+void dc_set_power_state(
 		struct dc *dc,
 		enum dc_acpi_cm_power_state power_state);
 void dc_resume(struct dc *dc);
-- 
2.34.1