<div>Yeah,</div><div>but I'm focusing on the importer side. For some drivers like AST/UDL acting as importers, they may not need to call `map_dma_buf` at all. I'd like to provide another `DRM_GEM_SHMEM_DRIVER_OPS` (distinct from the existing one) that skips `dma_buf_map_attachment` for these drivers, eliminating unnecessary issues like "swiotlb buffer is full".</div><div> </div><div>Regards,</div><div>Shixiong Ou </div><div> </div><div></div><div><div style="font-size:12px;padding:2px 0;" class="xm_mail_oringinal_describe">---Original---</div><div style="font-size:12px;background:#f0f0f0;color:#212121;padding:8px!important;border-radius:4px;line-height:1.5;"><div class="xm_mail_oringinal_describe">From: "Christian König"<christian.koenig@amd.com></div><div class="xm_mail_oringinal_describe">Date: Wed, Apr 30, 2025 22:48 PM</div><div class="xm_mail_oringinal_describe">To: "oushixiong"<oushixiong1025@163.com>;"Sumit Semwal"<sumit.semwal@linaro.org>;</div><div class="xm_mail_oringinal_describe">Cc: "linux-media"<linux-media@vger.kernel.org>;"dri-devel"<dri-devel@lists.freedesktop.org>;"linaro-mm-sig"<linaro-mm-sig@lists.linaro.org>;"linux-kernel"<linux-kernel@vger.kernel.org>;"Maarten Lankhorst"<maarten.lankhorst@linux.intel.com>;"Maxime Ripard"<mripard@kernel.org>;"Thomas Zimmermann"<tzimmermann@suse.de>;"David Airlie"<airlied@gmail.com>;"Simona Vetter"<simona@ffwll.ch>;"Dave Airlie"<airlied@redhat.com>;"Sean Paul"<sean@poorly.run>;"Shixiong Ou"<oushixiong@kylinos.cn>;</div><div class="xm_mail_oringinal_describe">Subject: Re: [PATCH 2/3] drm/prime: Support importing DMA-BUF without sg_table</div></div> 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 Ou<oushixiong@kylinos.cn> >>> >>> [WHY] >>> On some boards, the dma_mask of Aspeed devices is 0xffff_ffff, this >>> quite possibly causes the SWIOTLB to be triggered when importing dmabuf. >>> However IO_TLB_SEGSIZE limits the maximum amount of available memory >>> for DMA Streaming Mapping, as dmesg following: >>> >>> [ 24.885303][ T1947] ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots) >>> >>> [HOW] Provide an interface so that attachment is not mapped when >>> importing dma-buf. >> This is unecessary. The extra abstraction in DRM is only useful when you want to implement the obj->funcs->get_sg_table() callback. >> >> When a driver doesn't want to expose an sg_table for a buffer or want some other special handling it can simply do so by implementing the DMA-buf interface directly. >> >> See drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c for an example on how to do this. >> >> Regards, >> Christian. > > > 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 Ou<oushixiong@kylinos.cn> >>> --- >>> drivers/gpu/drm/ast/ast_drv.c | 2 +- >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 17 +++++++ >>> drivers/gpu/drm/drm_prime.c | 67 ++++++++++++++++++++++++-- >>> drivers/gpu/drm/udl/udl_drv.c | 2 +- >>> include/drm/drm_drv.h | 3 ++ >>> include/drm/drm_gem_shmem_helper.h | 6 +++ >>> 6 files changed, 91 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c >>> index 6fbf62a99c48..2dac6acf79e7 100644 >>> --- a/drivers/gpu/drm/ast/ast_drv.c >>> +++ b/drivers/gpu/drm/ast/ast_drv.c >>> @@ -64,7 +64,7 @@ static const struct drm_driver ast_driver = { >>> .minor = DRIVER_MINOR, >>> .patchlevel = DRIVER_PATCHLEVEL, >>> - DRM_GEM_SHMEM_DRIVER_OPS, >>> + DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS, >>> DRM_FBDEV_SHMEM_DRIVER_OPS, >>> }; >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> index d99dee67353a..655d841df933 100644 >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> @@ -799,6 +799,23 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >>> } >>> EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table); >>> +struct drm_gem_object * >>> +drm_gem_shmem_prime_import_attachment(struct drm_device *dev, >>> + struct dma_buf_attachment *attach) >>> +{ >>> + size_t size = PAGE_ALIGN(attach->dmabuf->size); >>> + struct drm_gem_shmem_object *shmem; >>> + >>> + shmem = __drm_gem_shmem_create(dev, size, true, NULL); >>> + if (IS_ERR(shmem)) >>> + return ERR_CAST(shmem); >>> + >>> + drm_dbg_prime(dev, "size = %zu\n", size); >>> + >>> + return &shmem->base; >>> +} >>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_attachment); >>> + >>> MODULE_DESCRIPTION("DRM SHMEM memory-management helpers"); >>> MODULE_IMPORT_NS("DMA_BUF"); >>> MODULE_LICENSE("GPL v2"); >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >>> index 8e70abca33b9..522cf974e202 100644 >>> --- a/drivers/gpu/drm/drm_prime.c >>> +++ b/drivers/gpu/drm/drm_prime.c >>> @@ -911,6 +911,62 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj, >>> } >>> EXPORT_SYMBOL(drm_gem_prime_export); >>> +/** >>> + * drm_gem_prime_import_dev_skip_map - core implementation of the import callback >>> + * @dev: drm_device to import into >>> + * @dma_buf: dma-buf object to import >>> + * @attach_dev: struct device to dma_buf attach >>> + * >>> + * This function exports a dma-buf without get it's scatter/gather table. >>> + * >>> + * Drivers who need to get an scatter/gather table for objects need to call >>> + * drm_gem_prime_import_dev() instead. >>> + */ >>> +struct drm_gem_object *drm_gem_prime_import_dev_skip_map(struct drm_device *dev, >>> + struct dma_buf *dma_buf, >>> + struct device *attach_dev) >>> +{ >>> + struct dma_buf_attachment *attach; >>> + struct drm_gem_object *obj; >>> + int ret; >>> + >>> + if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) { >>> + obj = dma_buf->priv; >>> + if (obj->dev == dev) { >>> + /* >>> + * Importing dmabuf exported from our own gem increases >>> + * refcount on gem itself instead of f_count of dmabuf. >>> + */ >>> + drm_gem_object_get(obj); >>> + return obj; >>> + } >>> + } >>> + >>> + attach = dma_buf_attach(dma_buf, attach_dev, true); >>> + if (IS_ERR(attach)) >>> + return ERR_CAST(attach); >>> + >>> + get_dma_buf(dma_buf); >>> + >>> + obj = dev->driver->gem_prime_import_attachment(dev, attach); >>> + if (IS_ERR(obj)) { >>> + ret = PTR_ERR(obj); >>> + goto fail_detach; >>> + } >>> + >>> + obj->import_attach = attach; >>> + obj->resv = dma_buf->resv; >>> + >>> + return obj; >>> + >>> +fail_detach: >>> + dma_buf_detach(dma_buf, attach); >>> + dma_buf_put(dma_buf); >>> + >>> + return ERR_PTR(ret); >>> +} >>> +EXPORT_SYMBOL(drm_gem_prime_import_dev_skip_map); >>> + >>> /** >>> * drm_gem_prime_import_dev - core implementation of the import callback >>> * @dev: drm_device to import into >>> @@ -946,9 +1002,6 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, >>> } >>> } >>> - if (!dev->driver->gem_prime_import_sg_table) >>> - return ERR_PTR(-EINVAL); >>> - >>> attach = dma_buf_attach(dma_buf, attach_dev, false); >>> if (IS_ERR(attach)) >>> return ERR_CAST(attach); >>> @@ -998,7 +1051,13 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); >>> struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, >>> struct dma_buf *dma_buf) >>> { >>> - return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); >>> + if (dev->driver->gem_prime_import_sg_table) >>> + return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); >>> + else if (dev->driver->gem_prime_import_attachment) >>> + return drm_gem_prime_import_dev_skip_map(dev, dma_buf, dev->dev); >>> + else >>> + return ERR_PTR(-EINVAL); >>> + >>> } >>> EXPORT_SYMBOL(drm_gem_prime_import); >>> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c >>> index 05b3a152cc33..c00d8b8834f2 100644 >>> --- a/drivers/gpu/drm/udl/udl_drv.c >>> +++ b/drivers/gpu/drm/udl/udl_drv.c >>> @@ -72,7 +72,7 @@ static const struct drm_driver driver = { >>> /* GEM hooks */ >>> .fops = &udl_driver_fops, >>> - DRM_GEM_SHMEM_DRIVER_OPS, >>> + DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS, >>> .gem_prime_import = udl_driver_gem_prime_import, >>> DRM_FBDEV_SHMEM_DRIVER_OPS, >>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h >>> index a43d707b5f36..aef8d9051fcd 100644 >>> --- a/include/drm/drm_drv.h >>> +++ b/include/drm/drm_drv.h >>> @@ -326,6 +326,9 @@ struct drm_driver { >>> struct dma_buf_attachment *attach, >>> struct sg_table *sgt); >>> + struct drm_gem_object *(*gem_prime_import_attachment)( >>> + struct drm_device *dev, >>> + struct dma_buf_attachment *attach); >>> /** >>> * @dumb_create: >>> * >>> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h >>> index cef5a6b5a4d6..39a93c222aaa 100644 >>> --- a/include/drm/drm_gem_shmem_helper.h >>> +++ b/include/drm/drm_gem_shmem_helper.h >>> @@ -274,6 +274,9 @@ struct drm_gem_object * >>> drm_gem_shmem_prime_import_sg_table(struct drm_device *dev, >>> struct dma_buf_attachment *attach, >>> struct sg_table *sgt); >>> +struct drm_gem_object * >>> +drm_gem_shmem_prime_import_attachment(struct drm_device *dev, >>> + struct dma_buf_attachment *attach); >>> int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, >>> struct drm_mode_create_dumb *args); >>> @@ -287,4 +290,7 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, >>> .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \ >>> .dumb_create = drm_gem_shmem_dumb_create >>> +#define DRM_GEM_SHMEM_SIMPLE_DRIVER_OPS \ >>> + .gem_prime_import_attachment = drm_gem_shmem_prime_import_attachment, \ >>> + .dumb_create = drm_gem_shmem_dumb_create >>> #endif /* __DRM_GEM_SHMEM_HELPER_H__ */ </oushixiong@kylinos.cn></oushixiong@kylinos.cn></div>