All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
@ 2021-03-02 18:47 kernel test robot
  0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-03-02 18:47 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 9909 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210301093127.11028-1-tzimmermann@suse.de>
References: <20210301093127.11028-1-tzimmermann@suse.de>
TO: Thomas Zimmermann <tzimmermann@suse.de>
TO: daniel(a)ffwll.ch
TO: airlied(a)linux.ie
TO: maarten.lankhorst(a)linux.intel.com
TO: mripard(a)kernel.org
TO: sumit.semwal(a)linaro.org
TO: christian.koenig(a)amd.com
TO: gregkh(a)linuxfoundation.org
TO: hdegoede(a)redhat.com
TO: sean(a)poorly.run
TO: noralf(a)tronnes.org

Hi Thomas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Use-USB-controller-s-DMA-mask-when-importing-dmabufs/20210301-173210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
:::::: branch date: 33 hours ago
:::::: commit date: 33 hours ago
config: x86_64-randconfig-m001-20210302 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/tiny/gm12u320.c:662 gm12u320_usb_probe() error: potentially dereferencing uninitialized 'dev'.

vim +/dev +662 drivers/gpu/drm/tiny/gm12u320.c

e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  642  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  643  static int gm12u320_usb_probe(struct usb_interface *interface,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  644  			      const struct usb_device_id *id)
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  645  {
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  646  	struct gm12u320_device *gm12u320;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  647  	struct drm_device *dev;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  648  	int ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  649  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  650  	/*
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  651  	 * The gm12u320 presents itself to the system as 2 usb mass-storage
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  652  	 * interfaces, we only care about / need the first one.
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  653  	 */
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  654  	if (interface->cur_altsetting->desc.bInterfaceNumber != 0)
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  655  		return -ENODEV;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  656  
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  657  	gm12u320 = devm_drm_dev_alloc(&interface->dev, &gm12u320_drm_driver,
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  658  				      struct gm12u320_device, dev);
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  659  	if (IS_ERR(gm12u320))
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  660  		return PTR_ERR(gm12u320);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  661  
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01 @662  	gm12u320->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  663  	if (!gm12u320->dmadev)
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  664  		drm_warn(dev, "buffer sharing not supported"); /* not an error */
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  665  
8f2cb9379fb4ab drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  666  	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  667  	mutex_init(&gm12u320->fb_update.lock);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  668  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  669  	dev = &gm12u320->dev;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  670  
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  671  	ret = drmm_mode_config_init(dev);
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  672  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  673  		return ret;
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  674  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  675  	dev->mode_config.min_width = GM12U320_USER_WIDTH;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  676  	dev->mode_config.max_width = GM12U320_USER_WIDTH;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  677  	dev->mode_config.min_height = GM12U320_HEIGHT;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  678  	dev->mode_config.max_height = GM12U320_HEIGHT;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  679  	dev->mode_config.funcs = &gm12u320_mode_config_funcs;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  680  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  681  	ret = gm12u320_usb_alloc(gm12u320);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  682  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  683  		return ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  684  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  685  	ret = gm12u320_set_ecomode(gm12u320);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  686  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  687  		return ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  688  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  689  	ret = gm12u320_conn_init(gm12u320);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  690  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  691  		return ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  692  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  693  	ret = drm_simple_display_pipe_init(&gm12u320->dev,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  694  					   &gm12u320->pipe,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  695  					   &gm12u320_pipe_funcs,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  696  					   gm12u320_pipe_formats,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  697  					   ARRAY_SIZE(gm12u320_pipe_formats),
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  698  					   gm12u320_pipe_modifiers,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  699  					   &gm12u320->conn);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  700  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  701  		return ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  702  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  703  	drm_mode_config_reset(dev);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  704  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  705  	usb_set_intfdata(interface, dev);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  706  	ret = drm_dev_register(dev, 0);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  707  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  708  		return ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  709  
8515090ce51441 drivers/gpu/drm/tiny/gm12u320.c     Hans de Goede     2019-07-30  710  	drm_fbdev_generic_setup(dev, 0);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  711  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  712  	return 0;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  713  }
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  714  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36655 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
  2021-03-03  5:51   ` Dan Carpenter
  (?)
@ 2021-03-03  8:25   ` Thomas Zimmermann
  -1 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-03-03  8:25 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5261 bytes --]

Hi

Am 03.03.21 um 06:51 schrieb Dan Carpenter:
> Hi Thomas,
> 
> url:    https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Use-USB-controller-s-DMA-mask-when-importing-dmabufs/20210301-173210
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> config: x86_64-randconfig-m001-20210302 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> drivers/gpu/drm/tiny/gm12u320.c:662 gm12u320_usb_probe() error: potentially dereferencing uninitialized 'dev'.

Thanks for the bug report. This will be fixed in v7, but it's worrisome 
that building with W=1 doesn't show me a warning. :/

Best regards
Thomas

