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 >> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices") >> Tested-by: Pavel Machek >> Reviewed-by: Greg Kroah-Hartman >> Acked-by: Christian König >> Acked-by: Daniel Vetter >> Cc: Christoph Hellwig >> Cc: Greg Kroah-Hartman >> Cc: # v5.10+ >> Signed-off-by: Thomas Zimmermann >> --- >> 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