Hi, thanks for reviewing. This will be fixed in the next iteration. Best regards Thomas Am 03.03.21 um 11:58 schrieb Noralf Trønnes:
Den 03.03.2021 09.45, skrev Thomas Zimmermann: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.This doesn't seem to be the case anymore.Tested by joining/mirroring displays of udl and radeon un der Gnome/X11.s/un der/under/v7: * fix use-before-init bug in gm12u320 (Dan) v6: * implement workaround in DRM drivers and hold reference to DMA device while USB device is in use * remove dev_is_usb() (Greg) * collapse USB helper into usb_intf_get_dma_device() (Alan) * integrate Daniel's TODO statement (Daniel) * fix typos (Greg) 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@xxxxxxx> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") Tested-by: Pavel Machek <pavel@xxxxxx> Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> Acked-by: Christian König <christian.koenig@xxxxxxx> Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Cc: Christoph Hellwig <hch@xxxxxx> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> Cc: <stable@xxxxxxxxxxxxxxx> # v5.10+ Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> --- Documentation/gpu/todo.rst | 21 +++++++++++++++++++++ drivers/gpu/drm/tiny/gm12u320.c | 28 ++++++++++++++++++++++++++-- drivers/gpu/drm/udl/udl_drv.c | 17 +++++++++++++++++ drivers/gpu/drm/udl/udl_drv.h | 1 + drivers/gpu/drm/udl/udl_main.c | 9 +++++++++ drivers/usb/core/usb.c | 32 ++++++++++++++++++++++++++++++++ include/linux/usb.h | 2 ++ 7 files changed, 108 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c index 0b4f4f2af1ef..4fe372f43cf5 100644 --- a/drivers/gpu/drm/tiny/gm12u320.c +++ b/drivers/gpu/drm/tiny/gm12u320.c[...]@@ -638,12 +656,15 @@ static int gm12u320_usb_probe(struct usb_interface *interface, struct gm12u320_device, dev); if (IS_ERR(gm12u320)) return PTR_ERR(gm12u320); + dev = &gm12u320->dev; + + gm12u320->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev)); + if (!gm12u320->dmadev) + drm_warn(dev, "buffer sharing not supported"); /* not an error */When implementing this in my own driver I discovered that this device ref will leak if probing fails after this. I've done it like this: gdrm->dmadev = usb_intf_get_dma_device(intf); if (!gdrm->dmadev) dev_warn(dev, "buffer sharing not supported"); ret = drm_dev_register(drm, 0); if (ret) { put_device(gdrm->dmadev); return ret; } An even better solution would be to have a devm_ version of the function. Noralf.INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work); mutex_init(&gm12u320->fb_update.lock);- dev = &gm12u320->dev;- ret = drmm_mode_config_init(dev); if (ret) return ret;
-- 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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
