[PATCH v2 0/6] drm/xe: More BIOS FB takeover fixes

[PATCH v2 0/6] drm/xe: More BIOS FB takeover fixes

[Maarten Lankhorst]

Small change since previous version, marking the entire GGTT as allocated
was causing regressions. So instead truncate to the allowed GGTT range.

This series separates the GGTT offset with the physical offset, and allows
us to fix a bug with MTL, where those are different.

Maarten Lankhorst (6):
  drm/xe/ggtt: Add xe_ggtt_insert_node_at
  drm/xe/ggtt: Add xe_ggtt_node_remove_noclear
  drm/xe/display: Reserve the original GGTT space before creating a bo
  drm/xe/display: Use the correct calculation for phys_base on
    integrated
  drm/xe/display: Remove duplicated code
  drm/xe/ggtt: Remove xe_ggtt_insert_bo_at

 drivers/gpu/drm/xe/display/xe_initial_plane.c | 62 ++++++++------
 drivers/gpu/drm/xe/xe_bo.c                    |  8 +-
 drivers/gpu/drm/xe/xe_ggtt.c                  | 81 ++++++++++++-------
 drivers/gpu/drm/xe/xe_ggtt.h                  |  5 +-
 4 files changed, 96 insertions(+), 60 deletions(-)

-- 
2.53.0

[PATCH v2 1/6] drm/xe/ggtt: Add xe_ggtt_insert_node_at

[Maarten Lankhorst]

Create a new function xe_ggtt_insert_node_at() which will be used
for reserving the part of GGTT where the initial framebuffer was
allocated.

This will allow us to either take over the initial mapping, or
reserve it to have the newly allocated GGTT mapping not overwriting
the initial mapping, which would cause flickering.

Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/xe/xe_ggtt.c | 35 +++++++++++++++++++++++++++++++----
 drivers/gpu/drm/xe/xe_ggtt.h |  2 ++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index 8ec23862477fc..c9f84db3bfecd 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -636,14 +636,17 @@ static struct xe_ggtt_node *ggtt_node_init(struct xe_ggtt *ggtt)
 }
 
 /**
- * xe_ggtt_insert_node - Insert a &xe_ggtt_node into the GGTT
+ * xe_ggtt_insert_node_at - Insert a &xe_ggtt_node into the GGTT
  * @ggtt: the &xe_ggtt into which the node should be inserted.
  * @size: size of the node
  * @align: alignment constrain of the node
+ * @start: Starting offset of range to insert node
+ * @end: Last offset for node insertion
  *
  * Return: &xe_ggtt_node on success or a ERR_PTR on failure.
  */
-struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 align)
+struct xe_ggtt_node *xe_ggtt_insert_node_at(struct xe_ggtt *ggtt, u32 size,
+					    u32 align, u64 start, u64 end)
 {
 	struct xe_ggtt_node *node;
 	int ret;
@@ -653,8 +656,19 @@ struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 ali
 		return node;
 
 	guard(mutex)(&ggtt->lock);
-	ret = xe_ggtt_insert_node_locked(node, size, align,
-					 DRM_MM_INSERT_HIGH);
+	if (start >= ggtt->start)
+		start -= ggtt->start;
+	else
+		start = 0;
+
+	/* Should never happen, but since we handle start, fail graciously for end */
+	if (end >= ggtt->start)
+		end -= ggtt->start;
+	else
+		end = 0;
+
+	ret = drm_mm_insert_node_in_range(&ggtt->mm, &node->base, size, align,
+					  0, start, end, DRM_MM_INSERT_HIGH);
 	if (ret) {
 		ggtt_node_fini(node);
 		return ERR_PTR(ret);
@@ -663,6 +677,19 @@ struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 ali
 	return node;
 }
 
+/**
+ * xe_ggtt_insert_node - Insert a &xe_ggtt_node into the GGTT
+ * @ggtt: the &xe_ggtt into which the node should be inserted.
+ * @size: size of the node
+ * @align: alignment constrain of the node
+ *
+ * Return: &xe_ggtt_node on success or a ERR_PTR on failure.
+ */
+struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 align)
+{
+	return xe_ggtt_insert_node_at(ggtt, size, align, 0, ~0ULL);
+}
+
 /**
  * xe_ggtt_node_pt_size() - Get the size of page table entries needed to map a GGTT node.
  * @node: the &xe_ggtt_node
diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
index c864cc975a695..69974da523f74 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.h
+++ b/drivers/gpu/drm/xe/xe_ggtt.h
@@ -22,6 +22,8 @@ void xe_ggtt_shift_nodes(struct xe_ggtt *ggtt, u64 new_base);
 u64 xe_ggtt_start(struct xe_ggtt *ggtt);
 u64 xe_ggtt_size(struct xe_ggtt *ggtt);
 
+struct xe_ggtt_node *
+xe_ggtt_insert_node_at(struct xe_ggtt *ggtt, u32 size, u32 align, u64 start, u64 end);
 struct xe_ggtt_node *
 xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 align);
 struct xe_ggtt_node *
-- 
2.53.0

[PATCH v2 2/6] drm/xe/ggtt: Add xe_ggtt_node_remove_noclear

[Maarten Lankhorst]

The last bit required for handling fb takeover is
ensuring we can release the old live framebuffer
without keeping track. The display code can then
safely perform the flip.

It's unfortunately a workaround for how display
is structured through callbacks instead of being
a midlayer.

Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/xe/xe_ggtt.c | 30 +++++++++++++++++++++++-------
 drivers/gpu/drm/xe/xe_ggtt.h |  1 +
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index c9f84db3bfecd..ff479e0a9f3b3 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -470,24 +470,40 @@ static void ggtt_node_fini(struct xe_ggtt_node *node)
 	kfree(node);
 }
 
-static void ggtt_node_remove(struct xe_ggtt_node *node)
+static void ggtt_node_remove(struct xe_ggtt_node *node, bool clear)
 {
 	struct xe_ggtt *ggtt = node->ggtt;
-	bool bound;
 
 	mutex_lock(&ggtt->lock);
-	bound = ggtt->flags & XE_GGTT_FLAGS_ONLINE;
-	if (bound)
+	if (clear)
+		clear = ggtt->flags & XE_GGTT_FLAGS_ONLINE;
+	if (clear)
 		xe_ggtt_clear(ggtt, xe_ggtt_node_addr(node), xe_ggtt_node_size(node));
 	drm_mm_remove_node(&node->base);
 	node->base.size = 0;
-	if (bound && node->invalidate_on_remove)
+	if (clear && node->invalidate_on_remove)
 		xe_ggtt_invalidate(ggtt);
 	mutex_unlock(&ggtt->lock);
 
 	ggtt_node_fini(node);
 }
 
+/**
+ * xe_ggtt_node_remove_noclear - Remove a &xe_ggtt_node from the GGTT without clearing entries
+ * @node: the &xe_ggtt_node to be removed
+ *
+ * This function is similar to xe_ggtt_node_remove(), but doesn't clear
+ * the entries. It's used to release the live FB mapping without
+ * clearing it.
+ *
+ * This function should only be called before xe_ggtt_init() in
+ * the bios FB takeover code.
+ */
+void xe_ggtt_node_remove_noclear(struct xe_ggtt_node *node)
+{
+	ggtt_node_remove(node, false);
+}
+
 static void ggtt_node_remove_work_func(struct work_struct *work)
 {
 	struct xe_ggtt_node *node = container_of(work, typeof(*node),
@@ -495,7 +511,7 @@ static void ggtt_node_remove_work_func(struct work_struct *work)
 	struct xe_device *xe = tile_to_xe(node->ggtt->tile);
 
 	guard(xe_pm_runtime)(xe);
-	ggtt_node_remove(node);
+	ggtt_node_remove(node, true);
 }
 
 /**
@@ -517,7 +533,7 @@ void xe_ggtt_node_remove(struct xe_ggtt_node *node, bool invalidate)
 	node->invalidate_on_remove = invalidate;
 
 	if (xe_pm_runtime_get_if_active(xe)) {
-		ggtt_node_remove(node);
+		ggtt_node_remove(node, true);
 		xe_pm_runtime_put(xe);
 	} else {
 		queue_work(ggtt->wq, &node->delayed_removal_work);
diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
index 69974da523f74..83654544feb6d 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.h
+++ b/drivers/gpu/drm/xe/xe_ggtt.h
@@ -32,6 +32,7 @@ xe_ggtt_insert_node_transform(struct xe_ggtt *ggtt,
 			      u64 size, u32 align,
 			      xe_ggtt_transform_cb transform, void *arg);
 void xe_ggtt_node_remove(struct xe_ggtt_node *node, bool invalidate);
+void xe_ggtt_node_remove_noclear(struct xe_ggtt_node *node);
 size_t xe_ggtt_node_pt_size(const struct xe_ggtt_node *node);
 void xe_ggtt_map_bo_unlocked(struct xe_ggtt *ggtt, struct xe_bo *bo);
 int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo, struct drm_exec *exec);
-- 
2.53.0

[PATCH v2 3/6] drm/xe/display: Reserve the original GGTT space before creating a bo

[Maarten Lankhorst]

Annotate the original area of the framebuffer as reserved in the GGTT
before creating a new GGTT entry. This allows us to remove the
range restrictions of GGTT in xe_bo_create_pin_map_at_novm().

Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/xe/display/xe_initial_plane.c | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
index 0f86b73036d03..f49dda8e28255 100644
--- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
+++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
@@ -37,6 +37,25 @@ static bool need_pte_local(struct xe_device *xe)
 	return IS_DGFX(xe) || has_lmembar(xe);
 }
 
+static struct xe_ggtt_node *reserve_original_node(struct xe_ggtt *ggtt, u32 base, u32 size, u64 page_size)
+{
+	u64 ggtt_start = xe_ggtt_start(ggtt), ggtt_end = ggtt_start + xe_ggtt_size(ggtt);
+
+	/* Completely truncated? */
+	if (base + size <= ggtt_start || base >= ggtt_end)
+		return NULL;
+
+	/* Partially truncated? */
+	if (base <= ggtt_start) {
+		size -= ggtt_start - base;
+		base = ggtt_start;
+	} else if (base + size >= ggtt_end) {
+		size = ggtt_end - base;
+	}
+
+	return xe_ggtt_insert_node_at(ggtt, size, page_size, base, base + size);
+}
+
 static struct xe_bo *
 initial_plane_bo(struct xe_device *xe,
 		 struct intel_initial_plane_config *plane_config)
