[PATCHv4 00/15] Media Controller compliance fixes

[PATCHv4 00/15] Media Controller compliance fixes

[Hans Verkuil]

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

Hi all,

Here is v4 of this patch series fixing various MC compliance and 
documentation issues.

Changes since v3:
- Added Acks from Sakari
- Incorporated Sakari's comments for the final patch (media.h).

Changes since v2:

- I dropped "v4l2-ioctl.c: fix VIDIOC_DV_TIMINGS_CAP: don't clear pad"
  as it is already included in the pull request for the tda1997x driver
  which needs this fix.
- I added a TODO as requested by Sakari to "v4l2-subdev: without controls 
  return -ENOTTY"
- The "media: document and zero reservedX fields in media_v2_topology"
  patch has been split up in separate code and doc patches.
- I now also zero the reserved field of struct media_links_enum.
  (Thanks Sakari!)
- Rebased and added Acks from Sakari.
- Added missing documentation for the reserved field of struct
  media_links_enum.
- Instead of requiring that apps and drivers zero the reserved fields,
  now change that to just drivers.

I plan to post a pull request later this week if there are no more
comments.

Note regarding patch 4 ("v4l2-subdev: clear reserved fields"): this is
also present in the g/s_frame_interval pull request:

https://patchwork.linuxtv.org/patch/47296/

since it is a prerequisite for that patch series. Whichever gets merged
first wins...

The final patch reorganizes media.h so it is actually understandable.
See here for how it looks after the patch is applied:

https://git.linuxtv.org/hverkuil/media_tree.git/plain/include/uapi/linux/media.h?h=media-fixes

The patch itself is hard to read, so looking at the reorganized header
is easier.

The two core changes in media.h are:

1) all functions are now grouped together
2) all legacy defines are now all moved to the end of the header

I would really like to see this merged soon. This fixes the most immediate
problems that I found.

Regards,

	Hans

Alexandre Courbot (1):
  media: media-types.rst: fix typo

Hans Verkuil (14):
  vimc: fix control event handling
  vimc: use correct subdev functions
  v4l2-subdev: without controls return -ENOTTY
  v4l2-subdev: clear reserved fields
  v4l2-subdev: implement VIDIOC_DBG_G_CHIP_INFO ioctl
  subdev-formats.rst: fix incorrect types
  media-ioc-g-topology.rst: fix interface-to-entity link description
  media-types.rst: fix type, small improvements
  media-device.c: zero reserved fields
  media.h: fix confusing typo in comment
  media: zero reservedX fields in media_v2_topology
  media: document the reservedX fields in media_v2_topology
  media-ioc-enum-entities/links.rst: document reserved fields
  media.h: reorganize header to make it easier to understand

 .../uapi/mediactl/media-ioc-enum-entities.rst      |  19 +-
 .../media/uapi/mediactl/media-ioc-enum-links.rst   |  18 ++
 .../media/uapi/mediactl/media-ioc-g-topology.rst   |  54 +++-
 Documentation/media/uapi/mediactl/media-types.rst  |  12 +-
 Documentation/media/uapi/v4l/subdev-formats.rst    |   6 +-
 drivers/media/media-device.c                       |   7 +
 drivers/media/media-entity.c                       |  16 -
 drivers/media/platform/vimc/vimc-common.c          |   4 +-
 drivers/media/platform/vimc/vimc-debayer.c         |   2 +-
 drivers/media/platform/vimc/vimc-scaler.c          |   2 +-
 drivers/media/platform/vimc/vimc-sensor.c          |  10 +-
 drivers/media/v4l2-core/v4l2-subdev.c              |  50 +++
 include/uapi/linux/media.h                         | 345 ++++++++++-----------
 13 files changed, 322 insertions(+), 223 deletions(-)

-- 
2.16.1

[PATCHv4 01/15] vimc: fix control event handling

[Hans Verkuil]

The sensor subdev didn't handle control events. Add support for this.
Found with v4l2-compliance.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/vimc/vimc-common.c | 4 +++-
 drivers/media/platform/vimc/vimc-sensor.c | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index 9d63c84a9876..617415c224fe 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -434,7 +434,9 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
 	v4l2_set_subdevdata(sd, ved);
 
 	/* Expose this subdev to user space */
-	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	if (sd->ctrl_handler)
+		sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
 
 	/* Initialize the media entity */
 	ret = media_entity_pads_init(&sd->entity, num_pads, ved->pads);
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 457e211514c6..54184cd9e0ff 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -23,6 +23,7 @@
 #include <linux/v4l2-mediabus.h>
 #include <linux/vmalloc.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-subdev.h>
 #include <media/tpg/v4l2-tpg.h>
 
