Re: [PATCH v3 3/3] drm/xe/vf: Use drm mm instead of drm sa for CCS read/write

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Wed, 2026-04-01 at 16:15 +0000, Satyanarayana K V P wrote:
> The suballocator algorithm tracks a hole cursor at the last
> allocation
> and tries to allocate after it. This is optimized for fence-ordered
> progress, where older allocations are expected to become reusable
> first.
> 
> In fence-enabled mode, that ordering assumption holds. In fence-
> disabled
> mode, allocations may be freed in arbitrary order, so limiting
> allocation
> to the current hole window can miss valid free space and fail
> allocations
> despite sufficient total space.
> 
> Use DRM memory manager instead of sub-allocator to get rid of this
> issue
> as CCS read/write operations do not use fences.
> 
> Fixes: 864690cf4dd62 ("drm/xe/vf: Attach and detach CCS copy commands
> with BO")
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Maarten Lankhorst <dev@lankhorst.se>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 
> ---
> V2 -> V3:
> - Used xe_mem_pool_init() and xe_mem_pool_shadow_init() to allocate
> BB
> pools.
> 
> V1 -> V2:
> - Renamed xe_drm_mm to xe_mm_suballoc (Thomas)

Claude's kreview:

commit 937a5e27ca20b0216530e9df59e85acfe2e3c1fb
Author: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>

drm/xe/vf: Use drm mm instead of drm sa for CCS read/write

The suballocator used for CCS batch buffer pools tracks a hole cursor
optimized for fence-ordered frees; in fence-disabled mode, out-of-order
frees can leave valid free space unreachable, causing allocation
failures
despite sufficient total space.  This patch replaces the suballocator
with drm_mm, which does not have that ordering constraint.

> diff --git a/drivers/gpu/drm/xe/xe_bo_types.h
b/drivers/gpu/drm/xe/xe_bo_types.h
> index d4fe3c8dca5b..97d8f9107434 100644
> --- a/drivers/gpu/drm/xe/xe_bo_types.h
> +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> @@ -18,6 +18,7 @@
>  #include "xe_ggtt_types.h"
>
>  struct xe_device;
> +struct xe_mm_sa_bb;
>  struct xe_vm;

This isn't a bug, but the forward declaration names a type xe_mm_sa_bb
that does not appear to be defined anywhere in the driver.  The field
added below uses struct xe_mem_pool_bb, not struct xe_mm_sa_bb.

Should this be:

    struct xe_mem_pool_bb;

or removed entirely?

> [...]
>
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> index db023fb66a27..3a0abe519536 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> @@ -141,43 +141,39 @@ static u64 get_ccs_bb_pool_size(struct
xe_device *xe)
>
>  static int alloc_bb_pool(struct xe_tile *tile, struct
xe_sriov_vf_ccs_ctx *ctx)
>  {
> [ ... ]
> +	pool_manager = xe_mem_pool_init(tile, bb_pool_size, flags);
> +	if (IS_ERR(pool_manager)) {
> +		xe_sriov_err(xe, "xe_mem_pool_init init failed with
error: %pe\n",
> +			     pool_manager);

The error string "xe_mem_pool_init init" has the word "init" twice.
Was this meant to be "xe_mem_pool_init failed" or "pool init failed"?

Additionally, this patch fixes a bug introduced by:

    Fixes: 864690cf4dd62 ("drm/xe/vf: Attach and detach CCS copy
                           commands with BO")

That commit carries a lore.kernel.org link and a maintainer Signed-off-
by,
suggesting it has landed in a released kernel.  Should this patch carry
a
Cc: stable@vger.kernel.org tag?

Thomas's comment:
Please use "dim fixes <SHA1>" do craft Fixes: tags.

Thanks,
Thomas