Re: [PATCH v4 25/29] media: v4l2-subdev: Move subdev client capabilities into a new struct

Sakari Ailus
2026-04-13T08:11:22Z

Hi Jacopo,

On Fri, Apr 10, 2026 at 03:31:32PM +0200, Jacopo Mondi wrote:
> Hi Sakari
>
> On Wed, Apr 08, 2026 at 06:39:34PM +0300, Sakari Ailus wrote:
> > Add struct v4l2_subdev_client_info to hold sub-device client capability
> > bits that used to be stored in the client_caps field of struct
> > v4l2_subdev_fh. The intent is to enable passing this struct to sub-device
> > pad operation callbacks for capability information. The main reason why
> > this is a new struct instead of a u64 field is that modifying the callback
> > arguments requires touching almost every sub-device driver and that is
> > desirable to avoid in the future, should more than the client capability bits
> > need to be known to the callbacks.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > ---
> > drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++----
> > include/media/v4l2-subdev.h | 12 ++++++++++--
> > 2 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 40b28e070726..eaa408832c6b 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -612,7 +612,7 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh,
> > case VIDIOC_SUBDEV_S_FRAME_INTERVAL: {
> > struct v4l2_subdev_frame_interval *fi = arg;
> >
> > - if (!(subdev_fh->client_caps &
> > + if (!(subdev_fh->ci.client_caps &
> > V4L2_SUBDEV_CLIENT_CAP_INTERVAL_USES_WHICH))
> > fi->which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >
> > @@ -652,7 +652,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
> > struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
> > bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags);
> > bool streams_subdev = sd->flags & V4L2_SUBDEV_FL_STREAMS;
> > - bool client_supports_streams = subdev_fh->client_caps &
> > + bool client_supports_streams = subdev_fh->ci.client_caps &
> > V4L2_SUBDEV_CLIENT_CAP_STREAMS;
> > int rval;
> >
> > @@ -1119,7 +1119,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
> > case VIDIOC_SUBDEV_G_CLIENT_CAP: {
> > struct v4l2_subdev_client_capability *client_cap = arg;
> >
> > - client_cap->capabilities = subdev_fh->client_caps;
> > + client_cap->capabilities = subdev_fh->ci.client_caps;
> >
> > return 0;
> > }
> > @@ -1139,7 +1139,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
> > client_cap->capabilities &= (V4L2_SUBDEV_CLIENT_CAP_STREAMS |
> > V4L2_SUBDEV_CLIENT_CAP_INTERVAL_USES_WHICH);
> >
> > - subdev_fh->client_caps = client_cap->capabilities;
> > + subdev_fh->ci.client_caps = client_cap->capabilities;
>
>
> I'm sorry for being annoying, but the discussion on v2 ended with the
> question on why we can't propagate the client caps to the drivers
> using the subdev state.
>
> I understand we have the active state which is stored in the subdev
> and not per-file handle. But ioctls are called on a file handle and it
> seems trivial to me copy the caps from the subdev_fh to the active
> state.
>
> It would result in a much smaller set of changes.
>
> What am I missing ?

I thought the discussion ended with the conclusion that we can't do that as
the sub-device active state isn't bound to a file handle. :-)

--
Kind regards,

Sakari Ailus