[PATCH 0/3] document types of hardware control for V4L2

[PATCH 0/3] document types of hardware control for V4L2

[Mauro Carvalho Chehab]

On 2010, we introduced a new way to control complex V4L2 devices used
on embedded systems, but this was never documented, nor it is possible
for an userspace applicatin to detect the kind of control a device supports.

This series fill the gap.

Mauro Carvalho Chehab (3):
  media: open.rst: document devnode-centric and mc-centric types
  media: videodev2: add a flag for vdev-centric devices
  media: add V4L2_CAP_VDEV_CENTERED flag on vdev-centric drivers

 Documentation/media/uapi/v4l/open.rst            | 56 ++++++++++++++++++++++++
 Documentation/media/uapi/v4l/vidioc-querycap.rst |  4 ++
 drivers/media/pci/bt8xx/bttv-driver.c            |  4 +-
 drivers/media/pci/cobalt/cobalt-v4l2.c           |  3 +-
 drivers/media/pci/cx18/cx18-ioctl.c              |  4 +-
 drivers/media/pci/cx23885/cx23885-417.c          |  2 +-
 drivers/media/pci/cx23885/cx23885-video.c        |  3 +-
 drivers/media/pci/cx25821/cx25821-video.c        |  6 ++-
 drivers/media/pci/cx88/cx88-video.c              |  3 +-
 drivers/media/pci/dt3155/dt3155.c                |  3 +-
 drivers/media/pci/ivtv/ivtv-ioctl.c              |  5 ++-
 drivers/media/pci/meye/meye.c                    |  2 +-
 drivers/media/pci/saa7134/saa7134-video.c        |  3 +-
 drivers/media/pci/saa7164/saa7164-encoder.c      |  3 +-
 drivers/media/pci/saa7164/saa7164-vbi.c          |  3 +-
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c   |  3 +-
 drivers/media/pci/solo6x10/solo6x10-v4l2.c       |  3 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c          |  3 +-
 drivers/media/pci/tw5864/tw5864-video.c          |  2 +-
 drivers/media/pci/tw68/tw68-video.c              |  3 +-
 drivers/media/pci/tw686x/tw686x-video.c          |  2 +-
 drivers/media/pci/zoran/zoran_driver.c           |  3 +-
 drivers/media/platform/rcar_drif.c               |  3 +-
 drivers/media/platform/vivid/vivid-core.c        |  2 +-
 drivers/media/usb/airspy/airspy.c                |  3 +-
 drivers/media/usb/au0828/au0828-video.c          |  3 +-
 drivers/media/usb/cpia2/cpia2_v4l.c              |  5 ++-
 drivers/media/usb/cx231xx/cx231xx-video.c        |  5 ++-
 drivers/media/usb/em28xx/em28xx-video.c          | 11 +++--
 drivers/media/usb/go7007/go7007-v4l2.c           |  2 +-
 drivers/media/usb/gspca/gspca.c                  |  3 +-
 drivers/media/usb/hackrf/hackrf.c                |  8 ++--
 drivers/media/usb/hdpvr/hdpvr-video.c            |  2 +-
 drivers/media/usb/msi2500/msi2500.c              |  3 +-
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c         |  6 ++-
 drivers/media/usb/pwc/pwc-v4l.c                  |  2 +-
 drivers/media/usb/s2255/s2255drv.c               |  2 +-
 drivers/media/usb/stk1160/stk1160-v4l.c          |  3 +-
 drivers/media/usb/stkwebcam/stk-webcam.c         |  3 +-
 drivers/media/usb/tm6000/tm6000-video.c          |  5 ++-
 drivers/media/usb/usbtv/usbtv-video.c            |  2 +-
 drivers/media/usb/usbvision/usbvision-video.c    |  5 ++-
 drivers/media/usb/uvc/uvc_v4l2.c                 |  8 ++--
 drivers/media/usb/zr364xx/zr364xx.c              |  5 ++-
 include/uapi/linux/videodev2.h                   |  2 +
 45 files changed, 158 insertions(+), 58 deletions(-)

-- 
2.13.3

[PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Mauro Carvalho Chehab]

From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>

When we added support for omap3, back in 2010, we added a new
type of V4L2 devices that aren't fully controlled via the V4L2
device node. Yet, we never made it clear, at the V4L2 spec,
about the differences between both types.

Let's document them with the current implementation.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst | 50 +++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index afd116edb40d..a72d142897c0 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -6,6 +6,56 @@
 Opening and Closing Devices
 ***************************
 
+Types of V4L2 hardware control
+==============================
+
+V4L2 devices are usually complex: they are implemented via a main driver and
+often several additional drivers. The main driver always exposes one or
+more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).
+
+The other drivers are called **V4L2 sub-devices** and provide control to
+other parts of the hardware usually connected via a serial bus (like
+I²C, SMBus or SPI). They can be implicitly controlled directly by the
+main driver or explicitly through via the **V4L2 sub-device API** interface.
+
+When V4L2 was originally designed, there was only one type of device control.
+The entire device was controlled via the **V4L2 device nodes**. We refer to
+this kind of control as **V4L2 device node centric** (or, simply,
+**vdev-centric**).
+
+Since the end of 2010, a new type of V4L2 device control was added in order
+to support complex devices that are common for embedded systems. Those
+devices are controlled mainly via the media controller and sub-devices.
+So, they're called: **Media controller centric** (or, simply,
+"**MC-centric**").
+
+For **vdev-centric** control, the device and their corresponding hardware
+pipelines are controlled via the **V4L2 device** node. They may optionally
+expose via the :ref:`media controller API <media_controller>`.
+
+For **MC-centric** control, before using the V4L2 device, it is required to
+set the hardware pipelines via the
+:ref:`media controller API <media_controller>`. For those devices, the
+sub-devices' configuration can be controlled via the
+:ref:`sub-device API <subdev>`, with creates one device node per sub device.
+
+In summary, for **MC-centric** devices:
+
+- The **V4L2 device** node is responsible for controlling the streaming
+  features;
+- The **media controller device** is responsible to setup the pipelines;
+- The **V4L2 sub-devices** are responsible for sub-device
+  specific settings.
+
+.. note::
+
+   A **vdev-centric** may optionally expose V4L2 sub-devices via
+   :ref:`sub-device API <subdev>`. In that case, it has to implement
+   the :ref:`media controller API <media_controller>` as well.
+
+
+
+.. _v4l2_device_naming:
 
 Device Naming
 =============
-- 
2.13.3

[PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

As both vdev-centric and mc-centric devices may implement the
same APIs, we need a flag to allow userspace to distinguish
between them.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst            | 6 ++++++
 Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
 include/uapi/linux/videodev2.h                   | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index a72d142897c0..eb3f0ec57edb 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
 pipelines are controlled via the **V4L2 device** node. They may optionally
 expose via the :ref:`media controller API <media_controller>`.
 
+.. note::
+
+   **vdev-centric** devices should report V4L2_VDEV_CENTERED
+   :c:type:`v4l2_capability` flag (see :ref:`VIDIOC_QUERYCAP`).
+
+
 For **MC-centric** control, before using the V4L2 device, it is required to
 set the hardware pipelines via the
 :ref:`media controller API <media_controller>`. For those devices, the
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 12e0d9a63cd8..4856821b7608 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -252,6 +252,10 @@ specification the ioctl returns an ``EINVAL`` error code.
     * - ``V4L2_CAP_TOUCH``
       - 0x10000000
       - This is a touch device.
+    * - ``V4L2_VDEV_CENTERED``
+      - 0x20000000
+      - This is controlled via V4L2 device nodes (radio, video, vbi,
+        sdr
     * - ``V4L2_CAP_DEVICE_CAPS``
       - 0x80000000
       - The driver fills the ``device_caps`` field. This capability can
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45cf7359822c..d89090d99042 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -460,6 +460,8 @@ struct v4l2_capability {
 
 #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
 
+#define V4L2_CAP_VDEV_CENTERED          0x20000000  /* V4L2 Device is controlled via V4L2 device devnode */
+
 #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
 
 /*
-- 
2.13.3

[PATCH 3/3] media: add V4L2_CAP_VDEV_CENTERED flag on vdev-centric drivers

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Those devices are controlled via their V4L2 device. Add a
flag to indicate them as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 drivers/media/pci/bt8xx/bttv-driver.c          |  4 +++-
 drivers/media/pci/cobalt/cobalt-v4l2.c         |  3 ++-
 drivers/media/pci/cx18/cx18-ioctl.c            |  4 ++--
 drivers/media/pci/cx23885/cx23885-417.c        |  2 +-
 drivers/media/pci/cx23885/cx23885-video.c      |  3 ++-
 drivers/media/pci/cx25821/cx25821-video.c      |  6 ++++--
 drivers/media/pci/cx88/cx88-video.c            |  3 ++-
 drivers/media/pci/dt3155/dt3155.c              |  3 ++-
 drivers/media/pci/ivtv/ivtv-ioctl.c            |  5 +++--
 drivers/media/pci/meye/meye.c                  |  2 +-
 drivers/media/pci/saa7134/saa7134-video.c      |  3 ++-
 drivers/media/pci/saa7164/saa7164-encoder.c    |  3 ++-
 drivers/media/pci/saa7164/saa7164-vbi.c        |  3 ++-
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c |  3 ++-
 drivers/media/pci/solo6x10/solo6x10-v4l2.c     |  3 ++-
 drivers/media/pci/sta2x11/sta2x11_vip.c        |  3 ++-
 drivers/media/pci/tw5864/tw5864-video.c        |  2 +-
 drivers/media/pci/tw68/tw68-video.c            |  3 ++-
 drivers/media/pci/tw686x/tw686x-video.c        |  2 +-
 drivers/media/pci/zoran/zoran_driver.c         |  3 ++-
 drivers/media/platform/rcar_drif.c             |  3 ++-
 drivers/media/platform/vivid/vivid-core.c      |  2 +-
 drivers/media/usb/airspy/airspy.c              |  3 ++-
 drivers/media/usb/au0828/au0828-video.c        |  3 ++-
 drivers/media/usb/cpia2/cpia2_v4l.c            |  5 +++--
 drivers/media/usb/cx231xx/cx231xx-video.c      |  5 +++--
 drivers/media/usb/em28xx/em28xx-video.c        | 11 +++++++----
 drivers/media/usb/go7007/go7007-v4l2.c         |  2 +-
 drivers/media/usb/gspca/gspca.c                |  3 ++-
 drivers/media/usb/hackrf/hackrf.c              |  8 +++++---
 drivers/media/usb/hdpvr/hdpvr-video.c          |  2 +-
 drivers/media/usb/msi2500/msi2500.c            |  3 ++-
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c       |  6 ++++--
 drivers/media/usb/pwc/pwc-v4l.c                |  2 +-
 drivers/media/usb/s2255/s2255drv.c             |  2 +-
 drivers/media/usb/stk1160/stk1160-v4l.c        |  3 ++-
 drivers/media/usb/stkwebcam/stk-webcam.c       |  3 ++-
 drivers/media/usb/tm6000/tm6000-video.c        |  5 +++--
 drivers/media/usb/usbtv/usbtv-video.c          |  2 +-
 drivers/media/usb/usbvision/usbvision-video.c  |  5 +++--
 drivers/media/usb/uvc/uvc_v4l2.c               |  8 +++++---
 drivers/media/usb/zr364xx/zr364xx.c            |  5 +++--
 42 files changed, 96 insertions(+), 58 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 40110be4e986..382cc76b954b 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -2481,6 +2481,7 @@ static int bttv_querycap(struct file *file, void  *priv,
 		V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_READWRITE |
 		V4L2_CAP_STREAMING |
+		V4L2_CAP_VDEV_CENTERED |
 		V4L2_CAP_DEVICE_CAPS;
 	if (no_overlay <= 0)
 		cap->capabilities |= V4L2_CAP_VIDEO_OVERLAY;
@@ -2511,7 +2512,8 @@ static int bttv_querycap(struct file *file, void  *priv,
 			 V4L2_CAP_STREAMING |
 			 V4L2_CAP_TUNER);
 	else {
-		cap->device_caps = V4L2_CAP_RADIO | V4L2_CAP_TUNER;
+		cap->device_caps = V4L2_CAP_RADIO | V4L2_CAP_TUNER |
+				   V4L2_CAP_VDEV_CENTERED;
 		if (btv->has_saa6588)
 			cap->device_caps |= V4L2_CAP_READWRITE |
 						V4L2_CAP_RDS_CAPTURE;
diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index def4a3b37084..803a9cf09a9f 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -495,7 +495,8 @@ static int cobalt_querycap(struct file *file, void *priv_fh,
 	strlcpy(vcap->card, "cobalt", sizeof(vcap->card));
 	snprintf(vcap->bus_info, sizeof(vcap->bus_info),
 		 "PCIe:%s", pci_name(cobalt->pci_dev));
-	vcap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+	vcap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_READWRITE |
+			    V4L2_CAP_VDEV_CENTERED;
 	if (s->is_output)
 		vcap->device_caps |= V4L2_CAP_VIDEO_OUTPUT;
 	else
diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c
index 80b902b12a78..3e258fdc6685 100644
--- a/drivers/media/pci/cx18/cx18-ioctl.c
+++ b/drivers/media/pci/cx18/cx18-ioctl.c
@@ -402,8 +402,8 @@ static int cx18_querycap(struct file *file, void *fh,
 	snprintf(vcap->bus_info, sizeof(vcap->bus_info),
 		 "PCI:%s", pci_name(cx->pci_dev));
 	vcap->capabilities = cx->v4l2_cap;	/* capabilities */
-	vcap->device_caps = s->v4l2_dev_caps;	/* device capabilities */
-	vcap->capabilities |= V4L2_CAP_DEVICE_CAPS;
+	vcap->device_caps = V4L2_CAP_VDEV_CENTERED | s->v4l2_dev_caps;	/* device capabilities */
+	vcap->capabilities |= V4L2_CAP_DEVICE_CAPS | V4L2_CAP_VDEV_CENTERED;
 	return 0;
 }
 
diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index a71f3c7569ce..69a35e8ee894 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1334,7 +1334,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 		sizeof(cap->card));
 	sprintf(cap->bus_info, "PCIe:%s", pci_name(dev->pci));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-			   V4L2_CAP_STREAMING;
+			   V4L2_CAP_STREAMING | V4L2_CAP_VDEV_CENTERED;
 	if (dev->tuner_type != TUNER_ABSENT)
 		cap->device_caps |= V4L2_CAP_TUNER;
 	cap->capabilities = cap->device_caps | V4L2_CAP_VBI_CAPTURE |
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index ecc580af0148..073c4201de34 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -642,7 +642,8 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	strlcpy(cap->card, cx23885_boards[dev->board].name,
 		sizeof(cap->card));
 	sprintf(cap->bus_info, "PCIe:%s", pci_name(dev->pci));
-	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING | V4L2_CAP_AUDIO;
+	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
+			   V4L2_CAP_AUDIO | V4L2_CAP_VDEV_CENTERED;
 	if (dev->tuner_type != TUNER_ABSENT)
 		cap->device_caps |= V4L2_CAP_TUNER;
 	if (vdev->vfl_type == VFL_TYPE_VBI)
diff --git a/drivers/media/pci/cx25821/cx25821-video.c b/drivers/media/pci/cx25821/cx25821-video.c
index dbaf42ec26cd..6356f823bac2 100644
--- a/drivers/media/pci/cx25821/cx25821-video.c
+++ b/drivers/media/pci/cx25821/cx25821-video.c
@@ -438,8 +438,10 @@ static int cx25821_vidioc_querycap(struct file *file, void *priv,
 	struct cx25821_channel *chan = video_drvdata(file);
 	struct cx25821_dev *dev = chan->dev;
 	const u32 cap_input = V4L2_CAP_VIDEO_CAPTURE |
-			V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
-	const u32 cap_output = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_READWRITE;
+			      V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
+			      V4L2_CAP_VDEV_CENTERED;
+	const u32 cap_output = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_READWRITE |
+			       V4L2_CAP_VDEV_CENTERED;
 
 	strcpy(cap->driver, "cx25821");
 	strlcpy(cap->card, cx25821_boards[dev->board].name, sizeof(cap->card));
diff --git a/drivers/media/pci/cx88/cx88-video.c b/drivers/media/pci/cx88/cx88-video.c
index 7d25ecd4404b..51fa90f56a3e 100644
--- a/drivers/media/pci/cx88/cx88-video.c
+++ b/drivers/media/pci/cx88/cx88-video.c
@@ -812,7 +812,8 @@ void cx88_querycap(struct file *file, struct cx88_core *core,
 	struct video_device *vdev = video_devdata(file);
 
 	strlcpy(cap->card, core->board.name, sizeof(cap->card));
-	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
+			   V4L2_CAP_VDEV_CENTERED;
 	if (core->board.tuner_type != UNSET)
 		cap->device_caps |= V4L2_CAP_TUNER;
 	switch (vdev->vfl_type) {
diff --git a/drivers/media/pci/dt3155/dt3155.c b/drivers/media/pci/dt3155/dt3155.c
index 6a219694b225..77771e070338 100644
--- a/drivers/media/pci/dt3155/dt3155.c
+++ b/drivers/media/pci/dt3155/dt3155.c
@@ -311,7 +311,8 @@ static int dt3155_querycap(struct file *filp, void *p,
 	strcpy(cap->card, DT3155_NAME " frame grabber");
 	sprintf(cap->bus_info, "PCI:%s", pci_name(pd->pdev));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-		V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+			   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 670462d195b5..d8d25f4ccac7 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -750,8 +750,9 @@ static int ivtv_querycap(struct file *file, void *fh, struct v4l2_capability *vc
 	strlcpy(vcap->driver, IVTV_DRIVER_NAME, sizeof(vcap->driver));
 	strlcpy(vcap->card, itv->card_name, sizeof(vcap->card));
 	snprintf(vcap->bus_info, sizeof(vcap->bus_info), "PCI:%s", pci_name(itv->pdev));
-	vcap->capabilities = itv->v4l2_cap | V4L2_CAP_DEVICE_CAPS;
-	vcap->device_caps = s->caps;
+	vcap->capabilities = itv->v4l2_cap | V4L2_CAP_VDEV_CENTERED
+			     | V4L2_CAP_DEVICE_CAPS;
+	vcap->device_caps = s->caps | V4L2_CAP_VDEV_CENTERED;
 	if ((s->caps & V4L2_CAP_VIDEO_OUTPUT_OVERLAY) &&
 	    !itv->osd_video_pbase) {
 		vcap->capabilities &= ~V4L2_CAP_VIDEO_OUTPUT_OVERLAY;
diff --git a/drivers/media/pci/meye/meye.c b/drivers/media/pci/meye/meye.c
index 0fe76bea2393..d13723c9af2c 100644
--- a/drivers/media/pci/meye/meye.c
+++ b/drivers/media/pci/meye/meye.c
@@ -1024,7 +1024,7 @@ static int vidioc_querycap(struct file *file, void *fh,
 	sprintf(cap->bus_info, "PCI:%s", pci_name(meye.mchip_dev));
 
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-			    V4L2_CAP_STREAMING;
+			   V4L2_CAP_STREAMING | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 51d42bbf969e..4b7449622db1 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1507,7 +1507,8 @@ int saa7134_querycap(struct file *file, void *priv,
 		sizeof(cap->card));
 	sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
 
-	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING
+			   | V4L2_CAP_VDEV_CENTERED;
 	if ((tuner_type != TUNER_ABSENT) && (tuner_type != UNSET))
 		cap->device_caps |= V4L2_CAP_TUNER;
 
diff --git a/drivers/media/pci/saa7164/saa7164-encoder.c b/drivers/media/pci/saa7164/saa7164-encoder.c
index f21c245a54f7..fa6e63607868 100644
--- a/drivers/media/pci/saa7164/saa7164-encoder.c
+++ b/drivers/media/pci/saa7164/saa7164-encoder.c
@@ -505,7 +505,8 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	cap->device_caps =
 		V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_READWRITE |
-		V4L2_CAP_TUNER;
+		V4L2_CAP_TUNER |
+		V4L2_CAP_VDEV_CENTERED;
 
 	cap->capabilities = cap->device_caps |
 		V4L2_CAP_VBI_CAPTURE |
diff --git a/drivers/media/pci/saa7164/saa7164-vbi.c b/drivers/media/pci/saa7164/saa7164-vbi.c
index 9255d7d23947..a86e1fcfc792 100644
--- a/drivers/media/pci/saa7164/saa7164-vbi.c
+++ b/drivers/media/pci/saa7164/saa7164-vbi.c
@@ -216,7 +216,8 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	cap->device_caps =
 		V4L2_CAP_VBI_CAPTURE |
 		V4L2_CAP_READWRITE |
-		V4L2_CAP_TUNER;
+		V4L2_CAP_TUNER |
+		V4L2_CAP_VDEV_CENTERED;
 
 	cap->capabilities = cap->device_caps |
 		V4L2_CAP_VIDEO_CAPTURE |
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 25f9f2ebff1d..dce9ee5086c7 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -781,7 +781,8 @@ static int solo_enc_querycap(struct file *file, void  *priv,
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(solo_dev->pdev));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-			V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+			   V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
index 3266fc21825f..b832e607e0fd 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
@@ -388,7 +388,8 @@ static int solo_querycap(struct file *file, void  *priv,
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(solo_dev->pdev));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-			V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+			   V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 6343d24eb1d5..37badd722d94 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -419,7 +419,8 @@ static int vidioc_querycap(struct file *file, void *priv,
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(vip->pdev));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-			   V4L2_CAP_STREAMING;
+			   V4L2_CAP_STREAMING |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
diff --git a/drivers/media/pci/tw5864/tw5864-video.c b/drivers/media/pci/tw5864/tw5864-video.c
index e7bd2b8484e3..2cb94183de82 100644
--- a/drivers/media/pci/tw5864/tw5864-video.c
+++ b/drivers/media/pci/tw5864/tw5864-video.c
@@ -930,7 +930,7 @@ static const struct video_device tw5864_video_template = {
 	.release = video_device_release_empty,
 	.tvnorms = TW5864_NORMS,
 	.device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-		V4L2_CAP_STREAMING,
+		       V4L2_CAP_STREAMING | V4L2_CAP_VDEV_CENTERED,
 };
 
 /* Motion Detection Threshold matrix */
diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 58c4dd75bfa1..8b9645e67703 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -741,7 +741,8 @@ static int tw68_querycap(struct file *file, void  *priv,
 	cap->device_caps =
 		V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_READWRITE |
-		V4L2_CAP_STREAMING;
+		V4L2_CAP_STREAMING |
+		V4L2_CAP_VDEV_CENTERED;
 
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index c3fafa97b2d0..1915cf3365ee 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -770,7 +770,7 @@ static int tw686x_querycap(struct file *file, void *priv,
 	snprintf(cap->bus_info, sizeof(cap->bus_info),
 		 "PCI:%s", pci_name(dev->pci_dev));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
-			   V4L2_CAP_READWRITE;
+			   V4L2_CAP_READWRITE | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/pci/zoran/zoran_driver.c b/drivers/media/pci/zoran/zoran_driver.c
index a11cb501c550..9d9725aec8fb 100644
--- a/drivers/media/pci/zoran/zoran_driver.c
+++ b/drivers/media/pci/zoran/zoran_driver.c
@@ -1514,7 +1514,8 @@ static int zoran_querycap(struct file *file, void *__fh, struct v4l2_capability
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(zr->pci_dev));
 	cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE |
-			   V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VIDEO_OVERLAY;
+			   V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VIDEO_OVERLAY |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
index 522364ff0d5d..2faba25dec8e 100644
--- a/drivers/media/platform/rcar_drif.c
+++ b/drivers/media/platform/rcar_drif.c
@@ -1079,7 +1079,8 @@ static int rcar_drif_sdr_register(struct rcar_drif_sdr *sdr)
 	sdr->vdev->ctrl_handler = &sdr->ctrl_hdl;
 	sdr->vdev->v4l2_dev = &sdr->v4l2_dev;
 	sdr->vdev->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_TUNER |
-		V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+				 V4L2_CAP_STREAMING | V4L2_CAP_READWRITE |
+				 V4L2_CAP_VDEV_CENTERED;
 	video_set_drvdata(sdr->vdev, sdr);
 
 	/* Register V4L2 SDR device */
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index ef344b9a48af..2286b3088e5f 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -1172,7 +1172,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 			 "vivid-%03d-vid-cap", inst);
 		vfd->fops = &vivid_fops;
 		vfd->ioctl_ops = &vivid_ioctl_ops;
-		vfd->device_caps = dev->vid_cap_caps;
+		vfd->device_caps = dev->vid_cap_caps | V4L2_CAP_VDEV_CENTERED;
 		vfd->release = video_device_release_empty;
 		vfd->v4l2_dev = &dev->v4l2_dev;
 		vfd->queue = &dev->vb_vid_cap_q;
diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c
index 07f3f4e7144a..ab8ff78d2088 100644
--- a/drivers/media/usb/airspy/airspy.c
+++ b/drivers/media/usb/airspy/airspy.c
@@ -623,7 +623,8 @@ static int airspy_querycap(struct file *file, void *fh,
 	strlcpy(cap->card, s->vdev.name, sizeof(cap->card));
 	usb_make_path(s->udev, cap->bus_info, sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_STREAMING |
-			V4L2_CAP_READWRITE | V4L2_CAP_TUNER;
+			   V4L2_CAP_READWRITE | V4L2_CAP_TUNER |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 9342402b92f7..b5e53f8252b4 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1199,7 +1199,8 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	cap->device_caps = V4L2_CAP_AUDIO |
 		V4L2_CAP_READWRITE |
 		V4L2_CAP_STREAMING |
-		V4L2_CAP_TUNER;
+		V4L2_CAP_TUNER |
+		V4L2_CAP_VDEV_CENTERED;
 	if (vdev->vfl_type == VFL_TYPE_GRABBER)
 		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE;
 	else
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 7122023e7004..b73f44f31253 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -261,8 +261,9 @@ static int cpia2_querycap(struct file *file, void *fh, struct v4l2_capability *v
 		memset(vc->bus_info,0, sizeof(vc->bus_info));
 
 	vc->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-			   V4L2_CAP_READWRITE |
-			   V4L2_CAP_STREAMING;
+			  V4L2_CAP_READWRITE |
+			  V4L2_CAP_STREAMING |
+			  V4L2_CAP_VDEV_CENTERED;
 	vc->capabilities = vc->device_caps |
 			   V4L2_CAP_DEVICE_CAPS;
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 179b8481a870..2255a8135337 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1558,9 +1558,10 @@ int cx231xx_querycap(struct file *file, void *priv,
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
 
 	if (vdev->vfl_type == VFL_TYPE_RADIO)
-		cap->device_caps = V4L2_CAP_RADIO;
+		cap->device_caps = V4L2_CAP_RADIO | V4L2_CAP_VDEV_CENTERED;
 	else {
-		cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+		cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING
+				   | V4L2_CAP_VDEV_CENTERED;
 		if (vdev->vfl_type == VFL_TYPE_VBI)
 			cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
 		else
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 8d253a5df0a9..7b3026dbfa71 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1872,11 +1872,13 @@ static int vidioc_querycap(struct file *file, void  *priv,
 
 	if (vdev->vfl_type == VFL_TYPE_GRABBER)
 		cap->device_caps = V4L2_CAP_READWRITE |
-			V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+			 	   V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+				   V4L2_CAP_VDEV_CENTERED;
 	else if (vdev->vfl_type == VFL_TYPE_RADIO)
-		cap->device_caps = V4L2_CAP_RADIO;
+		cap->device_caps = V4L2_CAP_RADIO | V4L2_CAP_VDEV_CENTERED;
 	else
-		cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_VBI_CAPTURE;
+		cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_VBI_CAPTURE |
+				   V4L2_CAP_VDEV_CENTERED;
 
 	if (dev->int_audio_type != EM28XX_INT_AUDIO_NONE)
 		cap->device_caps |= V4L2_CAP_AUDIO;
@@ -1885,7 +1887,8 @@ static int vidioc_querycap(struct file *file, void  *priv,
 		cap->device_caps |= V4L2_CAP_TUNER;
 
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS |
-		V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+			    V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE |
+			    V4L2_CAP_STREAMING;
 	if (video_is_registered(&v4l2->vbi_dev))
 		cap->capabilities |= V4L2_CAP_VBI_CAPTURE;
 	if (video_is_registered(&v4l2->radio_dev))
diff --git a/drivers/media/usb/go7007/go7007-v4l2.c b/drivers/media/usb/go7007/go7007-v4l2.c
index 445f17b850c5..fa3f3508844f 100644
--- a/drivers/media/usb/go7007/go7007-v4l2.c
+++ b/drivers/media/usb/go7007/go7007-v4l2.c
@@ -289,7 +289,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	strlcpy(cap->bus_info, go->bus_info, sizeof(cap->bus_info));
 
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-				V4L2_CAP_STREAMING;
+			   V4L2_CAP_STREAMING | V4L2_CAP_VDEV_CENTERED;
 
 	if (go->board_info->num_aud_inputs)
 		cap->device_caps |= V4L2_CAP_AUDIO;
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 0f141762abf1..30813741e706 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1345,7 +1345,8 @@ static int vidioc_querycap(struct file *file, void  *priv,
 			sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE
 			  | V4L2_CAP_STREAMING
-			  | V4L2_CAP_READWRITE;
+			  | V4L2_CAP_READWRITE
+			  | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
index a41b305c55d4..1a0905651450 100644
--- a/drivers/media/usb/hackrf/hackrf.c
+++ b/drivers/media/usb/hackrf/hackrf.c
@@ -911,16 +911,18 @@ static int hackrf_querycap(struct file *file, void *fh,
 
 	if (vdev->vfl_dir == VFL_DIR_RX)
 		cap->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_TUNER |
-				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE |
+				   V4L2_CAP_VDEV_CENTERED;
 
 	else
 		cap->device_caps = V4L2_CAP_SDR_OUTPUT | V4L2_CAP_MODULATOR |
-				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE |
+				   V4L2_CAP_VDEV_CENTERED;
 
 	cap->capabilities = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_TUNER |
 			    V4L2_CAP_SDR_OUTPUT | V4L2_CAP_MODULATOR |
 			    V4L2_CAP_STREAMING | V4L2_CAP_READWRITE |
-			    V4L2_CAP_DEVICE_CAPS;
+			    V4L2_CAP_VDEV_CENTERED | V4L2_CAP_DEVICE_CAPS;
 	strlcpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
 	strlcpy(cap->card, dev->rx_vdev.name, sizeof(cap->card));
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index 7fb036d6a86e..8ee4f78ada94 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -582,7 +582,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	strcpy(cap->card, "Hauppauge HD PVR");
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_AUDIO |
-			    V4L2_CAP_READWRITE;
+			    V4L2_CAP_READWRITE | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/msi2500/msi2500.c b/drivers/media/usb/msi2500/msi2500.c
index 79bfd2dbe649..2c5f65a994e2 100644
--- a/drivers/media/usb/msi2500/msi2500.c
+++ b/drivers/media/usb/msi2500/msi2500.c
@@ -608,7 +608,8 @@ static int msi2500_querycap(struct file *file, void *fh,
 	strlcpy(cap->card, dev->vdev.name, sizeof(cap->card));
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_STREAMING |
-			V4L2_CAP_READWRITE | V4L2_CAP_TUNER;
+			   V4L2_CAP_READWRITE | V4L2_CAP_TUNER |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index 8f13c60198ed..debc99b5f179 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -145,7 +145,8 @@ static int pvr2_querycap(struct file *file, void *priv, struct v4l2_capability *
 	strlcpy(cap->card, pvr2_hdw_get_desc(hdw), sizeof(cap->card));
 	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
 			    V4L2_CAP_AUDIO | V4L2_CAP_RADIO |
-			    V4L2_CAP_READWRITE | V4L2_CAP_DEVICE_CAPS;
+			    V4L2_CAP_READWRITE | V4L2_CAP_VDEV_CENTERED |
+			    V4L2_CAP_DEVICE_CAPS;
 	switch (fh->pdi->devbase.vfl_type) {
 	case VFL_TYPE_GRABBER:
 		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_AUDIO;
@@ -154,7 +155,8 @@ static int pvr2_querycap(struct file *file, void *priv, struct v4l2_capability *
 		cap->device_caps = V4L2_CAP_RADIO;
 		break;
 	}
-	cap->device_caps |= V4L2_CAP_TUNER | V4L2_CAP_READWRITE;
+	cap->device_caps |= V4L2_CAP_TUNER | V4L2_CAP_READWRITE |
+			    V4L2_CAP_VDEV_CENTERED;
 	return 0;
 }
 
diff --git a/drivers/media/usb/pwc/pwc-v4l.c b/drivers/media/usb/pwc/pwc-v4l.c
index 043b2b97cee6..3066ca5ff21e 100644
--- a/drivers/media/usb/pwc/pwc-v4l.c
+++ b/drivers/media/usb/pwc/pwc-v4l.c
@@ -496,7 +496,7 @@ static int pwc_querycap(struct file *file, void *fh, struct v4l2_capability *cap
 	strlcpy(cap->card, pdev->vdev.name, sizeof(cap->card));
 	usb_make_path(pdev->udev, cap->bus_info, sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
-					V4L2_CAP_READWRITE;
+			   V4L2_CAP_READWRITE | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 23f606e7cd73..a82b7df0e271 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -733,7 +733,7 @@ static int vidioc_querycap(struct file *file, void *priv,
 	strlcpy(cap->card, "s2255", sizeof(cap->card));
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
-		V4L2_CAP_READWRITE;
+			   V4L2_CAP_READWRITE | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index a132faa590df..23908a688e30 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -350,7 +350,8 @@ static int vidioc_querycap(struct file *file,
 	cap->device_caps =
 		V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_STREAMING |
-		V4L2_CAP_READWRITE;
+		V4L2_CAP_READWRITE |
+		V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
index 39abb58c65dd..5aee0f110f59 100644
--- a/drivers/media/usb/stkwebcam/stk-webcam.c
+++ b/drivers/media/usb/stkwebcam/stk-webcam.c
@@ -797,7 +797,8 @@ static int stk_vidioc_querycap(struct file *filp,
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
 
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE
-		| V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+			   | V4L2_CAP_READWRITE | V4L2_CAP_STREAMING
+			   | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
diff --git a/drivers/media/usb/tm6000/tm6000-video.c b/drivers/media/usb/tm6000/tm6000-video.c
index ec8c4d2534dc..2ca27e53a4a7 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -877,9 +877,10 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	if (vdev->vfl_type == VFL_TYPE_GRABBER)
 		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE |
 				V4L2_CAP_STREAMING |
-				V4L2_CAP_READWRITE;
+				V4L2_CAP_READWRITE |
+				V4L2_CAP_VDEV_CENTERED;
 	else
-		cap->device_caps |= V4L2_CAP_RADIO;
+		cap->device_caps |= V4L2_CAP_RADIO | V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS |
 		V4L2_CAP_RADIO | V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE;
 
diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c
index 8135614f395a..724dbcb8de9b 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -520,7 +520,7 @@ static int usbtv_querycap(struct file *file, void *priv,
 	strlcpy(cap->driver, "usbtv", sizeof(cap->driver));
 	strlcpy(cap->card, "usbtv", sizeof(cap->card));
 	usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info));
-	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE;
+	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VDEV_CENTERED;
 	cap->device_caps |= V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
diff --git a/drivers/media/usb/usbvision/usbvision-video.c b/drivers/media/usb/usbvision/usbvision-video.c
index 756322c4ac05..816dbb7ba29f 100644
--- a/drivers/media/usb/usbvision/usbvision-video.c
+++ b/drivers/media/usb/usbvision/usbvision-video.c
@@ -476,9 +476,10 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	vc->device_caps = usbvision->have_tuner ? V4L2_CAP_TUNER : 0;
 	if (vdev->vfl_type == VFL_TYPE_GRABBER)
 		vc->device_caps |= V4L2_CAP_VIDEO_CAPTURE |
-			V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+				   V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
+				   V4L2_CAP_VDEV_CENTERED;
 	else
-		vc->device_caps |= V4L2_CAP_RADIO;
+		vc->device_caps |= V4L2_CAP_RADIO | V4L2_CAP_VDEV_CENTERED;
 
 	vc->capabilities = vc->device_caps | V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_READWRITE | V4L2_CAP_STREAMING | V4L2_CAP_DEVICE_CAPS;
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..95430bb8d2a4 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -567,11 +567,13 @@ static int uvc_ioctl_querycap(struct file *file, void *fh,
 	strlcpy(cap->card, vdev->name, sizeof(cap->card));
 	usb_make_path(stream->dev->udev, cap->bus_info, sizeof(cap->bus_info));
 	cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
-			  | chain->caps;
+			  V4L2_CAP_VDEV_CENTERED | chain->caps;
 	if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+		cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING
+				   | V4L2_CAP_VDEV_CENTERED;
 	else
-		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+		cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING
+				   | V4L2_CAP_VDEV_CENTERED;
 
 	return 0;
 }
diff --git a/drivers/media/usb/zr364xx/zr364xx.c b/drivers/media/usb/zr364xx/zr364xx.c
index d4bb56baad9b..bb3597bce912 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -710,8 +710,9 @@ static int zr364xx_vidioc_querycap(struct file *file, void *priv,
 	strlcpy(cap->bus_info, dev_name(&cam->udev->dev),
 		sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-			    V4L2_CAP_READWRITE |
-			    V4L2_CAP_STREAMING;
+			   V4L2_CAP_READWRITE |
+			   V4L2_CAP_STREAMING |
+			   V4L2_CAP_VDEV_CENTERED;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
-- 
2.13.3

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Hans Verkuil]

On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> As both vdev-centric and mc-centric devices may implement the
> same APIs, we need a flag to allow userspace to distinguish
> between them.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
>  include/uapi/linux/videodev2.h                   | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index a72d142897c0..eb3f0ec57edb 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
>  pipelines are controlled via the **V4L2 device** node. They may optionally
>  expose via the :ref:`media controller API <media_controller>`.
>  
> +.. note::
> +
> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED

You mean CENTRIC, not CENTERED.

But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
so it makes a lot more sense to keep that as the default and only set the cap for
MC-centric drivers.

Regards,

	Hans

> +   :c:type:`v4l2_capability` flag (see :ref:`VIDIOC_QUERYCAP`).
> +
> +
>  For **MC-centric** control, before using the V4L2 device, it is required to
>  set the hardware pipelines via the
>  :ref:`media controller API <media_controller>`. For those devices, the
> diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> index 12e0d9a63cd8..4856821b7608 100644
> --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> @@ -252,6 +252,10 @@ specification the ioctl returns an ``EINVAL`` error code.
>      * - ``V4L2_CAP_TOUCH``
>        - 0x10000000
>        - This is a touch device.
> +    * - ``V4L2_VDEV_CENTERED``
> +      - 0x20000000
> +      - This is controlled via V4L2 device nodes (radio, video, vbi,
> +        sdr
>      * - ``V4L2_CAP_DEVICE_CAPS``
>        - 0x80000000
>        - The driver fills the ``device_caps`` field. This capability can
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45cf7359822c..d89090d99042 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -460,6 +460,8 @@ struct v4l2_capability {
>  
>  #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
>  
> +#define V4L2_CAP_VDEV_CENTERED          0x20000000  /* V4L2 Device is controlled via V4L2 device devnode */
> +
>  #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
>  
>  /*
> 

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 11:44:27 +0200
Hans Verkuil <hansverk@cisco.com> escreveu:

> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:
> > From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > As both vdev-centric and mc-centric devices may implement the
> > same APIs, we need a flag to allow userspace to distinguish
> > between them.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > ---
> >  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> >  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> >  include/uapi/linux/videodev2.h                   | 2 ++
> >  3 files changed, 12 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> > index a72d142897c0..eb3f0ec57edb 100644
> > --- a/Documentation/media/uapi/v4l/open.rst
> > +++ b/Documentation/media/uapi/v4l/open.rst
> > @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
> >  pipelines are controlled via the **V4L2 device** node. They may optionally
> >  expose via the :ref:`media controller API <media_controller>`.
> >  
> > +.. note::
> > +
> > +   **vdev-centric** devices should report V4L2_VDEV_CENTERED  
> 
> You mean CENTRIC, not CENTERED.

Yeah, true. I'll fix it.

> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
> so it makes a lot more sense to keep that as the default and only set the cap for
> MC-centric drivers.

I actually focused it on what an userspace application would do.

An specialized application for a given hardware will likely just
ignore whatever flag is added, and use vdev, mc and subdev APIs
as it pleases. So, those applications don't need any flag at all.

However, a generic application needs a flag to allow them to check
if a given hardware can be controlled by the traditional way
to control the device (e. g. if it accepts vdev-centric type of
hardware control).

It is an old desire (since when MC was designed) to allow that
generic V4L2 apps to also work with MC-centric hardware somehow.

At the moment we add that (either in Kernelspace, as proposed for
iMX6 [1] or via libv4l), a mc-centric hardware can also be 
vdev-centric.

[1] one alternative proposed for iMX6 driver, would be to enable
    vdev-centric control only for hardware with a single camera
    slot, like those cheap RPi3-camera compatible hardware, by
    using some info at the DT.

> 
> Regards,
> 
> 	Hans
> 
> > +   :c:type:`v4l2_capability` flag (see :ref:`VIDIOC_QUERYCAP`).
> > +
> > +
> >  For **MC-centric** control, before using the V4L2 device, it is required to
> >  set the hardware pipelines via the
> >  :ref:`media controller API <media_controller>`. For those devices, the
> > diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> > index 12e0d9a63cd8..4856821b7608 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> > @@ -252,6 +252,10 @@ specification the ioctl returns an ``EINVAL`` error code.
> >      * - ``V4L2_CAP_TOUCH``
> >        - 0x10000000
> >        - This is a touch device.
> > +    * - ``V4L2_VDEV_CENTERED``
> > +      - 0x20000000
> > +      - This is controlled via V4L2 device nodes (radio, video, vbi,
> > +        sdr
> >      * - ``V4L2_CAP_DEVICE_CAPS``
> >        - 0x80000000
> >        - The driver fills the ``device_caps`` field. This capability can
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 45cf7359822c..d89090d99042 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -460,6 +460,8 @@ struct v4l2_capability {
> >  
> >  #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
> >  
> > +#define V4L2_CAP_VDEV_CENTERED          0x20000000  /* V4L2 Device is controlled via V4L2 device devnode */
> > +
> >  #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
> >  
> >  /*
> >   
> 



Thanks,
Mauro

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Hans Verkuil]

On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 25 Aug 2017 11:44:27 +0200
> Hans Verkuil <hansverk@cisco.com> escreveu:
> 
>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:
>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>
>>> As both vdev-centric and mc-centric devices may implement the
>>> same APIs, we need a flag to allow userspace to distinguish
>>> between them.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>>> ---
>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
>>>  include/uapi/linux/videodev2.h                   | 2 ++
>>>  3 files changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
>>> index a72d142897c0..eb3f0ec57edb 100644
>>> --- a/Documentation/media/uapi/v4l/open.rst
>>> +++ b/Documentation/media/uapi/v4l/open.rst
>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
>>>  expose via the :ref:`media controller API <media_controller>`.
>>>  
>>> +.. note::
>>> +
>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED  
>>
>> You mean CENTRIC, not CENTERED.
> 
> Yeah, true. I'll fix it.
> 
>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
>> so it makes a lot more sense to keep that as the default and only set the cap for
>> MC-centric drivers.
> 
> I actually focused it on what an userspace application would do.
> 
> An specialized application for a given hardware will likely just
> ignore whatever flag is added, and use vdev, mc and subdev APIs
> as it pleases. So, those applications don't need any flag at all.
> 
> However, a generic application needs a flag to allow them to check
> if a given hardware can be controlled by the traditional way
> to control the device (e. g. if it accepts vdev-centric type of
> hardware control).
> 
> It is an old desire (since when MC was designed) to allow that
> generic V4L2 apps to also work with MC-centric hardware somehow.

No, not true. The desire is that they can use the MC to find the
various device nodes (video, radio, vbi, rc, cec, ...). But they
remain vdev-centric. vdev vs mc centric has nothing to do with the
presence of the MC. It's how they are controlled.

Regarding userspace applications: they can't check for a VDEV_CENTRIC
cap since we never had any. I.e., if they do:

	if (!(caps & VDEV_CENTRIC))
		/* unsupported device */

then they would fail for older kernels that do not set this flag.

But this works:

	if (caps & MC_CENTRIC)
		/* unsupported device */

So this really needs to be an MC_CENTRIC capability.

Regards,

	Hans

> 
> At the moment we add that (either in Kernelspace, as proposed for
> iMX6 [1] or via libv4l), a mc-centric hardware can also be 
> vdev-centric.
> 
> [1] one alternative proposed for iMX6 driver, would be to enable
>     vdev-centric control only for hardware with a single camera
>     slot, like those cheap RPi3-camera compatible hardware, by
>     using some info at the DT.
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>> +   :c:type:`v4l2_capability` flag (see :ref:`VIDIOC_QUERYCAP`).
>>> +
>>> +
>>>  For **MC-centric** control, before using the V4L2 device, it is required to
>>>  set the hardware pipelines via the
>>>  :ref:`media controller API <media_controller>`. For those devices, the
>>> diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
>>> index 12e0d9a63cd8..4856821b7608 100644
>>> --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
>>> +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
>>> @@ -252,6 +252,10 @@ specification the ioctl returns an ``EINVAL`` error code.
>>>      * - ``V4L2_CAP_TOUCH``
>>>        - 0x10000000
>>>        - This is a touch device.
>>> +    * - ``V4L2_VDEV_CENTERED``
>>> +      - 0x20000000
>>> +      - This is controlled via V4L2 device nodes (radio, video, vbi,
>>> +        sdr
>>>      * - ``V4L2_CAP_DEVICE_CAPS``
>>>        - 0x80000000
>>>        - The driver fills the ``device_caps`` field. This capability can
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 45cf7359822c..d89090d99042 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -460,6 +460,8 @@ struct v4l2_capability {
>>>  
>>>  #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
>>>  
>>> +#define V4L2_CAP_VDEV_CENTERED          0x20000000  /* V4L2 Device is controlled via V4L2 device devnode */
>>> +
>>>  #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
>>>  
>>>  /*
>>>   
>>
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Hans Verkuil]

On 25/08/17 11:40, Mauro Carvalho Chehab wrote:
> From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
> 
> When we added support for omap3, back in 2010, we added a new
> type of V4L2 devices that aren't fully controlled via the V4L2
> device node. Yet, we never made it clear, at the V4L2 spec,
> about the differences between both types.
> 
> Let's document them with the current implementation.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst | 50 +++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index afd116edb40d..a72d142897c0 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -6,6 +6,56 @@
>  Opening and Closing Devices
>  ***************************
>  
> +Types of V4L2 hardware control
> +==============================
> +
> +V4L2 devices are usually complex: they are implemented via a main driver and

V4L2 hardware is usually complex: support for the hardware is implemented...

> +often several additional drivers. The main driver always exposes one or

Should we change this to "via a main (also known as 'bridge') driver"?

To help understanding terminology that we use?

> +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).
> +
> +The other drivers are called **V4L2 sub-devices** and provide control to
> +other parts of the hardware usually connected via a serial bus (like
> +I²C, SMBus or SPI). They can be implicitly controlled directly by the
> +main driver or explicitly through via the **V4L2 sub-device API** interface.
> +
> +When V4L2 was originally designed, there was only one type of device control.

hardware control

> +The entire device was controlled via the **V4L2 device nodes**. We refer to

entire V4L2 hardware

> +this kind of control as **V4L2 device node centric** (or, simply,
> +**vdev-centric**).
> +
> +Since the end of 2010, a new type of V4L2 device control was added in order

hardware control

> +to support complex devices that are common for embedded systems. Those
> +devices are controlled mainly via the media controller and sub-devices.
> +So, they're called: **Media controller centric** (or, simply,

they are

> +"**MC-centric**").
> +
> +For **vdev-centric** control, the device and their corresponding hardware

s/control/hardware control/

s/the device and their corresponding hardware pipelines are/the hardware is/

(let's keep it simple).

> +pipelines are controlled via the **V4L2 device** node. They may optionally

node -> node (or nodes, if there is more than one V4L2 device node)

> +expose via the :ref:`media controller API <media_controller>`.

I'd say: "support the :ref:`media controller API <media_controller>` as well
in order to let the application know which device nodes are available.

> +
> +For **MC-centric** control, before using the V4L2 device, it is required to

s/control/hardware control/

s/device/hardware/ (I think. Or did you mean "device node"?)

> +set the hardware pipelines via the
> +:ref:`media controller API <media_controller>`. For those devices, the
> +sub-devices' configuration can be controlled via the
> +:ref:`sub-device API <subdev>`, with creates one device node per sub device.

s/with/which/

> +
> +In summary, for **MC-centric** devices:

s/devices/hardware/

> +
> +- The **V4L2 device** node is responsible for controlling the streaming
> +  features;
> +- The **media controller device** is responsible to setup the pipelines;
> +- The **V4L2 sub-devices** are responsible for sub-device
> +  specific settings.
> +
> +.. note::
> +
> +   A **vdev-centric** may optionally expose V4L2 sub-devices via
> +   :ref:`sub-device API <subdev>`. In that case, it has to implement
> +   the :ref:`media controller API <media_controller>` as well.

This note should be moved up to the vdev-centric description. It's weird now
as it comes after the MC centric summary, but deals with a vdev-centric driver.

> +
> +
> +
> +.. _v4l2_device_naming:
>  
>  Device Naming
>  =============
> 

Regards,

	Hans

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 12:13:53 +0200
Hans Verkuil <hansverk@cisco.com> escreveu:

> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 25 Aug 2017 11:44:27 +0200
> > Hans Verkuil <hansverk@cisco.com> escreveu:
> >   
> >> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:  
> >>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>
> >>> As both vdev-centric and mc-centric devices may implement the
> >>> same APIs, we need a flag to allow userspace to distinguish
> >>> between them.
> >>>
> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> >>> ---
> >>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> >>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> >>>  include/uapi/linux/videodev2.h                   | 2 ++
> >>>  3 files changed, 12 insertions(+)
> >>>
> >>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> >>> index a72d142897c0..eb3f0ec57edb 100644
> >>> --- a/Documentation/media/uapi/v4l/open.rst
> >>> +++ b/Documentation/media/uapi/v4l/open.rst
> >>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
> >>>  pipelines are controlled via the **V4L2 device** node. They may optionally
> >>>  expose via the :ref:`media controller API <media_controller>`.
> >>>  
> >>> +.. note::
> >>> +
> >>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED    
> >>
> >> You mean CENTRIC, not CENTERED.  
> > 
> > Yeah, true. I'll fix it.
> >   
> >> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
> >> so it makes a lot more sense to keep that as the default and only set the cap for
> >> MC-centric drivers.  
> > 
> > I actually focused it on what an userspace application would do.
> > 
> > An specialized application for a given hardware will likely just
> > ignore whatever flag is added, and use vdev, mc and subdev APIs
> > as it pleases. So, those applications don't need any flag at all.
> > 
> > However, a generic application needs a flag to allow them to check
> > if a given hardware can be controlled by the traditional way
> > to control the device (e. g. if it accepts vdev-centric type of
> > hardware control).
> > 
> > It is an old desire (since when MC was designed) to allow that
> > generic V4L2 apps to also work with MC-centric hardware somehow.  
> 
> No, not true. The desire is that they can use the MC to find the
> various device nodes (video, radio, vbi, rc, cec, ...). But they
> remain vdev-centric. vdev vs mc centric has nothing to do with the
> presence of the MC. It's how they are controlled.

No, that's not I'm talking about. I'm talking about libv4l plugin
(or whatever) that would allow a generic app to work with a mc-centric
device. That's there for a long time (since when we were reviewing
the MC patches back in 2009 or 2010).

> 
> Regarding userspace applications: they can't check for a VDEV_CENTRIC
> cap since we never had any. I.e., if they do:
> 
> 	if (!(caps & VDEV_CENTRIC))
> 		/* unsupported device */
> 
> then they would fail for older kernels that do not set this flag.
> 
> But this works:
> 
> 	if (caps & MC_CENTRIC)
> 		/* unsupported device */
> 
> So this really needs to be an MC_CENTRIC capability.

That won't work. The test should take into account the API version
too.

Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
the check would be:


	/*
         * There's no need to check version here: libv4l may override it
	 * to support a mc-centric device even for older versions of the
	 * Kernel
         */
	if (caps & V4L2_CAP_VDEV_CENTRIC)
		is_supported = true;

	/*
	 * For API version lower than 4.15, there's no way to know for
	 * sure if the device is vdev-centric or not. So, either additional
	 * tests are needed, or it would assume vdev-centric and output
	 * some note about that.
	 */
	if (version < KERNEL_VERSION(4, 15, 0))
		maybe_supported = true;

Thanks,
Mauro

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Hans Verkuil]

On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 25 Aug 2017 12:13:53 +0200
> Hans Verkuil <hansverk@cisco.com> escreveu:
> 
>> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 25 Aug 2017 11:44:27 +0200
>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>   
>>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:  
>>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>>
>>>>> As both vdev-centric and mc-centric devices may implement the
>>>>> same APIs, we need a flag to allow userspace to distinguish
>>>>> between them.
>>>>>
>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>>>>> ---
>>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
>>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
>>>>>  include/uapi/linux/videodev2.h                   | 2 ++
>>>>>  3 files changed, 12 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
>>>>> index a72d142897c0..eb3f0ec57edb 100644
>>>>> --- a/Documentation/media/uapi/v4l/open.rst
>>>>> +++ b/Documentation/media/uapi/v4l/open.rst
>>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
>>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
>>>>>  expose via the :ref:`media controller API <media_controller>`.
>>>>>  
>>>>> +.. note::
>>>>> +
>>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED    
>>>>
>>>> You mean CENTRIC, not CENTERED.  
>>>
>>> Yeah, true. I'll fix it.
>>>   
>>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
>>>> so it makes a lot more sense to keep that as the default and only set the cap for
>>>> MC-centric drivers.  
>>>
>>> I actually focused it on what an userspace application would do.
>>>
>>> An specialized application for a given hardware will likely just
>>> ignore whatever flag is added, and use vdev, mc and subdev APIs
>>> as it pleases. So, those applications don't need any flag at all.
>>>
>>> However, a generic application needs a flag to allow them to check
>>> if a given hardware can be controlled by the traditional way
>>> to control the device (e. g. if it accepts vdev-centric type of
>>> hardware control).
>>>
>>> It is an old desire (since when MC was designed) to allow that
>>> generic V4L2 apps to also work with MC-centric hardware somehow.  
>>
>> No, not true. The desire is that they can use the MC to find the
>> various device nodes (video, radio, vbi, rc, cec, ...). But they
>> remain vdev-centric. vdev vs mc centric has nothing to do with the
>> presence of the MC. It's how they are controlled.
> 
> No, that's not I'm talking about. I'm talking about libv4l plugin
> (or whatever) that would allow a generic app to work with a mc-centric
> device. That's there for a long time (since when we were reviewing
> the MC patches back in 2009 or 2010).

So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
perfect sense.

There are a lot of userspace applications that do not use libv4l. It's
optional, not required, to use that library. We cannot design our API with
the assumption that this library will be used.

> 
>>
>> Regarding userspace applications: they can't check for a VDEV_CENTRIC
>> cap since we never had any. I.e., if they do:
>>
>> 	if (!(caps & VDEV_CENTRIC))
>> 		/* unsupported device */
>>
>> then they would fail for older kernels that do not set this flag.
>>
>> But this works:
>>
>> 	if (caps & MC_CENTRIC)
>> 		/* unsupported device */
>>
>> So this really needs to be an MC_CENTRIC capability.
> 
> That won't work. The test should take into account the API version
> too.
> 
> Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
> the check would be:
> 
> 
> 	/*
>          * There's no need to check version here: libv4l may override it
> 	 * to support a mc-centric device even for older versions of the
> 	 * Kernel
>          */
> 	if (caps & V4L2_CAP_VDEV_CENTRIC)
> 		is_supported = true;
> 
> 	/*
> 	 * For API version lower than 4.15, there's no way to know for
> 	 * sure if the device is vdev-centric or not. So, either additional
> 	 * tests are needed, or it would assume vdev-centric and output
> 	 * some note about that.
> 	 */
> 	if (version < KERNEL_VERSION(4, 15, 0))
> 		maybe_supported = true;


	is_supported = true;
	if (caps & V4L2_CAP_MC_CENTRIC)
		is_supported = false;
 	if (version < KERNEL_VERSION(4, 15, 0))
 		maybe_supported = true;

I don't see the difference. BTW, no application will ever do that version check.
It doesn't help them in any way to know that it 'may' be supported.

Regards,

	Hans

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 12:42:51 +0200
Hans Verkuil <hansverk@cisco.com> escreveu:

> On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 25 Aug 2017 12:13:53 +0200
> > Hans Verkuil <hansverk@cisco.com> escreveu:
> >   
> >> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:  
> >>> Em Fri, 25 Aug 2017 11:44:27 +0200
> >>> Hans Verkuil <hansverk@cisco.com> escreveu:
> >>>     
> >>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:    
> >>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>>>
> >>>>> As both vdev-centric and mc-centric devices may implement the
> >>>>> same APIs, we need a flag to allow userspace to distinguish
> >>>>> between them.
> >>>>>
> >>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> >>>>> ---
> >>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> >>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> >>>>>  include/uapi/linux/videodev2.h                   | 2 ++
> >>>>>  3 files changed, 12 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> >>>>> index a72d142897c0..eb3f0ec57edb 100644
> >>>>> --- a/Documentation/media/uapi/v4l/open.rst
> >>>>> +++ b/Documentation/media/uapi/v4l/open.rst
> >>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
> >>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
> >>>>>  expose via the :ref:`media controller API <media_controller>`.
> >>>>>  
> >>>>> +.. note::
> >>>>> +
> >>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED      
> >>>>
> >>>> You mean CENTRIC, not CENTERED.    
> >>>
> >>> Yeah, true. I'll fix it.
> >>>     
> >>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
> >>>> so it makes a lot more sense to keep that as the default and only set the cap for
> >>>> MC-centric drivers.    
> >>>
> >>> I actually focused it on what an userspace application would do.
> >>>
> >>> An specialized application for a given hardware will likely just
> >>> ignore whatever flag is added, and use vdev, mc and subdev APIs
> >>> as it pleases. So, those applications don't need any flag at all.
> >>>
> >>> However, a generic application needs a flag to allow them to check
> >>> if a given hardware can be controlled by the traditional way
> >>> to control the device (e. g. if it accepts vdev-centric type of
> >>> hardware control).
> >>>
> >>> It is an old desire (since when MC was designed) to allow that
> >>> generic V4L2 apps to also work with MC-centric hardware somehow.    
> >>
> >> No, not true. The desire is that they can use the MC to find the
> >> various device nodes (video, radio, vbi, rc, cec, ...). But they
> >> remain vdev-centric. vdev vs mc centric has nothing to do with the
> >> presence of the MC. It's how they are controlled.  
> > 
> > No, that's not I'm talking about. I'm talking about libv4l plugin
> > (or whatever) that would allow a generic app to work with a mc-centric
> > device. That's there for a long time (since when we were reviewing
> > the MC patches back in 2009 or 2010).  
> 
> So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
> perfect sense.
> 
> There are a lot of userspace applications that do not use libv4l. It's
> optional, not required, to use that library. We cannot design our API with
> the assumption that this library will be used.
> 
> >   
> >>
> >> Regarding userspace applications: they can't check for a VDEV_CENTRIC
> >> cap since we never had any. I.e., if they do:
> >>
> >> 	if (!(caps & VDEV_CENTRIC))
> >> 		/* unsupported device */
> >>
> >> then they would fail for older kernels that do not set this flag.
> >>
> >> But this works:
> >>
> >> 	if (caps & MC_CENTRIC)
> >> 		/* unsupported device */
> >>
> >> So this really needs to be an MC_CENTRIC capability.  
> > 
> > That won't work. The test should take into account the API version
> > too.
> > 
> > Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
> > the check would be:
> > 
> > 
> > 	/*
> >          * There's no need to check version here: libv4l may override it
> > 	 * to support a mc-centric device even for older versions of the
> > 	 * Kernel
> >          */
> > 	if (caps & V4L2_CAP_VDEV_CENTRIC)
> > 		is_supported = true;
> > 
> > 	/*
> > 	 * For API version lower than 4.15, there's no way to know for
> > 	 * sure if the device is vdev-centric or not. So, either additional
> > 	 * tests are needed, or it would assume vdev-centric and output
> > 	 * some note about that.
> > 	 */
> > 	if (version < KERNEL_VERSION(4, 15, 0))
> > 		maybe_supported = true;  
> 
> 
> 	is_supported = true;
> 	if (caps & V4L2_CAP_MC_CENTRIC)
> 		is_supported = false;
>  	if (version < KERNEL_VERSION(4, 15, 0))
>  		maybe_supported = true;
> 
> I don't see the difference. BTW, no application will ever do that version check.
> It doesn't help them in any way to know that it 'may' be supported.

Yeah, this can work. The only drawback is that, if we end by
implementing vdev compatible support is that such drivers will
have to clean the V4L2_CAP_MC_CENTRIC flag.

Thanks,
Mauro

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Hans Verkuil]

On 25/08/17 12:50, Mauro Carvalho Chehab wrote:
> Em Fri, 25 Aug 2017 12:42:51 +0200
> Hans Verkuil <hansverk@cisco.com> escreveu:
> 
>> On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 25 Aug 2017 12:13:53 +0200
>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>   
>>>> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:  
>>>>> Em Fri, 25 Aug 2017 11:44:27 +0200
>>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>>>     
>>>>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:    
>>>>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>>>>
>>>>>>> As both vdev-centric and mc-centric devices may implement the
>>>>>>> same APIs, we need a flag to allow userspace to distinguish
>>>>>>> between them.
>>>>>>>
>>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>>>>>>> ---
>>>>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
>>>>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
>>>>>>>  include/uapi/linux/videodev2.h                   | 2 ++
>>>>>>>  3 files changed, 12 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
>>>>>>> index a72d142897c0..eb3f0ec57edb 100644
>>>>>>> --- a/Documentation/media/uapi/v4l/open.rst
>>>>>>> +++ b/Documentation/media/uapi/v4l/open.rst
>>>>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
>>>>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
>>>>>>>  expose via the :ref:`media controller API <media_controller>`.
>>>>>>>  
>>>>>>> +.. note::
>>>>>>> +
>>>>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED      
>>>>>>
>>>>>> You mean CENTRIC, not CENTERED.    
>>>>>
>>>>> Yeah, true. I'll fix it.
>>>>>     
>>>>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
>>>>>> so it makes a lot more sense to keep that as the default and only set the cap for
>>>>>> MC-centric drivers.    
>>>>>
>>>>> I actually focused it on what an userspace application would do.
>>>>>
>>>>> An specialized application for a given hardware will likely just
>>>>> ignore whatever flag is added, and use vdev, mc and subdev APIs
>>>>> as it pleases. So, those applications don't need any flag at all.
>>>>>
>>>>> However, a generic application needs a flag to allow them to check
>>>>> if a given hardware can be controlled by the traditional way
>>>>> to control the device (e. g. if it accepts vdev-centric type of
>>>>> hardware control).
>>>>>
>>>>> It is an old desire (since when MC was designed) to allow that
>>>>> generic V4L2 apps to also work with MC-centric hardware somehow.    
>>>>
>>>> No, not true. The desire is that they can use the MC to find the
>>>> various device nodes (video, radio, vbi, rc, cec, ...). But they
>>>> remain vdev-centric. vdev vs mc centric has nothing to do with the
>>>> presence of the MC. It's how they are controlled.  
>>>
>>> No, that's not I'm talking about. I'm talking about libv4l plugin
>>> (or whatever) that would allow a generic app to work with a mc-centric
>>> device. That's there for a long time (since when we were reviewing
>>> the MC patches back in 2009 or 2010).  
>>
>> So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
>> perfect sense.
>>
>> There are a lot of userspace applications that do not use libv4l. It's
>> optional, not required, to use that library. We cannot design our API with
>> the assumption that this library will be used.
>>
>>>   
>>>>
>>>> Regarding userspace applications: they can't check for a VDEV_CENTRIC
>>>> cap since we never had any. I.e., if they do:
>>>>
>>>> 	if (!(caps & VDEV_CENTRIC))
>>>> 		/* unsupported device */
>>>>
>>>> then they would fail for older kernels that do not set this flag.
>>>>
>>>> But this works:
>>>>
>>>> 	if (caps & MC_CENTRIC)
>>>> 		/* unsupported device */
>>>>
>>>> So this really needs to be an MC_CENTRIC capability.  
>>>
>>> That won't work. The test should take into account the API version
>>> too.
>>>
>>> Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
>>> the check would be:
>>>
>>>
>>> 	/*
>>>          * There's no need to check version here: libv4l may override it
>>> 	 * to support a mc-centric device even for older versions of the
>>> 	 * Kernel
>>>          */
>>> 	if (caps & V4L2_CAP_VDEV_CENTRIC)
>>> 		is_supported = true;
>>>
>>> 	/*
>>> 	 * For API version lower than 4.15, there's no way to know for
>>> 	 * sure if the device is vdev-centric or not. So, either additional
>>> 	 * tests are needed, or it would assume vdev-centric and output
>>> 	 * some note about that.
>>> 	 */
>>> 	if (version < KERNEL_VERSION(4, 15, 0))
>>> 		maybe_supported = true;  
>>
>>
>> 	is_supported = true;
>> 	if (caps & V4L2_CAP_MC_CENTRIC)
>> 		is_supported = false;
>>  	if (version < KERNEL_VERSION(4, 15, 0))
>>  		maybe_supported = true;
>>
>> I don't see the difference. BTW, no application will ever do that version check.
>> It doesn't help them in any way to know that it 'may' be supported.
> 
> Yeah, this can work. The only drawback is that, if we end by
> implementing vdev compatible support is that such drivers will
> have to clean the V4L2_CAP_MC_CENTRIC flag.

You mean implementing vdev compatible support in libv4l? (Just making sure
I understand you correctly)

In that case it doesn't matter if the libv4l code would set the VDEV_CENTRIC flag
or remove the MC_CENTRIC flag. That makes no difference, or course.

Regards,

	Hans

Re: [PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Sakari Ailus]

Hi Mauro,

Thanks for the update. A few more small comments below.

On Fri, Aug 25, 2017 at 06:40:05AM -0300, Mauro Carvalho Chehab wrote:
> From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
> 
> When we added support for omap3, back in 2010, we added a new
> type of V4L2 devices that aren't fully controlled via the V4L2
> device node. Yet, we never made it clear, at the V4L2 spec,
> about the differences between both types.
> 
> Let's document them with the current implementation.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst | 50 +++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index afd116edb40d..a72d142897c0 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -6,6 +6,56 @@
>  Opening and Closing Devices
>  ***************************
>  
> +Types of V4L2 hardware control
> +==============================
> +
> +V4L2 devices are usually complex: they are implemented via a main driver and
> +often several additional drivers. The main driver always exposes one or
> +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).

s/V4L2 device\*\* devnodes/V4L2 device nodes\*\*/

(As you also have a few paragraphs below.)

> +
> +The other drivers are called **V4L2 sub-devices** and provide control to
> +other parts of the hardware usually connected via a serial bus (like
> +I²C, SMBus or SPI). They can be implicitly controlled directly by the

s/They/Depending on the main driver, they/

> +main driver or explicitly through via the **V4L2 sub-device API** interface.

Through or via, but not both. " interface" is redundant.

> +
> +When V4L2 was originally designed, there was only one type of device control.
> +The entire device was controlled via the **V4L2 device nodes**. We refer to
> +this kind of control as **V4L2 device node centric** (or, simply,
> +**vdev-centric**).

s/vdev-centric/V4L2-centric/ ?

> +
> +Since the end of 2010, a new type of V4L2 device control was added in order
> +to support complex devices that are common for embedded systems. Those
> +devices are controlled mainly via the media controller and sub-devices.
> +So, they're called: **Media controller centric** (or, simply,
> +"**MC-centric**").
> +
> +For **vdev-centric** control, the device and their corresponding hardware
> +pipelines are controlled via the **V4L2 device** node. They may optionally

I'd add highlighting to " node" as well.

> +expose via the :ref:`media controller API <media_controller>`.
> +
> +For **MC-centric** control, before using the V4L2 device, it is required to
> +set the hardware pipelines via the
> +:ref:`media controller API <media_controller>`. For those devices, the
> +sub-devices' configuration can be controlled via the
> +:ref:`sub-device API <subdev>`, with creates one device node per sub device.
> +
> +In summary, for **MC-centric** devices:
> +
> +- The **V4L2 device** node is responsible for controlling the streaming
> +  features;

Same here. Or, just remove node. The other device nodes are just as much
nodes as this one.

> +- The **media controller device** is responsible to setup the pipelines;
> +- The **V4L2 sub-devices** are responsible for sub-device
> +  specific settings.
> +
> +.. note::
> +
> +   A **vdev-centric** may optionally expose V4L2 sub-devices via
> +   :ref:`sub-device API <subdev>`. In that case, it has to implement
> +   the :ref:`media controller API <media_controller>` as well.

Looks good!

> +
> +
> +
> +.. _v4l2_device_naming:
>  
>  Device Naming
>  =============

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 12:56:30 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 25/08/17 12:50, Mauro Carvalho Chehab wrote:
> > Em Fri, 25 Aug 2017 12:42:51 +0200
> > Hans Verkuil <hansverk@cisco.com> escreveu:
> >   
> >> On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:  
> >>> Em Fri, 25 Aug 2017 12:13:53 +0200
> >>> Hans Verkuil <hansverk@cisco.com> escreveu:
> >>>     
> >>>> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:    
> >>>>> Em Fri, 25 Aug 2017 11:44:27 +0200
> >>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
> >>>>>       
> >>>>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:      
> >>>>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>>>>>
> >>>>>>> As both vdev-centric and mc-centric devices may implement the
> >>>>>>> same APIs, we need a flag to allow userspace to distinguish
> >>>>>>> between them.
> >>>>>>>
> >>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> >>>>>>> ---
> >>>>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> >>>>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> >>>>>>>  include/uapi/linux/videodev2.h                   | 2 ++
> >>>>>>>  3 files changed, 12 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> >>>>>>> index a72d142897c0..eb3f0ec57edb 100644
> >>>>>>> --- a/Documentation/media/uapi/v4l/open.rst
> >>>>>>> +++ b/Documentation/media/uapi/v4l/open.rst
> >>>>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
> >>>>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
> >>>>>>>  expose via the :ref:`media controller API <media_controller>`.
> >>>>>>>  
> >>>>>>> +.. note::
> >>>>>>> +
> >>>>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED        
> >>>>>>
> >>>>>> You mean CENTRIC, not CENTERED.      
> >>>>>
> >>>>> Yeah, true. I'll fix it.
> >>>>>       
> >>>>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
> >>>>>> so it makes a lot more sense to keep that as the default and only set the cap for
> >>>>>> MC-centric drivers.      
> >>>>>
> >>>>> I actually focused it on what an userspace application would do.
> >>>>>
> >>>>> An specialized application for a given hardware will likely just
> >>>>> ignore whatever flag is added, and use vdev, mc and subdev APIs
> >>>>> as it pleases. So, those applications don't need any flag at all.
> >>>>>
> >>>>> However, a generic application needs a flag to allow them to check
> >>>>> if a given hardware can be controlled by the traditional way
> >>>>> to control the device (e. g. if it accepts vdev-centric type of
> >>>>> hardware control).
> >>>>>
> >>>>> It is an old desire (since when MC was designed) to allow that
> >>>>> generic V4L2 apps to also work with MC-centric hardware somehow.      
> >>>>
> >>>> No, not true. The desire is that they can use the MC to find the
> >>>> various device nodes (video, radio, vbi, rc, cec, ...). But they
> >>>> remain vdev-centric. vdev vs mc centric has nothing to do with the
> >>>> presence of the MC. It's how they are controlled.    
> >>>
> >>> No, that's not I'm talking about. I'm talking about libv4l plugin
> >>> (or whatever) that would allow a generic app to work with a mc-centric
> >>> device. That's there for a long time (since when we were reviewing
> >>> the MC patches back in 2009 or 2010).    
> >>
> >> So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
> >> perfect sense.
> >>
> >> There are a lot of userspace applications that do not use libv4l. It's
> >> optional, not required, to use that library. We cannot design our API with
> >> the assumption that this library will be used.
> >>  
> >>>     
> >>>>
> >>>> Regarding userspace applications: they can't check for a VDEV_CENTRIC
> >>>> cap since we never had any. I.e., if they do:
> >>>>
> >>>> 	if (!(caps & VDEV_CENTRIC))
> >>>> 		/* unsupported device */
> >>>>
> >>>> then they would fail for older kernels that do not set this flag.
> >>>>
> >>>> But this works:
> >>>>
> >>>> 	if (caps & MC_CENTRIC)
> >>>> 		/* unsupported device */
> >>>>
> >>>> So this really needs to be an MC_CENTRIC capability.    
> >>>
> >>> That won't work. The test should take into account the API version
> >>> too.
> >>>
> >>> Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
> >>> the check would be:
> >>>
> >>>
> >>> 	/*
> >>>          * There's no need to check version here: libv4l may override it
> >>> 	 * to support a mc-centric device even for older versions of the
> >>> 	 * Kernel
> >>>          */
> >>> 	if (caps & V4L2_CAP_VDEV_CENTRIC)
> >>> 		is_supported = true;
> >>>
> >>> 	/*
> >>> 	 * For API version lower than 4.15, there's no way to know for
> >>> 	 * sure if the device is vdev-centric or not. So, either additional
> >>> 	 * tests are needed, or it would assume vdev-centric and output
> >>> 	 * some note about that.
> >>> 	 */
> >>> 	if (version < KERNEL_VERSION(4, 15, 0))
> >>> 		maybe_supported = true;    
> >>
> >>
> >> 	is_supported = true;
> >> 	if (caps & V4L2_CAP_MC_CENTRIC)
> >> 		is_supported = false;
> >>  	if (version < KERNEL_VERSION(4, 15, 0))
> >>  		maybe_supported = true;
> >>
> >> I don't see the difference. BTW, no application will ever do that version check.
> >> It doesn't help them in any way to know that it 'may' be supported.  
> > 
> > Yeah, this can work. The only drawback is that, if we end by
> > implementing vdev compatible support is that such drivers will
> > have to clean the V4L2_CAP_MC_CENTRIC flag.  
> 
> You mean implementing vdev compatible support in libv4l? (Just making sure
> I understand you correctly)

Yes, either there or at the Kernel, as it seems we'll never have it
there, as nobody is working on it anymore.

> In that case it doesn't matter if the libv4l code would set the VDEV_CENTRIC flag
> or remove the MC_CENTRIC flag. That makes no difference, or course.

True, but the text will have to be clear that a MC_CENTRIC device is a
device that can't be controlled by a V4L2-centric application.

Thanks,
Mauro

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 08:15:03 -0300
Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:

> Em Fri, 25 Aug 2017 12:56:30 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
> > On 25/08/17 12:50, Mauro Carvalho Chehab wrote:  
> > > Em Fri, 25 Aug 2017 12:42:51 +0200
> > > Hans Verkuil <hansverk@cisco.com> escreveu:
> > >     
> > >> On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:    
> > >>> Em Fri, 25 Aug 2017 12:13:53 +0200
> > >>> Hans Verkuil <hansverk@cisco.com> escreveu:
> > >>>       
> > >>>> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:      
> > >>>>> Em Fri, 25 Aug 2017 11:44:27 +0200
> > >>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
> > >>>>>         
> > >>>>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:        
> > >>>>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > >>>>>>>
> > >>>>>>> As both vdev-centric and mc-centric devices may implement the
> > >>>>>>> same APIs, we need a flag to allow userspace to distinguish
> > >>>>>>> between them.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > >>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > >>>>>>> ---
> > >>>>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> > >>>>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> > >>>>>>>  include/uapi/linux/videodev2.h                   | 2 ++
> > >>>>>>>  3 files changed, 12 insertions(+)
> > >>>>>>>
> > >>>>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> > >>>>>>> index a72d142897c0..eb3f0ec57edb 100644
> > >>>>>>> --- a/Documentation/media/uapi/v4l/open.rst
> > >>>>>>> +++ b/Documentation/media/uapi/v4l/open.rst
> > >>>>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
> > >>>>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
> > >>>>>>>  expose via the :ref:`media controller API <media_controller>`.
> > >>>>>>>  
> > >>>>>>> +.. note::
> > >>>>>>> +
> > >>>>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED          
> > >>>>>>
> > >>>>>> You mean CENTRIC, not CENTERED.        
> > >>>>>
> > >>>>> Yeah, true. I'll fix it.
> > >>>>>         
> > >>>>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
> > >>>>>> so it makes a lot more sense to keep that as the default and only set the cap for
> > >>>>>> MC-centric drivers.        
> > >>>>>
> > >>>>> I actually focused it on what an userspace application would do.
> > >>>>>
> > >>>>> An specialized application for a given hardware will likely just
> > >>>>> ignore whatever flag is added, and use vdev, mc and subdev APIs
> > >>>>> as it pleases. So, those applications don't need any flag at all.
> > >>>>>
> > >>>>> However, a generic application needs a flag to allow them to check
> > >>>>> if a given hardware can be controlled by the traditional way
> > >>>>> to control the device (e. g. if it accepts vdev-centric type of
> > >>>>> hardware control).
> > >>>>>
> > >>>>> It is an old desire (since when MC was designed) to allow that
> > >>>>> generic V4L2 apps to also work with MC-centric hardware somehow.        
> > >>>>
> > >>>> No, not true. The desire is that they can use the MC to find the
> > >>>> various device nodes (video, radio, vbi, rc, cec, ...). But they
> > >>>> remain vdev-centric. vdev vs mc centric has nothing to do with the
> > >>>> presence of the MC. It's how they are controlled.      
> > >>>
> > >>> No, that's not I'm talking about. I'm talking about libv4l plugin
> > >>> (or whatever) that would allow a generic app to work with a mc-centric
> > >>> device. That's there for a long time (since when we were reviewing
> > >>> the MC patches back in 2009 or 2010).      
> > >>
> > >> So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
> > >> perfect sense.
> > >>
> > >> There are a lot of userspace applications that do not use libv4l. It's
> > >> optional, not required, to use that library. We cannot design our API with
> > >> the assumption that this library will be used.
> > >>    
> > >>>       
> > >>>>
> > >>>> Regarding userspace applications: they can't check for a VDEV_CENTRIC
> > >>>> cap since we never had any. I.e., if they do:
> > >>>>
> > >>>> 	if (!(caps & VDEV_CENTRIC))
> > >>>> 		/* unsupported device */
> > >>>>
> > >>>> then they would fail for older kernels that do not set this flag.
> > >>>>
> > >>>> But this works:
> > >>>>
> > >>>> 	if (caps & MC_CENTRIC)
> > >>>> 		/* unsupported device */
> > >>>>
> > >>>> So this really needs to be an MC_CENTRIC capability.      
> > >>>
> > >>> That won't work. The test should take into account the API version
> > >>> too.
> > >>>
> > >>> Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
> > >>> the check would be:
> > >>>
> > >>>
> > >>> 	/*
> > >>>          * There's no need to check version here: libv4l may override it
> > >>> 	 * to support a mc-centric device even for older versions of the
> > >>> 	 * Kernel
> > >>>          */
> > >>> 	if (caps & V4L2_CAP_VDEV_CENTRIC)
> > >>> 		is_supported = true;
> > >>>
> > >>> 	/*
> > >>> 	 * For API version lower than 4.15, there's no way to know for
> > >>> 	 * sure if the device is vdev-centric or not. So, either additional
> > >>> 	 * tests are needed, or it would assume vdev-centric and output
> > >>> 	 * some note about that.
> > >>> 	 */
> > >>> 	if (version < KERNEL_VERSION(4, 15, 0))
> > >>> 		maybe_supported = true;      
> > >>
> > >>
> > >> 	is_supported = true;
> > >> 	if (caps & V4L2_CAP_MC_CENTRIC)
> > >> 		is_supported = false;
> > >>  	if (version < KERNEL_VERSION(4, 15, 0))
> > >>  		maybe_supported = true;
> > >>
> > >> I don't see the difference. BTW, no application will ever do that version check.
> > >> It doesn't help them in any way to know that it 'may' be supported.    
> > > 
> > > Yeah, this can work. The only drawback is that, if we end by
> > > implementing vdev compatible support is that such drivers will
> > > have to clean the V4L2_CAP_MC_CENTRIC flag.    
> > 
> > You mean implementing vdev compatible support in libv4l? (Just making sure
> > I understand you correctly)  
> 
> Yes, either there or at the Kernel, as it seems we'll never have it
> there, as nobody is working on it anymore.
> 
> > In that case it doesn't matter if the libv4l code would set the VDEV_CENTRIC flag
> > or remove the MC_CENTRIC flag. That makes no difference, or course.  
> 
> True, but the text will have to be clear that a MC_CENTRIC device is a
> device that can't be controlled by a V4L2-centric application.

Ok, that's the "reverse" patch. IMHO, it is very confusing, as we're
actually using MC_CENTRIC to actually describe the lack of a capability.

Perhaps we should name it as NOT_VDEV_CENTRIC instead.

Regards,
Mauro

---

media: videodev2: add a flag for mc-centric devices
    
As both vdev-centric and mc-centric devices may implement the
same APIs, we need a flag to allow userspace to distinguish
between them.
    
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index a72d142897c0..4b2f807269e7 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -6,6 +6,8 @@
 Opening and Closing Devices
 ***************************
 
+.. _v4l2_hardware_control:
+
 Types of V4L2 hardware control
 ==============================
 
@@ -33,6 +35,13 @@ For **vdev-centric** control, the device and their corresponding hardware
 pipelines are controlled via the **V4L2 device** node. They may optionally
 expose via the :ref:`media controller API <media_controller>`.
 
+.. note::
+
+   Devices that are **not** capable of **vdev-centric** hardware control
+   should report a ``V4L2_MC_CENTRIC`` :c:type:`v4l2_capability` flag
+   (see :ref:`VIDIOC_QUERYCAP`).
+
+
 For **MC-centric** control, before using the V4L2 device, it is required to
 set the hardware pipelines via the
 :ref:`media controller API <media_controller>`. For those devices, the
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 12e0d9a63cd8..299a53d66032 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -252,6 +252,11 @@ specification the ioctl returns an ``EINVAL`` error code.
     * - ``V4L2_CAP_TOUCH``
       - 0x10000000
       - This is a touch device.
+    * - ``V4L2_MC_CENTRIC``
+      - 0x20000000
+      - Indicates that the device only provides **mc-centric** hardware
+        control, and can't be used by **v4l2-centric** applications.
+        See :ref:`v4l2_hardware_control` for more details.
     * - ``V4L2_CAP_DEVICE_CAPS``
       - 0x80000000
       - The driver fills the ``device_caps`` field. This capability can
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45cf7359822c..569eef0630ab 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -460,6 +460,8 @@ struct v4l2_capability {
 
 #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
 
+#define V4L2_CAP_MC_CENTRIC             0x20000000  /* Device can't be controlled via V4L2 device devnodes */
+
 #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
 
 /*

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 08:30:37 -0300
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:

> Em Fri, 25 Aug 2017 08:15:03 -0300
> Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> 
> > Em Fri, 25 Aug 2017 12:56:30 +0200
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > 
> > > On 25/08/17 12:50, Mauro Carvalho Chehab wrote:  
> > > > Em Fri, 25 Aug 2017 12:42:51 +0200
> > > > Hans Verkuil <hansverk@cisco.com> escreveu:
> > > >     
> > > >> On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:    
> > > >>> Em Fri, 25 Aug 2017 12:13:53 +0200
> > > >>> Hans Verkuil <hansverk@cisco.com> escreveu:
> > > >>>       
> > > >>>> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:      
> > > >>>>> Em Fri, 25 Aug 2017 11:44:27 +0200
> > > >>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
> > > >>>>>         
> > > >>>>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:        
> > > >>>>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > > >>>>>>>
> > > >>>>>>> As both vdev-centric and mc-centric devices may implement the
> > > >>>>>>> same APIs, we need a flag to allow userspace to distinguish
> > > >>>>>>> between them.
> > > >>>>>>>
> > > >>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > > >>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > >>>>>>> ---
> > > >>>>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> > > >>>>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> > > >>>>>>>  include/uapi/linux/videodev2.h                   | 2 ++
> > > >>>>>>>  3 files changed, 12 insertions(+)
> > > >>>>>>>
> > > >>>>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> > > >>>>>>> index a72d142897c0..eb3f0ec57edb 100644
> > > >>>>>>> --- a/Documentation/media/uapi/v4l/open.rst
> > > >>>>>>> +++ b/Documentation/media/uapi/v4l/open.rst
> > > >>>>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
> > > >>>>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
> > > >>>>>>>  expose via the :ref:`media controller API <media_controller>`.
> > > >>>>>>>  
> > > >>>>>>> +.. note::
> > > >>>>>>> +
> > > >>>>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED          
> > > >>>>>>
> > > >>>>>> You mean CENTRIC, not CENTERED.        
> > > >>>>>
> > > >>>>> Yeah, true. I'll fix it.
> > > >>>>>         
> > > >>>>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
> > > >>>>>> so it makes a lot more sense to keep that as the default and only set the cap for
> > > >>>>>> MC-centric drivers.        
> > > >>>>>
> > > >>>>> I actually focused it on what an userspace application would do.
> > > >>>>>
> > > >>>>> An specialized application for a given hardware will likely just
> > > >>>>> ignore whatever flag is added, and use vdev, mc and subdev APIs
> > > >>>>> as it pleases. So, those applications don't need any flag at all.
> > > >>>>>
> > > >>>>> However, a generic application needs a flag to allow them to check
> > > >>>>> if a given hardware can be controlled by the traditional way
> > > >>>>> to control the device (e. g. if it accepts vdev-centric type of
> > > >>>>> hardware control).
> > > >>>>>
> > > >>>>> It is an old desire (since when MC was designed) to allow that
> > > >>>>> generic V4L2 apps to also work with MC-centric hardware somehow.        
> > > >>>>
> > > >>>> No, not true. The desire is that they can use the MC to find the
> > > >>>> various device nodes (video, radio, vbi, rc, cec, ...). But they
> > > >>>> remain vdev-centric. vdev vs mc centric has nothing to do with the
> > > >>>> presence of the MC. It's how they are controlled.      
> > > >>>
> > > >>> No, that's not I'm talking about. I'm talking about libv4l plugin
> > > >>> (or whatever) that would allow a generic app to work with a mc-centric
> > > >>> device. That's there for a long time (since when we were reviewing
> > > >>> the MC patches back in 2009 or 2010).      
> > > >>
> > > >> So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
> > > >> perfect sense.
> > > >>
> > > >> There are a lot of userspace applications that do not use libv4l. It's
> > > >> optional, not required, to use that library. We cannot design our API with
> > > >> the assumption that this library will be used.
> > > >>    
> > > >>>       
> > > >>>>
> > > >>>> Regarding userspace applications: they can't check for a VDEV_CENTRIC
> > > >>>> cap since we never had any. I.e., if they do:
> > > >>>>
> > > >>>> 	if (!(caps & VDEV_CENTRIC))
> > > >>>> 		/* unsupported device */
> > > >>>>
> > > >>>> then they would fail for older kernels that do not set this flag.
> > > >>>>
> > > >>>> But this works:
> > > >>>>
> > > >>>> 	if (caps & MC_CENTRIC)
> > > >>>> 		/* unsupported device */
> > > >>>>
> > > >>>> So this really needs to be an MC_CENTRIC capability.      
> > > >>>
> > > >>> That won't work. The test should take into account the API version
> > > >>> too.
> > > >>>
> > > >>> Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
> > > >>> the check would be:
> > > >>>
> > > >>>
> > > >>> 	/*
> > > >>>          * There's no need to check version here: libv4l may override it
> > > >>> 	 * to support a mc-centric device even for older versions of the
> > > >>> 	 * Kernel
> > > >>>          */
> > > >>> 	if (caps & V4L2_CAP_VDEV_CENTRIC)
> > > >>> 		is_supported = true;
> > > >>>
> > > >>> 	/*
> > > >>> 	 * For API version lower than 4.15, there's no way to know for
> > > >>> 	 * sure if the device is vdev-centric or not. So, either additional
> > > >>> 	 * tests are needed, or it would assume vdev-centric and output
> > > >>> 	 * some note about that.
> > > >>> 	 */
> > > >>> 	if (version < KERNEL_VERSION(4, 15, 0))
> > > >>> 		maybe_supported = true;      
> > > >>
> > > >>
> > > >> 	is_supported = true;
> > > >> 	if (caps & V4L2_CAP_MC_CENTRIC)
> > > >> 		is_supported = false;
> > > >>  	if (version < KERNEL_VERSION(4, 15, 0))
> > > >>  		maybe_supported = true;
> > > >>
> > > >> I don't see the difference. BTW, no application will ever do that version check.
> > > >> It doesn't help them in any way to know that it 'may' be supported.    
> > > > 
> > > > Yeah, this can work. The only drawback is that, if we end by
> > > > implementing vdev compatible support is that such drivers will
> > > > have to clean the V4L2_CAP_MC_CENTRIC flag.    
> > > 
> > > You mean implementing vdev compatible support in libv4l? (Just making sure
> > > I understand you correctly)  
> > 
> > Yes, either there or at the Kernel, as it seems we'll never have it
> > there, as nobody is working on it anymore.
> > 
> > > In that case it doesn't matter if the libv4l code would set the VDEV_CENTRIC flag
> > > or remove the MC_CENTRIC flag. That makes no difference, or course.  
> > 
> > True, but the text will have to be clear that a MC_CENTRIC device is a
> > device that can't be controlled by a V4L2-centric application.
> 
> Ok, that's the "reverse" patch. IMHO, it is very confusing, as we're
> actually using MC_CENTRIC to actually describe the lack of a capability.
> 
> Perhaps we should name it as NOT_VDEV_CENTRIC instead.

Hans suggested an alternative word at the IRC ("require"), with actually
sounds better. Patch follows.

I can live it that :-)

Regards,
Mauro

-

media: videodev2: add a flag for vdev-centric devices
    
As both vdev-centric and mc-centric devices may implement the
same APIs, we need a flag to allow userspace to distinguish
between them.
    
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index a72d142897c0..4b344dccd2ac 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -6,6 +6,8 @@
 Opening and Closing Devices
 ***************************
 
+.. _v4l2_hardware_control:
+
 Types of V4L2 hardware control
 ==============================
 
@@ -33,6 +35,13 @@ For **vdev-centric** control, the device and their corresponding hardware
 pipelines are controlled via the **V4L2 device** node. They may optionally
 expose via the :ref:`media controller API <media_controller>`.
 
+.. note::
+
+   Devices that require **mc-centric** hardware control should report
+   a ``V4L2_MC_CENTRIC`` :c:type:`v4l2_capability` flag
+   (see :ref:`VIDIOC_QUERYCAP`).
+
+
 For **MC-centric** control, before using the V4L2 device, it is required to
 set the hardware pipelines via the
 :ref:`media controller API <media_controller>`. For those devices, the
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 12e0d9a63cd8..2b08723375bc 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -252,6 +252,11 @@ specification the ioctl returns an ``EINVAL`` error code.
     * - ``V4L2_CAP_TOUCH``
       - 0x10000000
       - This is a touch device.
+    * - ``V4L2_MC_CENTRIC``
+      - 0x20000000
+      - Indicates that the device require **mc-centric** hardware
+        control, and thus can't be used by **v4l2-centric** applications.
+        See :ref:`v4l2_hardware_control` for more details.
     * - ``V4L2_CAP_DEVICE_CAPS``
       - 0x80000000
       - The driver fills the ``device_caps`` field. This capability can
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45cf7359822c..7b490fe97980 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -460,6 +460,8 @@ struct v4l2_capability {
 
 #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
 
+#define V4L2_CAP_MC_CENTRIC             0x20000000  /* Device require mc-centric hardware control */
+
 #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
 
 /*

-- 
Thanks,
Mauro

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Hans Verkuil]

On 25/08/17 13:35, Mauro Carvalho Chehab wrote:
> Em Fri, 25 Aug 2017 08:30:37 -0300
> Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:
> 
>> Em Fri, 25 Aug 2017 08:15:03 -0300
>> Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
>>
>>> Em Fri, 25 Aug 2017 12:56:30 +0200
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>
>>>> On 25/08/17 12:50, Mauro Carvalho Chehab wrote:  
>>>>> Em Fri, 25 Aug 2017 12:42:51 +0200
>>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>>>     
>>>>>> On 08/25/2017 12:35 PM, Mauro Carvalho Chehab wrote:    
>>>>>>> Em Fri, 25 Aug 2017 12:13:53 +0200
>>>>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>>>>>       
>>>>>>>> On 08/25/2017 12:06 PM, Mauro Carvalho Chehab wrote:      
>>>>>>>>> Em Fri, 25 Aug 2017 11:44:27 +0200
>>>>>>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>>>>>>>         
>>>>>>>>>> On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:        
>>>>>>>>>>> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>>>>>>>>
>>>>>>>>>>> As both vdev-centric and mc-centric devices may implement the
>>>>>>>>>>> same APIs, we need a flag to allow userspace to distinguish
>>>>>>>>>>> between them.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>>>>>>>>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>>>>>>>>>>> ---
>>>>>>>>>>>  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
>>>>>>>>>>>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
>>>>>>>>>>>  include/uapi/linux/videodev2.h                   | 2 ++
>>>>>>>>>>>  3 files changed, 12 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
>>>>>>>>>>> index a72d142897c0..eb3f0ec57edb 100644
>>>>>>>>>>> --- a/Documentation/media/uapi/v4l/open.rst
>>>>>>>>>>> +++ b/Documentation/media/uapi/v4l/open.rst
>>>>>>>>>>> @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their corresponding hardware
>>>>>>>>>>>  pipelines are controlled via the **V4L2 device** node. They may optionally
>>>>>>>>>>>  expose via the :ref:`media controller API <media_controller>`.
>>>>>>>>>>>  
>>>>>>>>>>> +.. note::
>>>>>>>>>>> +
>>>>>>>>>>> +   **vdev-centric** devices should report V4L2_VDEV_CENTERED          
>>>>>>>>>>
>>>>>>>>>> You mean CENTRIC, not CENTERED.        
>>>>>>>>>
>>>>>>>>> Yeah, true. I'll fix it.
>>>>>>>>>         
>>>>>>>>>> But I would change this to MC_CENTRIC: the vast majority of drivers are VDEV centric,
>>>>>>>>>> so it makes a lot more sense to keep that as the default and only set the cap for
>>>>>>>>>> MC-centric drivers.        
>>>>>>>>>
>>>>>>>>> I actually focused it on what an userspace application would do.
>>>>>>>>>
>>>>>>>>> An specialized application for a given hardware will likely just
>>>>>>>>> ignore whatever flag is added, and use vdev, mc and subdev APIs
>>>>>>>>> as it pleases. So, those applications don't need any flag at all.
>>>>>>>>>
>>>>>>>>> However, a generic application needs a flag to allow them to check
>>>>>>>>> if a given hardware can be controlled by the traditional way
>>>>>>>>> to control the device (e. g. if it accepts vdev-centric type of
>>>>>>>>> hardware control).
>>>>>>>>>
>>>>>>>>> It is an old desire (since when MC was designed) to allow that
>>>>>>>>> generic V4L2 apps to also work with MC-centric hardware somehow.        
>>>>>>>>
>>>>>>>> No, not true. The desire is that they can use the MC to find the
>>>>>>>> various device nodes (video, radio, vbi, rc, cec, ...). But they
>>>>>>>> remain vdev-centric. vdev vs mc centric has nothing to do with the
>>>>>>>> presence of the MC. It's how they are controlled.      
>>>>>>>
>>>>>>> No, that's not I'm talking about. I'm talking about libv4l plugin
>>>>>>> (or whatever) that would allow a generic app to work with a mc-centric
>>>>>>> device. That's there for a long time (since when we were reviewing
>>>>>>> the MC patches back in 2009 or 2010).      
>>>>>>
>>>>>> So? Such a plugin would obviously remove the MC_CENTRIC cap. Which makes
>>>>>> perfect sense.
>>>>>>
>>>>>> There are a lot of userspace applications that do not use libv4l. It's
>>>>>> optional, not required, to use that library. We cannot design our API with
>>>>>> the assumption that this library will be used.
>>>>>>    
>>>>>>>       
>>>>>>>>
>>>>>>>> Regarding userspace applications: they can't check for a VDEV_CENTRIC
>>>>>>>> cap since we never had any. I.e., if they do:
>>>>>>>>
>>>>>>>> 	if (!(caps & VDEV_CENTRIC))
>>>>>>>> 		/* unsupported device */
>>>>>>>>
>>>>>>>> then they would fail for older kernels that do not set this flag.
>>>>>>>>
>>>>>>>> But this works:
>>>>>>>>
>>>>>>>> 	if (caps & MC_CENTRIC)
>>>>>>>> 		/* unsupported device */
>>>>>>>>
>>>>>>>> So this really needs to be an MC_CENTRIC capability.      
>>>>>>>
>>>>>>> That won't work. The test should take into account the API version
>>>>>>> too.
>>>>>>>
>>>>>>> Assuming that such flag would be added for version 4.15, with a VDEV_CENTRIC,
>>>>>>> the check would be:
>>>>>>>
>>>>>>>
>>>>>>> 	/*
>>>>>>>          * There's no need to check version here: libv4l may override it
>>>>>>> 	 * to support a mc-centric device even for older versions of the
>>>>>>> 	 * Kernel
>>>>>>>          */
>>>>>>> 	if (caps & V4L2_CAP_VDEV_CENTRIC)
>>>>>>> 		is_supported = true;
>>>>>>>
>>>>>>> 	/*
>>>>>>> 	 * For API version lower than 4.15, there's no way to know for
>>>>>>> 	 * sure if the device is vdev-centric or not. So, either additional
>>>>>>> 	 * tests are needed, or it would assume vdev-centric and output
>>>>>>> 	 * some note about that.
>>>>>>> 	 */
>>>>>>> 	if (version < KERNEL_VERSION(4, 15, 0))
>>>>>>> 		maybe_supported = true;      
>>>>>>
>>>>>>
>>>>>> 	is_supported = true;
>>>>>> 	if (caps & V4L2_CAP_MC_CENTRIC)
>>>>>> 		is_supported = false;
>>>>>>  	if (version < KERNEL_VERSION(4, 15, 0))
>>>>>>  		maybe_supported = true;
>>>>>>
>>>>>> I don't see the difference. BTW, no application will ever do that version check.
>>>>>> It doesn't help them in any way to know that it 'may' be supported.    
>>>>>
>>>>> Yeah, this can work. The only drawback is that, if we end by
>>>>> implementing vdev compatible support is that such drivers will
>>>>> have to clean the V4L2_CAP_MC_CENTRIC flag.    
>>>>
>>>> You mean implementing vdev compatible support in libv4l? (Just making sure
>>>> I understand you correctly)  
>>>
>>> Yes, either there or at the Kernel, as it seems we'll never have it
>>> there, as nobody is working on it anymore.
>>>
>>>> In that case it doesn't matter if the libv4l code would set the VDEV_CENTRIC flag
>>>> or remove the MC_CENTRIC flag. That makes no difference, or course.  
>>>
>>> True, but the text will have to be clear that a MC_CENTRIC device is a
>>> device that can't be controlled by a V4L2-centric application.
>>
>> Ok, that's the "reverse" patch. IMHO, it is very confusing, as we're
>> actually using MC_CENTRIC to actually describe the lack of a capability.
>>
>> Perhaps we should name it as NOT_VDEV_CENTRIC instead.
> 
> Hans suggested an alternative word at the IRC ("require"), with actually
> sounds better. Patch follows.
> 
> I can live it that :-)
> 
> Regards,
> Mauro
> 
> -
> 
> media: videodev2: add a flag for vdev-centric devices
>     
> As both vdev-centric and mc-centric devices may implement the
> same APIs, we need a flag to allow userspace to distinguish
> between them.
>     
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index a72d142897c0..4b344dccd2ac 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -6,6 +6,8 @@
>  Opening and Closing Devices
>  ***************************
>  
> +.. _v4l2_hardware_control:
> +
>  Types of V4L2 hardware control
>  ==============================
>  
> @@ -33,6 +35,13 @@ For **vdev-centric** control, the device and their corresponding hardware
>  pipelines are controlled via the **V4L2 device** node. They may optionally
>  expose via the :ref:`media controller API <media_controller>`.
>  
> +.. note::
> +
> +   Devices that require **mc-centric** hardware control should report

s/should/shall/

Shouldn't we use MC-centric? (i.e. capital MC) I think that's better.

> +   a ``V4L2_MC_CENTRIC`` :c:type:`v4l2_capability` flag

s/a/the/

> +   (see :ref:`VIDIOC_QUERYCAP`).
> +
> +
>  For **MC-centric** control, before using the V4L2 device, it is required to
>  set the hardware pipelines via the
>  :ref:`media controller API <media_controller>`. For those devices, the
> diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> index 12e0d9a63cd8..2b08723375bc 100644
> --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> @@ -252,6 +252,11 @@ specification the ioctl returns an ``EINVAL`` error code.
>      * - ``V4L2_CAP_TOUCH``
>        - 0x10000000
>        - This is a touch device.
> +    * - ``V4L2_MC_CENTRIC``
> +      - 0x20000000
> +      - Indicates that the device require **mc-centric** hardware

requires

> +        control, and thus can't be used by **v4l2-centric** applications.

So are we using V4L2-centric or vdev-centric? It seems to be mixed now.
And if we use V4L2-centric then it should be capitals as well.

> +        See :ref:`v4l2_hardware_control` for more details.
>      * - ``V4L2_CAP_DEVICE_CAPS``
>        - 0x80000000
>        - The driver fills the ``device_caps`` field. This capability can
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45cf7359822c..7b490fe97980 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -460,6 +460,8 @@ struct v4l2_capability {
>  
>  #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
>  
> +#define V4L2_CAP_MC_CENTRIC             0x20000000  /* Device require mc-centric hardware control */

requires

> +
>  #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
>  
>  /*
> 

This is getting close. Can you post the whole patch series for the next revision?
It's a bit hard to review with patches in replies.

Regards,

	Hans

Re: [PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Friday, 25 August 2017 12:40:05 EEST Mauro Carvalho Chehab wrote:
> From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
> 
> When we added support for omap3, back in 2010, we added a new
> type of V4L2 devices that aren't fully controlled via the V4L2
> device node. Yet, we never made it clear, at the V4L2 spec,
> about the differences between both types.

Nitpicking (and there will be lots of nitpicking in this review as I think 
it's very important to get this piece of the documentation right down to 
details):

s/at the V4L2 spec/in the V4L2 spec/

"make it clear about" doesn't sound good to me. How about the following ?

"Yet, we have never clearly documented in the V4L2 specification the 
differences between the two types."

> Let's document them with the current implementation.

s/with the/based on the/

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst | 50 ++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst
> b/Documentation/media/uapi/v4l/open.rst index afd116edb40d..a72d142897c0
> 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -6,6 +6,56 @@
>  Opening and Closing Devices
>  ***************************
> 
> +Types of V4L2 hardware control
> +==============================
> +
> +V4L2 devices are usually complex: they are implemented via a main driver
> and
> +often several additional drivers. The main driver always exposes one or
> +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).

First of all, as stated in a previous e-mail, I think we should start by 
defining the terms we are going to use. The word "device" is ambiguous, it can 
mean

- device node

The /dev device node through which a character- or block-based API is exposed 
through userspace.

The term "device node" is commonly used in the kernel, I think we can use it 
as-is without ambiguity.

- kernel struct device

An instance of the kernel struct device. Kernel devices are used to represent 
hardware devices (and are then embedded in bus-specific device structure such 
as platform_device, pci_device or i2c_client) and logical devices (and are 
then embedded in class-spcific device structures such as struct video_device 
or struct net_device).

For now I believe this isn't relevant to the V4L2 userspace API discussion, so 
we can leave it out.

- hardware device

The hardware counterpart of the first category of the kernel struct device, a 
hardware (and thus physical) device such as an SoC IP core or an I2C chip.

We could call this "hardware device" or "hardware component". Other proposals 
are welcome.

- hardware peripheral

A group of hardware devices that together make a larger user-facing functional 
peripheral. For instance the SoC ISP IP cores and external camera sensors 
together make a camera peripheral.

If we call the previous device a "hardware component" this could be called 
"hardware device". Otherwise "hardware peripheral" is a possible name, but we 
can probably come up with a better name.

- software peripheral

The software counterpart of the hardware peripheral. In V4L2 this is modeled 
by a struct v4l2_device, often associated with a struct media_device.

I don't like the name "software peripheral", other proposals are welcome.

Once we agree on names and definitions I'll comment on the reworked version of 
this patch, as it's pointless to bikeshed the wording without first agreeing 
on clear definitions of the concepts we're discussing. Please still see below 
for a few comments not related to particular wording.

> +The other drivers are called **V4L2 sub-devices** and provide control to
> +other parts of the hardware usually connected via a serial bus (like
> +I²C, SMBus or SPI). They can be implicitly controlled directly by the
> +main driver or explicitly through via the **V4L2 sub-device API**
> interface.
> +
> +When V4L2 was originally designed, there was only one type of device
> control.
> +The entire device was controlled via the **V4L2 device nodes**. We refer to
> +this kind of control as **V4L2 device node centric** (or,
> simply, +**vdev-centric**).
> +
> +Since the end of 2010, a new type of V4L2 device control was added in order
> +to support complex devices that are common for embedded systems. Those
> +devices are controlled mainly via the media controller and sub-devices.
> +So, they're called: **Media controller centric** (or, simply,
> +"**MC-centric**").

Do we really need to explain the development history with dates ? I don't 
think it helps understanding the separation between vdev-centric and MC-
centric devices. For instance the previous paragraph that explains that V4L2 
originally had a single type of device control that we now call vdev-centric 
doesn't really help understanding what a vdev-centric device is.

It would be clearer in my opinion to explain the two kind of devices with an 
associated use case, without referring to the history.

> +For **vdev-centric** control, the device and their corresponding hardware
> +pipelines are controlled via the **V4L2 device** node. They may optionally
> +expose via the :ref:`media controller API <media_controller>`.
> +
> +For **MC-centric** control, before using the V4L2 device, it is required to
> +set the hardware pipelines via the
> +:ref:`media controller API <media_controller>`. For those devices, the
> +sub-devices' configuration can be controlled via the
> +:ref:`sub-device API <subdev>`, with creates one device node per sub
> device.
> +
> +In summary, for **MC-centric** devices:
> +
> +- The **V4L2 device** node is responsible for controlling the streaming
> +  features;
> +- The **media controller device** is responsible to setup the pipelines;
> +- The **V4L2 sub-devices** are responsible for sub-device
> +  specific settings.

How about a summary of vdev-centric devices too ? Or, possibly, no summary at 
all ? The need to summarize 5 short paragraphs probably indicates that those 
paragraphis are not clear enough :-)

I can try to help by proposing enhancements to the documentation once we agree 
on the glossary above.

> +.. note::
> +
> +   A **vdev-centric** may optionally expose V4L2 sub-devices via
> +   :ref:`sub-device API <subdev>`. In that case, it has to implement
> +   the :ref:`media controller API <media_controller>` as well.

The separation between vdev-centric and MC-centric devices is quite clear. If 
we allow a vdev-centric device to expose subdev nodes we will open the door to 
all kind of hybrid implementations that have no clear definition today. It 
will be very important in that case to document in details what is allowed and 
what isn't, and how the video device nodes and subdev device nodes interact 
with each other. I prefer not giving a green light to such implementations 
until we have to, and I also prefer discussing this topic separately. It will 
require a fair amount of work to document (and thus first agree on), and 
there's no need to block the rest of this patch until we complete that work. 
For those reasons I would like to explicitly disallow those hybrid devices in 
this patch, and start discussing the use cases we have for them, and how they 
would operate.

> +.. _v4l2_device_naming:
> 
>  Device Naming
>  =============


-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] media: videodev2: add a flag for vdev-centric devices

[Laurent Pinchart]

Hi Mauro,

On Friday, 25 August 2017 13:06:32 EEST Mauro Carvalho Chehab wrote:
> Em Fri, 25 Aug 2017 11:44:27 +0200 Hans Verkuil escreveu:
> > On 08/25/2017 11:40 AM, Mauro Carvalho Chehab wrote:
> > > From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > > 
> > > As both vdev-centric and mc-centric devices may implement the
> > > same APIs, we need a flag to allow userspace to distinguish
> > > between them.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > ---
> > > 
> > >  Documentation/media/uapi/v4l/open.rst            | 6 ++++++
> > >  Documentation/media/uapi/v4l/vidioc-querycap.rst | 4 ++++
> > >  include/uapi/linux/videodev2.h                   | 2 ++
> > >  3 files changed, 12 insertions(+)
> > > 
> > > diff --git a/Documentation/media/uapi/v4l/open.rst
> > > b/Documentation/media/uapi/v4l/open.rst index
> > > a72d142897c0..eb3f0ec57edb 100644
> > > --- a/Documentation/media/uapi/v4l/open.rst
> > > +++ b/Documentation/media/uapi/v4l/open.rst
> > > @@ -33,6 +33,12 @@ For **vdev-centric** control, the device and their
> > > corresponding hardware> > 
> > >  pipelines are controlled via the **V4L2 device** node. They may
> > >  optionally
> > >  expose via the :ref:`media controller API <media_controller>`.
> > > 
> > > +.. note::
> > > +
> > > +   **vdev-centric** devices should report V4L2_VDEV_CENTERED
> > 
> > You mean CENTRIC, not CENTERED.
> 
> Yeah, true. I'll fix it.
> 
> > But I would change this to MC_CENTRIC: the vast majority of drivers are
> > VDEV centric, so it makes a lot more sense to keep that as the default
> > and only set the cap for MC-centric drivers.
> 
> I actually focused it on what an userspace application would do.
> 
> An specialized application for a given hardware will likely just
> ignore whatever flag is added, and use vdev, mc and subdev APIs
> as it pleases. So, those applications don't need any flag at all.
> 
> However, a generic application needs a flag to allow them to check
> if a given hardware can be controlled by the traditional way
> to control the device (e. g. if it accepts vdev-centric type of
> hardware control).
> 
> It is an old desire (since when MC was designed) to allow that
> generic V4L2 apps to also work with MC-centric hardware somehow.
> 
> At the moment we add that (either in Kernelspace, as proposed for
> iMX6 [1] or via libv4l), a mc-centric hardware can also be
> vdev-centric.
> 
> [1] one alternative proposed for iMX6 driver, would be to enable
>     vdev-centric control only for hardware with a single camera
>     slot, like those cheap RPi3-camera compatible hardware, by
>     using some info at the DT.

DT isn't the right place for this, it should describe the hardware, not how it 
gets exposed by the kernel to userspace. It could be up to each device driver 
to decide, based on the complexity of the hardware as defined in DT, whether 
to expose a vdev-centric or MC-centric API, but I wouldn't recommend that as 
it would drasticly increase the complexity of the driver.

> >> +   :c:type:`v4l2_capability` flag (see :ref:`VIDIOC_QUERYCAP`).
> >> +
> >> +
> >>  For **MC-centric** control, before using the V4L2 device, it is
> >>  required to set the hardware pipelines via the
> >>  :ref:`media controller API <media_controller>`. For those devices, the
> >> diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> >> b/Documentation/media/uapi/v4l/vidioc-querycap.rst index
> >> 12e0d9a63cd8..4856821b7608 100644
> >> --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> >> +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> >> @@ -252,6 +252,10 @@ specification the ioctl returns an ``EINVAL`` error
> >> code.
> >>      * - ``V4L2_CAP_TOUCH``
> >>        - 0x10000000
> >>        - This is a touch device.
> >> +    * - ``V4L2_VDEV_CENTERED``
> >> +      - 0x20000000
> >> +      - This is controlled via V4L2 device nodes (radio, video, vbi,
> >> +        sdr
> >>      * - ``V4L2_CAP_DEVICE_CAPS``
> >>        - 0x80000000
> >>        - The driver fills the ``device_caps`` field. This capability can
> >> 
> >> diff --git a/include/uapi/linux/videodev2.h
> >> b/include/uapi/linux/videodev2.h index 45cf7359822c..d89090d99042
> >> 100644
> >> --- a/include/uapi/linux/videodev2.h
> >> +++ b/include/uapi/linux/videodev2.h
> >> @@ -460,6 +460,8 @@ struct v4l2_capability {
> >> 
> >>  #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch
> >>  device */
> >> 
> >> +#define V4L2_CAP_VDEV_CENTERED          0x20000000  /* V4L2 Device is
> >> controlled via V4L2 device devnode */ +
> >> 
> >>  #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device
> >>  capabilities field */
> >>  
> >>  /*

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Mauro Carvalho Chehab]

Em Fri, 25 Aug 2017 16:11:15 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Friday, 25 August 2017 12:40:05 EEST Mauro Carvalho Chehab wrote:
> > From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
> > 
> > When we added support for omap3, back in 2010, we added a new
> > type of V4L2 devices that aren't fully controlled via the V4L2
> > device node. Yet, we never made it clear, at the V4L2 spec,
> > about the differences between both types.  
> 
> Nitpicking (and there will be lots of nitpicking in this review as I think 
> it's very important to get this piece of the documentation right down to 
> details):

Sure.

> 
> s/at the V4L2 spec/in the V4L2 spec/
> 
> "make it clear about" doesn't sound good to me. How about the following ?
> 
> "Yet, we have never clearly documented in the V4L2 specification the 
> differences between the two types."
> 
> > Let's document them with the current implementation.  
> 
> s/with the/based on the/

OK.

> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > ---
> >  Documentation/media/uapi/v4l/open.rst | 50 ++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/v4l/open.rst
> > b/Documentation/media/uapi/v4l/open.rst index afd116edb40d..a72d142897c0
> > 100644
> > --- a/Documentation/media/uapi/v4l/open.rst
> > +++ b/Documentation/media/uapi/v4l/open.rst
> > @@ -6,6 +6,56 @@
> >  Opening and Closing Devices
> >  ***************************
> > 
> > +Types of V4L2 hardware control
> > +==============================
> > +
> > +V4L2 devices are usually complex: they are implemented via a main driver
> > and
> > +often several additional drivers. The main driver always exposes one or
> > +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).  
> 
> First of all, as stated in a previous e-mail, I think we should start by 
> defining the terms we are going to use. 

Well, we can add a Glossary at the spec, but this is a huge work,
as we'll need to seek for other terms and to adjust all references
to use the Glossary wording. This is a separate work than what this
patch proposes to solve.

Yet, I agree that ambiguity should be solved. I believe that the
second version of this patch series address it.

> The word "device" is ambiguous, it can 
> mean
> 
> - device node
> 
> The /dev device node through which a character- or block-based API is exposed 
> through userspace.
> 
> The term "device node" is commonly used in the kernel, I think we can use it 
> as-is without ambiguity.

Yes. IMO, we should use:

- "device node" when it may refer to any device node,
  including mc and subdev;
- "V4L2 device node" when it refers only to the devices that are now 
  described at the "V4L2 Device Node Naming" section (see patch 1 of the
  new patch series).

> 
> - kernel struct device
> 
> An instance of the kernel struct device. Kernel devices are used to represent 
> hardware devices (and are then embedded in bus-specific device structure such 
> as platform_device, pci_device or i2c_client) and logical devices (and are 
> then embedded in class-spcific device structures such as struct video_device 
> or struct net_device).
> 
> For now I believe this isn't relevant to the V4L2 userspace API discussion, so 
> we can leave it out.

Yes. Let's leave it out.

> - hardware device
> 
> The hardware counterpart of the first category of the kernel struct device, a 
> hardware (and thus physical) device such as an SoC IP core or an I2C chip.
> 
> We could call this "hardware device" or "hardware component". Other proposals 
> are welcome.

as proposed by Hans, on the newer version, we're just using "hardware".

> 
> - hardware peripheral
> 
> A group of hardware devices that together make a larger user-facing functional 
> peripheral. For instance the SoC ISP IP cores and external camera sensors 
> together make a camera peripheral.
> 
> If we call the previous device a "hardware component" this could be called 
> "hardware device". Otherwise "hardware peripheral" is a possible name, but we 
> can probably come up with a better name.
> 
> - software peripheral
> 
> The software counterpart of the hardware peripheral. In V4L2 this is modeled 
> by a struct v4l2_device, often associated with a struct media_device.
> 
> I don't like the name "software peripheral", other proposals are welcome.

I don't see the need of using those "peripheral" wording terms in this chapter.
Let's leave it out for now. 

> Once we agree on names and definitions I'll comment on the reworked version of 
> this patch, as it's pointless to bikeshed the wording without first agreeing 
> on clear definitions of the concepts we're discussing. Please still see below 
> for a few comments not related to particular wording.
> 
> > +The other drivers are called **V4L2 sub-devices** and provide control to
> > +other parts of the hardware usually connected via a serial bus (like
> > +I²C, SMBus or SPI). They can be implicitly controlled directly by the
> > +main driver or explicitly through via the **V4L2 sub-device API**
> > interface.
> > +
> > +When V4L2 was originally designed, there was only one type of device
> > control.
> > +The entire device was controlled via the **V4L2 device nodes**. We refer to
> > +this kind of control as **V4L2 device node centric** (or,
> > simply, +**vdev-centric**).
> > +
> > +Since the end of 2010, a new type of V4L2 device control was added in order
> > +to support complex devices that are common for embedded systems. Those
> > +devices are controlled mainly via the media controller and sub-devices.
> > +So, they're called: **Media controller centric** (or, simply,
> > +"**MC-centric**").  
> 
> Do we really need to explain the development history with dates ? I don't 
> think it helps understanding the separation between vdev-centric and MC-
> centric devices.

No, but it helps to identify when drivers became incompatible with
generic apps. We could, instead point on what Kernel version it
happened. That probably makes more sense and will match the history
part of the spec.

> For instance the previous paragraph that explains that V4L2 
> originally had a single type of device control that we now call vdev-centric 
> doesn't really help understanding what a vdev-centric device is.
> 
> It would be clearer in my opinion to explain the two kind of devices with an 
> associated use case, without referring to the history.

The history is important for app developers, as they need to know
what Kernel versions are affected by V4L2 device nodes that look
like vdev-centric ones but aren't.

Yet, you're right on one point: the date doesn't matter, but, instead,
the Kernel version where the first mc-centric driver were added.

> > +For **vdev-centric** control, the device and their corresponding hardware
> > +pipelines are controlled via the **V4L2 device** node. They may optionally
> > +expose via the :ref:`media controller API <media_controller>`.
> > +
> > +For **MC-centric** control, before using the V4L2 device, it is required to
> > +set the hardware pipelines via the
> > +:ref:`media controller API <media_controller>`. For those devices, the
> > +sub-devices' configuration can be controlled via the
> > +:ref:`sub-device API <subdev>`, with creates one device node per sub
> > device.
> > +
> > +In summary, for **MC-centric** devices:
> > +
> > +- The **V4L2 device** node is responsible for controlling the streaming
> > +  features;
> > +- The **media controller device** is responsible to setup the pipelines;
> > +- The **V4L2 sub-devices** are responsible for sub-device
> > +  specific settings.  
> 
> How about a summary of vdev-centric devices too ? Or, possibly, no summary at 
> all ? The need to summarize 5 short paragraphs probably indicates that those 
> paragraphis are not clear enough :-)

Done at version 2, already submitted.

> 
> I can try to help by proposing enhancements to the documentation once we agree 
> on the glossary above.
> 
> > +.. note::
> > +
> > +   A **vdev-centric** may optionally expose V4L2 sub-devices via
> > +   :ref:`sub-device API <subdev>`. In that case, it has to implement
> > +   the :ref:`media controller API <media_controller>` as well.  
> 
> The separation between vdev-centric and MC-centric devices is quite clear. If 
> we allow a vdev-centric device to expose subdev nodes we will open the door to 
> all kind of hybrid implementations that have no clear definition today. 

There are already such devices, as discussed at #media-maint ML. We're
just documenting the existing status.

> It 
> will be very important in that case to document in details what is allowed and 
> what isn't, and how the video device nodes and subdev device nodes interact 
> with each other. I prefer not giving a green light to such implementations 
> until we have to, and I also prefer discussing this topic separately. It will 
> require a fair amount of work to document (and thus first agree on), and 
> there's no need to block the rest of this patch until we complete that work. 
> For those reasons I would like to explicitly disallow those hybrid devices in 
> this patch, and start discussing the use cases we have for them, and how they 
> would operate.

What we can do is to add a notice that this is not recommended or
that people should not add it without a discussion about what that
is needed.

> 
> > +.. _v4l2_device_naming:
> > 
> >  Device Naming
> >  =============  
> 
> 



Thanks,
Mauro

Re: [PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Laurent Pinchart]

Hi Mauro,

On Friday, 25 August 2017 16:38:23 EEST Mauro Carvalho Chehab wrote:
> Em Fri, 25 Aug 2017 16:11:15 +0300 Laurent Pinchart escreveu:
> > On Friday, 25 August 2017 12:40:05 EEST Mauro Carvalho Chehab wrote:
> >> From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
> >> 
> >> When we added support for omap3, back in 2010, we added a new
> >> type of V4L2 devices that aren't fully controlled via the V4L2
> >> device node. Yet, we never made it clear, at the V4L2 spec,
> >> about the differences between both types.
> > 
> > Nitpicking (and there will be lots of nitpicking in this review as I think
> > it's very important to get this piece of the documentation right down to
> > details):
> >
> Sure.
> 
> > s/at the V4L2 spec/in the V4L2 spec/
> > 
> > "make it clear about" doesn't sound good to me. How about the following ?
> > 
> > "Yet, we have never clearly documented in the V4L2 specification the
> > differences between the two types."
> > 
> >> Let's document them with the current implementation.
> > 
> > s/with the/based on the/
> 
> OK.
> 
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> >> ---
> >> 
> >>  Documentation/media/uapi/v4l/open.rst | 50 +++++++++++++++++++++++++++++
> >>  1 file changed, 50 insertions(+)
> >> 
> >> diff --git a/Documentation/media/uapi/v4l/open.rst
> >> b/Documentation/media/uapi/v4l/open.rst index afd116edb40d..a72d142897c0
> >> 100644
> >> --- a/Documentation/media/uapi/v4l/open.rst
> >> +++ b/Documentation/media/uapi/v4l/open.rst
> >> @@ -6,6 +6,56 @@
> >>  Opening and Closing Devices
> >>  ***************************
> >> 
> >> +Types of V4L2 hardware control
> >> +==============================
> >> +
> >> +V4L2 devices are usually complex: they are implemented via a main
> >> driver and
> >> +often several additional drivers. The main driver always exposes one or
> >> +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).
> > 
> > First of all, as stated in a previous e-mail, I think we should start by
> > defining the terms we are going to use.
> 
> Well, we can add a Glossary at the spec, but this is a huge work,
> as we'll need to seek for other terms and to adjust all references
> to use the Glossary wording. This is a separate work than what this
> patch proposes to solve.
> 
> Yet, I agree that ambiguity should be solved. I believe that the
> second version of this patch series address it.

As discussed over IRC we don't need a full glossary. We can start with 
definitions of the terms we use here, and enrich it incrementally in the 
future.

> > The word "device" is ambiguous, it can mean
> > 
> > - device node
> > 
> > The /dev device node through which a character- or block-based API is
> > exposed through userspace.
> > 
> > The term "device node" is commonly used in the kernel, I think we can use
> > it as-is without ambiguity.
> 
> Yes. IMO, we should use:
> 
> - "device node" when it may refer to any device node,
>   including mc and subdev;
> - "V4L2 device node" when it refers only to the devices that are now
>   described at the "V4L2 Device Node Naming" section (see patch 1 of the
>   new patch series).

And "V4L2 subdev device node" and "MC device node" when we want to refer 
specifically to the /dev/v4l-subdev* and /dev/media* device nodes ?

> > - kernel struct device
> > 
> > An instance of the kernel struct device. Kernel devices are used to
> > represent hardware devices (and are then embedded in bus-specific device
> > structure such as platform_device, pci_device or i2c_client) and logical
> > devices (and are then embedded in class-spcific device structures such as
> > struct video_device or struct net_device).
> > 
> > For now I believe this isn't relevant to the V4L2 userspace API
> > discussion, so we can leave it out.
> 
> Yes. Let's leave it out.
> 
> > - hardware device
> > 
> > The hardware counterpart of the first category of the kernel struct
> > device, a hardware (and thus physical) device such as an SoC IP core or
> > an I2C chip.
> > 
> > We could call this "hardware device" or "hardware component". Other
> > proposals are welcome.
> 
> as proposed by Hans, on the newer version, we're just using "hardware".

Hardware is very generic too. A PCI capture card is hardware, an antenna 
connector is hardware, a TV tuner is hardware, a DMA engine is hardware.

The distinction between components and peripherals is in my opinion important, 
otherwise we wouldn't have both struct video_device and struct v4l2_device. 
Userspace middleware developers (I include libv4l2 in this category) will be 
more interested in components, which applications developers will be more 
interested in peripherals. In the longer term, we should introduce a 
peripheral concept for userspace too (in the interface between middlewares and 
applications), the same way we have done it in the kernel with struct 
v4l2_device.

> > - hardware peripheral
> > 
> > A group of hardware devices that together make a larger user-facing
> > functional peripheral. For instance the SoC ISP IP cores and external
> > camera sensors together make a camera peripheral.
> > 
> > If we call the previous device a "hardware component" this could be called
> > "hardware device". Otherwise "hardware peripheral" is a possible name, but
> > we can probably come up with a better name.
> > 
> > - software peripheral
> > 
> > The software counterpart of the hardware peripheral. In V4L2 this is
> > modeled by a struct v4l2_device, often associated with a struct
> > media_device.
> > 
> > I don't like the name "software peripheral", other proposals are welcome.
> 
> I don't see the need of using those "peripheral" wording terms in this
> chapter. Let's leave it out for now.

I think they're needed. When you talk about pipeline configuration for 
instance, you talk about peripherals. Individual components don't have 
pipelines, they are assembled in a pipeline in a peripheral. For instance, 
when you say "The entire device was controlled via the V4L2 device nodes", the 
usage of the word "device" is ambiguous. I think we should avoid using 
"device" without any qualifier except in context where there can be no 
confusion (for instance in a section that repeatedly talk about device nodes 
and devices nodes only, the word "device" could be used in some places as a 
shortcut).

> > Once we agree on names and definitions I'll comment on the reworked
> > version of this patch, as it's pointless to bikeshed the wording without
> > first agreeing on clear definitions of the concepts we're discussing.
> > Please still see below for a few comments not related to particular
> > wording.
> > 
> >> +The other drivers are called **V4L2 sub-devices** and provide control
> >> to
> >> +other parts of the hardware usually connected via a serial bus (like
> >> +I²C, SMBus or SPI). They can be implicitly controlled directly by the
> >> +main driver or explicitly through via the **V4L2 sub-device API**
> >> interface.
> > > +
> >> +When V4L2 was originally designed, there was only one type of device
> >> control.
> >> +The entire device was controlled via the **V4L2 device nodes**. We
> >> refer to
> >> +this kind of control as **V4L2 device node centric** (or, simply,
> >> +**vdev-centric**).
> >> +
> >> +Since the end of 2010, a new type of V4L2 device control was added in
> >> order
> >> +to support complex devices that are common for embedded systems. Those
> >> +devices are controlled mainly via the media controller and sub-devices.
> >> +So, they're called: **Media controller centric** (or, simply,
> > > +"**MC-centric**").
> > 
> > Do we really need to explain the development history with dates ? I don't
> > think it helps understanding the separation between vdev-centric and MC-
> > centric devices.
> 
> No, but it helps to identify when drivers became incompatible with
> generic apps. We could, instead point on what Kernel version it
> happened. That probably makes more sense and will match the history
> part of the spec.

That was v2.6.39. Do you expect userspace developers to target kernels older 
than that ? :-)

> > For instance the previous paragraph that explains that V4L2
> > originally had a single type of device control that we now call
> > vdev-centric doesn't really help understanding what a vdev-centric device
> > is.
> > 
> > It would be clearer in my opinion to explain the two kind of devices with
> > an associated use case, without referring to the history.
> 
> The history is important for app developers, as they need to know
> what Kernel versions are affected by V4L2 device nodes that look
> like vdev-centric ones but aren't.
> 
> Yet, you're right on one point: the date doesn't matter, but, instead,
> the Kernel version where the first mc-centric driver were added.
> 
> >> +For **vdev-centric** control, the device and their corresponding
> >> hardware
> >> +pipelines are controlled via the **V4L2 device** node. They may
> >> optionally
> >> +expose via the :ref:`media controller API <media_controller>`.
> >> +
> >> +For **MC-centric** control, before using the V4L2 device, it is
> >> required to +set the hardware pipelines via the
> >> +:ref:`media controller API <media_controller>`. For those devices, the
> >> +sub-devices' configuration can be controlled via the
> >> +:ref:`sub-device API <subdev>`, with creates one device node per sub
> >> device.
> >> +
> >> +In summary, for **MC-centric** devices:
> >> +
> >> +- The **V4L2 device** node is responsible for controlling the streaming
> >> +  features;
> >> +- The **media controller device** is responsible to setup the
> >> pipelines;
> >> +- The **V4L2 sub-devices** are responsible for sub-device
> >> +  specific settings.
> > 
> > How about a summary of vdev-centric devices too ? Or, possibly, no summary
> > at all ? The need to summarize 5 short paragraphs probably indicates that
> > those paragraphis are not clear enough :-)
> 
> Done at version 2, already submitted.

I'll review v3 once we agree on definitions for device-related terms :-)

> > I can try to help by proposing enhancements to the documentation once we
> > agree on the glossary above.
> > 
> >> +.. note::
> >> +
> >> +   A **vdev-centric** may optionally expose V4L2 sub-devices via
> >> +   :ref:`sub-device API <subdev>`. In that case, it has to implement
> >> +   the :ref:`media controller API <media_controller>` as well.
> > 
> > The separation between vdev-centric and MC-centric devices is quite clear.
> > If we allow a vdev-centric device to expose subdev nodes we will open the
> > door to all kind of hybrid implementations that have no clear definition
> > today.
> 
> There are already such devices, as discussed at #media-maint ML. We're
> just documenting the existing status.

The specification doesn't need to document the odd cases that we would have 
rejected if we had known better.

> > It will be very important in that case to document in details what is
> > allowed and what isn't, and how the video device nodes and subdev device
> > nodes interact with each other. I prefer not giving a green light to such
> > implementations until we have to, and I also prefer discussing this topic
> > separately. It will require a fair amount of work to document (and thus
> > first agree on), and there's no need to block the rest of this patch
> > until we complete that work. For those reasons I would like to explicitly
> > disallow those hybrid devices in this patch, and start discussing the use
> > cases we have for them, and how they would operate.
> 
> What we can do is to add a notice that this is not recommended or
> that people should not add it without a discussion about what that
> is needed.

How about just saying that's it's forbidden by the specification ? We will 
have a few drivers that don't comply, but we will obviously not blame their 
authors.

I prefer using the strongest possible wording when we want to discourage 
something. There will always be developers who will still try, but the 
stronger the wording, the lower the risk.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] media: open.rst: document devnode-centric and mc-centric types

[Hans Verkuil]

On 25/08/17 15:11, Laurent Pinchart wrote:
> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Friday, 25 August 2017 12:40:05 EEST Mauro Carvalho Chehab wrote:
>> From: "mchehab@s-opensource.com" <mchehab@s-opensource.com>
>>
>> When we added support for omap3, back in 2010, we added a new
>> type of V4L2 devices that aren't fully controlled via the V4L2
>> device node. Yet, we never made it clear, at the V4L2 spec,
>> about the differences between both types.
> 
> Nitpicking (and there will be lots of nitpicking in this review as I think 
> it's very important to get this piece of the documentation right down to 
> details):
> 
> s/at the V4L2 spec/in the V4L2 spec/
> 
> "make it clear about" doesn't sound good to me. How about the following ?
> 
> "Yet, we have never clearly documented in the V4L2 specification the 
> differences between the two types."
> 
>> Let's document them with the current implementation.
> 
> s/with the/based on the/
> 
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
>> ---
>>  Documentation/media/uapi/v4l/open.rst | 50 ++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>
>> diff --git a/Documentation/media/uapi/v4l/open.rst
>> b/Documentation/media/uapi/v4l/open.rst index afd116edb40d..a72d142897c0
>> 100644
>> --- a/Documentation/media/uapi/v4l/open.rst
>> +++ b/Documentation/media/uapi/v4l/open.rst
>> @@ -6,6 +6,56 @@
>>  Opening and Closing Devices
>>  ***************************
>>
>> +Types of V4L2 hardware control
>> +==============================
>> +
>> +V4L2 devices are usually complex: they are implemented via a main driver
>> and
>> +often several additional drivers. The main driver always exposes one or
>> +more **V4L2 device** devnodes (see :ref:`v4l2_device_naming`).
> 
> First of all, as stated in a previous e-mail, I think we should start by 
> defining the terms we are going to use. The word "device" is ambiguous, it can 
> mean
> 
> - device node
> 
> The /dev device node through which a character- or block-based API is exposed 
> through userspace.
> 
> The term "device node" is commonly used in the kernel, I think we can use it 
> as-is without ambiguity.

Ack.

> 
> - kernel struct device
> 
> An instance of the kernel struct device. Kernel devices are used to represent 
> hardware devices (and are then embedded in bus-specific device structure such 
> as platform_device, pci_device or i2c_client) and logical devices (and are 
> then embedded in class-spcific device structures such as struct video_device 
> or struct net_device).
> 
> For now I believe this isn't relevant to the V4L2 userspace API discussion, so 
> we can leave it out.

Agree.

> 
> - hardware device
> 
> The hardware counterpart of the first category of the kernel struct device, a 
> hardware (and thus physical) device such as an SoC IP core or an I2C chip.
> 
> We could call this "hardware device" or "hardware component". Other proposals 
> are welcome.

I like "hardware component". I prefer to avoid the word 'device' as much as possible.

> 
> - hardware peripheral
> 
> A group of hardware devices that together make a larger user-facing functional 
> peripheral. For instance the SoC ISP IP cores and external camera sensors 
> together make a camera peripheral.
> 
> If we call the previous device a "hardware component" this could be called 
> "hardware device". Otherwise "hardware peripheral" is a possible name, but we 
> can probably come up with a better name.

"hardware device" can be used provided it is clearly defined as being a collection
of hardware components that together form the fully functioning device. Or something
along those lines.

> 
> - software peripheral
> 
> The software counterpart of the hardware peripheral. In V4L2 this is modeled 
> by a struct v4l2_device, often associated with a struct media_device.

struct v4l2_device isn't visible to userspace (and is an awful name, I'm
really sorry about that, guys!).

> 
> I don't like the name "software peripheral", other proposals are welcome.

Where would you use the term "software peripheral"? In what context? Do we
need this at all?

> 
> Once we agree on names and definitions I'll comment on the reworked version of 
> this patch, as it's pointless to bikeshed the wording without first agreeing 
> on clear definitions of the concepts we're discussing. Please still see below 
> for a few comments not related to particular wording.
> 
>> +The other drivers are called **V4L2 sub-devices** and provide control to
>> +other parts of the hardware usually connected via a serial bus (like
>> +I²C, SMBus or SPI). They can be implicitly controlled directly by the
>> +main driver or explicitly through via the **V4L2 sub-device API**
>> interface.
>> +
>> +When V4L2 was originally designed, there was only one type of device
>> control.
>> +The entire device was controlled via the **V4L2 device nodes**. We refer to
>> +this kind of control as **V4L2 device node centric** (or,
>> simply, +**vdev-centric**).
>> +
>> +Since the end of 2010, a new type of V4L2 device control was added in order
>> +to support complex devices that are common for embedded systems. Those
>> +devices are controlled mainly via the media controller and sub-devices.
>> +So, they're called: **Media controller centric** (or, simply,
>> +"**MC-centric**").
> 
> Do we really need to explain the development history with dates ? I don't 
> think it helps understanding the separation between vdev-centric and MC-
> centric devices. For instance the previous paragraph that explains that V4L2 
> originally had a single type of device control that we now call vdev-centric 
> doesn't really help understanding what a vdev-centric device is.
> 
> It would be clearer in my opinion to explain the two kind of devices with an 
> associated use case, without referring to the history.

It doesn't hurt to mention that MC-centric came later. There are after all API
oddities that are due to the history of the API. But we can simply say "Later"
instead of "Since the end of 2010".

>> +For **vdev-centric** control, the device and their corresponding hardware
>> +pipelines are controlled via the **V4L2 device** node. They may optionally
>> +expose via the :ref:`media controller API <media_controller>`.
>> +
>> +For **MC-centric** control, before using the V4L2 device, it is required to
>> +set the hardware pipelines via the
>> +:ref:`media controller API <media_controller>`. For those devices, the
>> +sub-devices' configuration can be controlled via the
>> +:ref:`sub-device API <subdev>`, with creates one device node per sub
>> device.
>> +
>> +In summary, for **MC-centric** devices:
>> +
>> +- The **V4L2 device** node is responsible for controlling the streaming
>> +  features;
>> +- The **media controller device** is responsible to setup the pipelines;
>> +- The **V4L2 sub-devices** are responsible for sub-device
>> +  specific settings.
> 
> How about a summary of vdev-centric devices too ? Or, possibly, no summary at 
> all ? The need to summarize 5 short paragraphs probably indicates that those 
> paragraphis are not clear enough :-)

I was thinking the same. It should probably be dropped.

> I can try to help by proposing enhancements to the documentation once we agree 
> on the glossary above.
> 
>> +.. note::
>> +
>> +   A **vdev-centric** may optionally expose V4L2 sub-devices via
>> +   :ref:`sub-device API <subdev>`. In that case, it has to implement
>> +   the :ref:`media controller API <media_controller>` as well.
> 
> The separation between vdev-centric and MC-centric devices is quite clear. If 
> we allow a vdev-centric device to expose subdev nodes we will open the door to 
> all kind of hybrid implementations that have no clear definition today. It 
> will be very important in that case to document in details what is allowed and 
> what isn't, and how the video device nodes and subdev device nodes interact 
> with each other. I prefer not giving a green light to such implementations 
> until we have to

Well, they exist already. The original MC design that I made allowed for this.
In practice the only reason for doing this is to set private controls for
subdevs (i.e. controls not inherited by the video device node).

Three drivers use this: rcar-drif, cobalt (not an issue, I can just remove
support for this as Cisco is the only user) and I know there was a third one
that you found, but I keep forgetting the name of that driver.

>, and I also prefer discussing this topic separately. It will 
> require a fair amount of work to document (and thus first agree on), and 
> there's no need to block the rest of this patch until we complete that work. 
> For those reasons I would like to explicitly disallow those hybrid devices in 
> this patch, and start discussing the use cases we have for them, and how they 
> would operate.

I have no objection to dropping this note for the first version.

Regards,

	Hans

> 
>> +.. _v4l2_device_naming:
>>
>>  Device Naming
>>  =============
> 
>