[CI v2 3/4] drm/xe: Move suballocator init to after display init

[Lucas De Marchi]

On Fri, Oct 18, 2024 at 07:06:30PM +0200, Maarten Lankhorst wrote:
>No allocations should be done before we have had a chance to preserve
>the display fb.
>
>Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>---
> drivers/gpu/drm/xe/xe_device.c |  6 ++++++
> drivers/gpu/drm/xe/xe_tile.c   | 12 ++++++++----
> drivers/gpu/drm/xe/xe_tile.h   |  1 +
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>index 51bb9d875268f..76d8c6868d15e 100644
>--- a/drivers/gpu/drm/xe/xe_device.c
>+++ b/drivers/gpu/drm/xe/xe_device.c
>@@ -736,6 +736,12 @@ int xe_device_probe(struct xe_device *xe)
> 	if (err)
> 		goto err;
>
>+	for_each_tile(tile, xe, id) {
>+		err = xe_tile_init(tile);
>+		if (err)
>+			goto err;
>+	}
>+
> 	for_each_gt(gt, xe, id) {
> 		last_gt = id;
>
>diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
>index 07cf7cfe4abd5..2825553b568f7 100644
>--- a/drivers/gpu/drm/xe/xe_tile.c
>+++ b/drivers/gpu/drm/xe/xe_tile.c
>@@ -170,10 +170,6 @@ int xe_tile_init_noalloc(struct xe_tile *tile)
> 	if (err)
> 		return err;
>
>-	tile->mem.kernel_bb_pool = xe_sa_bo_manager_init(tile, SZ_1M, 16);

ha... so this here was against the function doc, which was
supposed to be **_noalloc** and made this way exactly to cover the
display case:

/**
  * xe_tile_init_noalloc - Init tile up to the point where allocations can happen.
  * @tile: The tile to initialize.
  *
  * This function prepares the tile to allow memory allocations to VRAM, but is
  * not allowed to allocate memory itself. This state is useful for display
  * readout, because the inherited display framebuffer will otherwise be
  * overwritten as it is usually put at the start of VRAM.
   ...

I think this deserves a trailer so we backport this at least to 6.11:

| Fixes: 876611c2b756 ("drm/xe: Memory allocations are tile-based, not GT-based")

Before that commit the allocation was done later, on gt init, and hence
the xe_display_init_noaccel() was already called.


Lucas De Marchi

>-	if (IS_ERR(tile->mem.kernel_bb_pool))
>-		return PTR_ERR(tile->mem.kernel_bb_pool);
>-
> 	xe_wa_apply_tile_workarounds(tile);
>
> 	err = xe_tile_sysfs_init(tile);
>@@ -181,6 +177,14 @@ int xe_tile_init_noalloc(struct xe_tile *tile)
> 	return 0;
> }
>

missing kernel-doc here. Something along the lines:

/**
  * xe_tile_init
  ...

  * Do all the rest of tile initialization, including memory allocation.
  * After this point the tile is fully initialized.
  */

. I think it's important to note the order in
which the init functions are called and what's the expected pre/post
state.


thanks
Lucas De Marchi

>+int xe_tile_init(struct xe_tile *tile)
>+{
>+	tile->mem.kernel_bb_pool = xe_sa_bo_manager_init(tile, SZ_1M, 16);
>+	if (IS_ERR(tile->mem.kernel_bb_pool))
>+		return PTR_ERR(tile->mem.kernel_bb_pool);
>+
>+	return 0;
>+}
> void xe_tile_migrate_wait(struct xe_tile *tile)
> {
> 	xe_migrate_wait(tile->migrate);
>diff --git a/drivers/gpu/drm/xe/xe_tile.h b/drivers/gpu/drm/xe/xe_tile.h
>index 1c9e42ade6b05..eb939316d55b0 100644
>--- a/drivers/gpu/drm/xe/xe_tile.h
>+++ b/drivers/gpu/drm/xe/xe_tile.h
>@@ -12,6 +12,7 @@ struct xe_tile;
>
> int xe_tile_init_early(struct xe_tile *tile, struct xe_device *xe, u8 id);
> int xe_tile_init_noalloc(struct xe_tile *tile);
>+int xe_tile_init(struct xe_tile *tile);
>
> void xe_tile_migrate_wait(struct xe_tile *tile);
>
>-- 
>2.45.2
>