On 4/30/25 16:13, oushixiong wrote:

在 2025/4/30 19:03, Christian König 写道:

...

On 4/30/25 10:56,oushixiong1025@163.com wrote:

...

From: Shixiong Ououshixiong@kylinos.cn

[WHY] On some boards, the dma_mask of Aspeed devices is 0xffff_ffff, this quite possibly causes the SWIOTLB to be triggered when importing dmabuf. However IO_TLB_SEGSIZE limits the maximum amount of available memory for DMA Streaming Mapping, as dmesg following:

[   24.885303][ T1947] ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)

[HOW] Provide an interface so that attachment is not mapped when importing dma-buf.

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

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

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

Regards, Christian.

Thanks for the reminder,

most drivers that use DRM_GEM_SHADOW_PLANE_HELPER_FUNCSand DRM_GEM_SHMEM_DRIVER_OPS

don't need to import the sg_table, such as the udl and the ast and so on at the moment.

They just need to call dma_buf_vmap() to get the kernel virtual address of the shared buffer.

So I wondered if there was a simple generic PRIME implementation for these drivers.

If you don't recommend this, Maybe try to implement it in DRM_GEM_SHMEM_DRIVER_OPS ?

Well if you only want to implement vmap/vunmap the necessary code in the driver would look something like this:

const struct dma_buf_ops amdgpu_dmabuf_ops = { .map_dma_buf = dummy_map_function, .release = drm_gem_dmabuf_release, .mmap = drm_gem_dmabuf_mmap, .vmap = drm_gem_dmabuf_vmap, .vunmap = drm_gem_dmabuf_vunmap, };

struct dma_buf *drv_gem_prime_export(struct drm_gem_object *gobj, int flags) { struct dma_buf *buf;

buf = drm_gem_prime_export(gobj, flags); if (!IS_ERR(buf)) buf->ops = &amdgpu_dmabuf_ops;

return buf; }

The only thing which could be improved is the dummy_map_function. As far as I can see we could make the map function optional in DMA-buf now.

Apart from that you could make a DRM helper from that few lines, but to be honest I don't think it's worth it. It reduces the loc a bit, but there is no real complexity here which drivers could share.

Regards, Christian.

Regards,

Shixiong Ou.

...

...

Signed-off-by: Shixiong Ououshixiong@kylinos.cn

drivers/gpu/drm/ast/ast_drv.c          |  2 +-   drivers/gpu/drm/drm_gem_shmem_helper.c | 17 +++++++   drivers/gpu/drm/drm_prime.c            | 67 ++++++++++++++++++++++++--   drivers/gpu/drm/udl/udl_drv.c          |  2 +-   include/drm/drm_drv.h                  |  3 ++   include/drm/drm_gem_shmem_helper.h     |  6 +++   6 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index 6fbf62a99c48..2dac6acf79e7 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -64,7 +64,7 @@ static const struct drm_driver ast_driver = {       .minor = DRIVER_MINOR,       .patchlevel = DRIVER_PATCHLEVEL,   -    DRM_GEM_SHMEM_DRIVER_OPS, +    DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS,       DRM_FBDEV_SHMEM_DRIVER_OPS,   };   diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index d99dee67353a..655d841df933 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -799,6 +799,23 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,   }   EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);   +struct drm_gem_object * +drm_gem_shmem_prime_import_attachment(struct drm_device *dev, +                      struct dma_buf_attachment *attach) +{ +    size_t size = PAGE_ALIGN(attach->dmabuf->size); +    struct drm_gem_shmem_object *shmem;

+    shmem = __drm_gem_shmem_create(dev, size, true, NULL); +    if (IS_ERR(shmem)) +        return ERR_CAST(shmem);

+    drm_dbg_prime(dev, "size = %zu\n", size);

+    return &shmem->base; +} +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_attachment);

