* [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table
@ 2025-05-01 6:43 oushixiong1025
2025-05-01 6:43 ` [PATCH 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: oushixiong1025 @ 2025-05-01 6:43 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: 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>
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++
include/drm/drm_gem_shmem_helper.h | 24 +++++++
2 files changed, 119 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index d99dee67353a..9e41e350ff6f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF");
static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
.free = drm_gem_shmem_object_free,
.print_info = drm_gem_shmem_object_print_info,
+ .export = drm_gem_shmem_object_prime_export,
.pin = drm_gem_shmem_object_pin,
.unpin = drm_gem_shmem_object_unpin,
.get_sg_table = drm_gem_shmem_object_get_sg_table,
@@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
+const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = {
+ .cache_sgt_mapping = true,
+ .attach = drm_gem_map_attach,
+ .detach = drm_gem_map_detach,
+ .map_dma_buf = drm_gem_map_dma_buf,
+ .unmap_dma_buf = drm_gem_unmap_dma_buf,
+ .release = drm_gem_dmabuf_release,
+ .mmap = drm_gem_dmabuf_mmap,
+ .vmap = drm_gem_dmabuf_vmap,
+ .vunmap = drm_gem_dmabuf_vunmap,
+};
+
+/**
+ * drm_gem_shmem_prime_export - implementation of the export callback
+ * @shmem: shmem GEM object
+ */
+struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem,
+ int flags)
+{
+ struct drm_gem_object *obj = &shmem->base;
+ struct drm_device *dev = obj->dev;
+ struct dma_buf_export_info exp_info = {
+ .exp_name = KBUILD_MODNAME, /* white lie for debug */
+ .owner = dev->driver->fops->owner,
+ .ops = &drm_gem_shmem_prime_dmabuf_ops,
+ .size = obj->size,
+ .flags = flags,
+ .priv = obj,
+ .resv = obj->resv,
+ };
+
+ return drm_gem_dmabuf_export(dev, &exp_info);
+}
+
+/**
+ * drm_gem_shmem_prime_import - 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(struct drm_device *dev,
+ struct dma_buf *dma_buf)
+{
+ struct dma_buf_attachment *attach;
+ struct drm_gem_shmem_object *shmem;
+ size_t size;
+ int ret;
+
+ if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) {
+ struct drm_gem_object *obj;
+
+ 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, 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);
+
MODULE_DESCRIPTION("DRM SHMEM memory-management helpers");
MODULE_IMPORT_NS("DMA_BUF");
MODULE_LICENSE("GPL v2");
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index cef5a6b5a4d6..78ef91593a8e 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem,
struct iosys_map *map);
int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma);
+struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem,
+ int flags);
int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem);
void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem);
@@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign
drm_gem_shmem_print_info(shmem, p, indent);
}
+/**
+ * drm_gem_shmem_object_prime_export - GEM object function for export()
+ * @obj: GEM object
+ *
+ */
+static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj,
+ int flags)
+{
+ struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+ return drm_gem_shmem_prime_export(shmem, flags);
+}
/**
* drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin()
* @obj: GEM object
@@ -276,6 +290,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(struct drm_device *dev,
+ struct dma_buf *buf);
/**
* DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
@@ -287,4 +303,12 @@ 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
+/**
+ * This macro provides a shmem GEM operations that implementate a simple
+ * gem_prime_import.
+ */
+#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \
+ .gem_prime_import = drm_gem_shmem_prime_import, \
+ .dumb_create = drm_gem_shmem_dumb_create
+
#endif /* __DRM_GEM_SHMEM_HELPER_H__ */
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS 2025-05-01 6:43 [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025 @ 2025-05-01 6:43 ` oushixiong1025 2025-05-01 6:43 ` [PATCH 3/3] drm/udl: " oushixiong1025 2025-05-05 11:12 ` [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Thomas Zimmermann 2 siblings, 0 replies; 9+ messages in thread From: oushixiong1025 @ 2025-05-01 6:43 UTC (permalink / raw) To: Maarten Lankhorst Cc: 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..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, }; -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] drm/udl: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS 2025-05-01 6:43 [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025 2025-05-01 6:43 ` [PATCH 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025 @ 2025-05-01 6:43 ` oushixiong1025 2025-05-05 11:12 ` [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Thomas Zimmermann 2 siblings, 0 replies; 9+ messages in thread From: oushixiong1025 @ 2025-05-01 6:43 UTC (permalink / raw) To: Maarten Lankhorst Cc: 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 | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index 05b3a152cc33..8107549b12e5 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -49,22 +49,6 @@ static int udl_usb_reset_resume(struct usb_interface *interface) return drm_mode_config_helper_resume(dev); } -/* - * FIXME: Dma-buf sharing requires DMA support by the importing device. - * This function is a workaround to make USB devices work as well. - * See todo.rst for how to fix the issue in the dma-buf framework. - */ -static struct drm_gem_object *udl_driver_gem_prime_import(struct drm_device *dev, - struct dma_buf *dma_buf) -{ - struct udl_device *udl = to_udl(dev); - - if (!udl->dmadev) - return ERR_PTR(-ENODEV); - - return drm_gem_prime_import_dev(dev, dma_buf, udl->dmadev); -} - DEFINE_DRM_GEM_FOPS(udl_driver_fops); static const struct drm_driver driver = { @@ -72,8 +56,7 @@ static const struct drm_driver driver = { /* GEM hooks */ .fops = &udl_driver_fops, - DRM_GEM_SHMEM_DRIVER_OPS, - .gem_prime_import = udl_driver_gem_prime_import, + DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS, DRM_FBDEV_SHMEM_DRIVER_OPS, .name = DRIVER_NAME, -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table 2025-05-01 6:43 [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025 2025-05-01 6:43 ` [PATCH 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025 2025-05-01 6:43 ` [PATCH 3/3] drm/udl: " oushixiong1025 @ 2025-05-05 11:12 ` Thomas Zimmermann 2025-05-05 11:25 ` Christian König 2 siblings, 1 reply; 9+ messages in thread From: Thomas Zimmermann @ 2025-05-05 11:12 UTC (permalink / raw) To: oushixiong1025, Maarten Lankhorst Cc: Maxime Ripard, David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou, Christian König (cc'ing Christian) Hi, I don't feel qualified to fully review this patch. It would be good to have the issue with sg-tables solved, but I dislike the dedicated initializer macros. So my question is if this has any drawbacks. Or could we make this available and the default for all shmem-based drivers? Best regards Thomas Am 01.05.25 um 08:43 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> > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++ > include/drm/drm_gem_shmem_helper.h | 24 +++++++ > 2 files changed, 119 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index d99dee67353a..9e41e350ff6f 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF"); > static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { > .free = drm_gem_shmem_object_free, > .print_info = drm_gem_shmem_object_print_info, > + .export = drm_gem_shmem_object_prime_export, > .pin = drm_gem_shmem_object_pin, > .unpin = drm_gem_shmem_object_unpin, > .get_sg_table = drm_gem_shmem_object_get_sg_table, > @@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); > > +const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = { > + .cache_sgt_mapping = true, > + .attach = drm_gem_map_attach, > + .detach = drm_gem_map_detach, > + .map_dma_buf = drm_gem_map_dma_buf, > + .unmap_dma_buf = drm_gem_unmap_dma_buf, > + .release = drm_gem_dmabuf_release, > + .mmap = drm_gem_dmabuf_mmap, > + .vmap = drm_gem_dmabuf_vmap, > + .vunmap = drm_gem_dmabuf_vunmap, > +}; > + > +/** > + * drm_gem_shmem_prime_export - implementation of the export callback > + * @shmem: shmem GEM object > + */ > +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, > + int flags) > +{ > + struct drm_gem_object *obj = &shmem->base; > + struct drm_device *dev = obj->dev; > + struct dma_buf_export_info exp_info = { > + .exp_name = KBUILD_MODNAME, /* white lie for debug */ > + .owner = dev->driver->fops->owner, > + .ops = &drm_gem_shmem_prime_dmabuf_ops, > + .size = obj->size, > + .flags = flags, > + .priv = obj, > + .resv = obj->resv, > + }; > + > + return drm_gem_dmabuf_export(dev, &exp_info); > +} > + > +/** > + * drm_gem_shmem_prime_import - 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(struct drm_device *dev, > + struct dma_buf *dma_buf) > +{ > + struct dma_buf_attachment *attach; > + struct drm_gem_shmem_object *shmem; > + size_t size; > + int ret; > + > + if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) { > + struct drm_gem_object *obj; > + > + 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, 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); > + > MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); > MODULE_IMPORT_NS("DMA_BUF"); > MODULE_LICENSE("GPL v2"); > diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h > index cef5a6b5a4d6..78ef91593a8e 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, > void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, > struct iosys_map *map); > int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); > +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, > + int flags); > > int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); > void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); > @@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign > drm_gem_shmem_print_info(shmem, p, indent); > } > > +/** > + * drm_gem_shmem_object_prime_export - GEM object function for export() > + * @obj: GEM object > + * > + */ > +static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj, > + int flags) > +{ > + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > + > + return drm_gem_shmem_prime_export(shmem, flags); > +} > /** > * drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin() > * @obj: GEM object > @@ -276,6 +290,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(struct drm_device *dev, > + struct dma_buf *buf); > > /** > * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations > @@ -287,4 +303,12 @@ 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 > > +/** > + * This macro provides a shmem GEM operations that implementate a simple > + * gem_prime_import. > + */ > +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ > + .gem_prime_import = drm_gem_shmem_prime_import, \ > + .dumb_create = drm_gem_shmem_dumb_create > + > #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ -- -- 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] 9+ messages in thread
* Re: [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table 2025-05-05 11:12 ` [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Thomas Zimmermann @ 2025-05-05 11:25 ` Christian König 2025-05-05 14:22 ` oushixiong 0 siblings, 1 reply; 9+ messages in thread From: Christian König @ 2025-05-05 11:25 UTC (permalink / raw) To: Thomas Zimmermann, oushixiong1025, Maarten Lankhorst Cc: Maxime Ripard, David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou Hi Thomas & Shixiong, first of all the patch is still based on outdated code. For example the cache_sgt_mapping member is already removed in drm-misc-next. So if I'm not completely mistaken the issue is already resolved upstream. Regards, Christian. On 5/5/25 13:12, Thomas Zimmermann wrote: > (cc'ing Christian) > > Hi, > > I don't feel qualified to fully review this patch. > > It would be good to have the issue with sg-tables solved, but I dislike the dedicated initializer macros. So my question is if this has any drawbacks. Or could we make this available and the default for all shmem-based drivers? > > Best regards > Thomas > > Am 01.05.25 um 08:43 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> >> --- >> drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++ >> include/drm/drm_gem_shmem_helper.h | 24 +++++++ >> 2 files changed, 119 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >> index d99dee67353a..9e41e350ff6f 100644 >> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >> @@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF"); >> static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { >> .free = drm_gem_shmem_object_free, >> .print_info = drm_gem_shmem_object_print_info, >> + .export = drm_gem_shmem_object_prime_export, >> .pin = drm_gem_shmem_object_pin, >> .unpin = drm_gem_shmem_object_unpin, >> .get_sg_table = drm_gem_shmem_object_get_sg_table, >> @@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >> } >> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); >> +const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = { >> + .cache_sgt_mapping = true, >> + .attach = drm_gem_map_attach, >> + .detach = drm_gem_map_detach, >> + .map_dma_buf = drm_gem_map_dma_buf, >> + .unmap_dma_buf = drm_gem_unmap_dma_buf, >> + .release = drm_gem_dmabuf_release, >> + .mmap = drm_gem_dmabuf_mmap, >> + .vmap = drm_gem_dmabuf_vmap, >> + .vunmap = drm_gem_dmabuf_vunmap, >> +}; >> + >> +/** >> + * drm_gem_shmem_prime_export - implementation of the export callback >> + * @shmem: shmem GEM object >> + */ >> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >> + int flags) >> +{ >> + struct drm_gem_object *obj = &shmem->base; >> + struct drm_device *dev = obj->dev; >> + struct dma_buf_export_info exp_info = { >> + .exp_name = KBUILD_MODNAME, /* white lie for debug */ >> + .owner = dev->driver->fops->owner, >> + .ops = &drm_gem_shmem_prime_dmabuf_ops, >> + .size = obj->size, >> + .flags = flags, >> + .priv = obj, >> + .resv = obj->resv, >> + }; >> + >> + return drm_gem_dmabuf_export(dev, &exp_info); >> +} >> + >> +/** >> + * drm_gem_shmem_prime_import - 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(struct drm_device *dev, >> + struct dma_buf *dma_buf) >> +{ >> + struct dma_buf_attachment *attach; >> + struct drm_gem_shmem_object *shmem; >> + size_t size; >> + int ret; >> + >> + if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) { >> + struct drm_gem_object *obj; >> + >> + 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, 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); >> + >> MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); >> MODULE_IMPORT_NS("DMA_BUF"); >> MODULE_LICENSE("GPL v2"); >> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h >> index cef5a6b5a4d6..78ef91593a8e 100644 >> --- a/include/drm/drm_gem_shmem_helper.h >> +++ b/include/drm/drm_gem_shmem_helper.h >> @@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, >> void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, >> struct iosys_map *map); >> int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); >> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >> + int flags); >> int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); >> void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); >> @@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign >> drm_gem_shmem_print_info(shmem, p, indent); >> } >> +/** >> + * drm_gem_shmem_object_prime_export - GEM object function for export() >> + * @obj: GEM object >> + * >> + */ >> +static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj, >> + int flags) >> +{ >> + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); >> + >> + return drm_gem_shmem_prime_export(shmem, flags); >> +} >> /** >> * drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin() >> * @obj: GEM object >> @@ -276,6 +290,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(struct drm_device *dev, >> + struct dma_buf *buf); >> /** >> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations >> @@ -287,4 +303,12 @@ 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 >> +/** >> + * This macro provides a shmem GEM operations that implementate a simple >> + * gem_prime_import. >> + */ >> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ >> + .gem_prime_import = drm_gem_shmem_prime_import, \ >> + .dumb_create = drm_gem_shmem_dumb_create >> + >> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table 2025-05-05 11:25 ` Christian König @ 2025-05-05 14:22 ` oushixiong 2025-05-05 14:32 ` Christian König 0 siblings, 1 reply; 9+ messages in thread From: oushixiong @ 2025-05-05 14:22 UTC (permalink / raw) To: Christian König, Thomas Zimmermann, Maarten Lankhorst Cc: Maxime Ripard, David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou Hi Christian, My patch is based on linux-next, so this patch is not based on the latest code. Then, I'd like to ask which patch resolved the issue with sg-tables? Thanks and Regards, Shixiong. 在 2025/5/5 19:25, Christian König 写道: > Hi Thomas & Shixiong, > > first of all the patch is still based on outdated code. For example the cache_sgt_mapping member is already removed in drm-misc-next. > > So if I'm not completely mistaken the issue is already resolved upstream. > > Regards, > Christian. > > On 5/5/25 13:12, Thomas Zimmermann wrote: >> (cc'ing Christian) >> >> Hi, >> >> I don't feel qualified to fully review this patch. >> >> It would be good to have the issue with sg-tables solved, but I dislike the dedicated initializer macros. So my question is if this has any drawbacks. Or could we make this available and the default for all shmem-based drivers? >> >> Best regards >> Thomas >> >> Am 01.05.25 um 08:43 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> >>> --- >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++ >>> include/drm/drm_gem_shmem_helper.h | 24 +++++++ >>> 2 files changed, 119 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> index d99dee67353a..9e41e350ff6f 100644 >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> @@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF"); >>> static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { >>> .free = drm_gem_shmem_object_free, >>> .print_info = drm_gem_shmem_object_print_info, >>> + .export = drm_gem_shmem_object_prime_export, >>> .pin = drm_gem_shmem_object_pin, >>> .unpin = drm_gem_shmem_object_unpin, >>> .get_sg_table = drm_gem_shmem_object_get_sg_table, >>> @@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >>> } >>> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); >>> +const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = { >>> + .cache_sgt_mapping = true, >>> + .attach = drm_gem_map_attach, >>> + .detach = drm_gem_map_detach, >>> + .map_dma_buf = drm_gem_map_dma_buf, >>> + .unmap_dma_buf = drm_gem_unmap_dma_buf, >>> + .release = drm_gem_dmabuf_release, >>> + .mmap = drm_gem_dmabuf_mmap, >>> + .vmap = drm_gem_dmabuf_vmap, >>> + .vunmap = drm_gem_dmabuf_vunmap, >>> +}; >>> + >>> +/** >>> + * drm_gem_shmem_prime_export - implementation of the export callback >>> + * @shmem: shmem GEM object >>> + */ >>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>> + int flags) >>> +{ >>> + struct drm_gem_object *obj = &shmem->base; >>> + struct drm_device *dev = obj->dev; >>> + struct dma_buf_export_info exp_info = { >>> + .exp_name = KBUILD_MODNAME, /* white lie for debug */ >>> + .owner = dev->driver->fops->owner, >>> + .ops = &drm_gem_shmem_prime_dmabuf_ops, >>> + .size = obj->size, >>> + .flags = flags, >>> + .priv = obj, >>> + .resv = obj->resv, >>> + }; >>> + >>> + return drm_gem_dmabuf_export(dev, &exp_info); >>> +} >>> + >>> +/** >>> + * drm_gem_shmem_prime_import - 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(struct drm_device *dev, >>> + struct dma_buf *dma_buf) >>> +{ >>> + struct dma_buf_attachment *attach; >>> + struct drm_gem_shmem_object *shmem; >>> + size_t size; >>> + int ret; >>> + >>> + if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) { >>> + struct drm_gem_object *obj; >>> + >>> + 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, 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); >>> + >>> MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); >>> MODULE_IMPORT_NS("DMA_BUF"); >>> MODULE_LICENSE("GPL v2"); >>> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h >>> index cef5a6b5a4d6..78ef91593a8e 100644 >>> --- a/include/drm/drm_gem_shmem_helper.h >>> +++ b/include/drm/drm_gem_shmem_helper.h >>> @@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, >>> void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, >>> struct iosys_map *map); >>> int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); >>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>> + int flags); >>> int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); >>> void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); >>> @@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign >>> drm_gem_shmem_print_info(shmem, p, indent); >>> } >>> +/** >>> + * drm_gem_shmem_object_prime_export - GEM object function for export() >>> + * @obj: GEM object >>> + * >>> + */ >>> +static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj, >>> + int flags) >>> +{ >>> + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); >>> + >>> + return drm_gem_shmem_prime_export(shmem, flags); >>> +} >>> /** >>> * drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin() >>> * @obj: GEM object >>> @@ -276,6 +290,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(struct drm_device *dev, >>> + struct dma_buf *buf); >>> /** >>> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations >>> @@ -287,4 +303,12 @@ 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 >>> +/** >>> + * This macro provides a shmem GEM operations that implementate a simple >>> + * gem_prime_import. >>> + */ >>> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ >>> + .gem_prime_import = drm_gem_shmem_prime_import, \ >>> + .dumb_create = drm_gem_shmem_dumb_create >>> + >>> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table 2025-05-05 14:22 ` oushixiong @ 2025-05-05 14:32 ` Christian König 2025-05-05 15:11 ` oushixiong 0 siblings, 1 reply; 9+ messages in thread From: Christian König @ 2025-05-05 14:32 UTC (permalink / raw) To: oushixiong, Thomas Zimmermann, Maarten Lankhorst Cc: Maxime Ripard, David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou Hi Shixiong, for drm changes please base your patches on drm-misc-next or drm-next. That is probably fixed by this one here: commit b72f66f22c0e39ae6684c43fead774c13db24e73 Author: Christian König <christian.koenig@amd.com> Date: Tue Feb 11 17:20:53 2025 +0100 dma-buf: drop caching of sg_tables That was purely for the transition from static to dynamic dma-buf handling and can be removed again now. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christian.koenig@amd.com After this patch SG tables where only created when necessary. Regards, Christian. On 5/5/25 16:22, oushixiong wrote: > Hi Christian, > > My patch is based on linux-next, so this patch is not based on the latest code. Then, I'd like to ask which patch resolved the issue with sg-tables? > > > Thanks and Regards, > > Shixiong. > > > 在 2025/5/5 19:25, Christian König 写道: >> Hi Thomas & Shixiong, >> >> first of all the patch is still based on outdated code. For example the cache_sgt_mapping member is already removed in drm-misc-next. >> >> So if I'm not completely mistaken the issue is already resolved upstream. >> >> Regards, >> Christian. >> >> On 5/5/25 13:12, Thomas Zimmermann wrote: >>> (cc'ing Christian) >>> >>> Hi, >>> >>> I don't feel qualified to fully review this patch. >>> >>> It would be good to have the issue with sg-tables solved, but I dislike the dedicated initializer macros. So my question is if this has any drawbacks. Or could we make this available and the default for all shmem-based drivers? >>> >>> Best regards >>> Thomas >>> >>> Am 01.05.25 um 08:43 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> >>>> --- >>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++ >>>> include/drm/drm_gem_shmem_helper.h | 24 +++++++ >>>> 2 files changed, 119 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>> index d99dee67353a..9e41e350ff6f 100644 >>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>> @@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF"); >>>> static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { >>>> .free = drm_gem_shmem_object_free, >>>> .print_info = drm_gem_shmem_object_print_info, >>>> + .export = drm_gem_shmem_object_prime_export, >>>> .pin = drm_gem_shmem_object_pin, >>>> .unpin = drm_gem_shmem_object_unpin, >>>> .get_sg_table = drm_gem_shmem_object_get_sg_table, >>>> @@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >>>> } >>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); >>>> +const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = { >>>> + .cache_sgt_mapping = true, >>>> + .attach = drm_gem_map_attach, >>>> + .detach = drm_gem_map_detach, >>>> + .map_dma_buf = drm_gem_map_dma_buf, >>>> + .unmap_dma_buf = drm_gem_unmap_dma_buf, >>>> + .release = drm_gem_dmabuf_release, >>>> + .mmap = drm_gem_dmabuf_mmap, >>>> + .vmap = drm_gem_dmabuf_vmap, >>>> + .vunmap = drm_gem_dmabuf_vunmap, >>>> +}; >>>> + >>>> +/** >>>> + * drm_gem_shmem_prime_export - implementation of the export callback >>>> + * @shmem: shmem GEM object >>>> + */ >>>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>>> + int flags) >>>> +{ >>>> + struct drm_gem_object *obj = &shmem->base; >>>> + struct drm_device *dev = obj->dev; >>>> + struct dma_buf_export_info exp_info = { >>>> + .exp_name = KBUILD_MODNAME, /* white lie for debug */ >>>> + .owner = dev->driver->fops->owner, >>>> + .ops = &drm_gem_shmem_prime_dmabuf_ops, >>>> + .size = obj->size, >>>> + .flags = flags, >>>> + .priv = obj, >>>> + .resv = obj->resv, >>>> + }; >>>> + >>>> + return drm_gem_dmabuf_export(dev, &exp_info); >>>> +} >>>> + >>>> +/** >>>> + * drm_gem_shmem_prime_import - 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(struct drm_device *dev, >>>> + struct dma_buf *dma_buf) >>>> +{ >>>> + struct dma_buf_attachment *attach; >>>> + struct drm_gem_shmem_object *shmem; >>>> + size_t size; >>>> + int ret; >>>> + >>>> + if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) { >>>> + struct drm_gem_object *obj; >>>> + >>>> + 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, 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); >>>> + >>>> MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); >>>> MODULE_IMPORT_NS("DMA_BUF"); >>>> MODULE_LICENSE("GPL v2"); >>>> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h >>>> index cef5a6b5a4d6..78ef91593a8e 100644 >>>> --- a/include/drm/drm_gem_shmem_helper.h >>>> +++ b/include/drm/drm_gem_shmem_helper.h >>>> @@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, >>>> void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, >>>> struct iosys_map *map); >>>> int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); >>>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>>> + int flags); >>>> int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); >>>> void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); >>>> @@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign >>>> drm_gem_shmem_print_info(shmem, p, indent); >>>> } >>>> +/** >>>> + * drm_gem_shmem_object_prime_export - GEM object function for export() >>>> + * @obj: GEM object >>>> + * >>>> + */ >>>> +static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj, >>>> + int flags) >>>> +{ >>>> + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); >>>> + >>>> + return drm_gem_shmem_prime_export(shmem, flags); >>>> +} >>>> /** >>>> * drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin() >>>> * @obj: GEM object >>>> @@ -276,6 +290,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(struct drm_device *dev, >>>> + struct dma_buf *buf); >>>> /** >>>> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations >>>> @@ -287,4 +303,12 @@ 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 >>>> +/** >>>> + * This macro provides a shmem GEM operations that implementate a simple >>>> + * gem_prime_import. >>>> + */ >>>> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ >>>> + .gem_prime_import = drm_gem_shmem_prime_import, \ >>>> + .dumb_create = drm_gem_shmem_dumb_create >>>> + >>>> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table 2025-05-05 14:32 ` Christian König @ 2025-05-05 15:11 ` oushixiong 2025-05-05 15:23 ` Christian König 0 siblings, 1 reply; 9+ messages in thread From: oushixiong @ 2025-05-05 15:11 UTC (permalink / raw) To: Christian König, Thomas Zimmermann, Maarten Lankhorst Cc: Maxime Ripard, David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou Hi Christian, I don't see strong relevance between my patch and the patches you're referring to. Because the drm_gem_prime_import() function imports sg_table by default, my patch provides an alternative import callback implementation for SHMEM-based drivers that differs from drm_gem_prime_import(). drm_gem_shmem_prime_import_sg_table() doesn't need to call dma_buf_map_attachment_unlocked() to import sg_table. Alternatively, I might not have fully understood the patches you mentioned. Thanks and Regards, Shixiong. 在 2025/5/5 22:32, Christian König 写道: > Hi Shixiong, > > for drm changes please base your patches on drm-misc-next or drm-next. > > That is probably fixed by this one here: > > commit b72f66f22c0e39ae6684c43fead774c13db24e73 > Author: Christian König <christian.koenig@amd.com> > Date: Tue Feb 11 17:20:53 2025 +0100 > > dma-buf: drop caching of sg_tables > > That was purely for the transition from static to dynamic dma-buf > handling and can be removed again now. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch> > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christian.koenig@amd.com > > After this patch SG tables where only created when necessary. > > Regards, > Christian. > > On 5/5/25 16:22, oushixiong wrote: >> Hi Christian, >> >> My patch is based on linux-next, so this patch is not based on the latest code. Then, I'd like to ask which patch resolved the issue with sg-tables? >> >> >> Thanks and Regards, >> >> Shixiong. >> >> >> 在 2025/5/5 19:25, Christian König 写道: >>> Hi Thomas & Shixiong, >>> >>> first of all the patch is still based on outdated code. For example the cache_sgt_mapping member is already removed in drm-misc-next. >>> >>> So if I'm not completely mistaken the issue is already resolved upstream. >>> >>> Regards, >>> Christian. >>> >>> On 5/5/25 13:12, Thomas Zimmermann wrote: >>>> (cc'ing Christian) >>>> >>>> Hi, >>>> >>>> I don't feel qualified to fully review this patch. >>>> >>>> It would be good to have the issue with sg-tables solved, but I dislike the dedicated initializer macros. So my question is if this has any drawbacks. Or could we make this available and the default for all shmem-based drivers? >>>> >>>> Best regards >>>> Thomas >>>> >>>> Am 01.05.25 um 08:43 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> >>>>> --- >>>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++ >>>>> include/drm/drm_gem_shmem_helper.h | 24 +++++++ >>>>> 2 files changed, 119 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>> index d99dee67353a..9e41e350ff6f 100644 >>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>> @@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF"); >>>>> static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { >>>>> .free = drm_gem_shmem_object_free, >>>>> .print_info = drm_gem_shmem_object_print_info, >>>>> + .export = drm_gem_shmem_object_prime_export, >>>>> .pin = drm_gem_shmem_object_pin, >>>>> .unpin = drm_gem_shmem_object_unpin, >>>>> .get_sg_table = drm_gem_shmem_object_get_sg_table, >>>>> @@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >>>>> } >>>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); >>>>> +const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = { >>>>> + .cache_sgt_mapping = true, >>>>> + .attach = drm_gem_map_attach, >>>>> + .detach = drm_gem_map_detach, >>>>> + .map_dma_buf = drm_gem_map_dma_buf, >>>>> + .unmap_dma_buf = drm_gem_unmap_dma_buf, >>>>> + .release = drm_gem_dmabuf_release, >>>>> + .mmap = drm_gem_dmabuf_mmap, >>>>> + .vmap = drm_gem_dmabuf_vmap, >>>>> + .vunmap = drm_gem_dmabuf_vunmap, >>>>> +}; >>>>> + >>>>> +/** >>>>> + * drm_gem_shmem_prime_export - implementation of the export callback >>>>> + * @shmem: shmem GEM object >>>>> + */ >>>>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>>>> + int flags) >>>>> +{ >>>>> + struct drm_gem_object *obj = &shmem->base; >>>>> + struct drm_device *dev = obj->dev; >>>>> + struct dma_buf_export_info exp_info = { >>>>> + .exp_name = KBUILD_MODNAME, /* white lie for debug */ >>>>> + .owner = dev->driver->fops->owner, >>>>> + .ops = &drm_gem_shmem_prime_dmabuf_ops, >>>>> + .size = obj->size, >>>>> + .flags = flags, >>>>> + .priv = obj, >>>>> + .resv = obj->resv, >>>>> + }; >>>>> + >>>>> + return drm_gem_dmabuf_export(dev, &exp_info); >>>>> +} >>>>> + >>>>> +/** >>>>> + * drm_gem_shmem_prime_import - 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(struct drm_device *dev, >>>>> + struct dma_buf *dma_buf) >>>>> +{ >>>>> + struct dma_buf_attachment *attach; >>>>> + struct drm_gem_shmem_object *shmem; >>>>> + size_t size; >>>>> + int ret; >>>>> + >>>>> + if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) { >>>>> + struct drm_gem_object *obj; >>>>> + >>>>> + 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, 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); >>>>> + >>>>> MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); >>>>> MODULE_IMPORT_NS("DMA_BUF"); >>>>> MODULE_LICENSE("GPL v2"); >>>>> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h >>>>> index cef5a6b5a4d6..78ef91593a8e 100644 >>>>> --- a/include/drm/drm_gem_shmem_helper.h >>>>> +++ b/include/drm/drm_gem_shmem_helper.h >>>>> @@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, >>>>> void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, >>>>> struct iosys_map *map); >>>>> int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); >>>>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>>>> + int flags); >>>>> int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); >>>>> void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); >>>>> @@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign >>>>> drm_gem_shmem_print_info(shmem, p, indent); >>>>> } >>>>> +/** >>>>> + * drm_gem_shmem_object_prime_export - GEM object function for export() >>>>> + * @obj: GEM object >>>>> + * >>>>> + */ >>>>> +static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj, >>>>> + int flags) >>>>> +{ >>>>> + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); >>>>> + >>>>> + return drm_gem_shmem_prime_export(shmem, flags); >>>>> +} >>>>> /** >>>>> * drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin() >>>>> * @obj: GEM object >>>>> @@ -276,6 +290,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(struct drm_device *dev, >>>>> + struct dma_buf *buf); >>>>> /** >>>>> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations >>>>> @@ -287,4 +303,12 @@ 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 >>>>> +/** >>>>> + * This macro provides a shmem GEM operations that implementate a simple >>>>> + * gem_prime_import. >>>>> + */ >>>>> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ >>>>> + .gem_prime_import = drm_gem_shmem_prime_import, \ >>>>> + .dumb_create = drm_gem_shmem_dumb_create >>>>> + >>>>> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table 2025-05-05 15:11 ` oushixiong @ 2025-05-05 15:23 ` Christian König 0 siblings, 0 replies; 9+ messages in thread From: Christian König @ 2025-05-05 15:23 UTC (permalink / raw) To: oushixiong, Thomas Zimmermann, Maarten Lankhorst Cc: Maxime Ripard, David Airlie, Simona Vetter, Sean Paul, Jocelyn Falempe, dri-devel, linux-kernel, Shixiong Ou Hi Shixiong, could also be that I didn't fully got what you try to do here. Between which drivers and implementations are the BOs shared exactly? You could also just rebase your patches on drm-misc-next and send them out again and I can take another look. Regards, Christian. On 5/5/25 17:11, oushixiong wrote: > Hi Christian, > > I don't see strong relevance between my patch and the patches you're referring to. > Because the drm_gem_prime_import() function imports sg_table by default, my patch provides an alternative > import callback implementation for SHMEM-based drivers that differs from drm_gem_prime_import(). > drm_gem_shmem_prime_import_sg_table() doesn't need to call dma_buf_map_attachment_unlocked() to import sg_table. > > Alternatively, I might not have fully understood the patches you mentioned. > > > Thanks and Regards, > > Shixiong. > > 在 2025/5/5 22:32, Christian König 写道: >> Hi Shixiong, >> >> for drm changes please base your patches on drm-misc-next or drm-next. >> >> That is probably fixed by this one here: >> >> commit b72f66f22c0e39ae6684c43fead774c13db24e73 >> Author: Christian König <christian.koenig@amd.com> >> Date: Tue Feb 11 17:20:53 2025 +0100 >> >> dma-buf: drop caching of sg_tables >> That was purely for the transition from static to dynamic dma-buf >> handling and can be removed again now. >> Signed-off-by: Christian König <christian.koenig@amd.com> >> Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch> >> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >> Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christian.koenig@amd.com >> >> After this patch SG tables where only created when necessary. >> >> Regards, >> Christian. >> >> On 5/5/25 16:22, oushixiong wrote: >>> Hi Christian, >>> >>> My patch is based on linux-next, so this patch is not based on the latest code. Then, I'd like to ask which patch resolved the issue with sg-tables? >>> >>> >>> Thanks and Regards, >>> >>> Shixiong. >>> >>> >>> 在 2025/5/5 19:25, Christian König 写道: >>>> Hi Thomas & Shixiong, >>>> >>>> first of all the patch is still based on outdated code. For example the cache_sgt_mapping member is already removed in drm-misc-next. >>>> >>>> So if I'm not completely mistaken the issue is already resolved upstream. >>>> >>>> Regards, >>>> Christian. >>>> >>>> On 5/5/25 13:12, Thomas Zimmermann wrote: >>>>> (cc'ing Christian) >>>>> >>>>> Hi, >>>>> >>>>> I don't feel qualified to fully review this patch. >>>>> >>>>> It would be good to have the issue with sg-tables solved, but I dislike the dedicated initializer macros. So my question is if this has any drawbacks. Or could we make this available and the default for all shmem-based drivers? >>>>> >>>>> Best regards >>>>> Thomas >>>>> >>>>> Am 01.05.25 um 08:43 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> >>>>>> --- >>>>>> drivers/gpu/drm/drm_gem_shmem_helper.c | 95 ++++++++++++++++++++++++++ >>>>>> include/drm/drm_gem_shmem_helper.h | 24 +++++++ >>>>>> 2 files changed, 119 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>>> index d99dee67353a..9e41e350ff6f 100644 >>>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>>>>> @@ -39,6 +39,7 @@ MODULE_IMPORT_NS("DMA_BUF"); >>>>>> static const struct drm_gem_object_funcs drm_gem_shmem_funcs = { >>>>>> .free = drm_gem_shmem_object_free, >>>>>> .print_info = drm_gem_shmem_object_print_info, >>>>>> + .export = drm_gem_shmem_object_prime_export, >>>>>> .pin = drm_gem_shmem_object_pin, >>>>>> .unpin = drm_gem_shmem_object_unpin, >>>>>> .get_sg_table = drm_gem_shmem_object_get_sg_table, >>>>>> @@ -799,6 +800,100 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); >>>>>> +const struct dma_buf_ops drm_gem_shmem_prime_dmabuf_ops = { >>>>>> + .cache_sgt_mapping = true, >>>>>> + .attach = drm_gem_map_attach, >>>>>> + .detach = drm_gem_map_detach, >>>>>> + .map_dma_buf = drm_gem_map_dma_buf, >>>>>> + .unmap_dma_buf = drm_gem_unmap_dma_buf, >>>>>> + .release = drm_gem_dmabuf_release, >>>>>> + .mmap = drm_gem_dmabuf_mmap, >>>>>> + .vmap = drm_gem_dmabuf_vmap, >>>>>> + .vunmap = drm_gem_dmabuf_vunmap, >>>>>> +}; >>>>>> + >>>>>> +/** >>>>>> + * drm_gem_shmem_prime_export - implementation of the export callback >>>>>> + * @shmem: shmem GEM object >>>>>> + */ >>>>>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>>>>> + int flags) >>>>>> +{ >>>>>> + struct drm_gem_object *obj = &shmem->base; >>>>>> + struct drm_device *dev = obj->dev; >>>>>> + struct dma_buf_export_info exp_info = { >>>>>> + .exp_name = KBUILD_MODNAME, /* white lie for debug */ >>>>>> + .owner = dev->driver->fops->owner, >>>>>> + .ops = &drm_gem_shmem_prime_dmabuf_ops, >>>>>> + .size = obj->size, >>>>>> + .flags = flags, >>>>>> + .priv = obj, >>>>>> + .resv = obj->resv, >>>>>> + }; >>>>>> + >>>>>> + return drm_gem_dmabuf_export(dev, &exp_info); >>>>>> +} >>>>>> + >>>>>> +/** >>>>>> + * drm_gem_shmem_prime_import - 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(struct drm_device *dev, >>>>>> + struct dma_buf *dma_buf) >>>>>> +{ >>>>>> + struct dma_buf_attachment *attach; >>>>>> + struct drm_gem_shmem_object *shmem; >>>>>> + size_t size; >>>>>> + int ret; >>>>>> + >>>>>> + if (dma_buf->ops == &drm_gem_shmem_prime_dmabuf_ops) { >>>>>> + struct drm_gem_object *obj; >>>>>> + >>>>>> + 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, 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); >>>>>> + >>>>>> MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); >>>>>> MODULE_IMPORT_NS("DMA_BUF"); >>>>>> MODULE_LICENSE("GPL v2"); >>>>>> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h >>>>>> index cef5a6b5a4d6..78ef91593a8e 100644 >>>>>> --- a/include/drm/drm_gem_shmem_helper.h >>>>>> +++ b/include/drm/drm_gem_shmem_helper.h >>>>>> @@ -110,6 +110,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, >>>>>> void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, >>>>>> struct iosys_map *map); >>>>>> int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); >>>>>> +struct dma_buf *drm_gem_shmem_prime_export(struct drm_gem_shmem_object *shmem, >>>>>> + int flags); >>>>>> int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); >>>>>> void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); >>>>>> @@ -168,6 +170,18 @@ static inline void drm_gem_shmem_object_print_info(struct drm_printer *p, unsign >>>>>> drm_gem_shmem_print_info(shmem, p, indent); >>>>>> } >>>>>> +/** >>>>>> + * drm_gem_shmem_object_prime_export - GEM object function for export() >>>>>> + * @obj: GEM object >>>>>> + * >>>>>> + */ >>>>>> +static inline struct dma_buf *drm_gem_shmem_object_prime_export(struct drm_gem_object *obj, >>>>>> + int flags) >>>>>> +{ >>>>>> + struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); >>>>>> + >>>>>> + return drm_gem_shmem_prime_export(shmem, flags); >>>>>> +} >>>>>> /** >>>>>> * drm_gem_shmem_object_pin - GEM object function for drm_gem_shmem_pin() >>>>>> * @obj: GEM object >>>>>> @@ -276,6 +290,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(struct drm_device *dev, >>>>>> + struct dma_buf *buf); >>>>>> /** >>>>>> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations >>>>>> @@ -287,4 +303,12 @@ 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 >>>>>> +/** >>>>>> + * This macro provides a shmem GEM operations that implementate a simple >>>>>> + * gem_prime_import. >>>>>> + */ >>>>>> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ >>>>>> + .gem_prime_import = drm_gem_shmem_prime_import, \ >>>>>> + .dumb_create = drm_gem_shmem_dumb_create >>>>>> + >>>>>> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-05-05 15:23 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-05-01 6:43 [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table oushixiong1025 2025-05-01 6:43 ` [PATCH 2/3] drm/ast: use DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS oushixiong1025 2025-05-01 6:43 ` [PATCH 3/3] drm/udl: " oushixiong1025 2025-05-05 11:12 ` [PATCH 1/3] drm/shmem-helper: Import dmabuf without mapping its sg_table Thomas Zimmermann 2025-05-05 11:25 ` Christian König 2025-05-05 14:22 ` oushixiong 2025-05-05 14:32 ` Christian König 2025-05-05 15:11 ` oushixiong 2025-05-05 15:23 ` Christian König
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®