* [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table
@ 2025-05-07 9:47 oushixiong1025
2025-05-07 9:47 ` [PATCH v3 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: oushixiong1025 @ 2025-05-07 9:47 UTC (permalink / raw)
To: Christian König
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe,
dri-devel, linux-kernel, Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
[WHY]
1. Drivers using DRM_GEM_SHADOW_PLANE_HELPER_FUNCS and
DRM_GEM_SHMEM_DRIVER_OPS (e.g., udl, ast) do not require
sg_table import.
They only need dma_buf_vmap() to access the shared buffer's
kernel virtual address.
2. On certain Aspeed-based boards, a dma_mask of 0xffff_ffff may
trigger SWIOTLB during dmabuf import. However, IO_TLB_SEGSIZE
restricts the maximum DMA streaming mapping memory, resulting in
errors like:
ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
[HOW]
Provide a gem_prime_import implementation without sg_table mapping
to avoid issues (e.g., "swiotlb buffer is full"). Drivers that do not
require sg_table can adopt this.
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
v1->v2:
Patch rebase.
v2->v3:
Rename the import callback function.
Remove drm_gem_shmem_prime_export() and separate some codes
to drm_gem_prime_import_self().
drivers/gpu/drm/drm_gem_shmem_helper.c | 51 ++++++++++++++++++++++++++
drivers/gpu/drm/drm_prime.c | 46 +++++++++++++++++------
include/drm/drm_gem_shmem_helper.h | 15 ++++++++
include/drm/drm_prime.h | 3 ++
4 files changed, 104 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index aa43265f4f4f..85900ec1954a 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -800,6 +800,57 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
+/**
+ * drm_gem_shmem_prime_import_no_sgt - Import dmabuf without mapping its sg_table
+ * @dev: Device to import into
+ * @dma_buf: dma-buf object to import
+ *
+ * Drivers that use the shmem helpers but also wants to import dmabuf without
+ * mapping its sg_table can use this as their &drm_driver.gem_prime_import
+ * implementation.
+ */
+struct drm_gem_object *drm_gem_shmem_prime_import_no_sgt(struct drm_device *dev,
+ struct dma_buf *dma_buf)
+{
+ struct dma_buf_attachment *attach;
+ struct drm_gem_shmem_object *shmem;
+ struct drm_gem_object *obj;
+ size_t size;
+ int ret;
+
+ obj = drm_gem_prime_import_self(dev, dma_buf);
+ if (obj)
+ return obj;
+
+ attach = dma_buf_attach(dma_buf, dev->dev);
+ if (IS_ERR(attach))
+ return ERR_CAST(attach);
+
+ get_dma_buf(dma_buf);
+
+ size = PAGE_ALIGN(attach->dmabuf->size);
+
+ shmem = __drm_gem_shmem_create(dev, size, true, NULL);
+ if (IS_ERR(shmem)) {
+ ret = PTR_ERR(shmem);
+ goto fail_detach;
+ }
+
+ drm_dbg_prime(dev, "size = %zu\n", size);
+
+ shmem->base.import_attach = attach;
+ shmem->base.resv = dma_buf->resv;
+
+ return &shmem->base;
+
+fail_detach:
+ dma_buf_detach(dma_buf, attach);
+ dma_buf_put(dma_buf);
+
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_no_sgt);
+
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 d828502268b8..1e43b6022329 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -910,6 +910,38 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
}
EXPORT_SYMBOL(drm_gem_prime_export);
+
+/**
+ * drm_gem_prime_import_self - Import a DMA-BUF exported from the same DRM device.
+ * @dev: drm_device to check against
+ * @dma_buf: dma-buf object to import
+ *
+ * This function checks if the DMA-BUF was exported from a GEM object belonging
+ * to @dev. If so, it increments the GEM object's refcount and returns it directly.
+ *
+ * Return: GEM object if it belongs to @dev, NULL otherwise.
+ */
+struct drm_gem_object *drm_gem_prime_import_self(struct drm_device *dev,
+ struct dma_buf *dma_buf)
+{
+ struct drm_gem_object *obj;
+
+ 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;
+ }
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL(drm_gem_prime_import_self);
+
/**
* drm_gem_prime_import_dev - core implementation of the import callback
* @dev: drm_device to import into
@@ -933,17 +965,9 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
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;
- }
- }
+ obj = drm_gem_prime_import_self(dev, dma_buf);
+ if (obj)
+ return obj;
if (!dev->driver->gem_prime_import_sg_table)
return ERR_PTR(-EINVAL);
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index b4f993da3cae..6e8e907bc43e 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -287,6 +287,8 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
struct sg_table *sgt);
int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
+struct drm_gem_object *drm_gem_shmem_prime_import_no_sgt(struct drm_device *dev,
+ struct dma_buf *buf);
/**
* DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
@@ -298,4 +300,17 @@ 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
+/**
+ * DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP - shmem GEM operations
+ * without mapping sg_table on
+ * imported buffer.
+ *
+ * This macro provides a shortcut for setting the shmem GEM operations in
+ * the &drm_driver structure for drivers that do not require a sg_table on
+ * imported buffers. Only vmap is supported on imported buffer.
+ */
+#define DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP \
+ .gem_prime_import = drm_gem_shmem_prime_import_no_sgt, \
+ .dumb_create = drm_gem_shmem_dumb_create
+
#endif /* __DRM_GEM_SHMEM_HELPER_H__ */
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index fa085c44d4ca..b5179a9cde93 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -100,6 +100,9 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
/* helper functions for importing */
+struct drm_gem_object *drm_gem_prime_import_self(struct drm_device *dev,
+ struct dma_buf *dma_buf);
+
struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
struct dma_buf *dma_buf,
struct device *attach_dev);
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS
2025-05-07 9:47 [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025
@ 2025-05-07 9:47 ` oushixiong1025
2025-05-07 9:47 ` [PATCH v3 3/3] drm/udl: " oushixiong1025
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: oushixiong1025 @ 2025-05-07 9:47 UTC (permalink / raw)
To: Christian König
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe,
dri-devel, linux-kernel, Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
Import dmabuf without mapping its sg_table to avoid issues likes:
ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
drivers/gpu/drm/ast/ast_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 6fbf62a99c48..b626660e1fd7 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_DRIVER_OPS_FOR_VMAP,
DRM_FBDEV_SHMEM_DRIVER_OPS,
};
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 3/3] drm/udl: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS
2025-05-07 9:47 [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025
2025-05-07 9:47 ` [PATCH v3 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025
@ 2025-05-07 9:47 ` oushixiong1025
2025-05-07 12:59 ` [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Christian König
2025-05-08 9:30 ` Thomas Zimmermann
3 siblings, 0 replies; 5+ messages in thread
From: oushixiong1025 @ 2025-05-07 9:47 UTC (permalink / raw)
To: Christian König
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe,
dri-devel, linux-kernel, Shixiong Ou
From: Shixiong Ou <oushixiong@kylinos.cn>
Import dmabuf without mapping its sg_table to avoid issues likes:
udl 2-1.4:1.0: swiotlb buffer is full (sz: 2097152 bytes), total 65536 (slots), used 1 (slots)
Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
drivers/gpu/drm/udl/udl_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 1922988625eb..cbfdcb75f1f2 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -57,7 +57,7 @@ static const struct drm_driver driver = {
/* GEM hooks */
.fops = &udl_driver_fops,
- DRM_GEM_SHMEM_DRIVER_OPS,
+ DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP,
DRM_FBDEV_SHMEM_DRIVER_OPS,
.name = DRIVER_NAME,
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table
2025-05-07 9:47 [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025
2025-05-07 9:47 ` [PATCH v3 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025
2025-05-07 9:47 ` [PATCH v3 3/3] drm/udl: " oushixiong1025
@ 2025-05-07 12:59 ` Christian König
2025-05-08 9:30 ` Thomas Zimmermann
3 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2025-05-07 12:59 UTC (permalink / raw)
To: oushixiong1025
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe,
dri-devel, linux-kernel, Shixiong Ou
On 5/7/25 11:47, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [WHY]
> 1. Drivers using DRM_GEM_SHADOW_PLANE_HELPER_FUNCS and
> DRM_GEM_SHMEM_DRIVER_OPS (e.g., udl, ast) do not require
> sg_table import.
> They only need dma_buf_vmap() to access the shared buffer's
> kernel virtual address.
>
> 2. On certain Aspeed-based boards, a dma_mask of 0xffff_ffff may
> trigger SWIOTLB during dmabuf import. However, IO_TLB_SEGSIZE
> restricts the maximum DMA streaming mapping memory, resulting in
> errors like:
>
> ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
>
> [HOW]
> Provide a gem_prime_import implementation without sg_table mapping
> to avoid issues (e.g., "swiotlb buffer is full"). Drivers that do not
> require sg_table can adopt this.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
At least from my side that looks clean, so feel free to add Reviewed-by: Christian König <christian.koenig@amd.com>
But I would like to hear other opinions, e.g. Thomas as well. If nobody objects ping me to get this merged into drm-misc-next.
Regards,
Christian.
> ---
> v1->v2:
> Patch rebase.
> v2->v3:
> Rename the import callback function.
> Remove drm_gem_shmem_prime_export() and separate some codes
> to drm_gem_prime_import_self().
>
> drivers/gpu/drm/drm_gem_shmem_helper.c | 51 ++++++++++++++++++++++++++
> drivers/gpu/drm/drm_prime.c | 46 +++++++++++++++++------
> include/drm/drm_gem_shmem_helper.h | 15 ++++++++
> include/drm/drm_prime.h | 3 ++
> 4 files changed, 104 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index aa43265f4f4f..85900ec1954a 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -800,6 +800,57 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
>
> +/**
> + * drm_gem_shmem_prime_import_no_sgt - Import dmabuf without mapping its sg_table
> + * @dev: Device to import into
> + * @dma_buf: dma-buf object to import
> + *
> + * Drivers that use the shmem helpers but also wants to import dmabuf without
> + * mapping its sg_table can use this as their &drm_driver.gem_prime_import
> + * implementation.
> + */
> +struct drm_gem_object *drm_gem_shmem_prime_import_no_sgt(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + struct dma_buf_attachment *attach;
> + struct drm_gem_shmem_object *shmem;
> + struct drm_gem_object *obj;
> + size_t size;
> + int ret;
> +
> + obj = drm_gem_prime_import_self(dev, dma_buf);
> + if (obj)
> + return obj;
> +
> + attach = dma_buf_attach(dma_buf, dev->dev);
> + if (IS_ERR(attach))
> + return ERR_CAST(attach);
> +
> + get_dma_buf(dma_buf);
> +
> + size = PAGE_ALIGN(attach->dmabuf->size);
> +
> + shmem = __drm_gem_shmem_create(dev, size, true, NULL);
> + if (IS_ERR(shmem)) {
> + ret = PTR_ERR(shmem);
> + goto fail_detach;
> + }
> +
> + drm_dbg_prime(dev, "size = %zu\n", size);
> +
> + shmem->base.import_attach = attach;
> + shmem->base.resv = dma_buf->resv;
> +
> + return &shmem->base;
> +
> +fail_detach:
> + dma_buf_detach(dma_buf, attach);
> + dma_buf_put(dma_buf);
> +
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_no_sgt);
> +
> 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 d828502268b8..1e43b6022329 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -910,6 +910,38 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
> }
> EXPORT_SYMBOL(drm_gem_prime_export);
>
> +
> +/**
> + * drm_gem_prime_import_self - Import a DMA-BUF exported from the same DRM device.
> + * @dev: drm_device to check against
> + * @dma_buf: dma-buf object to import
> + *
> + * This function checks if the DMA-BUF was exported from a GEM object belonging
> + * to @dev. If so, it increments the GEM object's refcount and returns it directly.
> + *
> + * Return: GEM object if it belongs to @dev, NULL otherwise.
> + */
> +struct drm_gem_object *drm_gem_prime_import_self(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + struct drm_gem_object *obj;
> +
> + 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;
> + }
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_gem_prime_import_self);
> +
> /**
> * drm_gem_prime_import_dev - core implementation of the import callback
> * @dev: drm_device to import into
> @@ -933,17 +965,9 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
> 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;
> - }
> - }
> + obj = drm_gem_prime_import_self(dev, dma_buf);
> + if (obj)
> + return obj;
>
> if (!dev->driver->gem_prime_import_sg_table)
> return ERR_PTR(-EINVAL);
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index b4f993da3cae..6e8e907bc43e 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -287,6 +287,8 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
> struct sg_table *sgt);
> int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args);
> +struct drm_gem_object *drm_gem_shmem_prime_import_no_sgt(struct drm_device *dev,
> + struct dma_buf *buf);
>
> /**
> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
> @@ -298,4 +300,17 @@ 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
>
> +/**
> + * DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP - shmem GEM operations
> + * without mapping sg_table on
> + * imported buffer.
> + *
> + * This macro provides a shortcut for setting the shmem GEM operations in
> + * the &drm_driver structure for drivers that do not require a sg_table on
> + * imported buffers. Only vmap is supported on imported buffer.
> + */
> +#define DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP \
> + .gem_prime_import = drm_gem_shmem_prime_import_no_sgt, \
> + .dumb_create = drm_gem_shmem_dumb_create
> +
> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */
> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
> index fa085c44d4ca..b5179a9cde93 100644
> --- a/include/drm/drm_prime.h
> +++ b/include/drm/drm_prime.h
> @@ -100,6 +100,9 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
> unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
>
> /* helper functions for importing */
> +struct drm_gem_object *drm_gem_prime_import_self(struct drm_device *dev,
> + struct dma_buf *dma_buf);
> +
> struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
> struct dma_buf *dma_buf,
> struct device *attach_dev);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table
2025-05-07 9:47 [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025
` (2 preceding siblings ...)
2025-05-07 12:59 ` [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Christian König
@ 2025-05-08 9:30 ` Thomas Zimmermann
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2025-05-08 9:30 UTC (permalink / raw)
To: oushixiong1025, Christian König
Cc: Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou
Hi
Am 07.05.25 um 11:47 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [WHY]
> 1. Drivers using DRM_GEM_SHADOW_PLANE_HELPER_FUNCS and
> DRM_GEM_SHMEM_DRIVER_OPS (e.g., udl, ast) do not require
> sg_table import.
> They only need dma_buf_vmap() to access the shared buffer's
> kernel virtual address.
>
> 2. On certain Aspeed-based boards, a dma_mask of 0xffff_ffff may
> trigger SWIOTLB during dmabuf import. However, IO_TLB_SEGSIZE
> restricts the maximum DMA streaming mapping memory, resulting in
> errors like:
>
> ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
>
> [HOW]
> Provide a gem_prime_import implementation without sg_table mapping
> to avoid issues (e.g., "swiotlb buffer is full"). Drivers that do not
> require sg_table can adopt this.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> v1->v2:
> Patch rebase.
> v2->v3:
> Rename the import callback function.
> Remove drm_gem_shmem_prime_export() and separate some codes
> to drm_gem_prime_import_self().
>
> drivers/gpu/drm/drm_gem_shmem_helper.c | 51 ++++++++++++++++++++++++++
> drivers/gpu/drm/drm_prime.c | 46 +++++++++++++++++------
> include/drm/drm_gem_shmem_helper.h | 15 ++++++++
> include/drm/drm_prime.h | 3 ++
> 4 files changed, 104 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index aa43265f4f4f..85900ec1954a 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -800,6 +800,57 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
>
> +/**
> + * drm_gem_shmem_prime_import_no_sgt - Import dmabuf without mapping its sg_table
> + * @dev: Device to import into
> + * @dma_buf: dma-buf object to import
> + *
> + * Drivers that use the shmem helpers but also wants to import dmabuf without
> + * mapping its sg_table can use this as their &drm_driver.gem_prime_import
> + * implementation.
> + */
> +struct drm_gem_object *drm_gem_shmem_prime_import_no_sgt(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + struct dma_buf_attachment *attach;
> + struct drm_gem_shmem_object *shmem;
> + struct drm_gem_object *obj;
> + size_t size;
> + int ret;
> +
> + obj = drm_gem_prime_import_self(dev, dma_buf);
> + if (obj)
> + return obj;
> +
> + attach = dma_buf_attach(dma_buf, dev->dev);
> + if (IS_ERR(attach))
> + return ERR_CAST(attach);
> +
> + get_dma_buf(dma_buf);
> +
> + size = PAGE_ALIGN(attach->dmabuf->size);
> +
> + shmem = __drm_gem_shmem_create(dev, size, true, NULL);
> + if (IS_ERR(shmem)) {
> + ret = PTR_ERR(shmem);
> + goto fail_detach;
> + }
> +
> + drm_dbg_prime(dev, "size = %zu\n", size);
> +
> + shmem->base.import_attach = attach;
> + shmem->base.resv = dma_buf->resv;
> +
> + return &shmem->base;
> +
> +fail_detach:
> + dma_buf_detach(dma_buf, attach);
> + dma_buf_put(dma_buf);
> +
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_no_sgt);
> +
> 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 d828502268b8..1e43b6022329 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -910,6 +910,38 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
> }
> EXPORT_SYMBOL(drm_gem_prime_export);
>
> +
> +/**
> + * drm_gem_prime_import_self - Import a DMA-BUF exported from the same DRM device.
> + * @dev: drm_device to check against
> + * @dma_buf: dma-buf object to import
> + *
> + * This function checks if the DMA-BUF was exported from a GEM object belonging
> + * to @dev. If so, it increments the GEM object's refcount and returns it directly.
> + *
> + * Return: GEM object if it belongs to @dev, NULL otherwise.
> + */
> +struct drm_gem_object *drm_gem_prime_import_self(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + struct drm_gem_object *obj;
> +
> + 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;
> + }
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_gem_prime_import_self);
> +
I feel this helper is doing too much. Rather separate the test from the
policy. Do something like
bool drm_gem_prime_exported_dma_buf(dev, dma_buf)
{
obj = dma_buf->priv;
return (dma_buf->ops == drm_gem_prime_dmabuf_ops) && (obj->dev == dev)
}
and in the callers do
if (drm_gem_prime_exported_dma_buf(dev, dma_buf)) {
/* ... */
obj = dma_buf->obj;
get(obj);
return obj;
}
> /**
> * drm_gem_prime_import_dev - core implementation of the import callback
> * @dev: drm_device to import into
> @@ -933,17 +965,9 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
> 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;
> - }
> - }
> + obj = drm_gem_prime_import_self(dev, dma_buf);
> + if (obj)
> + return obj;
>
> if (!dev->driver->gem_prime_import_sg_table)
> return ERR_PTR(-EINVAL);
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index b4f993da3cae..6e8e907bc43e 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -287,6 +287,8 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
> struct sg_table *sgt);
> int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args);
> +struct drm_gem_object *drm_gem_shmem_prime_import_no_sgt(struct drm_device *dev,
> + struct dma_buf *buf);
>
> /**
> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
> @@ -298,4 +300,17 @@ 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
>
> +/**
> + * DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP - shmem GEM operations
> + * without mapping sg_table on
> + * imported buffer.
> + *
> + * This macro provides a shortcut for setting the shmem GEM operations in
> + * the &drm_driver structure for drivers that do not require a sg_table on
> + * imported buffers. Only vmap is supported on imported buffer.
That final sentence is misleading. It would still be possible to
pin/unpin and mmap the buffer. Only direct access from DMA-capable
hardware is not. (Right?)
> + */
> +#define DRM_GEM_SHMEM_DRIVER_OPS_FOR_VMAP \
Rather call this DRM_GEM_SHMEM_DRIVER_OPS_NO_SGT please. Calling it
_FOR_VMAP seems misleading to me. Best regards Thomas
> + .gem_prime_import = drm_gem_shmem_prime_import_no_sgt, \
> + .dumb_create = drm_gem_shmem_dumb_create
> +
> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */
> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
> index fa085c44d4ca..b5179a9cde93 100644
> --- a/include/drm/drm_prime.h
> +++ b/include/drm/drm_prime.h
> @@ -100,6 +100,9 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
> unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
>
> /* helper functions for importing */
> +struct drm_gem_object *drm_gem_prime_import_self(struct drm_device *dev,
> + struct dma_buf *dma_buf);
> +
> struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
> struct dma_buf *dma_buf,
> struct device *attach_dev);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-08 9:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-07 9:47 [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025
2025-05-07 9:47 ` [PATCH v3 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025
2025-05-07 9:47 ` [PATCH v3 3/3] drm/udl: " oushixiong1025
2025-05-07 12:59 ` [PATCH v3 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Christian König
2025-05-08 9:30 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®