> 
> vim +/dev +662 drivers/gpu/drm/tiny/gm12u320.c
> 
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  643  static int gm12u320_usb_probe(struct usb_interface *interface,
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  644  			      const struct usb_device_id *id)
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  645  {
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  646  	struct gm12u320_device *gm12u320;
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  647  	struct drm_device *dev;
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  648  	int ret;
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  649
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  650  	/*
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  651  	 * The gm12u320 presents itself to the system as 2 usb mass-storage
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  652  	 * interfaces, we only care about / need the first one.
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  653  	 */
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  654  	if (interface->cur_altsetting->desc.bInterfaceNumber != 0)
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  655  		return -ENODEV;
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  656
> 9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  657  	gm12u320 = devm_drm_dev_alloc(&interface->dev, &gm12u320_drm_driver,
> 9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  658  				      struct gm12u320_device, dev);
> 9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  659  	if (IS_ERR(gm12u320))
> 9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  660  		return PTR_ERR(gm12u320);
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  661
> 6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01 @662  	gm12u320->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
>                                                                                                                                                      ^^^^^^^^
> "dev" isn't initalized until a few lines later.
> 
> 6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  663  	if (!gm12u320->dmadev)
> 6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  664  		drm_warn(dev, "buffer sharing not supported"); /* not an error */
> 6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  665
> 8f2cb9379fb4ab drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  666  	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  667  	mutex_init(&gm12u320->fb_update.lock);
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  668
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  669  	dev = &gm12u320->dev;
>                                                                                          ^^^^^^^^^^^^^^^^^^^^
> 
> e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  670
> 08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  671  	ret = drmm_mode_config_init(dev);
> 08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  672  	if (ret)
> 993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  673  		return ret;
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> 

-- 
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 #2: OpenPGP_signature.sig --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
  2021-03-01  9:31 ` Thomas Zimmermann
@ 2021-03-03  5:51   ` Dan Carpenter
  -1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2021-03-03  5:51 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]

Hi Thomas,

url:    https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Use-USB-controller-s-DMA-mask-when-importing-dmabufs/20210301-173210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-m001-20210302 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/tiny/gm12u320.c:662 gm12u320_usb_probe() error: potentially dereferencing uninitialized 'dev'.

vim +/dev +662 drivers/gpu/drm/tiny/gm12u320.c

e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  643  static int gm12u320_usb_probe(struct usb_interface *interface,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  644  			      const struct usb_device_id *id)
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  645  {
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  646  	struct gm12u320_device *gm12u320;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  647  	struct drm_device *dev;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  648  	int ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  649  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  650  	/*
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  651  	 * The gm12u320 presents itself to the system as 2 usb mass-storage
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  652  	 * interfaces, we only care about / need the first one.
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  653  	 */
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  654  	if (interface->cur_altsetting->desc.bInterfaceNumber != 0)
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  655  		return -ENODEV;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  656  
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  657  	gm12u320 = devm_drm_dev_alloc(&interface->dev, &gm12u320_drm_driver,
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  658  				      struct gm12u320_device, dev);
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  659  	if (IS_ERR(gm12u320))
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  660  		return PTR_ERR(gm12u320);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  661  
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01 @662  	gm12u320->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
                                                                                                                                                    ^^^^^^^^
"dev" isn't initalized until a few lines later.

6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  663  	if (!gm12u320->dmadev)
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  664  		drm_warn(dev, "buffer sharing not supported"); /* not an error */
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  665  
8f2cb9379fb4ab drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  666  	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  667  	mutex_init(&gm12u320->fb_update.lock);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  668  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  669  	dev = &gm12u320->dev;
                                                                                        ^^^^^^^^^^^^^^^^^^^^

e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  670  
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  671  	ret = drmm_mode_config_init(dev);
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  672  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  673  		return ret;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36655 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
@ 2021-03-03  5:51   ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2021-03-03  5:51 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]

Hi Thomas,

url:    https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Use-USB-controller-s-DMA-mask-when-importing-dmabufs/20210301-173210
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-m001-20210302 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/tiny/gm12u320.c:662 gm12u320_usb_probe() error: potentially dereferencing uninitialized 'dev'.

vim +/dev +662 drivers/gpu/drm/tiny/gm12u320.c

