[PATCH v3.1] drm/xe/display: Fix fbdev GGTT mapping handling.

[Lucas De Marchi]

On Wed, Feb 19, 2025 at 04:34:40PM +0100, Maarten Lankhorst wrote:
>The fbdev vma is a normal unrotated VMA, so add ane explicit check
>before re-using.
>
>When re-using, we have to restore the GGTT mapping on resume, so add
>some code to do that too.
>
>Fixes: 67a98f7e27ba ("drm/xe/display: Re-use display vmas when possible")
>Signed-off-by: Maarten Lankhorst <dev at lankhorst.se>
>---
> drivers/gpu/drm/xe/display/xe_display.c |  6 +++++-
> drivers/gpu/drm/xe/display/xe_fb_pin.c  | 24 +++++++++++++++++++++++-
> drivers/gpu/drm/xe/display/xe_fb_pin.h  | 13 +++++++++++++
> 3 files changed, 41 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/xe/display/xe_fb_pin.h
>
>diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>index 02a413a073824..999f25a562c19 100644
>--- a/drivers/gpu/drm/xe/display/xe_display.c
>+++ b/drivers/gpu/drm/xe/display/xe_display.c
>@@ -30,6 +30,7 @@
> #include "intel_hotplug.h"
> #include "intel_opregion.h"
> #include "skl_watermark.h"
>+#include "xe_fb_pin.h"
> #include "xe_module.h"
>
> /* Xe device functions */
>@@ -492,8 +493,11 @@ void xe_display_pm_resume(struct xe_device *xe)
> 		intel_display_driver_enable_user_access(display);
> 	}
>
>-	if (has_display(xe))
>+	if (has_display(xe)) {
>+		xe_fb_pin_resume(xe);

this looks odd. I remember when we had issues with LNL about pages
coming with garbage after a resume we talked about marking them as
"needed" on suspend so they'd be saved by mm. The ggtt afair was one of
them. Or did we go with a different approach and I'm misremembering?

+Matthew Brost / +Matthew Auld
I think this was about commit dd886a63d6e2 ("drm/xe: Restore system
memory GGTT mappings")

Lucas De Marchi

>+
> 		intel_hpd_poll_disable(xe);
>+	}
>
> 	intel_opregion_resume(display);
>
>diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>index 11a6b996d739b..dee1f6531c849 100644
>--- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>@@ -12,6 +12,7 @@
> #include "intel_fbdev.h"
> #include "xe_bo.h"
> #include "xe_device.h"
>+#include "xe_fb_pin.h"
> #include "xe_ggtt.h"
> #include "xe_pm.h"
>
>@@ -398,7 +399,8 @@ static bool reuse_vma(struct intel_plane_state *new_plane_state,
> 		goto found;
> 	}
>
>-	if (fb == intel_fbdev_framebuffer(xe->display.fbdev.fbdev)) {
>+	if (fb == intel_fbdev_framebuffer(xe->display.fbdev.fbdev) &&
>+	    new_plane_state->view.gtt.type == I915_GTT_VIEW_NORMAL) {
> 		vma = intel_fbdev_vma_pointer(xe->display.fbdev.fbdev);
> 		if (vma)
> 			goto found;
>@@ -444,6 +446,26 @@ void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
> 	old_plane_state->ggtt_vma = NULL;
> }
>
>+void xe_fb_pin_resume(struct xe_device *xe)
>+{
>+	struct xe_ggtt *ggtt = xe_device_get_root_tile(xe)->mem.ggtt;
>+	struct i915_vma *vma = intel_fbdev_vma_pointer(xe->display.fbdev.fbdev);
>+	struct xe_bo *bo;
>+
>+	if (!vma)
>+		return;
>+
>+	bo = vma->bo;
>+
>+	mutex_lock(&ggtt->lock);
>+	for (u32 x = 0; x < bo->ttm.base.size; x += XE_PAGE_SIZE) {
>+		u64 pte = ggtt->pt_ops->pte_encode_bo(bo, x, xe->pat.idx[XE_CACHE_NONE]);
>+
>+		ggtt->pt_ops->ggtt_set_pte(ggtt, vma->node->base.start + x, pte);
>+	}
>+	mutex_unlock(&ggtt->lock);
>+}
>+
> /*
>  * For Xe introduce dummy intel_dpt_create which just return NULL,
>  * intel_dpt_destroy which does nothing, and fake intel_dpt_ofsset returning 0;
>diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.h b/drivers/gpu/drm/xe/display/xe_fb_pin.h
>new file mode 100644
>index 0000000000000..39d48ff637002
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.h
>@@ -0,0 +1,13 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#ifndef _XE_FB_PIN_H_
>+#define _XE_FB_PIN_H_
>+
>+struct xe_device;
>+
>+void xe_fb_pin_resume(struct xe_device *xe);
>+
>+#endif /* _XE_FB_PIN_H_ */
>-- 
>2.47.1
>