All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
@ 2025-03-07  8:03 Thomas Zimmermann
  2025-03-07  8:03 ` [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports Thomas Zimmermann
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-07  8:03 UTC (permalink / raw)
  To: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula
  Cc: dri-devel, Thomas Zimmermann

Importing dma-bufs via PRIME requires a DMA-capable hardware device.
This is not the case for USB, where DMA is performed entirely by the
USB controller instead of the USB devices.

Drivers for USB-based hardware maintain their own workarounds for this
problem. The original idea to resolve this was to provide different
PRIME helpers for such devices, but the dma-buf code internally assumes
DMA functionality as well. So that ideas is not realistic.

Let's instead turn the current workaround into a feature. Patch 1 adds a
dma_dev field to struct drm_device and makes the PRIME code use it. Patches
2 to 5 replace related driver code.

It will also be useful in other code. The exynos and mediatek drivers
already maintain a dedicated DMA device for non-PRIME code. They could
likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
memory with the regular parent device. They should support the dma_dev
settings as well.

Tested with udl.

v2:
- maintain reference on dma_dev (Jani)
- improve docs (Maxime)
- update appletbdrm

Thomas Zimmermann (5):
  drm/prime: Support dedicated DMA device for dma-buf imports
  drm/appletbdrm: Set struct drm_device.dma_dev
  drm/gm12u320: Set struct drm_device.dma_dev
  drm/gud: Set struct drm_device.dma_dev
  drm/udl: Set struct drm_device.dma_dev

 drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
 drivers/gpu/drm/drm_prime.c        |  2 +-
 drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
 drivers/gpu/drm/gud/gud_internal.h |  1 -
 drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
 drivers/gpu/drm/tiny/gm12u320.c    | 46 +++++++++---------------------
 drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
 drivers/gpu/drm/udl/udl_drv.h      |  1 -
 drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
 include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
 10 files changed, 102 insertions(+), 101 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
@ 2025-03-07  8:03 ` Thomas Zimmermann
  2025-03-07 12:45   ` Jani Nikula
  2025-03-07 13:03   ` Maxime Ripard
  2025-03-07  8:04 ` [PATCH v2 2/5] drm/appletbdrm: Set struct drm_device.dma_dev Thomas Zimmermann
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-07  8:03 UTC (permalink / raw)
  To: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula
  Cc: dri-devel, Thomas Zimmermann

Importing dma-bufs via PRIME requires a DMA-capable device. Devices on
peripheral busses, such as USB, often cannot perform DMA by themselves.
Without DMA-capable device PRIME import fails. DRM drivers for USB
devices already use a separate DMA device for dma-buf imports. Make the
mechanism generally available.

Besides the case of USB, there are embedded DRM devices without DMA
capability. DMA is performed by a separate controller. DRM drivers should
set this accordingly.

Add the field dma_dev to struct drm_device to refer to the device's DMA
device. For USB this should be the USB controller. Use dma_dev in the
PRIME import helpers, if set.

v2:
- acquire internal reference on dma_dev (Jani)
- add DMA-controller usecase to docs (Maxime)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_drv.c   | 21 +++++++++++++++++++
 drivers/gpu/drm/drm_prime.c |  2 +-
 include/drm/drm_device.h    | 41 +++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 17fc5dc708f4..c9487bc88624 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -500,6 +500,25 @@ void drm_dev_unplug(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_dev_unplug);
 
+/**
+ * drm_dev_set_dma_dev - set the DMA device for a DRM device
+ * @dev: DRM device
+ * @dma_dev: DMA device or NULL
+ *
+ * Sets the DMA device of the given DRM device. Only required if
+ * the DMA device is different from the DRM device's parent. After
+ * calling this function, the DRM device holds a reference on
+ * @dma_dev. Pass NULL to clear the DMA device.
+ */
+void drm_dev_set_dma_dev(struct drm_device *dev, struct device *dma_dev)
+{
+	dma_dev = get_device(dma_dev);
+
+	put_device(dev->dma_dev);
+	dev->dma_dev = dma_dev;
+}
+EXPORT_SYMBOL(drm_dev_set_dma_dev);
+
 /*
  * Available recovery methods for wedged device. To be sent along with device
  * wedged uevent.
@@ -654,6 +673,8 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
 {
 	drm_fs_inode_free(dev->anon_inode);
 
+	put_device(dev->dma_dev);
+	dev->dma_dev = NULL;
 	put_device(dev->dev);
 	/* Prevent use-after-free in drm_managed_release when debugging is
 	 * enabled. Slightly awkward, but can't really be helped. */
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index bdb51c8f262e..ed4e5f06b79d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -998,7 +998,7 @@ 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);
+	return drm_gem_prime_import_dev(dev, dma_buf, drm_dev_dma_dev(dev));
 }
 EXPORT_SYMBOL(drm_gem_prime_import);
 
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 6ea54a578cda..e2f894f1b90a 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -64,6 +64,28 @@ struct drm_device {
 	/** @dev: Device structure of bus-device */
 	struct device *dev;
 
+	/**
+	 * @dma_dev:
+	 *
+	 * Device for DMA operations. Only required if the device @dev
+	 * cannot perform DMA by itself. Should be NULL otherwise. Call
+	 * drm_dev_dma_dev() to get the DMA device instead of using this
+	 * field directly. Call drm_dev_set_dma_dev() to set this field.
+	 *
+	 * DRM devices are sometimes bound to virtual devices that cannot
+	 * perform DMA by themselves. Drivers should set this field to the
+	 * respective DMA controller.
+	 *
+	 * Devices on USB and other peripheral busses also cannot perform
+	 * DMA by themselves. The @dma_dev field should point the bus
+	 * controller that does DMA on behalve of such a device. Required
+	 * for importing buffers via dma-buf.
+	 *
+	 * If set, the DRM core automatically releases the reference on the
+	 * device.
+	 */
+	struct device *dma_dev;
+
 	/**
 	 * @managed:
 	 *
@@ -327,4 +349,23 @@ struct drm_device {
 	struct dentry *debugfs_root;
 };
 
+void drm_dev_set_dma_dev(struct drm_device *dev, struct device *dma_dev);
+
+/**
+ * drm_dev_dma_dev - returns the DMA device for a DRM device
+ * @dev: DRM device
+ *
+ * Returns the DMA device of the given DRM device. By default, this
+ * the DRM device's parent. See drm_dev_set_dma_dev().
+ *
+ * Returns:
+ * A DMA-capable device for the DRM device.
+ */
+static inline struct device *drm_dev_dma_dev(struct drm_device *dev)
+{
+	if (dev->dma_dev)
+		return dev->dma_dev;
+	return dev->dev;
+}
+
 #endif
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] drm/appletbdrm: Set struct drm_device.dma_dev
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
  2025-03-07  8:03 ` [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports Thomas Zimmermann
@ 2025-03-07  8:04 ` Thomas Zimmermann
  2025-03-07 11:14   ` Aditya Garg
  2025-03-07  8:04 ` [PATCH v2 3/5] drm/gm12u320: " Thomas Zimmermann
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-07  8:04 UTC (permalink / raw)
  To: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula
  Cc: dri-devel, Thomas Zimmermann

Set the dma_dev field provided by the DRM device. Required for PRIME
dma-buf import. Remove the driver's implementation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/appletbdrm.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
index 394c8f9bd41a..703b9a41a086 100644
--- a/drivers/gpu/drm/tiny/appletbdrm.c
+++ b/drivers/gpu/drm/tiny/appletbdrm.c
@@ -45,7 +45,7 @@
 #define APPLETBDRM_BULK_MSG_TIMEOUT	1000
 
 #define drm_to_adev(_drm)		container_of(_drm, struct appletbdrm_device, drm)
-#define adev_to_udev(adev)		interface_to_usbdev(to_usb_interface(adev->dmadev))
+#define adev_to_udev(adev)		interface_to_usbdev(to_usb_interface((adev)->drm.dev))
 
 struct appletbdrm_msg_request_header {
 	__le16 unk_00;
@@ -123,8 +123,6 @@ struct appletbdrm_fb_request_response {
 } __packed;
 
 struct appletbdrm_device {
-	struct device *dmadev;
-
 	unsigned int in_ep;
 	unsigned int out_ep;
 
@@ -612,22 +610,10 @@ static const struct drm_encoder_funcs appletbdrm_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
 
-static struct drm_gem_object *appletbdrm_driver_gem_prime_import(struct drm_device *dev,
-								 struct dma_buf *dma_buf)
-{
-	struct appletbdrm_device *adev = drm_to_adev(dev);
-
-	if (!adev->dmadev)
-		return ERR_PTR(-ENODEV);
-
-	return drm_gem_prime_import_dev(dev, dma_buf, adev->dmadev);
-}
-
 DEFINE_DRM_GEM_FOPS(appletbdrm_drm_fops);
 
 static const struct drm_driver appletbdrm_drm_driver = {
 	DRM_GEM_SHMEM_DRIVER_OPS,
-	.gem_prime_import	= appletbdrm_driver_gem_prime_import,
 	.name			= "appletbdrm",
 	.desc			= "Apple Touch Bar DRM Driver",
 	.major			= 1,
@@ -747,6 +733,7 @@ static int appletbdrm_probe(struct usb_interface *intf,
 	struct device *dev = &intf->dev;
 	struct appletbdrm_device *adev;
 	struct drm_device *drm = NULL;
+	struct device *dma_dev;
 	int ret;
 
 	ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
@@ -761,12 +748,19 @@ static int appletbdrm_probe(struct usb_interface *intf,
 
 	adev->in_ep = bulk_in->bEndpointAddress;
 	adev->out_ep = bulk_out->bEndpointAddress;
-	adev->dmadev = dev;
 
 	drm = &adev->drm;
 
 	usb_set_intfdata(intf, adev);
 
+	dma_dev = usb_intf_get_dma_device(intf);
+	if (dma_dev) {
+		drm_dev_set_dma_dev(drm, dma_dev);
+		put_device(dma_dev);
+	} else {
+		drm_warn(drm, "buffer sharing not supported"); /* not an error */
+	}
+
 	ret = appletbdrm_get_information(adev);
 	if (ret) {
 		drm_err(drm, "Failed to get display information\n");
@@ -805,7 +799,6 @@ static void appletbdrm_disconnect(struct usb_interface *intf)
 	struct appletbdrm_device *adev = usb_get_intfdata(intf);
 	struct drm_device *drm = &adev->drm;
 
-	put_device(adev->dmadev);
 	drm_dev_unplug(drm);
 	drm_atomic_helper_shutdown(drm);
 }
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] drm/gm12u320: Set struct drm_device.dma_dev
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
  2025-03-07  8:03 ` [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports Thomas Zimmermann
  2025-03-07  8:04 ` [PATCH v2 2/5] drm/appletbdrm: Set struct drm_device.dma_dev Thomas Zimmermann
@ 2025-03-07  8:04 ` Thomas Zimmermann
  2025-03-07  8:04 ` [PATCH v2 4/5] drm/gud: " Thomas Zimmermann
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-07  8:04 UTC (permalink / raw)
  To: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula
  Cc: dri-devel, Thomas Zimmermann

Set the dma_dev field provided by the DRM device. Required for PRIME
dma-buf import. Remove the driver's implementation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/gm12u320.c | 46 ++++++++++-----------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 41e9bfb2e2ff..fb0004166f4a 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -86,7 +86,6 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright, more silent)");
 
 struct gm12u320_device {
 	struct drm_device	         dev;
-	struct device                   *dmadev;
 	struct drm_simple_display_pipe   pipe;
 	struct drm_connector	         conn;
 	unsigned char                   *cmd_buf;
@@ -602,22 +601,6 @@ static const uint64_t gm12u320_pipe_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
-/*
- * 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 *gm12u320_gem_prime_import(struct drm_device *dev,
-							struct dma_buf *dma_buf)
-{
-	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
-
-	if (!gm12u320->dmadev)
-		return ERR_PTR(-ENODEV);
-
-	return drm_gem_prime_import_dev(dev, dma_buf, gm12u320->dmadev);
-}
-
 DEFINE_DRM_GEM_FOPS(gm12u320_fops);
 
 static const struct drm_driver gm12u320_drm_driver = {
@@ -630,7 +613,6 @@ static const struct drm_driver gm12u320_drm_driver = {
 
 	.fops		 = &gm12u320_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
-	.gem_prime_import = gm12u320_gem_prime_import,
 	DRM_FBDEV_SHMEM_DRIVER_OPS,
 };
 
@@ -645,6 +627,7 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 {
 	struct gm12u320_device *gm12u320;
 	struct drm_device *dev;
+	struct device *dma_dev;
 	int ret;
 
 	/*
@@ -660,16 +643,20 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 		return PTR_ERR(gm12u320);
 	dev = &gm12u320->dev;
 
-	gm12u320->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
-	if (!gm12u320->dmadev)
+	dma_dev = usb_intf_get_dma_device(interface);
+	if (dma_dev) {
+		drm_dev_set_dma_dev(dev, dma_dev);
+		put_device(dma_dev);
+	} else {
 		drm_warn(dev, "buffer sharing not supported"); /* not an error */
+	}
 
 	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
 	mutex_init(&gm12u320->fb_update.lock);
 
 	ret = drmm_mode_config_init(dev);
 	if (ret)
-		goto err_put_device;
+		return ret;
 
 	dev->mode_config.min_width = GM12U320_USER_WIDTH;
 	dev->mode_config.max_width = GM12U320_USER_WIDTH;
@@ -679,15 +666,15 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 
 	ret = gm12u320_usb_alloc(gm12u320);
 	if (ret)
-		goto err_put_device;
+		return ret;
 
 	ret = gm12u320_set_ecomode(gm12u320);
 	if (ret)
-		goto err_put_device;
+		return ret;
 
 	ret = gm12u320_conn_init(gm12u320);
 	if (ret)
-		goto err_put_device;
+		return ret;
 
 	ret = drm_simple_display_pipe_init(&gm12u320->dev,
 					   &gm12u320->pipe,
@@ -697,31 +684,24 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 					   gm12u320_pipe_modifiers,
 					   &gm12u320->conn);
 	if (ret)
-		goto err_put_device;
+		return ret;
 
 	drm_mode_config_reset(dev);
 
 	usb_set_intfdata(interface, dev);
 	ret = drm_dev_register(dev, 0);
 	if (ret)
-		goto err_put_device;
+		return ret;
 
 	drm_client_setup(dev, NULL);
 
 	return 0;
-
-err_put_device:
-	put_device(gm12u320->dmadev);
-	return ret;
 }
 
 static void gm12u320_usb_disconnect(struct usb_interface *interface)
 {
 	struct drm_device *dev = usb_get_intfdata(interface);
-	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
 
-	put_device(gm12u320->dmadev);
-	gm12u320->dmadev = NULL;
 	drm_dev_unplug(dev);
 	drm_atomic_helper_shutdown(dev);
 }
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/5] drm/gud: Set struct drm_device.dma_dev
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2025-03-07  8:04 ` [PATCH v2 3/5] drm/gm12u320: " Thomas Zimmermann
@ 2025-03-07  8:04 ` Thomas Zimmermann
  2025-03-07  8:04 ` [PATCH v2 5/5] drm/udl: " Thomas Zimmermann
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-07  8:04 UTC (permalink / raw)
  To: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula
  Cc: dri-devel, Thomas Zimmermann

Set the dma_dev field provided by the DRM device. Required for PRIME
dma-buf import. Remove the driver's implementation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++++----------------------
 drivers/gpu/drm/gud/gud_internal.h |  1 -
 2 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index cb405771d6e2..5385a2126e45 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -309,21 +309,6 @@ static int gud_get_properties(struct gud_device *gdrm)
 	return ret;
 }
 
-/*
- * 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 *gud_gem_prime_import(struct drm_device *drm, struct dma_buf *dma_buf)
-{
-	struct gud_device *gdrm = to_gud_device(drm);
-
-	if (!gdrm->dmadev)
-		return ERR_PTR(-ENODEV);
-
-	return drm_gem_prime_import_dev(drm, dma_buf, gdrm->dmadev);
-}
-
 static int gud_stats_debugfs(struct seq_file *m, void *data)
 {
 	struct drm_debugfs_entry *entry = m->private;
@@ -376,7 +361,6 @@ static const struct drm_driver gud_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
 	.fops			= &gud_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
-	.gem_prime_import	= gud_gem_prime_import,
 	DRM_FBDEV_SHMEM_DRIVER_OPS,
 
 	.name			= "gud",
@@ -434,6 +418,7 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	size_t max_buffer_size = 0;
 	struct gud_device *gdrm;
 	struct drm_device *drm;
+	struct device *dma_dev;
 	u8 *formats_dev;
 	u32 *formats;
 	int ret, i;
@@ -609,17 +594,19 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id)
 
 	usb_set_intfdata(intf, gdrm);
 
-	gdrm->dmadev = usb_intf_get_dma_device(intf);
-	if (!gdrm->dmadev)
-		dev_warn(dev, "buffer sharing not supported");
+	dma_dev = usb_intf_get_dma_device(intf);
+	if (dma_dev) {
+		drm_dev_set_dma_dev(drm, dma_dev);
+		put_device(dma_dev);
+	} else {
+		dev_warn(dev, "buffer sharing not supported"); /* not an error */
+	}
 
 	drm_debugfs_add_file(drm, "stats", gud_stats_debugfs, NULL);
 
 	ret = drm_dev_register(drm, 0);
