All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Generic line based metadata support, internal pads
@ 2023-06-30 20:43 Sakari Ailus
  2023-06-30 20:43 ` [PATCH 1/7] media: mc: Check pad flag validity Sakari Ailus
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Hi folks,

Here are a few patches to add support generic, line based metadata as well
as internal source pads. While the amount of code is not very large, to
the contrary it is quite small actually IMO, I presume what this is about
and why it is being proposed requires some explaining.

Metadata mbus codes and formats have existed for some time in V4L2. They
however have been only used by drivers that produce the data itself and
effectively this metadata has always been statistics of some sort (at
least when it comes to ISPs). What is different here is that we intend to
add support for metadata originating from camera sensors.

Camera sensors produce different kinds of metadata, embedded data (usually
register address--value pairs used to capture the frame, in a more or less
sensor specific format), histograms (in a very sensor specific format),
dark pixels etc. The number of these formats is probably going to be about
as large as image data formats if not larger, as the image data formats
are much better standardised but a smaller subset of them will be
supported by V4L2, at least initially but possibly much more in the long
run.

Having this many device specific formats would be a major problem for all
the other drivers along that pipeline (not to mention the users of those
drivers), including bridge (e.g. CSI-2 to parallel) but especially CSI-2
receiver drivers that have DMA: the poor driver developer would not only
need to know camera sensor specific formats but to choose the specific
packing of that format suitable for the DMA used by the hardware. It is
unlikely many of these would ever get tested while being present on the
driver API. Also adding new sensors with new embedded data formats would
involve updating all bridge and CSI-2 receiver drivers. I don't expect
this to be a workable approach.

Instead what I'm proposing is to use specific metadata formats on the
sensor devices only, on internal pads (more about those soon) of the
sensors, only visible in the UAPI, and then generic mbus formats along the
pipeline and finally generic V4L2 metadata formats on the DMAs (specific
to bit depth and packing). This would unsnarl the two, defining what data
there is (specific mbus code) and how that is transported and packed
(generic mbus codes and V4L2 formats).

The user space would be required to "know" the path of that data from the
sensor's internal pad to the V4L2 video node. I do not see this as these
devices require at least some knowledge of the pipeline, i.e. hardware at
hand. Separating what the data means and how it is packed may even be
beneficial: it allows separating code that interprets the data (sensor
internal mbus code) from the code that accesses it (packing).

These formats are in practice line based, meaning that there may be
padding at the end of the line, depending on the bus as well as the DMA.
If non-line based formats are needed, it is always possible to set the
"height" field to 1.

The internal (source) pads are an alternative to source routes [1]. The
source routes were not universally liked and I do have to say I like
re-using existing interface concepts (pads and everything you can do with
pads, including access format, selections etc.) wherever it makes sense,
instead of duplicating functionality.

Effectively internal source pads behave mostly just like sink pads, but
they describe a flow of data that originates from a sub-device instead of
arriving to a sub-device. The SUBDEV_S_ROUTING IOCTLs are used to enable
and disable routes from internal source pads to sub-device's source pads.
The subdev format IOCTLs are usable, too, so one can find which subdev
format is available on given internal source pad.

This set depends on these patches:

<URL:https://lore.kernel.org/linux-media/20230505205416.55002-1-sakari.ailus@linux.intel.com/T/#t>

I've also pushed these here and I'll keep updating the branch:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=metadata>

Questions and comments are most welcome.

Driver support is to be added, as well as an example.

[1] <URL:https://lore.kernel.org/linux-media/20220831141357.1396081-20-tomi.valkeinen@ideasonboard.com/>

since v1:

- Make the new pad flag just "INTERNAL", requiring either SINK or SOURCE
  pad flag to accompany it. Removed the union in struct v4l2_subdev_route.

- Add the term "stream" to MC glossary.

- Improved and fixed documentation (according to comments).

- Note these formats are little endian.

- Remove 1X8 from the names of the mbus codes. These formats have generally
  8 bits per pixel.

- Fix mbus code numbering (had holes in RFC).

- Add new metadata fields to debug prints.

- Fix a minor documentation build issue.

Sakari Ailus (7):
  media: mc: Check pad flag validity
  media: mc: Add INTERNAL pad flag
  media: v4l: subdev: Support INTERNAL pads in routing IOCTLs
  media: uapi: v4l: Document source routes
  media: uapi: Add generic serial metadata mbus formats
  media: uapi: Add generic 8-bit metadata format definitions
  media: v4l: Support line-based metadata capture

 .../userspace-api/media/glossary.rst          |   6 +
 .../media/mediactl/media-types.rst            |   6 +
 .../userspace-api/media/v4l/dev-meta.rst      |  15 +
 .../userspace-api/media/v4l/dev-subdev.rst    |  20 ++
 .../userspace-api/media/v4l/meta-formats.rst  |   1 +
 .../media/v4l/metafmt-generic.rst             | 331 ++++++++++++++++++
 .../media/v4l/subdev-formats.rst              | 257 ++++++++++++++
 .../media/v4l/vidioc-enum-fmt.rst             |   7 +
 .../media/videodev2.h.rst.exceptions          |   1 +
 drivers/media/mc/mc-entity.c                  |  17 +-
 drivers/media/v4l2-core/v4l2-ioctl.c          |  13 +-
 drivers/media/v4l2-core/v4l2-subdev.c         |   3 +-
 include/uapi/linux/media-bus-format.h         |   9 +
 include/uapi/linux/media.h                    |   1 +
 include/uapi/linux/videodev2.h                |  19 +
 15 files changed, 701 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/userspace-api/media/v4l/metafmt-generic.rst

-- 
2.39.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/7] media: mc: Check pad flag validity
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-07-03  7:53   ` Jacopo Mondi
  2023-06-30 20:43 ` [PATCH 2/7] media: mc: Add INTERNAL pad flag Sakari Ailus
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Check the validity of pad flags on entity init. Exactly one of the flags
must be set.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/mc/mc-entity.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 83468d4a440b..4991281dcccc 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	struct media_device *mdev = entity->graph_obj.mdev;
 	struct media_pad *iter;
 	unsigned int i = 0;
+	int ret = 0;
 
 	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
 		return -E2BIG;
@@ -210,6 +211,14 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	media_entity_for_each_pad(entity, iter) {
 		iter->entity = entity;
 		iter->index = i++;
+
+		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
+					     MEDIA_PAD_FL_SOURCE))
+		    != 1) {
+			ret = -EINVAL;
+			break;
+		}
+
 		if (mdev)
 			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
 					  &iter->graph_obj);
@@ -218,7 +227,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	if (mdev)
 		mutex_unlock(&mdev->graph_mutex);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(media_entity_pads_init);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/7] media: mc: Add INTERNAL pad flag
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
  2023-06-30 20:43 ` [PATCH 1/7] media: mc: Check pad flag validity Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-06-30 20:43 ` [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs Sakari Ailus
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Internal source pads will be used as routing endpoints in V4L2
[GS]_ROUTING IOCTLs, to indicate that the stream begins in the entity.

Also prevent creating links to pads that have been flagged as internal.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/userspace-api/media/glossary.rst             | 6 ++++++
 Documentation/userspace-api/media/mediactl/media-types.rst | 6 ++++++
 drivers/media/mc/mc-entity.c                               | 6 +++++-
 include/uapi/linux/media.h                                 | 1 +
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst
index 96a360edbf3b..f7b99a4527c7 100644
--- a/Documentation/userspace-api/media/glossary.rst
+++ b/Documentation/userspace-api/media/glossary.rst
@@ -173,6 +173,12 @@ Glossary
 	An integrated circuit that integrates all components of a computer
 	or other electronic systems.
 
+_media-glossary-stream:
+    Stream
+	A distinct flow of data (image data or metadata) over a media pipeline
+	from source to sink. A source may be e.g. an image sensor and a sink
+	e.g. a memory buffer.
+
     V4L2 API
 	**V4L2 userspace API**
 
diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
index 0ffeece1e0c8..28941da27790 100644
--- a/Documentation/userspace-api/media/mediactl/media-types.rst
+++ b/Documentation/userspace-api/media/mediactl/media-types.rst
@@ -361,6 +361,7 @@ Types and flags used to represent the media graph elements
 .. _MEDIA-PAD-FL-SINK:
 .. _MEDIA-PAD-FL-SOURCE:
 .. _MEDIA-PAD-FL-MUST-CONNECT:
+.. _MEDIA-PAD-FL-INTERNAL:
 
 .. flat-table:: Media pad flags
     :header-rows:  0
@@ -382,6 +383,11 @@ Types and flags used to represent the media graph elements
 	  when this flag isn't set; the absence of the flag doesn't imply
 	  there is none.
 
+    *  -  ``MEDIA_PAD_FL_INTERNAL``
+       -  The internal flag indicates an internal pad that has no external
+	  connections. Such a pad shall not be connected with a link. The
+	  internal flag indicates that the :ref:``stream
+	  <media-glossary-stream>`` either starts or ends in the entity.
 
 One and only one of ``MEDIA_PAD_FL_SINK`` and ``MEDIA_PAD_FL_SOURCE``
 must be set for every pad.
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 4991281dcccc..03a9e0b8ebab 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -1071,7 +1071,8 @@ int media_get_pad_index(struct media_entity *entity, u32 pad_type,
 
 	for (i = 0; i < entity->num_pads; i++) {
 		if ((entity->pads[i].flags &
-		     (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE)) != pad_type)
+		     (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE |
+		      MEDIA_PAD_FL_INTERNAL)) != pad_type)
 			continue;
 
 		if (entity->pads[i].sig_type == sig_type)
@@ -1094,6 +1095,9 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
 		return -EINVAL;
 	if (WARN_ON(!(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE)))
 		return -EINVAL;
+	if (WARN_ON(source->pads[source_pad].flags & MEDIA_PAD_FL_SOURCE &&
+		    source->pads[source_pad].flags & MEDIA_PAD_FL_INTERNAL))
+		return -EINVAL;
 	if (WARN_ON(!(sink->pads[sink_pad].flags & MEDIA_PAD_FL_SINK)))
 		return -EINVAL;
 
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 1c80b1d6bbaf..80cfd12a43fc 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -208,6 +208,7 @@ struct media_entity_desc {
 #define MEDIA_PAD_FL_SINK			(1U << 0)
 #define MEDIA_PAD_FL_SOURCE			(1U << 1)
 #define MEDIA_PAD_FL_MUST_CONNECT		(1U << 2)
+#define MEDIA_PAD_FL_INTERNAL			(1U << 3)
 
 struct media_pad_desc {
 	__u32 entity;		/* entity ID */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
  2023-06-30 20:43 ` [PATCH 1/7] media: mc: Check pad flag validity Sakari Ailus
  2023-06-30 20:43 ` [PATCH 2/7] media: mc: Add INTERNAL pad flag Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-07-02 11:57   ` Andrey Konovalov
  2023-06-30 20:43 ` [PATCH 4/7] media: uapi: v4l: Document source routes Sakari Ailus
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Take the new INTERNAL pad flag into account in validating routing IOCTL
argument. Effectively this is a SINK pad in this respect. Due to the union
there's no need to check for the particular name.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 2ec179cd1264..36886a9047c4 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1787,7 +1787,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 
 		/* Validate the sink and source pad numbers. */
 		if (route->sink_pad >= sd->entity.num_pads ||
-		    !(sd->entity.pads[route->sink_pad].flags & MEDIA_PAD_FL_SINK)) {
+		    !(sd->entity.pads[route->sink_pad].flags &
+		      MEDIA_PAD_FL_SINK)) {
 			dev_dbg(sd->dev, "route %u sink (%u) is not a sink pad\n",
 				i, route->sink_pad);
 			goto out;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/7] media: uapi: v4l: Document source routes
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
                   ` (2 preceding siblings ...)
  2023-06-30 20:43 ` [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-07-03  7:47   ` Jacopo Mondi
  2023-06-30 20:43 ` [PATCH 5/7] media: uapi: Add generic serial metadata mbus formats Sakari Ailus
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Document how internal pads are used on source routes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/v4l/dev-subdev.rst    | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
index a4f1df7093e8..5a46c9a9d352 100644
--- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
+++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
@@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
 sub-device and a (pad, stream) pair. For sub-devices that do not support
 multiplexed streams the 'stream' field is always 0.
 
+.. _v4l2-subdev-source-routes:
+
+Source routes
+^^^^^^^^^^^^^
+
+Cases where a single sub-device pad is a source of multiple streams are special
+as there is no external sink pad for such a route. In those cases, the sources
+of the streams are indicated by source routes that have an internal source pad
+as the sink pad of such a route. Internal source pads have the
+:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
+pad flags set.
+
+Internal source pads have all the properties of a sink pad in such case,
+including formats and selections. The format in this case is the source format
+of the stream. An internal pad always has a single stream only (0).
+
+Generally source routes are not modifiable but they can be activated and
+deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
+<v4l2-subdev-routing-flags>` flag, depending on driver capabilities.
+
 Interaction between routes, streams, formats and selections
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/7] media: uapi: Add generic serial metadata mbus formats
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
                   ` (3 preceding siblings ...)
  2023-06-30 20:43 ` [PATCH 4/7] media: uapi: v4l: Document source routes Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-06-30 20:43 ` [PATCH 6/7] media: uapi: Add generic 8-bit metadata format definitions Sakari Ailus
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Add generic serial metadata mbus formats. These formats describe data
width and packing but not the content itself. The reason for specifying
such formats is that the formats as such are fairly device specific but
they are still handled by CSI-2 receiver drivers that should not be aware
of device specific formats. What makes generic metadata formats possible
is that these formats are parsed by software only, after capturing the
data to system memory.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/v4l/subdev-formats.rst              | 257 ++++++++++++++++++
 include/uapi/linux/media-bus-format.h         |   9 +
 2 files changed, 266 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index a3a35eeed708..c615da08502d 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -8234,3 +8234,260 @@ The following table lists the existing metadata formats.
 	both sides of the link and the bus format is a fixed
 	metadata format that is not configurable from userspace.
 	Width and height will be set to 0 for this format.
+
+Generic Serial Metadata Formats
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Generic serial metadata formats are used on serial busses where the actual data
+content is more or less device specific but the data is transmitted and received
+by multiple devices that do not process the data in any way, simply writing
+it to system memory for processing in software at the end of the pipeline.
+
+The more specific variant describing the actual data is used on the internal
+source pad of the originating sub-device.
+
+"b" in an array cell signifies a byte of data, followed by the number of byte
+and finally the bit number in subscript. "p" indicates a padding bit.
+
+.. _media-bus-format-generic-meta:
+
+.. cssclass: longtable
+
+.. flat-table:: Generic Serial Metadata Formats
+    :header-rows:  2
+    :stub-columns: 0
+
+    * - Identifier
+      - Code
+      -
+      - :cspan:`23` Data organization
+    * -
+      -
+      - Bit
+      - 23
+      - 22
+      - 21
+      - 20
+      - 19
+      - 18
+      - 17
+      - 16
+      - 15
+      - 14
+      - 13
+      - 12
+      - 11
+      - 10
+      - 9
+      - 8
+      - 7
+      - 6
+      - 5
+      - 4
+      - 3
+      - 2
+      - 1
+      - 0
+    * .. _MEDIA-BUS-FMT-META-8:
+
+      - MEDIA_BUS_FMT_META_8
+      - 0x8001
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+    * .. _MEDIA-BUS-FMT-META-10:
+
+      - MEDIA_BUS_FMT_META_10
+      - 0x8002
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+      - p
+      - p
+    * .. _MEDIA-BUS-FMT-META-12:
+
+      - MEDIA_BUS_FMT_META_12
+      - 0x8003
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+      - p
+      - p
+      - p
+      - p
+    * .. _MEDIA-BUS-FMT-META-14:
+
+      - MEDIA_BUS_FMT_META_14
+      - 0x8004
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+    * .. _MEDIA-BUS-FMT-META-16:
+
+      - MEDIA_BUS_FMT_META_16
+      - 0x8005
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+    * .. _MEDIA-BUS-FMT-META-20:
+
+      - MEDIA_BUS_FMT_META_20
+      - 0x8007
+      -
+      -
+      -
+      -
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+    * .. _MEDIA-BUS-FMT-META-24:
+
+      - MEDIA_BUS_FMT_META_24
+      - 0x8009
+      -
+      - b0\ :sub:`7`
+      - b0\ :sub:`6`
+      - b0\ :sub:`5`
+      - b0\ :sub:`4`
+      - b0\ :sub:`3`
+      - b0\ :sub:`2`
+      - b0\ :sub:`1`
+      - b0\ :sub:`0`
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
+      - p
diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
index a03c543cb072..9ee031397372 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -173,4 +173,13 @@
  */
 #define MEDIA_BUS_FMT_METADATA_FIXED		0x7001
 
+/* Generic line based metadata formats for serial buses. Next is 0x8008. */
+#define MEDIA_BUS_FMT_META_8			0x8001
+#define MEDIA_BUS_FMT_META_10			0x8002
+#define MEDIA_BUS_FMT_META_12			0x8003
+#define MEDIA_BUS_FMT_META_14			0x8004
+#define MEDIA_BUS_FMT_META_16			0x8005
+#define MEDIA_BUS_FMT_META_20			0x8006
+#define MEDIA_BUS_FMT_META_24			0x8007
+
 #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/7] media: uapi: Add generic 8-bit metadata format definitions
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
                   ` (4 preceding siblings ...)
  2023-06-30 20:43 ` [PATCH 5/7] media: uapi: Add generic serial metadata mbus formats Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-06-30 20:43 ` [PATCH 7/7] media: v4l: Support line-based metadata capture Sakari Ailus
  2023-07-04 12:57 ` [PATCH 0/7] Generic line based metadata support, internal pads Laurent Pinchart
  7 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Generic 8-bit metadata formats define the in-memory data layout but not
the format of the data itself. The reasoning for having such formats is to
allow CSI-2 receiver drivers to receive and DMA drivers to write the data
to memory without knowing a large number of device specific formats.

These formats may be used only in conjunction of a Media controller
pipeline where the internal pad of the source sub-device defines the
specific format of the data (using an mbus code).

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/v4l/meta-formats.rst  |   1 +
 .../media/v4l/metafmt-generic.rst             | 331 ++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c          |   8 +
 include/uapi/linux/videodev2.h                |   9 +
 4 files changed, 349 insertions(+)
 create mode 100644 Documentation/userspace-api/media/v4l/metafmt-generic.rst

diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
index 0bb61fc5bc00..919f595576b9 100644
--- a/Documentation/userspace-api/media/v4l/meta-formats.rst
+++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
@@ -19,3 +19,4 @@ These formats are used for the :ref:`metadata` interface only.
     metafmt-vsp1-hgo
     metafmt-vsp1-hgt
     metafmt-vivid
+    metafmt-generic
diff --git a/Documentation/userspace-api/media/v4l/metafmt-generic.rst b/Documentation/userspace-api/media/v4l/metafmt-generic.rst
new file mode 100644
index 000000000000..a27bfc721edf
--- /dev/null
+++ b/Documentation/userspace-api/media/v4l/metafmt-generic.rst
@@ -0,0 +1,331 @@
+.. SPDX-License-Identifier: GPL-2.0 OR GFDL-1.1-no-invariants-or-later
+
+**************************************************************************************************************************************************************************************************************************************************************************************************************************
+V4L2_META_FMT_GENERIC_8 ('MET8'), V4L2_META_FMT_GENERIC_CSI2_10 ('MC1A'), V4L2_META_FMT_GENERIC_CSI2_12 ('MC1C'), V4L2_META_FMT_GENERIC_CSI2_14 ('MC1E'), V4L2_META_FMT_GENERIC_CSI2_16 ('MC1G'), V4L2_META_FMT_GENERIC_CSI2_20 ('MC1K'), V4L2_META_FMT_GENERIC_CSI2_24 ('MC1O'), V4L2_META_FMT_GENERIC_CSI2_2_24 ('MC2O')
+**************************************************************************************************************************************************************************************************************************************************************************************************************************
+
+
+Generic line-based metadata formats
+
+
+Description
+===========
+
+These generic line-based metadata formats define the memory layout of the data
+without defining the format or meaning of the metadata itself. These formats may
+only be used with a Media controller pipeline where the more specific format is
+defined in an :ref:`internal source pad <MEDIA-PAD-FL-INTERNAL>` of the source
+sub-device. See also :ref:`source routes <v4l2-subdev-source-routes>`.
+
+.. _v4l2-meta-fmt-generic-8:
+
+V4L2_META_FMT_GENERIC_8
+-----------------------
+
+The V4L2_META_FMT_GENERIC_8 format is a plain 8-bit metadata format.
+
+This format is also used on CSI-2 on both 8 bits per sample as well as on
+16 bits per sample when two bytes of metadata are packed into one sample.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_8.**
+Each cell is one byte. "M" denotes a byte of metadata.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - M\ :sub:`10`
+      - M\ :sub:`20`
+      - M\ :sub:`30`
+    * - start + 4:
+      - M\ :sub:`01`
+      - M\ :sub:`11`
+      - M\ :sub:`21`
+      - M\ :sub:`31`
+
+.. _v4l2-meta-fmt-generic-csi2-10:
+
+V4L2_META_FMT_GENERIC_CSI2_10
+-----------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_10 contains packed 8-bit generic metadata, 10 bits
+for each 8 bits of data. Every four bytes of metadata is followed by a single
+byte of padding. The way the data is stored follows the CSI-2 specification.
+
+This format is also used on CSI-2 on 20 bits per sample format that packs two
+bytes of metadata into one sample.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_10.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - M\ :sub:`10`
+      - M\ :sub:`20`
+      - M\ :sub:`30`
+      - p
+    * - start + 5:
+      - M\ :sub:`01`
+      - M\ :sub:`11`
+      - M\ :sub:`21`
+      - M\ :sub:`31`
+      - p
+
+.. _v4l2-meta-fmt-generic-csi2-12:
+
+V4L2_META_FMT_GENERIC_CSI2_12
+-----------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_12 contains packed 8-bit generic metadata, 12 bits
+for each 8 bits of data. Every four bytes of metadata is followed by two bytes
+of padding. The way the data is stored follows the CSI-2 specification.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_12.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{.8cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - M\ :sub:`10`
+      - M\ :sub:`20`
+      - M\ :sub:`30`
+      - p
+      - p
+    * - start + 6:
+      - M\ :sub:`01`
+      - M\ :sub:`11`
+      - M\ :sub:`21`
+      - M\ :sub:`31`
+      - p
+      - p
+
+.. _v4l2-meta-fmt-generic-csi2-14:
+
+V4L2_META_FMT_GENERIC_CSI2_14
+-----------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_14 contains packed 8-bit generic metadata, 14 bits
+for each 8 bits of data. Every four bytes of metadata is followed by three
+bytes of padding. The way the data is stored follows the CSI-2 specification.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_14.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{1.2cm}|p{.8cm}|p{.8cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - M\ :sub:`10`
+      - M\ :sub:`20`
+      - M\ :sub:`30`
+      - p
+      - p
+      - p
+    * - start + 7:
+      - M\ :sub:`01`
+      - M\ :sub:`11`
+      - M\ :sub:`21`
+      - M\ :sub:`31`
+      - p
+      - p
+      - p
+
+.. _v4l2-meta-fmt-generic-csi2-16:
+
+V4L2_META_FMT_GENERIC_CSI2_16
+-----------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_16 contains packed 8-bit generic metadata, 16 bits
+for each 8 bits of data. Every byte of metadata is followed by one byte of
+padding. The way the data is stored follows the CSI-2 specification.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_16.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{1.2cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - p
+      - M\ :sub:`10`
+      - p
+      - M\ :sub:`20`
+      - p
+      - M\ :sub:`30`
+      - p
+    * - start + 8:
+      - M\ :sub:`01`
+      - p
+      - M\ :sub:`11`
+      - p
+      - M\ :sub:`21`
+      - p
+      - M\ :sub:`31`
+      - p
+
+.. _v4l2-meta-fmt-generic-csi2-20:
+
+V4L2_META_FMT_GENERIC_CSI2_20
+-----------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_20 contains packed 8-bit generic metadata, 20 bits
+for each 8 bits of data. Every byte of metadata is followed by alternating one
+and two bytes of padding. The way the data is stored follows the CSI-2
+specification.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_20.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - p
+      - M\ :sub:`10`
+      - p
+      - p
+      - M\ :sub:`20`
+      - p
+      - M\ :sub:`30`
+      - p
+      - p
+    * - start + 10:
+      - M\ :sub:`01`
+      - p
+      - M\ :sub:`11`
+      - p
+      - p
+      - M\ :sub:`21`
+      - p
+      - M\ :sub:`31`
+      - p
+      - p
+
+.. _v4l2-meta-fmt-generic-csi2-24:
+
+V4L2_META_FMT_GENERIC_CSI2_24
+-----------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_24 contains packed 8-bit generic metadata, 24 bits
+for each 8 bits of data. Every byte of metadata is followed by two bytes of
+padding. The way the data is stored follows the CSI-2 specification.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_24.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{.8cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{.8cm}|p{1.2cm}|p{.8cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8 8 8 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - p
+      - p
+      - M\ :sub:`10`
+      - p
+      - p
+      - M\ :sub:`20`
+      - p
+      - p
+      - M\ :sub:`30`
+      - p
+      - p
+    * - start + 12:
+      - M\ :sub:`01`
+      - p
+      - p
+      - M\ :sub:`11`
+      - p
+      - p
+      - M\ :sub:`21`
+      - p
+      - p
+      - M\ :sub:`31`
+      - p
+      - p
+
+.. _v4l2-meta-fmt-generic-csi2-2-24:
+
+V4L2_META_FMT_GENERIC_CSI2_2_24
+-------------------------------
+
+V4L2_META_FMT_GENERIC_CSI2_2_24 contains packed 8-bit generic metadata, 24 bits
+for each two times 8 bits of data. Every two bytes of metadata are followed by
+one byte of padding. The way the data is stored follows the CSI-2
+specification.
+
+This format is little endian.
+
+**Byte Order Of V4L2_META_FMT_GENERIC_CSI2_2_24.**
+Each cell is one byte. "M" denotes a byte of metadata and "p" a byte of padding.
+
+.. tabularcolumns:: |p{2.4cm}|p{1.2cm}|p{1.2cm}|p{.8cm}|p{1.2cm}|p{1.2cm}|p{.8cm}|
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 12 8 8 8 8 8 8
+
+    * - start + 0:
+      - M\ :sub:`00`
+      - M\ :sub:`10`
+      - p
+      - M\ :sub:`20`
+      - M\ :sub:`30`
+      - p
+    * - start + 6:
+      - M\ :sub:`01`
+      - M\ :sub:`11`
+      - p
+      - M\ :sub:`21`
+      - M\ :sub:`31`
+      - p
+
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index a858acea6547..7781b0bd3e7d 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1451,6 +1451,14 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_PIX_FMT_Y210:		descr = "10-bit YUYV Packed"; break;
 	case V4L2_PIX_FMT_Y212:		descr = "12-bit YUYV Packed"; break;
 	case V4L2_PIX_FMT_Y216:		descr = "16-bit YUYV Packed"; break;
+	case V4L2_META_FMT_GENERIC_8:	descr = "8-bit Generic Metadata"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_10:	descr = "8b Generic Meta, 10b CSI-2"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_12:	descr = "8b Generic Meta, 12b CSI-2"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_14:	descr = "8b Generic Meta, 14b CSI-2"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_16:	descr = "8b Generic Meta, 16b CSI-2"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_20:	descr = "8b Generic Meta, 20b CSI-2"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_24:	descr = "8b Generic Meta, 24b CSI-2"; break;
+	case V4L2_META_FMT_GENERIC_CSI2_2_24:	descr = "2x8b Generic Meta, 24b CSI-2"; break;
 
 	default:
 		/* Compressed formats */
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 5d8bd754c69f..13d404699842 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -834,6 +834,15 @@ struct v4l2_pix_format {
 #define V4L2_META_FMT_RK_ISP1_PARAMS	v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */
 #define V4L2_META_FMT_RK_ISP1_STAT_3A	v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */
 
+#define V4L2_META_FMT_GENERIC_8		v4l2_fourcc('M', 'E', 'T', '8') /* Generic 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_10	v4l2_fourcc('M', 'C', '1', 'A') /* 10-bit CSI-2 packed 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_12	v4l2_fourcc('M', 'C', '1', 'C') /* 12-bit CSI-2 packed 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_14	v4l2_fourcc('M', 'C', '1', 'E') /* 14-bit CSI-2 packed 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_16	v4l2_fourcc('M', 'C', '1', 'G') /* 16-bit CSI-2 packed 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_20	v4l2_fourcc('M', 'C', '1', 'K') /* 20-bit CSI-2 packed 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_24	v4l2_fourcc('M', 'C', '1', 'O') /* 24-bit CSI-2 packed 8-bit metadata */
+#define V4L2_META_FMT_GENERIC_CSI2_2_24	v4l2_fourcc('M', 'C', '2', 'O') /* 2 bytes of 8-bit metadata, 24-bit CSI-2 packed */
+
 /* priv field value to indicates that subsequent fields are valid. */
 #define V4L2_PIX_FMT_PRIV_MAGIC		0xfeedcafe
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/7] media: v4l: Support line-based metadata capture
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
                   ` (5 preceding siblings ...)
  2023-06-30 20:43 ` [PATCH 6/7] media: uapi: Add generic 8-bit metadata format definitions Sakari Ailus
@ 2023-06-30 20:43 ` Sakari Ailus
  2023-07-04 12:57 ` [PATCH 0/7] Generic line based metadata support, internal pads Laurent Pinchart
  7 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-30 20:43 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

many camera sensors, among other devices, transmit embedded data and image
data for each CSI-2 frame. This embedded data typically contains register
configuration of the sensor that has been used to capture the image data
of the same frame.

The embedded data is received by the CSI-2 receiver and has the same
properties as the image data, including that it is line based: it has
width, height and bytesperline (stride).

Add these fields to struct v4l2_meta_format and document them.

Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
line-based i.e. these fields of struct v4l2_meta_format are valid for it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
 .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
 .../media/videodev2.h.rst.exceptions              |  1 +
 drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
 include/uapi/linux/videodev2.h                    | 10 ++++++++++
 5 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
index 0e7e1ee1471a..4b24bae6e171 100644
--- a/Documentation/userspace-api/media/v4l/dev-meta.rst
+++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
@@ -65,3 +65,18 @@ to 0.
       - ``buffersize``
       - Maximum buffer size in bytes required for data. The value is set by the
         driver.
+    * - __u32
+      - ``width``
+      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
+	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
+	:c:func:`VIDIOC_ENUM_FMT`.
+    * - __u32
+      - ``height``
+      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
+	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
+	:c:func:`VIDIOC_ENUM_FMT`.
+    * - __u32
+      - ``bytesperline``
+      - Offset in bytes between the beginning of two consecutive lines. Valid
+	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
+	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
index 000c154b0f98..6d7664345a4e 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
@@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
 	The application can ask to configure the quantization of the capture
 	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
 	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
+    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
+      - 0x0200
+      - The metadata format is line-based. In this case the ``width``,
+	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
+	valid. The buffer consists of ``height`` lines, each having ``width``
+	bytes of data and offset between the beginning of each two consecutive
+	lines is ``bytesperline``.
 
 Return Value
 ============
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 2a589d34b80e..5d623bf03bc5 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -211,6 +211,7 @@ replace define V4L2_FMT_FLAG_CSC_XFER_FUNC fmtdesc-flags
 replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
 replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
 replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
+replace define V4L2_FMT_FLAG_META_LINE_BASED fmtdesc-flags
 
 # V4L2 timecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 7781b0bd3e7d..2c7a7a6059cc 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -343,8 +343,9 @@ static void v4l_print_format(const void *arg, bool write_only)
 	case V4L2_BUF_TYPE_META_OUTPUT:
 		meta = &p->fmt.meta;
 		pixelformat = meta->dataformat;
-		pr_cont(", dataformat=%p4cc, buffersize=%u\n",
-			&pixelformat, meta->buffersize);
+		pr_cont(", dataformat=%p4cc, buffersize=%u, width=%u, height=%u, bytesperline=%u\n",
+			&pixelformat, meta->buffersize, meta->width,
+			meta->height, meta->bytesperline);
 		break;
 	}
 }
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 13d404699842..7485920f1ac9 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -873,6 +873,7 @@ struct v4l2_fmtdesc {
 #define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0080
 #define V4L2_FMT_FLAG_CSC_HSV_ENC		V4L2_FMT_FLAG_CSC_YCBCR_ENC
 #define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0100
+#define V4L2_FMT_FLAG_META_LINE_BASED		0x0200
 
 	/* Frame Size and frame rate enumeration */
 /*
@@ -2407,10 +2408,19 @@ struct v4l2_sdr_format {
  * struct v4l2_meta_format - metadata format definition
  * @dataformat:		little endian four character code (fourcc)
  * @buffersize:		maximum size in bytes required for data
+ * @width:		number of bytes of data per line (valid for line based
+ *			formats only, see format documentation)
+ * @height:		number of lines of data per buffer (valid for line based
+ *			formats only)
+ * @bytesperline:	offset between the beginnings of two adjacent lines in
+ *			bytes (valid for line based formats only)
  */
 struct v4l2_meta_format {
 	__u32				dataformat;
 	__u32				buffersize;
+	__u32				width;
+	__u32				height;
+	__u32				bytesperline;
 } __attribute__ ((packed));
 
 /**
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs
  2023-06-30 20:43 ` [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs Sakari Ailus
@ 2023-07-02 11:57   ` Andrey Konovalov
  2023-07-05 14:39     ` Sakari Ailus
  0 siblings, 1 reply; 19+ messages in thread
From: Andrey Konovalov @ 2023-07-02 11:57 UTC (permalink / raw)
  To: Sakari Ailus, linux-media
  Cc: Laurent Pinchart, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Hi Sakari,

On 30.06.2023 23:43, Sakari Ailus wrote:
> Take the new INTERNAL pad flag into account in validating routing IOCTL
> argument. Effectively this is a SINK pad in this respect. Due to the union
> there's no need to check for the particular name.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>   drivers/media/v4l2-core/v4l2-subdev.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 2ec179cd1264..36886a9047c4 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1787,7 +1787,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>   
>   		/* Validate the sink and source pad numbers. */
>   		if (route->sink_pad >= sd->entity.num_pads ||
> -		    !(sd->entity.pads[route->sink_pad].flags & MEDIA_PAD_FL_SINK)) {
> +		    !(sd->entity.pads[route->sink_pad].flags &
> +		      MEDIA_PAD_FL_SINK)) {

This is a white space only change now, so maybe this patch could be just 
dropped?

Thanks,
Andrey

>   			dev_dbg(sd->dev, "route %u sink (%u) is not a sink pad\n",
>   				i, route->sink_pad);
>   			goto out;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] media: uapi: v4l: Document source routes
  2023-06-30 20:43 ` [PATCH 4/7] media: uapi: v4l: Document source routes Sakari Ailus
@ 2023-07-03  7:47   ` Jacopo Mondi
  2023-07-06 10:20     ` Sakari Ailus
  2023-07-08 12:13     ` Sakari Ailus
  0 siblings, 2 replies; 19+ messages in thread
From: Jacopo Mondi @ 2023-07-03  7:47 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

Hi Sakari

On Fri, Jun 30, 2023 at 11:43:35PM +0300, Sakari Ailus wrote:
> Document how internal pads are used on source routes.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../userspace-api/media/v4l/dev-subdev.rst    | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> index a4f1df7093e8..5a46c9a9d352 100644
> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> @@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
>  sub-device and a (pad, stream) pair. For sub-devices that do not support
>  multiplexed streams the 'stream' field is always 0.
>
> +.. _v4l2-subdev-source-routes:
> +
> +Source routes
> +^^^^^^^^^^^^^

I always found the concept of source routes a bit confusing, should we
instead just present internal pads ?

> +
> +Cases where a single sub-device pad is a source of multiple streams are special
> +as there is no external sink pad for such a route. In those cases, the sources
> +of the streams are indicated by source routes that have an internal source pad
> +as the sink pad of such a route. Internal source pads have the
> +:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> +pad flags set.

All this last part is a little bit hard to parse, not your fault but
the fact "internal source pads" are actually "SINK" pads is a bit
confusing ?

Can we remove the "source route" concept to avoid mixing source/sink ?

This can be rewritten as

Internal pads
^^^^^^^^^^^^^

Cases where a single sub-device pad is a source of multiple streams are special
as there is no external sink pad for such a route. A typical example is a
sensor device which produces a video stream and a metadata stream of
embedded data. To support such cases internal pads are introduced as
sink pads of such internally generated streams.
Internal source pads have the :ref:`MEDIA_PAD_FL_INTERNAL
<MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK`` pad flags set.

> +Internal source pads have all the properties of a sink pad in such case,

Also here, "Internal source pads" are actually sinks :)

I would drop "source" from "Internal source pads"


> +including formats and selections. The format in this case is the source format
> +of the stream. An internal pad always has a single stream only (0).
> +
> +Generally source routes are not modifiable but they can be activated and

s/source routes/internal routes/ ?

> +deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
> +<v4l2-subdev-routing-flags>` flag, depending on driver capabilities.
> +
>  Interaction between routes, streams, formats and selections
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> --
> 2.39.2
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/7] media: mc: Check pad flag validity
  2023-06-30 20:43 ` [PATCH 1/7] media: mc: Check pad flag validity Sakari Ailus
@ 2023-07-03  7:53   ` Jacopo Mondi
  2023-07-05 14:40     ` Sakari Ailus
  0 siblings, 1 reply; 19+ messages in thread
From: Jacopo Mondi @ 2023-07-03  7:53 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

Hi Sakari

On Fri, Jun 30, 2023 at 11:43:32PM +0300, Sakari Ailus wrote:
> Check the validity of pad flags on entity init. Exactly one of the flags
> must be set.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/mc/mc-entity.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 83468d4a440b..4991281dcccc 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	struct media_device *mdev = entity->graph_obj.mdev;
>  	struct media_pad *iter;
>  	unsigned int i = 0;
> +	int ret = 0;
>
>  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
>  		return -E2BIG;
> @@ -210,6 +211,14 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	media_entity_for_each_pad(entity, iter) {
>  		iter->entity = entity;
>  		iter->index = i++;
> +
> +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> +					     MEDIA_PAD_FL_SOURCE))
> +		    != 1) {

Fits on the previous line ?

> +			ret = -EINVAL;
> +			break;
> +		}
> +
>  		if (mdev)
>  			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
>  					  &iter->graph_obj);
> @@ -218,7 +227,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	if (mdev)
>  		mutex_unlock(&mdev->graph_mutex);
>
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(media_entity_pads_init);
>
> --
> 2.39.2
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] Generic line based metadata support, internal pads
  2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
                   ` (6 preceding siblings ...)
  2023-06-30 20:43 ` [PATCH 7/7] media: v4l: Support line-based metadata capture Sakari Ailus
@ 2023-07-04 12:57 ` Laurent Pinchart
  2023-07-05  8:27   ` Sakari Ailus
  7 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2023-07-04 12:57 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Hi Sakari,

On Fri, Jun 30, 2023 at 11:43:31PM +0300, Sakari Ailus wrote:
> Hi folks,
> 
> Here are a few patches to add support generic, line based metadata as well
> as internal source pads. While the amount of code is not very large, to
> the contrary it is quite small actually IMO, I presume what this is about
> and why it is being proposed requires some explaining.
> 
> Metadata mbus codes and formats have existed for some time in V4L2. They
> however have been only used by drivers that produce the data itself and
> effectively this metadata has always been statistics of some sort (at
> least when it comes to ISPs). What is different here is that we intend to
> add support for metadata originating from camera sensors.

I've just realized that, unless I'm mistaken, we've never documented
which of the v4l2_mbus_framefmt fields are valid for metadata formats.
Should this be fixed as part of this series ?

> Camera sensors produce different kinds of metadata, embedded data (usually
> register address--value pairs used to capture the frame, in a more or less
> sensor specific format), histograms (in a very sensor specific format),
> dark pixels etc. The number of these formats is probably going to be about
> as large as image data formats if not larger, as the image data formats
> are much better standardised but a smaller subset of them will be
> supported by V4L2, at least initially but possibly much more in the long
> run.
> 
> Having this many device specific formats would be a major problem for all
> the other drivers along that pipeline (not to mention the users of those
> drivers), including bridge (e.g. CSI-2 to parallel) but especially CSI-2
> receiver drivers that have DMA: the poor driver developer would not only
> need to know camera sensor specific formats but to choose the specific
> packing of that format suitable for the DMA used by the hardware. It is
> unlikely many of these would ever get tested while being present on the
> driver API. Also adding new sensors with new embedded data formats would
> involve updating all bridge and CSI-2 receiver drivers. I don't expect
> this to be a workable approach.
> 
> Instead what I'm proposing is to use specific metadata formats on the
> sensor devices only, on internal pads (more about those soon) of the
> sensors, only visible in the UAPI, and then generic mbus formats along the
> pipeline and finally generic V4L2 metadata formats on the DMAs (specific
> to bit depth and packing). This would unsnarl the two, defining what data
> there is (specific mbus code) and how that is transported and packed
> (generic mbus codes and V4L2 formats).
> 
> The user space would be required to "know" the path of that data from the
> sensor's internal pad to the V4L2 video node. I do not see this as these
> devices require at least some knowledge of the pipeline, i.e. hardware at
> hand. Separating what the data means and how it is packed may even be
> beneficial: it allows separating code that interprets the data (sensor
> internal mbus code) from the code that accesses it (packing).
> 
> These formats are in practice line based, meaning that there may be
> padding at the end of the line, depending on the bus as well as the DMA.
> If non-line based formats are needed, it is always possible to set the
> "height" field to 1.
> 
> The internal (source) pads are an alternative to source routes [1]. The
> source routes were not universally liked and I do have to say I like
> re-using existing interface concepts (pads and everything you can do with
> pads, including access format, selections etc.) wherever it makes sense,
> instead of duplicating functionality.
> 
> Effectively internal source pads behave mostly just like sink pads, but
> they describe a flow of data that originates from a sub-device instead of
> arriving to a sub-device. The SUBDEV_S_ROUTING IOCTLs are used to enable
> and disable routes from internal source pads to sub-device's source pads.
> The subdev format IOCTLs are usable, too, so one can find which subdev
> format is available on given internal source pad.
> 
> This set depends on these patches:
> 
> <URL:https://lore.kernel.org/linux-media/20230505205416.55002-1-sakari.ailus@linux.intel.com/T/#t>
> 
> I've also pushed these here and I'll keep updating the branch:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=metadata>
> 
> Questions and comments are most welcome.
> 
> Driver support is to be added, as well as an example.

Jacopo and I are working on this, including libcamera support.

> [1] <URL:https://lore.kernel.org/linux-media/20220831141357.1396081-20-tomi.valkeinen@ideasonboard.com/>
> 
> since v1:
> 
> - Make the new pad flag just "INTERNAL", requiring either SINK or SOURCE
>   pad flag to accompany it. Removed the union in struct v4l2_subdev_route.
> 
> - Add the term "stream" to MC glossary.
> 
> - Improved and fixed documentation (according to comments).
> 
> - Note these formats are little endian.
> 
> - Remove 1X8 from the names of the mbus codes. These formats have generally
>   8 bits per pixel.
> 
> - Fix mbus code numbering (had holes in RFC).
> 
> - Add new metadata fields to debug prints.
> 
> - Fix a minor documentation build issue.
> 
> Sakari Ailus (7):
>   media: mc: Check pad flag validity
>   media: mc: Add INTERNAL pad flag
>   media: v4l: subdev: Support INTERNAL pads in routing IOCTLs
>   media: uapi: v4l: Document source routes
>   media: uapi: Add generic serial metadata mbus formats
>   media: uapi: Add generic 8-bit metadata format definitions
>   media: v4l: Support line-based metadata capture
> 
>  .../userspace-api/media/glossary.rst          |   6 +
>  .../media/mediactl/media-types.rst            |   6 +
>  .../userspace-api/media/v4l/dev-meta.rst      |  15 +
>  .../userspace-api/media/v4l/dev-subdev.rst    |  20 ++
>  .../userspace-api/media/v4l/meta-formats.rst  |   1 +
>  .../media/v4l/metafmt-generic.rst             | 331 ++++++++++++++++++
>  .../media/v4l/subdev-formats.rst              | 257 ++++++++++++++
>  .../media/v4l/vidioc-enum-fmt.rst             |   7 +
>  .../media/videodev2.h.rst.exceptions          |   1 +
>  drivers/media/mc/mc-entity.c                  |  17 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          |  13 +-
>  drivers/media/v4l2-core/v4l2-subdev.c         |   3 +-
>  include/uapi/linux/media-bus-format.h         |   9 +
>  include/uapi/linux/media.h                    |   1 +
>  include/uapi/linux/videodev2.h                |  19 +
>  15 files changed, 701 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/userspace-api/media/v4l/metafmt-generic.rst

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] Generic line based metadata support, internal pads
  2023-07-04 12:57 ` [PATCH 0/7] Generic line based metadata support, internal pads Laurent Pinchart
