Re: [PATCH v3 8/8] uvc: use vb2 ioctl and fop helpers

[Hans Verkuil]

On 12/03/2021 13:48, Ricardo Ribalda wrote:
> From: Hans Verkuil <hverkuil@xxxxxxxxx>
> 
> When uvc was written the vb2 ioctl and file operation helpers didn't exist.
> 
> This patch switches uvc over to those helpers, which removes a lot of boilerplate
> code and simplifies VIDIOC_G/S_PRIORITY handling and allows us to drop the
> 'privileges' scheme, since that's now handled inside the vb2 helpers.
> 
> This makes it possible for uvc to pass the v4l2-compliance streaming tests.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
>  drivers/media/usb/uvc/uvc_driver.c   |   7 +-
>  drivers/media/usb/uvc/uvc_metadata.c |   8 +-
>  drivers/media/usb/uvc/uvc_queue.c    | 131 -------------
>  drivers/media/usb/uvc/uvc_v4l2.c     | 272 ++-------------------------
>  drivers/media/usb/uvc/uvcvideo.h     |  28 ---
>  5 files changed, 24 insertions(+), 422 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 6c928e708615..b9f3984e9f80 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1911,7 +1911,6 @@ static struct uvc_video_chain *uvc_alloc_chain(struct uvc_device *dev)
>  	INIT_LIST_HEAD(&chain->entities);
>  	mutex_init(&chain->ctrl_mutex);
>  	chain->dev = dev;
> -	v4l2_prio_init(&chain->prio);
>  
>  	return chain;
>  }
> @@ -2194,7 +2193,7 @@ int uvc_register_video_device(struct uvc_device *dev,
>  	vdev->fops = fops;
>  	vdev->ioctl_ops = ioctl_ops;
>  	vdev->release = uvc_release;
> -	vdev->prio = &stream->chain->prio;
> +	vdev->queue = &queue->queue;
>  	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
>  		vdev->vfl_dir = VFL_DIR_TX;
>  	else
> @@ -2560,8 +2559,8 @@ static int __uvc_resume(struct usb_interface *intf, int reset)
>  		if (stream->intf == intf) {
>  			ret = uvc_video_resume(stream, reset);
>  			if (ret < 0)
> -				uvc_queue_streamoff(&stream->queue,
> -						    stream->queue.queue.type);
> +				vb2_streamoff(&stream->queue.queue,
> +					      stream->queue.queue.type);
>  			return ret;
>  		}
>  	}
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index b6279ad7ac84..2f52cdc62929 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -96,7 +96,7 @@ static int uvc_meta_v4l2_set_format(struct file *file, void *fh,
>  	 */
>  	mutex_lock(&stream->mutex);
>  
> -	if (uvc_queue_allocated(&stream->queue))
> +	if (vb2_is_busy(&stream->queue.queue))

This should be &stream->meta.queue.queue. That explains this failure that I get
with v4l2-compliance -d1 when streaming with v4l2-ctl --stream-mmap:

fail: v4l2-test-formats.cpp(452): expected EINVAL, but got 16 when getting format for buftype 13
        test VIDIOC_S_FMT: FAIL

Regards,

	Hans