| Message ID | 20250228094457.239442-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm: Provide a dedicated DMA device for PRIME import | expand |
On Fri, 28 Feb 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Importing dma-bufs via PRIME requires a DMA-capable device. Devices on > peripheral busses, such as USB, often cannot perform DMA by themselves. > Without DMA-capable device PRIME import fails. DRM drivers for USB > devices already use a separate DMA device for dma-buf imports. Make the > mechanism generally available. > > Add the field dma_dev to struct drm_device to refer to the device's DMA > device. For USB this should be the USB controller. Use dma_dev in the > PRIME import helpers, if set. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_drv.c | 2 ++ > drivers/gpu/drm/drm_prime.c | 2 +- > include/drm/drm_device.h | 37 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 17fc5dc708f4..f8c3c9f77d22 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -654,6 +654,8 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) > { > drm_fs_inode_free(dev->anon_inode); > > + put_device(dev->dma_dev); > + dev->dma_dev = NULL; The asymmetry in who gets and puts the ->dma_dev bugs me. You could avoid that by providing a helper for setting ->dma_dev, and having that get the device: int drm_dev_set_dma_device(struct drm_device *drm, struct device *dma_device) { dev->dma_dev = get_device(dma_device); return dev->dma_dev ? 0 : -ENODEV. } Taking drm/gm12u320 from patch 2 as an example, it would become: dma_device = usb_intf_get_dma_device(to_usb_interface(dev->dev)) if (drm_dev_set_dma_device(drm, dma_device)) drm_warn(dev, "buffer sharing not supported"); /* not an error */ put_device(dma_device); Sligthly more verbose, but has a get/put pair in the driver, and a get/put pair in the core, instead of the asymmetry. I'm not insisting, but something to consider. BR, Jani. > put_device(dev->dev); > /* Prevent use-after-free in drm_managed_release when debugging is > * enabled. Slightly awkward, but can't really be helped. */ > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 32a8781cfd67..258858f2f8dd 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -1004,7 +1004,7 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); > struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, > struct dma_buf *dma_buf) > { > - return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); > + return drm_gem_prime_import_dev(dev, dma_buf, drm_dev_dma_dev(dev)); > } > EXPORT_SYMBOL(drm_gem_prime_import); > > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index 6ea54a578cda..a24cac4b2077 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -64,6 +64,23 @@ struct drm_device { > /** @dev: Device structure of bus-device */ > struct device *dev; > > + /** > + * @dma_dev: > + * > + * Device for DMA operations. Only required if the device @dev > + * cannot perform DMA by itself. Should be NULL otherwise. > + * > + * Devices on USB and other peripheral busses cannot perform DMA > + * by themselves. The @dma_dev field should point the bus controller > + * that does DMA on behalve of such a device. Required for importing > + * buffers via dma-buf. > + * > + * If set, the DRM driver has to acquire a reference on the DMA > + * device, which will be owned and released automatically by the > + * DRM core. > + */ > + struct device *dma_dev; > + > /** > * @managed: > * > @@ -327,4 +344,24 @@ struct drm_device { > struct dentry *debugfs_root; > }; > > +/** > + * drm_dev_dma_dev - returns the DMA device for a DRM device > + * @dev: DRM device > + * > + * Returns the DMA device of the given DRM device. This is usually > + * the same as the DRM device's parent. Devices on some peripheral > + * busses, such as USB, are incapable of performing DMA by themselves. > + * Drivers should set struct &drm_device.dma_dev to the bus controller > + * to make DMA work. Required for PRIME buffer import. > + * > + * Returns: > + * A DMA-capable device for the DRM device. > + */ > +static inline struct device *drm_dev_dma_dev(struct drm_device *dev) > +{ > + if (dev->dma_dev) > + return dev->dma_dev; > + return dev->dev; > +} > + > #endif
Hi Am 28.02.25 um 12:21 schrieb Jani Nikula: > On Fri, 28 Feb 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Importing dma-bufs via PRIME requires a DMA-capable device. Devices on >> peripheral busses, such as USB, often cannot perform DMA by themselves. >> Without DMA-capable device PRIME import fails. DRM drivers for USB >> devices already use a separate DMA device for dma-buf imports. Make the >> mechanism generally available. >> >> Add the field dma_dev to struct drm_device to refer to the device's DMA >> device. For USB this should be the USB controller. Use dma_dev in the >> PRIME import helpers, if set. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/drm_drv.c | 2 ++ >> drivers/gpu/drm/drm_prime.c | 2 +- >> include/drm/drm_device.h | 37 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 40 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index 17fc5dc708f4..f8c3c9f77d22 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -654,6 +654,8 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) >> { >> drm_fs_inode_free(dev->anon_inode); >> >> + put_device(dev->dma_dev); >> + dev->dma_dev = NULL; > The asymmetry in who gets and puts the ->dma_dev bugs me. > > You could avoid that by providing a helper for setting ->dma_dev, and > having that get the device: > > int drm_dev_set_dma_device(struct drm_device *drm, struct device *dma_device) > { > dev->dma_dev = get_device(dma_device); > > return dev->dma_dev ? 0 : -ENODEV. > } > > Taking drm/gm12u320 from patch 2 as an example, it would become: > > dma_device = usb_intf_get_dma_device(to_usb_interface(dev->dev)) > if (drm_dev_set_dma_device(drm, dma_device)) > drm_warn(dev, "buffer sharing not supported"); /* not an error */ > put_device(dma_device); > > Sligthly more verbose, but has a get/put pair in the driver, and a > get/put pair in the core, instead of the asymmetry. > > I'm not insisting, but something to consider. We're doing a get_device() on the regular parent kept in dev. So yeah, keeping a ref on the dma_dev would make sense. Best regards Thomas > > > BR, > Jani. > > >> put_device(dev->dev); >> /* Prevent use-after-free in drm_managed_release when debugging is >> * enabled. Slightly awkward, but can't really be helped. */ >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 32a8781cfd67..258858f2f8dd 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -1004,7 +1004,7 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); >> struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, >> struct dma_buf *dma_buf) >> { >> - return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); >> + return drm_gem_prime_import_dev(dev, dma_buf, drm_dev_dma_dev(dev)); >> } >> EXPORT_SYMBOL(drm_gem_prime_import); >> >> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h >> index 6ea54a578cda..a24cac4b2077 100644 >> --- a/include/drm/drm_device.h >> +++ b/include/drm/drm_device.h >> @@ -64,6 +64,23 @@ struct drm_device { >> /** @dev: Device structure of bus-device */ >> struct device *dev; >> >> + /** >> + * @dma_dev: >> + * >> + * Device for DMA operations. Only required if the device @dev >> + * cannot perform DMA by itself. Should be NULL otherwise. >> + * >> + * Devices on USB and other peripheral busses cannot perform DMA >> + * by themselves. The @dma_dev field should point the bus controller >> + * that does DMA on behalve of such a device. Required for importing >> + * buffers via dma-buf. >> + * >> + * If set, the DRM driver has to acquire a reference on the DMA >> + * device, which will be owned and released automatically by the >> + * DRM core. >> + */ >> + struct device *dma_dev; >> + >> /** >> * @managed: >> * >> @@ -327,4 +344,24 @@ struct drm_device { >> struct dentry *debugfs_root; >> }; >> >> +/** >> + * drm_dev_dma_dev - returns the DMA device for a DRM device >> + * @dev: DRM device >> + * >> + * Returns the DMA device of the given DRM device. This is usually >> + * the same as the DRM device's parent. Devices on some peripheral >> + * busses, such as USB, are incapable of performing DMA by themselves. >> + * Drivers should set struct &drm_device.dma_dev to the bus controller >> + * to make DMA work. Required for PRIME buffer import. >> + * >> + * Returns: >> + * A DMA-capable device for the DRM device. >> + */ >> +static inline struct device *drm_dev_dma_dev(struct drm_device *dev) >> +{ >> + if (dev->dma_dev) >> + return dev->dma_dev; >> + return dev->dev; >> +} >> + >> #endif
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 17fc5dc708f4..f8c3c9f77d22 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -654,6 +654,8 @@ static void drm_dev_init_release(struct drm_device *dev, void *res) { drm_fs_inode_free(dev->anon_inode); + put_device(dev->dma_dev); + dev->dma_dev = NULL; put_device(dev->dev); /* Prevent use-after-free in drm_managed_release when debugging is * enabled. Slightly awkward, but can't really be helped. */ diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 32a8781cfd67..258858f2f8dd 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -1004,7 +1004,7 @@ EXPORT_SYMBOL(drm_gem_prime_import_dev); struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf) { - return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + return drm_gem_prime_import_dev(dev, dma_buf, drm_dev_dma_dev(dev)); } EXPORT_SYMBOL(drm_gem_prime_import); diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 6ea54a578cda..a24cac4b2077 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -64,6 +64,23 @@ struct drm_device { /** @dev: Device structure of bus-device */ struct device *dev; + /** + * @dma_dev: + * + * Device for DMA operations. Only required if the device @dev + * cannot perform DMA by itself. Should be NULL otherwise. + * + * Devices on USB and other peripheral busses cannot perform DMA + * by themselves. The @dma_dev field should point the bus controller + * that does DMA on behalve of such a device. Required for importing + * buffers via dma-buf. + * + * If set, the DRM driver has to acquire a reference on the DMA + * device, which will be owned and released automatically by the + * DRM core. + */ + struct device *dma_dev; + /** * @managed: * @@ -327,4 +344,24 @@ struct drm_device { struct dentry *debugfs_root; }; +/** + * drm_dev_dma_dev - returns the DMA device for a DRM device + * @dev: DRM device + * + * Returns the DMA device of the given DRM device. This is usually + * the same as the DRM device's parent. Devices on some peripheral + * busses, such as USB, are incapable of performing DMA by themselves. + * Drivers should set struct &drm_device.dma_dev to the bus controller + * to make DMA work. Required for PRIME buffer import. + * + * Returns: + * A DMA-capable device for the DRM device. + */ +static inline struct device *drm_dev_dma_dev(struct drm_device *dev) +{ + if (dev->dma_dev) + return dev->dma_dev; + return dev->dev; +} + #endif
Importing dma-bufs via PRIME requires a DMA-capable device. Devices on peripheral busses, such as USB, often cannot perform DMA by themselves. Without DMA-capable device PRIME import fails. DRM drivers for USB devices already use a separate DMA device for dma-buf imports. Make the mechanism generally available. Add the field dma_dev to struct drm_device to refer to the device's DMA device. For USB this should be the USB controller. Use dma_dev in the PRIME import helpers, if set. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_drv.c | 2 ++ drivers/gpu/drm/drm_prime.c | 2 +- include/drm/drm_device.h | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-)