-	if (ret) {
-		put_device(gdrm->dmadev);
+	if (ret)
 		return ret;
-	}
 
 	drm_kms_helper_poll_init(drm);
 
@@ -638,8 +625,6 @@ static void gud_disconnect(struct usb_interface *interface)
 	drm_kms_helper_poll_fini(drm);
 	drm_dev_unplug(drm);
 	drm_atomic_helper_shutdown(drm);
-	put_device(gdrm->dmadev);
-	gdrm->dmadev = NULL;
 }
 
 static int gud_suspend(struct usb_interface *intf, pm_message_t message)
diff --git a/drivers/gpu/drm/gud/gud_internal.h b/drivers/gpu/drm/gud/gud_internal.h
index 0d148a6f27aa..d6fb25388722 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -16,7 +16,6 @@
 struct gud_device {
 	struct drm_device drm;
 	struct drm_simple_display_pipe pipe;
-	struct device *dmadev;
 	struct work_struct work;
 	u32 flags;
 	const struct drm_format_info *xrgb8888_emulation_format;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/5] drm/udl: Set struct drm_device.dma_dev
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2025-03-07  8:04 ` [PATCH v2 4/5] drm/gud: " Thomas Zimmermann
@ 2025-03-07  8:04 ` Thomas Zimmermann
  2025-03-07 12:50 ` [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Jani Nikula
  2025-03-07 13:32 ` Simona Vetter
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-07  8:04 UTC (permalink / raw)
  To: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula
  Cc: dri-devel, Thomas Zimmermann

