Re: [PATCH 2/2] drm/xe: Copy cpu_caching when importing DMA-BUF.

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

Hi, Maarten.

On Thu, 2025-03-20 at 22:15 +0100, Maarten Lankhorst wrote:
> When testing xe with P2PDMA DMA-BUF import,
> VM_BIND was failing because cpu_caching was not set.
> 
> Set cpu_caching if available from another xe driver,
> to allow uncached VM_BIND on imported DMA-BUF.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>

I don't think this is correct, we shouldn't assume anything about
imported dma-bufs, really. If we do we might end up in a situation
where code works for dma-bufs imported from one device but not from
another.

I think what we can do is to relax the COH mode check. It only really
matters on IGPU. On DGPU, all accesses to system memory is coherent.

Next step would perhaps be to assume that on p2p access, a dma-buf is
coherently cleared when all KERNEL dma-fences are signaled, but that
would require a cross-driver agreement, so in essence allow non-
coherent VM_BIND to device memory. This would matter only for IGPU
mapping a device p2p.

/Thomas



> ---
>  drivers/gpu/drm/xe/xe_dma_buf.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> b/drivers/gpu/drm/xe/xe_dma_buf.c
> index c5b95470fa324..9b1813e65e0a4 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -266,12 +266,12 @@ struct drm_gem_object
> *xe_gem_prime_import(struct drm_device *dev,
>  	const struct dma_buf_attach_ops *attach_ops;
>  	struct dma_buf_attachment *attach;
>  	struct drm_gem_object *obj;
> -	struct xe_bo *bo;
> +	struct xe_bo *bo, *foreign_bo = NULL;
>  
> -	if (dma_buf->ops == &xe_dmabuf_ops) {
> +	if (dma_buf->ops == &xe_dmabuf_ops &&
> +	    !XE_TEST_ONLY(test && test->force_different_devices)) {
>  		obj = dma_buf->priv;
> -		if (obj->dev == dev &&
> -		    !XE_TEST_ONLY(test && test-
> >force_different_devices)) {
> +		if (obj->dev == dev) {
>  			/*
>  			 * Importing dmabuf exported from out own
> gem increases
>  			 * refcount on gem itself instead of f_count
> of dmabuf.
> @@ -279,6 +279,8 @@ struct drm_gem_object *xe_gem_prime_import(struct
> drm_device *dev,
>  			drm_gem_object_get(obj);
>  			return obj;
>  		}
> +
> +		foreign_bo = gem_to_xe_bo(obj);
>  	}
>  
>  	/*
> @@ -310,6 +312,9 @@ struct drm_gem_object *xe_gem_prime_import(struct
> drm_device *dev,
>  
>  	get_dma_buf(dma_buf);
>  	obj->import_attach = attach;
> +	if (foreign_bo)
> +		bo->cpu_caching = foreign_bo->cpu_caching;
> +
>  	return obj;
>  
>  out_err: