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

[Thomas Zimmermann]

Hi

Am 23.02.21 um 14:44 schrieb Takashi Iwai:
> On Tue, 23 Feb 2021 11:58:42 +0100,
> Thomas Zimmermann wrote:
>>
>> USB devices cannot perform DMA and hence have no dma_mask set in their
>> device structure. Importing dmabuf into a USB-based driver fails, which
>> break joining and mirroring of display in X11.
>>
>> For USB devices, pick the associated USB controller as attachment device,
>> so that it can perform DMA. If the DMa controller does not support DMA
>> transfers, we're aout of luck and cannot import.
>>
>> 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.
>>
>> 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 at suse.de>
>> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
>> Cc: Christoph Hellwig <hch at lst.de>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> Cc: Johan Hovold <johan at kernel.org>
>> Cc: Alan Stern <stern at rowland.harvard.edu>
>> Cc: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
>> Cc: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
>> Cc: Mathias Nyman <mathias.nyman at linux.intel.com>
>> Cc: Oliver Neukum <oneukum at suse.com>
>> Cc: Thomas Gleixner <tglx at linutronix.de>
>> Cc: <stable at vger.kernel.org> # v5.10+
>> ---
>>   drivers/gpu/drm/drm_prime.c        | 36 ++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/tiny/gm12u320.c    |  2 +-
>>   drivers/gpu/drm/udl/udl_drv.c      |  2 +-
>>   include/drm/drm_gem_shmem_helper.h | 13 +++++++++++
>>   include/drm/drm_prime.h            |  5 +++++
>>   5 files changed, 56 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>> index 2a54f86856af..9015850f2160 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,38 @@ 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().
>> + */
>> +#ifdef CONFIG_USB
>> +struct drm_gem_object *drm_gem_prime_import_usb(struct drm_device *dev,
>> +						struct dma_buf *dma_buf)
>> +{
>> +	struct usb_device *udev;
>> +	struct device *usbhost;
>> +
>> +	if (dev->dev->bus != &usb_bus_type)
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	udev = interface_to_usbdev(to_usb_interface(dev->dev));
>> +	if (!udev->bus)
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	usbhost = udev->bus->controller;
> 
> Aside from the discussion whether this "workaround" is needed, the use
> of udev->bus->controller here looks a bit suspicious.  As the old USB
> code (before the commit 6eb0233ec2d0) indicated, it was rather
> usb->bus->sysdev that was used for the DMA mask, and it's also the one
> most of USB core code refers to.  A similar question came up while
> fixing the same kind of bug in the media subsystem, and we concluded
> that bus->sysdev is a better choice.

Good to hear that we're not the only ones affected by this. Wrt the 
original code, using sysdev makes even more sense.

Best regards
Thomas

> 
> 
> Takashi
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210223/582b0f8c/attachment.sig>