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@suse.de Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") Tested-by: Pavel Machek pavel@ucw.cz Acked-by: Christian König christian.koenig@amd.com Acked-by: Daniel Vetter daniel.vetter@ffwll.ch Cc: Christoph Hellwig hch@lst.de Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: stable@vger.kernel.org # 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@suse.de, 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);
- dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
- if (drm_WARN_ONCE(dev, !dmadev, "buffer sharing not supported"))
return ERR_PTR(-ENODEV);
- obj = drm_gem_prime_import_dev(dev, dma_buf, dmadev);
- put_device(dmadev);
- return obj;
+} +EXPORT_SYMBOL(drm_gem_prime_import_usb); +#endif diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c index 0b4f4f2af1ef..99e7bd36a220 100644 --- a/drivers/gpu/drm/tiny/gm12u320.c +++ b/drivers/gpu/drm/tiny/gm12u320.c @@ -611,7 +611,7 @@ static const struct drm_driver gm12u320_drm_driver = { .minor = DRIVER_MINOR, .fops = &gm12u320_fops,
- DRM_GEM_SHMEM_DRIVER_OPS,
- DRM_GEM_SHMEM_DRIVER_OPS_USB,
}; static const struct drm_mode_config_funcs gm12u320_mode_config_funcs = { diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index 9269092697d8..2db483b2b199 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -39,7 +39,7 @@ static const struct drm_driver driver = { /* GEM hooks */ .fops = &udl_driver_fops,
- DRM_GEM_SHMEM_DRIVER_OPS,
- DRM_GEM_SHMEM_DRIVER_OPS_USB,
.name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 8f07b0516100..5e07921e87ba 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -748,6 +748,37 @@ void usb_put_intf(struct usb_interface *intf) } EXPORT_SYMBOL_GPL(usb_put_intf); +/**
- usb_get_dma_device - acquire a reference on the usb device's DMA endpoint
- @udev: usb device
- While a USB device cannot perform DMA operations by itself, many USB
- controllers can. A call to usb_get_dma_device() returns the DMA endpoint
- for the given USB device, if any. The returned device structure should be
- released with put_device().
"must be" released.
Same for the other documentation in here.
thanks,
greg k-h