@ 2023-07-05  8:27   ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-07-05  8:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, tomi.valkeinen, bingbu.cao, hongju.wang, hverkuil

Hi Laurent,

On Tue, Jul 04, 2023 at 03:57:20PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Fri, Jun 30, 2023 at 11:43:31PM +0300, Sakari Ailus wrote:
> > Hi folks,
> > 
> > Here are a few patches to add support generic, line based metadata as well
> > as internal source pads. While the amount of code is not very large, to
> > the contrary it is quite small actually IMO, I presume what this is about
> > and why it is being proposed requires some explaining.
> > 
> > Metadata mbus codes and formats have existed for some time in V4L2. They
> > however have been only used by drivers that produce the data itself and
> > effectively this metadata has always been statistics of some sort (at
> > least when it comes to ISPs). What is different here is that we intend to
> > add support for metadata originating from camera sensors.
> 
> I've just realized that, unless I'm mistaken, we've never documented
> which of the v4l2_mbus_framefmt fields are valid for metadata formats.
> Should this be fixed as part of this series ?

Good point. I'll add a patch to document this. It's bound to be dependent
on mbus code as we don't have (which is a good thing) any indication of
type of the data there.

> 
> > Camera sensors produce different kinds of metadata, embedded data (usually
> > register address--value pairs used to capture the frame, in a more or less
> > sensor specific format), histograms (in a very sensor specific format),
> > dark pixels etc. The number of these formats is probably going to be about
> > as large as image data formats if not larger, as the image data formats
> > are much better standardised but a smaller subset of them will be
> > supported by V4L2, at least initially but possibly much more in the long
> > run.
> > 
> > Having this many device specific formats would be a major problem for all
> > the other drivers along that pipeline (not to mention the users of those
> > drivers), including bridge (e.g. CSI-2 to parallel) but especially CSI-2
> > receiver drivers that have DMA: the poor driver developer would not only
> > need to know camera sensor specific formats but to choose the specific
> > packing of that format suitable for the DMA used by the hardware. It is
> > unlikely many of these would ever get tested while being present on the
> > driver API. Also adding new sensors with new embedded data formats would
> > involve updating all bridge and CSI-2 receiver drivers. I don't expect
> > this to be a workable approach.
> > 
> > Instead what I'm proposing is to use specific metadata formats on the
> > sensor devices only, on internal pads (more about those soon) of the
> > sensors, only visible in the UAPI, and then generic mbus formats along the
> > pipeline and finally generic V4L2 metadata formats on the DMAs (specific
> > to bit depth and packing). This would unsnarl the two, defining what data
> > there is (specific mbus code) and how that is transported and packed
> > (generic mbus codes and V4L2 formats).
> > 
> > The user space would be required to "know" the path of that data from the
> > sensor's internal pad to the V4L2 video node. I do not see this as these
> > devices require at least some knowledge of the pipeline, i.e. hardware at
> > hand. Separating what the data means and how it is packed may even be
> > beneficial: it allows separating code that interprets the data (sensor
> > internal mbus code) from the code that accesses it (packing).
> > 
> > These formats are in practice line based, meaning that there may be
> > padding at the end of the line, depending on the bus as well as the DMA.
> > If non-line based formats are needed, it is always possible to set the
> > "height" field to 1.
> > 
> > The internal (source) pads are an alternative to source routes [1]. The
> > source routes were not universally liked and I do have to say I like
> > re-using existing interface concepts (pads and everything you can do with
> > pads, including access format, selections etc.) wherever it makes sense,
> > instead of duplicating functionality.
> > 
> > Effectively internal source pads behave mostly just like sink pads, but
> > they describe a flow of data that originates from a sub-device instead of
> > arriving to a sub-device. The SUBDEV_S_ROUTING IOCTLs are used to enable
> > and disable routes from internal source pads to sub-device's source pads.
> > The subdev format IOCTLs are usable, too, so one can find which subdev
> > format is available on given internal source pad.
> > 
> > This set depends on these patches:
> > 
> > <URL:https://lore.kernel.org/linux-media/20230505205416.55002-1-sakari.ailus@linux.intel.com/T/#t>
> > 
> > I've also pushed these here and I'll keep updating the branch:
> > 
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=metadata>
> > 
> > Questions and comments are most welcome.
> > 
> > Driver support is to be added, as well as an example.
> 
> Jacopo and I are working on this, including libcamera support.

