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 oushixiong1...@163.com:
From: Shixiong Ou <oushixi...@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 <oushixi...@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__ */

Reply via email to