Set the dma_dev field provided by the DRM device. Required for PRIME
dma-buf import. Remove the driver's implementation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/udl/udl_drv.c  | 17 -----------------
 drivers/gpu/drm/udl/udl_drv.h  |  1 -
 drivers/gpu/drm/udl/udl_main.c | 14 +++++++-------
 3 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 05b3a152cc33..3b56ca2f6eb8 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 = {
@@ -73,7 +57,6 @@ 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_FBDEV_SHMEM_DRIVER_OPS,
 
 	.name = DRIVER_NAME,
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index be00dc1d87a1..e67e7e2e6f1f 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -51,7 +51,6 @@ struct urb_list {
 struct udl_device {
 	struct drm_device drm;
 	struct device *dev;
-	struct device *dmadev;
 
 	struct drm_plane primary_plane;
 	struct drm_crtc crtc;
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 3ebe2ce55dfd..cbb0169cc030 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -308,12 +308,17 @@ int udl_init(struct udl_device *udl)
 {
 	struct drm_device *dev = &udl->drm;
 	int ret = -ENOMEM;
+	struct device *dma_dev;
 
 	DRM_DEBUG("\n");
 
-	udl->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
-	if (!udl->dmadev)
+	dma_dev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
+	if (dma_dev) {
+		drm_dev_set_dma_dev(dev, dma_dev);
+		put_device(dma_dev);
+	} else {
 		drm_warn(dev, "buffer sharing not supported"); /* not an error */
+	}
 
 	mutex_init(&udl->gem_lock);
 
@@ -343,18 +348,13 @@ int udl_init(struct udl_device *udl)
 err:
 	if (udl->urbs.count)
 		udl_free_urb_list(dev);
-	put_device(udl->dmadev);
 	DRM_ERROR("%d\n", ret);
 	return ret;
 }
 
 int udl_drop_usb(struct drm_device *dev)
 {
-	struct udl_device *udl = to_udl(dev);
-
 	udl_free_urb_list(dev);
-	put_device(udl->dmadev);
-	udl->dmadev = NULL;
 
 	return 0;
 }
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/5] drm/appletbdrm: Set struct drm_device.dma_dev
  2025-03-07  8:04 ` [PATCH v2 2/5] drm/appletbdrm: Set struct drm_device.dma_dev Thomas Zimmermann
@ 2025-03-07 11:14   ` Aditya Garg
  0 siblings, 0 replies; 15+ messages in thread
From: Aditya Garg @ 2025-03-07 11:14 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: simona@ffwll.ch, javierm@redhat.com, airlied@gmail.com,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	hdegoede@redhat.com, airlied@redhat.com, sean@poorly.run,
	sumit.semwal@linaro.org, christian.koenig@amd.com,
	admin@kodeit.net, jani.nikula@intel.com,
	dri-devel@lists.freedesktop.org

Hi

> On 7 Mar 2025, at 1:34 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
> Set the dma_dev field provided by the DRM device. Required for PRIME
> dma-buf import. Remove the driver's implementation.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/tiny/appletbdrm.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)

For this patch

Tested-by: Aditya Garg <gargaditya08@live.com>
Reviewed-by: Aditya Garg <gargaditya08@live.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports
  2025-03-07  8:03 ` [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports Thomas Zimmermann
@ 2025-03-07 12:45   ` Jani Nikula
  2025-03-07 13:03   ` Maxime Ripard
  1 sibling, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2025-03-07 12:45 UTC (permalink / raw)
  To: Thomas Zimmermann, simona, javierm, airlied, maarten.lankhorst,
	mripard, hdegoede, airlied, sean, sumit.semwal, christian.koenig,
	admin, gargaditya08
  Cc: dri-devel, Thomas Zimmermann

On Fri, 07 Mar 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Importing dma-bufs via PRIME requires a DMA-capable device. Devices on
> peripheral busses, such as USB, often cannot perform DMA by themselves.
> Without DMA-capable device PRIME import fails. DRM drivers for USB
> devices already use a separate DMA device for dma-buf imports. Make the
> mechanism generally available.
>
> Besides the case of USB, there are embedded DRM devices without DMA
> capability. DMA is performed by a separate controller. DRM drivers should
> set this accordingly.
>
> Add the field dma_dev to struct drm_device to refer to the device's DMA
> device. For USB this should be the USB controller. Use dma_dev in the
> PRIME import helpers, if set.
>
> v2:
> - acquire internal reference on dma_dev (Jani)
> - add DMA-controller usecase to docs (Maxime)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/drm_drv.c   | 21 +++++++++++++++++++
>  drivers/gpu/drm/drm_prime.c |  2 +-
>  include/drm/drm_device.h    | 41 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 17fc5dc708f4..c9487bc88624 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -500,6 +500,25 @@ void drm_dev_unplug(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_dev_unplug);
>  
> +/**
> + * drm_dev_set_dma_dev - set the DMA device for a DRM device
> + * @dev: DRM device
> + * @dma_dev: DMA device or NULL
> + *
> + * Sets the DMA device of the given DRM device. Only required if
> + * the DMA device is different from the DRM device's parent. After
> + * calling this function, the DRM device holds a reference on
> + * @dma_dev. Pass NULL to clear the DMA device.
> + */
> +void drm_dev_set_dma_dev(struct drm_device *dev, struct device *dma_dev)
> +{
> +	dma_dev = get_device(dma_dev);
> +
> +	put_device(dev->dma_dev);
> +	dev->dma_dev = dma_dev;
> +}
> +EXPORT_SYMBOL(drm_dev_set_dma_dev);
> +
>  /*
>   * Available recovery methods for wedged device. To be sent along with device
>   * wedged uevent.
> @@ -654,6 +673,8 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
>  {
>  	drm_fs_inode_free(dev->anon_inode);
>  
> +	put_device(dev->dma_dev);
> +	dev->dma_dev = NULL;
>  	put_device(dev->dev);
>  	/* Prevent use-after-free in drm_managed_release when debugging is
>  	 * enabled. Slightly awkward, but can't really be helped. */
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index bdb51c8f262e..ed4e5f06b79d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -998,7 +998,7 @@ 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);
> +	return drm_gem_prime_import_dev(dev, dma_buf, drm_dev_dma_dev(dev));
>  }
>  EXPORT_SYMBOL(drm_gem_prime_import);
>  
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 6ea54a578cda..e2f894f1b90a 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -64,6 +64,28 @@ struct drm_device {
>  	/** @dev: Device structure of bus-device */
>  	struct device *dev;
>  
> +	/**
> +	 * @dma_dev:
> +	 *
> +	 * Device for DMA operations. Only required if the device @dev
> +	 * cannot perform DMA by itself. Should be NULL otherwise. Call
> +	 * drm_dev_dma_dev() to get the DMA device instead of using this
> +	 * field directly. Call drm_dev_set_dma_dev() to set this field.
> +	 *
> +	 * DRM devices are sometimes bound to virtual devices that cannot
> +	 * perform DMA by themselves. Drivers should set this field to the
> +	 * respective DMA controller.
> +	 *
> +	 * Devices on USB and other peripheral busses also cannot perform
> +	 * DMA by themselves. The @dma_dev field should point the bus
> +	 * controller that does DMA on behalve of such a device. Required
> +	 * for importing buffers via dma-buf.
> +	 *
> +	 * If set, the DRM core automatically releases the reference on the
> +	 * device.
> +	 */
> +	struct device *dma_dev;
> +
>  	/**
>  	 * @managed:
>  	 *
> @@ -327,4 +349,23 @@ struct drm_device {
>  	struct dentry *debugfs_root;
>  };
>  
> +void drm_dev_set_dma_dev(struct drm_device *dev, struct device *dma_dev);
> +
> +/**
> + * drm_dev_dma_dev - returns the DMA device for a DRM device
> + * @dev: DRM device
> + *
> + * Returns the DMA device of the given DRM device. By default, this
> + * the DRM device's parent. See drm_dev_set_dma_dev().
> + *
> + * Returns:
> + * A DMA-capable device for the DRM device.
> + */
> +static inline struct device *drm_dev_dma_dev(struct drm_device *dev)
> +{
> +	if (dev->dma_dev)
> +		return dev->dma_dev;
> +	return dev->dev;
> +}
> +
>  #endif

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2025-03-07  8:04 ` [PATCH v2 5/5] drm/udl: " Thomas Zimmermann
@ 2025-03-07 12:50 ` Jani Nikula
  2025-03-07 13:32 ` Simona Vetter
  6 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2025-03-07 12:50 UTC (permalink / raw)
  To: Thomas Zimmermann, simona, javierm, airlied, maarten.lankhorst,
	mripard, hdegoede, airlied, sean, sumit.semwal, christian.koenig,
	admin, gargaditya08
  Cc: dri-devel, Thomas Zimmermann

On Fri, 07 Mar 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Importing dma-bufs via PRIME requires a DMA-capable hardware device.
> This is not the case for USB, where DMA is performed entirely by the
> USB controller instead of the USB devices.
>
> Drivers for USB-based hardware maintain their own workarounds for this
> problem. The original idea to resolve this was to provide different
> PRIME helpers for such devices, but the dma-buf code internally assumes
> DMA functionality as well. So that ideas is not realistic.
>
> Let's instead turn the current workaround into a feature. Patch 1 adds a
> dma_dev field to struct drm_device and makes the PRIME code use it. Patches
> 2 to 5 replace related driver code.
>
> It will also be useful in other code. The exynos and mediatek drivers
> already maintain a dedicated DMA device for non-PRIME code. They could
> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
> memory with the regular parent device. They should support the dma_dev
> settings as well.
>
> Tested with udl.

I mainly reviewed the first patch, but then glanced through all the rest
too, and didn't spot anything obviously wrong.

So FWIW,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

on the rest too.


>
> v2:
> - maintain reference on dma_dev (Jani)
> - improve docs (Maxime)
> - update appletbdrm
>
> Thomas Zimmermann (5):
>   drm/prime: Support dedicated DMA device for dma-buf imports
>   drm/appletbdrm: Set struct drm_device.dma_dev
>   drm/gm12u320: Set struct drm_device.dma_dev
>   drm/gud: Set struct drm_device.dma_dev
>   drm/udl: Set struct drm_device.dma_dev
>
>  drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
>  drivers/gpu/drm/drm_prime.c        |  2 +-
>  drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
>  drivers/gpu/drm/gud/gud_internal.h |  1 -
>  drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
>  drivers/gpu/drm/tiny/gm12u320.c    | 46 +++++++++---------------------
>  drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
>  drivers/gpu/drm/udl/udl_drv.h      |  1 -
>  drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
>  include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
>  10 files changed, 102 insertions(+), 101 deletions(-)

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports
  2025-03-07  8:03 ` [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports Thomas Zimmermann
  2025-03-07 12:45   ` Jani Nikula
@ 2025-03-07 13:03   ` Maxime Ripard
  1 sibling, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2025-03-07 13:03 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: admin, airlied, airlied, christian.koenig, dri-devel,
	gargaditya08, hdegoede, jani.nikula, javierm, maarten.lankhorst,
	mripard, sean, simona, sumit.semwal, Maxime Ripard

On Fri, 7 Mar 2025 09:03:59 +0100, Thomas Zimmermann wrote:
> Importing dma-bufs via PRIME requires a DMA-capable device. Devices on
> peripheral busses, such as USB, often cannot perform DMA by themselves.
> Without DMA-capable device PRIME import fails. DRM drivers for USB
> devices already use a separate DMA device for dma-buf imports. Make the
> mechanism generally available.
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
  2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
                   ` (5 preceding siblings ...)
  2025-03-07 12:50 ` [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Jani Nikula
@ 2025-03-07 13:32 ` Simona Vetter
  2025-03-10  9:50   ` Thomas Zimmermann
  6 siblings, 1 reply; 15+ messages in thread
From: Simona Vetter @ 2025-03-07 13:32 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula, dri-devel

On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote:
> Importing dma-bufs via PRIME requires a DMA-capable hardware device.
> This is not the case for USB, where DMA is performed entirely by the
> USB controller instead of the USB devices.
> 
> Drivers for USB-based hardware maintain their own workarounds for this
> problem. The original idea to resolve this was to provide different
> PRIME helpers for such devices, but the dma-buf code internally assumes
> DMA functionality as well. So that ideas is not realistic.

So dma-buf without dma is doable, but you have to avoid dma_buf_attach.
And that is a lot of surgery in the current prime helpers, since those
assume that an attachment always exists. But dma-buf itself is entirely
fine with cpu-only access through either userspace mmap or kernel vmap.

I think as an interim step this is still good, since it makes the current
hacks easier to find because at least it's all common now.
-Sima

> Let's instead turn the current workaround into a feature. Patch 1 adds a
> dma_dev field to struct drm_device and makes the PRIME code use it. Patches
> 2 to 5 replace related driver code.
> 
> It will also be useful in other code. The exynos and mediatek drivers
> already maintain a dedicated DMA device for non-PRIME code. They could
> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
> memory with the regular parent device. They should support the dma_dev
> settings as well.
> 
> Tested with udl.
> 
> v2:
> - maintain reference on dma_dev (Jani)
> - improve docs (Maxime)
> - update appletbdrm
> 
> Thomas Zimmermann (5):
>   drm/prime: Support dedicated DMA device for dma-buf imports
>   drm/appletbdrm: Set struct drm_device.dma_dev
>   drm/gm12u320: Set struct drm_device.dma_dev
>   drm/gud: Set struct drm_device.dma_dev
>   drm/udl: Set struct drm_device.dma_dev
> 
>  drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
>  drivers/gpu/drm/drm_prime.c        |  2 +-
>  drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
>  drivers/gpu/drm/gud/gud_internal.h |  1 -
>  drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
>  drivers/gpu/drm/tiny/gm12u320.c    | 46 +++++++++---------------------
>  drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
>  drivers/gpu/drm/udl/udl_drv.h      |  1 -
>  drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
>  include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
>  10 files changed, 102 insertions(+), 101 deletions(-)
> 
> -- 
> 2.48.1
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
  2025-03-07 13:32 ` Simona Vetter
@ 2025-03-10  9:50   ` Thomas Zimmermann
  2025-03-10 10:04     ` Thomas Zimmermann
  2025-03-10 13:56     ` Christian König
  0 siblings, 2 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-10  9:50 UTC (permalink / raw)
  To: Simona Vetter
  Cc: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula, dri-devel

Hi

Am 07.03.25 um 14:32 schrieb Simona Vetter:
> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote:
>> Importing dma-bufs via PRIME requires a DMA-capable hardware device.
>> This is not the case for USB, where DMA is performed entirely by the
>> USB controller instead of the USB devices.
>>
>> Drivers for USB-based hardware maintain their own workarounds for this
>> problem. The original idea to resolve this was to provide different
>> PRIME helpers for such devices, but the dma-buf code internally assumes
>> DMA functionality as well. So that ideas is not realistic.
> So dma-buf without dma is doable, but you have to avoid dma_buf_attach.
> And that is a lot of surgery in the current prime helpers, since those
> assume that an attachment always exists. But dma-buf itself is entirely
> fine with cpu-only access through either userspace mmap or kernel vmap.

Right. That's roughly how far I got in this direction. The field 
import_attach, set up by dma_buf_attach(), is currently used throughout 
the DRM code and drivers. Hence this series and the other one that 
replaced some of the uses of import_attach. Once this has all been 
resolved, there will be a few users of the field left, which might be 
uncritical.

Best regards
Thomas

>
> I think as an interim step this is still good, since it makes the current
> hacks easier to find because at least it's all common now.
> -Sima
>
>> Let's instead turn the current workaround into a feature. Patch 1 adds a
>> dma_dev field to struct drm_device and makes the PRIME code use it. Patches
>> 2 to 5 replace related driver code.
>>
>> It will also be useful in other code. The exynos and mediatek drivers
>> already maintain a dedicated DMA device for non-PRIME code. They could
>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
>> memory with the regular parent device. They should support the dma_dev
>> settings as well.
>>
>> Tested with udl.
>>
>> v2:
>> - maintain reference on dma_dev (Jani)
>> - improve docs (Maxime)
>> - update appletbdrm
>>
>> Thomas Zimmermann (5):
>>    drm/prime: Support dedicated DMA device for dma-buf imports
>>    drm/appletbdrm: Set struct drm_device.dma_dev
>>    drm/gm12u320: Set struct drm_device.dma_dev
>>    drm/gud: Set struct drm_device.dma_dev
>>    drm/udl: Set struct drm_device.dma_dev
>>
>>   drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
>>   drivers/gpu/drm/drm_prime.c        |  2 +-
>>   drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
>>   drivers/gpu/drm/gud/gud_internal.h |  1 -
>>   drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
>>   drivers/gpu/drm/tiny/gm12u320.c    | 46 +++++++++---------------------
>>   drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
>>   drivers/gpu/drm/udl/udl_drv.h      |  1 -
>>   drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
>>   include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
>>   10 files changed, 102 insertions(+), 101 deletions(-)
>>
>> -- 
>> 2.48.1
>>

-- 
--
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] 15+ messages in thread

* Re: [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
  2025-03-10  9:50   ` Thomas Zimmermann
@ 2025-03-10 10:04     ` Thomas Zimmermann
  2025-03-10 13:56     ` Christian König
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-10 10:04 UTC (permalink / raw)
  To: Simona Vetter
  Cc: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, christian.koenig, admin,
	gargaditya08, jani.nikula, dri-devel



Am 10.03.25 um 10:50 schrieb Thomas Zimmermann:
> Hi
>
> Am 07.03.25 um 14:32 schrieb Simona Vetter:
>> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote:
>>> Importing dma-bufs via PRIME requires a DMA-capable hardware device.
>>> This is not the case for USB, where DMA is performed entirely by the
>>> USB controller instead of the USB devices.
>>>
>>> Drivers for USB-based hardware maintain their own workarounds for this
>>> problem. The original idea to resolve this was to provide different
>>> PRIME helpers for such devices, but the dma-buf code internally assumes
>>> DMA functionality as well. So that ideas is not realistic.
>> So dma-buf without dma is doable, but you have to avoid dma_buf_attach.

FYI. I was referring to [1], which use the attachment to get the S/G 
table for import.

[1] 
https://elixir.bootlin.com/linux/v6.13.6/source/drivers/gpu/drm/drm_prime.c#L964

>> And that is a lot of surgery in the current prime helpers, since those
>> assume that an attachment always exists. But dma-buf itself is entirely
>> fine with cpu-only access through either userspace mmap or kernel vmap.
>
> Right. That's roughly how far I got in this direction. The field 
> import_attach, set up by dma_buf_attach(), is currently used 
> throughout the DRM code and drivers. Hence this series and the other 
> one that replaced some of the uses of import_attach. Once this has all 
> been resolved, there will be a few users of the field left, which 
> might be uncritical.
>
> Best regards
> Thomas
>
>>
>> I think as an interim step this is still good, since it makes the 
>> current
>> hacks easier to find because at least it's all common now.
>> -Sima
>>
>>> Let's instead turn the current workaround into a feature. Patch 1 
>>> adds a
>>> dma_dev field to struct drm_device and makes the PRIME code use it. 
>>> Patches
>>> 2 to 5 replace related driver code.
>>>
>>> It will also be useful in other code. The exynos and mediatek drivers
>>> already maintain a dedicated DMA device for non-PRIME code. They could
>>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
>>> memory with the regular parent device. They should support the dma_dev
>>> settings as well.
>>>
>>> Tested with udl.
>>>
>>> v2:
>>> - maintain reference on dma_dev (Jani)
>>> - improve docs (Maxime)
>>> - update appletbdrm
>>>
>>> Thomas Zimmermann (5):
>>>    drm/prime: Support dedicated DMA device for dma-buf imports
>>>    drm/appletbdrm: Set struct drm_device.dma_dev
>>>    drm/gm12u320: Set struct drm_device.dma_dev
>>>    drm/gud: Set struct drm_device.dma_dev
>>>    drm/udl: Set struct drm_device.dma_dev
>>>
>>>   drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
>>>   drivers/gpu/drm/drm_prime.c        |  2 +-
>>>   drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
>>>   drivers/gpu/drm/gud/gud_internal.h |  1 -
>>>   drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
>>>   drivers/gpu/drm/tiny/gm12u320.c    | 46 
>>> +++++++++---------------------
>>>   drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
>>>   drivers/gpu/drm/udl/udl_drv.h      |  1 -
>>>   drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
>>>   include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
>>>   10 files changed, 102 insertions(+), 101 deletions(-)
>>>
>>> -- 
>>> 2.48.1
>>>
>

-- 
--
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] 15+ messages in thread

* Re: [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
  2025-03-10  9:50   ` Thomas Zimmermann
  2025-03-10 10:04     ` Thomas Zimmermann
@ 2025-03-10 13:56     ` Christian König
  2025-03-10 14:30       ` Thomas Zimmermann
  1 sibling, 1 reply; 15+ messages in thread
From: Christian König @ 2025-03-10 13:56 UTC (permalink / raw)
  To: Thomas Zimmermann, Simona Vetter
  Cc: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, admin, gargaditya08, jani.nikula,
	dri-devel

Am 10.03.25 um 10:50 schrieb Thomas Zimmermann:
> Hi
>
> Am 07.03.25 um 14:32 schrieb Simona Vetter:
>> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote:
>>> Importing dma-bufs via PRIME requires a DMA-capable hardware device.
>>> This is not the case for USB, where DMA is performed entirely by the
>>> USB controller instead of the USB devices.
>>>
>>> Drivers for USB-based hardware maintain their own workarounds for this
>>> problem. The original idea to resolve this was to provide different
>>> PRIME helpers for such devices, but the dma-buf code internally assumes
>>> DMA functionality as well. So that ideas is not realistic.
>> So dma-buf without dma is doable, but you have to avoid dma_buf_attach.
>> And that is a lot of surgery in the current prime helpers, since those
>> assume that an attachment always exists. But dma-buf itself is entirely
>> fine with cpu-only access through either userspace mmap or kernel vmap.
>
> Right. That's roughly how far I got in this direction. The field import_attach, set up by dma_buf_attach(), is currently used throughout the DRM code and drivers. Hence this series and the other one that replaced some of the uses of import_attach. Once this has all been resolved, there will be a few users of the field left, which might be uncritical.

Mhm, if I remember correctly the DMA subsystem used to use the DMA mask and other parameters of the parent device when a specific device couldn't do DMA by itself.

I do remember a lot of discussion about that for the DMA-buf tee driver.

Going to read up on that once more. Could be that this patch here is not really necessary.

Regards,
Christian.

>
> Best regards
> Thomas
>
>>
>> I think as an interim step this is still good, since it makes the current
>> hacks easier to find because at least it's all common now.
>> -Sima
>>
>>> Let's instead turn the current workaround into a feature. Patch 1 adds a
>>> dma_dev field to struct drm_device and makes the PRIME code use it. Patches
>>> 2 to 5 replace related driver code.
>>>
>>> It will also be useful in other code. The exynos and mediatek drivers
>>> already maintain a dedicated DMA device for non-PRIME code. They could
>>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
>>> memory with the regular parent device. They should support the dma_dev
>>> settings as well.
>>>
>>> Tested with udl.
>>>
>>> v2:
>>> - maintain reference on dma_dev (Jani)
>>> - improve docs (Maxime)
>>> - update appletbdrm
>>>
>>> Thomas Zimmermann (5):
>>>    drm/prime: Support dedicated DMA device for dma-buf imports
>>>    drm/appletbdrm: Set struct drm_device.dma_dev
>>>    drm/gm12u320: Set struct drm_device.dma_dev
>>>    drm/gud: Set struct drm_device.dma_dev
>>>    drm/udl: Set struct drm_device.dma_dev
>>>
>>>   drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
>>>   drivers/gpu/drm/drm_prime.c        |  2 +-
>>>   drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
>>>   drivers/gpu/drm/gud/gud_internal.h |  1 -
>>>   drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
>>>   drivers/gpu/drm/tiny/gm12u320.c    | 46 +++++++++---------------------
>>>   drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
>>>   drivers/gpu/drm/udl/udl_drv.h      |  1 -
>>>   drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
>>>   include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
>>>   10 files changed, 102 insertions(+), 101 deletions(-)
>>>
>>> -- 
>>> 2.48.1
>>>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import
  2025-03-10 13:56     ` Christian König
@ 2025-03-10 14:30       ` Thomas Zimmermann
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2025-03-10 14:30 UTC (permalink / raw)
  To: Christian König, Simona Vetter
  Cc: simona, javierm, airlied, maarten.lankhorst, mripard, hdegoede,
	airlied, sean, sumit.semwal, admin, gargaditya08, jani.nikula,
	dri-devel

Hi

Am 10.03.25 um 14:56 schrieb Christian König:
> Am 10.03.25 um 10:50 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 07.03.25 um 14:32 schrieb Simona Vetter:
>>> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote:
>>>> Importing dma-bufs via PRIME requires a DMA-capable hardware device.
>>>> This is not the case for USB, where DMA is performed entirely by the
>>>> USB controller instead of the USB devices.
>>>>
>>>> Drivers for USB-based hardware maintain their own workarounds for this
>>>> problem. The original idea to resolve this was to provide different
>>>> PRIME helpers for such devices, but the dma-buf code internally assumes
>>>> DMA functionality as well. So that ideas is not realistic.
>>> So dma-buf without dma is doable, but you have to avoid dma_buf_attach.
>>> And that is a lot of surgery in the current prime helpers, since those
>>> assume that an attachment always exists. But dma-buf itself is entirely
>>> fine with cpu-only access through either userspace mmap or kernel vmap.
>> Right. That's roughly how far I got in this direction. The field import_attach, set up by dma_buf_attach(), is currently used throughout the DRM code and drivers. Hence this series and the other one that replaced some of the uses of import_attach. Once this has all been resolved, there will be a few users of the field left, which might be uncritical.
> Mhm, if I remember correctly the DMA subsystem used to use the DMA mask and other parameters of the parent device when a specific device couldn't do DMA by itself.

This went away (for USB at least) with commit 6eb0233ec2d0 ("usb: don't 
inherity DMA properties for USB devices"). Buffer sharing with USB 
graphics devices failed and we worked around it in 659ab7a49cbe ("drm: 
Use USB controller's DMA mask when importing dmabufs").

Best regards
Thomas

>
> I do remember a lot of discussion about that for the DMA-buf tee driver.
>
> Going to read up on that once more. Could be that this patch here is not really necessary.
>
> Regards,
> Christian.
>
>> Best regards
>> Thomas
>>
>>> I think as an interim step this is still good, since it makes the current
>>> hacks easier to find because at least it's all common now.
>>> -Sima
>>>
>>>> Let's instead turn the current workaround into a feature. Patch 1 adds a
>>>> dma_dev field to struct drm_device and makes the PRIME code use it. Patches
>>>> 2 to 5 replace related driver code.
>>>>
>>>> It will also be useful in other code. The exynos and mediatek drivers
>>>> already maintain a dedicated DMA device for non-PRIME code. They could
>>>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA
>>>> memory with the regular parent device. They should support the dma_dev
>>>> settings as well.
>>>>
>>>> Tested with udl.
>>>>
>>>> v2:
>>>> - maintain reference on dma_dev (Jani)
>>>> - improve docs (Maxime)
>>>> - update appletbdrm
>>>>
>>>> Thomas Zimmermann (5):
>>>>     drm/prime: Support dedicated DMA device for dma-buf imports
>>>>     drm/appletbdrm: Set struct drm_device.dma_dev
>>>>     drm/gm12u320: Set struct drm_device.dma_dev
>>>>     drm/gud: Set struct drm_device.dma_dev
>>>>     drm/udl: Set struct drm_device.dma_dev
>>>>
>>>>    drivers/gpu/drm/drm_drv.c          | 21 ++++++++++++++
>>>>    drivers/gpu/drm/drm_prime.c        |  2 +-
>>>>    drivers/gpu/drm/gud/gud_drv.c      | 33 ++++++---------------
>>>>    drivers/gpu/drm/gud/gud_internal.h |  1 -
>>>>    drivers/gpu/drm/tiny/appletbdrm.c  | 27 +++++++-----------
>>>>    drivers/gpu/drm/tiny/gm12u320.c    | 46 +++++++++---------------------
>>>>    drivers/gpu/drm/udl/udl_drv.c      | 17 -----------
>>>>    drivers/gpu/drm/udl/udl_drv.h      |  1 -
>>>>    drivers/gpu/drm/udl/udl_main.c     | 14 ++++-----
>>>>    include/drm/drm_device.h           | 41 ++++++++++++++++++++++++++
>>>>    10 files changed, 102 insertions(+), 101 deletions(-)
>>>>
>>>> -- 
>>>> 2.48.1
>>>>

-- 
--
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] 15+ messages in thread

end of thread, other threads:[~2025-03-10 14:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-07  8:03 [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Thomas Zimmermann
2025-03-07  8:03 ` [PATCH v2 1/5] drm/prime: Support dedicated DMA device for dma-buf imports Thomas Zimmermann
2025-03-07 12:45   ` Jani Nikula
2025-03-07 13:03   ` Maxime Ripard
2025-03-07  8:04 ` [PATCH v2 2/5] drm/appletbdrm: Set struct drm_device.dma_dev Thomas Zimmermann
2025-03-07 11:14   ` Aditya Garg
2025-03-07  8:04 ` [PATCH v2 3/5] drm/gm12u320: " Thomas Zimmermann
2025-03-07  8:04 ` [PATCH v2 4/5] drm/gud: " Thomas Zimmermann
2025-03-07  8:04 ` [PATCH v2 5/5] drm/udl: " Thomas Zimmermann
2025-03-07 12:50 ` [PATCH v2 0/5] drm: Provide a dedicated DMA device for PRIME import Jani Nikula
2025-03-07 13:32 ` Simona Vetter
2025-03-10  9:50   ` Thomas Zimmermann
2025-03-10 10:04     ` Thomas Zimmermann
2025-03-10 13:56     ` Christian König
2025-03-10 14:30       ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.