Thanks, this is much appreciated!

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs
  2023-07-02 11:57   ` Andrey Konovalov
@ 2023-07-05 14:39     ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-07-05 14:39 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

Hi Andrey,

On Sun, Jul 02, 2023 at 02:57:44PM +0300, Andrey Konovalov wrote:
> Hi Sakari,
> 
> On 30.06.2023 23:43, Sakari Ailus wrote:
> > Take the new INTERNAL pad flag into account in validating routing IOCTL
> > argument. Effectively this is a SINK pad in this respect. Due to the union
> > there's no need to check for the particular name.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >   drivers/media/v4l2-core/v4l2-subdev.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 2ec179cd1264..36886a9047c4 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -1787,7 +1787,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
> >   		/* Validate the sink and source pad numbers. */
> >   		if (route->sink_pad >= sd->entity.num_pads ||
> > -		    !(sd->entity.pads[route->sink_pad].flags & MEDIA_PAD_FL_SINK)) {
> > +		    !(sd->entity.pads[route->sink_pad].flags &
> > +		      MEDIA_PAD_FL_SINK)) {
> 
> This is a white space only change now, so maybe this patch could be just
> dropped?

Oops. Well, I can still do the line wrap, but the commit message needs to
change. It's no longer related to internal pads.

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/7] media: mc: Check pad flag validity
  2023-07-03  7:53   ` Jacopo Mondi