@@ -284,11 +285,18 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
+static struct v4l2_subdev_core_ops vimc_sen_core_ops = {
+	.log_status = v4l2_ctrl_subdev_log_status,
+	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
 static const struct v4l2_subdev_video_ops vimc_sen_video_ops = {
 	.s_stream = vimc_sen_s_stream,
 };
 
 static const struct v4l2_subdev_ops vimc_sen_ops = {
+	.core = &vimc_sen_core_ops,
 	.pad = &vimc_sen_pad_ops,
 	.video = &vimc_sen_video_ops,
 };
-- 
2.16.1

[PATCHv4 02/15] vimc: use correct subdev functions

[Hans Verkuil]

Instead of calling everything a MEDIA_ENT_F_ATV_DECODER, pick the
correct functions for these blocks.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/vimc/vimc-debayer.c | 2 +-
 drivers/media/platform/vimc/vimc-scaler.c  | 2 +-
 drivers/media/platform/vimc/vimc-sensor.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 4d663e89d33f..6e10b63ba9ec 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -533,7 +533,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
 	/* Initialize ved and sd */
 	ret = vimc_ent_sd_register(&vdeb->ved, &vdeb->sd, v4l2_dev,
 				   pdata->entity_name,
-				   MEDIA_ENT_F_ATV_DECODER, 2,
+				   MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV, 2,
 				   (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
 				   MEDIA_PAD_FL_SOURCE},
 				   &vimc_deb_ops);
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index e1602e0bc230..e583ec7a91da 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -395,7 +395,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
 	/* Initialize ved and sd */
 	ret = vimc_ent_sd_register(&vsca->ved, &vsca->sd, v4l2_dev,
 				   pdata->entity_name,
-				   MEDIA_ENT_F_ATV_DECODER, 2,
+				   MEDIA_ENT_F_PROC_VIDEO_SCALER, 2,
 				   (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
 				   MEDIA_PAD_FL_SOURCE},
 				   &vimc_sca_ops);
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 54184cd9e0ff..605e2a2d5dd5 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -386,7 +386,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
 	/* Initialize ved and sd */
 	ret = vimc_ent_sd_register(&vsen->ved, &vsen->sd, v4l2_dev,
 				   pdata->entity_name,
-				   MEDIA_ENT_F_ATV_DECODER, 1,
+				   MEDIA_ENT_F_CAM_SENSOR, 1,
 				   (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE},
 				   &vimc_sen_ops);
 	if (ret)
-- 
2.16.1

[PATCHv4 03/15] v4l2-subdev: without controls return -ENOTTY

[Hans Verkuil]

If the subdev did not define any controls, then return -ENOTTY if
userspace attempts to call these ioctls.

The control framework functions will return -EINVAL, not -ENOTTY if
vfh->ctrl_handler is NULL.

Several of these framework functions are also called directly from
drivers, so I don't want to change the error code there.

Found with vimc and v4l2-compliance.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index c5639817db34..b14c5e1705ca 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -187,27 +187,51 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 
 	switch (cmd) {
 	case VIDIOC_QUERYCTRL:
+		/*
+		 * TODO: this really should be folded into v4l2_queryctrl (this
+		 * currently returns -EINVAL for NULL control handlers).
+		 * However, v4l2_queryctrl() is still called directly by
+		 * drivers as well and until that has been addressed I believe
+		 * it is safer to do the check here. The same is true for the
+		 * other control ioctls below.
+		 */
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_queryctrl(vfh->ctrl_handler, arg);
 
 	case VIDIOC_QUERY_EXT_CTRL:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_query_ext_ctrl(vfh->ctrl_handler, arg);
 
 	case VIDIOC_QUERYMENU:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_querymenu(vfh->ctrl_handler, arg);
 
 	case VIDIOC_G_CTRL:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_g_ctrl(vfh->ctrl_handler, arg);
 
 	case VIDIOC_S_CTRL:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_s_ctrl(vfh, vfh->ctrl_handler, arg);
 
 	case VIDIOC_G_EXT_CTRLS:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_g_ext_ctrls(vfh->ctrl_handler, arg);
 
 	case VIDIOC_S_EXT_CTRLS:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_s_ext_ctrls(vfh, vfh->ctrl_handler, arg);
 
 	case VIDIOC_TRY_EXT_CTRLS:
+		if (!vfh->ctrl_handler)
+			return -ENOTTY;
 		return v4l2_try_ext_ctrls(vfh->ctrl_handler, arg);
 
 	case VIDIOC_DQEVENT:
-- 
2.16.1

[PATCHv4 04/15] v4l2-subdev: clear reserved fields

[Hans Verkuil]

Clear the reserved fields for these ioctls according to the specification:

VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL
VIDIOC_SUBDEV_ENUM_FRAME_SIZE
VIDIOC_SUBDEV_ENUM_MBUS_CODE
VIDIOC_SUBDEV_G_CROP, VIDIOC_SUBDEV_S_CROP
VIDIOC_SUBDEV_G_FMT, VIDIOC_SUBDEV_S_FMT
VIDIOC_SUBDEV_G_FRAME_INTERVAL, VIDIOC_SUBDEV_S_FRAME_INTERVAL
VIDIOC_SUBDEV_G_SELECTION, VIDIOC_SUBDEV_S_SELECTION

Found with v4l2-compliance.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index b14c5e1705ca..168ecb4bed23 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -284,6 +284,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (rval)
 			return rval;
 
+		memset(format->reserved, 0, sizeof(format->reserved));
+		memset(format->format.reserved, 0, sizeof(format->format.reserved));
 		return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh->pad, format);
 	}
 
@@ -294,6 +296,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (rval)
 			return rval;
 
+		memset(format->reserved, 0, sizeof(format->reserved));
+		memset(format->format.reserved, 0, sizeof(format->format.reserved));
 		return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);
 	}
 
@@ -301,6 +305,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		struct v4l2_subdev_crop *crop = arg;
 		struct v4l2_subdev_selection sel;
 
+		memset(crop->reserved, 0, sizeof(crop->reserved));
 		rval = check_crop(sd, crop);
 		if (rval)
 			return rval;
@@ -326,6 +331,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (rval)
 			return rval;
 
+		memset(crop->reserved, 0, sizeof(crop->reserved));
 		memset(&sel, 0, sizeof(sel));
 		sel.which = crop->which;
 		sel.pad = crop->pad;
@@ -350,6 +356,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (code->pad >= sd->entity.num_pads)
 			return -EINVAL;
 
+		memset(code->reserved, 0, sizeof(code->reserved));
 		return v4l2_subdev_call(sd, pad, enum_mbus_code, subdev_fh->pad,
 					code);
 	}
@@ -364,6 +371,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (fse->pad >= sd->entity.num_pads)
 			return -EINVAL;
 
+		memset(fse->reserved, 0, sizeof(fse->reserved));
 		return v4l2_subdev_call(sd, pad, enum_frame_size, subdev_fh->pad,
 					fse);
 	}
@@ -374,6 +382,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (fi->pad >= sd->entity.num_pads)
 			return -EINVAL;
 
+		memset(fi->reserved, 0, sizeof(fi->reserved));
 		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
 	}
 
@@ -383,6 +392,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (fi->pad >= sd->entity.num_pads)
 			return -EINVAL;
 
+		memset(fi->reserved, 0, sizeof(fi->reserved));
 		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
 	}
 
@@ -396,6 +406,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (fie->pad >= sd->entity.num_pads)
 			return -EINVAL;
 
+		memset(fie->reserved, 0, sizeof(fie->reserved));
 		return v4l2_subdev_call(sd, pad, enum_frame_interval, subdev_fh->pad,
 					fie);
 	}
@@ -407,6 +418,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (rval)
 			return rval;
 
+		memset(sel->reserved, 0, sizeof(sel->reserved));
 		return v4l2_subdev_call(
 			sd, pad, get_selection, subdev_fh->pad, sel);
 	}
@@ -418,6 +430,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (rval)
 			return rval;
 
+		memset(sel->reserved, 0, sizeof(sel->reserved));
 		return v4l2_subdev_call(
 			sd, pad, set_selection, subdev_fh->pad, sel);
 	}
-- 
2.16.1

[PATCHv4 05/15] v4l2-subdev: implement VIDIOC_DBG_G_CHIP_INFO ioctl

[Hans Verkuil]

The VIDIOC_DBG_G/S_REGISTER ioctls imply that VIDIOC_DBG_G_CHIP_INFO is also
present, since without that you cannot use v4l2-dbg.

Just like the implementation in v4l2-ioctl.c this can be implemented in the
core and no drivers need to be modified.

It also makes it possible for v4l2-compliance to properly test the
VIDIOC_DBG_G/S_REGISTER ioctls.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 168ecb4bed23..7cff4922e8d1 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -263,6 +263,19 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 			return -EPERM;
 		return v4l2_subdev_call(sd, core, s_register, p);
 	}
