[PATCH v2 2/2] drm/xe/display: Make display suspend/resume work on discrete

[Shankar, Uma]

> -----Original Message-----
> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Maarten
> Lankhorst
> Sent: Tuesday, August 6, 2024 4:21 PM
> To: intel-xe at lists.freedesktop.org
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>;
> stable at vger.kernel.org
> Subject: [PATCH v2 2/2] drm/xe/display: Make display suspend/resume work on
> discrete
> 
> We should unpin before evicting all memory, and repin after GT resume.
> This way, we preserve the contents of the framebuffers, and won't hang on
> resume due to migration engine not being restored yet.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Cc: <stable at vger.kernel.org> # v6.8+
> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 23 +++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_pm.c              | 11 ++++++-----
>  2 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> b/drivers/gpu/drm/xe/display/xe_display.c
> index d544d18ad1ecc..4b9ce1f34f4c7 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -283,6 +283,27 @@ static bool suspend_to_idle(void)
>  	return false;
>  }
> 
> +static void xe_display_flush_cleanup_work(struct xe_device *xe) {
> +	struct intel_crtc *crtc;
> +
> +	for_each_intel_crtc(&xe->drm, crtc) {
> +		struct drm_crtc_commit *commit;
> +
> +		spin_lock(&crtc->base.commit_lock);
> +		commit = list_first_entry_or_null(&crtc->base.commit_list,
> +						  struct drm_crtc_commit,
> commit_entry);
> +		if (commit)
> +			drm_crtc_commit_get(commit);
> +		spin_unlock(&crtc->base.commit_lock);
> +
> +		if (commit) {
> +			wait_for_completion(&commit->cleanup_done);
> +			drm_crtc_commit_put(commit);
> +		}
> +	}
> +}
> +
>  void xe_display_pm_suspend(struct xe_device *xe, bool runtime)  {
>  	bool s2idle = suspend_to_idle();
> @@ -303,6 +324,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool
> runtime)
>  	if (!runtime)
>  		intel_display_driver_suspend(xe);
> 
> +	xe_display_flush_cleanup_work(xe);
> +
>  	intel_dp_mst_suspend(xe);
> 
>  	intel_hpd_cancel_work(xe);
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index
> 9f3c14fd9f337..fcfb49af8c891 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -93,13 +93,13 @@ int xe_pm_suspend(struct xe_device *xe)
>  	for_each_gt(gt, xe, id)
>  		xe_gt_suspend_prepare(gt);
> 
> +	xe_display_pm_suspend(xe, false);
> +
>  	/* FIXME: Super racey... */
>  	err = xe_bo_evict_all(xe);
>  	if (err)
>  		goto err;
> 
> -	xe_display_pm_suspend(xe, false);
> -
>  	for_each_gt(gt, xe, id) {
>  		err = xe_gt_suspend(gt);
>  		if (err) {
> @@ -154,11 +154,11 @@ int xe_pm_resume(struct xe_device *xe)
> 
>  	xe_irq_resume(xe);
> 
> -	xe_display_pm_resume(xe, false);
> -
>  	for_each_gt(gt, xe, id)
>  		xe_gt_resume(gt);
> 
> +	xe_display_pm_resume(xe, false);
> +
>  	err = xe_bo_restore_user(xe);
>  	if (err)
>  		goto err;
> @@ -367,10 +367,11 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
>  	mutex_unlock(&xe->mem_access.vram_userfault.lock);
> 
>  	if (xe->d3cold.allowed) {
> +		xe_display_pm_suspend(xe, true);
> +
>  		err = xe_bo_evict_all(xe);
>  		if (err)
>  			goto out;
> -		xe_display_pm_suspend(xe, true);
>  	}
> 
>  	for_each_gt(gt, xe, id) {
> --
> 2.45.2