@@ -46,6 +65,7 @@ initial_plane_bo(struct xe_device *xe,
 	resource_size_t phys_base;
 	u32 base, size, flags;
 	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K;
+	struct xe_ggtt_node *original_ggtt_node;
 
 	if (plane_config->size == 0)
 		return NULL;
@@ -111,8 +131,15 @@ initial_plane_bo(struct xe_device *xe,
 		}
 	}
 
+	original_ggtt_node = reserve_original_node(tile0->mem.ggtt, base, size, page_size);
+	if (IS_ERR(original_ggtt_node))
+		return NULL;
+
 	bo = xe_bo_create_pin_map_at_novm(xe, tile0, size, phys_base,
 					  ttm_bo_type_kernel, flags, 0, false);
+	if (original_ggtt_node)
+		xe_ggtt_node_remove_noclear(original_ggtt_node);
+
 	if (IS_ERR(bo)) {
 		drm_dbg_kms(&xe->drm,
 			    "Failed to create bo phys_base=%pa size %u with flags %x: %li\n",
-- 
2.53.0

[PATCH v2 4/6] drm/xe/display: Use the correct calculation for phys_base on integrated

[Maarten Lankhorst]

The code originally assumed there was a 1:1 mapping between
stolen and GGTT offset. This is no longer true as of MTL, so
perform the correct calculations, similar to how they're
performed for the DGFX case.

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/xe/display/xe_initial_plane.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
index f49dda8e28255..5540b0fca392a 100644
--- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
+++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
@@ -120,7 +120,7 @@ initial_plane_bo(struct xe_device *xe,
 			return NULL;
 		}
 
-		phys_base = base;
+		phys_base = (pte & ~(page_size - 1)) - xe_ttm_stolen_gpu_offset(xe);
 		flags |= XE_BO_FLAG_STOLEN;
 
 		if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
-- 
2.53.0

[PATCH v2 5/6] drm/xe/display: Remove duplicated code

[Maarten Lankhorst]

The order of pte vs checks isn't important, so read the pte
outside the if block. This makes it slightly more readable.

Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/xe/display/xe_initial_plane.c | 35 ++++++-------------
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
index 5540b0fca392a..e16a6a1e6288a 100644
--- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
+++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
@@ -64,7 +64,7 @@ initial_plane_bo(struct xe_device *xe,
 	struct xe_bo *bo;
 	resource_size_t phys_base;
 	u32 base, size, flags;
-	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K;
+	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K, pte;
 	struct xe_ggtt_node *original_ggtt_node;
 
 	if (plane_config->size == 0)
@@ -77,16 +77,14 @@ initial_plane_bo(struct xe_device *xe,
 			page_size);
 	size -= base;
 
-	if (IS_DGFX(xe)) {
-		u64 pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
-
-		if (is_pte_local(pte) != need_pte_local(xe)) {
-			drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
-			return NULL;
-		}
-
-		phys_base = pte & ~(page_size - 1);
+	pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
+	phys_base = pte & ~(page_size - 1);
+	if (is_pte_local(pte) != need_pte_local(xe)) {
+		drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
+		return NULL;
+	}
 
+	if (IS_DGFX(xe)) {
 		flags |= XE_BO_FLAG_VRAM0;
 
 		/*
@@ -104,25 +102,14 @@ initial_plane_bo(struct xe_device *xe,
 			    "Using phys_base=%pa, based on initial plane programming\n",
 			    &phys_base);
 	} else {
-		struct ttm_resource_manager *stolen;
-		u64 pte;
+		flags |= XE_BO_FLAG_STOLEN;
+		phys_base -= xe_ttm_stolen_gpu_offset(xe);
 
-		stolen = ttm_manager_type(&xe->ttm, XE_PL_STOLEN);
-		if (!stolen) {
+		if (!ttm_manager_type(&xe->ttm, XE_PL_STOLEN)) {
 			drm_dbg_kms(&xe->drm, "No stolen for initial FB\n");
 			return NULL;
 		}
 
-		pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
-
-		if (is_pte_local(pte) != need_pte_local(xe)) {
-			drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
-			return NULL;
-		}
-
-		phys_base = (pte & ~(page_size - 1)) - xe_ttm_stolen_gpu_offset(xe);
-		flags |= XE_BO_FLAG_STOLEN;
-
 		if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
 		    IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
 		    !xe_display_bo_fbdev_prefer_stolen(xe, plane_config->size)) {
-- 
2.53.0

[PATCH v2 6/6] drm/xe/ggtt: Remove xe_ggtt_insert_bo_at

[Maarten Lankhorst]

This code was created specifically for the display handover,
and can be removed now that we temporarily reserve the area
of GGTT that contains the original framebuffer contents.

Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/xe/xe_bo.c   |  8 +-------
 drivers/gpu/drm/xe/xe_ggtt.c | 16 ----------------
 drivers/gpu/drm/xe/xe_ggtt.h |  2 --
 3 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index c266fa6bade1b..5d7d91444dce3 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -2560,13 +2560,7 @@ __xe_bo_create_locked(struct xe_device *xe,
 			if (t != tile && !(bo->flags & XE_BO_FLAG_GGTTx(t)))
 				continue;
 
-			if (flags & XE_BO_FLAG_FIXED_PLACEMENT) {
-				err = xe_ggtt_insert_bo_at(t->mem.ggtt, bo,
-							   start + xe_bo_size(bo), U64_MAX,
-							   exec);
-			} else {
-				err = xe_ggtt_insert_bo(t->mem.ggtt, bo, exec);
-			}
+			err = xe_ggtt_insert_bo(t->mem.ggtt, bo, exec);
 			if (err)
 				goto err_unlock_put_bo;
 		}
diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index ff479e0a9f3b3..017e7eeeb2c50 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -900,22 +900,6 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
 	return err;
 }
 
-/**
- * xe_ggtt_insert_bo_at - Insert BO at a specific GGTT space
- * @ggtt: the &xe_ggtt where bo will be inserted
- * @bo: the &xe_bo to be inserted
- * @start: address where it will be inserted
- * @end: end of the range where it will be inserted
- * @exec: The drm_exec transaction to use for exhaustive eviction.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
-			 u64 start, u64 end, struct drm_exec *exec)
-{
-	return __xe_ggtt_insert_bo_at(ggtt, bo, start, end, exec);
-}
-
 /**
  * xe_ggtt_insert_bo - Insert BO into GGTT
  * @ggtt: the &xe_ggtt where bo will be inserted
diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
index 83654544feb6d..02e72dbae0e58 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.h
+++ b/drivers/gpu/drm/xe/xe_ggtt.h
@@ -36,8 +36,6 @@ void xe_ggtt_node_remove_noclear(struct xe_ggtt_node *node);
 size_t xe_ggtt_node_pt_size(const struct xe_ggtt_node *node);
 void xe_ggtt_map_bo_unlocked(struct xe_ggtt *ggtt, struct xe_bo *bo);
 int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo, struct drm_exec *exec);
-int xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
-			 u64 start, u64 end, struct drm_exec *exec);
 void xe_ggtt_remove_bo(struct xe_ggtt *ggtt, struct xe_bo *bo);
 u64 xe_ggtt_largest_hole(struct xe_ggtt *ggtt, u64 alignment, u64 *spare);
 
-- 
2.53.0

Re: [PATCH v2 5/6] drm/xe/display: Remove duplicated code

[Maarten Lankhorst]

On 7/15/26 13:05, Maarten Lankhorst wrote:
> The order of pte vs checks isn't important, so read the pte
> outside the if block. This makes it slightly more readable.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/xe/display/xe_initial_plane.c | 35 ++++++-------------
>  1 file changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> index 5540b0fca392a..e16a6a1e6288a 100644
> --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> @@ -64,7 +64,7 @@ initial_plane_bo(struct xe_device *xe,
>  	struct xe_bo *bo;
>  	resource_size_t phys_base;
>  	u32 base, size, flags;
> -	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K;
> +	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K, pte;
>  	struct xe_ggtt_node *original_ggtt_node;
>  
>  	if (plane_config->size == 0)
> @@ -77,16 +77,14 @@ initial_plane_bo(struct xe_device *xe,
>  			page_size);
>  	size -= base;
>  
> -	if (IS_DGFX(xe)) {
> -		u64 pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> -
> -		if (is_pte_local(pte) != need_pte_local(xe)) {
> -			drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
> -			return NULL;
> -		}
> -
> -		phys_base = pte & ~(page_size - 1);
> +	pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> +	phys_base = pte & ~(page_size - 1);
> +	if (is_pte_local(pte) != need_pte_local(xe)) {
> +		drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
> +		return NULL;
> +	}
>  
> +	if (IS_DGFX(xe)) {
>  		flags |= XE_BO_FLAG_VRAM0;
>  
>  		/*
> @@ -104,25 +102,14 @@ initial_plane_bo(struct xe_device *xe,
>  			    "Using phys_base=%pa, based on initial plane programming\n",
>  			    &phys_base);
>  	} else {
> -		struct ttm_resource_manager *stolen;
> -		u64 pte;
> +		flags |= XE_BO_FLAG_STOLEN;
> +		phys_base -= xe_ttm_stolen_gpu_offset(xe);
>  
> -		stolen = ttm_manager_type(&xe->ttm, XE_PL_STOLEN);
> -		if (!stolen) {
> +		if (!ttm_manager_type(&xe->ttm, XE_PL_STOLEN)) {
>  			drm_dbg_kms(&xe->drm, "No stolen for initial FB\n");
>  			return NULL;
>  		}
>

Woops, phys_base adjustment should be after !stolen check,
result should be the same though.

> -		pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> -
> -		if (is_pte_local(pte) != need_pte_local(xe)) {
> -			drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
> -			return NULL;
> -		}
> -
> -		phys_base = (pte & ~(page_size - 1)) - xe_ttm_stolen_gpu_offset(xe);
> -		flags |= XE_BO_FLAG_STOLEN;
> -
>  		if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
>  		    IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
>  		    !xe_display_bo_fbdev_prefer_stolen(xe, plane_config->size)) {

✓ i915.CI.BAT: success for drm/xe: More BIOS FB takeover fixes (rev2)

[Patchwork]

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

== Series Details ==

Series: drm/xe: More BIOS FB takeover fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/170431/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_18827 -> Patchwork_170431v2
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170431v2/index.html

Participating hosts (41 -> 39)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 


Changes
-------

  No changes found


Build changes
-------------

  * Linux: CI_DRM_18827 -> Patchwork_170431v2

  CI-20190529: 20190529
  CI_DRM_18827: 6dd678fdc2f39062bed466d7e3c851736e376531 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9006: 6380a8af26359dd222e22679442272ded836c463 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_170431v2: 6dd678fdc2f39062bed466d7e3c851736e376531 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170431v2/index.html

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

✓ i915.CI.Full: success for drm/xe: More BIOS FB takeover fixes (rev2)

[Patchwork]

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

== Series Details ==

Series: drm/xe: More BIOS FB takeover fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/170431/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_18827_full -> Patchwork_170431v2_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Changes
-------

  No changes found


Build changes
-------------

  * Linux: CI_DRM_18827 -> Patchwork_170431v2

  CI-20190529: 20190529
  CI_DRM_18827: 6dd678fdc2f39062bed466d7e3c851736e376531 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9006: 6380a8af26359dd222e22679442272ded836c463 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_170431v2: 6dd678fdc2f39062bed466d7e3c851736e376531 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170431v2/index.html

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

Re: [PATCH v2 1/6] drm/xe/ggtt: Add xe_ggtt_insert_node_at

[Ville Syrjälä]

On Wed, Jul 15, 2026 at 01:05:52PM +0200, Maarten Lankhorst wrote:
> Create a new function xe_ggtt_insert_node_at() which will be used
> for reserving the part of GGTT where the initial framebuffer was
> allocated.
> 
> This will allow us to either take over the initial mapping, or
> reserve it to have the newly allocated GGTT mapping not overwriting
> the initial mapping, which would cause flickering.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 35 +++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/xe/xe_ggtt.h |  2 ++
>  2 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 8ec23862477fc..c9f84db3bfecd 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -636,14 +636,17 @@ static struct xe_ggtt_node *ggtt_node_init(struct xe_ggtt *ggtt)
>  }
>  
>  /**
> - * xe_ggtt_insert_node - Insert a &xe_ggtt_node into the GGTT
> + * xe_ggtt_insert_node_at - Insert a &xe_ggtt_node into the GGTT
>   * @ggtt: the &xe_ggtt into which the node should be inserted.
>   * @size: size of the node
>   * @align: alignment constrain of the node
> + * @start: Starting offset of range to insert node
> + * @end: Last offset for node insertion
>   *
>   * Return: &xe_ggtt_node on success or a ERR_PTR on failure.
>   */
> -struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 align)
> +struct xe_ggtt_node *xe_ggtt_insert_node_at(struct xe_ggtt *ggtt, u32 size,
> +					    u32 align, u64 start, u64 end)
>  {
>  	struct xe_ggtt_node *node;
>  	int ret;
> @@ -653,8 +656,19 @@ struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 ali
>  		return node;
>  
>  	guard(mutex)(&ggtt->lock);
> -	ret = xe_ggtt_insert_node_locked(node, size, align,
> -					 DRM_MM_INSERT_HIGH);
> +	if (start >= ggtt->start)
> +		start -= ggtt->start;
> +	else
> +		start = 0;
> +
> +	/* Should never happen, but since we handle start, fail graciously for end */

I remember seeing this weird comment in the existing code as well.
I confused me then and still does. Which should never happen, the
'if' or the 'else'? And both cases seem entirely possible to me. 
The default end==~0ull is certainly going to hit the 'if', and the
initial fb can certainly be fully below ggtt->start so 'else' seems
possible as well.

> +	if (end >= ggtt->start)
> +		end -= ggtt->start;
> +	else
> +		end = 0;
> +
> +	ret = drm_mm_insert_node_in_range(&ggtt->mm, &node->base, size, align,
> +					  0, start, end, DRM_MM_INSERT_HIGH);

That is going to fail if the size matches the original range, and
then we reduce the range due to ggtt_start/end.

Can I presume the drm_mm code can deal with the start/end > ggtt_end case?

Hmm, I now see that you handle those cases in the caller in the
later patch. But that just makes just this whole function feel
rather strange; Why do we even allow start/end that aren't within
the valid range for the mm if the caller has to handle that anyway?
OTOH I guess the 0/~0ull stuff does need this here :/

>  	if (ret) {
>  		ggtt_node_fini(node);
>  		return ERR_PTR(ret);
> @@ -663,6 +677,19 @@ struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 ali
>  	return node;
>  }
>  
> +/**
> + * xe_ggtt_insert_node - Insert a &xe_ggtt_node into the GGTT
> + * @ggtt: the &xe_ggtt into which the node should be inserted.
> + * @size: size of the node
> + * @align: alignment constrain of the node
> + *
> + * Return: &xe_ggtt_node on success or a ERR_PTR on failure.
> + */
> +struct xe_ggtt_node *xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 align)
> +{
> +	return xe_ggtt_insert_node_at(ggtt, size, align, 0, ~0ULL);
> +}
> +
>  /**
>   * xe_ggtt_node_pt_size() - Get the size of page table entries needed to map a GGTT node.
>   * @node: the &xe_ggtt_node
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index c864cc975a695..69974da523f74 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -22,6 +22,8 @@ void xe_ggtt_shift_nodes(struct xe_ggtt *ggtt, u64 new_base);
>  u64 xe_ggtt_start(struct xe_ggtt *ggtt);
>  u64 xe_ggtt_size(struct xe_ggtt *ggtt);
>  
> +struct xe_ggtt_node *
> +xe_ggtt_insert_node_at(struct xe_ggtt *ggtt, u32 size, u32 align, u64 start, u64 end);
>  struct xe_ggtt_node *
>  xe_ggtt_insert_node(struct xe_ggtt *ggtt, u32 size, u32 align);
>  struct xe_ggtt_node *
> -- 
> 2.53.0

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 3/6] drm/xe/display: Reserve the original GGTT space before creating a bo

[Ville Syrjälä]

On Wed, Jul 15, 2026 at 01:05:54PM +0200, Maarten Lankhorst wrote:
> Annotate the original area of the framebuffer as reserved in the GGTT
> before creating a new GGTT entry. This allows us to remove the
> range restrictions of GGTT in xe_bo_create_pin_map_at_novm().
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/xe/display/xe_initial_plane.c | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> index 0f86b73036d03..f49dda8e28255 100644
> --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> @@ -37,6 +37,25 @@ static bool need_pte_local(struct xe_device *xe)
>  	return IS_DGFX(xe) || has_lmembar(xe);
>  }
>  
> +static struct xe_ggtt_node *reserve_original_node(struct xe_ggtt *ggtt, u32 base, u32 size, u64 page_size)
> +{
> +	u64 ggtt_start = xe_ggtt_start(ggtt), ggtt_end = ggtt_start + xe_ggtt_size(ggtt);
> +
> +	/* Completely truncated? */
> +	if (base + size <= ggtt_start || base >= ggtt_end)
> +		return NULL;
> +
> +	/* Partially truncated? */
> +	if (base <= ggtt_start) {
> +		size -= ggtt_start - base;
> +		base = ggtt_start;
> +	} else if (base + size >= ggtt_end) {
> +		size = ggtt_end - base;
> +	}

This is exactly what I didn't want to have in the display code.
These are xe_ggtt.c implementation details that we shouldn't
have to know. So IMO either xe_ggtt_insert_node_at() should handle
all of it, or this reserve_original_node() wrapper should live in
xe_ggtt.c.

> +
> +	return xe_ggtt_insert_node_at(ggtt, size, page_size, base, base + size);
> +}
> +
>  static struct xe_bo *
>  initial_plane_bo(struct xe_device *xe,
>  		 struct intel_initial_plane_config *plane_config)
> @@ -46,6 +65,7 @@ initial_plane_bo(struct xe_device *xe,
>  	resource_size_t phys_base;
>  	u32 base, size, flags;
>  	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K;
> +	struct xe_ggtt_node *original_ggtt_node;
>  
>  	if (plane_config->size == 0)
>  		return NULL;
> @@ -111,8 +131,15 @@ initial_plane_bo(struct xe_device *xe,
>  		}
>  	}
>  
> +	original_ggtt_node = reserve_original_node(tile0->mem.ggtt, base, size, page_size);
> +	if (IS_ERR(original_ggtt_node))
> +		return NULL;
> +
>  	bo = xe_bo_create_pin_map_at_novm(xe, tile0, size, phys_base,
>  					  ttm_bo_type_kernel, flags, 0, false);
> +	if (original_ggtt_node)
> +		xe_ggtt_node_remove_noclear(original_ggtt_node);
> +
>  	if (IS_ERR(bo)) {
>  		drm_dbg_kms(&xe->drm,
>  			    "Failed to create bo phys_base=%pa size %u with flags %x: %li\n",
> -- 
> 2.53.0

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 6/6] drm/xe/ggtt: Remove xe_ggtt_insert_bo_at

[Ville Syrjälä]

On Wed, Jul 15, 2026 at 01:05:57PM +0200, Maarten Lankhorst wrote:
> This code was created specifically for the display handover,
> and can be removed now that we temporarily reserve the area
> of GGTT that contains the original framebuffer contents.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/xe/xe_bo.c   |  8 +-------
>  drivers/gpu/drm/xe/xe_ggtt.c | 16 ----------------
>  drivers/gpu/drm/xe/xe_ggtt.h |  2 --
>  3 files changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index c266fa6bade1b..5d7d91444dce3 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2560,13 +2560,7 @@ __xe_bo_create_locked(struct xe_device *xe,
>  			if (t != tile && !(bo->flags & XE_BO_FLAG_GGTTx(t)))
>  				continue;
>  
> -			if (flags & XE_BO_FLAG_FIXED_PLACEMENT) {
> -				err = xe_ggtt_insert_bo_at(t->mem.ggtt, bo,
> -							   start + xe_bo_size(bo), U64_MAX,
> -							   exec);

The commit message should probable mention that this
ggtt_start==phys_start assumption was nonsense to begin
with.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> -			} else {
> -				err = xe_ggtt_insert_bo(t->mem.ggtt, bo, exec);
> -			}
> +			err = xe_ggtt_insert_bo(t->mem.ggtt, bo, exec);
>  			if (err)
>  				goto err_unlock_put_bo;
>  		}
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index ff479e0a9f3b3..017e7eeeb2c50 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -900,22 +900,6 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
>  	return err;
>  }
>  
> -/**
> - * xe_ggtt_insert_bo_at - Insert BO at a specific GGTT space
> - * @ggtt: the &xe_ggtt where bo will be inserted
> - * @bo: the &xe_bo to be inserted
> - * @start: address where it will be inserted
> - * @end: end of the range where it will be inserted
> - * @exec: The drm_exec transaction to use for exhaustive eviction.
> - *
> - * Return: 0 on success or a negative error code on failure.
> - */
> -int xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
> -			 u64 start, u64 end, struct drm_exec *exec)
> -{
> -	return __xe_ggtt_insert_bo_at(ggtt, bo, start, end, exec);
> -}
> -
>  /**
>   * xe_ggtt_insert_bo - Insert BO into GGTT
>   * @ggtt: the &xe_ggtt where bo will be inserted
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index 83654544feb6d..02e72dbae0e58 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -36,8 +36,6 @@ void xe_ggtt_node_remove_noclear(struct xe_ggtt_node *node);
>  size_t xe_ggtt_node_pt_size(const struct xe_ggtt_node *node);
>  void xe_ggtt_map_bo_unlocked(struct xe_ggtt *ggtt, struct xe_bo *bo);
>  int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo, struct drm_exec *exec);
> -int xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
> -			 u64 start, u64 end, struct drm_exec *exec);
>  void xe_ggtt_remove_bo(struct xe_ggtt *ggtt, struct xe_bo *bo);
>  u64 xe_ggtt_largest_hole(struct xe_ggtt *ggtt, u64 alignment, u64 *spare);
>  
> -- 
> 2.53.0

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 2/6] drm/xe/ggtt: Add xe_ggtt_node_remove_noclear

[Ville Syrjälä]

On Wed, Jul 15, 2026 at 01:05:53PM +0200, Maarten Lankhorst wrote:
> The last bit required for handling fb takeover is
> ensuring we can release the old live framebuffer
> without keeping track. The display code can then
> safely perform the flip.
> 
> It's unfortunately a workaround for how display
> is structured through callbacks instead of being
> a midlayer.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 30 +++++++++++++++++++++++-------
>  drivers/gpu/drm/xe/xe_ggtt.h |  1 +
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index c9f84db3bfecd..ff479e0a9f3b3 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -470,24 +470,40 @@ static void ggtt_node_fini(struct xe_ggtt_node *node)
>  	kfree(node);
>  }
>  
> -static void ggtt_node_remove(struct xe_ggtt_node *node)
> +static void ggtt_node_remove(struct xe_ggtt_node *node, bool clear)
>  {
>  	struct xe_ggtt *ggtt = node->ggtt;
> -	bool bound;
>  
>  	mutex_lock(&ggtt->lock);
> -	bound = ggtt->flags & XE_GGTT_FLAGS_ONLINE;
> -	if (bound)
> +	if (clear)
> +		clear = ggtt->flags & XE_GGTT_FLAGS_ONLINE;
> +	if (clear)
>  		xe_ggtt_clear(ggtt, xe_ggtt_node_addr(node), xe_ggtt_node_size(node));
>  	drm_mm_remove_node(&node->base);
>  	node->base.size = 0;
> -	if (bound && node->invalidate_on_remove)
> +	if (clear && node->invalidate_on_remove)
>  		xe_ggtt_invalidate(ggtt);
>  	mutex_unlock(&ggtt->lock);
>  
>  	ggtt_node_fini(node);
>  }
>  
> +/**
> + * xe_ggtt_node_remove_noclear - Remove a &xe_ggtt_node from the GGTT without clearing entries
> + * @node: the &xe_ggtt_node to be removed
> + *
> + * This function is similar to xe_ggtt_node_remove(), but doesn't clear
> + * the entries. It's used to release the live FB mapping without
> + * clearing it.
> + *
> + * This function should only be called before xe_ggtt_init() in
> + * the bios FB takeover code.
> + */
> +void xe_ggtt_node_remove_noclear(struct xe_ggtt_node *node)
> +{
> +	ggtt_node_remove(node, false);
> +}
> +
>  static void ggtt_node_remove_work_func(struct work_struct *work)
>  {
>  	struct xe_ggtt_node *node = container_of(work, typeof(*node),
> @@ -495,7 +511,7 @@ static void ggtt_node_remove_work_func(struct work_struct *work)
>  	struct xe_device *xe = tile_to_xe(node->ggtt->tile);
>  
>  	guard(xe_pm_runtime)(xe);
> -	ggtt_node_remove(node);
> +	ggtt_node_remove(node, true);
>  }
>  
>  /**
> @@ -517,7 +533,7 @@ void xe_ggtt_node_remove(struct xe_ggtt_node *node, bool invalidate)
>  	node->invalidate_on_remove = invalidate;
>  
>  	if (xe_pm_runtime_get_if_active(xe)) {
> -		ggtt_node_remove(node);
> +		ggtt_node_remove(node, true);
>  		xe_pm_runtime_put(xe);
>  	} else {
>  		queue_work(ggtt->wq, &node->delayed_removal_work);
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index 69974da523f74..83654544feb6d 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -32,6 +32,7 @@ xe_ggtt_insert_node_transform(struct xe_ggtt *ggtt,
>  			      u64 size, u32 align,
>  			      xe_ggtt_transform_cb transform, void *arg);
>  void xe_ggtt_node_remove(struct xe_ggtt_node *node, bool invalidate);
> +void xe_ggtt_node_remove_noclear(struct xe_ggtt_node *node);
>  size_t xe_ggtt_node_pt_size(const struct xe_ggtt_node *node);
>  void xe_ggtt_map_bo_unlocked(struct xe_ggtt *ggtt, struct xe_bo *bo);
>  int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo, struct drm_exec *exec);
> -- 
> 2.53.0

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 5/6] drm/xe/display: Remove duplicated code

[Ville Syrjälä]

On Wed, Jul 15, 2026 at 01:34:28PM +0200, Maarten Lankhorst wrote:
> 
> 
> On 7/15/26 13:05, Maarten Lankhorst wrote:
> > The order of pte vs checks isn't important, so read the pte
> > outside the if block. This makes it slightly more readable.
> > 
> > Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> > ---
> >  drivers/gpu/drm/xe/display/xe_initial_plane.c | 35 ++++++-------------
> >  1 file changed, 11 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> > index 5540b0fca392a..e16a6a1e6288a 100644
> > --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
> > +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
> > @@ -64,7 +64,7 @@ initial_plane_bo(struct xe_device *xe,
> >  	struct xe_bo *bo;
> >  	resource_size_t phys_base;
> >  	u32 base, size, flags;
> > -	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K;
> > +	u64 page_size = xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K, pte;
> >  	struct xe_ggtt_node *original_ggtt_node;
> >  
> >  	if (plane_config->size == 0)
> > @@ -77,16 +77,14 @@ initial_plane_bo(struct xe_device *xe,
> >  			page_size);
> >  	size -= base;
> >  
> > -	if (IS_DGFX(xe)) {
> > -		u64 pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> > -
> > -		if (is_pte_local(pte) != need_pte_local(xe)) {
> > -			drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
> > -			return NULL;
> > -		}
> > -
> > -		phys_base = pte & ~(page_size - 1);
> > +	pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> > +	phys_base = pte & ~(page_size - 1);
> > +	if (is_pte_local(pte) != need_pte_local(xe)) {
> > +		drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
> > +		return NULL;
> > +	}
> >  
> > +	if (IS_DGFX(xe)) {
> >  		flags |= XE_BO_FLAG_VRAM0;
> >  
> >  		/*
> > @@ -104,25 +102,14 @@ initial_plane_bo(struct xe_device *xe,
> >  			    "Using phys_base=%pa, based on initial plane programming\n",
> >  			    &phys_base);
> >  	} else {
> > -		struct ttm_resource_manager *stolen;
> > -		u64 pte;
> > +		flags |= XE_BO_FLAG_STOLEN;
> > +		phys_base -= xe_ttm_stolen_gpu_offset(xe);
> >  
> > -		stolen = ttm_manager_type(&xe->ttm, XE_PL_STOLEN);
> > -		if (!stolen) {
> > +		if (!ttm_manager_type(&xe->ttm, XE_PL_STOLEN)) {
> >  			drm_dbg_kms(&xe->drm, "No stolen for initial FB\n");
> >  			return NULL;
> >  		}
> >
> 
> Woops, phys_base adjustment should be after !stolen check,
> result should be the same though.

I would still like to see the dma_addr/phys_base/ggtt offset
(old and new) debug stuff from
https://patchwork.freedesktop.org/patch/724207/?series=166362&rev=1
sucked in as well, as a separate patch. It's useful to see all of
that when debugging this stuff...

But this patch seems fine, with the reordering to avoid oopsing
if stolen isn't there
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> > -		pte = xe_ggtt_read_pte(tile0->mem.ggtt, base);
> > -
> > -		if (is_pte_local(pte) != need_pte_local(xe)) {
> > -			drm_err(&xe->drm, "Initial plane PTE has bad local memory bit\n");
> > -			return NULL;
> > -		}
> > -
> > -		phys_base = (pte & ~(page_size - 1)) - xe_ttm_stolen_gpu_offset(xe);
> > -		flags |= XE_BO_FLAG_STOLEN;
> > -
> >  		if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
> >  		    IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> >  		    !xe_display_bo_fbdev_prefer_stolen(xe, plane_config->size)) {

-- 
Ville Syrjälä
Intel