Re: [PATCH 2/3] drm/prime: Support importing DMA-BUF without sg_table

["Christian König" <christian.koenig@amd.com>]

On 4/30/25 10:56, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
> 
> [WHY]
> On some boards, the dma_mask of Aspeed devices is 0xffff_ffff, this
> quite possibly causes the SWIOTLB to be triggered when importing dmabuf.
> However IO_TLB_SEGSIZE limits the maximum amount of available memory
> for DMA Streaming Mapping, as dmesg following:
> 
> [   24.885303][ T1947] ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
> 
> [HOW] Provide an interface so that attachment is not mapped when
> importing dma-buf.

This is unecessary. The extra abstraction in DRM is only useful when you want to implement the obj->funcs->get_sg_table() callback.

When a driver doesn't want to expose an sg_table for a buffer or want some other special handling it can simply do so by implementing the DMA-buf interface directly.

See drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c for an example on how to do this.

Regards,
Christian.

> 
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
>  drivers/gpu/drm/ast/ast_drv.c          |  2 +-
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 17 +++++++
>  drivers/gpu/drm/drm_prime.c            | 67 ++++++++++++++++++++++++--
>  drivers/gpu/drm/udl/udl_drv.c          |  2 +-
>  include/drm/drm_drv.h                  |  3 ++
>  include/drm/drm_gem_shmem_helper.h     |  6 +++
>  6 files changed, 91 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 6fbf62a99c48..2dac6acf79e7 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -64,7 +64,7 @@ static const struct drm_driver ast_driver = {
>  	.minor = DRIVER_MINOR,
>  	.patchlevel = DRIVER_PATCHLEVEL,
>  
> -	DRM_GEM_SHMEM_DRIVER_OPS,
> +	DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS,
>  	DRM_FBDEV_SHMEM_DRIVER_OPS,
>  };
>  
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index d99dee67353a..655d841df933 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -799,6 +799,23 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
>  
> +struct drm_gem_object *
> +drm_gem_shmem_prime_import_attachment(struct drm_device *dev,
> +				      struct dma_buf_attachment *attach)
> +{
> +	size_t size = PAGE_ALIGN(attach->dmabuf->size);
> +	struct drm_gem_shmem_object *shmem;
> +
> +	shmem = __drm_gem_shmem_create(dev, size, true, NULL);
> +	if (IS_ERR(shmem))
> +		return ERR_CAST(shmem);
> +
> +	drm_dbg_prime(dev, "size = %zu\n", size);
> +
> +	return &shmem->base;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_attachment);
> +
>  MODULE_DESCRIPTION("DRM SHMEM memory-management helpers");
>  MODULE_IMPORT_NS("DMA_BUF");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 8e70abca33b9..522cf974e202 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -911,6 +911,62 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
>  }
>  EXPORT_SYMBOL(drm_gem_prime_export);
>  
> +/**
> + * drm_gem_prime_import_dev_skip_map - core implementation of the import callback
> + * @dev: drm_device to import into
> + * @dma_buf: dma-buf object to import
> + * @attach_dev: struct device to dma_buf attach
> + *
> + * This function exports a dma-buf without get it's scatter/gather table.
> + *
> + * Drivers who need to get an scatter/gather table for objects need to call
> + * drm_gem_prime_import_dev() instead.
> + */
> +struct drm_gem_object *drm_gem_prime_import_dev_skip_map(struct drm_device *dev,
> +							 struct dma_buf *dma_buf,
> +							 struct device *attach_dev)
> +{
> +	struct dma_buf_attachment *attach;
> +	struct drm_gem_object *obj;
> +	int ret;
> +
> +	if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) {
> +		obj = dma_buf->priv;
> +		if (obj->dev == dev) {
> +			/*
> +			 * Importing dmabuf exported from our own gem increases
> +			 * refcount on gem itself instead of f_count of dmabuf.
> +			 */
> +			drm_gem_object_get(obj);
> +			return obj;
> +		}
> +	}
> +
> +	attach = dma_buf_attach(dma_buf, attach_dev, true);
> +	if (IS_ERR(attach))
> +		return ERR_CAST(attach);
> +
> +	get_dma_buf(dma_buf);
> +
> +	obj = dev->driver->gem_prime_import_attachment(dev, attach);
> +	if (IS_ERR(obj)) {
> +		ret = PTR_ERR(obj);
> +		goto fail_detach;
> +	}
> +
> +	obj->import_attach = attach;
> +	obj->resv = dma_buf->resv;
> +
> +	return obj;
> +
> +fail_detach:
> +	dma_buf_detach(dma_buf, attach);
> +	dma_buf_put(dma_buf);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(drm_gem_prime_import_dev_skip_map);
> +
>  /**
>   * drm_gem_prime_import_dev - core implementation of the import callback
>   * @dev: drm_device to import into
> @@ -946,9 +1002,6 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
>  		}
>  	}
>  
> -	if (!dev->driver->gem_prime_import_sg_table)
> -		return ERR_PTR(-EINVAL);
> -
>  	attach = dma_buf_attach(dma_buf, attach_dev, false);
>  	if (IS_ERR(attach))
>  		return ERR_CAST(attach);
> @@ -998,7 +1051,13 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev);
>  struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
>  					    struct dma_buf *dma_buf)
>  {
> -	return drm_gem_prime_import_dev(dev, dma_buf, dev->dev);
> +	if (dev->driver->gem_prime_import_sg_table)
> +		return drm_gem_prime_import_dev(dev, dma_buf, dev->dev);
> +	else if (dev->driver->gem_prime_import_attachment)
> +		return drm_gem_prime_import_dev_skip_map(dev, dma_buf, dev->dev);
> +	else
> +		return ERR_PTR(-EINVAL);
> +
>  }
>  EXPORT_SYMBOL(drm_gem_prime_import);
>  
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 05b3a152cc33..c00d8b8834f2 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -72,7 +72,7 @@ static const struct drm_driver driver = {
>  
>  	/* GEM hooks */
>  	.fops = &udl_driver_fops,
> -	DRM_GEM_SHMEM_DRIVER_OPS,
> +	DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS,
>  	.gem_prime_import = udl_driver_gem_prime_import,
>  	DRM_FBDEV_SHMEM_DRIVER_OPS,
>  
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index a43d707b5f36..aef8d9051fcd 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -326,6 +326,9 @@ struct drm_driver {
>  				struct dma_buf_attachment *attach,
>  				struct sg_table *sgt);
>  
> +	struct drm_gem_object *(*gem_prime_import_attachment)(
> +				struct drm_device *dev,
> +				struct dma_buf_attachment *attach);
>  	/**
>  	 * @dumb_create:
>  	 *
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index cef5a6b5a4d6..39a93c222aaa 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -274,6 +274,9 @@ struct drm_gem_object *
>  drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
>  				    struct dma_buf_attachment *attach,
>  				    struct sg_table *sgt);
> +struct drm_gem_object *
> +drm_gem_shmem_prime_import_attachment(struct drm_device *dev,
> +				      struct dma_buf_attachment *attach);
>  int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
>  			      struct drm_mode_create_dumb *args);
>  
> @@ -287,4 +290,7 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
>  	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
>  	.dumb_create		   = drm_gem_shmem_dumb_create
>  
> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \
> +	.gem_prime_import_attachment = drm_gem_shmem_prime_import_attachment, \
> +	.dumb_create                 = drm_gem_shmem_dumb_create
>  #endif /* __DRM_GEM_SHMEM_HELPER_H__ */