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

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

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;