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

From: Alan Stern <stern@rowland.harvard.edu>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: daniel@ffwll.ch, airlied@linux.ie,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	sumit.semwal@linaro.org, christian.koenig@amd.com,
	gregkh@linuxfoundation.org, hdegoede@redhat.com, sean@poorly.run,
	noralf@tronnes.org, dri-devel@lists.freedesktop.org,
	Christoph Hellwig <hch@lst.de>,
	stable@vger.kernel.org
Subject: Re: [PATCH v4] drm: Use USB controller's DMA mask when importing dmabufs
Date: Wed, 24 Feb 2021 10:21:53 -0500	[thread overview]
Message-ID: <20210224152153.GA1307460@rowland.harvard.edu> (raw)
In-Reply-To: <20210224092304.29932-1-tzimmermann@suse.de>

On Wed, Feb 24, 2021 at 10:23:04AM +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.
> 
> 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")
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # v5.10+
> ---

> +struct drm_gem_object *drm_gem_prime_import_usb(struct drm_device *dev,
> +						struct dma_buf *dma_buf)
> +{
> +	struct usb_device *udev;
> +	struct device *dmadev;
> +	struct drm_gem_object *obj;
> +
> +	if (!dev_is_usb(dev->dev))
> +		return ERR_PTR(-ENODEV);
> +	udev = interface_to_usbdev(to_usb_interface(dev->dev));
> +
> +	dmadev = usb_get_dma_device(udev);

You can do it this way if you want, but I think usb_get_dma_device would 
be easier to use if its argument was a pointer to struct usb_interface 
or (even better) a pointer to a usb_interface's embedded struct device.  
Then you wouldn't need to compute udev, and the same would be true for 
other callers.

Alan Stern