Re: [PATCH v5] drm: Use USB controller's DMA mask when importing dmabufs

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

 



Hi

Am 26.02.21 um 11:19 schrieb Greg KH:
On Fri, Feb 26, 2021 at 10:26:47AM +0100, Thomas Zimmermann wrote:
USB devices cannot perform DMA and hence have no dma_mask set in their
device structure. Therefore importing dmabuf into a USB-based driver
fails, which breaks joining and mirroring of display in X11.

For USB devices, pick the associated USB controller as attachment device.
This allows the DRM import helpers to perform the DMA setup. If the DMA
controller does not support DMA transfers, we're out of luck and cannot
import. Our current USB-based DRM drivers don't use DMA, so the actual
DMA device is not important.

Drivers should use DRM_GEM_SHMEM_DROVER_OPS_USB to initialize their
instance of struct drm_driver.

Tested by joining/mirroring displays of udl and radeon un der Gnome/X11.

v5:
	* provide a helper for USB interfaces (Alan)
	* add FIXME item to documentation and TODO list (Daniel)
v4:
	* implement workaround with USB helper functions (Greg)
	* use struct usb_device->bus->sysdev as DMA device (Takashi)
v3:
	* drop gem_create_object
	* use DMA mask of USB controller, if any (Daniel, Christian, Noralf)
v2:
	* move fix to importer side (Christian, Daniel)
	* update SHMEM and CMA helpers for new PRIME callbacks

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
Tested-by: Pavel Machek <pavel@xxxxxx>
Acked-by: Christian König <christian.koenig@xxxxxxx>
Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # v5.10+
---
  Documentation/gpu/todo.rst         | 15 ++++++++++
  drivers/gpu/drm/drm_prime.c        | 45 ++++++++++++++++++++++++++++++
  drivers/gpu/drm/tiny/gm12u320.c    |  2 +-
  drivers/gpu/drm/udl/udl_drv.c      |  2 +-
  drivers/usb/core/usb.c             | 31 ++++++++++++++++++++
  include/drm/drm_gem_shmem_helper.h | 16 +++++++++++
  include/drm/drm_prime.h            |  5 ++++
  include/linux/usb.h                | 24 ++++++++++++++++
  8 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index f872d3d33218..c185e0a2951e 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -617,6 +617,21 @@ Contact: Daniel Vetter
Level: Intermediate +Remove automatic page mapping from dma-buf importing
+----------------------------------------------------
+
+When importing dma-bufs, the dma-buf and PRIME frameworks automatically map
+imported pages into the importer's DMA area. This is a problem for USB devices,
+which do not support DMA operations. By default, importing fails for USB
+devices. USB-based drivers work around this problem by employing
+drm_gem_prime_import_usb(). To fix the issue, automatic page mappings should
+be removed from the buffer-sharing code.
+
+Contact: Thomas Zimmermann <tzimmermann@xxxxxxx>, Daniel Vetter
+
+Level: Advanced
+
+
  Better Testing
  ==============
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 2a54f86856af..59013bb1cd4b 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -29,6 +29,7 @@
  #include <linux/export.h>
  #include <linux/dma-buf.h>
  #include <linux/rbtree.h>
+#include <linux/usb.h>
#include <drm/drm.h>
  #include <drm/drm_drv.h>
@@ -1055,3 +1056,47 @@ void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg)
  	dma_buf_put(dma_buf);
  }
  EXPORT_SYMBOL(drm_prime_gem_destroy);
+
+/**
+ * drm_gem_prime_import_usb - helper library implementation of the import callback for USB devices
+ * @dev: drm_device to import into
+ * @dma_buf: dma-buf object to import
+ *
+ * This is an implementation of drm_gem_prime_import() for USB-based devices.
+ * USB devices cannot perform DMA directly. This function selects the USB host
+ * controller as DMA device instead. Drivers can use this as their
+ * &drm_driver.gem_prime_import implementation.
+ *
+ * See also drm_gem_prime_import().
+ *
+ * FIXME: The dma-buf framework expects to map the exported pages into
+ *        the importer's DMA area. USB devices don't support DMA, and
+ *        importing would fail. Foir the time being, this function provides
+ *        a workaround by using the USB controller's DMA area. The real
+ *        solution is to remove page-mapping operations from the dma-buf
+ *        framework.
+ *
+ * Returns: A GEM object on success, or a pointer-encoder errno value otherwise.
+ */
+#ifdef CONFIG_USB
+struct drm_gem_object *drm_gem_prime_import_usb(struct drm_device *dev,
+						struct dma_buf *dma_buf)
+{
+	struct device *dmadev;
+	struct drm_gem_object *obj;
+
+	if (!dev_is_usb(dev->dev))
+		return ERR_PTR(-ENODEV);

I have resisted the "dev_is_*()" type of function for USB for a long
time now, and I really don't want to add it now.

The driver core explicitly was not created with RTI (run type
identification), but over time it has been slowly added on a per-bus
basis for various reasons, some good and others not good.

In this function, why would a drm device that was NOT a usb device ever
call it?  Because of that, I don't think dev_is_usb() is needed at all,
just don't call this function unless it really is a USB device.

It was simply a safety measure. There's really no reason a non-USB device would ever call this function. So not using dev_is_usb() is fine. It'll be gone in v6.

Best regards
Thomas


If you need help enforcing it, add a 'struct usb_interface *' to the
function parameters but please don't create this function.

Hm, looks like we have 2 extern definitions of usb_bus_type in different
.h files, I should go fix that up now to try to prevent this type of
thing in the first place...

thanks,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[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