[{"id":4629,"web_url":"https://patchwork.libcamera.org/comment/4629/","msgid":"<20200428212858.GC5381@paasikivi.fi.intel.com>","date":"2020-04-28T21:28:58","subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Jacopo,\n\nOn Tue, Apr 28, 2020 at 11:06:08PM +0200, Jacopo Mondi wrote:\n> From: Hans Verkuil <hans.verkuil@cisco.com>\n> \n> While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n> that apps can call to determine that it is indeed a V4L2 device, there\n> is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n> solve that, and it will allow utilities like v4l2-compliance to be used\n> with these devices as well.\n> \n> SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n> subdevice. Define as the initial set of subdev_caps the read-only or\n> read/write flags, to signal to userspace which set of IOCTLs are\n> available on the subdevice.\n> \n> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n>  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++\n>  2 files changed, 27 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> index f3fe515b8ccb..b8c0071aa4d0 100644\n> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> @@ -15,6 +15,7 @@\n>  #include <linux/types.h>\n>  #include <linux/videodev2.h>\n>  #include <linux/export.h>\n> +#include <linux/version.h>\n>  \n>  #include <media/v4l2-ctrls.h>\n>  #include <media/v4l2-device.h>\n> @@ -331,6 +332,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n>  \tint rval;\n>  \n>  \tswitch (cmd) {\n> +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n> +\t\tstruct v4l2_subdev_capability *cap = arg;\n> +\n> +\t\tmemset(cap, 0, sizeof(*cap));\n> +\t\tcap->version = LINUX_VERSION_CODE;\n> +\t\tcap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n> +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tcase VIDIOC_QUERYCTRL:\n>  \t\t/*\n>  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n> index 03970ce30741..89dc8f2ba6b3 100644\n> --- a/include/uapi/linux/v4l2-subdev.h\n> +++ b/include/uapi/linux/v4l2-subdev.h\n> @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {\n>  \t__u32 reserved[8];\n>  };\n>  \n> +/**\n> + * struct v4l2_subdev_capability - subdev capabilities\n> + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.\n> + */\n> +struct v4l2_subdev_capability {\n> +\t__u32 version;\n> +\t__u32 subdev_caps;\n\nHow do you intend to address additional fields being added to the struct in\nthe future? Something else than what's been done in V4L2 traditionally?\n\n> +};\n> +\n> +/* The v4l2 sub-device video device node is registered in read-only mode. */\n> +#define V4L2_SUBDEV_CAP_RO_SUBDEV\t\tBIT(0)\n> +/* The v4l2 sub-device video device node is registered in read/write mode. */\n> +#define V4L2_SUBDEV_CAP_RW_SUBDEV\t\tBIT(1)\n> +\n>  /* Backwards compatibility define --- to be removed */\n>  #define v4l2_subdev_edid v4l2_edid\n>  \n> +#define VIDIOC_SUBDEV_QUERYCAP\t\t\t_IOR('V',  0, struct v4l2_subdev_capability)\n>  #define VIDIOC_SUBDEV_G_FMT\t\t\t_IOWR('V',  4, struct v4l2_subdev_format)\n>  #define VIDIOC_SUBDEV_S_FMT\t\t\t_IOWR('V',  5, struct v4l2_subdev_format)\n>  #define VIDIOC_SUBDEV_G_FRAME_INTERVAL\t\t_IOWR('V', 21, struct v4l2_subdev_frame_interval)","headers":{"Return-Path":"<sakari.ailus@linux.intel.com>","Received":["from mga04.intel.com (mga04.intel.com [192.55.52.120])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 666EE60AF4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 28 Apr 2020 23:29:04 +0200 (CEST)","from fmsmga001.fm.intel.com ([10.253.24.23])\n\tby fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t28 Apr 2020 14:29:02 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby fmsmga001-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Apr 2020 14:29:00 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid E87C9202F2; Wed, 29 Apr 2020 00:28:58 +0300 (EEST)"],"IronPort-SDR":["l1XOMVL9+6auGjqvNlVGD7tSNnu5c9vinywTwzzX7m/2i2B/2bg/MSU0raZEdYiQbcDCcndPTN\n\tFIysjZmvaXrA==","8XwNGzG4p4tPap+LZoX1aq4HwHDwJQwL5IaKrPDDtvl3r7Dhx2xKTHg1fDNd37ao2+CS9lNWWX\n\tuniW+xiYx5iQ=="],"X-Amp-Result":"SKIPPED(no attachment in message)","X-Amp-File-Uploaded":"False","X-IronPort-AV":"E=Sophos;i=\"5.73,328,1583222400\"; d=\"scan'208\";a=\"367621732\"","Date":"Wed, 29 Apr 2020 00:28:58 +0300","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, hverkuil-cisco@xs4all.nl,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com,\n\tHans Verkuil <hans.verkuil@cisco.com>","Message-ID":"<20200428212858.GC5381@paasikivi.fi.intel.com>","References":"<20200428210609.6793-1-jacopo@jmondi.org>\n\t<20200428210609.6793-6-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200428210609.6793-6-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 28 Apr 2020 21:29:05 -0000"}},{"id":4638,"web_url":"https://patchwork.libcamera.org/comment/4638/","msgid":"<20200429080949.walimwkrth3ixn2o@uno.localdomain>","date":"2020-04-29T08:09:49","subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Sakari,\n\nOn Wed, Apr 29, 2020 at 12:28:58AM +0300, Sakari Ailus wrote:\n> Hi Jacopo,\n>\n> On Tue, Apr 28, 2020 at 11:06:08PM +0200, Jacopo Mondi wrote:\n> > From: Hans Verkuil <hans.verkuil@cisco.com>\n> >\n> > While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n> > that apps can call to determine that it is indeed a V4L2 device, there\n> > is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n> > solve that, and it will allow utilities like v4l2-compliance to be used\n> > with these devices as well.\n> >\n> > SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n> > subdevice. Define as the initial set of subdev_caps the read-only or\n> > read/write flags, to signal to userspace which set of IOCTLs are\n> > available on the subdevice.\n> >\n> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n> >  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++\n> >  2 files changed, 27 insertions(+)\n> >\n> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> > index f3fe515b8ccb..b8c0071aa4d0 100644\n> > --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> > @@ -15,6 +15,7 @@\n> >  #include <linux/types.h>\n> >  #include <linux/videodev2.h>\n> >  #include <linux/export.h>\n> > +#include <linux/version.h>\n> >\n> >  #include <media/v4l2-ctrls.h>\n> >  #include <media/v4l2-device.h>\n> > @@ -331,6 +332,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> >  \tint rval;\n> >\n> >  \tswitch (cmd) {\n> > +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n> > +\t\tstruct v4l2_subdev_capability *cap = arg;\n> > +\n> > +\t\tmemset(cap, 0, sizeof(*cap));\n> > +\t\tcap->version = LINUX_VERSION_CODE;\n> > +\t\tcap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n> > +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> >  \tcase VIDIOC_QUERYCTRL:\n> >  \t\t/*\n> >  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n> > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n> > index 03970ce30741..89dc8f2ba6b3 100644\n> > --- a/include/uapi/linux/v4l2-subdev.h\n> > +++ b/include/uapi/linux/v4l2-subdev.h\n> > @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {\n> >  \t__u32 reserved[8];\n> >  };\n> >\n> > +/**\n> > + * struct v4l2_subdev_capability - subdev capabilities\n> > + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.\n> > + */\n> > +struct v4l2_subdev_capability {\n> > +\t__u32 version;\n> > +\t__u32 subdev_caps;\n>\n> How do you intend to address additional fields being added to the struct in\n> the future? Something else than what's been done in V4L2 traditionally?\n>\n\nI'm not sure I get what you mean here, so I assume I don't know what\n\"has been done in V4L2 traditionally\" and why what I have here goes\nagainst it...","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E480601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 29 Apr 2020 10:06:41 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id B858BFF805;\n\tWed, 29 Apr 2020 08:06:38 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 29 Apr 2020 10:09:49 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Sakari Ailus <sakari.ailus@linux.intel.com>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, hverkuil-cisco@xs4all.nl,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com,\n\tHans Verkuil <hans.verkuil@cisco.com>","Message-ID":"<20200429080949.walimwkrth3ixn2o@uno.localdomain>","References":"<20200428210609.6793-1-jacopo@jmondi.org>\n\t<20200428210609.6793-6-jacopo@jmondi.org>\n\t<20200428212858.GC5381@paasikivi.fi.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200428212858.GC5381@paasikivi.fi.intel.com>","Subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 29 Apr 2020 08:06:41 -0000"}},{"id":4639,"web_url":"https://patchwork.libcamera.org/comment/4639/","msgid":"<20200429081859.GA9190@paasikivi.fi.intel.com>","date":"2020-04-29T08:18:59","subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Jacopo,\n\nOn Wed, Apr 29, 2020 at 10:09:49AM +0200, Jacopo Mondi wrote:\n> Hi Sakari,\n> \n> On Wed, Apr 29, 2020 at 12:28:58AM +0300, Sakari Ailus wrote:\n> > Hi Jacopo,\n> >\n> > On Tue, Apr 28, 2020 at 11:06:08PM +0200, Jacopo Mondi wrote:\n> > > From: Hans Verkuil <hans.verkuil@cisco.com>\n> > >\n> > > While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n> > > that apps can call to determine that it is indeed a V4L2 device, there\n> > > is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n> > > solve that, and it will allow utilities like v4l2-compliance to be used\n> > > with these devices as well.\n> > >\n> > > SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n> > > subdevice. Define as the initial set of subdev_caps the read-only or\n> > > read/write flags, to signal to userspace which set of IOCTLs are\n> > > available on the subdevice.\n> > >\n> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > >  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n> > >  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++\n> > >  2 files changed, 27 insertions(+)\n> > >\n> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> > > index f3fe515b8ccb..b8c0071aa4d0 100644\n> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> > > @@ -15,6 +15,7 @@\n> > >  #include <linux/types.h>\n> > >  #include <linux/videodev2.h>\n> > >  #include <linux/export.h>\n> > > +#include <linux/version.h>\n> > >\n> > >  #include <media/v4l2-ctrls.h>\n> > >  #include <media/v4l2-device.h>\n> > > @@ -331,6 +332,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > >  \tint rval;\n> > >\n> > >  \tswitch (cmd) {\n> > > +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n> > > +\t\tstruct v4l2_subdev_capability *cap = arg;\n> > > +\n> > > +\t\tmemset(cap, 0, sizeof(*cap));\n> > > +\t\tcap->version = LINUX_VERSION_CODE;\n> > > +\t\tcap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n> > > +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n> > > +\n> > > +\t\treturn 0;\n> > > +\t}\n> > > +\n> > >  \tcase VIDIOC_QUERYCTRL:\n> > >  \t\t/*\n> > >  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n> > > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n> > > index 03970ce30741..89dc8f2ba6b3 100644\n> > > --- a/include/uapi/linux/v4l2-subdev.h\n> > > +++ b/include/uapi/linux/v4l2-subdev.h\n> > > @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {\n> > >  \t__u32 reserved[8];\n> > >  };\n> > >\n> > > +/**\n> > > + * struct v4l2_subdev_capability - subdev capabilities\n> > > + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.\n> > > + */\n> > > +struct v4l2_subdev_capability {\n> > > +\t__u32 version;\n> > > +\t__u32 subdev_caps;\n> >\n> > How do you intend to address additional fields being added to the struct in\n> > the future? Something else than what's been done in V4L2 traditionally?\n> >\n> \n> I'm not sure I get what you mean here, so I assume I don't know what\n> \"has been done in V4L2 traditionally\" and why what I have here goes\n> against it...\n\nI can't help noticing you have no reserved fields in your IOCTL argument\nstruct. That has generally been the way V4L2 IOCTLs have been extended when\nthere's been a need to.\n\nMedia controller adopted a different approach to that recently, based on\nthe argument size. We've discussed doing that on V4L2 but I don't think\na decision has been made.","headers":{"Return-Path":"<sakari.ailus@linux.intel.com>","Received":["from mga07.intel.com (mga07.intel.com [134.134.136.100])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3838F601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 29 Apr 2020 10:19:05 +0200 (CEST)","from fmsmga003.fm.intel.com ([10.253.24.29])\n\tby orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t29 Apr 2020 01:19:03 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby fmsmga003-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Apr 2020 01:19:01 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid 5C02D202AE; Wed, 29 Apr 2020 11:18:59 +0300 (EEST)"],"IronPort-SDR":["iI8wxhJG4nujhpKReTfBjZwP0MyYPS1I7P/UptNY7+vN1sJsOMRC1B4OQZjj9sHAubnoI5vTcC\n\tsLx13TXVKnlg==","dWaM/Uqn/tr2NLjHgzqKO6t87wdI+vAO81T5DmBqjRcWpBe9npmDa5RforRpMRBG44Z7Z17UUV\n\t83WTrrLoVmIQ=="],"X-Amp-Result":"SKIPPED(no attachment in message)","X-Amp-File-Uploaded":"False","X-IronPort-AV":"E=Sophos;i=\"5.73,330,1583222400\"; d=\"scan'208\";a=\"302937268\"","Date":"Wed, 29 Apr 2020 11:18:59 +0300","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, hverkuil-cisco@xs4all.nl,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com,\n\tHans Verkuil <hans.verkuil@cisco.com>","Message-ID":"<20200429081859.GA9190@paasikivi.fi.intel.com>","References":"<20200428210609.6793-1-jacopo@jmondi.org>\n\t<20200428210609.6793-6-jacopo@jmondi.org>\n\t<20200428212858.GC5381@paasikivi.fi.intel.com>\n\t<20200429080949.walimwkrth3ixn2o@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200429080949.walimwkrth3ixn2o@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 29 Apr 2020 08:19:05 -0000"}},{"id":4751,"web_url":"https://patchwork.libcamera.org/comment/4751/","msgid":"<15053210-8d02-afbd-0d02-b08f9b5f4378@xs4all.nl>","date":"2020-05-06T13:29:03","subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 29/04/2020 10:18, Sakari Ailus wrote:\n> Hi Jacopo,\n> \n> On Wed, Apr 29, 2020 at 10:09:49AM +0200, Jacopo Mondi wrote:\n>> Hi Sakari,\n>>\n>> On Wed, Apr 29, 2020 at 12:28:58AM +0300, Sakari Ailus wrote:\n>>> Hi Jacopo,\n>>>\n>>> On Tue, Apr 28, 2020 at 11:06:08PM +0200, Jacopo Mondi wrote:\n>>>> From: Hans Verkuil <hans.verkuil@cisco.com>\n>>>>\n>>>> While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n>>>> that apps can call to determine that it is indeed a V4L2 device, there\n>>>> is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n>>>> solve that, and it will allow utilities like v4l2-compliance to be used\n>>>> with these devices as well.\n>>>>\n>>>> SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n>>>> subdevice. Define as the initial set of subdev_caps the read-only or\n>>>> read/write flags, to signal to userspace which set of IOCTLs are\n>>>> available on the subdevice.\n>>>>\n>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n>>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>>>> ---\n>>>>  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n>>>>  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++\n>>>>  2 files changed, 27 insertions(+)\n>>>>\n>>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n>>>> index f3fe515b8ccb..b8c0071aa4d0 100644\n>>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n>>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n>>>> @@ -15,6 +15,7 @@\n>>>>  #include <linux/types.h>\n>>>>  #include <linux/videodev2.h>\n>>>>  #include <linux/export.h>\n>>>> +#include <linux/version.h>\n>>>>\n>>>>  #include <media/v4l2-ctrls.h>\n>>>>  #include <media/v4l2-device.h>\n>>>> @@ -331,6 +332,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n>>>>  \tint rval;\n>>>>\n>>>>  \tswitch (cmd) {\n>>>> +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n>>>> +\t\tstruct v4l2_subdev_capability *cap = arg;\n>>>> +\n>>>> +\t\tmemset(cap, 0, sizeof(*cap));\n>>>> +\t\tcap->version = LINUX_VERSION_CODE;\n>>>> +\t\tcap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n>>>> +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n>>>> +\n>>>> +\t\treturn 0;\n>>>> +\t}\n>>>> +\n>>>>  \tcase VIDIOC_QUERYCTRL:\n>>>>  \t\t/*\n>>>>  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n>>>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n>>>> index 03970ce30741..89dc8f2ba6b3 100644\n>>>> --- a/include/uapi/linux/v4l2-subdev.h\n>>>> +++ b/include/uapi/linux/v4l2-subdev.h\n>>>> @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {\n>>>>  \t__u32 reserved[8];\n>>>>  };\n>>>>\n>>>> +/**\n>>>> + * struct v4l2_subdev_capability - subdev capabilities\n>>>> + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.\n>>>> + */\n>>>> +struct v4l2_subdev_capability {\n>>>> +\t__u32 version;\n>>>> +\t__u32 subdev_caps;\n>>>\n>>> How do you intend to address additional fields being added to the struct in\n>>> the future? Something else than what's been done in V4L2 traditionally?\n>>>\n>>\n>> I'm not sure I get what you mean here, so I assume I don't know what\n>> \"has been done in V4L2 traditionally\" and why what I have here goes\n>> against it...\n> \n> I can't help noticing you have no reserved fields in your IOCTL argument\n> struct. That has generally been the way V4L2 IOCTLs have been extended when\n> there's been a need to.\n> \n> Media controller adopted a different approach to that recently, based on\n> the argument size. We've discussed doing that on V4L2 but I don't think\n> a decision has been made.\n> \n\nWhile I agree that using the argument size to do 'versioning' of the API\nis a better solution, the fact is that historically we always used a 'reserved'\nfield. And I think we need to do that here as well. It's consistent with\nthe existing subdev ioctls, so I would be in favor of adding a 'u32 reserved[6];'\nfield.\n\nIf there are such strong feelings against it that it wouldn't be merged, then\nwe can always just leave it as-is. It's not worth blocking this patch just\nbecause of that.\n\nBTW, one thing that should be changed is the name 'subdev_caps': just name it\n'capabilities'. It's a field in a struct v4l2_subdev_capability, so it is\nalready obvious that this is subdev specific.\n\nRegards,\n\n\tHans","headers":{"Return-Path":"<hverkuil-cisco@xs4all.nl>","Received":["from lb1-smtp-cloud7.xs4all.net (lb1-smtp-cloud7.xs4all.net\n\t[194.109.24.24])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21E03603F2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 May 2020 15:29:13 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid WK6ijO6LgtKAsWK6ljVg4u; Wed, 06 May 2020 15:29:07 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"CZ2KDN+F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588771747; bh=aqNROeyX2+PxHX9HQGrwvzOy+za58cgm2i7qjbMVz4o=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=CZ2KDN+FN9L0z2VAAqXUdk3cnRw+r0nK4VZ9eDaZ+J+RAwwsDUZyWhHkw28q7BXiA\n\tefsV8ViIOMnu5l6CnAGZT+TcbTcJM4KN4jXkAxg/MgBqWt2eBT6oq4j9IuSiaCBE+l\n\tukjsaCL9J/lIKpT5VH2KKf0E5KAYcOKGGZTJIZmsy9cZSymyoSKmncOAUAz3mW0jq0\n\tW9fzSiFj4mX5/T77k0cpBCQdEQBJkiwZQCYr25M0mEMr/EORFtNkPcuTYeDZAD4KNO\n\tKYr6Kka9kdkCpqC5n/VdwTl2e/agWeZKyxkOZ5bNJiZyLR4dCMO15yYcGuB1BPsqZ0\n\tVQZ/efxwfMmng==","To":"Sakari Ailus <sakari.ailus@linux.intel.com>,\n\tJacopo Mondi <jacopo@jmondi.org>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, andrey.konovalov@linaro.org,\n\tlaurent.pinchart@ideasonboard.com, Hans Verkuil <hans.verkuil@cisco.com>","References":"<20200428210609.6793-1-jacopo@jmondi.org>\n\t<20200428210609.6793-6-jacopo@jmondi.org>\n\t<20200428212858.GC5381@paasikivi.fi.intel.com>\n\t<20200429080949.walimwkrth3ixn2o@uno.localdomain>\n\t<20200429081859.GA9190@paasikivi.fi.intel.com>","From":"Hans Verkuil <hverkuil-cisco@xs4all.nl>","Message-ID":"<15053210-8d02-afbd-0d02-b08f9b5f4378@xs4all.nl>","Date":"Wed, 6 May 2020 15:29:03 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200429081859.GA9190@paasikivi.fi.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfETklDH3sZyRMUIGQI05mh4mCgeEYM8SyJJOe2rFR3kcSmHNwosR6h8ytdFLtTfcWePhMdRbVdap99igX4nyeiq2hxM9caqD64AjOlCZ6yZGsgrVrMNg\n\tnGtsqyVmTOWGvM9I0HH2YxHcHaamVpFH7N99X0YxxSyHWqhH9f/R3f6yJuRzxB9U7VG4lUo/dsbKHCohOZnIgBux9Prrzvv2Bkz6w327vBBqmx68x4Kb6gAs\n\tHQfNHrmWObTIMZtFVGPNsRU9sYRwlqdyIiSSNYudx28tC087b9f2mnmGffk7H3EPOuEQAllw9kxcxtKw9mwDNqCFSBx++S8FcEvU1D8eARtTM5ETXtFpbkH5\n\tfjWq8kurzjgyL8R5jjomuprpaDVSrqVyEU/cpwhUqKGJ5l4TbJhz2dOCTnXcr5fdZoAUg6R07FYmQ/ElvHSxCKdBimGc07MUbXrkJFnTJTTuAVNoMTg=","Subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 06 May 2020 13:29:13 -0000"}},{"id":4753,"web_url":"https://patchwork.libcamera.org/comment/4753/","msgid":"<20200506183459.GA9190@paasikivi.fi.intel.com>","date":"2020-05-06T18:34:59","subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Hans, Jacopo,\n\nOn Wed, May 06, 2020 at 03:29:03PM +0200, Hans Verkuil wrote:\n> On 29/04/2020 10:18, Sakari Ailus wrote:\n> > Hi Jacopo,\n> > \n> > On Wed, Apr 29, 2020 at 10:09:49AM +0200, Jacopo Mondi wrote:\n> >> Hi Sakari,\n> >>\n> >> On Wed, Apr 29, 2020 at 12:28:58AM +0300, Sakari Ailus wrote:\n> >>> Hi Jacopo,\n> >>>\n> >>> On Tue, Apr 28, 2020 at 11:06:08PM +0200, Jacopo Mondi wrote:\n> >>>> From: Hans Verkuil <hans.verkuil@cisco.com>\n> >>>>\n> >>>> While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n> >>>> that apps can call to determine that it is indeed a V4L2 device, there\n> >>>> is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n> >>>> solve that, and it will allow utilities like v4l2-compliance to be used\n> >>>> with these devices as well.\n> >>>>\n> >>>> SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n> >>>> subdevice. Define as the initial set of subdev_caps the read-only or\n> >>>> read/write flags, to signal to userspace which set of IOCTLs are\n> >>>> available on the subdevice.\n> >>>>\n> >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n> >>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >>>> ---\n> >>>>  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n> >>>>  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++\n> >>>>  2 files changed, 27 insertions(+)\n> >>>>\n> >>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> >>>> index f3fe515b8ccb..b8c0071aa4d0 100644\n> >>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> >>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> >>>> @@ -15,6 +15,7 @@\n> >>>>  #include <linux/types.h>\n> >>>>  #include <linux/videodev2.h>\n> >>>>  #include <linux/export.h>\n> >>>> +#include <linux/version.h>\n> >>>>\n> >>>>  #include <media/v4l2-ctrls.h>\n> >>>>  #include <media/v4l2-device.h>\n> >>>> @@ -331,6 +332,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> >>>>  \tint rval;\n> >>>>\n> >>>>  \tswitch (cmd) {\n> >>>> +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n> >>>> +\t\tstruct v4l2_subdev_capability *cap = arg;\n> >>>> +\n> >>>> +\t\tmemset(cap, 0, sizeof(*cap));\n> >>>> +\t\tcap->version = LINUX_VERSION_CODE;\n> >>>> +\t\tcap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n> >>>> +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n> >>>> +\n> >>>> +\t\treturn 0;\n> >>>> +\t}\n> >>>> +\n> >>>>  \tcase VIDIOC_QUERYCTRL:\n> >>>>  \t\t/*\n> >>>>  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n> >>>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n> >>>> index 03970ce30741..89dc8f2ba6b3 100644\n> >>>> --- a/include/uapi/linux/v4l2-subdev.h\n> >>>> +++ b/include/uapi/linux/v4l2-subdev.h\n> >>>> @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {\n> >>>>  \t__u32 reserved[8];\n> >>>>  };\n> >>>>\n> >>>> +/**\n> >>>> + * struct v4l2_subdev_capability - subdev capabilities\n> >>>> + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.\n> >>>> + */\n> >>>> +struct v4l2_subdev_capability {\n> >>>> +\t__u32 version;\n> >>>> +\t__u32 subdev_caps;\n> >>>\n> >>> How do you intend to address additional fields being added to the struct in\n> >>> the future? Something else than what's been done in V4L2 traditionally?\n> >>>\n> >>\n> >> I'm not sure I get what you mean here, so I assume I don't know what\n> >> \"has been done in V4L2 traditionally\" and why what I have here goes\n> >> against it...\n> > \n> > I can't help noticing you have no reserved fields in your IOCTL argument\n> > struct. That has generally been the way V4L2 IOCTLs have been extended when\n> > there's been a need to.\n> > \n> > Media controller adopted a different approach to that recently, based on\n> > the argument size. We've discussed doing that on V4L2 but I don't think\n> > a decision has been made.\n> > \n> \n> While I agree that using the argument size to do 'versioning' of the API\n> is a better solution, the fact is that historically we always used a 'reserved'\n> field. And I think we need to do that here as well. It's consistent with\n> the existing subdev ioctls, so I would be in favor of adding a 'u32 reserved[6];'\n> field.\n\nAgreed. Could be even 14, in practice it matters little performance-wise.\n\n> \n> If there are such strong feelings against it that it wouldn't be merged, then\n> we can always just leave it as-is. It's not worth blocking this patch just\n> because of that.\n\nI'm not (strongly) pushing either here; just that we need to make a\ndecision. I'm in favour of the reserved field for the same reason. I was\njust wondering whether it was intentional. :-)\n\n> \n> BTW, one thing that should be changed is the name 'subdev_caps': just name it\n> 'capabilities'. It's a field in a struct v4l2_subdev_capability, so it is\n> already obvious that this is subdev specific.","headers":{"Return-Path":"<sakari.ailus@linux.intel.com>","Received":["from mga11.intel.com (mga11.intel.com [192.55.52.93])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2093C603F2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 May 2020 20:35:07 +0200 (CEST)","from orsmga002.jf.intel.com ([10.7.209.21])\n\tby fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t06 May 2020 11:35:04 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga002-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2020 11:35:01 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid 9F9692074F; Wed,  6 May 2020 21:34:59 +0300 (EEST)"],"IronPort-SDR":["rdADDgaJIIscCBw4pXXwsO749nEaQWuoYY4DU2M3dlKFBkiAEn/7YZ0On/05Y1JPdwos+POGO5\n\tbLX7D5WmzTyQ==","Cx6KS5BlRXQUDRWWNZWh0OohDRgyNkn+SwfXSPjE+isB6JyxUaohzUNk02gMWLfcFfy/lKLYEq\n\t5eCk1R6BmQOw=="],"X-Amp-Result":"SKIPPED(no attachment in message)","X-Amp-File-Uploaded":"False","X-IronPort-AV":"E=Sophos;i=\"5.73,360,1583222400\"; d=\"scan'208\";a=\"278314152\"","Date":"Wed, 6 May 2020 21:34:59 +0300","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","To":"Hans Verkuil <hverkuil-cisco@xs4all.nl>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, linux-media@vger.kernel.org,\n\tlibcamera-devel@lists.libcamera.org, mchehab@kernel.org,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com,\n\tHans Verkuil <hans.verkuil@cisco.com>","Message-ID":"<20200506183459.GA9190@paasikivi.fi.intel.com>","References":"<20200428210609.6793-1-jacopo@jmondi.org>\n\t<20200428210609.6793-6-jacopo@jmondi.org>\n\t<20200428212858.GC5381@paasikivi.fi.intel.com>\n\t<20200429080949.walimwkrth3ixn2o@uno.localdomain>\n\t<20200429081859.GA9190@paasikivi.fi.intel.com>\n\t<15053210-8d02-afbd-0d02-b08f9b5f4378@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<15053210-8d02-afbd-0d02-b08f9b5f4378@xs4all.nl>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 06 May 2020 18:35:09 -0000"}},{"id":4757,"web_url":"https://patchwork.libcamera.org/comment/4757/","msgid":"<5a016d20-6582-1dff-935f-ad49434c5eeb@xs4all.nl>","date":"2020-05-07T07:14:15","subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 06/05/2020 20:34, Sakari Ailus wrote:\n> Hi Hans, Jacopo,\n> \n> On Wed, May 06, 2020 at 03:29:03PM +0200, Hans Verkuil wrote:\n>> On 29/04/2020 10:18, Sakari Ailus wrote:\n>>> Hi Jacopo,\n>>>\n>>> On Wed, Apr 29, 2020 at 10:09:49AM +0200, Jacopo Mondi wrote:\n>>>> Hi Sakari,\n>>>>\n>>>> On Wed, Apr 29, 2020 at 12:28:58AM +0300, Sakari Ailus wrote:\n>>>>> Hi Jacopo,\n>>>>>\n>>>>> On Tue, Apr 28, 2020 at 11:06:08PM +0200, Jacopo Mondi wrote:\n>>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>\n>>>>>>\n>>>>>> While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n>>>>>> that apps can call to determine that it is indeed a V4L2 device, there\n>>>>>> is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n>>>>>> solve that, and it will allow utilities like v4l2-compliance to be used\n>>>>>> with these devices as well.\n>>>>>>\n>>>>>> SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n>>>>>> subdevice. Define as the initial set of subdev_caps the read-only or\n>>>>>> read/write flags, to signal to userspace which set of IOCTLs are\n>>>>>> available on the subdevice.\n>>>>>>\n>>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n>>>>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>>>>>> ---\n>>>>>>  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n>>>>>>  include/uapi/linux/v4l2-subdev.h      | 15 +++++++++++++++\n>>>>>>  2 files changed, 27 insertions(+)\n>>>>>>\n>>>>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n>>>>>> index f3fe515b8ccb..b8c0071aa4d0 100644\n>>>>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n>>>>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n>>>>>> @@ -15,6 +15,7 @@\n>>>>>>  #include <linux/types.h>\n>>>>>>  #include <linux/videodev2.h>\n>>>>>>  #include <linux/export.h>\n>>>>>> +#include <linux/version.h>\n>>>>>>\n>>>>>>  #include <media/v4l2-ctrls.h>\n>>>>>>  #include <media/v4l2-device.h>\n>>>>>> @@ -331,6 +332,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n>>>>>>  \tint rval;\n>>>>>>\n>>>>>>  \tswitch (cmd) {\n>>>>>> +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n>>>>>> +\t\tstruct v4l2_subdev_capability *cap = arg;\n>>>>>> +\n>>>>>> +\t\tmemset(cap, 0, sizeof(*cap));\n>>>>>> +\t\tcap->version = LINUX_VERSION_CODE;\n>>>>>> +\t\tcap->subdev_caps |= ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n>>>>>> +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n>>>>>> +\n>>>>>> +\t\treturn 0;\n>>>>>> +\t}\n>>>>>> +\n>>>>>>  \tcase VIDIOC_QUERYCTRL:\n>>>>>>  \t\t/*\n>>>>>>  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n>>>>>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n>>>>>> index 03970ce30741..89dc8f2ba6b3 100644\n>>>>>> --- a/include/uapi/linux/v4l2-subdev.h\n>>>>>> +++ b/include/uapi/linux/v4l2-subdev.h\n>>>>>> @@ -155,9 +155,24 @@ struct v4l2_subdev_selection {\n>>>>>>  \t__u32 reserved[8];\n>>>>>>  };\n>>>>>>\n>>>>>> +/**\n>>>>>> + * struct v4l2_subdev_capability - subdev capabilities\n>>>>>> + * @device_caps: the subdev capabilities, see V4L2_SUBDEV_CAP_*.\n>>>>>> + */\n>>>>>> +struct v4l2_subdev_capability {\n>>>>>> +\t__u32 version;\n>>>>>> +\t__u32 subdev_caps;\n>>>>>\n>>>>> How do you intend to address additional fields being added to the struct in\n>>>>> the future? Something else than what's been done in V4L2 traditionally?\n>>>>>\n>>>>\n>>>> I'm not sure I get what you mean here, so I assume I don't know what\n>>>> \"has been done in V4L2 traditionally\" and why what I have here goes\n>>>> against it...\n>>>\n>>> I can't help noticing you have no reserved fields in your IOCTL argument\n>>> struct. That has generally been the way V4L2 IOCTLs have been extended when\n>>> there's been a need to.\n>>>\n>>> Media controller adopted a different approach to that recently, based on\n>>> the argument size. We've discussed doing that on V4L2 but I don't think\n>>> a decision has been made.\n>>>\n>>\n>> While I agree that using the argument size to do 'versioning' of the API\n>> is a better solution, the fact is that historically we always used a 'reserved'\n>> field. And I think we need to do that here as well. It's consistent with\n>> the existing subdev ioctls, so I would be in favor of adding a 'u32 reserved[6];'\n>> field.\n> \n> Agreed. Could be even 14, in practice it matters little performance-wise.\n\nTrue. Let's make it 14.\n\nRegards,\n\n\tHans\n\n> \n>>\n>> If there are such strong feelings against it that it wouldn't be merged, then\n>> we can always just leave it as-is. It's not worth blocking this patch just\n>> because of that.\n> \n> I'm not (strongly) pushing either here; just that we need to make a\n> decision. I'm in favour of the reserved field for the same reason. I was\n> just wondering whether it was intentional. :-)\n> \n>>\n>> BTW, one thing that should be changed is the name 'subdev_caps': just name it\n>> 'capabilities'. It's a field in a struct v4l2_subdev_capability, so it is\n>> already obvious that this is subdev specific.\n>","headers":{"Return-Path":"<hverkuil-cisco@xs4all.nl>","Received":["from lb3-smtp-cloud9.xs4all.net (lb3-smtp-cloud9.xs4all.net\n\t[194.109.24.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E35A1600EA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2020 09:14:20 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid WajXjt77j8hmdWajbjESeT; Thu, 07 May 2020 09:14:20 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"BcwGC74k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588835660; bh=+qDNWwp87lKMUHdFLqR7C9oEa5HDPiDUf0yQGM9reW4=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=BcwGC74kEZ4arp0j6SJREbhrlWgXdkMIwUYa1Qr04/MG9QzCU4J+N+zuPNegu0phU\n\tS32TRGQr9ixvNFR9BALLc/ENwwCWDvogSpQt0o4HvgzKB+ybebOzdq+4Oi+HC1Urkm\n\t3VaJvMGE8BsrykbGMnvwQEWwBVgDTz4I4xxm55yRfT9SlVCfkfSHCH7GYf0KZVzEu4\n\tK0BersbtZONYMNwKQtp/Ll2ySoSvIQTQx6M5vESLu00b6xlg6P58qsHfwDOeyYDeAQ\n\tvfHdzf4ePFNEGPulRUd34MwBk7qKRHDmO6+Ct+uhLGfsWiYp2AfC8/vOH3VVg5bdfD\n\tjs3xnZ4k8sjyA==","To":"Sakari Ailus <sakari.ailus@linux.intel.com>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, linux-media@vger.kernel.org,\n\tlibcamera-devel@lists.libcamera.org, mchehab@kernel.org,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com,\n\tHans Verkuil <hans.verkuil@cisco.com>","References":"<20200428210609.6793-1-jacopo@jmondi.org>\n\t<20200428210609.6793-6-jacopo@jmondi.org>\n\t<20200428212858.GC5381@paasikivi.fi.intel.com>\n\t<20200429080949.walimwkrth3ixn2o@uno.localdomain>\n\t<20200429081859.GA9190@paasikivi.fi.intel.com>\n\t<15053210-8d02-afbd-0d02-b08f9b5f4378@xs4all.nl>\n\t<20200506183459.GA9190@paasikivi.fi.intel.com>","From":"Hans Verkuil <hverkuil-cisco@xs4all.nl>","Message-ID":"<5a016d20-6582-1dff-935f-ad49434c5eeb@xs4all.nl>","Date":"Thu, 7 May 2020 09:14:15 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200506183459.GA9190@paasikivi.fi.intel.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfCSmQ0b8oiZu9UvRlLVqzFsS7TmJ1LBnD1fHi4yHm6RgiiiAa87xyI8bkkQvp+ixxvcdxBHRA8Aa3+8TK3iaIduVss1X7xB/2HCXzt1Xujk1IOVxmJDh\n\taWvoBDen5X2oRi3xaKfo28nGTSjgV4N10VFiVDtXxaAH1CnKANwE11gIugfk9OtQZ4eMxPeUxMUZJbM2VO9OIcrFrcX7TcJdmpHAwC23iOvpPewW6JDraSjG\n\tLXX9NTRx/5MqpHuqI8Thqgc+xaSQHn6PVgnoNILhkEI2GtkkyOs0zjLNESR5yyHCvhgTO+vmhooy1gxmHUMUw0wlOcVOgBpDibeLImf5+GIxhOqeBBLMXuFC\n\tCrvdFtdTxfemV68jiGTOrq6Xeccz24Tge6HbxR/oTaMc1IAC5TGfAblL0+u0YvzBVpKcaIeqqO0rH1rErgMMofq30LxaN/DUN2zPoDnupT9vAIOz804=","Subject":"Re: [libcamera-devel] [PATCH v5 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 07 May 2020 07:14:21 -0000"}}]