Re: [PATCH v3 2/3] drm/xe: add xe_migrate_resolve wrapper and is_vram_resolve support

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Oct 15, 2025 at 04:17:08PM +0530, Nitin Gote wrote:
> Introduce an internal __xe_migrate_copy(..., is_vram_resolve) path and
> expose a small wrapper xe_migrate_resolve() that calls it with
> is_vram_resolve=true.
> 
> For resolve/decompression operations we must ensure the copy code uses
> the compression PAT index when appropriate; this change centralizes that
> behavior and allows callers to schedule a resolve (decompress) operation
> via the migrate API.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_migrate.c | 90 +++++++++++++++++++++++----------
>  drivers/gpu/drm/xe/xe_migrate.h |  7 +++
>  2 files changed, 70 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 4ca48dd1cfd8..1ab649c1e4e0 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -777,31 +777,13 @@ static u32 xe_migrate_ccs_copy(struct xe_migrate *m,
>  	return flush_flags;
>  }
>  
> -/**
> - * xe_migrate_copy() - Copy content of TTM resources.
> - * @m: The migration context.
> - * @src_bo: The buffer object @src is currently bound to.
> - * @dst_bo: If copying between resources created for the same bo, set this to
> - * the same value as @src_bo. If copying between buffer objects, set it to
> - * the buffer object @dst is currently bound to.
> - * @src: The source TTM resource.
> - * @dst: The dst TTM resource.
> - * @copy_only_ccs: If true copy only CCS metadata
> - *
> - * Copies the contents of @src to @dst: On flat CCS devices,
> - * the CCS metadata is copied as well if needed, or if not present,
> - * the CCS metadata of @dst is cleared for security reasons.
> - *
> - * Return: Pointer to a dma_fence representing the last copy batch, or
> - * an error pointer on failure. If there is a failure, any copy operation
> - * started by the function call has been synced.
> - */
> -struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
> -				  struct xe_bo *src_bo,
> -				  struct xe_bo *dst_bo,
> -				  struct ttm_resource *src,
> -				  struct ttm_resource *dst,
> -				  bool copy_only_ccs)
> +static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
> +					   struct xe_bo *src_bo,
> +					   struct xe_bo *dst_bo,
> +					   struct ttm_resource *src,
> +					   struct ttm_resource *dst,
> +					   bool copy_only_ccs,
> +					   bool is_vram_resolve)
>  {
>  	struct xe_gt *gt = m->tile->primary_gt;
>  	struct xe_device *xe = gt_to_xe(gt);
> @@ -822,8 +804,19 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
>  	bool copy_ccs = xe_device_has_flat_ccs(xe) &&
>  		xe_bo_needs_ccs_pages(src_bo) && xe_bo_needs_ccs_pages(dst_bo);
>  	bool copy_system_ccs = copy_ccs && (!src_is_vram || !dst_is_vram);
> -	bool use_comp_pat = type_device && xe_device_has_flat_ccs(xe) &&
> -		GRAPHICS_VER(xe) >= 20 && src_is_vram && !dst_is_vram;
> +	bool use_comp_pat;

I'd write this like:

	bool use_comp_pat = is_vram_resolve || (type_device &&
		xe_device_has_flat_ccs(xe) && GRAPHICS_VER(xe) >= 20 &&
		src_is_vram && !dst_is_vram);


> +
> +	/*
> +	 * For decompression operation, always use the compression PAT index.
> +	 * Otherwise, only use the compression PAT index for device memory
> +	 * when copying from VRAM to system memory.
> +	 */

Feel free leave this comment in as it is helpful.

> +	if (is_vram_resolve) {
> +		use_comp_pat = true;
> +	} else {
> +		use_comp_pat = type_device && xe_device_has_flat_ccs(xe) &&
> +			GRAPHICS_VER(xe) >= 20 && src_is_vram && !dst_is_vram;
> +	}
>  
>  	/* Copying CCS between two different BOs is not supported yet. */
>  	if (XE_WARN_ON(copy_ccs && src_bo != dst_bo))
> @@ -982,6 +975,49 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
>  	return fence;
>  }
>  
> +/**
> + * xe_migrate_copy() - Copy content of TTM resources.
> + * @m: The migration context.
> + * @src_bo: The buffer object @src is currently bound to.
> + * @dst_bo: If copying between resources created for the same bo, set this to
> + * the same value as @src_bo. If copying between buffer objects, set it to
> + * the buffer object @dst is currently bound to.
> + * @src: The source TTM resource.
> + * @dst: The dst TTM resource.
> + * @copy_only_ccs: If true copy only CCS metadata
> + *
> + * Copies the contents of @src to @dst: On flat CCS devices,
> + * the CCS metadata is copied as well if needed, or if not present,
> + * the CCS metadata of @dst is cleared for security reasons.
> + *
> + * Return: Pointer to a dma_fence representing the last copy batch, or
> + * an error pointer on failure. If there is a failure, any copy operation
> + * started by the function call has been synced.
> + */
> +struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
> +				  struct xe_bo *src_bo,
> +				  struct xe_bo *dst_bo,
> +				  struct ttm_resource *src,
> +				  struct ttm_resource *dst,
> +				  bool copy_only_ccs)
> +{
> +	return __xe_migrate_copy(m, src_bo, dst_bo, src, dst, copy_only_ccs, false);
> +}
> +
> +/**
> + * xe_migrate_resolve() - Resolve and decompress a buffer object if required.
> + * This wrapper forwards to __xe_migrate_copy() with is_vram_resolve=true.
> + */
> +struct dma_fence *xe_migrate_resolve(struct xe_migrate *m,
> +				     struct xe_bo *src_bo,
> +				     struct xe_bo *dst_bo,
> +				     struct ttm_resource *src,
> +				     struct ttm_resource *dst,
> +				     bool copy_only_ccs)
> +{
> +	return __xe_migrate_copy(m, src_bo, dst_bo, src, dst, copy_only_ccs, true);
> +}
> +
>  /**
>   * xe_migrate_lrc() - Get the LRC from migrate context.
>   * @migrate: Migrate context.
> diff --git a/drivers/gpu/drm/xe/xe_migrate.h b/drivers/gpu/drm/xe/xe_migrate.h
> index 4fad324b6253..ade68ab32a71 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.h
> +++ b/drivers/gpu/drm/xe/xe_migrate.h
> @@ -125,6 +125,13 @@ struct dma_fence *xe_migrate_copy(struct xe_migrate *m,
>  				  struct ttm_resource *dst,
>  				  bool copy_only_ccs);
>  
> +struct dma_fence *xe_migrate_resolve(struct xe_migrate *m,
> +				     struct xe_bo *src_bo,
> +				     struct xe_bo *dst_bo,
> +				     struct ttm_resource *src,
> +				     struct ttm_resource *dst,
> +				     bool copy_only_ccs);

I think you can have single BO / resource argument as resolves are always the
same src / dst.

Likewise for resolve copy_only_ccs is always false, right? So can likely drop
that argument too.

Matt

> +
>  int xe_migrate_ccs_rw_copy(struct xe_tile *tile, struct xe_exec_queue *q,
>  			   struct xe_bo *src_bo,
>  			   enum xe_sriov_vf_ccs_rw_ctxs read_write);
> -- 
> 2.25.1
>