[PATCH v5] drm: Use USB controller's DMA mask when importing dmabufs
Thomas Zimmermann
tzimmermann at suse.de
Fri Feb 26 10:29:05 UTC 2021
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 at suse.de>
>> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
>> Tested-by: Pavel Machek <pavel at ucw.cz>
>> Acked-by: Christian König <christian.koenig at amd.com>
>> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Christoph Hellwig <hch at lst.de>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> Cc: <stable at 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 at 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);
>
> 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 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/20210226/1316db84/attachment.sig>
More information about the dri-devel
mailing list