[RFC PATCH 0/3] Add VIDIOC_SUBDEV_QUERYCAP

[RFC PATCH 0/3] Add VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series adds the VIDIOC_SUBDEV_QUERYCAP ioctl for v4l-subdev devices
as discussed during the ELC in San Jose and as discussed here:

http://www.spinics.net/lists/linux-media/msg88009.html

It also adds the entity_id to v4l2_capability.


Hans Verkuil (3):
  v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
  DocBook/media: document VIDIOC_SUBDEV_QUERYCAP
  videodev2.h: add entity_id to struct v4l2_capability

 Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
 .../DocBook/media/v4l/vidioc-querycap.xml          |  18 ++-
 .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 +++++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c               |   7 ++
 drivers/media/v4l2-core/v4l2-subdev.c              |  19 +++
 include/uapi/linux/v4l2-subdev.h                   |  12 ++
 include/uapi/linux/videodev2.h                     |   5 +-
 7 files changed, 199 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml

-- 
2.1.4

[RFC PATCH 1/3] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@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.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++
 include/uapi/linux/v4l2-subdev.h      | 12 ++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 6359606..2ab1f7d 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -25,6 +25,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>
@@ -187,6 +188,24 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 #endif
 
 	switch (cmd) {
+	case VIDIOC_SUBDEV_QUERYCAP: {
+		struct v4l2_subdev_capability *cap = arg;
+
+		cap->version = LINUX_VERSION_CODE;
+		cap->device_caps = 0;
+		cap->pads = 0;
+		cap->entity_id = 0;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+		if (sd->entity.parent) {
+			cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
+			cap->pads = sd->entity.num_pads;
+			cap->entity_id = sd->entity.id;
+		}
+#endif
+		memset(cap->reserved, 0, sizeof(cap->reserved));
+		break;
+	}
+
 	case VIDIOC_QUERYCTRL:
 		return v4l2_queryctrl(vfh->ctrl_handler, arg);
 
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index dbce2b5..e48b9fd 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -154,9 +154,21 @@ struct v4l2_subdev_selection {
 	__u32 reserved[8];
 };
 
+struct v4l2_subdev_capability {
+	__u32 version;
+	__u32 device_caps;
+	__u32 pads;
+	__u32 entity_id;
+	__u32 reserved[48];
+};
+
+/* This v4l2_subdev is also a media entity and the entity_id field is valid */
+#define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
+
 /* 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)
-- 
2.1.4

[RFC PATCH 2/3] DocBook/media: document VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Add documentation for the new VIDIOC_SUBDEV_QUERYCAP ioctl.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
 .../DocBook/media/v4l/vidioc-querycap.xml          |   2 +-
 .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 +++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml

diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index e98caa1..23607bc 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -669,6 +669,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-subdev-g-fmt;
     &sub-subdev-g-frame-interval;
     &sub-subdev-g-selection;
+    &sub-subdev-querycap;
     &sub-subscribe-event;
     <!-- End of ioctls. -->
     &sub-mmap;
diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
index 20fda75..c1ed844 100644
--- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
@@ -54,7 +54,7 @@ kernel devices compatible with this specification and to obtain
 information about driver and hardware capabilities. The ioctl takes a
 pointer to a &v4l2-capability; which is filled by the driver. When the
 driver is not compatible with this specification the ioctl returns an
-&EINVAL;.</para>
+&ENOTTY;.</para>
 
     <table pgwide="1" frame="none" id="v4l2-capability">
       <title>struct <structname>v4l2_capability</structname></title>
diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
new file mode 100644
index 0000000..a1cbb36
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
@@ -0,0 +1,140 @@
+<refentry id="vidioc-subdev-querycap">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_SUBDEV_QUERYCAP</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_SUBDEV_QUERYCAP</refname>
+    <refpurpose>Query sub-device capabilities</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_subdev_capability *<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_SUBDEV_QUERYCAP</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>All V4L2 sub-devices support the
+<constant>VIDIOC_SUBDEV_QUERYCAP</constant> ioctl. It is used to identify
+kernel devices compatible with this specification and to obtain
+information about driver and hardware capabilities. The ioctl takes a
+pointer to a &v4l2-subdev-capability; which is filled by the driver. When the
+driver is not compatible with this specification the ioctl returns an
+&ENOTTY;.</para>
+
+    <table pgwide="1" frame="none" id="v4l2-subdev-capability">
+      <title>struct <structname>v4l2_subdev_capability</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>version</structfield></entry>
+	    <entry><para>Version number of the driver.</para>
+<para>The version reported is provided by the
+V4L2 subsystem following the kernel numbering scheme. However, it
+may not always return the same version as the kernel if, for example,
+a stable or distribution-modified kernel uses the V4L2 stack from a
+newer kernel.</para>
+<para>The version number is formatted using the
+<constant>KERNEL_VERSION()</constant> macro:</para></entry>
+	  </row>
+	  <row>
+	    <entry spanname="hspan"><para>
+<programlisting>
+#define KERNEL_VERSION(a,b,c) (((a) &lt;&lt; 16) + ((b) &lt;&lt; 8) + (c))
+
+__u32 version = KERNEL_VERSION(0, 8, 1);
+
+printf ("Version: %u.%u.%u\n",
+	(version &gt;&gt; 16) &amp; 0xFF,
+	(version &gt;&gt; 8) &amp; 0xFF,
+	 version &amp; 0xFF);
+</programlisting></para></entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>device_caps</structfield></entry>
+	    <entry>Sub-device capabilities of the opened device, see <xref
+		linkend="subdevice-capabilities" />.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>pads</structfield></entry>
+	    <entry>The number of pads of this sub-device. May be 0 if there are no
+	    pads.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>entity_id</structfield></entry>
+	    <entry>The media controller entity ID of the sub-device. This is only valid if
+	    the <constant>V4L2_SUBDEV_CAP_ENTITY</constant> capability is set.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[48]</entry>
+	    <entry>Reserved for future extensions. Drivers must set
+this array to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table pgwide="1" frame="none" id="subdevice-capabilities">
+      <title>Sub-Device Capabilities Flags</title>
+      <tgroup cols="3">
+	&cs-def;
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>V4L2_SUBDEV_CAP_ENTITY</constant></entry>
+	    <entry>0x00000001</entry>
+	    <entry>The sub-device is a media controller entity and
+	    the <structfield>entity_id</structfield> field of &v4l2-subdev-capability;
+	    is valid.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+  </refsect1>
+</refentry>
-- 
2.1.4

[RFC PATCH 3/3] videodev2.h: add entity_id to struct v4l2_capability

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Export the entity ID (if any) of the video device.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/vidioc-querycap.xml | 16 +++++++++++++++-
 drivers/media/v4l2-core/v4l2-ioctl.c                |  7 +++++++
 include/uapi/linux/videodev2.h                      |  5 ++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
index c1ed844..4a7737c 100644
--- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
@@ -154,7 +154,14 @@ printf ("Version: %u.%u.%u\n",
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[3]</entry>
+	    <entry><structfield>entity_id</structfield></entry>
+	    <entry>The media controller entity ID of the device. This is only valid if
+	    the <constant>V4L2_CAP_ENTITY</constant> capability is set.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[2]</entry>
 	    <entry>Reserved for future extensions. Drivers must set
 this array to zero.</entry>
 	  </row>
@@ -308,6 +315,13 @@ modulator programming see
 fields.</entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_CAP_ENTITY</constant></entry>
+	    <entry>0x00400000</entry>
+	    <entry>The device is a media controller entity and
+	    the <structfield>entity_id</structfield> field of &v4l2-capability;
+	    is valid.</entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_CAP_READWRITE</constant></entry>
 	    <entry>0x01000000</entry>
 	    <entry>The device supports the <link
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 1476602..5179611 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1011,6 +1011,7 @@ static void v4l_sanitize_format(struct v4l2_format *fmt)
 static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
+	struct video_device *vfd = video_devdata(file);
 	struct v4l2_capability *cap = (struct v4l2_capability *)arg;
 	int ret;
 
@@ -1019,6 +1020,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
 	ret = ops->vidioc_querycap(file, fh, cap);
 
 	cap->capabilities |= V4L2_CAP_EXT_PIX_FORMAT;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	if (vfd->entity.parent) {
+		cap->capabilities |= V4L2_CAP_ENTITY;
+		cap->entity_id = vfd->entity.id;
+	}
+#endif
 	/*
 	 * Drivers MUST fill in device_caps, so check for this and
 	 * warn if it was forgotten.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index fa376f7..af7a667 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -307,6 +307,7 @@ struct v4l2_fract {
   * @version:	   KERNEL_VERSION
   * @capabilities: capabilities of the physical device as a whole
   * @device_caps:  capabilities accessed via this particular device (node)
+  * @entity_id:    the media controller entity ID
   * @reserved:	   reserved fields for future extensions
   */
 struct v4l2_capability {
@@ -316,7 +317,8 @@ struct v4l2_capability {
 	__u32   version;
 	__u32	capabilities;
 	__u32	device_caps;
-	__u32	reserved[3];
+	__u32	entity_id;
+	__u32	reserved[2];
 };
 
 /* Values for 'capabilities' field */
@@ -348,6 +350,7 @@ struct v4l2_capability {
 
 #define V4L2_CAP_SDR_CAPTURE		0x00100000  /* Is a SDR capture device */
 #define V4L2_CAP_EXT_PIX_FORMAT		0x00200000  /* Supports the extended pixel format */
+#define V4L2_CAP_ENTITY                 0x00400000  /* This is a Media Controller entity */
 
 #define V4L2_CAP_READWRITE              0x01000000  /* read/write systemcalls */
 #define V4L2_CAP_ASYNCIO                0x02000000  /* async I/O */
-- 
2.1.4

Re: [RFC PATCH 0/3] Add VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

On 05/01/2015 01:33 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch series adds the VIDIOC_SUBDEV_QUERYCAP ioctl for v4l-subdev devices
> as discussed during the ELC in San Jose and as discussed here:
> 
> http://www.spinics.net/lists/linux-media/msg88009.html
> 
> It also adds the entity_id to v4l2_capability.

Question: why do we have CONFIG_VIDEO_V4L2_SUBDEV_API? I don't really see the
point of this and I would propose to remove this config option and instead
use CONFIG_MEDIA_CONTROLLER.

I don't see the use-case of having MEDIA_CONTROLLER defined but not
VIDEO_V4L2_SUBDEV_API.

Comments?

	Hans

> 
> 
> Hans Verkuil (3):
>   v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
>   DocBook/media: document VIDIOC_SUBDEV_QUERYCAP
>   videodev2.h: add entity_id to struct v4l2_capability
> 
>  Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
>  .../DocBook/media/v4l/vidioc-querycap.xml          |  18 ++-
>  .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 +++++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c               |   7 ++
>  drivers/media/v4l2-core/v4l2-subdev.c              |  19 +++
>  include/uapi/linux/v4l2-subdev.h                   |  12 ++
>  include/uapi/linux/videodev2.h                     |   5 +-
>  7 files changed, 199 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
> 

Re: [RFC PATCH 1/3] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Friday 01 May 2015 13:33:48 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@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.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++
>  include/uapi/linux/v4l2-subdev.h      | 12 ++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> b/drivers/media/v4l2-core/v4l2-subdev.c index 6359606..2ab1f7d 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -25,6 +25,7 @@
>  #include <linux/types.h>
>  #include <linux/videodev2.h>
>  #include <linux/export.h>
> +#include <linux/version.h>

Nitpicking, I'd insert version.h between types.h and videodev2.h to keep 
entries alphabetically sorted (I know that export.h is misplaced, but that's 
the only one.).

>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> @@ -187,6 +188,24 @@ static long subdev_do_ioctl(struct file *file, unsigned
> int cmd, void *arg) #endif
> 
>  	switch (cmd) {
> +	case VIDIOC_SUBDEV_QUERYCAP: {
> +		struct v4l2_subdev_capability *cap = arg;
> +
> +		cap->version = LINUX_VERSION_CODE;
> +		cap->device_caps = 0;
> +		cap->pads = 0;
> +		cap->entity_id = 0;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +		if (sd->entity.parent) {
> +			cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
> +			cap->pads = sd->entity.num_pads;
> +			cap->entity_id = sd->entity.id;
> +		}
> +#endif
> +		memset(cap->reserved, 0, sizeof(cap->reserved));
> +		break;
> +	}
> +
>  	case VIDIOC_QUERYCTRL:
>  		return v4l2_queryctrl(vfh->ctrl_handler, arg);
> 
> diff --git a/include/uapi/linux/v4l2-subdev.h
> b/include/uapi/linux/v4l2-subdev.h index dbce2b5..e48b9fd 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -154,9 +154,21 @@ struct v4l2_subdev_selection {
>  	__u32 reserved[8];
>  };
> 
> +struct v4l2_subdev_capability {
> +	__u32 version;
> +	__u32 device_caps;
> +	__u32 pads;

If we restrict pad number reporting to subdevs that are also entities, should 
we report the number of pads at all here ? Applications could find it out 
through the MC API using the entity ID. I don't have a too strong opinion on 
this for now, but we should consider whether reporting the same information 
through two different means wouldn't cause issues.

> +	__u32 entity_id;
> +	__u32 reserved[48];
> +};
> +
> +/* This v4l2_subdev is also a media entity and the entity_id field is valid
> */ +#define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
> +
>  /* 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

Re: [RFC PATCH 2/3] DocBook/media: document VIDIOC_SUBDEV_QUERYCAP

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Friday 01 May 2015 13:33:49 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add documentation for the new VIDIOC_SUBDEV_QUERYCAP ioctl.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
>  .../DocBook/media/v4l/vidioc-querycap.xml          |   2 +-
>  .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 ++++++++++++++++++
>  3 files changed, 142 insertions(+), 1 deletion(-)
>  create mode 100644
> Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
> 
> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml
> b/Documentation/DocBook/media/v4l/v4l2.xml index e98caa1..23607bc 100644
> --- a/Documentation/DocBook/media/v4l/v4l2.xml
> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> @@ -669,6 +669,7 @@ and discussions on the V4L mailing list.</revremark>
>      &sub-subdev-g-fmt;
>      &sub-subdev-g-frame-interval;
>      &sub-subdev-g-selection;
> +    &sub-subdev-querycap;
>      &sub-subscribe-event;
>      <!-- End of ioctls. -->
>      &sub-mmap;
> diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> b/Documentation/DocBook/media/v4l/vidioc-querycap.xml index
> 20fda75..c1ed844 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> @@ -54,7 +54,7 @@ kernel devices compatible with this specification and to
> obtain information about driver and hardware capabilities. The ioctl takes
> a pointer to a &v4l2-capability; which is filled by the driver. When the
> driver is not compatible with this specification the ioctl returns an
> -&EINVAL;.</para>
> +&ENOTTY;.</para>

I'd split this change to a separate patch as it's unrelated to 
VIDIOC_SUBDEV_QUERYCAP.

We can't really guarantee that non-V4L2 drivers will return -ENOTTY, they 
might be buggy and return a different error code. That's slightly nitpicking 
though.

>      <table pgwide="1" frame="none" id="v4l2-capability">
>        <title>struct <structname>v4l2_capability</structname></title>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
> b/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml new file mode
> 100644
> index 0000000..a1cbb36
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
> @@ -0,0 +1,140 @@
> +<refentry id="vidioc-subdev-querycap">
> +  <refmeta>
> +    <refentrytitle>ioctl VIDIOC_SUBDEV_QUERYCAP</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +
> +  <refnamediv>
> +    <refname>VIDIOC_SUBDEV_QUERYCAP</refname>
> +    <refpurpose>Query sub-device capabilities</refpurpose>
> +  </refnamediv>
> +
> +  <refsynopsisdiv>
> +    <funcsynopsis>
> +      <funcprototype>
> +	<funcdef>int <function>ioctl</function></funcdef>
> +	<paramdef>int <parameter>fd</parameter></paramdef>
> +	<paramdef>int <parameter>request</parameter></paramdef>
> +	<paramdef>struct v4l2_subdev_capability
> *<parameter>argp</parameter></paramdef>
> +      </funcprototype>
> +    </funcsynopsis>
> +  </refsynopsisdiv>
> +
> +  <refsect1>
> +    <title>Arguments</title>
> +
> +    <variablelist>
> +      <varlistentry>
> +	<term><parameter>fd</parameter></term>
> +	<listitem>
> +	  <para>&fd;</para>
> +	</listitem>
> +      </varlistentry>
> +      <varlistentry>
> +	<term><parameter>request</parameter></term>
> +	<listitem>
> +	  <para>VIDIOC_SUBDEV_QUERYCAP</para>
> +	</listitem>
> +      </varlistentry>
> +      <varlistentry>
> +	<term><parameter>argp</parameter></term>
> +	<listitem>
> +	  <para></para>
> +	</listitem>
> +      </varlistentry>
> +    </variablelist>
> +  </refsect1>
> +
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>All V4L2 sub-devices support the
> +<constant>VIDIOC_SUBDEV_QUERYCAP</constant> ioctl. It is used to identify
> +kernel devices compatible with this specification and to obtain
> +information about driver and hardware capabilities. The ioctl takes a
> +pointer to a &v4l2-subdev-capability; which is filled by the driver. When
> the
> +driver is not compatible with this specification the ioctl returns an
> +&ENOTTY;.</para>
> +
> +    <table pgwide="1" frame="none" id="v4l2-subdev-capability">
> +      <title>struct <structname>v4l2_subdev_capability</structname></title>
> +      <tgroup cols="3">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>version</structfield></entry>
> +	    <entry><para>Version number of the driver.</para>
> +<para>The version reported is provided by the
> +V4L2 subsystem following the kernel numbering scheme. However, it
> +may not always return the same version as the kernel if, for example,
> +a stable or distribution-modified kernel uses the V4L2 stack from a
> +newer kernel.</para>
> +<para>The version number is formatted using the
> +<constant>KERNEL_VERSION()</constant> macro:</para></entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="hspan"><para>
> +<programlisting>
> +#define KERNEL_VERSION(a,b,c) (((a) &lt;&lt; 16) + ((b) &lt;&lt; 8) + (c))
> +
> +__u32 version = KERNEL_VERSION(0, 8, 1);
> +
> +printf ("Version: %u.%u.%u\n",
> +	(version &gt;&gt; 16) &amp; 0xFF,
> +	(version &gt;&gt; 8) &amp; 0xFF,
> +	 version &amp; 0xFF);
> +</programlisting></para></entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>device_caps</structfield></entry>
> +	    <entry>Sub-device capabilities of the opened device, see <xref
> +		linkend="subdevice-capabilities" />.
> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>pads</structfield></entry>
> +	    <entry>The number of pads of this sub-device. May be 0 if there are 
no
> +	    pads.

Should we mention explicitly that the pads field is only valid if 
V4L2_SUBDEV_CAP_ENTITY is set ?

> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>entity_id</structfield></entry>
> +	    <entry>The media controller entity ID of the sub-device. This is only
> valid if
> +	    the <constant>V4L2_SUBDEV_CAP_ENTITY</constant> capability is set.
> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>reserved</structfield>[48]</entry>
> +	    <entry>Reserved for future extensions. Drivers must set
> +this array to zero.</entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
> +    <table pgwide="1" frame="none" id="subdevice-capabilities">
> +      <title>Sub-Device Capabilities Flags</title>
> +      <tgroup cols="3">
> +	&cs-def;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry><constant>V4L2_SUBDEV_CAP_ENTITY</constant></entry>
> +	    <entry>0x00000001</entry>
> +	    <entry>The sub-device is a media controller entity and
> +	    the <structfield>entity_id</structfield> field of
> &v4l2-subdev-capability;
> +	    is valid.</entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +  </refsect1>
> +
> +  <refsect1>
> +    &return-value;
> +  </refsect1>
> +</refentry>

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 3/3] videodev2.h: add entity_id to struct v4l2_capability

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Friday 01 May 2015 13:33:50 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Export the entity ID (if any) of the video device.

I would postpone this until we finish the DVB+MC discussion and properly 
define the relationship between device nodes and MC, as it could have 
implications on the V4L2 side as well.

> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/DocBook/media/v4l/vidioc-querycap.xml | 16 +++++++++++++++-
>  drivers/media/v4l2-core/v4l2-ioctl.c                |  7 +++++++
>  include/uapi/linux/videodev2.h                      |  5 ++++-
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> b/Documentation/DocBook/media/v4l/vidioc-querycap.xml index
> c1ed844..4a7737c 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> @@ -154,7 +154,14 @@ printf ("Version: %u.%u.%u\n",
>  	  </row>
>  	  <row>
>  	    <entry>__u32</entry>
> -	    <entry><structfield>reserved</structfield>[3]</entry>
> +	    <entry><structfield>entity_id</structfield></entry>
> +	    <entry>The media controller entity ID of the device. This is only
> valid if
> +	    the <constant>V4L2_CAP_ENTITY</constant> capability is set.
> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>reserved</structfield>[2]</entry>
>  	    <entry>Reserved for future extensions. Drivers must set
>  this array to zero.</entry>
>  	  </row>
> @@ -308,6 +315,13 @@ modulator programming see
>  fields.</entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_CAP_ENTITY</constant></entry>
> +	    <entry>0x00400000</entry>
> +	    <entry>The device is a media controller entity and
> +	    the <structfield>entity_id</structfield> field of &v4l2-capability;
> +	    is valid.</entry>
> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_CAP_READWRITE</constant></entry>
>  	    <entry>0x01000000</entry>
>  	    <entry>The device supports the <link
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c index 1476602..5179611 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1011,6 +1011,7 @@ static void v4l_sanitize_format(struct v4l2_format
> *fmt) static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
>  				struct file *file, void *fh, void *arg)
>  {
> +	struct video_device *vfd = video_devdata(file);
>  	struct v4l2_capability *cap = (struct v4l2_capability *)arg;
>  	int ret;
> 
> @@ -1019,6 +1020,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops
> *ops, ret = ops->vidioc_querycap(file, fh, cap);
> 
>  	cap->capabilities |= V4L2_CAP_EXT_PIX_FORMAT;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	if (vfd->entity.parent) {
> +		cap->capabilities |= V4L2_CAP_ENTITY;
> +		cap->entity_id = vfd->entity.id;
> +	}
> +#endif
>  	/*
>  	 * Drivers MUST fill in device_caps, so check for this and
>  	 * warn if it was forgotten.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index fa376f7..af7a667 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -307,6 +307,7 @@ struct v4l2_fract {
>    * @version:	   KERNEL_VERSION
>    * @capabilities: capabilities of the physical device as a whole
>    * @device_caps:  capabilities accessed via this particular device (node)
> +  * @entity_id:    the media controller entity ID
>    * @reserved:	   reserved fields for future extensions
>    */
>  struct v4l2_capability {
> @@ -316,7 +317,8 @@ struct v4l2_capability {
>  	__u32   version;
>  	__u32	capabilities;
>  	__u32	device_caps;
> -	__u32	reserved[3];
> +	__u32	entity_id;
> +	__u32	reserved[2];
>  };
> 
>  /* Values for 'capabilities' field */
> @@ -348,6 +350,7 @@ struct v4l2_capability {
> 
>  #define V4L2_CAP_SDR_CAPTURE		0x00100000  /* Is a SDR capture device */
>  #define V4L2_CAP_EXT_PIX_FORMAT		0x00200000  /* Supports the extended 
pixel
> format */ +#define V4L2_CAP_ENTITY                 0x00400000  /* This is a
> Media Controller entity */
> 
>  #define V4L2_CAP_READWRITE              0x01000000  /* read/write
> systemcalls */ #define V4L2_CAP_ASYNCIO                0x02000000  /* async
> I/O */

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 0/3] Add VIDIOC_SUBDEV_QUERYCAP

[Laurent Pinchart]

Hi Hans,

On Friday 01 May 2015 13:41:03 Hans Verkuil wrote:
> On 05/01/2015 01:33 PM, Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > This patch series adds the VIDIOC_SUBDEV_QUERYCAP ioctl for v4l-subdev
> > devices as discussed during the ELC in San Jose and as discussed here:
> > 
> > http://www.spinics.net/lists/linux-media/msg88009.html
> > 
> > It also adds the entity_id to v4l2_capability.
> 
> Question: why do we have CONFIG_VIDEO_V4L2_SUBDEV_API? I don't really see
> the point of this and I would propose to remove this config option and
> instead use CONFIG_MEDIA_CONTROLLER.
> 
> I don't see the use-case of having MEDIA_CONTROLLER defined but not
> VIDEO_V4L2_SUBDEV_API.
> 
> Comments?

The idea is to compile the subdev userspace API code out when not needed. Not 
all media controller drivers need that API.

> > Hans Verkuil (3):
> >   v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
> >   DocBook/media: document VIDIOC_SUBDEV_QUERYCAP
> >   videodev2.h: add entity_id to struct v4l2_capability
> >  
> >  Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
> >  .../DocBook/media/v4l/vidioc-querycap.xml          |  18 ++-
> >  .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 ++++++++++++++++
> >  drivers/media/v4l2-core/v4l2-ioctl.c               |   7 ++
> >  drivers/media/v4l2-core/v4l2-subdev.c              |  19 +++
> >  include/uapi/linux/v4l2-subdev.h                   |  12 ++
> >  include/uapi/linux/videodev2.h                     |   5 +-
> >  7 files changed, 199 insertions(+), 3 deletions(-)
> >  create mode 100644
> >  Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 2/3] DocBook/media: document VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

On 05/04/2015 12:29 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Friday 01 May 2015 13:33:49 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add documentation for the new VIDIOC_SUBDEV_QUERYCAP ioctl.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
>>  .../DocBook/media/v4l/vidioc-querycap.xml          |   2 +-
>>  .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 ++++++++++++++++++
>>  3 files changed, 142 insertions(+), 1 deletion(-)
>>  create mode 100644
>> Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
>>
>> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml
>> b/Documentation/DocBook/media/v4l/v4l2.xml index e98caa1..23607bc 100644
>> --- a/Documentation/DocBook/media/v4l/v4l2.xml
>> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
>> @@ -669,6 +669,7 @@ and discussions on the V4L mailing list.</revremark>
>>      &sub-subdev-g-fmt;
>>      &sub-subdev-g-frame-interval;
>>      &sub-subdev-g-selection;
>> +    &sub-subdev-querycap;
>>      &sub-subscribe-event;
>>      <!-- End of ioctls. -->
>>      &sub-mmap;
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
>> b/Documentation/DocBook/media/v4l/vidioc-querycap.xml index
>> 20fda75..c1ed844 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
>> @@ -54,7 +54,7 @@ kernel devices compatible with this specification and to
>> obtain information about driver and hardware capabilities. The ioctl takes
>> a pointer to a &v4l2-capability; which is filled by the driver. When the
>> driver is not compatible with this specification the ioctl returns an
>> -&EINVAL;.</para>
>> +&ENOTTY;.</para>
> 
> I'd split this change to a separate patch as it's unrelated to 
> VIDIOC_SUBDEV_QUERYCAP.

You are right. I just happened to come across this while adding the subdev-querycap
text.

> We can't really guarantee that non-V4L2 drivers will return -ENOTTY, they 
> might be buggy and return a different error code. That's slightly nitpicking 
> though.

Well, ENOTTY is much more likely than EINVAL :-) But I can replace it with:

"...returns an error, most likely &ENOTTY;." and use the same phrase for
subdev-querycap.

> 
>>      <table pgwide="1" frame="none" id="v4l2-capability">
>>        <title>struct <structname>v4l2_capability</structname></title>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
>> b/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml new file mode
>> 100644
>> index 0000000..a1cbb36
>> --- /dev/null
>> +++ b/Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
>> @@ -0,0 +1,140 @@
>> +<refentry id="vidioc-subdev-querycap">
>> +  <refmeta>
>> +    <refentrytitle>ioctl VIDIOC_SUBDEV_QUERYCAP</refentrytitle>
>> +    &manvol;
>> +  </refmeta>
>> +
>> +  <refnamediv>
>> +    <refname>VIDIOC_SUBDEV_QUERYCAP</refname>
>> +    <refpurpose>Query sub-device capabilities</refpurpose>
>> +  </refnamediv>
>> +
>> +  <refsynopsisdiv>
>> +    <funcsynopsis>
>> +      <funcprototype>
>> +	<funcdef>int <function>ioctl</function></funcdef>
>> +	<paramdef>int <parameter>fd</parameter></paramdef>
>> +	<paramdef>int <parameter>request</parameter></paramdef>
>> +	<paramdef>struct v4l2_subdev_capability
>> *<parameter>argp</parameter></paramdef>
>> +      </funcprototype>
>> +    </funcsynopsis>
>> +  </refsynopsisdiv>
>> +
>> +  <refsect1>
>> +    <title>Arguments</title>
>> +
>> +    <variablelist>
>> +      <varlistentry>
>> +	<term><parameter>fd</parameter></term>
>> +	<listitem>
>> +	  <para>&fd;</para>
>> +	</listitem>
>> +      </varlistentry>
>> +      <varlistentry>
>> +	<term><parameter>request</parameter></term>
>> +	<listitem>
>> +	  <para>VIDIOC_SUBDEV_QUERYCAP</para>
>> +	</listitem>
>> +      </varlistentry>
>> +      <varlistentry>
>> +	<term><parameter>argp</parameter></term>
>> +	<listitem>
>> +	  <para></para>
>> +	</listitem>
>> +      </varlistentry>
>> +    </variablelist>
>> +  </refsect1>
>> +
>> +  <refsect1>
>> +    <title>Description</title>
>> +
>> +    <para>All V4L2 sub-devices support the
>> +<constant>VIDIOC_SUBDEV_QUERYCAP</constant> ioctl. It is used to identify
>> +kernel devices compatible with this specification and to obtain
>> +information about driver and hardware capabilities. The ioctl takes a
>> +pointer to a &v4l2-subdev-capability; which is filled by the driver. When
>> the
>> +driver is not compatible with this specification the ioctl returns an
>> +&ENOTTY;.</para>
>> +
>> +    <table pgwide="1" frame="none" id="v4l2-subdev-capability">
>> +      <title>struct <structname>v4l2_subdev_capability</structname></title>
>> +      <tgroup cols="3">
>> +	&cs-str;
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>version</structfield></entry>
>> +	    <entry><para>Version number of the driver.</para>
>> +<para>The version reported is provided by the
>> +V4L2 subsystem following the kernel numbering scheme. However, it
>> +may not always return the same version as the kernel if, for example,
>> +a stable or distribution-modified kernel uses the V4L2 stack from a
>> +newer kernel.</para>
>> +<para>The version number is formatted using the
>> +<constant>KERNEL_VERSION()</constant> macro:</para></entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry spanname="hspan"><para>
>> +<programlisting>
>> +#define KERNEL_VERSION(a,b,c) (((a) &lt;&lt; 16) + ((b) &lt;&lt; 8) + (c))
>> +
>> +__u32 version = KERNEL_VERSION(0, 8, 1);
>> +
>> +printf ("Version: %u.%u.%u\n",
>> +	(version &gt;&gt; 16) &amp; 0xFF,
>> +	(version &gt;&gt; 8) &amp; 0xFF,
>> +	 version &amp; 0xFF);
>> +</programlisting></para></entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>device_caps</structfield></entry>
>> +	    <entry>Sub-device capabilities of the opened device, see <xref
>> +		linkend="subdevice-capabilities" />.
>> +	    </entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>pads</structfield></entry>
>> +	    <entry>The number of pads of this sub-device. May be 0 if there are 
> no
>> +	    pads.
> 
> Should we mention explicitly that the pads field is only valid if 
> V4L2_SUBDEV_CAP_ENTITY is set ?

Yes, we can do that. I checked that all subdev drivers that create a v4l-subdev
node are also a media entity. I wasn't sure about that before.

Regards,

	Hans

> 
>> +	    </entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>entity_id</structfield></entry>
>> +	    <entry>The media controller entity ID of the sub-device. This is only
>> valid if
>> +	    the <constant>V4L2_SUBDEV_CAP_ENTITY</constant> capability is set.
>> +	    </entry>
>> +	  </row>
>> +	  <row>
>> +	    <entry>__u32</entry>
>> +	    <entry><structfield>reserved</structfield>[48]</entry>
>> +	    <entry>Reserved for future extensions. Drivers must set
>> +this array to zero.</entry>
>> +	  </row>
>> +	</tbody>
>> +      </tgroup>
>> +    </table>
>> +
>> +    <table pgwide="1" frame="none" id="subdevice-capabilities">
>> +      <title>Sub-Device Capabilities Flags</title>
>> +      <tgroup cols="3">
>> +	&cs-def;
>> +	<tbody valign="top">
>> +	  <row>
>> +	    <entry><constant>V4L2_SUBDEV_CAP_ENTITY</constant></entry>
>> +	    <entry>0x00000001</entry>
>> +	    <entry>The sub-device is a media controller entity and
>> +	    the <structfield>entity_id</structfield> field of
>> &v4l2-subdev-capability;
>> +	    is valid.</entry>
>> +	  </row>
>> +	</tbody>
>> +      </tgroup>
>> +    </table>
>> +  </refsect1>
>> +
>> +  <refsect1>
>> +    &return-value;
>> +  </refsect1>
>> +</refentry>
> 

Re: [RFC PATCH 1/3] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

On 05/04/2015 12:20 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Friday 01 May 2015 13:33:48 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@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.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++
>>  include/uapi/linux/v4l2-subdev.h      | 12 ++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
>> b/drivers/media/v4l2-core/v4l2-subdev.c index 6359606..2ab1f7d 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/types.h>
>>  #include <linux/videodev2.h>
>>  #include <linux/export.h>
>> +#include <linux/version.h>
> 
> Nitpicking, I'd insert version.h between types.h and videodev2.h to keep 
> entries alphabetically sorted (I know that export.h is misplaced, but that's 
> the only one.).
> 
>>  #include <media/v4l2-ctrls.h>
>>  #include <media/v4l2-device.h>
>> @@ -187,6 +188,24 @@ static long subdev_do_ioctl(struct file *file, unsigned
>> int cmd, void *arg) #endif
>>
>>  	switch (cmd) {
>> +	case VIDIOC_SUBDEV_QUERYCAP: {
>> +		struct v4l2_subdev_capability *cap = arg;
>> +
>> +		cap->version = LINUX_VERSION_CODE;
>> +		cap->device_caps = 0;
>> +		cap->pads = 0;
>> +		cap->entity_id = 0;
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +		if (sd->entity.parent) {
>> +			cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
>> +			cap->pads = sd->entity.num_pads;
>> +			cap->entity_id = sd->entity.id;
>> +		}
>> +#endif
>> +		memset(cap->reserved, 0, sizeof(cap->reserved));
>> +		break;
>> +	}
>> +
>>  	case VIDIOC_QUERYCTRL:
>>  		return v4l2_queryctrl(vfh->ctrl_handler, arg);
>>
>> diff --git a/include/uapi/linux/v4l2-subdev.h
>> b/include/uapi/linux/v4l2-subdev.h index dbce2b5..e48b9fd 100644
>> --- a/include/uapi/linux/v4l2-subdev.h
>> +++ b/include/uapi/linux/v4l2-subdev.h
>> @@ -154,9 +154,21 @@ struct v4l2_subdev_selection {
>>  	__u32 reserved[8];
>>  };
>>
>> +struct v4l2_subdev_capability {
>> +	__u32 version;
>> +	__u32 device_caps;
>> +	__u32 pads;
> 
> If we restrict pad number reporting to subdevs that are also entities, should 
> we report the number of pads at all here ? Applications could find it out 
> through the MC API using the entity ID. I don't have a too strong opinion on 
> this for now, but we should consider whether reporting the same information 
> through two different means wouldn't cause issues.

My problem is that that would require e.g. v4l2-compliance to hunt for the
media device when all it needs is the number of pads. I don't mind dropping
this, but then we need a good way to find the associated media device (and
sysfs is not a good way).

The goal is to be able to write v4l2-compliance tests, so I need this info
one way or another.

Regards,

	Hans

> 
>> +	__u32 entity_id;
>> +	__u32 reserved[48];
>> +};
>> +
>> +/* This v4l2_subdev is also a media entity and the entity_id field is valid
>> */ +#define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
>> +
>>  /* 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)
> 

Re: [RFC PATCH 0/3] Add VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

On 05/04/2015 12:33 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday 01 May 2015 13:41:03 Hans Verkuil wrote:
>> On 05/01/2015 01:33 PM, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> This patch series adds the VIDIOC_SUBDEV_QUERYCAP ioctl for v4l-subdev
>>> devices as discussed during the ELC in San Jose and as discussed here:
>>>
>>> http://www.spinics.net/lists/linux-media/msg88009.html
>>>
>>> It also adds the entity_id to v4l2_capability.
>>
>> Question: why do we have CONFIG_VIDEO_V4L2_SUBDEV_API? I don't really see
>> the point of this and I would propose to remove this config option and
>> instead use CONFIG_MEDIA_CONTROLLER.
>>
>> I don't see the use-case of having MEDIA_CONTROLLER defined but not
>> VIDEO_V4L2_SUBDEV_API.
>>
>> Comments?
> 
> The idea is to compile the subdev userspace API code out when not needed. Not 
> all media controller drivers need that API.

Sure, but it is not a lot of code, and I think it is somewhat confusing.

Just my opinion, though.

	Hans

> 
>>> Hans Verkuil (3):
>>>   v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
>>>   DocBook/media: document VIDIOC_SUBDEV_QUERYCAP
>>>   videodev2.h: add entity_id to struct v4l2_capability
>>>  
>>>  Documentation/DocBook/media/v4l/v4l2.xml           |   1 +
>>>  .../DocBook/media/v4l/vidioc-querycap.xml          |  18 ++-
>>>  .../DocBook/media/v4l/vidioc-subdev-querycap.xml   | 140 ++++++++++++++++
>>>  drivers/media/v4l2-core/v4l2-ioctl.c               |   7 ++
>>>  drivers/media/v4l2-core/v4l2-subdev.c              |  19 +++
>>>  include/uapi/linux/v4l2-subdev.h                   |  12 ++
>>>  include/uapi/linux/videodev2.h                     |   5 +-
>>>  7 files changed, 199 insertions(+), 3 deletions(-)
>>>  create mode 100644
>>>  Documentation/DocBook/media/v4l/vidioc-subdev-querycap.xml
> 

Re: [RFC PATCH 1/3] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Sakari Ailus]

Hi Hans,

Thanks for the patch!

On Fri, May 01, 2015 at 01:33:48PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@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.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++
>  include/uapi/linux/v4l2-subdev.h      | 12 ++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 6359606..2ab1f7d 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -25,6 +25,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>
> @@ -187,6 +188,24 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  #endif
>  
>  	switch (cmd) {
> +	case VIDIOC_SUBDEV_QUERYCAP: {
> +		struct v4l2_subdev_capability *cap = arg;
> +
> +		cap->version = LINUX_VERSION_CODE;
> +		cap->device_caps = 0;
> +		cap->pads = 0;
> +		cap->entity_id = 0;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +		if (sd->entity.parent) {
> +			cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
> +			cap->pads = sd->entity.num_pads;
> +			cap->entity_id = sd->entity.id;
> +		}
> +#endif
> +		memset(cap->reserved, 0, sizeof(cap->reserved));
> +		break;
> +	}
> +
>  	case VIDIOC_QUERYCTRL:
>  		return v4l2_queryctrl(vfh->ctrl_handler, arg);
>  
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index dbce2b5..e48b9fd 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -154,9 +154,21 @@ struct v4l2_subdev_selection {
>  	__u32 reserved[8];
>  };
>  
> +struct v4l2_subdev_capability {
> +	__u32 version;
> +	__u32 device_caps;

This is called capabilities in struct v4l2_capability. I'd follow the same
pattern.

> +	__u32 pads;
> +	__u32 entity_id;

What's the use case for the entity_id field btw.? Supposing that the user
wouldn't be using the MC interface to obtain it, is the entity_id relevant
in this context? Or is your intent first open the sub-device, and then find
out more information on the entity?

> +	__u32 reserved[48];

Why 48?

As memory is typically allocated in powers of two (or n^2 + (n-1)^2), how
about aligning it accordingly? I don't think we lose anything by making this
e.g. 60. Although 28 would probably suffice as well (or 29 with the pads
field removed as discussed). Even that much sounds like a lot.

> +};
> +
> +/* This v4l2_subdev is also a media entity and the entity_id field is valid */
> +#define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
> +
>  /* 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)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [RFC PATCH 1/3] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

Hi Sakari,

On 07/02/15 15:01, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks for the patch!
> 
> On Fri, May 01, 2015 at 01:33:48PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@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.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++
>>  include/uapi/linux/v4l2-subdev.h      | 12 ++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index 6359606..2ab1f7d 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -25,6 +25,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>
>> @@ -187,6 +188,24 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>>  #endif
>>  
>>  	switch (cmd) {
>> +	case VIDIOC_SUBDEV_QUERYCAP: {
>> +		struct v4l2_subdev_capability *cap = arg;
>> +
>> +		cap->version = LINUX_VERSION_CODE;
>> +		cap->device_caps = 0;
>> +		cap->pads = 0;
>> +		cap->entity_id = 0;
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> +		if (sd->entity.parent) {
>> +			cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
>> +			cap->pads = sd->entity.num_pads;
>> +			cap->entity_id = sd->entity.id;
>> +		}
>> +#endif
>> +		memset(cap->reserved, 0, sizeof(cap->reserved));
>> +		break;
>> +	}
>> +
>>  	case VIDIOC_QUERYCTRL:
>>  		return v4l2_queryctrl(vfh->ctrl_handler, arg);
>>  
>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
>> index dbce2b5..e48b9fd 100644
>> --- a/include/uapi/linux/v4l2-subdev.h
>> +++ b/include/uapi/linux/v4l2-subdev.h
>> @@ -154,9 +154,21 @@ struct v4l2_subdev_selection {
>>  	__u32 reserved[8];
>>  };
>>  
>> +struct v4l2_subdev_capability {
>> +	__u32 version;
>> +	__u32 device_caps;
> 
> This is called capabilities in struct v4l2_capability. I'd follow the same
> pattern.

No, it's called device_caps in struct v4l2_capability. The capabilities field
in that same structure contains the capabilities of the device as a whole: so
not just of the device node in question, but the union of the capabilities of
all device nodes created by the driver.

> 
>> +	__u32 pads;
>> +	__u32 entity_id;
> 
> What's the use case for the entity_id field btw.? Supposing that the user
> wouldn't be using the MC interface to obtain it, is the entity_id relevant
> in this context? Or is your intent first open the sub-device, and then find
> out more information on the entity?

Right, the latter. I want to be able to do v4l2-compliance -d /dev/v4l-subdev0
and it should be able to figure everything out from there.

> 
>> +	__u32 reserved[48];
> 
> Why 48?

Can't remember :-)

> 
> As memory is typically allocated in powers of two (or n^2 + (n-1)^2), how
> about aligning it accordingly? I don't think we lose anything by making this
> e.g. 60. Although 28 would probably suffice as well (or 29 with the pads
> field removed as discussed). Even that much sounds like a lot.

29 would work fine as well. In general I am more generous with reserved fields
these days since we've hit the limits of it too often.

Regards,

	Hans

> 
>> +};
>> +
>> +/* This v4l2_subdev is also a media entity and the entity_id field is valid */
>> +#define V4L2_SUBDEV_CAP_ENTITY		(1 << 0)
>> +
>>  /* 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)
>