[PATCH v3] drm: Use USB controller's DMA mask when importing dmabufs
Daniel Vetter
daniel at ffwll.ch
Tue Feb 23 12:47:30 UTC 2021
On Tue, Feb 23, 2021 at 1:37 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
>
> Hi
>
> Am 23.02.21 um 12:19 schrieb Greg KH:
> > On Tue, Feb 23, 2021 at 11:58:42AM +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;
> >> + if (!usbhost || !usbhost->dma_mask)
> >> + return ERR_PTR(-ENODEV);
> >
> > If individual USB drivers need access to this type of thing, shouldn't
> > that be done in the USB core itself?
> >
> > {hint, yes}
> >
> > There shouldn't be anything "special" about a DRM driver that needs this
> > vs. any other driver that might want to know about DMA things related to
> > a specific USB device. Why isn't this an issue with the existing
> > storage or v4l USB devices?
>
> I don't know about vc4 or storage. My guess is that they don't call
> dma_map_sgtable() for devices with dma_mask. Ideally, USB DRM devices
> wouldn't do that either, but, as Daniel explained, DRM's PRIME framework
> expects a dma_mask on the importing device.
>
> The real fix would move this from framework to drivers, so that each
> driver can import the dmabuf according to its capabilities. I tried to
> do this with v2 of this patch, but I was not feasible at this time.
>
> For this to work, we'd have rework at least 3 drivers, the PRIME
> framework and the dmabuf framework. I don't think the stable maintainer
> would be keen on merging that. ;)
>
> Wrt your question about the USB core: what we do here is a workaround
> for dmabuf importing. The DRM USB drivers don't even use the resulting
> page mapping directly. Putting the workaround into the USB core is maybe
> not useful. If we ever use DMA directly for streaming framebuffers to
> the device, thinks might be different.
Yeah if we ever do this (direct dma from dma-buf to usb devices) then
a clean interface would need the usb subsystem to take care of the
dma-buf attachment, dma setup and everything in the usb host driver.
So even in that hypothetical world, none of this is anything that usb
ever will want, we'd need an interface at a slightly higher level.
Essentially urb transfer function that can receive/send into/from
dma-buf.
-Daniel
>
> Best regards
> Thomas
>
> >
> > 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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list