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 <[email protected]>
> ---
> 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
