[PATCH RESEND 0/4] drm/gem-dma: Support dedicated DMA device for allocation

[PATCH RESEND 0/4] drm/gem-dma: Support dedicated DMA device for allocation

[Chen-Yu Tsai]

(resent with Samuel's email address fixed)

Hi folks,

This series expands the "dedicated DMA device" support in DRM to the GEM
DMA helpers, and converts the MediaTek DRM driver to setting the DMA
device and dropping the custom GEM helpers that implemented this
function.

Various display drivers implement the "dedicated DMA device" support
with custom GEM helpers. These include Exynos, MediaTek, and Rockchip
to name a few. Allwinner does something entirely different, calling
of_dma_configure() on the virtual display device using the OF node of
the actual DMA device. Recently this causes a warning if IOMMUs are
involved.

This series intends to allow the core helpers to deal with it, and not
have every driver implement it in slightly different ways, duplicating
code.

Patch 1 adds dedicated DMA device support to drm_prime_pages_to_sg().
I believe this was missing from the original change that added dedicated
DMA devices for PRIME.

Patch 2 adds support for dedicated DMA device to the GEM DMA helpers
for GEM buffer allocation and mmap.

Patch 3 converts the MediaTek DRM driver to use the dedicated DMA device
support, and drop all the remaining custom GEM callbacks that deal with
it.

Patch 4 converts the Allwinner sun4i DRM driver to use the dedicated DMA
device support, instead of the of_dma_configure() hack it currently has.

The series should be merged through drm-misc-next so that other drivers
can take advantage of the change.

I also intend to try to convert the Exynos and Rockchip drivers, however
both also have options to set DMA_ATTR_NO_KERNEL_MAPPING when using
dma_alloc_attrs() to allocate memory for the buffers. I intend to
resurrect the DRM_MODE_DUMB_KERNEL_MAP work from Rob Herring [1]
to handle this. The Rockchip driver also has custom IOMMU attachment
that I'm still trying to understand.


Thanks
ChenYu


Chen-Yu Tsai (4):
  drm/prime: Limit scatter list size with dedicated DMA device
  drm/gem-dma: Support dedicated DMA device for allocation and mapping
  drm/mediatek: Set dedicated DMA device and drop custom GEM callbacks
  drm/sun4i: Use backend/mixer as dedicated DMA device

 drivers/gpu/drm/drm_gem_dma_helper.c   |  21 ++-
 drivers/gpu/drm/drm_prime.c            |   2 +-
 drivers/gpu/drm/mediatek/mtk_crtc.c    |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_drv.c |  21 +--
 drivers/gpu/drm/mediatek/mtk_drm_drv.h |   1 -
 drivers/gpu/drm/mediatek/mtk_gem.c     | 231 -------------------------
 drivers/gpu/drm/mediatek/mtk_gem.h     |  17 --
 drivers/gpu/drm/sun4i/sun4i_backend.c  |  27 +--
 drivers/gpu/drm/sun4i/sun8i_mixer.c    |  27 +--
 9 files changed, 46 insertions(+), 302 deletions(-)
 delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.c
 delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.h

-- 
2.53.0.473.g4a7958ca14-goog

[PATCH RESEND 1/4] drm/prime: Limit scatter list size with dedicated DMA device

[Chen-Yu Tsai]

If a dedicated DMA device is specified for the DRM device, then the
scatter list size limit should pertain to the DMA device.

Use the dedicated DMA device, if given, to limit the scatter list size.
This only applies to drivers that have called drm_dev_set_dma_dev() and
are using drm_prime_pages_to_sg() either directly or through the SHMEM
helpers. At the time of this writing, the former case only includes the
Rockchip DRM driver, while the latter case includes the gud, udl, and
the tiny appletbdrm and gm12u320 drivers.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/drm_prime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 51fdb06d3e9f..9b44c78cd77f 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -859,7 +859,7 @@ struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	if (dev)
-		max_segment = dma_max_mapping_size(dev->dev);
+		max_segment = dma_max_mapping_size(drm_dev_dma_dev(dev));
 	if (max_segment == 0)
 		max_segment = UINT_MAX;
 	err = sg_alloc_table_from_pages_segment(sg, pages, nr_pages, 0,
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH RESEND 2/4] drm/gem-dma: Support dedicated DMA device for allocation and mapping

[Chen-Yu Tsai]

Support for a dedicated DMA device for prime imports was added in commit
143ec8d3f939 ("drm/prime: Support dedicated DMA device for dma-buf imports").
This allowed the DRM driver to provide a dedicated DMA device when its
own underlying device was not capable of DMA, for example when it is a
USB device (the original target) or a virtual device. The latter case is
common on embedded SoCs, on which the display pipeline is composed of
various fixed function blocks, and the DRM device is simply a made-up
device, an address space managing the routing between the blocks, or
whichever block the implementor thought made sense at the time. The
point is that the chosen device is often not the actual device doing
the DMA. Various drivers have used workarounds or reimplemented the
GEM DMA helpers to get the DMA addresses and IOMMUs to work correctly.

Add support for the dedicated DMA device to the GEM DMA helpers.

No existing driver currently uses the GEM DMA helpers and calls
drm_dev_set_dma_dev() to set a dedicated DMA device, so no existing
users should be affected.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/drm_gem_dma_helper.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c
index ecb9746f4da8..70f83e464476 100644
--- a/drivers/gpu/drm/drm_gem_dma_helper.c
+++ b/drivers/gpu/drm/drm_gem_dma_helper.c
@@ -146,12 +146,13 @@ struct drm_gem_dma_object *drm_gem_dma_create(struct drm_device *drm,
 		return dma_obj;
 
 	if (dma_obj->map_noncoherent) {
-		dma_obj->vaddr = dma_alloc_noncoherent(drm->dev, size,
+		dma_obj->vaddr = dma_alloc_noncoherent(drm_dev_dma_dev(drm),
+						       size,
 						       &dma_obj->dma_addr,
 						       DMA_TO_DEVICE,
 						       GFP_KERNEL | __GFP_NOWARN);
 	} else {
-		dma_obj->vaddr = dma_alloc_wc(drm->dev, size,
+		dma_obj->vaddr = dma_alloc_wc(drm_dev_dma_dev(drm), size,
 					      &dma_obj->dma_addr,
 					      GFP_KERNEL | __GFP_NOWARN);
 	}
@@ -236,12 +237,14 @@ void drm_gem_dma_free(struct drm_gem_dma_object *dma_obj)
 		drm_prime_gem_destroy(gem_obj, dma_obj->sgt);
 	} else if (dma_obj->vaddr) {
 		if (dma_obj->map_noncoherent)
-			dma_free_noncoherent(gem_obj->dev->dev, dma_obj->base.size,
+			dma_free_noncoherent(drm_dev_dma_dev(gem_obj->dev),
+					     dma_obj->base.size,
 					     dma_obj->vaddr, dma_obj->dma_addr,
 					     DMA_TO_DEVICE);
 		else
-			dma_free_wc(gem_obj->dev->dev, dma_obj->base.size,
-				    dma_obj->vaddr, dma_obj->dma_addr);
+			dma_free_wc(drm_dev_dma_dev(gem_obj->dev),
+				    dma_obj->base.size, dma_obj->vaddr,
+				    dma_obj->dma_addr);
 	}
 
 	drm_gem_object_release(gem_obj);
@@ -432,7 +435,7 @@ struct sg_table *drm_gem_dma_get_sg_table(struct drm_gem_dma_object *dma_obj)
 	if (!sgt)
 		return ERR_PTR(-ENOMEM);
 
-	ret = dma_get_sgtable(obj->dev->dev, sgt, dma_obj->vaddr,
+	ret = dma_get_sgtable(drm_dev_dma_dev(obj->dev), sgt, dma_obj->vaddr,
 			      dma_obj->dma_addr, obj->size);
 	if (ret < 0)
 		goto out;
@@ -539,12 +542,12 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *
 	if (dma_obj->map_noncoherent) {
 		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
 
-		ret = dma_mmap_pages(dma_obj->base.dev->dev,
+		ret = dma_mmap_pages(drm_dev_dma_dev(dma_obj->base.dev),
 				     vma, vma->vm_end - vma->vm_start,
 				     virt_to_page(dma_obj->vaddr));
 	} else {
-		ret = dma_mmap_wc(dma_obj->base.dev->dev, vma, dma_obj->vaddr,
-				  dma_obj->dma_addr,
+		ret = dma_mmap_wc(drm_dev_dma_dev(dma_obj->base.dev), vma,
+				  dma_obj->vaddr, dma_obj->dma_addr,
 				  vma->vm_end - vma->vm_start);
 	}
 	if (ret)
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH RESEND 3/4] drm/mediatek: Set dedicated DMA device and drop custom GEM callbacks

[Chen-Yu Tsai]

In commit 9b54a32c7c6a ("drm/mediatek: mtk_gem: Partial refactor and
use drm_gem_dma_object") the MediaTek DRM driver was refactored to use
drm_gem_dma_object, but custom callbacks were still needed to deal with
using the first device of the pipeline as the DMA device, instead of
the MMSYS device that the DRM driver binds to.

Turns out there is already partial support for dedicated DMA devices in
the DRM subsystem for PRIME imports. The preceding patches add support
for dedicated DMA devices to the GEM DMA helpers.

This allows us to just set the dedicated DMA device for the DRM device,
and drop all the custom GEM callbacks. Also drop the .dma_dev field
from the driver private data as it is no longer needed.

There are slight differences in the mmap helper: the VM_DONTDUMP and
VM_IO flags are no longer set. Both were lifted from drm_gem_mmap_obj().
VM_IO probably doesn't make sense since the buffer is allocated using
dma_alloc_attrs().

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_crtc.c    |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_drv.c |  21 +--
 drivers/gpu/drm/mediatek/mtk_drm_drv.h |   1 -
 drivers/gpu/drm/mediatek/mtk_gem.c     | 231 -------------------------
 drivers/gpu/drm/mediatek/mtk_gem.h     |  17 --
 5 files changed, 3 insertions(+), 268 deletions(-)
 delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.c
 delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.h

diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index 351d58c50b84..fcb16f3f7b23 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -23,7 +23,6 @@
 #include "mtk_crtc.h"
 #include "mtk_ddp_comp.h"
 #include "mtk_drm_drv.h"
-#include "mtk_gem.h"
 #include "mtk_plane.h"
 
 /*
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index a94c51a83261..6f6db2e1980e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -19,6 +19,7 @@
 #include <drm/drm_fbdev_dma.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_gem_dma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_ioctl.h>
 #include <drm/drm_of.h>
@@ -29,7 +30,6 @@
 #include "mtk_ddp_comp.h"
 #include "mtk_disp_drv.h"
 #include "mtk_drm_drv.h"
-#include "mtk_gem.h"
 
 #define DRIVER_NAME "mediatek"
 #define DRIVER_DESC "Mediatek SoC DRM"
@@ -565,8 +565,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 		goto err_component_unbind;
 	}
 
-	for (i = 0; i < private->data->mmsys_dev_num; i++)
-		private->all_drm_private[i]->dma_dev = dma_dev;
+	drm_dev_set_dma_dev(drm, dma_dev);
 
 	/*
 	 * Configure the DMA segment size to make sure we get contiguous IOVA
@@ -600,26 +599,12 @@ static void mtk_drm_kms_deinit(struct drm_device *drm)
 
 DEFINE_DRM_GEM_FOPS(mtk_drm_fops);
 
-/*
- * We need to override this because the device used to import the memory is
- * not dev->dev, as drm_gem_prime_import() expects.
- */
-static struct drm_gem_object *mtk_gem_prime_import(struct drm_device *dev,
-						   struct dma_buf *dma_buf)
-{
-	struct mtk_drm_private *private = dev->dev_private;
-
-	return drm_gem_prime_import_dev(dev, dma_buf, private->dma_dev);
-}
-
 static const struct drm_driver mtk_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
 
-	.dumb_create = mtk_gem_dumb_create,
+	DRM_GEM_DMA_DRIVER_OPS,
 	DRM_FBDEV_DMA_DRIVER_OPS,
 
-	.gem_prime_import = mtk_gem_prime_import,
-	.gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
 	.fops = &mtk_drm_fops,
 
 	.name = DRIVER_NAME,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 675cdc90a440..1fc3df4b5485 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -54,7 +54,6 @@ struct mtk_mmsys_driver_data {
 
 struct mtk_drm_private {
 	struct drm_device *drm;
-	struct device *dma_dev;
 	bool mtk_drm_bound;
 	bool drm_master;
 	struct device *dev;
diff --git a/drivers/gpu/drm/mediatek/mtk_gem.c b/drivers/gpu/drm/mediatek/mtk_gem.c
deleted file mode 100644
index f059a1452220..000000000000
--- a/drivers/gpu/drm/mediatek/mtk_gem.c
+++ /dev/null
@@ -1,231 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2015 MediaTek Inc.
- * Copyright (c) 2025 Collabora Ltd.
- *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
- */
-
-#include <linux/dma-buf.h>
-#include <linux/vmalloc.h>
-
-#include <drm/drm.h>
-#include <drm/drm_device.h>
-#include <drm/drm_gem.h>
-#include <drm/drm_gem_dma_helper.h>
-#include <drm/drm_prime.h>
-#include <drm/drm_print.h>
-
-#include "mtk_drm_drv.h"
-#include "mtk_gem.h"
-
-static int mtk_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
-
-static void mtk_gem_free_object(struct drm_gem_object *obj)
-{
-	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(obj);
-	struct mtk_drm_private *priv = obj->dev->dev_private;
-
-	if (dma_obj->sgt)
-		drm_prime_gem_destroy(obj, dma_obj->sgt);
-	else
-		dma_free_wc(priv->dma_dev, dma_obj->base.size,
-			    dma_obj->vaddr, dma_obj->dma_addr);
-
-	/* release file pointer to gem object. */
-	drm_gem_object_release(obj);
-
-	kfree(dma_obj);
-}
-
-/*
- * Allocate a sg_table for this GEM object.
- * Note: Both the table's contents, and the sg_table itself must be freed by
- *       the caller.
- * Returns a pointer to the newly allocated sg_table, or an ERR_PTR() error.
- */
-static struct sg_table *mtk_gem_prime_get_sg_table(struct drm_gem_object *obj)
-{
-	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(obj);
-	struct mtk_drm_private *priv = obj->dev->dev_private;
-	struct sg_table *sgt;
-	int ret;
-
-	sgt = kzalloc_obj(*sgt);
-	if (!sgt)
-		return ERR_PTR(-ENOMEM);
-
-	ret = dma_get_sgtable(priv->dma_dev, sgt, dma_obj->vaddr,
-			      dma_obj->dma_addr, obj->size);
-	if (ret) {
-		DRM_ERROR("failed to allocate sgt, %d\n", ret);
-		kfree(sgt);
-		return ERR_PTR(ret);
-	}
-
-	return sgt;
-}
-
-static const struct drm_gem_object_funcs mtk_gem_object_funcs = {
-	.free = mtk_gem_free_object,
-	.print_info = drm_gem_dma_object_print_info,
-	.get_sg_table = mtk_gem_prime_get_sg_table,
-	.vmap = drm_gem_dma_object_vmap,
-	.mmap = mtk_gem_object_mmap,
-	.vm_ops = &drm_gem_dma_vm_ops,
-};
-
-static struct drm_gem_dma_object *mtk_gem_init(struct drm_device *dev,
-					unsigned long size, bool private)
-{
-	struct drm_gem_dma_object *dma_obj;
-	int ret;
-
-	size = round_up(size, PAGE_SIZE);
-
-	if (size == 0)
-		return ERR_PTR(-EINVAL);
-
-	dma_obj = kzalloc_obj(*dma_obj);
-	if (!dma_obj)
-		return ERR_PTR(-ENOMEM);
-
-	dma_obj->base.funcs = &mtk_gem_object_funcs;
-
-	if (private) {
-		ret = 0;
-		drm_gem_private_object_init(dev, &dma_obj->base, size);
-	} else {
-		ret = drm_gem_object_init(dev, &dma_obj->base, size);
-	}
-	if (ret) {
-		DRM_ERROR("failed to initialize gem object\n");
-		kfree(dma_obj);
-		return ERR_PTR(ret);
-	}
-
-	return dma_obj;
-}
-
-static struct drm_gem_dma_object *mtk_gem_create(struct drm_device *dev, size_t size)
-{
-	struct mtk_drm_private *priv = dev->dev_private;
-	struct drm_gem_dma_object *dma_obj;
-	struct drm_gem_object *obj;
-	int ret;
-
-	dma_obj = mtk_gem_init(dev, size, false);
-	if (IS_ERR(dma_obj))
-		return ERR_CAST(dma_obj);
-
-	obj = &dma_obj->base;
-
-	dma_obj->vaddr = dma_alloc_wc(priv->dma_dev, obj->size,
-				      &dma_obj->dma_addr,
-				      GFP_KERNEL | __GFP_NOWARN);
-	if (!dma_obj->vaddr) {
-		DRM_ERROR("failed to allocate %zx byte dma buffer", obj->size);
-		ret = -ENOMEM;
-		goto err_gem_free;
-	}
-
-	DRM_DEBUG_DRIVER("vaddr = %p dma_addr = %pad size = %zu\n",
-			 dma_obj->vaddr, &dma_obj->dma_addr,
-			 size);
-
-	return dma_obj;
-
-err_gem_free:
-	drm_gem_object_release(obj);
-	kfree(dma_obj);
-	return ERR_PTR(ret);
-}
-
-int mtk_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev,
-			struct drm_mode_create_dumb *args)
-{
-	struct drm_gem_dma_object *dma_obj;
-	int ret;
-
-	args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
-
-	/*
-	 * Multiply 2 variables of different types,
-	 * for example: args->size = args->spacing * args->height;
-	 * may cause coverity issue with unintentional overflow.
-	 */
-	args->size = args->pitch;
-	args->size *= args->height;
-
-	dma_obj = mtk_gem_create(dev, args->size);
-	if (IS_ERR(dma_obj))
-		return PTR_ERR(dma_obj);
-
-	/*
-	 * allocate a id of idr table where the obj is registered
-	 * and handle has the id what user can see.
-	 */
-	ret = drm_gem_handle_create(file_priv, &dma_obj->base, &args->handle);
-	if (ret)
-		goto err_handle_create;
-
-	/* drop reference from allocate - handle holds it now. */
-	drm_gem_object_put(&dma_obj->base);
-
-	return 0;
-
-err_handle_create:
-	mtk_gem_free_object(&dma_obj->base);
-	return ret;
-}
-
-static int mtk_gem_object_mmap(struct drm_gem_object *obj,
-			       struct vm_area_struct *vma)
-
-{
-	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(obj);
-	struct mtk_drm_private *priv = obj->dev->dev_private;
-	int ret;
-
-	/*
-	 * Set vm_pgoff (used as a fake buffer offset by DRM) to 0 and map the
-	 * whole buffer from the start.
-	 */
-	vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
-
-	/*
-	 * dma_alloc_attrs() allocated a struct page table for mtk_gem, so clear
-	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
-	 */
-	vm_flags_mod(vma, VM_IO | VM_DONTEXPAND | VM_DONTDUMP, VM_PFNMAP);
-
-	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
-	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-
-	ret = dma_mmap_wc(priv->dma_dev, vma, dma_obj->vaddr,
-			  dma_obj->dma_addr, obj->size);
-	if (ret)
-		drm_gem_vm_close(vma);
-
-	return ret;
-}
-
-struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,
-			struct dma_buf_attachment *attach, struct sg_table *sgt)
-{
-	struct drm_gem_dma_object *dma_obj;
-
-	/* check if the entries in the sg_table are contiguous */
-	if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) {
-		DRM_ERROR("sg_table is not contiguous");
-		return ERR_PTR(-EINVAL);
-	}
-
-	dma_obj = mtk_gem_init(dev, attach->dmabuf->size, true);
-	if (IS_ERR(dma_obj))
-		return ERR_CAST(dma_obj);
-
-	dma_obj->dma_addr = sg_dma_address(sgt->sgl);
-	dma_obj->sgt = sgt;
-
-	return &dma_obj->base;
-}
diff --git a/drivers/gpu/drm/mediatek/mtk_gem.h b/drivers/gpu/drm/mediatek/mtk_gem.h
deleted file mode 100644
index afebc3a970a8..000000000000
--- a/drivers/gpu/drm/mediatek/mtk_gem.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2015 MediaTek Inc.
- */
-
-#ifndef _MTK_GEM_H_
-#define _MTK_GEM_H_
-
-#include <drm/drm_gem.h>
-#include <drm/drm_gem_dma_helper.h>
-
-int mtk_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev,
-			struct drm_mode_create_dumb *args);
-struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,
-			struct dma_buf_attachment *attach, struct sg_table *sg);
-
-#endif
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH RESEND 4/4] drm/sun4i: Use backend/mixer as dedicated DMA device

[Chen-Yu Tsai]

The sun4i DRM driver deals with DMA constraints in a peculiar way.
Instead of using the actual DMA device in various helpers, it justs
reconfigures the DMA constraints of the virtual display device using
the DMA device's device tree node by calling of_dma_configure().

Turns out of_dma_configure() should only be called from bus code.
Lately this also triggers a big warning through of_iommu_configure()
and ultimately __iommu_probe_device():

    late IOMMU probe at driver bind, something fishy here!

Now that the GEM DMA helpers have proper support for allocating
and mapping buffers with a dedicated DMA device, switch over to
it as the proper solution.

The mixer change was tested on a Pine H64 model B. The backend change
was only compile tested. Though I don't expect any issues, help testing
on an older device would be appreciated.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 27 +++++++++++++++------------
 drivers/gpu/drm/sun4i/sun8i_mixer.c   | 27 +++++++++++++++------------
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 6391bdc94a5c..a57fb5151def 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -798,18 +798,21 @@ static int sun4i_backend_bind(struct device *dev, struct device *master,
 	dev_set_drvdata(dev, backend);
 	spin_lock_init(&backend->frontend_lock);
 
-	if (of_property_present(dev->of_node, "interconnects")) {
-		/*
-		 * This assume we have the same DMA constraints for all our the
-		 * devices in our pipeline (all the backends, but also the
-		 * frontends). This sounds bad, but it has always been the case
-		 * for us, and DRM doesn't do per-device allocation either, so
-		 * we would need to fix DRM first...
-		 */
-		ret = of_dma_configure(drm->dev, dev->of_node, true);
-		if (ret)
-			return ret;
-	}
+	/*
+	 * This assume we have the same DMA constraints for all our the
+	 * devices in our pipeline (all the backends, but also the
+	 * frontends). This sounds bad, but it has always been the case
+	 * for us, and DRM doesn't do per-device allocation either, so
+	 * we would need to fix DRM first...
+	 *
+	 * Always use the first bound backend as the DMA device. While
+	 * our device trees always have all backends enabled, some in
+	 * the wild may actually have the first one disabled. If both
+	 * are enabled, the order in which they are bound is guaranteed
+	 * since the driver adds components in order.
+	 */
+	if (drm_dev_dma_dev(drm) == drm->dev)
+		drm_dev_set_dma_dev(drm, dev);
 
 	backend->engine.node = dev->of_node;
 	backend->engine.ops = &sun4i_backend_engine_ops;
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 02acc7cbdb97..4071ab38b4ae 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -536,18 +536,21 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
 	mixer->engine.ops = &sun8i_engine_ops;
 	mixer->engine.node = dev->of_node;
 
-	if (of_property_present(dev->of_node, "iommus")) {
-		/*
-		 * This assume we have the same DMA constraints for
-		 * all our the mixers in our pipeline. This sounds
-		 * bad, but it has always been the case for us, and
-		 * DRM doesn't do per-device allocation either, so we
-		 * would need to fix DRM first...
-		 */
-		ret = of_dma_configure(drm->dev, dev->of_node, true);
-		if (ret)
-			return ret;
-	}
+	/*
+	 * This assume we have the same DMA constraints for all our the
+	 * devices in our pipeline (all the backends, but also the
+	 * frontends). This sounds bad, but it has always been the case
+	 * for us, and DRM doesn't do per-device allocation either, so
+	 * we would need to fix DRM first...
+	 *
+	 * Always use the first bound backend as the DMA device. While
+	 * our device trees always have all backends enabled, some in
+	 * the wild may actually have the first one disabled. If both
+	 * are enabled, the order in which they are bound is guaranteed
+	 * since the driver adds components in order.
+	 */
+	if (drm_dev_dma_dev(drm) == drm->dev)
+		drm_dev_set_dma_dev(drm, dev);
 
 	/*
 	 * While this function can fail, we shouldn't do anything
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH RESEND 0/4] drm/gem-dma: Support dedicated DMA device for allocation

[Thomas Zimmermann]

Hi

Am 10.03.26 um 04:25 schrieb Chen-Yu Tsai:
> (resent with Samuel's email address fixed)
>
> Hi folks,
>
> This series expands the "dedicated DMA device" support in DRM to the GEM
> DMA helpers, and converts the MediaTek DRM driver to setting the DMA
> device and dropping the custom GEM helpers that implemented this
> function.

I wanted to do this myself at some point. So thanks a lot for picking up 
the task. Happy to see this series.

>
> Various display drivers implement the "dedicated DMA device" support
> with custom GEM helpers. These include Exynos, MediaTek, and Rockchip
> to name a few. Allwinner does something entirely different, calling
> of_dma_configure() on the virtual display device using the OF node of
> the actual DMA device. Recently this causes a warning if IOMMUs are
> involved.
>
> This series intends to allow the core helpers to deal with it, and not
> have every driver implement it in slightly different ways, duplicating
> code.
>
> Patch 1 adds dedicated DMA device support to drm_prime_pages_to_sg().
> I believe this was missing from the original change that added dedicated
> DMA devices for PRIME.

We originally added the dedicated DMA device for supporting PRIME on USB 
graphics adapters, where the DMA buffer has to be allocated wrt to the 
USB controller. We left out anything that wasn't necessary.

>
> Patch 2 adds support for dedicated DMA device to the GEM DMA helpers
> for GEM buffer allocation and mmap.
>
> Patch 3 converts the MediaTek DRM driver to use the dedicated DMA device
> support, and drop all the remaining custom GEM callbacks that deal with
> it.
>
> Patch 4 converts the Allwinner sun4i DRM driver to use the dedicated DMA
> device support, instead of the of_dma_configure() hack it currently has.
>
> The series should be merged through drm-misc-next so that other drivers
> can take advantage of the change.

Yes.

Best regards
Thomas

>
> I also intend to try to convert the Exynos and Rockchip drivers, however
> both also have options to set DMA_ATTR_NO_KERNEL_MAPPING when using
> dma_alloc_attrs() to allocate memory for the buffers. I intend to
> resurrect the DRM_MODE_DUMB_KERNEL_MAP work from Rob Herring [1]
> to handle this. The Rockchip driver also has custom IOMMU attachment
> that I'm still trying to understand.
>
>
> Thanks
> ChenYu
>
>
> Chen-Yu Tsai (4):
>    drm/prime: Limit scatter list size with dedicated DMA device
>    drm/gem-dma: Support dedicated DMA device for allocation and mapping
>    drm/mediatek: Set dedicated DMA device and drop custom GEM callbacks
>    drm/sun4i: Use backend/mixer as dedicated DMA device
>
>   drivers/gpu/drm/drm_gem_dma_helper.c   |  21 ++-
>   drivers/gpu/drm/drm_prime.c            |   2 +-
>   drivers/gpu/drm/mediatek/mtk_crtc.c    |   1 -
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c |  21 +--
>   drivers/gpu/drm/mediatek/mtk_drm_drv.h |   1 -
>   drivers/gpu/drm/mediatek/mtk_gem.c     | 231 -------------------------
>   drivers/gpu/drm/mediatek/mtk_gem.h     |  17 --
>   drivers/gpu/drm/sun4i/sun4i_backend.c  |  27 +--
>   drivers/gpu/drm/sun4i/sun8i_mixer.c    |  27 +--
>   9 files changed, 46 insertions(+), 302 deletions(-)
>   delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.c
>   delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.h
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH RESEND 1/4] drm/prime: Limit scatter list size with dedicated DMA device

[Thomas Zimmermann]

Am 10.03.26 um 04:25 schrieb Chen-Yu Tsai:
> If a dedicated DMA device is specified for the DRM device, then the
> scatter list size limit should pertain to the DMA device.
>
> Use the dedicated DMA device, if given, to limit the scatter list size.
> This only applies to drivers that have called drm_dev_set_dma_dev() and
> are using drm_prime_pages_to_sg() either directly or through the SHMEM
> helpers. At the time of this writing, the former case only includes the
> Rockchip DRM driver, while the latter case includes the gud, udl, and
> the tiny appletbdrm and gm12u320 drivers.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/drm_prime.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 51fdb06d3e9f..9b44c78cd77f 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -859,7 +859,7 @@ struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
>   		return ERR_PTR(-ENOMEM);
>   
>   	if (dev)
> -		max_segment = dma_max_mapping_size(dev->dev);
> +		max_segment = dma_max_mapping_size(drm_dev_dma_dev(dev));
>   	if (max_segment == 0)
>   		max_segment = UINT_MAX;
>   	err = sg_alloc_table_from_pages_segment(sg, pages, nr_pages, 0,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH RESEND 2/4] drm/gem-dma: Support dedicated DMA device for allocation and mapping

[Thomas Zimmermann]

Am 10.03.26 um 04:25 schrieb Chen-Yu Tsai:
> Support for a dedicated DMA device for prime imports was added in commit
> 143ec8d3f939 ("drm/prime: Support dedicated DMA device for dma-buf imports").
> This allowed the DRM driver to provide a dedicated DMA device when its
> own underlying device was not capable of DMA, for example when it is a
> USB device (the original target) or a virtual device. The latter case is
> common on embedded SoCs, on which the display pipeline is composed of
> various fixed function blocks, and the DRM device is simply a made-up
> device, an address space managing the routing between the blocks, or
> whichever block the implementor thought made sense at the time. The
> point is that the chosen device is often not the actual device doing
> the DMA. Various drivers have used workarounds or reimplemented the
> GEM DMA helpers to get the DMA addresses and IOMMUs to work correctly.
>
> Add support for the dedicated DMA device to the GEM DMA helpers.
>
> No existing driver currently uses the GEM DMA helpers and calls
> drm_dev_set_dma_dev() to set a dedicated DMA device, so no existing
> users should be affected.

In these patches (or at least the series' cover letter) you should 
mention that drm_dev_dma_dev() returns the regular device if no DMA 
device has been set.

>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/drm_gem_dma_helper.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c
> index ecb9746f4da8..70f83e464476 100644
> --- a/drivers/gpu/drm/drm_gem_dma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_dma_helper.c
> @@ -146,12 +146,13 @@ struct drm_gem_dma_object *drm_gem_dma_create(struct drm_device *drm,
>   		return dma_obj;
>   
>   	if (dma_obj->map_noncoherent) {
> -		dma_obj->vaddr = dma_alloc_noncoherent(drm->dev, size,
> +		dma_obj->vaddr = dma_alloc_noncoherent(drm_dev_dma_dev(drm),
> +						       size,
>   						       &dma_obj->dma_addr,
>   						       DMA_TO_DEVICE,
>   						       GFP_KERNEL | __GFP_NOWARN);
>   	} else {
> -		dma_obj->vaddr = dma_alloc_wc(drm->dev, size,
> +		dma_obj->vaddr = dma_alloc_wc(drm_dev_dma_dev(drm), size,
>   					      &dma_obj->dma_addr,
>   					      GFP_KERNEL | __GFP_NOWARN);
>   	}
> @@ -236,12 +237,14 @@ void drm_gem_dma_free(struct drm_gem_dma_object *dma_obj)
>   		drm_prime_gem_destroy(gem_obj, dma_obj->sgt);
>   	} else if (dma_obj->vaddr) {
>   		if (dma_obj->map_noncoherent)
> -			dma_free_noncoherent(gem_obj->dev->dev, dma_obj->base.size,
> +			dma_free_noncoherent(drm_dev_dma_dev(gem_obj->dev),
> +					     dma_obj->base.size,
>   					     dma_obj->vaddr, dma_obj->dma_addr,
>   					     DMA_TO_DEVICE);
>   		else
> -			dma_free_wc(gem_obj->dev->dev, dma_obj->base.size,
> -				    dma_obj->vaddr, dma_obj->dma_addr);
> +			dma_free_wc(drm_dev_dma_dev(gem_obj->dev),
> +				    dma_obj->base.size, dma_obj->vaddr,
> +				    dma_obj->dma_addr);
>   	}
>   
>   	drm_gem_object_release(gem_obj);
> @@ -432,7 +435,7 @@ struct sg_table *drm_gem_dma_get_sg_table(struct drm_gem_dma_object *dma_obj)
>   	if (!sgt)
>   		return ERR_PTR(-ENOMEM);
>   
> -	ret = dma_get_sgtable(obj->dev->dev, sgt, dma_obj->vaddr,
> +	ret = dma_get_sgtable(drm_dev_dma_dev(obj->dev), sgt, dma_obj->vaddr,
>   			      dma_obj->dma_addr, obj->size);
>   	if (ret < 0)
>   		goto out;
> @@ -539,12 +542,12 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *
>   	if (dma_obj->map_noncoherent) {
>   		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>   
> -		ret = dma_mmap_pages(dma_obj->base.dev->dev,
> +		ret = dma_mmap_pages(drm_dev_dma_dev(dma_obj->base.dev),
>   				     vma, vma->vm_end - vma->vm_start,
>   				     virt_to_page(dma_obj->vaddr));
>   	} else {
> -		ret = dma_mmap_wc(dma_obj->base.dev->dev, vma, dma_obj->vaddr,
> -				  dma_obj->dma_addr,
> +		ret = dma_mmap_wc(drm_dev_dma_dev(dma_obj->base.dev), vma,
> +				  dma_obj->vaddr, dma_obj->dma_addr,
>   				  vma->vm_end - vma->vm_start);
>   	}
>   	if (ret)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH RESEND 3/4] drm/mediatek: Set dedicated DMA device and drop custom GEM callbacks

[Thomas Zimmermann]

Hi

Am 10.03.26 um 04:25 schrieb Chen-Yu Tsai:
> In commit 9b54a32c7c6a ("drm/mediatek: mtk_gem: Partial refactor and
> use drm_gem_dma_object") the MediaTek DRM driver was refactored to use
> drm_gem_dma_object, but custom callbacks were still needed to deal with
> using the first device of the pipeline as the DMA device, instead of
> the MMSYS device that the DRM driver binds to.
>
> Turns out there is already partial support for dedicated DMA devices in
> the DRM subsystem for PRIME imports. The preceding patches add support
> for dedicated DMA devices to the GEM DMA helpers.
>
> This allows us to just set the dedicated DMA device for the DRM device,
> and drop all the custom GEM callbacks. Also drop the .dma_dev field
> from the driver private data as it is no longer needed.

My impression is that this patch should be split up: first replace 
dma_dev with the dma device; then replace the now-duplicate code with 
shared helpers.

>
> There are slight differences in the mmap helper: the VM_DONTDUMP and
> VM_IO flags are no longer set. Both were lifted from drm_gem_mmap_obj().
> VM_IO probably doesn't make sense since the buffer is allocated using
> dma_alloc_attrs().

We can add VM_DONTDUMP to gem-dma's mmap helper. [1] It's most likely 
the right thing to not dump graphics buffers and standard practice in 
most other GEM memory managers.

[1] 
https://elixir.bootlin.com/linux/v6.19.6/source/drivers/gpu/drm/drm_gem_dma_helper.c#L537

>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

The changes look correct, so I'm here's the

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

But this patch requires a review from the driver maintainer as well.

Best regards
Thomas

> ---
>   drivers/gpu/drm/mediatek/mtk_crtc.c    |   1 -
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c |  21 +--
>   drivers/gpu/drm/mediatek/mtk_drm_drv.h |   1 -
>   drivers/gpu/drm/mediatek/mtk_gem.c     | 231 -------------------------
>   drivers/gpu/drm/mediatek/mtk_gem.h     |  17 --
>   5 files changed, 3 insertions(+), 268 deletions(-)
>   delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.c
>   delete mode 100644 drivers/gpu/drm/mediatek/mtk_gem.h
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index 351d58c50b84..fcb16f3f7b23 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> @@ -23,7 +23,6 @@
>   #include "mtk_crtc.h"
>   #include "mtk_ddp_comp.h"
>   #include "mtk_drm_drv.h"
> -#include "mtk_gem.h"
>   #include "mtk_plane.h"
>   
>   /*
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index a94c51a83261..6f6db2e1980e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -19,6 +19,7 @@
>   #include <drm/drm_fbdev_dma.h>
>   #include <drm/drm_fourcc.h>
>   #include <drm/drm_gem.h>
> +#include <drm/drm_gem_dma_helper.h>
>   #include <drm/drm_gem_framebuffer_helper.h>
>   #include <drm/drm_ioctl.h>
>   #include <drm/drm_of.h>
> @@ -29,7 +30,6 @@
>   #include "mtk_ddp_comp.h"
>   #include "mtk_disp_drv.h"
>   #include "mtk_drm_drv.h"
> -#include "mtk_gem.h"
>   
>   #define DRIVER_NAME "mediatek"
>   #define DRIVER_DESC "Mediatek SoC DRM"
> @@ -565,8 +565,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>   		goto err_component_unbind;
>   	}
>   
> -	for (i = 0; i < private->data->mmsys_dev_num; i++)
> -		private->all_drm_private[i]->dma_dev = dma_dev;
> +	drm_dev_set_dma_dev(drm, dma_dev);
>   
>   	/*
>   	 * Configure the DMA segment size to make sure we get contiguous IOVA
> @@ -600,26 +599,12 @@ static void mtk_drm_kms_deinit(struct drm_device *drm)
>   
>   DEFINE_DRM_GEM_FOPS(mtk_drm_fops);
>   
> -/*
> - * We need to override this because the device used to import the memory is
> - * not dev->dev, as drm_gem_prime_import() expects.
> - */
> -static struct drm_gem_object *mtk_gem_prime_import(struct drm_device *dev,
> -						   struct dma_buf *dma_buf)
> -{
> -	struct mtk_drm_private *private = dev->dev_private;
> -
> -	return drm_gem_prime_import_dev(dev, dma_buf, private->dma_dev);
> -}
> -
>   static const struct drm_driver mtk_drm_driver = {
>   	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
>   
> -	.dumb_create = mtk_gem_dumb_create,
> +	DRM_GEM_DMA_DRIVER_OPS,
>   	DRM_FBDEV_DMA_DRIVER_OPS,
>   
> -	.gem_prime_import = mtk_gem_prime_import,
> -	.gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
>   	.fops = &mtk_drm_fops,
>   
>   	.name = DRIVER_NAME,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 675cdc90a440..1fc3df4b5485 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -54,7 +54,6 @@ struct mtk_mmsys_driver_data {
>   
>   struct mtk_drm_private {
>   	struct drm_device *drm;
> -	struct device *dma_dev;
>   	bool mtk_drm_bound;
>   	bool drm_master;
>   	struct device *dev;
> diff --git a/drivers/gpu/drm/mediatek/mtk_gem.c b/drivers/gpu/drm/mediatek/mtk_gem.c
> deleted file mode 100644
> index f059a1452220..000000000000
> --- a/drivers/gpu/drm/mediatek/mtk_gem.c
> +++ /dev/null
> @@ -1,231 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (c) 2015 MediaTek Inc.
> - * Copyright (c) 2025 Collabora Ltd.
> - *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> - */
> -
> -#include <linux/dma-buf.h>
> -#include <linux/vmalloc.h>
> -
> -#include <drm/drm.h>
> -#include <drm/drm_device.h>
> -#include <drm/drm_gem.h>
> -#include <drm/drm_gem_dma_helper.h>
> -#include <drm/drm_prime.h>
> -#include <drm/drm_print.h>
> -
> -#include "mtk_drm_drv.h"
> -#include "mtk_gem.h"
> -
> -static int mtk_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
> -
> -static void mtk_gem_free_object(struct drm_gem_object *obj)
> -{
> -	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(obj);
> -	struct mtk_drm_private *priv = obj->dev->dev_private;
> -
> -	if (dma_obj->sgt)
> -		drm_prime_gem_destroy(obj, dma_obj->sgt);
> -	else
> -		dma_free_wc(priv->dma_dev, dma_obj->base.size,
> -			    dma_obj->vaddr, dma_obj->dma_addr);
> -
> -	/* release file pointer to gem object. */
> -	drm_gem_object_release(obj);
> -
> -	kfree(dma_obj);
> -}
> -
> -/*
> - * Allocate a sg_table for this GEM object.
> - * Note: Both the table's contents, and the sg_table itself must be freed by
> - *       the caller.
> - * Returns a pointer to the newly allocated sg_table, or an ERR_PTR() error.
> - */
> -static struct sg_table *mtk_gem_prime_get_sg_table(struct drm_gem_object *obj)
> -{
> -	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(obj);
> -	struct mtk_drm_private *priv = obj->dev->dev_private;
> -	struct sg_table *sgt;
> -	int ret;
> -
> -	sgt = kzalloc_obj(*sgt);
> -	if (!sgt)
> -		return ERR_PTR(-ENOMEM);
> -
> -	ret = dma_get_sgtable(priv->dma_dev, sgt, dma_obj->vaddr,
> -			      dma_obj->dma_addr, obj->size);
> -	if (ret) {
> -		DRM_ERROR("failed to allocate sgt, %d\n", ret);
> -		kfree(sgt);
> -		return ERR_PTR(ret);
> -	}
> -
> -	return sgt;
> -}
> -
> -static const struct drm_gem_object_funcs mtk_gem_object_funcs = {
> -	.free = mtk_gem_free_object,
> -	.print_info = drm_gem_dma_object_print_info,
> -	.get_sg_table = mtk_gem_prime_get_sg_table,
> -	.vmap = drm_gem_dma_object_vmap,
> -	.mmap = mtk_gem_object_mmap,
> -	.vm_ops = &drm_gem_dma_vm_ops,
> -};
> -
> -static struct drm_gem_dma_object *mtk_gem_init(struct drm_device *dev,
> -					unsigned long size, bool private)
> -{
> -	struct drm_gem_dma_object *dma_obj;
> -	int ret;
> -
> -	size = round_up(size, PAGE_SIZE);
> -
> -	if (size == 0)
> -		return ERR_PTR(-EINVAL);
> -
> -	dma_obj = kzalloc_obj(*dma_obj);
> -	if (!dma_obj)
> -		return ERR_PTR(-ENOMEM);
> -
> -	dma_obj->base.funcs = &mtk_gem_object_funcs;
> -
> -	if (private) {
> -		ret = 0;
> -		drm_gem_private_object_init(dev, &dma_obj->base, size);
> -	} else {
> -		ret = drm_gem_object_init(dev, &dma_obj->base, size);
> -	}
> -	if (ret) {
> -		DRM_ERROR("failed to initialize gem object\n");
> -		kfree(dma_obj);
> -		return ERR_PTR(ret);
> -	}
> -
> -	return dma_obj;
> -}
> -
> -static struct drm_gem_dma_object *mtk_gem_create(struct drm_device *dev, size_t size)
> -{
> -	struct mtk_drm_private *priv = dev->dev_private;
> -	struct drm_gem_dma_object *dma_obj;
> -	struct drm_gem_object *obj;
> -	int ret;
> -
> -	dma_obj = mtk_gem_init(dev, size, false);
> -	if (IS_ERR(dma_obj))
> -		return ERR_CAST(dma_obj);
> -
> -	obj = &dma_obj->base;
> -
> -	dma_obj->vaddr = dma_alloc_wc(priv->dma_dev, obj->size,
> -				      &dma_obj->dma_addr,
> -				      GFP_KERNEL | __GFP_NOWARN);
> -	if (!dma_obj->vaddr) {
> -		DRM_ERROR("failed to allocate %zx byte dma buffer", obj->size);
> -		ret = -ENOMEM;
> -		goto err_gem_free;
> -	}
> -
> -	DRM_DEBUG_DRIVER("vaddr = %p dma_addr = %pad size = %zu\n",
> -			 dma_obj->vaddr, &dma_obj->dma_addr,
> -			 size);
> -
> -	return dma_obj;
> -
> -err_gem_free:
> -	drm_gem_object_release(obj);
> -	kfree(dma_obj);
> -	return ERR_PTR(ret);
> -}
> -
> -int mtk_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev,
> -			struct drm_mode_create_dumb *args)
> -{
> -	struct drm_gem_dma_object *dma_obj;
> -	int ret;
> -
> -	args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> -
> -	/*
> -	 * Multiply 2 variables of different types,
> -	 * for example: args->size = args->spacing * args->height;
> -	 * may cause coverity issue with unintentional overflow.
> -	 */
> -	args->size = args->pitch;
> -	args->size *= args->height;
> -
> -	dma_obj = mtk_gem_create(dev, args->size);
> -	if (IS_ERR(dma_obj))
> -		return PTR_ERR(dma_obj);
> -
> -	/*
> -	 * allocate a id of idr table where the obj is registered
> -	 * and handle has the id what user can see.
> -	 */
> -	ret = drm_gem_handle_create(file_priv, &dma_obj->base, &args->handle);
> -	if (ret)
> -		goto err_handle_create;
> -
> -	/* drop reference from allocate - handle holds it now. */
> -	drm_gem_object_put(&dma_obj->base);
> -
> -	return 0;
> -
> -err_handle_create:
> -	mtk_gem_free_object(&dma_obj->base);
> -	return ret;
> -}
> -
> -static int mtk_gem_object_mmap(struct drm_gem_object *obj,
> -			       struct vm_area_struct *vma)
> -
> -{
> -	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(obj);
> -	struct mtk_drm_private *priv = obj->dev->dev_private;
> -	int ret;
> -
> -	/*
> -	 * Set vm_pgoff (used as a fake buffer offset by DRM) to 0 and map the
> -	 * whole buffer from the start.
> -	 */
> -	vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
> -
> -	/*
> -	 * dma_alloc_attrs() allocated a struct page table for mtk_gem, so clear
> -	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
> -	 */
> -	vm_flags_mod(vma, VM_IO | VM_DONTEXPAND | VM_DONTDUMP, VM_PFNMAP);
> -
> -	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> -	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> -
> -	ret = dma_mmap_wc(priv->dma_dev, vma, dma_obj->vaddr,
> -			  dma_obj->dma_addr, obj->size);
> -	if (ret)
> -		drm_gem_vm_close(vma);
> -
> -	return ret;
> -}
> -
> -struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,
> -			struct dma_buf_attachment *attach, struct sg_table *sgt)
> -{
> -	struct drm_gem_dma_object *dma_obj;
> -
> -	/* check if the entries in the sg_table are contiguous */
> -	if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) {
> -		DRM_ERROR("sg_table is not contiguous");
> -		return ERR_PTR(-EINVAL);
> -	}
> -
> -	dma_obj = mtk_gem_init(dev, attach->dmabuf->size, true);
> -	if (IS_ERR(dma_obj))
> -		return ERR_CAST(dma_obj);
> -
> -	dma_obj->dma_addr = sg_dma_address(sgt->sgl);
> -	dma_obj->sgt = sgt;
> -
> -	return &dma_obj->base;
> -}
> diff --git a/drivers/gpu/drm/mediatek/mtk_gem.h b/drivers/gpu/drm/mediatek/mtk_gem.h
> deleted file mode 100644
> index afebc3a970a8..000000000000
> --- a/drivers/gpu/drm/mediatek/mtk_gem.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Copyright (c) 2015 MediaTek Inc.
> - */
> -
> -#ifndef _MTK_GEM_H_
> -#define _MTK_GEM_H_
> -
> -#include <drm/drm_gem.h>
> -#include <drm/drm_gem_dma_helper.h>
> -
> -int mtk_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev,
> -			struct drm_mode_create_dumb *args);
> -struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,
> -			struct dma_buf_attachment *attach, struct sg_table *sg);
> -
> -#endif

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Claude review: drm/gem-dma: Support dedicated DMA device for allocation

[Claude Code Review Bot]

Overall Series Review

Subject: drm/gem-dma: Support dedicated DMA device for allocation
Author: Chen-Yu Tsai <wenst@chromium.org>
Patches: 9
Reviewed: 2026-03-11T13:45:57.938628

---

This is a well-structured and clearly motivated series by Chen-Yu Tsai that extends the existing `drm_dev_set_dma_dev()` / `drm_dev_dma_dev()` infrastructure to cover GEM DMA allocation and mapping, then converts two drivers (MediaTek and sun4i) to use it instead of ad-hoc workarounds. The approach is sound: consolidating "dedicated DMA device" handling in the core helpers avoids code duplication across drivers and eliminates the problematic `of_dma_configure()` hack in sun4i.

The series is logically ordered, well-explained, and the net diffstat is very positive (deleting ~250 lines of custom code). The core changes in patches 1-2 are safe for existing users since no current driver both uses GEM DMA helpers and calls `drm_dev_set_dma_dev()`.

A few minor observations below, but overall this looks good and ready to merge through drm-misc-next.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/prime: Limit scatter list size with dedicated DMA device

[Claude Code Review Bot]

Patch Review

A straightforward one-liner that changes `dev->dev` to `drm_dev_dma_dev(dev)` in `drm_prime_pages_to_sg()`:

```c
-		max_segment = dma_max_mapping_size(dev->dev);
+		max_segment = dma_max_mapping_size(drm_dev_dma_dev(dev));
```

This is correct. When a dedicated DMA device is set, the DMA mapping size limit should come from that device, not the parent device (which may not even be DMA-capable). Since `drm_dev_dma_dev()` falls back to `dev->dev` when no dedicated device is set, this is safe for all existing callers.

No issues with this patch.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/gem-dma: Support dedicated DMA device for allocation and mapping

[Claude Code Review Bot]

Patch Review

This patch systematically replaces `drm->dev` / `gem_obj->dev->dev` with `drm_dev_dma_dev(drm)` / `drm_dev_dma_dev(gem_obj->dev)` in five locations across `drm_gem_dma_helper.c`:

1. `dma_alloc_noncoherent()` in `drm_gem_dma_create()`
2. `dma_alloc_wc()` in `drm_gem_dma_create()`
3. `dma_free_noncoherent()` in `drm_gem_dma_free()`
4. `dma_free_wc()` in `drm_gem_dma_free()`
5. `dma_get_sgtable()` in `drm_gem_dma_get_sg_table()`
6. `dma_mmap_pages()` in `drm_gem_dma_mmap()`
7. `dma_mmap_wc()` in `drm_gem_dma_mmap()`

All changes are consistent and correct. The commit message correctly notes that no existing driver currently combines GEM DMA helpers with `drm_dev_set_dma_dev()`, so existing users are unaffected.

No issues with this patch.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/mediatek: Set dedicated DMA device and drop custom GEM callbacks

[Claude Code Review Bot]

Patch Review

This is the main payoff patch, deleting the entire `mtk_gem.c` (231 lines) and `mtk_gem.h` (17 lines) files. The key changes:

1. Replaces the per-private-data `dma_dev` loop with a single `drm_dev_set_dma_dev(drm, dma_dev)` call:
```c
-	for (i = 0; i < private->data->mmsys_dev_num; i++)
-		private->all_drm_private[i]->dma_dev = dma_dev;
+	drm_dev_set_dma_dev(drm, dma_dev);
```

2. Replaces custom driver ops with `DRM_GEM_DMA_DRIVER_OPS`:
```c
-	.dumb_create = mtk_gem_dumb_create,
+	DRM_GEM_DMA_DRIVER_OPS,
 	DRM_FBDEV_DMA_DRIVER_OPS,
-
-	.gem_prime_import = mtk_gem_prime_import,
-	.gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
```

**Minor observation**: The old `mtk_gem_dumb_create` had a manual pitch calculation with `DIV_ROUND_UP(args->width * args->bpp, 8)` and explicit overflow-safe size computation (`args->size = args->pitch; args->size *= args->height`). The new `drm_gem_dma_dumb_create` uses `drm_mode_size_dumb()` which should handle this equivalently. This is fine.

**Minor observation**: As the commit message notes, the old mmap helper set `VM_IO | VM_DONTDUMP` flags that the standard GEM DMA mmap helper does not set. The commit message correctly argues that `VM_IO` doesn't make sense for `dma_alloc_attrs()` buffers. `VM_DONTDUMP` absence means these mappings will now appear in core dumps, which is a very minor behavioral change but acceptable.

No blocking issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/sun4i: Use backend/mixer as dedicated DMA device

[Claude Code Review Bot]

Patch Review

This patch replaces the `of_dma_configure()` hack with proper `drm_dev_set_dma_dev()` calls in both `sun4i_backend.c` and `sun8i_mixer.c`. The pattern is identical in both files:

```c
-	if (of_property_present(dev->of_node, "interconnects")) {
-		ret = of_dma_configure(drm->dev, dev->of_node, true);
-		if (ret)
-			return ret;
-	}
+	if (drm_dev_dma_dev(drm) == drm->dev)
+		drm_dev_set_dma_dev(drm, dev);
```

**Behavioral change worth noting**: The old code was conditional on the DT property (`"interconnects"` for backend, `"iommus"` for mixer). The new code unconditionally sets the DMA device (on first bind). This means even devices without IOMMU or interconnect properties will now use the backend/mixer as the DMA device instead of the virtual display device. This should be fine since the backend/mixer is the actual device doing DMA regardless of whether an IOMMU is present, but it is a subtle change from the original behavior.

**Comment nit**: The comment in the mixer says "backends" but should say "mixers":
```c
+	 * Always use the first bound backend as the DMA device. While
```
The backend comment is correct for `sun4i_backend.c`, but this same text was copied into `sun8i_mixer.c` where it should say "mixer" instead of "backend". This is a minor copy-paste issue in the comments.

**Unbind consideration**: Neither the backend nor the mixer `unbind` function resets the DMA device. If the component providing the DMA device is unbound while the DRM device persists, `drm_device.dma_dev` would point to a potentially freed device. However, looking at the existing code, the `of_dma_configure` approach had a similar issue (the DRM device would retain DMA configuration from a now-unbound component), and component unbind typically tears down the entire DRM device anyway. This is not a regression introduced by this patch.

No blocking issues, but the comment in `sun8i_mixer.c` should be fixed to say "mixer" instead of "backend".

---
Generated by Claude Code Patch Reviewer

Claude review: drm/gem-dma: Support dedicated DMA device for allocation

[Claude Code Review Bot]

Overall Series Review

Subject: drm/gem-dma: Support dedicated DMA device for allocation
Author: Chen-Yu Tsai <wenst@chromium.org>
Patches: 5
Reviewed: 2026-03-11T13:48:35.189516

---

This is a well-structured 4-patch series that extends the existing `drm_dev_dma_dev()` / `drm_dev_set_dma_dev()` infrastructure to the GEM DMA helpers for allocation and mmap operations. The motivation is clear: several embedded SoC display drivers (MediaTek, Allwinner, Exynos, Rockchip) have reinvented this wheel with custom GEM callbacks, and this series aims to consolidate that into the core helpers. The approach is sound and the code is straightforward. The MediaTek conversion is a satisfying deletion of ~250 lines of custom code. The sun4i conversion properly fixes a misuse of `of_dma_configure()` that triggers warnings with IOMMUs.

Overall the series looks good. I have a few observations below, mostly minor.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/gem-dma: Support dedicated DMA device for allocation

[Claude Code Review Bot]

Overall Series Review

Subject: drm/gem-dma: Support dedicated DMA device for allocation
Author: Chen-Yu Tsai <wenst@chromium.org>
Patches: 8
Reviewed: 2026-03-12T07:08:42.098677

---

This is a well-structured 4-patch series that extends the existing `drm_dev_set_dma_dev()` / `drm_dev_dma_dev()` infrastructure to the GEM DMA helpers, then converts two drivers (MediaTek and sun4i) to use it instead of custom workarounds. The approach is sound — centralizing dedicated DMA device handling in the core helpers rather than having each driver reimplement it. The patches are logically ordered with infrastructure changes first, followed by driver conversions.

The series is clean and the core patches (1 and 2) have already received Thomas Zimmermann's Reviewed-by. There are a few minor observations below, but nothing that blocks the series.

---
Generated by Claude Code Patch Reviewer