@ 2023-07-05 14:40     ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-07-05 14:40 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

On Mon, Jul 03, 2023 at 09:53:14AM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Fri, Jun 30, 2023 at 11:43:32PM +0300, Sakari Ailus wrote:
> > Check the validity of pad flags on entity init. Exactly one of the flags
> > must be set.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/mc/mc-entity.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index 83468d4a440b..4991281dcccc 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >  	struct media_device *mdev = entity->graph_obj.mdev;
> >  	struct media_pad *iter;
> >  	unsigned int i = 0;
> > +	int ret = 0;
> >
> >  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> >  		return -E2BIG;
> > @@ -210,6 +211,14 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >  	media_entity_for_each_pad(entity, iter) {
> >  		iter->entity = entity;
> >  		iter->index = i++;
> > +
> > +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> > +					     MEDIA_PAD_FL_SOURCE))
> > +		    != 1) {
> 
> Fits on the previous line ?

Yes. Thanks!

> 
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> > +
> >  		if (mdev)
> >  			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
> >  					  &iter->graph_obj);
> > @@ -218,7 +227,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >  	if (mdev)
> >  		mutex_unlock(&mdev->graph_mutex);
> >
> > -	return 0;
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(media_entity_pads_init);
> >
> > --
> > 2.39.2
> >

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] media: uapi: v4l: Document source routes
  2023-07-03  7:47   ` Jacopo Mondi
@ 2023-07-06 10:20     ` Sakari Ailus
  2023-07-08 12:13     ` Sakari Ailus
  1 sibling, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-07-06 10:20 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

Hi Jacopo,

Thanks for the review.

On Mon, Jul 03, 2023 at 09:47:37AM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Fri, Jun 30, 2023 at 11:43:35PM +0300, Sakari Ailus wrote:
> > Document how internal pads are used on source routes.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  .../userspace-api/media/v4l/dev-subdev.rst    | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > index a4f1df7093e8..5a46c9a9d352 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > @@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
> >  sub-device and a (pad, stream) pair. For sub-devices that do not support
> >  multiplexed streams the 'stream' field is always 0.
> >
> > +.. _v4l2-subdev-source-routes:
> > +
> > +Source routes
> > +^^^^^^^^^^^^^
> 
> I always found the concept of source routes a bit confusing, should we
> instead just present internal pads ?
> 
> > +
> > +Cases where a single sub-device pad is a source of multiple streams are special
> > +as there is no external sink pad for such a route. In those cases, the sources
> > +of the streams are indicated by source routes that have an internal source pad
> > +as the sink pad of such a route. Internal source pads have the
> > +:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> > +pad flags set.
> 
> All this last part is a little bit hard to parse, not your fault but
> the fact "internal source pads" are actually "SINK" pads is a bit
> confusing ?

Some confusion is bound to remain as we're using the sink end of the route
as a source.

> 
> Can we remove the "source route" concept to avoid mixing source/sink ?
> 
> This can be rewritten as
> 
> Internal pads
> ^^^^^^^^^^^^^
> 
> Cases where a single sub-device pad is a source of multiple streams are special
> as there is no external sink pad for such a route. A typical example is a
> sensor device which produces a video stream and a metadata stream of
> embedded data. To support such cases internal pads are introduced as
> sink pads of such internally generated streams.
> Internal source pads have the :ref:`MEDIA_PAD_FL_INTERNAL
> <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK`` pad flags set.

The latter part of this requires some further work.

> 
> > +Internal source pads have all the properties of a sink pad in such case,
> 
> Also here, "Internal source pads" are actually sinks :)
> 
> I would drop "source" from "Internal source pads"

Yes, I forgot to rework some of the documentation. I agree internal source
pads should no longer be mentioned as we no longer have the INTERNAL_SOURCE
flag.

> 
> 
> > +including formats and selections. The format in this case is the source format
> > +of the stream. An internal pad always has a single stream only (0).
> > +
> > +Generally source routes are not modifiable but they can be activated and
> 
> s/source routes/internal routes/ ?

I still think source routes make sense. But they should probably be added
to glossary somewhere.

> 
> > +deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +<v4l2-subdev-routing-flags>` flag, depending on driver capabilities.
> > +
> >  Interaction between routes, streams, formats and selections
> >  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] media: uapi: v4l: Document source routes
  2023-07-03  7:47   ` Jacopo Mondi
  2023-07-06 10:20     ` Sakari Ailus
@ 2023-07-08 12:13     ` Sakari Ailus
  2023-07-10 12:53       ` Jacopo Mondi
  1 sibling, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2023-07-08 12:13 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

Hi Jacopo,

On Mon, Jul 03, 2023 at 09:47:37AM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Fri, Jun 30, 2023 at 11:43:35PM +0300, Sakari Ailus wrote:
> > Document how internal pads are used on source routes.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  .../userspace-api/media/v4l/dev-subdev.rst    | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > index a4f1df7093e8..5a46c9a9d352 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > @@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
> >  sub-device and a (pad, stream) pair. For sub-devices that do not support
> >  multiplexed streams the 'stream' field is always 0.
> >
> > +.. _v4l2-subdev-source-routes:
> > +
> > +Source routes
> > +^^^^^^^^^^^^^
> 
> I always found the concept of source routes a bit confusing, should we
> instead just present internal pads ?
> 
> > +
> > +Cases where a single sub-device pad is a source of multiple streams are special
> > +as there is no external sink pad for such a route. In those cases, the sources
> > +of the streams are indicated by source routes that have an internal source pad
> > +as the sink pad of such a route. Internal source pads have the
> > +:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> > +pad flags set.
> 
> All this last part is a little bit hard to parse, not your fault but
> the fact "internal source pads" are actually "SINK" pads is a bit
> confusing ?
> 
> Can we remove the "source route" concept to avoid mixing source/sink ?
> 
> This can be rewritten as
> 
> Internal pads
> ^^^^^^^^^^^^^
> 
> Cases where a single sub-device pad is a source of multiple streams are special
> as there is no external sink pad for such a route. A typical example is a
> sensor device which produces a video stream and a metadata stream of
> embedded data. To support such cases internal pads are introduced as
> sink pads of such internally generated streams.
> Internal source pads have the :ref:`MEDIA_PAD_FL_INTERNAL
> <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK`` pad flags set.
> 
> > +Internal source pads have all the properties of a sink pad in such case,
> 
> Also here, "Internal source pads" are actually sinks :)
> 
> I would drop "source" from "Internal source pads"

How about this (compared to the patch):

diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
index 5a46c9a9d352..9d544a29e78a 100644
--- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
+++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
@@ -553,21 +553,23 @@ multiplexed streams the 'stream' field is always 0.
 
 .. _v4l2-subdev-source-routes:
 
-Source routes
-^^^^^^^^^^^^^
-
-Cases where a single sub-device pad is a source of multiple streams are special
-as there is no external sink pad for such a route. In those cases, the sources
-of the streams are indicated by source routes that have an internal source pad
-as the sink pad of such a route. Internal source pads have the
-:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
-pad flags set.
-
-Internal source pads have all the properties of a sink pad in such case,
-including formats and selections. The format in this case is the source format
-of the stream. An internal pad always has a single stream only (0).
-
-Generally source routes are not modifiable but they can be activated and
+Internal pads and source routes
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Cases where a single sub-device source pad is traversed by multiple streams one
+or more of which originate from within the sub-device itself are special as
+there is no external sink pad for such routes. In those cases, the sources of
+the internally generated streams are indicated by internal pads, pads which have
+a :ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set on the sink
+pad of the route. A typical use case for these is a camera sensor device which
+produces a pixel data stream and an embedded data stream.
+
+Internal pads have all the properties of an external pad, including formats and
+selections. The format in this case is the source format of the stream. An
+internal pad always has a single stream only (0).
+
+/Source routes/ are routes from an internal sink pad to a(n external) source
+pad. Generally source routes are not modifiable but they can be activated and
 deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
 <v4l2-subdev-routing-flags>` flag, depending on driver capabilities.

I'll also check the ACTIVE route flag, it wasn't merged with the rest of
the Tomi's streams series.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] media: uapi: v4l: Document source routes
  2023-07-08 12:13     ` Sakari Ailus
@ 2023-07-10 12:53       ` Jacopo Mondi
  2023-08-01 14:56         ` Sakari Ailus
  0 siblings, 1 reply; 19+ messages in thread
From: Jacopo Mondi @ 2023-07-10 12:53 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Jacopo Mondi, linux-media, Laurent Pinchart, tomi.valkeinen,
	bingbu.cao, hongju.wang, hverkuil

Hi Sakari

On Sat, Jul 08, 2023 at 12:13:17PM +0000, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Mon, Jul 03, 2023 at 09:47:37AM +0200, Jacopo Mondi wrote:
> > Hi Sakari
> >
> > On Fri, Jun 30, 2023 at 11:43:35PM +0300, Sakari Ailus wrote:
> > > Document how internal pads are used on source routes.
> > >
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  .../userspace-api/media/v4l/dev-subdev.rst    | 20 +++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > index a4f1df7093e8..5a46c9a9d352 100644
> > > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > @@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
> > >  sub-device and a (pad, stream) pair. For sub-devices that do not support
> > >  multiplexed streams the 'stream' field is always 0.
> > >
> > > +.. _v4l2-subdev-source-routes:
> > > +
> > > +Source routes
> > > +^^^^^^^^^^^^^
> >
> > I always found the concept of source routes a bit confusing, should we
> > instead just present internal pads ?
> >
> > > +
> > > +Cases where a single sub-device pad is a source of multiple streams are special
> > > +as there is no external sink pad for such a route. In those cases, the sources
> > > +of the streams are indicated by source routes that have an internal source pad
> > > +as the sink pad of such a route. Internal source pads have the
> > > +:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> > > +pad flags set.
> >
> > All this last part is a little bit hard to parse, not your fault but
> > the fact "internal source pads" are actually "SINK" pads is a bit
> > confusing ?
> >
> > Can we remove the "source route" concept to avoid mixing source/sink ?
> >
> > This can be rewritten as
> >
> > Internal pads
> > ^^^^^^^^^^^^^
> >
> > Cases where a single sub-device pad is a source of multiple streams are special
> > as there is no external sink pad for such a route. A typical example is a
> > sensor device which produces a video stream and a metadata stream of
> > embedded data. To support such cases internal pads are introduced as
> > sink pads of such internally generated streams.
> > Internal source pads have the :ref:`MEDIA_PAD_FL_INTERNAL
> > <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK`` pad flags set.
> >
> > > +Internal source pads have all the properties of a sink pad in such case,
> >
> > Also here, "Internal source pads" are actually sinks :)
> >
> > I would drop "source" from "Internal source pads"
>
> How about this (compared to the patch):
>
> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> index 5a46c9a9d352..9d544a29e78a 100644
> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> @@ -553,21 +553,23 @@ multiplexed streams the 'stream' field is always 0.
>
>  .. _v4l2-subdev-source-routes:
>
> -Source routes
> -^^^^^^^^^^^^^
> -
> -Cases where a single sub-device pad is a source of multiple streams are special
> -as there is no external sink pad for such a route. In those cases, the sources
> -of the streams are indicated by source routes that have an internal source pad
> -as the sink pad of such a route. Internal source pads have the
> -:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> -pad flags set.
> -
> -Internal source pads have all the properties of a sink pad in such case,
> -including formats and selections. The format in this case is the source format
> -of the stream. An internal pad always has a single stream only (0).
> -
> -Generally source routes are not modifiable but they can be activated and
> +Internal pads and source routes
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Cases where a single sub-device source pad is traversed by multiple streams one
> +or more of which originate from within the sub-device itself are special as
> +there is no external sink pad for such routes. In those cases, the sources of

"external" made me think of "a different entity". Can we just drop it
or am I too easy to confuse ?

> +the internally generated streams are indicated by internal pads, pads which have

s/indicated by/represented by/ ?

'internal pads, pads...'

or

/internal pads, which are sink pads with the
:ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set.'

and drop the "on the sink pad of the route" ?


> +a :ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set on the sink
> +pad of the route. A typical use case for these is a camera sensor device which
> +produces a pixel data stream and an embedded data stream.

This case is represented with two internal sink pads for the video and
the data streams and a single multiplexed source pad that connects to
the next entity in the pipeline.

> +
> +Internal pads have all the properties of an external pad, including formats and
> +selections. The format in this case is the source format of the stream. An
> +internal pad always has a single stream only (0).
> +
> +/Source routes/ are routes from an internal sink pad to a(n external) source
> +pad. Generally source routes are not modifiable but they can be activated and
>  deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
>  <v4l2-subdev-routing-flags>` flag, depending on driver capabilities.