+	case VIDIOC_DBG_G_CHIP_INFO:
+	{
+		struct v4l2_dbg_chip_info *p = arg;
+
+		if (p->match.type != V4L2_CHIP_MATCH_SUBDEV || p->match.addr)
+			return -EINVAL;
+		if (sd->ops->core && sd->ops->core->s_register)
+			p->flags |= V4L2_CHIP_FL_WRITABLE;
+		if (sd->ops->core && sd->ops->core->g_register)
+			p->flags |= V4L2_CHIP_FL_READABLE;
+		strlcpy(p->name, sd->name, sizeof(p->name));
+		return 0;
+	}
 #endif
 
 	case VIDIOC_LOG_STATUS: {
-- 
2.16.1

[PATCHv4 06/15] subdev-formats.rst: fix incorrect types

[Hans Verkuil]

The ycbcr_enc, quantization and xfer_func fields are __u16 and not enums.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/media/uapi/v4l/subdev-formats.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst b/Documentation/media/uapi/v4l/subdev-formats.rst
index b1eea44550e1..4f0c0b282f98 100644
--- a/Documentation/media/uapi/v4l/subdev-formats.rst
+++ b/Documentation/media/uapi/v4l/subdev-formats.rst
@@ -33,17 +33,17 @@ Media Bus Formats
       - Image colorspace, from enum
 	:c:type:`v4l2_colorspace`. See
 	:ref:`colorspaces` for details.
-    * - enum :c:type:`v4l2_ycbcr_encoding`
+    * - __u16
       - ``ycbcr_enc``
       - This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
 	streams, see :ref:`colorspaces`.
-    * - enum :c:type:`v4l2_quantization`
+    * - __u16
       - ``quantization``
       - This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
 	streams, see :ref:`colorspaces`.
-    * - enum :c:type:`v4l2_xfer_func`
+    * - __u16
       - ``xfer_func``
       - This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
-- 
2.16.1

[PATCHv4 07/15] media-ioc-g-topology.rst: fix interface-to-entity link description

[Hans Verkuil]

The source_id and sink_id descriptions were the same for interface-to-entity
links. The source_id is the interface ID, not the entity ID. Fix this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/media/uapi/mediactl/media-ioc-g-topology.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index 997e6b17440d..870a6c0d1f7a 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -334,7 +334,7 @@ desired arrays with the media graph elements.
 
        -  On pad to pad links: unique ID for the source pad.
 
-	  On interface to entity links: unique ID for the entity.
+	  On interface to entity links: unique ID for the interface.
 
     -  .. row 3
 
-- 
2.16.1

[PATCHv4 08/15] media-types.rst: fix type, small improvements

[Hans Verkuil]

data conector -> connector

... -> etc.

'...' looked odd when my browser put the ... by itself on the next line, 'etc.'
is clearer IMHO.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/media/uapi/mediactl/media-types.rst | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst
index 8d64b0c06ebc..9c1e3d3f590c 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -293,7 +293,7 @@ Types and flags used to represent the media graph elements
 
        -  ``MEDIA_ENT_F_PROC_VIDEO_STATISTICS``
 
-       -  Video statistics computation (histogram, 3A, ...). An entity
+       -  Video statistics computation (histogram, 3A, etc.). An entity
 	  capable of statistics computation must have one sink pad and
 	  one source pad. It computes statistics over the frames
 	  received on its sink pad and outputs the statistics data on
@@ -318,8 +318,8 @@ Types and flags used to represent the media graph elements
        - Video interface bridge. A video interface bridge entity must have at
          least one sink pad and at least one source pad. It receives video
          frames on its sink pad from an input video bus of one type (HDMI, eDP,
-         MIPI CSI-2, ...), and outputs them on its source pad to an output
-         video bus of another type (eDP, MIPI CSI-2, parallel, ...).
+         MIPI CSI-2, etc.), and outputs them on its source pad to an output
+         video bus of another type (eDP, MIPI CSI-2, parallel, etc.).
 
 ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
 
@@ -337,7 +337,7 @@ Types and flags used to represent the media graph elements
        -  ``MEDIA_ENT_FL_DEFAULT``
 
        -  Default entity for its type. Used to discover the default audio,
-	  VBI and video devices, the default camera sensor, ...
+	  VBI and video devices, the default camera sensor, etc.
 
     -  .. row 2
 
@@ -345,7 +345,7 @@ Types and flags used to represent the media graph elements
 
        -  ``MEDIA_ENT_FL_CONNECTOR``
 
-       -  The entity represents a data conector
+       -  The entity represents a connector.
 
 
 ..  tabularcolumns:: |p{6.5cm}|p{6.0cm}|p{5.0cm}|
-- 
2.16.1

[PATCHv4 09/15] media: media-types.rst: fix typo

[Hans Verkuil]

From: Alexandre Courbot <acourbot@chromium.org>

with -> which

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/media/uapi/mediactl/media-types.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst
index 9c1e3d3f590c..01b0b60771dd 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -26,7 +26,7 @@ Types and flags used to represent the media graph elements
 	  ``MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN``
 
        -  Unknown entity. That generally indicates that a driver didn't
-	  initialize properly the entity, with is a Kernel bug
+	  initialize properly the entity, which is a Kernel bug
 
     -  .. row 2
 
-- 
2.16.1

[PATCHv4 10/15] media-device.c: zero reserved fields

[Hans Verkuil]

MEDIA_IOC_SETUP_LINK didn't zero the reserved field of the media_link_desc
struct. Do so in media_device_setup_link().

MEDIA_IOC_ENUM_LINKS didn't zero the reserved field of the media_links_enum
struct. Do so in media_device_enum_links().

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index e79f72b8b858..639fa703e91e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -190,6 +190,7 @@ static long media_device_enum_links(struct media_device *mdev,
 			ulink_desc++;
 		}
 	}
+	memset(links->reserved, 0, sizeof(links->reserved));
 
 	return 0;
 }
@@ -218,6 +219,8 @@ static long media_device_setup_link(struct media_device *mdev,
 	if (link == NULL)
 		return -EINVAL;
 
+	memset(linkd->reserved, 0, sizeof(linkd->reserved));
+
 	/* Setup the link on both entities. */
 	return __media_entity_setup_link(link, linkd->flags);
 }
-- 
2.16.1

[PATCHv4 11/15] media.h: fix confusing typo in comment

[Hans Verkuil]

Subdevs are initialized with MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN, not
MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/uapi/linux/media.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index b9b9446095e9..573da38a21c3 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -131,7 +131,7 @@ struct media_device_info {
  * with the legacy v1 API.The number range is out of range by purpose:
  * several previously reserved numbers got excluded from this range.
  *
- * Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
+ * Subdevs are initialized with MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN,
  * in order to preserve backward compatibility.
  * Drivers must change to the proper subdev type before
  * registering the entity.
-- 
2.16.1

[PATCHv4 12/15] media: zero reservedX fields in media_v2_topology

[Hans Verkuil]

The MEDIA_IOC_G_TOPOLOGY implementation did not zero the reservedX fields.
Fix this.

Found with v4l2-compliance.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 639fa703e91e..5b1dbb8540af 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -266,6 +266,7 @@ static long media_device_get_topology(struct media_device *mdev,
 		uentity++;
 	}
 	topo->num_entities = i;
+	topo->reserved1 = 0;
 
 	/* Get interfaces and number of interfaces */
 	i = 0;
@@ -301,6 +302,7 @@ static long media_device_get_topology(struct media_device *mdev,
 		uintf++;
 	}
 	topo->num_interfaces = i;
+	topo->reserved2 = 0;
 
 	/* Get pads and number of pads */
 	i = 0;
@@ -327,6 +329,7 @@ static long media_device_get_topology(struct media_device *mdev,
 		upad++;
 	}
 	topo->num_pads = i;
+	topo->reserved3 = 0;
 
 	/* Get links and number of links */
 	i = 0;
@@ -358,6 +361,7 @@ static long media_device_get_topology(struct media_device *mdev,
 		ulink++;
 	}
 	topo->num_links = i;
+	topo->reserved4 = 0;
 
 	return ret;
 }
-- 
2.16.1

[PATCHv4 13/15] media: document the reservedX fields in media_v2_topology

[Hans Verkuil]

The MEDIA_IOC_G_TOPOLOGY documentation didn't document the reservedX
fields. Related to that was that the documented type of the num_* fields
was also wrong.

Fix both.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/uapi/mediactl/media-ioc-g-topology.rst   | 52 +++++++++++++++++-----
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index 870a6c0d1f7a..c8f9ea37db2d 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -68,7 +68,7 @@ desired arrays with the media graph elements.
 
     -  .. row 2
 
-       -  __u64
+       -  __u32
 
        -  ``num_entities``
 
@@ -76,6 +76,14 @@ desired arrays with the media graph elements.
 
     -  .. row 3
 
+       -  __u32
+
+       -  ``reserved1``
+
+       -  Applications and drivers shall set this to 0.
+
+    -  .. row 4
+
        -  __u64
 
        -  ``ptr_entities``
@@ -85,15 +93,23 @@ desired arrays with the media graph elements.
 	  the ioctl won't store the entities. It will just update
 	  ``num_entities``
 
-    -  .. row 4
+    -  .. row 5
 
-       -  __u64
+       -  __u32
 
        -  ``num_interfaces``
 
        -  Number of interfaces in the graph
 
-    -  .. row 5
+    -  .. row 6
+
+       -  __u32
+
+       -  ``reserved2``
+
+       -  Applications and drivers shall set this to 0.
+
+    -  .. row 7
 
        -  __u64
 
@@ -104,15 +120,23 @@ desired arrays with the media graph elements.
 	  the ioctl won't store the interfaces. It will just update
 	  ``num_interfaces``
 
-    -  .. row 6
+    -  .. row 8
 
-       -  __u64
+       -  __u32
 
        -  ``num_pads``
 
        -  Total number of pads in the graph
 
-    -  .. row 7
+    -  .. row 9
+
+       -  __u32
+
+       -  ``reserved3``
+
+       -  Applications and drivers shall set this to 0.
+
+    -  .. row 10
 
        -  __u64
 
@@ -122,15 +146,23 @@ desired arrays with the media graph elements.
 	  converted to a 64-bits integer. It can be zero. if zero, the ioctl
 	  won't store the pads. It will just update ``num_pads``
 
-    -  .. row 8
+    -  .. row 11
 
-       -  __u64
+       -  __u32
 
        -  ``num_links``
 
        -  Total number of data and interface links in the graph
 
-    -  .. row 9
+    -  .. row 12
+
+       -  __u32
+
+       -  ``reserved4``
+
+       -  Applications and drivers shall set this to 0.
+
+    -  .. row 13
 
        -  __u64
 
-- 
2.16.1

[PATCHv4 14/15] media-ioc-enum-entities/links.rst: document reserved fields

[Hans Verkuil]

These structures have reserved fields that were never documented.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/uapi/mediactl/media-ioc-enum-entities.rst   | 19 +++++++++++++++----
 .../media/uapi/mediactl/media-ioc-enum-links.rst      | 18 ++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
index b59ce149efb5..45e76e5bc1ea 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
@@ -144,10 +144,21 @@ id's until they get an error.
 
     -  .. row 9
 
-       -  union
+       -  __u32
+
+       -  ``reserved[4]``
+
+       -
+       -
+       -  Reserved for future extensions. Drivers and applications must set
+          the array to zero.
 
     -  .. row 10
 
+       -  union
+
+    -  .. row 11
+
        -
        -  struct
 
@@ -156,7 +167,7 @@ id's until they get an error.
        -
        -  Valid for (sub-)devices that create a single device node.
 
-    -  .. row 11
+    -  .. row 12
 
        -
        -
@@ -166,7 +177,7 @@ id's until they get an error.
 
        -  Device node major number.
 
-    -  .. row 12
+    -  .. row 13
 
        -
        -
@@ -176,7 +187,7 @@ id's until they get an error.
 
        -  Device node minor number.
 
-    -  .. row 13
+    -  .. row 14
 
        -
        -  __u8
diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
index d05be16ffaf6..256168b3c3be 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
@@ -125,6 +125,15 @@ returned during the enumeration process.
 
        -  Pad flags, see :ref:`media-pad-flag` for more details.
 
+    -  .. row 4
+
+       -  __u32
+
+       -  ``reserved[2]``
+
+       -  Reserved for future extensions. Drivers and applications must set
+          the array to zero.
+
 
 
 .. c:type:: media_link_desc
@@ -161,6 +170,15 @@ returned during the enumeration process.
 
        -  Link flags, see :ref:`media-link-flag` for more details.
 
+    -  .. row 4
+
+       -  __u32
+
+       -  ``reserved[4]``
+
+       -  Reserved for future extensions. Drivers and applications must set
+          the array to zero.
+
 
 Return Value
 ============
-- 
2.16.1

[PATCHv4 15/15] media.h: reorganize header to make it easier to understand

[Hans Verkuil]

The media.h public header is very messy. It mixes legacy and 'new' defines
and it is not easy to figure out what should and what shouldn't be used. It
also contains confusing comment that are either out of date or completely
uninteresting for anyone that needs to use this header.

The patch groups all entity functions together, including the 'old' defines
based on the old range base. The reader just wants to know about the available
functions and doesn't care about what range is used.

All legacy defines are moved to the end of the header, so it is easier to
locate them and just ignore them.

The legacy structs in the struct media_entity_desc are put under
also a much more effective signal to the reader that they shouldn't be used
compared to the old method of relying on '#if 1' followed by a comment.

The unused MEDIA_INTF_T_ALSA_* defines are also moved to the end of the header
in the legacy area. They are also dropped from intf_type() in media-entity.c.

All defines are also aligned at the same tab making the header easier to read.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-entity.c |  16 --
 include/uapi/linux/media.h   | 345 +++++++++++++++++++++----------------------
 2 files changed, 166 insertions(+), 195 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index f7c6d64e6031..3498551e618e 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -64,22 +64,6 @@ static inline const char *intf_type(struct media_interface *intf)
 		return "v4l-swradio";
 	case MEDIA_INTF_T_V4L_TOUCH:
 		return "v4l-touch";
-	case MEDIA_INTF_T_ALSA_PCM_CAPTURE:
-		return "alsa-pcm-capture";
-	case MEDIA_INTF_T_ALSA_PCM_PLAYBACK:
-		return "alsa-pcm-playback";
-	case MEDIA_INTF_T_ALSA_CONTROL:
-		return "alsa-control";
-	case MEDIA_INTF_T_ALSA_COMPRESS:
-		return "alsa-compress";
-	case MEDIA_INTF_T_ALSA_RAWMIDI:
-		return "alsa-rawmidi";
-	case MEDIA_INTF_T_ALSA_HWDEP:
-		return "alsa-hwdep";
-	case MEDIA_INTF_T_ALSA_SEQUENCER:
-		return "alsa-sequencer";
-	case MEDIA_INTF_T_ALSA_TIMER:
-		return "alsa-timer";
 	default:
 		return "unknown-intf";
 	}
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 573da38a21c3..b5043ee59108 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -15,10 +15,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #ifndef __LINUX_MEDIA_H
@@ -42,108 +38,65 @@ struct media_device_info {
 	__u32 reserved[31];
 };
 
-#define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
-
-/*
- * Initial value to be used when a new entity is created
- * Drivers should change it to something useful
- */
-#define MEDIA_ENT_F_UNKNOWN	0x00000000
-
 /*
  * Base number ranges for entity functions
  *
- * NOTE: those ranges and entity function number are phased just to
- * make it easier to maintain this file. Userspace should not rely on
- * the ranges to identify a group of function types, as newer
- * functions can be added with any name within the full u32 range.
+ * NOTE: Userspace should not rely on these ranges to identify a group
+ * of function types, as newer functions can be added with any name within
+ * the full u32 range.
+ *
+ * Some older functions use the MEDIA_ENT_F_OLD_*_BASE range. Do not
+ * changes this, this is for backwards compatibility. When adding new
+ * functions always use MEDIA_ENT_F_BASE.
  */
-#define MEDIA_ENT_F_BASE		0x00000000
-#define MEDIA_ENT_F_OLD_BASE		0x00010000
-#define MEDIA_ENT_F_OLD_SUBDEV_BASE	0x00020000
+#define MEDIA_ENT_F_BASE			0x00000000
+#define MEDIA_ENT_F_OLD_BASE			0x00010000
+#define MEDIA_ENT_F_OLD_SUBDEV_BASE		0x00020000
 
 /*
- * DVB entities
+ * Initial value to be used when a new entity is created
+ * Drivers should change it to something useful.
  */
-#define MEDIA_ENT_F_DTV_DEMOD		(MEDIA_ENT_F_BASE + 0x00001)
-#define MEDIA_ENT_F_TS_DEMUX		(MEDIA_ENT_F_BASE + 0x00002)
-#define MEDIA_ENT_F_DTV_CA		(MEDIA_ENT_F_BASE + 0x00003)
-#define MEDIA_ENT_F_DTV_NET_DECAP	(MEDIA_ENT_F_BASE + 0x00004)
+#define MEDIA_ENT_F_UNKNOWN			MEDIA_ENT_F_BASE
 
 /*
- * I/O entities
+ * Subdevs are initialized with MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN in order
+ * to preserve backward compatibility. Drivers must change to the proper
+ * subdev function before registering the entity.
  */
-#define MEDIA_ENT_F_IO_DTV		(MEDIA_ENT_F_BASE + 0x01001)
-#define MEDIA_ENT_F_IO_VBI		(MEDIA_ENT_F_BASE + 0x01002)
-#define MEDIA_ENT_F_IO_SWRADIO		(MEDIA_ENT_F_BASE + 0x01003)
+#define MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN		MEDIA_ENT_F_OLD_SUBDEV_BASE
 
 /*
- * Analog TV IF-PLL decoders
- *
- * It is a responsibility of the master/bridge drivers to create links
- * for MEDIA_ENT_F_IF_VID_DECODER and MEDIA_ENT_F_IF_AUD_DECODER.
+ * DVB entity functions
  */
-#define MEDIA_ENT_F_IF_VID_DECODER	(MEDIA_ENT_F_BASE + 0x02001)
-#define MEDIA_ENT_F_IF_AUD_DECODER	(MEDIA_ENT_F_BASE + 0x02002)
+#define MEDIA_ENT_F_DTV_DEMOD			(MEDIA_ENT_F_BASE + 0x00001)
+#define MEDIA_ENT_F_TS_DEMUX			(MEDIA_ENT_F_BASE + 0x00002)
+#define MEDIA_ENT_F_DTV_CA			(MEDIA_ENT_F_BASE + 0x00003)
+#define MEDIA_ENT_F_DTV_NET_DECAP		(MEDIA_ENT_F_BASE + 0x00004)
 
 /*
- * Audio Entity Functions
+ * I/O entity functions
  */
-#define MEDIA_ENT_F_AUDIO_CAPTURE	(MEDIA_ENT_F_BASE + 0x03001)
-#define MEDIA_ENT_F_AUDIO_PLAYBACK	(MEDIA_ENT_F_BASE + 0x03002)
-#define MEDIA_ENT_F_AUDIO_MIXER		(MEDIA_ENT_F_BASE + 0x03003)
+#define MEDIA_ENT_F_IO_V4L  			(MEDIA_ENT_F_OLD_BASE + 1)
+#define MEDIA_ENT_F_IO_DTV			(MEDIA_ENT_F_BASE + 0x01001)
+#define MEDIA_ENT_F_IO_VBI			(MEDIA_ENT_F_BASE + 0x01002)
+#define MEDIA_ENT_F_IO_SWRADIO			(MEDIA_ENT_F_BASE + 0x01003)
 
 /*
- * Processing entities
+ * Sensor functions
  */
-#define MEDIA_ENT_F_PROC_VIDEO_COMPOSER		(MEDIA_ENT_F_BASE + 0x4001)
-#define MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER	(MEDIA_ENT_F_BASE + 0x4002)
-#define MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV	(MEDIA_ENT_F_BASE + 0x4003)
-#define MEDIA_ENT_F_PROC_VIDEO_LUT		(MEDIA_ENT_F_BASE + 0x4004)
-#define MEDIA_ENT_F_PROC_VIDEO_SCALER		(MEDIA_ENT_F_BASE + 0x4005)
-#define MEDIA_ENT_F_PROC_VIDEO_STATISTICS	(MEDIA_ENT_F_BASE + 0x4006)
+#define MEDIA_ENT_F_CAM_SENSOR			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 1)
+#define MEDIA_ENT_F_FLASH			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 2)
+#define MEDIA_ENT_F_LENS			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 3)
 
 /*
- * Switch and bridge entitites
+ * Analog video decoder functions
  */
-#define MEDIA_ENT_F_VID_MUX			(MEDIA_ENT_F_BASE + 0x5001)
-#define MEDIA_ENT_F_VID_IF_BRIDGE		(MEDIA_ENT_F_BASE + 0x5002)
+#define MEDIA_ENT_F_ATV_DECODER			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
 
 /*
- * Connectors
- */
-/* It is a responsibility of the entity drivers to add connectors and links */
-#ifdef __KERNEL__
-	/*
-	 * For now, it should not be used in userspace, as some
-	 * definitions may change
-	 */
-
-#define MEDIA_ENT_F_CONN_RF		(MEDIA_ENT_F_BASE + 0x30001)
-#define MEDIA_ENT_F_CONN_SVIDEO		(MEDIA_ENT_F_BASE + 0x30002)
-#define MEDIA_ENT_F_CONN_COMPOSITE	(MEDIA_ENT_F_BASE + 0x30003)
-
-#endif
-
-/*
- * Don't touch on those. The ranges MEDIA_ENT_F_OLD_BASE and
- * MEDIA_ENT_F_OLD_SUBDEV_BASE are kept to keep backward compatibility
- * with the legacy v1 API.The number range is out of range by purpose:
- * several previously reserved numbers got excluded from this range.
+ * Digital TV, analog TV, radio and/or software defined radio tuner functions.
  *
- * Subdevs are initialized with MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN,
- * in order to preserve backward compatibility.
- * Drivers must change to the proper subdev type before
- * registering the entity.
- */
-
-#define MEDIA_ENT_F_IO_V4L  		(MEDIA_ENT_F_OLD_BASE + 1)
-
-#define MEDIA_ENT_F_CAM_SENSOR		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 1)
-#define MEDIA_ENT_F_FLASH		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 2)
-#define MEDIA_ENT_F_LENS		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 3)
-#define MEDIA_ENT_F_ATV_DECODER		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-/*
  * It is a responsibility of the master/bridge drivers to add connectors
  * and links for MEDIA_ENT_F_TUNER. Please notice that some old tuners
  * may require the usage of separate I2C chips to decode analog TV signals,
@@ -151,49 +104,46 @@ struct media_device_info {
  * On such cases, the IF-PLL staging is mapped via one or two entities:
  * MEDIA_ENT_F_IF_VID_DECODER and/or MEDIA_ENT_F_IF_AUD_DECODER.
  */
-#define MEDIA_ENT_F_TUNER		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 5)
+#define MEDIA_ENT_F_TUNER			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 5)
 
-#define MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN	MEDIA_ENT_F_OLD_SUBDEV_BASE
+/*
+ * Analog TV IF-PLL decoder functions
+ *
+ * It is a responsibility of the master/bridge drivers to create links
+ * for MEDIA_ENT_F_IF_VID_DECODER and MEDIA_ENT_F_IF_AUD_DECODER.
+ */
+#define MEDIA_ENT_F_IF_VID_DECODER		(MEDIA_ENT_F_BASE + 0x02001)
+#define MEDIA_ENT_F_IF_AUD_DECODER		(MEDIA_ENT_F_BASE + 0x02002)
 
-#if !defined(__KERNEL__) || defined(__NEED_MEDIA_LEGACY_API)
+/*
+ * Audio entity functions
+ */
+#define MEDIA_ENT_F_AUDIO_CAPTURE		(MEDIA_ENT_F_BASE + 0x03001)
+#define MEDIA_ENT_F_AUDIO_PLAYBACK		(MEDIA_ENT_F_BASE + 0x03002)
+#define MEDIA_ENT_F_AUDIO_MIXER			(MEDIA_ENT_F_BASE + 0x03003)
 
 /*
- * Legacy symbols used to avoid userspace compilation breakages
- *
- * Those symbols map the entity function into types and should be
- * used only on legacy programs for legacy hardware. Don't rely
- * on those for MEDIA_IOC_G_TOPOLOGY.
+ * Processing entity functions
  */
-#define MEDIA_ENT_TYPE_SHIFT		16
-#define MEDIA_ENT_TYPE_MASK		0x00ff0000
-#define MEDIA_ENT_SUBTYPE_MASK		0x0000ffff
-
-/* End of the old subdev reserved numberspace */
-#define MEDIA_ENT_T_DEVNODE_UNKNOWN	(MEDIA_ENT_T_DEVNODE | \
-					 MEDIA_ENT_SUBTYPE_MASK)
-
-#define MEDIA_ENT_T_DEVNODE		MEDIA_ENT_F_OLD_BASE
-#define MEDIA_ENT_T_DEVNODE_V4L		MEDIA_ENT_F_IO_V4L
-#define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
-#define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
-
-#define MEDIA_ENT_T_UNKNOWN		MEDIA_ENT_F_UNKNOWN
-#define MEDIA_ENT_T_V4L2_VIDEO		MEDIA_ENT_F_IO_V4L
-#define MEDIA_ENT_T_V4L2_SUBDEV		MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN
-#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	MEDIA_ENT_F_CAM_SENSOR
-#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	MEDIA_ENT_F_FLASH
-#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_F_LENS
-#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	MEDIA_ENT_F_ATV_DECODER
-#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	MEDIA_ENT_F_TUNER
+#define MEDIA_ENT_F_PROC_VIDEO_COMPOSER		(MEDIA_ENT_F_BASE + 0x4001)
+#define MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER	(MEDIA_ENT_F_BASE + 0x4002)
+#define MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV	(MEDIA_ENT_F_BASE + 0x4003)
+#define MEDIA_ENT_F_PROC_VIDEO_LUT		(MEDIA_ENT_F_BASE + 0x4004)
+#define MEDIA_ENT_F_PROC_VIDEO_SCALER		(MEDIA_ENT_F_BASE + 0x4005)
+#define MEDIA_ENT_F_PROC_VIDEO_STATISTICS	(MEDIA_ENT_F_BASE + 0x4006)
 
-/* Obsolete symbol for media_version, no longer used in the kernel */
-#define MEDIA_API_VERSION		KERNEL_VERSION(0, 1, 0)
-#endif
+/*
+ * Switch and bridge entity functions
+ */
+#define MEDIA_ENT_F_VID_MUX			(MEDIA_ENT_F_BASE + 0x5001)
+#define MEDIA_ENT_F_VID_IF_BRIDGE		(MEDIA_ENT_F_BASE + 0x5002)
 
 /* Entity flags */
-#define MEDIA_ENT_FL_DEFAULT		(1 << 0)
-#define MEDIA_ENT_FL_CONNECTOR		(1 << 1)
+#define MEDIA_ENT_FL_DEFAULT			(1 << 0)
+#define MEDIA_ENT_FL_CONNECTOR			(1 << 1)
+
+/* Flags for struct media_entity_desc id field */
+#define MEDIA_ENT_ID_FLAG_NEXT			(1 << 31)
 
 struct media_entity_desc {
 	__u32 id;
@@ -214,7 +164,7 @@ struct media_entity_desc {
 			__u32 minor;
 		} dev;
 
-#if 1
+#if !defined(__KERNEL__)
 		/*
 		 * TODO: this shouldn't have been added without
 		 * actual drivers that use this. When the first real driver
@@ -225,24 +175,17 @@ struct media_entity_desc {
 		 * contain the subdevice information. In addition, struct dev
 		 * can only refer to a single device, and not to multiple (e.g.
 		 * pcm and mixer devices).
-		 *
-		 * So for now mark this as a to do.
 		 */
 		struct {
 			__u32 card;
 			__u32 device;
 			__u32 subdevice;
 		} alsa;
-#endif
 
-#if 1
 		/*
 		 * DEPRECATED: previous node specifications. Kept just to
-		 * avoid breaking compilation, but media_entity_desc.dev
-		 * should be used instead. In particular, alsa and dvb
-		 * fields below are wrong: for all devnodes, there should
-		 * be just major/minor inside the struct, as this is enough
-		 * to represent any devnode, no matter what type.
+		 * avoid breaking compilation. Use media_entity_desc.dev
+		 * instead.
 		 */
 		struct {
 			__u32 major;
@@ -261,9 +204,9 @@ struct media_entity_desc {
 	};
 };
 
-#define MEDIA_PAD_FL_SINK		(1 << 0)
-#define MEDIA_PAD_FL_SOURCE		(1 << 1)
-#define MEDIA_PAD_FL_MUST_CONNECT	(1 << 2)
+#define MEDIA_PAD_FL_SINK			(1 << 0)
+#define MEDIA_PAD_FL_SOURCE			(1 << 1)
+#define MEDIA_PAD_FL_MUST_CONNECT		(1 << 2)
 
 struct media_pad_desc {
 	__u32 entity;		/* entity ID */
@@ -272,13 +215,13 @@ struct media_pad_desc {
 	__u32 reserved[2];
 };
 
-#define MEDIA_LNK_FL_ENABLED		(1 << 0)
-#define MEDIA_LNK_FL_IMMUTABLE		(1 << 1)
-#define MEDIA_LNK_FL_DYNAMIC		(1 << 2)
+#define MEDIA_LNK_FL_ENABLED			(1 << 0)
+#define MEDIA_LNK_FL_IMMUTABLE			(1 << 1)
+#define MEDIA_LNK_FL_DYNAMIC			(1 << 2)
 
-#define MEDIA_LNK_FL_LINK_TYPE		(0xf << 28)
-#  define MEDIA_LNK_FL_DATA_LINK	(0 << 28)
-#  define MEDIA_LNK_FL_INTERFACE_LINK	(1 << 28)
+#define MEDIA_LNK_FL_LINK_TYPE			(0xf << 28)
+#  define MEDIA_LNK_FL_DATA_LINK		(0 << 28)
+#  define MEDIA_LNK_FL_INTERFACE_LINK		(1 << 28)
 
 struct media_link_desc {
 	struct media_pad_desc source;
@@ -298,57 +241,47 @@ struct media_links_enum {
 
 /* Interface type ranges */
 
-#define MEDIA_INTF_T_DVB_BASE	0x00000100
-#define MEDIA_INTF_T_V4L_BASE	0x00000200
-#define MEDIA_INTF_T_ALSA_BASE	0x00000300
+#define MEDIA_INTF_T_DVB_BASE			0x00000100
+#define MEDIA_INTF_T_V4L_BASE			0x00000200
 
 /* Interface types */
 
-#define MEDIA_INTF_T_DVB_FE    	(MEDIA_INTF_T_DVB_BASE)
-#define MEDIA_INTF_T_DVB_DEMUX  (MEDIA_INTF_T_DVB_BASE + 1)
-#define MEDIA_INTF_T_DVB_DVR    (MEDIA_INTF_T_DVB_BASE + 2)
-#define MEDIA_INTF_T_DVB_CA     (MEDIA_INTF_T_DVB_BASE + 3)
-#define MEDIA_INTF_T_DVB_NET    (MEDIA_INTF_T_DVB_BASE + 4)
-
-#define MEDIA_INTF_T_V4L_VIDEO  (MEDIA_INTF_T_V4L_BASE)
-#define MEDIA_INTF_T_V4L_VBI    (MEDIA_INTF_T_V4L_BASE + 1)
-#define MEDIA_INTF_T_V4L_RADIO  (MEDIA_INTF_T_V4L_BASE + 2)
-#define MEDIA_INTF_T_V4L_SUBDEV (MEDIA_INTF_T_V4L_BASE + 3)
-#define MEDIA_INTF_T_V4L_SWRADIO (MEDIA_INTF_T_V4L_BASE + 4)
-#define MEDIA_INTF_T_V4L_TOUCH	(MEDIA_INTF_T_V4L_BASE + 5)
-
-#define MEDIA_INTF_T_ALSA_PCM_CAPTURE   (MEDIA_INTF_T_ALSA_BASE)
-#define MEDIA_INTF_T_ALSA_PCM_PLAYBACK  (MEDIA_INTF_T_ALSA_BASE + 1)
-#define MEDIA_INTF_T_ALSA_CONTROL       (MEDIA_INTF_T_ALSA_BASE + 2)
-#define MEDIA_INTF_T_ALSA_COMPRESS      (MEDIA_INTF_T_ALSA_BASE + 3)
-#define MEDIA_INTF_T_ALSA_RAWMIDI       (MEDIA_INTF_T_ALSA_BASE + 4)
-#define MEDIA_INTF_T_ALSA_HWDEP         (MEDIA_INTF_T_ALSA_BASE + 5)
-#define MEDIA_INTF_T_ALSA_SEQUENCER     (MEDIA_INTF_T_ALSA_BASE + 6)
-#define MEDIA_INTF_T_ALSA_TIMER         (MEDIA_INTF_T_ALSA_BASE + 7)
+#define MEDIA_INTF_T_DVB_FE    			(MEDIA_INTF_T_DVB_BASE)
+#define MEDIA_INTF_T_DVB_DEMUX  		(MEDIA_INTF_T_DVB_BASE + 1)
+#define MEDIA_INTF_T_DVB_DVR    		(MEDIA_INTF_T_DVB_BASE + 2)
+#define MEDIA_INTF_T_DVB_CA     		(MEDIA_INTF_T_DVB_BASE + 3)
+#define MEDIA_INTF_T_DVB_NET    		(MEDIA_INTF_T_DVB_BASE + 4)
+
+#define MEDIA_INTF_T_V4L_VIDEO  		(MEDIA_INTF_T_V4L_BASE)
+#define MEDIA_INTF_T_V4L_VBI    		(MEDIA_INTF_T_V4L_BASE + 1)
+#define MEDIA_INTF_T_V4L_RADIO  		(MEDIA_INTF_T_V4L_BASE + 2)
+#define MEDIA_INTF_T_V4L_SUBDEV 		(MEDIA_INTF_T_V4L_BASE + 3)
+#define MEDIA_INTF_T_V4L_SWRADIO 		(MEDIA_INTF_T_V4L_BASE + 4)
+#define MEDIA_INTF_T_V4L_TOUCH			(MEDIA_INTF_T_V4L_BASE + 5)
+
+#if defined(__KERNEL__)
 
 /*
- * MC next gen API definitions
+ * Connector functions
  *
- * NOTE: The declarations below are close to the MC RFC for the Media
- *	 Controller, the next generation. Yet, there are a few adjustments
- *	 to do, as we want to be able to have a functional API before
- *	 the MC properties change. Those will be properly marked below.
- *	 Please also notice that I removed "num_pads", "num_links",
- *	 from the proposal, as a proper userspace application will likely
- *	 use lists for pads/links, just as we intend to do in Kernelspace.
- *	 The API definition should be freed from fields that are bound to
- *	 some specific data structure.
+ * For now these should not be used in userspace, as some definitions may
+ * change.
  *
- * FIXME: Currently, I opted to name the new types as "media_v2", as this
- *	  won't cause any conflict with the Kernelspace namespace, nor with
- *	  the previous kAPI media_*_desc namespace. This can be changed
- *	  later, before the adding this API upstream.
+ * It is the responsibility of the entity drivers to add connectors and links.
  */
+#define MEDIA_ENT_F_CONN_RF			(MEDIA_ENT_F_BASE + 0x30001)
+#define MEDIA_ENT_F_CONN_SVIDEO			(MEDIA_ENT_F_BASE + 0x30002)
+#define MEDIA_ENT_F_CONN_COMPOSITE		(MEDIA_ENT_F_BASE + 0x30003)
 
+#endif
+
+/*
+ * MC next gen API definitions
+ */
 
 struct media_v2_entity {
 	__u32 id;
-	char name[64];		/* FIXME: move to a property? (RFC says so) */
+	char name[64];
 	__u32 function;		/* Main function of the entity */
 	__u32 reserved[6];
 } __attribute__ ((packed));
@@ -406,12 +339,66 @@ struct media_v2_topology {
 	__u64 ptr_links;
 } __attribute__ ((packed));
 
+
 /* ioctls */
 
-#define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
-#define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
-#define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)
-#define MEDIA_IOC_SETUP_LINK		_IOWR('|', 0x03, struct media_link_desc)
-#define MEDIA_IOC_G_TOPOLOGY		_IOWR('|', 0x04, struct media_v2_topology)
+#define MEDIA_IOC_DEVICE_INFO	_IOWR('|', 0x00, struct media_device_info)
+#define MEDIA_IOC_ENUM_ENTITIES	_IOWR('|', 0x01, struct media_entity_desc)
+#define MEDIA_IOC_ENUM_LINKS	_IOWR('|', 0x02, struct media_links_enum)
+#define MEDIA_IOC_SETUP_LINK	_IOWR('|', 0x03, struct media_link_desc)
+#define MEDIA_IOC_G_TOPOLOGY	_IOWR('|', 0x04, struct media_v2_topology)
+
+
+#if !defined(__KERNEL__) || defined(__NEED_MEDIA_LEGACY_API)
+
+/*
+ * Legacy symbols used to avoid userspace compilation breakages.
+ * Do not use any of this in new applications!
+ *
+ * Those symbols map the entity function into types and should be
+ * used only on legacy programs for legacy hardware. Don't rely
+ * on those for MEDIA_IOC_G_TOPOLOGY.
+ */
+#define MEDIA_ENT_TYPE_SHIFT			16
+#define MEDIA_ENT_TYPE_MASK			0x00ff0000
+#define MEDIA_ENT_SUBTYPE_MASK			0x0000ffff
+
+#define MEDIA_ENT_T_DEVNODE_UNKNOWN		(MEDIA_ENT_F_OLD_BASE | \
+						 MEDIA_ENT_SUBTYPE_MASK)
+
+#define MEDIA_ENT_T_DEVNODE			MEDIA_ENT_F_OLD_BASE
+#define MEDIA_ENT_T_DEVNODE_V4L			MEDIA_ENT_F_IO_V4L
+#define MEDIA_ENT_T_DEVNODE_FB			(MEDIA_ENT_F_OLD_BASE + 2)
+#define MEDIA_ENT_T_DEVNODE_ALSA		(MEDIA_ENT_F_OLD_BASE + 3)
+#define MEDIA_ENT_T_DEVNODE_DVB			(MEDIA_ENT_F_OLD_BASE + 4)
+
+#define MEDIA_ENT_T_UNKNOWN			MEDIA_ENT_F_UNKNOWN
+#define MEDIA_ENT_T_V4L2_VIDEO			MEDIA_ENT_F_IO_V4L
+#define MEDIA_ENT_T_V4L2_SUBDEV			MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN
+#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR		MEDIA_ENT_F_CAM_SENSOR
+#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH		MEDIA_ENT_F_FLASH
+#define MEDIA_ENT_T_V4L2_SUBDEV_LENS		MEDIA_ENT_F_LENS
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER		MEDIA_ENT_F_ATV_DECODER
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER		MEDIA_ENT_F_TUNER
+
+/*
+ * There is still no ALSA support in the media controller. These
+ * defines should not have been added and we leave them here only
+ * in case some application tries to use these defines.
+ */
+#define MEDIA_INTF_T_ALSA_BASE			0x00000300
+#define MEDIA_INTF_T_ALSA_PCM_CAPTURE   	(MEDIA_INTF_T_ALSA_BASE)
+#define MEDIA_INTF_T_ALSA_PCM_PLAYBACK  	(MEDIA_INTF_T_ALSA_BASE + 1)
+#define MEDIA_INTF_T_ALSA_CONTROL       	(MEDIA_INTF_T_ALSA_BASE + 2)
+#define MEDIA_INTF_T_ALSA_COMPRESS      	(MEDIA_INTF_T_ALSA_BASE + 3)
+#define MEDIA_INTF_T_ALSA_RAWMIDI       	(MEDIA_INTF_T_ALSA_BASE + 4)
+#define MEDIA_INTF_T_ALSA_HWDEP         	(MEDIA_INTF_T_ALSA_BASE + 5)
+#define MEDIA_INTF_T_ALSA_SEQUENCER     	(MEDIA_INTF_T_ALSA_BASE + 6)
+#define MEDIA_INTF_T_ALSA_TIMER         	(MEDIA_INTF_T_ALSA_BASE + 7)
+
+/* Obsolete symbol for media_version, no longer used in the kernel */
+#define MEDIA_API_VERSION			KERNEL_VERSION(0, 1, 0)
+
+#endif
 
 #endif /* __LINUX_MEDIA_H */
-- 
2.16.1

Re: [PATCHv4 06/15] subdev-formats.rst: fix incorrect types

[Mauro Carvalho Chehab]

Hi Hans,

Em Wed, 21 Feb 2018 16:32:09 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> The ycbcr_enc, quantization and xfer_func fields are __u16 and not enums.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Thanks for your patch. I have one comment about it, though. See below.

> ---
>  Documentation/media/uapi/v4l/subdev-formats.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst b/Documentation/media/uapi/v4l/subdev-formats.rst
> index b1eea44550e1..4f0c0b282f98 100644
> --- a/Documentation/media/uapi/v4l/subdev-formats.rst
> +++ b/Documentation/media/uapi/v4l/subdev-formats.rst
> @@ -33,17 +33,17 @@ Media Bus Formats
>        - Image colorspace, from enum
>  	:c:type:`v4l2_colorspace`. See
>  	:ref:`colorspaces` for details.
> -    * - enum :c:type:`v4l2_ycbcr_encoding`
> +    * - __u16
>        - ``ycbcr_enc``
>        - This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
>  	streams, see :ref:`colorspaces`.

While this patch makes sense, it excludes an important information:
what are the valid values for this field.

I was expecting something like what's written for the code field:

     * - __u32
       - ``code``
       - Format code, from enum
        :ref:`v4l2_mbus_pixelcode <v4l2-mbus-pixelcode>`.

Something like:

    * - enum :c:type:`v4l2_ycbcr_encoding`
     * - __u16
       - This information supplements the ``colorspace`` and must be set by
 	the driver for capture streams and by the application for output
 	streams from enum :ref:`v4l2_mbus_pixelcode <v4l2-mbus-pixelcode>`.
	See :ref:`colorspaces`.

The same applies to the other changes below.

As this patch is independent from the others at your pull request,
I'm skipping it.

Regards,
Mauro

> -    * - enum :c:type:`v4l2_quantization`
> +    * - __u16
>        - ``quantization``
>        - This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output
>  	streams, see :ref:`colorspaces`.
> -    * - enum :c:type:`v4l2_xfer_func`
> +    * - __u16
>        - ``xfer_func``
>        - This information supplements the ``colorspace`` and must be set by
>  	the driver for capture streams and by the application for output



Thanks,
Mauro