Hi Christian,

Thank you for the patch.

On Mon, Jan 23, 2023 at 01:37:56PM +0100, Christian König wrote:
> Expose an indicator to let userspace know from which dma_heap
> to allocate for buffers of this device.
> 
> Signed-off-by: Christian König <christian.koe...@amd.com>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c 
> b/drivers/media/usb/uvc/uvc_driver.c
> index e4bcb5011360..b247026b68c5 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <linux/atomic.h>
> +#include <linux/dma-heap.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
> @@ -1909,6 +1910,8 @@ static void uvc_unregister_video(struct uvc_device *dev)
>  
>       if (dev->vdev.dev)
>               v4l2_device_unregister(&dev->vdev);
> +     dma_heap_remove_device_link(&dev->udev->dev);
> +

Could we avoid having to call this explicitly in drivers, possibly using
devres in dma_heap_create_device_link() ?

>  #ifdef CONFIG_MEDIA_CONTROLLER
>       if (media_devnode_is_registered(dev->mdev.devnode))
>               media_device_unregister(&dev->mdev);
> @@ -2181,6 +2184,14 @@ static int uvc_probe(struct usb_interface *intf,
>                        dev->uvc_version >> 8, dev->uvc_version & 0xff);
>       }
>  
> +     /*
> +      * UVC exports DMA-buf buffers with dirty CPU caches. For compatibility
> +      * with device which can't snoop the CPU cache it's best practice to
> +      * allocate DMA-bufs from the system DMA-heap.
> +      */
> +     if (dma_heap_create_device_link(&dev->udev->dev, "system"))

I don't think this is the right device. A UVC device is usually a
composite USB device with an audio (UAC) function in addition to UVC,
and that may require a different heap (at least conceptually). Wouldn't
the video_device be a better candidate to expose the link ? This would
create a race condition though, as the link will be created after
userspace gets notified of the device being available.

> +             goto error;
> +
>       /* Register the V4L2 device. */
>       if (v4l2_device_register(&intf->dev, &dev->vdev) < 0)
>               goto error;

-- 
Regards,

Laurent Pinchart

Reply via email to