Re: [PATCH 1/4] drm/prime: Support dedicated DMA device for dma-buf imports

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 28.02.25 um 12:21 schrieb Jani Nikula:
On Fri, 28 Feb 2025, Thomas Zimmermann <tzimmermann@xxxxxxx> 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.

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.

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

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 17fc5dc708f4..f8c3c9f77d22 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -654,6 +654,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;
The asymmetry in who gets and puts the ->dma_dev bugs me.

You could avoid that by providing a helper for setting ->dma_dev, and
having that get the device:

int drm_dev_set_dma_device(struct drm_device *drm, struct device *dma_device)
{
	dev->dma_dev = get_device(dma_device);

	return dev->dma_dev ? 0 : -ENODEV.
}

Taking drm/gm12u320 from patch 2 as an example, it would become:

	dma_device = usb_intf_get_dma_device(to_usb_interface(dev->dev))
	if (drm_dev_set_dma_device(drm, dma_device))
		drm_warn(dev, "buffer sharing not supported"); /* not an error */
	put_device(dma_device);

Sligthly more verbose, but has a get/put pair in the driver, and a
get/put pair in the core, instead of the asymmetry.

I'm not insisting, but something to consider.

We're doing a get_device() on the regular parent kept in dev. So yeah, keeping a ref on the dma_dev would make sense.

Best regards
Thomas



BR,
Jani.


  	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 32a8781cfd67..258858f2f8dd 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -1004,7 +1004,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..a24cac4b2077 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -64,6 +64,23 @@ 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.
+	 *
+	 * Devices on USB and other peripheral busses 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 driver has to acquire a reference on the DMA
+	 * device, which will be owned and released automatically by the
+	 * DRM core.
+	 */
+	struct device *dma_dev;
+
  	/**
  	 * @managed:
  	 *
@@ -327,4 +344,24 @@ struct drm_device {
  	struct dentry *debugfs_root;
  };
+/**
+ * drm_dev_dma_dev - returns the DMA device for a DRM device
+ * @dev: DRM device
+ *
+ * Returns the DMA device of the given DRM device. This is usually
+ * the same as the DRM device's parent. Devices on some peripheral
+ * busses, such as USB, are incapable of performing DMA by themselves.
+ * Drivers should set struct &drm_device.dma_dev to the bus controller
+ * to make DMA work. Required for PRIME buffer import.
+ *
+ * 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

--
--
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)




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux