[PATCHv6 00/12] media/mc: fix inconsistencies

[PATCHv6 00/12] media/mc: fix inconsistencies

[Hans Verkuil]

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

This series is v6 of my previous attempt:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg133178.html

The goal is to fix the inconsistencies between the 'old' and 'new' 
MC API. I hate the terms 'old' and 'new', there is nothing wrong IMHO with 
using an 'old' API if it meets the needs of the application.

Making G_TOPOLOGY useful is urgently needed since I think that will be
very helpful for the work we want to do on library support for complex
cameras.

Changes since v5:

- Added Reviewed-by tags from Laurent.
- Improved the commit log of patch 05/12 mentioning that it also modifies
  drivers.
- Rephrased the pad index explanation in patch 11/12: new pad indices do
  not necessarily have to be added to the end. The key is that existing pad
  indices should never be renumbered. Since there can be holes in the pad
  indices it is always possible that a new pad uses one of those unused
  indices.

Changes since v4:

- Improve documentation of the new index field in patch 2.
- Added patch 11 to sync the index field documentation in media-ioc-enum-links.rst
  with the index field documentation from media-ioc-g-topology.rst.
- Added patch 12 that clarifies that you should not hardcode ID values
  in applications since they can change between instances of the device.
  Also document that the entity name is unique within the media topology.

Changes since v3:

- Renamed MEDIA_ENT_F_DTV_ENCODER to MEDIA_ENT_F_DV_ENCODER
- Added a new patch renaming MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER.
- Added a new patch that reorders the function defines in media.h so that they
  are in increasing numerical order (the en/decoder functions where not in
  order).
- Added Sakari's Acks (except for the two new patches).

Regards,

        Hans


Hans Verkuil (12):
  media: add 'index' to struct media_v2_pad
  media-ioc-g-topology.rst: document new 'index' field
  media: add flags field to struct media_v2_entity
  media-ioc-g-topology.rst: document new 'flags' field
  media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER
  media.h: add MEDIA_ENT_F_DV_ENCODER
  media.h: reorder video en/decoder functions
  ad9389b/adv7511: set proper media entity function
  adv7180/tvp514x/tvp7002: fix entity function
  media/i2c: add missing entity functions
  media-ioc-enum-links.rst: improve pad index description
  media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

 .../uapi/mediactl/media-ioc-enum-entities.rst |  9 ++--
 .../uapi/mediactl/media-ioc-enum-links.rst    |  4 +-
 .../uapi/mediactl/media-ioc-g-topology.rst    | 42 +++++++++++++++----
 .../media/uapi/mediactl/media-types.rst       |  9 +++-
 drivers/media/i2c/ad9389b.c                   |  1 +
 drivers/media/i2c/adv7180.c                   |  2 +-
 drivers/media/i2c/adv7511.c                   |  1 +
 drivers/media/i2c/adv7604.c                   |  1 +
 drivers/media/i2c/adv7842.c                   |  1 +
 drivers/media/i2c/et8ek8/et8ek8_driver.c      |  1 +
 drivers/media/i2c/mt9m032.c                   |  1 +
 drivers/media/i2c/mt9p031.c                   |  1 +
 drivers/media/i2c/mt9t001.c                   |  1 +
 drivers/media/i2c/mt9v032.c                   |  1 +
 drivers/media/i2c/tda1997x.c                  |  2 +-
 drivers/media/i2c/tvp514x.c                   |  2 +-
 drivers/media/i2c/tvp7002.c                   |  2 +-
 drivers/media/media-device.c                  |  2 +
 include/uapi/linux/media.h                    | 39 +++++++++++++----
 19 files changed, 97 insertions(+), 25 deletions(-)

-- 
2.18.0

[PATCHv6 01/12] media: add 'index' to struct media_v2_pad

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

The v2 pad structure never exposed the pad index, which made it impossible
to call the MEDIA_IOC_SETUP_LINK ioctl, which needs that information.

It is really trivial to just expose this information, so implement this.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c |  1 +
 include/uapi/linux/media.h   | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 47bb2254fbfd..047d38372a27 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -331,6 +331,7 @@ static long media_device_get_topology(struct media_device *mdev, void *arg)
 		kpad.id = pad->graph_obj.id;
 		kpad.entity_id = pad->entity->graph_obj.id;
 		kpad.flags = pad->flags;
+		kpad.index = pad->index;
 
 		if (copy_to_user(upad, &kpad, sizeof(kpad)))
 			ret = -EFAULT;
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 86c7dcc9cba3..f6338bd57929 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -305,11 +305,21 @@ struct media_v2_interface {
 	};
 } __attribute__ ((packed));
 
+/*
+ * Appeared in 4.19.0.
+ *
+ * The media_version argument comes from the media_version field in
+ * struct media_device_info.
+ */
+#define MEDIA_V2_PAD_HAS_INDEX(media_version) \
+	((media_version) >= ((4 << 16) | (19 << 8) | 0))
+
 struct media_v2_pad {
 	__u32 id;
 	__u32 entity_id;
 	__u32 flags;
-	__u32 reserved[5];
+	__u32 index;
+	__u32 reserved[4];
 } __attribute__ ((packed));
 
 struct media_v2_link {
-- 
2.18.0

[PATCHv6 02/12] media-ioc-g-topology.rst: document new 'index' field

[Hans Verkuil]

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

Document the new struct media_v2_pad 'index' field.

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     | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index a3f259f83b25..bae2b4db89cc 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -176,7 +176,7 @@ desired arrays with the media graph elements.
     *  -  struct media_v2_intf_devnode
        -  ``devnode``
        -  Used only for device node interfaces. See
-	  :c:type:`media_v2_intf_devnode` for details..
+	  :c:type:`media_v2_intf_devnode` for details.
 
 
 .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
@@ -218,7 +218,15 @@ desired arrays with the media graph elements.
        -  Pad flags, see :ref:`media-pad-flag` for more details.
 
     *  -  __u32
-       -  ``reserved``\ [5]
+       -  ``index``
+       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
+	  returns true. The ``media_version`` is defined in struct
+	  :c:type:`media_device_info` and can be retrieved using
+	  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are added
+	  for an entity in the future, then those will be added at the end.
+
+    *  -  __u32
+       -  ``reserved``\ [4]
        -  Reserved for future extensions. Drivers and applications must set
 	  this array to zero.
 
-- 
2.18.0

[PATCHv6 03/12] media: add flags field to struct media_v2_entity

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

The v2 entity structure never exposed the entity flags, which made it
impossible to detect connector or default entities.

It is really trivial to just expose this information, so implement this.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c |  1 +
 include/uapi/linux/media.h   | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 047d38372a27..14959b19a342 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, void *arg)
 		memset(&kentity, 0, sizeof(kentity));
 		kentity.id = entity->graph_obj.id;
 		kentity.function = entity->function;
+		kentity.flags = entity->flags;
 		strlcpy(kentity.name, entity->name,
 			sizeof(kentity.name));
 
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index f6338bd57929..ebd2cda67833 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -280,11 +280,21 @@ struct media_links_enum {
  * MC next gen API definitions
  */
 
+/*
+ * Appeared in 4.19.0.
+ *
+ * The media_version argument comes from the media_version field in
+ * struct media_device_info.
+ */
+#define MEDIA_V2_ENTITY_HAS_FLAGS(media_version) \
+	((media_version) >= ((4 << 16) | (19 << 8) | 0))
+
 struct media_v2_entity {
 	__u32 id;
 	char name[64];
 	__u32 function;		/* Main function of the entity */
-	__u32 reserved[6];
+	__u32 flags;
+	__u32 reserved[5];
 } __attribute__ ((packed));
 
 /* Should match the specific fields at media_intf_devnode */
-- 
2.18.0

[PATCHv6 04/12] media-ioc-g-topology.rst: document new 'flags' field

[Hans Verkuil]

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

Document the new struct media_v2_entity 'flags' field.

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       | 10 +++++++++-
 1 file changed, 9 insertions(+), 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 bae2b4db89cc..e572dd0d806d 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -142,7 +142,15 @@ desired arrays with the media graph elements.
        -  Entity main function, see :ref:`media-entity-functions` for details.
 
     *  -  __u32
-       -  ``reserved``\ [6]
+       -  ``flags``
+       -  Entity flags, see :ref:`media-entity-flag` for details.
+	  Only valid if ``MEDIA_V2_ENTITY_HAS_FLAGS(media_version)``
+	  returns true. The ``media_version`` is defined in struct
+	  :c:type:`media_device_info` and can be retrieved using
+	  :ref:`MEDIA_IOC_DEVICE_INFO`.
+
+    *  -  __u32
+       -  ``reserved``\ [5]
        -  Reserved for future extensions. Drivers and applications must set
 	  this array to zero.
 
-- 
2.18.0

[PATCHv6 05/12] media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

The use of 'DTV' is very confusing since it normally refers to Digital
TV e.g. DVB etc.

Instead use 'DV' (Digital Video), which nicely corresponds to the
DV Timings API used to configure such receivers and transmitters.

We keep an alias to avoid breaking userspace applications.

Since this alias is only available if __KERNEL__ is *not* defined
(i.e. it is only available for userspace, not kernelspace), any
drivers that use it also have to be converted to the new define.
These drivers are adv7604, adv7842 and tda1997x.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 Documentation/media/uapi/mediactl/media-types.rst | 2 +-
 drivers/media/i2c/adv7604.c                       | 1 +
 drivers/media/i2c/adv7842.c                       | 1 +
 drivers/media/i2c/tda1997x.c                      | 2 +-
 include/uapi/linux/media.h                        | 4 +++-
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst
index 96910cf2eaaa..c11b0c7e890b 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -200,7 +200,7 @@ Types and flags used to represent the media graph elements
          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.).
 
-    *  -  ``MEDIA_ENT_F_DTV_DECODER``
+    *  -  ``MEDIA_ENT_F_DV_DECODER``
        -  Digital video decoder. The basic function of the video decoder is
 	  to accept digital video from a wide variety of sources
 	  and output it in some digital video standard, with appropriate
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1a3b2c04d9f9..668be2bca57a 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3499,6 +3499,7 @@ static int adv76xx_probe(struct i2c_client *client,
 	for (i = 0; i < state->source_pad; ++i)
 		state->pads[i].flags = MEDIA_PAD_FL_SINK;
 	state->pads[state->source_pad].flags = MEDIA_PAD_FL_SOURCE;
+	sd->entity.function = MEDIA_ENT_F_DV_DECODER;
 
 	err = media_entity_pads_init(&sd->entity, state->source_pad + 1,
 				state->pads);
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index fddac32e5051..99d781343fb1 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -3541,6 +3541,7 @@ static int adv7842_probe(struct i2c_client *client,
 	INIT_DELAYED_WORK(&state->delayed_work_enable_hotplug,
 			adv7842_delayed_work_enable_hotplug);
 
+	sd->entity.function = MEDIA_ENT_F_DV_DECODER;
 	state->pad.flags = MEDIA_PAD_FL_SOURCE;
 	err = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (err)
diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c
index 039a92c3294a..d114ac5243ec 100644
--- a/drivers/media/i2c/tda1997x.c
+++ b/drivers/media/i2c/tda1997x.c
@@ -2570,7 +2570,7 @@ static int tda1997x_probe(struct i2c_client *client,
 		 id->name, i2c_adapter_id(client->adapter),
 		 client->addr);
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
-	sd->entity.function = MEDIA_ENT_F_DTV_DECODER;
+	sd->entity.function = MEDIA_ENT_F_DV_DECODER;
 	sd->entity.ops = &tda1997x_media_ops;
 
 	/* set allowed mbus modes based on chip, bus-type, and bus-width */
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index ebd2cda67833..99f5e0978ebb 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -93,7 +93,7 @@ struct media_device_info {
  * Video decoder functions
  */
 #define MEDIA_ENT_F_ATV_DECODER			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-#define MEDIA_ENT_F_DTV_DECODER			(MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_DECODER			(MEDIA_ENT_F_BASE + 0x6001)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
@@ -400,6 +400,8 @@ struct media_v2_topology {
 #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_DTV_DECODER			MEDIA_ENT_F_DV_DECODER
+
 /*
  * There is still no ALSA support in the media controller. These
  * defines should not have been added and we leave them here only
-- 
2.18.0

[PATCHv6 06/12] media.h: add MEDIA_ENT_F_DV_ENCODER

[Hans Verkuil]

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

Add a new function for digital video encoders such as HDMI transmitters.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/media/uapi/mediactl/media-types.rst | 7 +++++++
 include/uapi/linux/media.h                        | 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst
index c11b0c7e890b..e90d4d0a7f8b 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -206,6 +206,13 @@ Types and flags used to represent the media graph elements
 	  and output it in some digital video standard, with appropriate
 	  timing signals.
 
+    *  -  ``MEDIA_ENT_F_DV_ENCODER``
+       -  Digital video encoder. The basic function of the video encoder is
+	  to accept digital video from some digital video standard with
+	  appropriate timing signals (usually a parallel video bus with sync
+	  signals) and output this to a digital video output connector such
+	  as HDMI or DisplayPort.
+
 ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
 
 .. _media-entity-flag:
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 99f5e0978ebb..6f594fa238c2 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -90,10 +90,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_LENS			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 3)
 
 /*
- * Video decoder functions
+ * Video decoder/encoder functions
  */
 #define MEDIA_ENT_F_ATV_DECODER			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
 #define MEDIA_ENT_F_DV_DECODER			(MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_ENCODER			(MEDIA_ENT_F_BASE + 0x6002)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
-- 
2.18.0

[PATCHv6 07/12] media.h: reorder video en/decoder functions

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Keep the function defines in numerical order: 0x6000 comes after
0x2000, so move it back.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/uapi/linux/media.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 6f594fa238c2..76d9bd64c116 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -89,13 +89,6 @@ struct media_device_info {
 #define MEDIA_ENT_F_FLASH			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 2)
 #define MEDIA_ENT_F_LENS			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 3)
 
-/*
- * Video decoder/encoder functions
- */
-#define MEDIA_ENT_F_ATV_DECODER			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-#define MEDIA_ENT_F_DV_DECODER			(MEDIA_ENT_F_BASE + 0x6001)
-#define MEDIA_ENT_F_DV_ENCODER			(MEDIA_ENT_F_BASE + 0x6002)
-
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
  *
@@ -140,6 +133,13 @@ struct media_device_info {
 #define MEDIA_ENT_F_VID_MUX			(MEDIA_ENT_F_BASE + 0x5001)
 #define MEDIA_ENT_F_VID_IF_BRIDGE		(MEDIA_ENT_F_BASE + 0x5002)
 
+/*
+ * Video decoder/encoder functions
+ */
+#define MEDIA_ENT_F_ATV_DECODER			(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
+#define MEDIA_ENT_F_DV_DECODER			(MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_ENCODER			(MEDIA_ENT_F_BASE + 0x6002)
+
 /* Entity flags */
 #define MEDIA_ENT_FL_DEFAULT			(1 << 0)
 #define MEDIA_ENT_FL_CONNECTOR			(1 << 1)
-- 
2.18.0

[PATCHv6 08/12] ad9389b/adv7511: set proper media entity function

[Hans Verkuil]

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

These two drivers both have function MEDIA_ENT_F_DV_ENCODER.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ad9389b.c | 1 +
 drivers/media/i2c/adv7511.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 91ff06088572..5b008b0002c0 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1134,6 +1134,7 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id *
 		goto err_hdl;
 	}
 	state->pad.flags = MEDIA_PAD_FL_SINK;
+	sd->entity.function = MEDIA_ENT_F_DV_ENCODER;
 	err = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (err)
 		goto err_hdl;
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index 5731751d3f2a..55c2ea0720d9 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -1847,6 +1847,7 @@ static int adv7511_probe(struct i2c_client *client, const struct i2c_device_id *
 		goto err_hdl;
 	}
 	state->pad.flags = MEDIA_PAD_FL_SINK;
+	sd->entity.function = MEDIA_ENT_F_DV_ENCODER;
 	err = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (err)
 		goto err_hdl;
-- 
2.18.0

[PATCHv6 09/12] adv7180/tvp514x/tvp7002: fix entity function

[Hans Verkuil]

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

The entity function was ORed with the flags field instead of
assigned to the function field. Correct this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/adv7180.c | 2 +-
 drivers/media/i2c/tvp514x.c | 2 +-
 drivers/media/i2c/tvp7002.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..a727d7f806a1 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1335,7 +1335,7 @@ static int adv7180_probe(struct i2c_client *client,
 		goto err_unregister_vpp_client;
 
 	state->pad.flags = MEDIA_PAD_FL_SOURCE;
-	sd->entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+	sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
 	ret = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (ret)
 		goto err_free_ctrl;
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 6a9890531d01..675b9ae212ab 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -1084,7 +1084,7 @@ tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id)
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
 	decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-	decoder->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+	decoder->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
 
 	ret = media_entity_pads_init(&decoder->sd.entity, 1, &decoder->pad);
 	if (ret < 0) {
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 4599b7e28a8d..4f5c627579c7 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -1010,7 +1010,7 @@ static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id)
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	device->pad.flags = MEDIA_PAD_FL_SOURCE;
 	device->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-	device->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+	device->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
 
 	error = media_entity_pads_init(&device->sd.entity, 1, &device->pad);
 	if (error < 0)
-- 
2.18.0

[PATCHv6 10/12] media/i2c: add missing entity functions

[Hans Verkuil]

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

Several drivers in media/i2c do not set the entity function.
Correct this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 1 +
 drivers/media/i2c/mt9m032.c              | 1 +
 drivers/media/i2c/mt9p031.c              | 1 +
 drivers/media/i2c/mt9t001.c              | 1 +
 drivers/media/i2c/mt9v032.c              | 1 +
 5 files changed, 5 insertions(+)

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index e9eff9039ef5..37ef38947e01 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -1446,6 +1446,7 @@ static int et8ek8_probe(struct i2c_client *client,
 	sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	sensor->subdev.internal_ops = &et8ek8_internal_ops;
 
+	sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&sensor->subdev.entity, 1, &sensor->pad);
 	if (ret < 0) {
diff --git a/drivers/media/i2c/mt9m032.c b/drivers/media/i2c/mt9m032.c
index 6a9e068462fd..b385f2b632ad 100644
--- a/drivers/media/i2c/mt9m032.c
+++ b/drivers/media/i2c/mt9m032.c
@@ -793,6 +793,7 @@ static int mt9m032_probe(struct i2c_client *client,
 	v4l2_ctrl_cluster(2, &sensor->hflip);
 
 	sensor->subdev.ctrl_handler = &sensor->ctrls;
+	sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&sensor->subdev.entity, 1, &sensor->pad);
 	if (ret < 0)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 91d822fc4443..715be3632b01 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -1111,6 +1111,7 @@ static int mt9p031_probe(struct i2c_client *client,
 	v4l2_i2c_subdev_init(&mt9p031->subdev, client, &mt9p031_subdev_ops);
 	mt9p031->subdev.internal_ops = &mt9p031_subdev_internal_ops;
 
+	mt9p031->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	mt9p031->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&mt9p031->subdev.entity, 1, &mt9p031->pad);
 	if (ret < 0)
diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 9d981d9f5686..f683d2cb0486 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -943,6 +943,7 @@ static int mt9t001_probe(struct i2c_client *client,
 	mt9t001->subdev.internal_ops = &mt9t001_subdev_internal_ops;
 	mt9t001->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+	mt9t001->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	mt9t001->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&mt9t001->subdev.entity, 1, &mt9t001->pad);
 
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index 4de63b2df334..f74730d24d8f 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -1162,6 +1162,7 @@ static int mt9v032_probe(struct i2c_client *client,
 	mt9v032->subdev.internal_ops = &mt9v032_subdev_internal_ops;
 	mt9v032->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+	mt9v032->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	mt9v032->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&mt9v032->subdev.entity, 1, &mt9v032->pad);
 	if (ret < 0)
-- 
2.18.0

[PATCHv6 11/12] media-ioc-enum-links.rst: improve pad index description

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Make it clearer that the index starts at 0, and that it won't change
since future new pads will never renumber existing pad indices.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 Documentation/media/uapi/mediactl/media-ioc-enum-links.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
index 17abdeed1a9c..f5cd75b42b69 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
@@ -92,7 +92,9 @@ returned during the enumeration process.
 
     *  -  __u16
        -  ``index``
-       -  0-based pad index.
+       -  Pad index, starts at 0. Pad indices are stable. So if new pads are added
+	  for an entity in the future, then the indices of the existing pads will
+	  never be renumbered, they will remain the same.
 
     *  -  __u32
        -  ``flags``
-- 
2.18.0

[PATCHv6 12/12] media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Mention that IDs should not be hardcoded in applications and that the
entity name must be unique within the media topology.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../uapi/mediactl/media-ioc-enum-entities.rst |  9 +++++---
 .../uapi/mediactl/media-ioc-g-topology.rst    | 22 ++++++++++++++-----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
index 961466ae821d..fc2e39c070c9 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
@@ -62,15 +62,18 @@ id's until they get an error.
        -  ``id``
        -
        -
-       -  Entity id, set by the application. When the id is or'ed with
+       -  Entity ID, set by the application. When the ID is or'ed with
 	  ``MEDIA_ENT_ID_FLAG_NEXT``, the driver clears the flag and returns
-	  the first entity with a larger id.
+	  the first entity with a larger ID. Do not expect that the ID will
+	  always be the same for each instance of the device. In other words,
+	  do not hardcode entity IDs in an application.
 
     *  -  char
        -  ``name``\ [32]
        -
        -
-       -  Entity name as an UTF-8 NULL-terminated string.
+       -  Entity name as an UTF-8 NULL-terminated string. This name must be unique
+          within the media topology.
 
     *  -  __u32
        -  ``type``
diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index e572dd0d806d..3a5f165d9811 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -131,11 +131,14 @@ desired arrays with the media graph elements.
 
     *  -  __u32
        -  ``id``
-       -  Unique ID for the entity.
+       -  Unique ID for the entity. Do not expect that the ID will
+	  always be the same for each instance of the device. In other words,
+	  do not hardcode entity IDs in an application.
 
     *  -  char
        -  ``name``\ [64]
-       -  Entity name as an UTF-8 NULL-terminated string.
+       -  Entity name as an UTF-8 NULL-terminated string. This name must be unique
+          within the media topology.
 
     *  -  __u32
        -  ``function``
@@ -166,7 +169,9 @@ desired arrays with the media graph elements.
 
     *  -  __u32
        -  ``id``
-       -  Unique ID for the interface.
+       -  Unique ID for the interface. Do not expect that the ID will
+	  always be the same for each instance of the device. In other words,
+	  do not hardcode interface IDs in an application.
 
     *  -  __u32
        -  ``intf_type``
@@ -215,7 +220,9 @@ desired arrays with the media graph elements.
 
     *  -  __u32
        -  ``id``
-       -  Unique ID for the pad.
+       -  Unique ID for the pad. Do not expect that the ID will
+	  always be the same for each instance of the device. In other words,
+	  do not hardcode pad IDs in an application.
 
     *  -  __u32
        -  ``entity_id``
@@ -231,7 +238,8 @@ desired arrays with the media graph elements.
 	  returns true. The ``media_version`` is defined in struct
 	  :c:type:`media_device_info` and can be retrieved using
 	  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are added
-	  for an entity in the future, then those will be added at the end.
+	  for an entity in the future, then those will be added at the end of the
+	  entity's pad array.
 
     *  -  __u32
        -  ``reserved``\ [4]
@@ -250,7 +258,9 @@ desired arrays with the media graph elements.
 
     *  -  __u32
        -  ``id``
-       -  Unique ID for the link.
+       -  Unique ID for the link. Do not expect that the ID will
+	  always be the same for each instance of the device. In other words,
+	  do not hardcode link IDs in an application.
 
     *  -  __u32
        -  ``source_id``
-- 
2.18.0

Re: [PATCHv6 02/12] media-ioc-g-topology.rst: document new 'index' field

[Mauro Carvalho Chehab]

Em Tue, 10 Jul 2018 10:45:02 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Document the new struct media_v2_pad 'index' field.
> 
> 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     | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> index a3f259f83b25..bae2b4db89cc 100644
> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> @@ -176,7 +176,7 @@ desired arrays with the media graph elements.
>      *  -  struct media_v2_intf_devnode
>         -  ``devnode``
>         -  Used only for device node interfaces. See
> -	  :c:type:`media_v2_intf_devnode` for details..
> +	  :c:type:`media_v2_intf_devnode` for details.
>  
>  
>  .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
> @@ -218,7 +218,15 @@ desired arrays with the media graph elements.
>         -  Pad flags, see :ref:`media-pad-flag` for more details.
>  
>      *  -  __u32
> -       -  ``reserved``\ [5]
> +       -  ``index``
> +       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
> +	  returns true. The ``media_version`` is defined in struct
> +	  :c:type:`media_device_info` and can be retrieved using
> +	  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are added
> +	  for an entity in the future, then those will be added at the end.

Hmm... Pad indexes may not be stable. That's by the way why we
need a better way to enum it, and the Properties API was thinking
to solve (and why we didn't add PAD index to this ioctl at the
first place). 

The problem happens for example on TV demods and tuners:
different models may have different kinds of output PADs:

	- analog luminance carrier samples;
	- analog chrominance sub-carrier samples;
	- sliced VBI data;
	- audio RF sub-carrier samples;
	- audio mono data;
	- audio stereo data.

The same bridge chip can live with different demods, but need to
setup the pipelines according with the type of the PAD. As right now
we don't have any way to associate a PAD with an specific type of
output, what happens is that the V4L2 core associates a pad number
with an specific type of output. So, drivers may be exposing
PADs that don't exist, in practice, just to make them compatible
with similar subdevs.

Once we add a properties API (or something equivalent), the
PAD numbers will change and subdevs will only expose the ones
that really exists.

Thanks,
Mauro

Re: [PATCHv6 02/12] media-ioc-g-topology.rst: document new 'index' field

[Hans Verkuil]

On 13/07/18 17:23, Mauro Carvalho Chehab wrote:
> Em Tue, 10 Jul 2018 10:45:02 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Document the new struct media_v2_pad 'index' field.
>>
>> 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     | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
>> index a3f259f83b25..bae2b4db89cc 100644
>> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
>> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
>> @@ -176,7 +176,7 @@ desired arrays with the media graph elements.
>>      *  -  struct media_v2_intf_devnode
>>         -  ``devnode``
>>         -  Used only for device node interfaces. See
>> -	  :c:type:`media_v2_intf_devnode` for details..
>> +	  :c:type:`media_v2_intf_devnode` for details.
>>  
>>  
>>  .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
>> @@ -218,7 +218,15 @@ desired arrays with the media graph elements.
>>         -  Pad flags, see :ref:`media-pad-flag` for more details.
>>  
>>      *  -  __u32
>> -       -  ``reserved``\ [5]
>> +       -  ``index``
>> +       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
>> +	  returns true. The ``media_version`` is defined in struct
>> +	  :c:type:`media_device_info` and can be retrieved using
>> +	  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are added
>> +	  for an entity in the future, then those will be added at the end.
> 
> Hmm... Pad indexes may not be stable. That's by the way why we
> need a better way to enum it, and the Properties API was thinking
> to solve (and why we didn't add PAD index to this ioctl at the
> first place). 
> 
> The problem happens for example on TV demods and tuners:
> different models may have different kinds of output PADs:
> 
> 	- analog luminance carrier samples;
> 	- analog chrominance sub-carrier samples;
> 	- sliced VBI data;
> 	- audio RF sub-carrier samples;
> 	- audio mono data;
> 	- audio stereo data.
> 
> The same bridge chip can live with different demods, but need to
> setup the pipelines according with the type of the PAD. As right now
> we don't have any way to associate a PAD with an specific type of
> output, what happens is that the V4L2 core associates a pad number
> with an specific type of output. So, drivers may be exposing
> PADs that don't exist, in practice, just to make them compatible
> with similar subdevs.
> 
> Once we add a properties API (or something equivalent), the
> PAD numbers will change and subdevs will only expose the ones
> that really exists.

So what do you suggest I do? There are two things here: you need the
pad index in order to use the SETUP_LINK ioctl, so adding this to
G_TOPOLOGY makes sense. The second is whether or not pad numbers
are stable. Currently they are, since there is no other way to
associate a pad with the type of signal it can carry.

Note that the index is already exposed with the older API, so changing
the pad index in the future will already potentially cause problems.

I am inclined to just remove the last two sentences of the description
above, so this becomes:

+       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
+	  returns true. The ``media_version`` is defined in struct
+	  :c:type:`media_device_info` and can be retrieved using
+	  :ref:`MEDIA_IOC_DEVICE_INFO`.

And we'll figure out what to do with this once we finally get properties.

That's something I might actually work on myself, but not before we get
the current API consistent.

Regards,

	Hans

Re: [PATCHv6 02/12] media-ioc-g-topology.rst: document new 'index' field

[Mauro Carvalho Chehab]

Em Tue, 17 Jul 2018 13:58:22 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 13/07/18 17:23, Mauro Carvalho Chehab wrote:
> > Em Tue, 10 Jul 2018 10:45:02 +0200
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >   
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>
> >> Document the new struct media_v2_pad 'index' field.
> >>
> >> 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     | 12 ++++++++++--
> >>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> >> index a3f259f83b25..bae2b4db89cc 100644
> >> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> >> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> >> @@ -176,7 +176,7 @@ desired arrays with the media graph elements.
> >>      *  -  struct media_v2_intf_devnode
> >>         -  ``devnode``
> >>         -  Used only for device node interfaces. See
> >> -	  :c:type:`media_v2_intf_devnode` for details..
> >> +	  :c:type:`media_v2_intf_devnode` for details.
> >>  
> >>  
> >>  .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
> >> @@ -218,7 +218,15 @@ desired arrays with the media graph elements.
> >>         -  Pad flags, see :ref:`media-pad-flag` for more details.
> >>  
> >>      *  -  __u32
> >> -       -  ``reserved``\ [5]
> >> +       -  ``index``
> >> +       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
> >> +	  returns true. The ``media_version`` is defined in struct
> >> +	  :c:type:`media_device_info` and can be retrieved using
> >> +	  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are added
> >> +	  for an entity in the future, then those will be added at the end.  
> > 
> > Hmm... Pad indexes may not be stable. That's by the way why we
> > need a better way to enum it, and the Properties API was thinking
> > to solve (and why we didn't add PAD index to this ioctl at the
> > first place). 
> > 
> > The problem happens for example on TV demods and tuners:
> > different models may have different kinds of output PADs:
> > 
> > 	- analog luminance carrier samples;
> > 	- analog chrominance sub-carrier samples;
> > 	- sliced VBI data;
> > 	- audio RF sub-carrier samples;
> > 	- audio mono data;
> > 	- audio stereo data.
> > 
> > The same bridge chip can live with different demods, but need to
> > setup the pipelines according with the type of the PAD. As right now
> > we don't have any way to associate a PAD with an specific type of
> > output, what happens is that the V4L2 core associates a pad number
> > with an specific type of output. So, drivers may be exposing
> > PADs that don't exist, in practice, just to make them compatible
> > with similar subdevs.
> > 
> > Once we add a properties API (or something equivalent), the
> > PAD numbers will change and subdevs will only expose the ones
> > that really exists.  
> 
> So what do you suggest I do? There are two things here: you need the
> pad index in order to use the SETUP_LINK ioctl, so adding this to
> G_TOPOLOGY makes sense. The second is whether or not pad numbers
> are stable. Currently they are, since there is no other way to
> associate a pad with the type of signal it can carry.
> 
> Note that the index is already exposed with the older API, so changing
> the pad index in the future will already potentially cause problems.
> 
> I am inclined to just remove the last two sentences of the description
> above, so this becomes:
> 
> +       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
> +	  returns true. The ``media_version`` is defined in struct
> +	  :c:type:`media_device_info` and can be retrieved using
> +	  :ref:`MEDIA_IOC_DEVICE_INFO`.
> 
> And we'll figure out what to do with this once we finally get properties.

Ok, perhaps this is the best way. It was never said at the API
documents that pad index are stable. 

> 
> That's something I might actually work on myself, but not before we get
> the current API consistent.
> 
> Regards,
> 
> 	Hans



Thanks,
Mauro

[PATCHv6.1 02/12] media-ioc-g-topology.rst: document new 'index' field

[Hans Verkuil]

Document the new struct media_v2_pad 'index' field.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Dropped the text about stable index values.
---
 .../media/uapi/mediactl/media-ioc-g-topology.rst      | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index a3f259f83b25..725961198143 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -176,7 +176,7 @@ desired arrays with the media graph elements.
     *  -  struct media_v2_intf_devnode
        -  ``devnode``
        -  Used only for device node interfaces. See
-	  :c:type:`media_v2_intf_devnode` for details..
+	  :c:type:`media_v2_intf_devnode` for details.


 .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
@@ -218,7 +218,14 @@ desired arrays with the media graph elements.
        -  Pad flags, see :ref:`media-pad-flag` for more details.

     *  -  __u32
-       -  ``reserved``\ [5]
+       -  ``index``
+       -  Pad index, starts at 0. Only valid if ``MEDIA_V2_PAD_HAS_INDEX(media_version)``
+	  returns true. The ``media_version`` is defined in struct
+	  :c:type:`media_device_info` and can be retrieved using
+	  :ref:`MEDIA_IOC_DEVICE_INFO`.
+
+    *  -  __u32
+       -  ``reserved``\ [4]
        -  Reserved for future extensions. Drivers and applications must set
 	  this array to zero.

-- 
2.18.0

[PATCHv6.1 11/12] media-ioc-enum-links.rst: improve pad index description

[Hans Verkuil]

Make it clearer that the index starts at 0.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
Dropped the text about stable index values.
---
 Documentation/media/uapi/mediactl/media-ioc-enum-links.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
index 17abdeed1a9c..f158c134e9b0 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
@@ -92,7 +92,7 @@ returned during the enumeration process.

     *  -  __u16
        -  ``index``
-       -  0-based pad index.
+       -  Pad index, starts at 0.

     *  -  __u32
        -  ``flags``
-- 
2.18.0