[libcamera-devel] [PATCH v4 4/5] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Laurent Pinchart]

Hi Sakari,

On Wed, Apr 22, 2020 at 12:49:58AM +0300, Sakari Ailus wrote:
> On Sat, Apr 18, 2020 at 12:32:15PM +0200, Jacopo Mondi wrote:
> > From: Hans Verkuil <hans.verkuil at cisco.com>
> > 
> > While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl
> > that apps can call to determine that it is indeed a V4L2 device, there
> > is currently no equivalent for v4l-subdev nodes. Adding this ioctl will
> > solve that, and it will allow utilities like v4l2-compliance to be used
> > with these devices as well.
> > 
> > SUBDEV_QUERYCAP currently returns the version and subdev_caps of the
> > subdevice. Define as the initial set of subdev_caps the read-only or
> > read/write flags, to signal to userspace which set of IOCTLs are
> > available on the subdevice.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil at cisco.com>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++
> >  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 1dc263c2ca0a..ca0aa54429c5 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/types.h>
> >  #include <linux/videodev2.h>
> >  #include <linux/export.h>
> > +#include <linux/version.h>
> >  
> >  #include <media/v4l2-ctrls.h>
> >  #include <media/v4l2-device.h>
> > @@ -336,6 +337,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> >  	int rval;
> >  
> >  	switch (cmd) {
> > +	case VIDIOC_SUBDEV_QUERYCAP: {
> > +		struct v4l2_subdev_capability *cap = arg;
> > +
> > +		memset(cap, 0, sizeof(*cap));
> > +		cap->version = LINUX_VERSION_CODE;
> > +		cap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV
> > +					      : V4L2_SUBDEV_CAP_RW_SUBDEV;
> > +
> > +		return 0;
> > +	}
> > +
> >  	case VIDIOC_QUERYCTRL:
> >  		/*
> >  		 * TODO: this really should be folded into v4l2_queryctrl (this
> > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> > index 03970ce30741..0886f88be193 100644
> > --- a/include/uapi/linux/v4l2-subdev.h
> > +++ b/include/uapi/linux/v4l2-subdev.h
> > @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {
> >  	__u32 reserved[8];
> >  };
> >  
> > +/**
> > + * struct v4l2_subdev_capability - subdev capabilities
> > + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.
> > + */
> > +struct v4l2_subdev_capability {
> > +	__u32 version;
> > +	__u32 subdev_caps;
> 
> No reserved fields? Is the intent to extend this later on based on the size
> of the IOCTL argument?

That would be my preferred option.

> > +};
> > +
> > +/* The v4l2 sub-device video device node is registered in read-only mode. */
> > +#define V4L2_SUBDEV_CAP_RO_SUBDEV		(1 << 0)
> 
> 1U << 0

BIT(0)

> 
> > +/* The v4l2 sub-device video device node is registered in read/write mode. */
> > +#define V4L2_SUBDEV_CAP_RW_SUBDEV		(1 << 1)
> 
> 1U << 1

BIT(1)

> > +
> >  /* Backwards compatibility define --- to be removed */
> >  #define v4l2_subdev_edid v4l2_edid
> >  
> > +#define VIDIOC_SUBDEV_QUERYCAP			_IOR('V',  0, struct v4l2_subdev_capability)
> >  #define VIDIOC_SUBDEV_G_FMT			_IOWR('V',  4, struct v4l2_subdev_format)
> >  #define VIDIOC_SUBDEV_S_FMT			_IOWR('V',  5, struct v4l2_subdev_format)
> >  #define VIDIOC_SUBDEV_G_FRAME_INTERVAL		_IOWR('V', 21, struct v4l2_subdev_frame_interval)

-- 
Regards,

Laurent Pinchart