If you want to keep the part about source routes, this form is ok with
me!

>
> I'll also check the ACTIVE route flag, it wasn't merged with the rest of
> the Tomi's streams series.
>
> --
> Kind regards,
>
> Sakari Ailus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] media: uapi: v4l: Document source routes
  2023-07-10 12:53       ` Jacopo Mondi
@ 2023-08-01 14:56         ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-08-01 14:56 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: linux-media, Laurent Pinchart, tomi.valkeinen, bingbu.cao,
	hongju.wang, hverkuil

Hi Jacopo,

On Mon, Jul 10, 2023 at 02:53:50PM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Sat, Jul 08, 2023 at 12:13:17PM +0000, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Mon, Jul 03, 2023 at 09:47:37AM +0200, Jacopo Mondi wrote:
> > > Hi Sakari
> > >
> > > On Fri, Jun 30, 2023 at 11:43:35PM +0300, Sakari Ailus wrote:
> > > > Document how internal pads are used on source routes.
> > > >
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > >  .../userspace-api/media/v4l/dev-subdev.rst    | 20 +++++++++++++++++++
> > > >  1 file changed, 20 insertions(+)
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > > index a4f1df7093e8..5a46c9a9d352 100644
> > > > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > > @@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
> > > >  sub-device and a (pad, stream) pair. For sub-devices that do not support
> > > >  multiplexed streams the 'stream' field is always 0.
> > > >
> > > > +.. _v4l2-subdev-source-routes:
> > > > +
> > > > +Source routes
> > > > +^^^^^^^^^^^^^
> > >
> > > I always found the concept of source routes a bit confusing, should we
> > > instead just present internal pads ?
> > >
> > > > +
> > > > +Cases where a single sub-device pad is a source of multiple streams are special
> > > > +as there is no external sink pad for such a route. In those cases, the sources
> > > > +of the streams are indicated by source routes that have an internal source pad
> > > > +as the sink pad of such a route. Internal source pads have the
> > > > +:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> > > > +pad flags set.
> > >
> > > All this last part is a little bit hard to parse, not your fault but
> > > the fact "internal source pads" are actually "SINK" pads is a bit
> > > confusing ?
> > >
> > > Can we remove the "source route" concept to avoid mixing source/sink ?
> > >
> > > This can be rewritten as
> > >
> > > Internal pads
> > > ^^^^^^^^^^^^^
> > >
> > > Cases where a single sub-device pad is a source of multiple streams are special
> > > as there is no external sink pad for such a route. A typical example is a
> > > sensor device which produces a video stream and a metadata stream of
> > > embedded data. To support such cases internal pads are introduced as
> > > sink pads of such internally generated streams.
> > > Internal source pads have the :ref:`MEDIA_PAD_FL_INTERNAL
> > > <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK`` pad flags set.
> > >
> > > > +Internal source pads have all the properties of a sink pad in such case,
> > >
> > > Also here, "Internal source pads" are actually sinks :)
> > >
> > > I would drop "source" from "Internal source pads"
> >
> > How about this (compared to the patch):
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > index 5a46c9a9d352..9d544a29e78a 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > @@ -553,21 +553,23 @@ multiplexed streams the 'stream' field is always 0.
> >
> >  .. _v4l2-subdev-source-routes:
> >
> > -Source routes
> > -^^^^^^^^^^^^^
> > -
> > -Cases where a single sub-device pad is a source of multiple streams are special
> > -as there is no external sink pad for such a route. In those cases, the sources
> > -of the streams are indicated by source routes that have an internal source pad
> > -as the sink pad of such a route. Internal source pads have the
> > -:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> > -pad flags set.
> > -
> > -Internal source pads have all the properties of a sink pad in such case,
> > -including formats and selections. The format in this case is the source format
> > -of the stream. An internal pad always has a single stream only (0).
> > -
> > -Generally source routes are not modifiable but they can be activated and
> > +Internal pads and source routes
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Cases where a single sub-device source pad is traversed by multiple streams one
> > +or more of which originate from within the sub-device itself are special as
> > +there is no external sink pad for such routes. In those cases, the sources of
> 
> "external" made me think of "a different entity". Can we just drop it
> or am I too easy to confuse ?

Now that we have an INTERNAL flag, we need to differentiate with internal
pads and external pads. Routes are inherently internal to the entity, after
all, so there should be little room for confusion.

And there is a pad, just that it happens to have the INTERNAL flag...

> 
> > +the internally generated streams are indicated by internal pads, pads which have
> 
> s/indicated by/represented by/ ?
> 
> 'internal pads, pads...'
> 
> or
> 
> /internal pads, which are sink pads with the
> :ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set.'
> 
> and drop the "on the sink pad of the route" ?

How about:

	In those cases, the sources of the internally generated streams are
	indicated by internal sink pads, which are sink pads that have the
	:ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set.

The intent was to explain the properties of the internal pads.

> 
> 
> > +a :ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set on the sink
> > +pad of the route. A typical use case for these is a camera sensor device which
> > +produces a pixel data stream and an embedded data stream.
> 
> This case is represented with two internal sink pads for the video and
> the data streams and a single multiplexed source pad that connects to
> the next entity in the pipeline.

How about:

	This case is represented with two internal sink pads, one for the
	video and another for the embedded data streams and a single source
	pad that connects to the next entity in the pipeline.

I feel this can, as the previous sentence, moved to the example section.

> 
> > +
> > +Internal pads have all the properties of an external pad, including formats and
> > +selections. The format in this case is the source format of the stream. An
> > +internal pad always has a single stream only (0).
> > +
> > +/Source routes/ are routes from an internal sink pad to a(n external) source
> > +pad. Generally source routes are not modifiable but they can be activated and
> >  deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
> >  <v4l2-subdev-routing-flags>` flag, depending on driver capabilities.
> 
> If you want to keep the part about source routes, this form is ok with
> me!

Thanks. I think it'd be useful to be able to refer to something like
"source routes", as the alternative is to try to refer to "internal source
pads" that have the sink pad flag set...

> 
> >
> > I'll also check the ACTIVE route flag, it wasn't merged with the rest of
> > the Tomi's streams series.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-08-01 14:57 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-30 20:43 [PATCH 0/7] Generic line based metadata support, internal pads Sakari Ailus
2023-06-30 20:43 ` [PATCH 1/7] media: mc: Check pad flag validity Sakari Ailus
2023-07-03  7:53   ` Jacopo Mondi
2023-07-05 14:40     ` Sakari Ailus
2023-06-30 20:43 ` [PATCH 2/7] media: mc: Add INTERNAL pad flag Sakari Ailus
2023-06-30 20:43 ` [PATCH 3/7] media: v4l: subdev: Support INTERNAL pads in routing IOCTLs Sakari Ailus
2023-07-02 11:57   ` Andrey Konovalov
2023-07-05 14:39     ` Sakari Ailus
2023-06-30 20:43 ` [PATCH 4/7] media: uapi: v4l: Document source routes Sakari Ailus
2023-07-03  7:47   ` Jacopo Mondi
2023-07-06 10:20     ` Sakari Ailus
2023-07-08 12:13     ` Sakari Ailus
2023-07-10 12:53       ` Jacopo Mondi
2023-08-01 14:56         ` Sakari Ailus
2023-06-30 20:43 ` [PATCH 5/7] media: uapi: Add generic serial metadata mbus formats Sakari Ailus
2023-06-30 20:43 ` [PATCH 6/7] media: uapi: Add generic 8-bit metadata format definitions Sakari Ailus
2023-06-30 20:43 ` [PATCH 7/7] media: v4l: Support line-based metadata capture Sakari Ailus
2023-07-04 12:57 ` [PATCH 0/7] Generic line based metadata support, internal pads Laurent Pinchart
2023-07-05  8:27   ` Sakari Ailus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.