MODULE_DESCRIPTION("DRM SHMEM memory-management helpers");   MODULE_IMPORT_NS("DMA_BUF");   MODULE_LICENSE("GPL v2"); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 8e70abca33b9..522cf974e202 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -911,6 +911,62 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,   }   EXPORT_SYMBOL(drm_gem_prime_export);   +/**

• • drm_gem_prime_import_dev_skip_map - core implementation of the import callback
• • @dev: drm_device to import into
• • @dma_buf: dma-buf object to import
• • @attach_dev: struct device to dma_buf attach
• • This function exports a dma-buf without get it's scatter/gather table.
• • Drivers who need to get an scatter/gather table for objects need to call
• • drm_gem_prime_import_dev() instead.
• */

+struct drm_gem_object *drm_gem_prime_import_dev_skip_map(struct drm_device *dev, +                             struct dma_buf *dma_buf, +                             struct device *attach_dev) +{ +    struct dma_buf_attachment *attach; +    struct drm_gem_object *obj; +    int ret;

+    if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) { +        obj = dma_buf->priv; +        if (obj->dev == dev) { +            /* +             * Importing dmabuf exported from our own gem increases +             * refcount on gem itself instead of f_count of dmabuf. +             */ +            drm_gem_object_get(obj); +            return obj; +        } +    }

+    attach = dma_buf_attach(dma_buf, attach_dev, true); +    if (IS_ERR(attach)) +        return ERR_CAST(attach);

+    get_dma_buf(dma_buf);

+    obj = dev->driver->gem_prime_import_attachment(dev, attach); +    if (IS_ERR(obj)) { +        ret = PTR_ERR(obj); +        goto fail_detach; +    }

+    obj->import_attach = attach; +    obj->resv = dma_buf->resv;

+    return obj;

+fail_detach: +    dma_buf_detach(dma_buf, attach); +    dma_buf_put(dma_buf);

+    return ERR_PTR(ret); +} +EXPORT_SYMBOL(drm_gem_prime_import_dev_skip_map);

/**    * drm_gem_prime_import_dev - core implementation of the import callback    * @dev: drm_device to import into @@ -946,9 +1002,6 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,           }       }   -    if (!dev->driver->gem_prime_import_sg_table) -        return ERR_PTR(-EINVAL);

attach = dma_buf_attach(dma_buf, attach_dev, false);       if (IS_ERR(attach))           return ERR_CAST(attach); @@ -998,7 +1051,13 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev);   struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,                           struct dma_buf *dma_buf)   { -    return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); +    if (dev->driver->gem_prime_import_sg_table) +        return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); +    else if (dev->driver->gem_prime_import_attachment) +        return drm_gem_prime_import_dev_skip_map(dev, dma_buf, dev->dev); +    else +        return ERR_PTR(-EINVAL);

}   EXPORT_SYMBOL(drm_gem_prime_import);   diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index 05b3a152cc33..c00d8b8834f2 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -72,7 +72,7 @@ static const struct drm_driver driver = {         /* GEM hooks */       .fops = &udl_driver_fops, -    DRM_GEM_SHMEM_DRIVER_OPS, +    DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS,       .gem_prime_import = udl_driver_gem_prime_import,       DRM_FBDEV_SHMEM_DRIVER_OPS,   diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index a43d707b5f36..aef8d9051fcd 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -326,6 +326,9 @@ struct drm_driver {                   struct dma_buf_attachment *attach,                   struct sg_table *sgt);   +    struct drm_gem_object *(*gem_prime_import_attachment)( +                struct drm_device *dev, +                struct dma_buf_attachment *attach);       /**        * @dumb_create:        * diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index cef5a6b5a4d6..39a93c222aaa 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -274,6 +274,9 @@ struct drm_gem_object *   drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,                       struct dma_buf_attachment *attach,                       struct sg_table *sgt); +struct drm_gem_object * +drm_gem_shmem_prime_import_attachment(struct drm_device *dev, +                      struct dma_buf_attachment *attach);   int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,                     struct drm_mode_create_dumb *args);   @@ -287,4 +290,7 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,       .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \       .dumb_create           = drm_gem_shmem_dumb_create   +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ +    .gem_prime_import_attachment = drm_gem_shmem_prime_import_attachment, \ +    .dumb_create                 = drm_gem_shmem_dumb_create   #endif /* __DRM_GEM_SHMEM_HELPER_H__ */