[PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device

Ricardo Ribalda posted 5 patches 3 months, 3 weeks ago
[PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Ricardo Ribalda 3 months, 3 weeks ago
Currently, a UVC device can have multiple chains, and each chain maintains
its own priority state. While this behavior is technically correct for UVC,
uvcvideo is the *only* V4L2 driver that does not utilize the priority state
defined within `v4l2_device`.

This patch modifies uvcvideo to use the `v4l2_device` priority state. While
this might not be strictly "correct" for uvcvideo's multi-chain design, it
aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
behavior and enabling code cleanup in v4l2-core. Also, multi-chain
devices are extremely rare, they are typically implemented as two
independent usb devices.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_driver.c | 2 --
 drivers/media/usb/uvc/uvcvideo.h   | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1728,7 +1728,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;
 }
@@ -2008,7 +2007,6 @@ 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;
 	vdev->lock = &queue->mutex;
 	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -354,7 +354,6 @@ struct uvc_video_chain {
 						 * uvc_fh.pending_async_ctrls
 						 */
 
-	struct v4l2_prio_state prio;		/* V4L2 priority state */
 	u32 caps;				/* V4L2 chain-wide caps */
 	u8 ctrl_class_bitmap;			/* Bitmap of valid classes */
 };

-- 
2.50.0.rc1.591.g9c95f17f64-goog
Re: [PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Hans Verkuil 3 months, 3 weeks ago
On 16/06/2025 17:24, Ricardo Ribalda wrote:
> Currently, a UVC device can have multiple chains, and each chain maintains
> its own priority state. While this behavior is technically correct for UVC,
> uvcvideo is the *only* V4L2 driver that does not utilize the priority state
> defined within `v4l2_device`.
> 
> This patch modifies uvcvideo to use the `v4l2_device` priority state. While
> this might not be strictly "correct" for uvcvideo's multi-chain design, it
> aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
> behavior and enabling code cleanup in v4l2-core. Also, multi-chain
> devices are extremely rare, they are typically implemented as two
> independent usb devices.

I don't think this change is needed. I'm fine with the current code.

If someone has a multi-chain USB device, then it would be nice to see if
there are any v4l2-compliance problems. I don't think so, but it would be
good to have confirmation of that.

Regards,

	Hans

> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 2 --
>  drivers/media/usb/uvc/uvcvideo.h   | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1728,7 +1728,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;
>  }
> @@ -2008,7 +2007,6 @@ 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;
>  	vdev->lock = &queue->mutex;
>  	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -354,7 +354,6 @@ struct uvc_video_chain {
>  						 * uvc_fh.pending_async_ctrls
>  						 */
>  
> -	struct v4l2_prio_state prio;		/* V4L2 priority state */
>  	u32 caps;				/* V4L2 chain-wide caps */
>  	u8 ctrl_class_bitmap;			/* Bitmap of valid classes */
>  };
>
Re: [PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Ricardo Ribalda 3 months, 3 weeks ago
Hello All

On Mon, 16 Jun 2025 at 17:24, Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Currently, a UVC device can have multiple chains, and each chain maintains
> its own priority state. While this behavior is technically correct for UVC,
> uvcvideo is the *only* V4L2 driver that does not utilize the priority state
> defined within `v4l2_device`.
>
> This patch modifies uvcvideo to use the `v4l2_device` priority state. While
> this might not be strictly "correct" for uvcvideo's multi-chain design, it
> aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
> behavior and enabling code cleanup in v4l2-core. Also, multi-chain
> devices are extremely rare, they are typically implemented as two
> independent usb devices.

As the cover letter says, this last patch 5/5 is a RFC. We can decide
if it is worth to keep it or not.

The pros is that we can do some cleanup in the core, the cons is that
it might break kAPI.

I checked in the debian sourcecode and I could only find a user of
PRIORITY for dvb and was optional.




>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 2 --
>  drivers/media/usb/uvc/uvcvideo.h   | 1 -
>  2 files changed, 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1728,7 +1728,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;
>  }
> @@ -2008,7 +2007,6 @@ 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;
>         vdev->lock = &queue->mutex;
>         if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -354,7 +354,6 @@ struct uvc_video_chain {
>                                                  * uvc_fh.pending_async_ctrls
>                                                  */
>
> -       struct v4l2_prio_state prio;            /* V4L2 priority state */
>         u32 caps;                               /* V4L2 chain-wide caps */
>         u8 ctrl_class_bitmap;                   /* Bitmap of valid classes */
>  };
>
> --
> 2.50.0.rc1.591.g9c95f17f64-goog
>


-- 
Ricardo Ribalda
Re: [PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Laurent Pinchart 3 months, 3 weeks ago
On Mon, Jun 16, 2025 at 08:30:08PM +0200, Ricardo Ribalda wrote:
> On Mon, 16 Jun 2025 at 17:24, Ricardo Ribalda <ribalda@chromium.org> wrote:
> >
> > Currently, a UVC device can have multiple chains, and each chain maintains
> > its own priority state. While this behavior is technically correct for UVC,
> > uvcvideo is the *only* V4L2 driver that does not utilize the priority state
> > defined within `v4l2_device`.
> >
> > This patch modifies uvcvideo to use the `v4l2_device` priority state. While
> > this might not be strictly "correct" for uvcvideo's multi-chain design, it
> > aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
> > behavior and enabling code cleanup in v4l2-core. Also, multi-chain
> > devices are extremely rare, they are typically implemented as two
> > independent usb devices.
> 
> As the cover letter says, this last patch 5/5 is a RFC. We can decide
> if it is worth to keep it or not.
> 
> The pros is that we can do some cleanup in the core,

What cleanups would that be ?

> the cons is that it might break kAPI.

Multi-chain devices are essentially multiple video devices inside a
single USB function. They are exposed as completely separate devices to
userspace, having the priority ioctls on one chain impact the other
chain wouldn't make much sense to me. I think we should drop this patch.

> I checked in the debian sourcecode and I could only find a user of
> PRIORITY for dvb and was optional.

We could discuss deprecating the priority ioctls overall if we think
they're not useful (and used) by userspace. I was however considering
using them in libcamera though, to prevent other applications from
modifying the camera configuration behind the library's back.

> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 2 --
> >  drivers/media/usb/uvc/uvcvideo.h   | 1 -
> >  2 files changed, 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -1728,7 +1728,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;
> >  }
> > @@ -2008,7 +2007,6 @@ 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;
> >         vdev->lock = &queue->mutex;
> >         if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -354,7 +354,6 @@ struct uvc_video_chain {
> >                                                  * uvc_fh.pending_async_ctrls
> >                                                  */
> >
> > -       struct v4l2_prio_state prio;            /* V4L2 priority state */
> >         u32 caps;                               /* V4L2 chain-wide caps */
> >         u8 ctrl_class_bitmap;                   /* Bitmap of valid classes */
> >  };

-- 
Regards,

Laurent Pinchart
Re: [PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Ricardo Ribalda 3 months, 3 weeks ago
On Tue, 17 Jun 2025 at 12:07, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jun 16, 2025 at 08:30:08PM +0200, Ricardo Ribalda wrote:
> > On Mon, 16 Jun 2025 at 17:24, Ricardo Ribalda <ribalda@chromium.org> wrote:
> > >
> > > Currently, a UVC device can have multiple chains, and each chain maintains
> > > its own priority state. While this behavior is technically correct for UVC,
> > > uvcvideo is the *only* V4L2 driver that does not utilize the priority state
> > > defined within `v4l2_device`.
> > >
> > > This patch modifies uvcvideo to use the `v4l2_device` priority state. While
> > > this might not be strictly "correct" for uvcvideo's multi-chain design, it
> > > aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
> > > behavior and enabling code cleanup in v4l2-core. Also, multi-chain
> > > devices are extremely rare, they are typically implemented as two
> > > independent usb devices.
> >
> > As the cover letter says, this last patch 5/5 is a RFC. We can decide
> > if it is worth to keep it or not.
> >
> > The pros is that we can do some cleanup in the core,
>
> What cleanups would that be ?
>
> > the cons is that it might break kAPI.
>
> Multi-chain devices are essentially multiple video devices inside a
> single USB function. They are exposed as completely separate devices to
> userspace, having the priority ioctls on one chain impact the other
> chain wouldn't make much sense to me. I think we should drop this patch.

Ack, let's drop it.

PS: Do you know about a multi chain device that is commercially
available? I would love to buy one for testing.
Also do you know any "output" device that I can buy?

>
> > I checked in the debian sourcecode and I could only find a user of
> > PRIORITY for dvb and was optional.
>
> We could discuss deprecating the priority ioctls overall if we think
> they're not useful (and used) by userspace. I was however considering
> using them in libcamera though, to prevent other applications from
> modifying the camera configuration behind the library's back.

For the record:
From: https://codesearch.debian.net/search?q=VIDIOC_S_PRIORITY
If I am not wrong, this is the only relevant usecase:
https://sources.debian.org/src/zvbi/0.2.44-1/daemon/proxyd.c/?hl=1523#L1523

O_EXCL does not work for you?


>
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  drivers/media/usb/uvc/uvc_driver.c | 2 --
> > >  drivers/media/usb/uvc/uvcvideo.h   | 1 -
> > >  2 files changed, 3 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -1728,7 +1728,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;
> > >  }
> > > @@ -2008,7 +2007,6 @@ 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;
> > >         vdev->lock = &queue->mutex;
> > >         if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -354,7 +354,6 @@ struct uvc_video_chain {
> > >                                                  * uvc_fh.pending_async_ctrls
> > >                                                  */
> > >
> > > -       struct v4l2_prio_state prio;            /* V4L2 priority state */
> > >         u32 caps;                               /* V4L2 chain-wide caps */
> > >         u8 ctrl_class_bitmap;                   /* Bitmap of valid classes */
> > >  };
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda
Re: [PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Laurent Pinchart 3 months, 3 weeks ago
On Tue, Jun 17, 2025 at 01:09:38PM +0200, Ricardo Ribalda wrote:
> On Tue, 17 Jun 2025 at 12:07, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > On Mon, Jun 16, 2025 at 08:30:08PM +0200, Ricardo Ribalda wrote:
> > > On Mon, 16 Jun 2025 at 17:24, Ricardo Ribalda <ribalda@chromium.org> wrote:
> > > >
> > > > Currently, a UVC device can have multiple chains, and each chain maintains
> > > > its own priority state. While this behavior is technically correct for UVC,
> > > > uvcvideo is the *only* V4L2 driver that does not utilize the priority state
> > > > defined within `v4l2_device`.
> > > >
> > > > This patch modifies uvcvideo to use the `v4l2_device` priority state. While
> > > > this might not be strictly "correct" for uvcvideo's multi-chain design, it
> > > > aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
> > > > behavior and enabling code cleanup in v4l2-core. Also, multi-chain
> > > > devices are extremely rare, they are typically implemented as two
> > > > independent usb devices.
> > >
> > > As the cover letter says, this last patch 5/5 is a RFC. We can decide
> > > if it is worth to keep it or not.
> > >
> > > The pros is that we can do some cleanup in the core,
> >
> > What cleanups would that be ?
> >
> > > the cons is that it might break kAPI.
> >
> > Multi-chain devices are essentially multiple video devices inside a
> > single USB function. They are exposed as completely separate devices to
> > userspace, having the priority ioctls on one chain impact the other
> > chain wouldn't make much sense to me. I think we should drop this patch.
> 
> Ack, let's drop it.
> 
> PS: Do you know about a multi chain device that is commercially
> available? I would love to buy one for testing.
> Also do you know any "output" device that I can buy?

The only output device I've worked with was a custom camera developed by
a customer that had a "UVC to VGA" output path. It was a loooooong time
ago, I don't have a device to test UVC output anymore.

> > > I checked in the debian sourcecode and I could only find a user of
> > > PRIORITY for dvb and was optional.
> >
> > We could discuss deprecating the priority ioctls overall if we think
> > they're not useful (and used) by userspace. I was however considering
> > using them in libcamera though, to prevent other applications from
> > modifying the camera configuration behind the library's back.
> 
> For the record:
> From: https://codesearch.debian.net/search?q=VIDIOC_S_PRIORITY
> If I am not wrong, this is the only relevant usecase:
> https://sources.debian.org/src/zvbi/0.2.44-1/daemon/proxyd.c/?hl=1523#L1523
> 
> O_EXCL does not work for you?

I haven't tried it, but tt's defined as

    O_EXCL Ensure that this call creates the file: if this flag is
	   specified in conjunction with O_CREAT, and pathname already
	   exists, then open() fails with the error EEXIST.

	   When these two flags are specified, symbolic links are not
	   followed: if pathname is a symbolic link, then open() fails
	   regardless of where the symbolic link points.

	   In general, the behavior of O_EXCL is undefined if it is used
	   without O_CREAT.  There is one exception: on Linux 2.6 and
	   later, O_EXCL can be used without O_CREAT  if  pathname
	   refers to a block device.  If the block device is in use by
	   the system (e.g., mounted), open() fails with the error
	   EBUSY.

so I don't expect it would work.

It's not a big deal, libcamera doesn't get exclusive access to video
devices today and the world doesn't collapse (at least not because of
this specific issue). And we don't have priority support on subdevs, so
we couldn't solve the whole problem anyway.

> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > >  drivers/media/usb/uvc/uvc_driver.c | 2 --
> > > >  drivers/media/usb/uvc/uvcvideo.h   | 1 -
> > > >  2 files changed, 3 deletions(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > > index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
> > > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > > @@ -1728,7 +1728,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;
> > > >  }
> > > > @@ -2008,7 +2007,6 @@ 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;
> > > >         vdev->lock = &queue->mutex;
> > > >         if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
> > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > @@ -354,7 +354,6 @@ struct uvc_video_chain {
> > > >                                                  * uvc_fh.pending_async_ctrls
> > > >                                                  */
> > > >
> > > > -       struct v4l2_prio_state prio;            /* V4L2 priority state */
> > > >         u32 caps;                               /* V4L2 chain-wide caps */
> > > >         u8 ctrl_class_bitmap;                   /* Bitmap of valid classes */
> > > >  };

-- 
Regards,

Laurent Pinchart
Re: [PATCH v4 5/5] media: uvcvideo: Use prio state from v4l2_device
Posted by Hans de Goede 3 months, 3 weeks ago
Hi Ricardo,

On 16-Jun-25 8:30 PM, Ricardo Ribalda wrote:
> Hello All
> 
> On Mon, 16 Jun 2025 at 17:24, Ricardo Ribalda <ribalda@chromium.org> wrote:
>>
>> Currently, a UVC device can have multiple chains, and each chain maintains
>> its own priority state. While this behavior is technically correct for UVC,
>> uvcvideo is the *only* V4L2 driver that does not utilize the priority state
>> defined within `v4l2_device`.
>>
>> This patch modifies uvcvideo to use the `v4l2_device` priority state. While
>> this might not be strictly "correct" for uvcvideo's multi-chain design, it
>> aligns uvcvideo with the rest of the V4L2 drivers, providing "correct enough"
>> behavior and enabling code cleanup in v4l2-core. Also, multi-chain
>> devices are extremely rare, they are typically implemented as two
>> independent usb devices.
> 
> As the cover letter says, this last patch 5/5 is a RFC. We can decide
> if it is worth to keep it or not.
> 
> The pros is that we can do some cleanup in the core, the cons is that
> it might break kAPI.

I've no objections against this change, but lets wait and see what
Laurent has to say.

Regards,

Hans




>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> ---
>>  drivers/media/usb/uvc/uvc_driver.c | 2 --
>>  drivers/media/usb/uvc/uvcvideo.h   | 1 -
>>  2 files changed, 3 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
>> index accfb4ca3c72cb899185ddc8ecf4e29143d58fc6..e3795e40f14dc325e5bd120f5f45b60937841641 100644
>> --- a/drivers/media/usb/uvc/uvc_driver.c
>> +++ b/drivers/media/usb/uvc/uvc_driver.c
>> @@ -1728,7 +1728,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;
>>  }
>> @@ -2008,7 +2007,6 @@ 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;
>>         vdev->lock = &queue->mutex;
>>         if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
>> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
>> index 3e6d2d912f3a1cfcf63b2bc8edd3f86f3da305db..5ed9785d59c698cc7e0ac69955b892f932961617 100644
>> --- a/drivers/media/usb/uvc/uvcvideo.h
>> +++ b/drivers/media/usb/uvc/uvcvideo.h
>> @@ -354,7 +354,6 @@ struct uvc_video_chain {
>>                                                  * uvc_fh.pending_async_ctrls
>>                                                  */
>>
>> -       struct v4l2_prio_state prio;            /* V4L2 priority state */
>>         u32 caps;                               /* V4L2 chain-wide caps */
>>         u8 ctrl_class_bitmap;                   /* Bitmap of valid classes */
>>  };
>>
>> --
>> 2.50.0.rc1.591.g9c95f17f64-goog
>>
> 
>