e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  643  static int gm12u320_usb_probe(struct usb_interface *interface,
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  644  			      const struct usb_device_id *id)
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  645  {
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  646  	struct gm12u320_device *gm12u320;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  647  	struct drm_device *dev;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  648  	int ret;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  649  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  650  	/*
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  651  	 * The gm12u320 presents itself to the system as 2 usb mass-storage
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  652  	 * interfaces, we only care about / need the first one.
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  653  	 */
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  654  	if (interface->cur_altsetting->desc.bInterfaceNumber != 0)
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  655  		return -ENODEV;
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  656  
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  657  	gm12u320 = devm_drm_dev_alloc(&interface->dev, &gm12u320_drm_driver,
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  658  				      struct gm12u320_device, dev);
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  659  	if (IS_ERR(gm12u320))
9213142d6b8c90 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-04-15  660  		return PTR_ERR(gm12u320);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  661  
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01 @662  	gm12u320->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
                                                                                                                                                    ^^^^^^^^
"dev" isn't initalized until a few lines later.

6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  663  	if (!gm12u320->dmadev)
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  664  		drm_warn(dev, "buffer sharing not supported"); /* not an error */
6ad12fb4c01449 drivers/gpu/drm/tiny/gm12u320.c     Thomas Zimmermann 2021-03-01  665  
8f2cb9379fb4ab drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  666  	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  667  	mutex_init(&gm12u320->fb_update.lock);
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  668  
e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  669  	dev = &gm12u320->dev;
                                                                                        ^^^^^^^^^^^^^^^^^^^^

e4f86e43716443 drivers/gpu/drm/gm12u320/gm12u320.c Hans de Goede     2019-07-21  670  
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  671  	ret = drmm_mode_config_init(dev);
08373edcb9a87e drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  672  	if (ret)
993f5b193d1455 drivers/gpu/drm/tiny/gm12u320.c     Daniel Vetter     2020-03-23  673  		return ret;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36655 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
  2021-03-01  9:31 ` Thomas Zimmermann
@ 2021-03-02  9:56   ` Greg KH
  -1 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2021-03-02  9:56 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: daniel, airlied, maarten.lankhorst, mripard, sumit.semwal,
	christian.koenig, hdegoede, sean, noralf, stern, dri-devel,
	Pavel Machek, Daniel Vetter, Christoph Hellwig, stable

On Mon, Mar 01, 2021 at 10:31:27AM +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.
> 
> 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@suse.de>
> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Christian König <christian.koenig@amd.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # v5.10+
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
@ 2021-03-02  9:56   ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2021-03-02  9:56 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Pavel Machek, airlied, Daniel Vetter, Christoph Hellwig,
	hdegoede, stern, dri-devel, stable, sean, christian.koenig

On Mon, Mar 01, 2021 at 10:31:27AM +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.
> 
> 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@suse.de>
> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Christian König <christian.koenig@amd.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # v5.10+
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
  2021-03-01  9:31 ` Thomas Zimmermann
@ 2021-03-02  9:45   ` Thomas Zimmermann
  -1 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-03-02  9:45 UTC (permalink / raw)
  To: daniel, airlied, maarten.lankhorst, mripard, sumit.semwal,
	christian.koenig, gregkh, hdegoede, sean, noralf, stern
  Cc: dri-devel, Pavel Machek, Daniel Vetter, Christoph Hellwig, stable


[-- Attachment #1.1: Type: text/plain, Size: 11231 bytes --]

Hi,

if there are no further comments on this patch, I intend to merge it via 
drm-misc.

Best regards
Thomas

Am 01.03.21 um 10:31 schrieb 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.
> 
> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11.
> 
> 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@suse.de>
> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Christian König <christian.koenig@amd.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # v5.10+
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   Documentation/gpu/todo.rst      | 21 +++++++++++++++++++++
>   drivers/gpu/drm/tiny/gm12u320.c | 25 +++++++++++++++++++++++++
>   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, 107 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 0631b9b323d5..fdfd6a1081ec 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -571,6 +571,27 @@ 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. drm_gem_prime_fd_to_handle() and
> +drm_gem_prime_handle_to_fd() require that importers call dma_buf_attach()
> +even if they never do actual device DMA, but only CPU access through
> +dma_buf_vmap(). This is a problem for USB devices, which do not support DMA
> +operations.
> +
> +To fix the issue, automatic page mappings should be removed from the
> +buffer-sharing code. Fixing this is a bit more involved, since the import/export
> +cache is also tied to &drm_gem_object.import_attach. Meanwhile we paper over
> +this problem for USB devices by fishing out the USB host controller device, as
> +long as that supports DMA. Otherwise importing can still needlessly fail.
> +
> +Contact: Thomas Zimmermann <tzimmermann@suse.de>, Daniel Vetter
> +
> +Level: Advanced
> +
> +
>   Better Testing
>   ==============
>   
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 0b4f4f2af1ef..48b5b06f70e8 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -84,6 +84,7 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright, more silent)");
>   
>   struct gm12u320_device {
>   	struct drm_device	         dev;
> +	struct device                   *dmadev;
>   	struct drm_simple_display_pipe   pipe;
>   	struct drm_connector	         conn;
>   	unsigned char                   *cmd_buf;
> @@ -599,6 +600,22 @@ static const uint64_t gm12u320_pipe_modifiers[] = {
>   	DRM_FORMAT_MOD_INVALID
>   };
>   
> +/*
> + * FIXME: Dma-buf sharing requires DMA support by the importing device.
> + *        This function is a workaround to make USB devices work as well.
> + *        See todo.rst for how to fix the issue in the dma-buf framework.
> + */
> +static struct drm_gem_object *gm12u320_gem_prime_import(struct drm_device *dev,
> +							struct dma_buf *dma_buf)
> +{
> +	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
> +
> +	if (!gm12u320->dmadev)
> +		return ERR_PTR(-ENODEV);
> +
> +	return drm_gem_prime_import_dev(dev, dma_buf, gm12u320->dmadev);
> +}
> +
>   DEFINE_DRM_GEM_FOPS(gm12u320_fops);
>   
>   static const struct drm_driver gm12u320_drm_driver = {
> @@ -612,6 +629,7 @@ static const struct drm_driver gm12u320_drm_driver = {
>   
>   	.fops		 = &gm12u320_fops,
>   	DRM_GEM_SHMEM_DRIVER_OPS,
> +	.gem_prime_import = gm12u320_gem_prime_import,
>   };
>   
>   static const struct drm_mode_config_funcs gm12u320_mode_config_funcs = {
> @@ -639,6 +657,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
>   	if (IS_ERR(gm12u320))
>   		return PTR_ERR(gm12u320);
>   
> +	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 */
> +
>   	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
>   	mutex_init(&gm12u320->fb_update.lock);
>   
> @@ -691,7 +713,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
>   static void gm12u320_usb_disconnect(struct usb_interface *interface)
>   {
>   	struct drm_device *dev = usb_get_intfdata(interface);
> +	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
>   
> +	put_device(gm12u320->dmadev);
> +	gm12u320->dmadev = NULL;
>   	drm_dev_unplug(dev);
>   	drm_atomic_helper_shutdown(dev);
>   }
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 9269092697d8..5703277c6f52 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -32,6 +32,22 @@ static int udl_usb_resume(struct usb_interface *interface)
>   	return drm_mode_config_helper_resume(dev);
>   }
>   
> +/*
> + * FIXME: Dma-buf sharing requires DMA support by the importing device.
> + *        This function is a workaround to make USB devices work as well.
> + *        See todo.rst for how to fix the issue in the dma-buf framework.
> + */
> +static struct drm_gem_object *udl_driver_gem_prime_import(struct drm_device *dev,
> +							  struct dma_buf *dma_buf)
> +{
> +	struct udl_device *udl = to_udl(dev);
> +
> +	if (!udl->dmadev)
> +		return ERR_PTR(-ENODEV);
> +
> +	return drm_gem_prime_import_dev(dev, dma_buf, udl->dmadev);
> +}
> +
>   DEFINE_DRM_GEM_FOPS(udl_driver_fops);
>   
>   static const struct drm_driver driver = {
> @@ -40,6 +56,7 @@ static const struct drm_driver driver = {
>   	/* GEM hooks */
>   	.fops = &udl_driver_fops,
>   	DRM_GEM_SHMEM_DRIVER_OPS,
> +	.gem_prime_import = udl_driver_gem_prime_import,
>   
>   	.name = DRIVER_NAME,
>   	.desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 875e73551ae9..cc16a13316e4 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -50,6 +50,7 @@ struct urb_list {
>   struct udl_device {
>   	struct drm_device drm;
>   	struct device *dev;
> +	struct device *dmadev;
>   
>   	struct drm_simple_display_pipe display_pipe;
>   
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 0e2a376cb075..7c0338bcadea 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -315,6 +315,10 @@ int udl_init(struct udl_device *udl)
>   
>   	DRM_DEBUG("\n");
>   
> +	udl->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
> +	if (!udl->dmadev)
> +		drm_warn(dev, "buffer sharing not supported"); /* not an error */
> +
>   	mutex_init(&udl->gem_lock);
>   
>   	if (!udl_parse_vendor_descriptor(udl)) {
> @@ -349,6 +353,11 @@ int udl_init(struct udl_device *udl)
>   
>   int udl_drop_usb(struct drm_device *dev)
>   {
> +	struct udl_device *udl = to_udl(dev);
> +
>   	udl_free_urb_list(dev);
> +	put_device(udl->dmadev);
> +	udl->dmadev = NULL;
> +
>   	return 0;
>   }
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 8f07b0516100..a566bb494e24 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -748,6 +748,38 @@ void usb_put_intf(struct usb_interface *intf)
>   }
>   EXPORT_SYMBOL_GPL(usb_put_intf);
>   
> +/**
> + * usb_intf_get_dma_device - acquire a reference on the usb interface's DMA endpoint
> + * @intf: the usb interface
> + *
> + * While a USB device cannot perform DMA operations by itself, many USB
> + * controllers can. A call to usb_intf_get_dma_device() returns the DMA endpoint
> + * for the given USB interface, if any. The returned device structure must be
> + * released with put_device().
> + *
> + * See also usb_get_dma_device().
> + *
> + * Returns: A reference to the usb interface's DMA endpoint; or NULL if none
> + *          exists.
> + */
> +struct device *usb_intf_get_dma_device(struct usb_interface *intf)
> +{
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	struct device *dmadev;
> +
> +	if (!udev->bus)
> +		return NULL;
> +
> +	dmadev = get_device(udev->bus->sysdev);
> +	if (!dmadev || !dmadev->dma_mask) {
> +		put_device(dmadev);
> +		return NULL;
> +	}
> +
> +	return dmadev;
> +}
> +EXPORT_SYMBOL_GPL(usb_intf_get_dma_device);
> +
>   /*			USB device locking
>    *
>    * USB devices and interfaces are locked using the semaphore in their
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 7d72c4e0713c..d6a41841b93e 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -746,6 +746,8 @@ extern int usb_lock_device_for_reset(struct usb_device *udev,
>   extern int usb_reset_device(struct usb_device *dev);
>   extern void usb_queue_reset_device(struct usb_interface *dev);
>   
> +extern struct device *usb_intf_get_dma_device(struct usb_interface *intf);
> +
>   #ifdef CONFIG_ACPI
>   extern int usb_acpi_set_power_state(struct usb_device *hdev, int index,
>   	bool enable);
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
@ 2021-03-02  9:45   ` Thomas Zimmermann
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-03-02  9:45 UTC (permalink / raw)
  To: daniel, airlied, maarten.lankhorst, mripard, sumit.semwal,
	christian.koenig, gregkh, hdegoede, sean, noralf, stern
  Cc: Daniel Vetter, stable, Pavel Machek, dri-devel, Christoph Hellwig


[-- Attachment #1.1.1: Type: text/plain, Size: 11231 bytes --]

Hi,

if there are no further comments on this patch, I intend to merge it via 
drm-misc.

Best regards
Thomas

Am 01.03.21 um 10:31 schrieb 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.
> 
> Tested by joining/mirroring displays of udl and radeon un der Gnome/X11.
> 
> 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@suse.de>
> Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Christian König <christian.koenig@amd.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # v5.10+
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   Documentation/gpu/todo.rst      | 21 +++++++++++++++++++++
>   drivers/gpu/drm/tiny/gm12u320.c | 25 +++++++++++++++++++++++++
>   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, 107 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 0631b9b323d5..fdfd6a1081ec 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -571,6 +571,27 @@ 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. drm_gem_prime_fd_to_handle() and
> +drm_gem_prime_handle_to_fd() require that importers call dma_buf_attach()
> +even if they never do actual device DMA, but only CPU access through
> +dma_buf_vmap(). This is a problem for USB devices, which do not support DMA
> +operations.
> +
> +To fix the issue, automatic page mappings should be removed from the
> +buffer-sharing code. Fixing this is a bit more involved, since the import/export
> +cache is also tied to &drm_gem_object.import_attach. Meanwhile we paper over
> +this problem for USB devices by fishing out the USB host controller device, as
> +long as that supports DMA. Otherwise importing can still needlessly fail.
> +
> +Contact: Thomas Zimmermann <tzimmermann@suse.de>, Daniel Vetter
> +
> +Level: Advanced
> +
> +
>   Better Testing
>   ==============
>   
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 0b4f4f2af1ef..48b5b06f70e8 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -84,6 +84,7 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright, more silent)");
>   
>   struct gm12u320_device {
>   	struct drm_device	         dev;
> +	struct device                   *dmadev;
>   	struct drm_simple_display_pipe   pipe;
>   	struct drm_connector	         conn;
>   	unsigned char                   *cmd_buf;
> @@ -599,6 +600,22 @@ static const uint64_t gm12u320_pipe_modifiers[] = {
>   	DRM_FORMAT_MOD_INVALID
>   };
>   
> +/*
> + * FIXME: Dma-buf sharing requires DMA support by the importing device.
> + *        This function is a workaround to make USB devices work as well.
> + *        See todo.rst for how to fix the issue in the dma-buf framework.
> + */
> +static struct drm_gem_object *gm12u320_gem_prime_import(struct drm_device *dev,
> +							struct dma_buf *dma_buf)
> +{
> +	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
> +
> +	if (!gm12u320->dmadev)
> +		return ERR_PTR(-ENODEV);
> +
> +	return drm_gem_prime_import_dev(dev, dma_buf, gm12u320->dmadev);
> +}
> +
>   DEFINE_DRM_GEM_FOPS(gm12u320_fops);
>   
>   static const struct drm_driver gm12u320_drm_driver = {
> @@ -612,6 +629,7 @@ static const struct drm_driver gm12u320_drm_driver = {
>   
>   	.fops		 = &gm12u320_fops,
>   	DRM_GEM_SHMEM_DRIVER_OPS,
> +	.gem_prime_import = gm12u320_gem_prime_import,
>   };
>   
>   static const struct drm_mode_config_funcs gm12u320_mode_config_funcs = {
> @@ -639,6 +657,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
>   	if (IS_ERR(gm12u320))
>   		return PTR_ERR(gm12u320);
>   
> +	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 */
> +
>   	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
>   	mutex_init(&gm12u320->fb_update.lock);
>   
> @@ -691,7 +713,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
>   static void gm12u320_usb_disconnect(struct usb_interface *interface)
>   {
>   	struct drm_device *dev = usb_get_intfdata(interface);
> +	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
>   
> +	put_device(gm12u320->dmadev);
> +	gm12u320->dmadev = NULL;
>   	drm_dev_unplug(dev);
>   	drm_atomic_helper_shutdown(dev);
>   }
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 9269092697d8..5703277c6f52 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -32,6 +32,22 @@ static int udl_usb_resume(struct usb_interface *interface)
>   	return drm_mode_config_helper_resume(dev);
>   }
>   
> +/*
> + * FIXME: Dma-buf sharing requires DMA support by the importing device.
> + *        This function is a workaround to make USB devices work as well.
> + *        See todo.rst for how to fix the issue in the dma-buf framework.
> + */
> +static struct drm_gem_object *udl_driver_gem_prime_import(struct drm_device *dev,
> +							  struct dma_buf *dma_buf)
> +{
> +	struct udl_device *udl = to_udl(dev);
> +
> +	if (!udl->dmadev)
> +		return ERR_PTR(-ENODEV);
> +
> +	return drm_gem_prime_import_dev(dev, dma_buf, udl->dmadev);
> +}
> +
>   DEFINE_DRM_GEM_FOPS(udl_driver_fops);
>   
>   static const struct drm_driver driver = {
> @@ -40,6 +56,7 @@ static const struct drm_driver driver = {
>   	/* GEM hooks */
>   	.fops = &udl_driver_fops,
>   	DRM_GEM_SHMEM_DRIVER_OPS,
> +	.gem_prime_import = udl_driver_gem_prime_import,
>   
>   	.name = DRIVER_NAME,
>   	.desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 875e73551ae9..cc16a13316e4 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -50,6 +50,7 @@ struct urb_list {
>   struct udl_device {
>   	struct drm_device drm;
>   	struct device *dev;
> +	struct device *dmadev;
>   
>   	struct drm_simple_display_pipe display_pipe;
>   
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index 0e2a376cb075..7c0338bcadea 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -315,6 +315,10 @@ int udl_init(struct udl_device *udl)
>   
>   	DRM_DEBUG("\n");
>   
> +	udl->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
> +	if (!udl->dmadev)
> +		drm_warn(dev, "buffer sharing not supported"); /* not an error */
> +
>   	mutex_init(&udl->gem_lock);
>   
>   	if (!udl_parse_vendor_descriptor(udl)) {
> @@ -349,6 +353,11 @@ int udl_init(struct udl_device *udl)
>   
>   int udl_drop_usb(struct drm_device *dev)
>   {
> +	struct udl_device *udl = to_udl(dev);
> +
>   	udl_free_urb_list(dev);
> +	put_device(udl->dmadev);
> +	udl->dmadev = NULL;
> +
>   	return 0;
>   }
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 8f07b0516100..a566bb494e24 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -748,6 +748,38 @@ void usb_put_intf(struct usb_interface *intf)
>   }
>   EXPORT_SYMBOL_GPL(usb_put_intf);
>   
> +/**
> + * usb_intf_get_dma_device - acquire a reference on the usb interface's DMA endpoint
> + * @intf: the usb interface
> + *
> + * While a USB device cannot perform DMA operations by itself, many USB
> + * controllers can. A call to usb_intf_get_dma_device() returns the DMA endpoint
> + * for the given USB interface, if any. The returned device structure must be
> + * released with put_device().
> + *
> + * See also usb_get_dma_device().
> + *
> + * Returns: A reference to the usb interface's DMA endpoint; or NULL if none
> + *          exists.
> + */
> +struct device *usb_intf_get_dma_device(struct usb_interface *intf)
> +{
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	struct device *dmadev;
> +
> +	if (!udev->bus)
> +		return NULL;
> +
> +	dmadev = get_device(udev->bus->sysdev);
> +	if (!dmadev || !dmadev->dma_mask) {
> +		put_device(dmadev);
> +		return NULL;
> +	}
> +
> +	return dmadev;
> +}
> +EXPORT_SYMBOL_GPL(usb_intf_get_dma_device);
> +
>   /*			USB device locking
>    *
>    * USB devices and interfaces are locked using the semaphore in their
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 7d72c4e0713c..d6a41841b93e 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -746,6 +746,8 @@ extern int usb_lock_device_for_reset(struct usb_device *udev,
>   extern int usb_reset_device(struct usb_device *dev);
>   extern void usb_queue_reset_device(struct usb_interface *dev);
>   
> +extern struct device *usb_intf_get_dma_device(struct usb_interface *intf);
> +
>   #ifdef CONFIG_ACPI
>   extern int usb_acpi_set_power_state(struct usb_device *hdev, int index,
>   	bool enable);
> 

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
@ 2021-03-01  9:31 ` Thomas Zimmermann
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-03-01  9:31 UTC (permalink / raw)
  To: daniel, airlied, maarten.lankhorst, mripard, sumit.semwal,
	christian.koenig, gregkh, hdegoede, sean, noralf, stern
  Cc: dri-devel, Thomas Zimmermann, Pavel Machek, Daniel Vetter,
	Christoph Hellwig, stable

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.

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@suse.de>
Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
Tested-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Christian König <christian.koenig@amd.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <stable@vger.kernel.org> # v5.10+
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/gpu/todo.rst      | 21 +++++++++++++++++++++
 drivers/gpu/drm/tiny/gm12u320.c | 25 +++++++++++++++++++++++++
 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, 107 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 0631b9b323d5..fdfd6a1081ec 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -571,6 +571,27 @@ 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. drm_gem_prime_fd_to_handle() and
+drm_gem_prime_handle_to_fd() require that importers call dma_buf_attach()
+even if they never do actual device DMA, but only CPU access through
+dma_buf_vmap(). This is a problem for USB devices, which do not support DMA
+operations.
+
+To fix the issue, automatic page mappings should be removed from the
+buffer-sharing code. Fixing this is a bit more involved, since the import/export
+cache is also tied to &drm_gem_object.import_attach. Meanwhile we paper over
+this problem for USB devices by fishing out the USB host controller device, as
+long as that supports DMA. Otherwise importing can still needlessly fail.
+
+Contact: Thomas Zimmermann <tzimmermann@suse.de>, Daniel Vetter
+
+Level: Advanced
+
+
 Better Testing
 ==============
 
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 0b4f4f2af1ef..48b5b06f70e8 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -84,6 +84,7 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright, more silent)");
 
 struct gm12u320_device {
 	struct drm_device	         dev;
+	struct device                   *dmadev;
 	struct drm_simple_display_pipe   pipe;
 	struct drm_connector	         conn;
 	unsigned char                   *cmd_buf;
@@ -599,6 +600,22 @@ static const uint64_t gm12u320_pipe_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
+/*
+ * FIXME: Dma-buf sharing requires DMA support by the importing device.
+ *        This function is a workaround to make USB devices work as well.
+ *        See todo.rst for how to fix the issue in the dma-buf framework.
+ */
+static struct drm_gem_object *gm12u320_gem_prime_import(struct drm_device *dev,
+							struct dma_buf *dma_buf)
+{
+	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
+
+	if (!gm12u320->dmadev)
+		return ERR_PTR(-ENODEV);
+
+	return drm_gem_prime_import_dev(dev, dma_buf, gm12u320->dmadev);
+}
+
 DEFINE_DRM_GEM_FOPS(gm12u320_fops);
 
 static const struct drm_driver gm12u320_drm_driver = {
@@ -612,6 +629,7 @@ static const struct drm_driver gm12u320_drm_driver = {
 
 	.fops		 = &gm12u320_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
+	.gem_prime_import = gm12u320_gem_prime_import,
 };
 
 static const struct drm_mode_config_funcs gm12u320_mode_config_funcs = {
@@ -639,6 +657,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 	if (IS_ERR(gm12u320))
 		return PTR_ERR(gm12u320);
 
+	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 */
+
 	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
 	mutex_init(&gm12u320->fb_update.lock);
 
@@ -691,7 +713,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 static void gm12u320_usb_disconnect(struct usb_interface *interface)
 {
 	struct drm_device *dev = usb_get_intfdata(interface);
+	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
 
+	put_device(gm12u320->dmadev);
+	gm12u320->dmadev = NULL;
 	drm_dev_unplug(dev);
 	drm_atomic_helper_shutdown(dev);
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 9269092697d8..5703277c6f52 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -32,6 +32,22 @@ static int udl_usb_resume(struct usb_interface *interface)
 	return drm_mode_config_helper_resume(dev);
 }
 
+/*
+ * FIXME: Dma-buf sharing requires DMA support by the importing device.
+ *        This function is a workaround to make USB devices work as well.
+ *        See todo.rst for how to fix the issue in the dma-buf framework.
+ */
+static struct drm_gem_object *udl_driver_gem_prime_import(struct drm_device *dev,
+							  struct dma_buf *dma_buf)
+{
+	struct udl_device *udl = to_udl(dev);
+
+	if (!udl->dmadev)
+		return ERR_PTR(-ENODEV);
+
+	return drm_gem_prime_import_dev(dev, dma_buf, udl->dmadev);
+}
+
 DEFINE_DRM_GEM_FOPS(udl_driver_fops);
 
 static const struct drm_driver driver = {
@@ -40,6 +56,7 @@ static const struct drm_driver driver = {
 	/* GEM hooks */
 	.fops = &udl_driver_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
+	.gem_prime_import = udl_driver_gem_prime_import,
 
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 875e73551ae9..cc16a13316e4 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -50,6 +50,7 @@ struct urb_list {
 struct udl_device {
 	struct drm_device drm;
 	struct device *dev;
+	struct device *dmadev;
 
 	struct drm_simple_display_pipe display_pipe;
 
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 0e2a376cb075..7c0338bcadea 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -315,6 +315,10 @@ int udl_init(struct udl_device *udl)
 
 	DRM_DEBUG("\n");
 
+	udl->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
+	if (!udl->dmadev)
+		drm_warn(dev, "buffer sharing not supported"); /* not an error */
+
 	mutex_init(&udl->gem_lock);
 
 	if (!udl_parse_vendor_descriptor(udl)) {
@@ -349,6 +353,11 @@ int udl_init(struct udl_device *udl)
 
 int udl_drop_usb(struct drm_device *dev)
 {
+	struct udl_device *udl = to_udl(dev);
+
 	udl_free_urb_list(dev);
+	put_device(udl->dmadev);
+	udl->dmadev = NULL;
+
 	return 0;
 }
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8f07b0516100..a566bb494e24 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -748,6 +748,38 @@ void usb_put_intf(struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(usb_put_intf);
 
+/**
+ * usb_intf_get_dma_device - acquire a reference on the usb interface's DMA endpoint
+ * @intf: the usb interface
+ *
+ * While a USB device cannot perform DMA operations by itself, many USB
+ * controllers can. A call to usb_intf_get_dma_device() returns the DMA endpoint
+ * for the given USB interface, if any. The returned device structure must be
+ * released with put_device().
+ *
+ * See also usb_get_dma_device().
+ *
+ * Returns: A reference to the usb interface's DMA endpoint; or NULL if none
+ *          exists.
+ */
+struct device *usb_intf_get_dma_device(struct usb_interface *intf)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct device *dmadev;
+
+	if (!udev->bus)
+		return NULL;
+
+	dmadev = get_device(udev->bus->sysdev);
+	if (!dmadev || !dmadev->dma_mask) {
+		put_device(dmadev);
+		return NULL;
+	}
+
+	return dmadev;
+}
+EXPORT_SYMBOL_GPL(usb_intf_get_dma_device);
+
 /*			USB device locking
  *
  * USB devices and interfaces are locked using the semaphore in their
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7d72c4e0713c..d6a41841b93e 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -746,6 +746,8 @@ extern int usb_lock_device_for_reset(struct usb_device *udev,
 extern int usb_reset_device(struct usb_device *dev);
 extern void usb_queue_reset_device(struct usb_interface *dev);
 
+extern struct device *usb_intf_get_dma_device(struct usb_interface *intf);
+
 #ifdef CONFIG_ACPI
 extern int usb_acpi_set_power_state(struct usb_device *hdev, int index,
 	bool enable);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs
@ 2021-03-01  9:31 ` Thomas Zimmermann
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2021-03-01  9:31 UTC (permalink / raw)
  To: daniel, airlied, maarten.lankhorst, mripard, sumit.semwal,
	christian.koenig, gregkh, hdegoede, sean, noralf, stern
  Cc: Thomas Zimmermann, Daniel Vetter, stable, dri-devel,
	Pavel Machek, Christoph Hellwig

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.

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@suse.de>
Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
Tested-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Christian König <christian.koenig@amd.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <stable@vger.kernel.org> # v5.10+
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/gpu/todo.rst      | 21 +++++++++++++++++++++
 drivers/gpu/drm/tiny/gm12u320.c | 25 +++++++++++++++++++++++++
 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, 107 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 0631b9b323d5..fdfd6a1081ec 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -571,6 +571,27 @@ 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. drm_gem_prime_fd_to_handle() and
+drm_gem_prime_handle_to_fd() require that importers call dma_buf_attach()
+even if they never do actual device DMA, but only CPU access through
+dma_buf_vmap(). This is a problem for USB devices, which do not support DMA
+operations.
+
+To fix the issue, automatic page mappings should be removed from the
+buffer-sharing code. Fixing this is a bit more involved, since the import/export
+cache is also tied to &drm_gem_object.import_attach. Meanwhile we paper over
+this problem for USB devices by fishing out the USB host controller device, as
+long as that supports DMA. Otherwise importing can still needlessly fail.
+
+Contact: Thomas Zimmermann <tzimmermann@suse.de>, Daniel Vetter
+
+Level: Advanced
+
+
 Better Testing
 ==============
 
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 0b4f4f2af1ef..48b5b06f70e8 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -84,6 +84,7 @@ MODULE_PARM_DESC(eco_mode, "Turn on Eco mode (less bright, more silent)");
 
 struct gm12u320_device {
 	struct drm_device	         dev;
+	struct device                   *dmadev;
 	struct drm_simple_display_pipe   pipe;
 	struct drm_connector	         conn;
 	unsigned char                   *cmd_buf;
@@ -599,6 +600,22 @@ static const uint64_t gm12u320_pipe_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
+/*
+ * FIXME: Dma-buf sharing requires DMA support by the importing device.
+ *        This function is a workaround to make USB devices work as well.
+ *        See todo.rst for how to fix the issue in the dma-buf framework.
+ */
+static struct drm_gem_object *gm12u320_gem_prime_import(struct drm_device *dev,
+							struct dma_buf *dma_buf)
+{
+	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
+
+	if (!gm12u320->dmadev)
+		return ERR_PTR(-ENODEV);
+
+	return drm_gem_prime_import_dev(dev, dma_buf, gm12u320->dmadev);
+}
+
 DEFINE_DRM_GEM_FOPS(gm12u320_fops);
 
 static const struct drm_driver gm12u320_drm_driver = {
@@ -612,6 +629,7 @@ static const struct drm_driver gm12u320_drm_driver = {
 
 	.fops		 = &gm12u320_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
+	.gem_prime_import = gm12u320_gem_prime_import,
 };
 
 static const struct drm_mode_config_funcs gm12u320_mode_config_funcs = {
@@ -639,6 +657,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 	if (IS_ERR(gm12u320))
 		return PTR_ERR(gm12u320);
 
+	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 */
+
 	INIT_DELAYED_WORK(&gm12u320->fb_update.work, gm12u320_fb_update_work);
 	mutex_init(&gm12u320->fb_update.lock);
 
@@ -691,7 +713,10 @@ static int gm12u320_usb_probe(struct usb_interface *interface,
 static void gm12u320_usb_disconnect(struct usb_interface *interface)
 {
 	struct drm_device *dev = usb_get_intfdata(interface);
+	struct gm12u320_device *gm12u320 = to_gm12u320(dev);
 
+	put_device(gm12u320->dmadev);
+	gm12u320->dmadev = NULL;
 	drm_dev_unplug(dev);
 	drm_atomic_helper_shutdown(dev);
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 9269092697d8..5703277c6f52 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -32,6 +32,22 @@ static int udl_usb_resume(struct usb_interface *interface)
 	return drm_mode_config_helper_resume(dev);
 }
 
+/*
+ * FIXME: Dma-buf sharing requires DMA support by the importing device.
+ *        This function is a workaround to make USB devices work as well.
+ *        See todo.rst for how to fix the issue in the dma-buf framework.
+ */
+static struct drm_gem_object *udl_driver_gem_prime_import(struct drm_device *dev,
+							  struct dma_buf *dma_buf)
+{
+	struct udl_device *udl = to_udl(dev);
+
+	if (!udl->dmadev)
+		return ERR_PTR(-ENODEV);
+
+	return drm_gem_prime_import_dev(dev, dma_buf, udl->dmadev);
+}
+
 DEFINE_DRM_GEM_FOPS(udl_driver_fops);
 
 static const struct drm_driver driver = {
@@ -40,6 +56,7 @@ static const struct drm_driver driver = {
 	/* GEM hooks */
 	.fops = &udl_driver_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
+	.gem_prime_import = udl_driver_gem_prime_import,
 
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 875e73551ae9..cc16a13316e4 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -50,6 +50,7 @@ struct urb_list {
 struct udl_device {
 	struct drm_device drm;
 	struct device *dev;
+	struct device *dmadev;
 
 	struct drm_simple_display_pipe display_pipe;
 
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 0e2a376cb075..7c0338bcadea 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -315,6 +315,10 @@ int udl_init(struct udl_device *udl)
 
 	DRM_DEBUG("\n");
 
+	udl->dmadev = usb_intf_get_dma_device(to_usb_interface(dev->dev));
+	if (!udl->dmadev)
+		drm_warn(dev, "buffer sharing not supported"); /* not an error */
+
 	mutex_init(&udl->gem_lock);
 
 	if (!udl_parse_vendor_descriptor(udl)) {
@@ -349,6 +353,11 @@ int udl_init(struct udl_device *udl)
 
 int udl_drop_usb(struct drm_device *dev)
 {
+	struct udl_device *udl = to_udl(dev);
+
 	udl_free_urb_list(dev);
+	put_device(udl->dmadev);
+	udl->dmadev = NULL;
+
 	return 0;
 }
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8f07b0516100..a566bb494e24 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -748,6 +748,38 @@ void usb_put_intf(struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(usb_put_intf);
 
+/**
+ * usb_intf_get_dma_device - acquire a reference on the usb interface's DMA endpoint
+ * @intf: the usb interface
+ *
+ * While a USB device cannot perform DMA operations by itself, many USB
+ * controllers can. A call to usb_intf_get_dma_device() returns the DMA endpoint
+ * for the given USB interface, if any. The returned device structure must be
+ * released with put_device().
+ *
+ * See also usb_get_dma_device().
+ *
+ * Returns: A reference to the usb interface's DMA endpoint; or NULL if none
+ *          exists.
+ */
+struct device *usb_intf_get_dma_device(struct usb_interface *intf)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct device *dmadev;
+
+	if (!udev->bus)
+		return NULL;
+
+	dmadev = get_device(udev->bus->sysdev);
+	if (!dmadev || !dmadev->dma_mask) {
+		put_device(dmadev);
+		return NULL;
+	}
+
+	return dmadev;
+}
+EXPORT_SYMBOL_GPL(usb_intf_get_dma_device);
+
 /*			USB device locking
  *
  * USB devices and interfaces are locked using the semaphore in their
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7d72c4e0713c..d6a41841b93e 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -746,6 +746,8 @@ extern int usb_lock_device_for_reset(struct usb_device *udev,
 extern int usb_reset_device(struct usb_device *dev);
 extern void usb_queue_reset_device(struct usb_interface *dev);
 
+extern struct device *usb_intf_get_dma_device(struct usb_interface *intf);
+
 #ifdef CONFIG_ACPI
 extern int usb_acpi_set_power_state(struct usb_device *hdev, int index,
 	bool enable);
-- 
2.30.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-03  8:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 18:47 [PATCH v6] drm: Use USB controller's DMA mask when importing dmabufs kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2021-03-01  9:31 Thomas Zimmermann
2021-03-01  9:31 ` Thomas Zimmermann
2021-03-02  9:45 ` Thomas Zimmermann
2021-03-02  9:45   ` Thomas Zimmermann
2021-03-02  9:56 ` Greg KH
2021-03-02  9:56   ` Greg KH
2021-03-03  5:51 ` Dan Carpenter
2021-03-03  5:51   ` Dan Carpenter
2021-03-03  8:25   ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.