linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
@ 2020-03-19  0:46 Laurent Pinchart
  2020-03-19  0:46 ` [PATCH v6 1/5] " Laurent Pinchart
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-19  0:46 UTC (permalink / raw)
  To: linux-media
  Cc: Helen Koike, Hans Verkuil, Sakari Ailus, linux-renesas-soc,
	Niklas Söderlund

Hello,

This is the sixth version of Niklas' eponymous series that aims to
reduce the amount of boiler plate code in video device drivers who's
inputs and/or outputs are controlled by the Media Controller instead of
the V4L2 API. I have merged it with the VIDIOC_ENUM_FMT extension for
MC-centric devices that was posted in [1], as the two are related.

Patch 1/5 adds the new video device capability flag V4L2_CAP_IO_MC and
is unchanged compared to Niklas' v5. Patch 2/5 is the rebased version of
the VIDIOC_ENUM_FMT extension that now depends on the V4L2_CAP_IO_MC
flag.

Patches 3/5, 4/5 and 5/5 converts the R-Car VIN, Intel IPU3 and VIMC
drivers to use the new default handlers and capability flag and delete
the now redundant boiler plate code. The IPU3 and VIMC drivers also
implement the VIDIOC_ENUM_FMT extension. This should be added to the
R-Car VIN driver in patch 3/5, that Niklas has nicely proposed to handle
for me :-)

A separate patch to v4l-utils has been posted ([2]) to add a test for
the V4L2_CAP_IO_MC feature to v4l2-compliance. Once the VIDIOC_ENUM_FMT
extension will stabilize, I will do the same for it.

[1] https://lore.kernel.org/linux-media/20200313152406.13347-1-laurent.pinchart@ideasonboard.com/
[2] https://lore.kernel.org/linux-media/20200318132722.3089925-1-niklas.soderlund+renesas@ragnatech.se/

Laurent Pinchart (1):
  media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices

Niklas Söderlund (4):
  v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
  rcar-vin: Make use of V4L2_CAP_IO_MC
  staging/intel-ipu3: Make use of V4L2_CAP_IO_MC
  vimc: Make use of V4L2_CAP_IO_MC

 .../media/uapi/v4l/vidioc-enum-fmt.rst        | 19 +++--
 .../media/uapi/v4l/vidioc-querycap.rst        |  6 ++
 .../media/videodev2.h.rst.exceptions          |  1 +
 drivers/media/platform/rcar-vin/rcar-v4l2.c   | 17 +----
 drivers/media/platform/vimc/vimc-capture.c    | 10 ++-
 drivers/media/v4l2-core/v4l2-dev.c            | 25 +++++--
 drivers/media/v4l2-core/v4l2-ioctl.c          | 70 +++++++++++++++++--
 drivers/staging/media/ipu3/ipu3-v4l2.c        | 64 ++---------------
 include/uapi/linux/videodev2.h                |  5 +-
 9 files changed, 125 insertions(+), 92 deletions(-)

-- 
Regards,

Laurent Pinchart


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

* [PATCH v6 1/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
  2020-03-19  0:46 [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Laurent Pinchart
@ 2020-03-19  0:46 ` Laurent Pinchart
  2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-19  0:46 UTC (permalink / raw)
  To: linux-media
  Cc: Helen Koike, Hans Verkuil, Sakari Ailus, linux-renesas-soc,
	Niklas Söderlund

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Add a video device capability flag to indicate that its inputs and/or
outputs are controlled by the Media Controller instead of the V4L2 API.
When this flag is set, ioctl for enum inputs and outputs are
automatically enabled and programmed to call a helper function.

Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../media/uapi/v4l/vidioc-querycap.rst        |  6 ++
 .../media/videodev2.h.rst.exceptions          |  1 +
 drivers/media/v4l2-core/v4l2-dev.c            | 25 ++++++--
 drivers/media/v4l2-core/v4l2-ioctl.c          | 57 +++++++++++++++++--
 include/uapi/linux/videodev2.h                |  2 +
 5 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 5f9930195d62..497a6aa2cbeb 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -264,6 +264,12 @@ specification the ioctl returns an ``EINVAL`` error code.
     * - ``V4L2_CAP_TOUCH``
       - 0x10000000
       - This is a touch device.
+    * - ``V4L2_CAP_IO_MC``
+      - 0x20000000
+      - There is only one input and/or output seen from userspace. The whole
+        video topology configuration, including which I/O entity is routed to
+        the input/output, is configured by userspace via the Media Controller.
+        See :ref:`media_controller`.
     * - ``V4L2_CAP_DEVICE_CAPS``
       - 0x80000000
       - The driver fills the ``device_caps`` field. This capability can
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index cb6ccf91776e..a625fb90e3a9 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -176,6 +176,7 @@ replace define V4L2_CAP_STREAMING device-capabilities
 replace define V4L2_CAP_META_OUTPUT device-capabilities
 replace define V4L2_CAP_DEVICE_CAPS device-capabilities
 replace define V4L2_CAP_TOUCH device-capabilities
+replace define V4L2_CAP_IO_MC device-capabilities
 
 # V4L2 pix flags
 replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index da42d172714a..a6f1dca08cbd 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -552,6 +552,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		       (vdev->device_caps & meta_caps);
 	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
+	bool is_io_mc = vdev->device_caps & V4L2_CAP_IO_MC;
 
 	bitmap_zero(valid_ioctls, BASE_VIDIOC_PRIVATE);
 
@@ -725,9 +726,15 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_G_STD, vidioc_g_std);
 		if (is_rx) {
 			SET_VALID_IOCTL(ops, VIDIOC_QUERYSTD, vidioc_querystd);
-			SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
-			SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
-			SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
+			if (is_io_mc && !is_meta) {
+				set_bit(_IOC_NR(VIDIOC_ENUMINPUT), valid_ioctls);
+				set_bit(_IOC_NR(VIDIOC_G_INPUT), valid_ioctls);
+				set_bit(_IOC_NR(VIDIOC_S_INPUT), valid_ioctls);
+			} else {
+				SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
+				SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
+				SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
+			}
 			SET_VALID_IOCTL(ops, VIDIOC_ENUMAUDIO, vidioc_enumaudio);
 			SET_VALID_IOCTL(ops, VIDIOC_G_AUDIO, vidioc_g_audio);
 			SET_VALID_IOCTL(ops, VIDIOC_S_AUDIO, vidioc_s_audio);
@@ -735,9 +742,15 @@ static void determine_valid_ioctls(struct video_device *vdev)
 			SET_VALID_IOCTL(ops, VIDIOC_S_EDID, vidioc_s_edid);
 		}
 		if (is_tx) {
-			SET_VALID_IOCTL(ops, VIDIOC_ENUMOUTPUT, vidioc_enum_output);
-			SET_VALID_IOCTL(ops, VIDIOC_G_OUTPUT, vidioc_g_output);
-			SET_VALID_IOCTL(ops, VIDIOC_S_OUTPUT, vidioc_s_output);
+			if (is_io_mc && !is_meta) {
+				set_bit(_IOC_NR(VIDIOC_ENUMOUTPUT), valid_ioctls);
+				set_bit(_IOC_NR(VIDIOC_G_OUTPUT), valid_ioctls);
+				set_bit(_IOC_NR(VIDIOC_S_OUTPUT), valid_ioctls);
+			} else {
+				SET_VALID_IOCTL(ops, VIDIOC_ENUMOUTPUT, vidioc_enum_output);
+				SET_VALID_IOCTL(ops, VIDIOC_G_OUTPUT, vidioc_g_output);
+				SET_VALID_IOCTL(ops, VIDIOC_S_OUTPUT, vidioc_s_output);
+			}
 			SET_VALID_IOCTL(ops, VIDIOC_ENUMAUDOUT, vidioc_enumaudout);
 			SET_VALID_IOCTL(ops, VIDIOC_G_AUDOUT, vidioc_g_audout);
 			SET_VALID_IOCTL(ops, VIDIOC_S_AUDOUT, vidioc_s_audout);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index aaf83e254272..3545a8adf844 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1085,6 +1085,32 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
 	return ret;
 }
 
+static int v4l_g_input(const struct v4l2_ioctl_ops *ops,
+		       struct file *file, void *fh, void *arg)
+{
+	struct video_device *vfd = video_devdata(file);
+
+	if (vfd->device_caps & V4L2_CAP_IO_MC) {
+		*(int *)arg = 0;
+		return 0;
+	}
+
+	return ops->vidioc_g_input(file, fh, arg);
+}
+
+static int v4l_g_output(const struct v4l2_ioctl_ops *ops,
+		       struct file *file, void *fh, void *arg)
+{
+	struct video_device *vfd = video_devdata(file);
+
+	if (vfd->device_caps & V4L2_CAP_IO_MC) {
+		*(int *)arg = 0;
+		return 0;
+	}
+
+	return ops->vidioc_g_output(file, fh, arg);
+}
+
 static int v4l_s_input(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
@@ -1094,12 +1120,21 @@ static int v4l_s_input(const struct v4l2_ioctl_ops *ops,
 	ret = v4l_enable_media_source(vfd);
 	if (ret)
 		return ret;
+
+	if (vfd->device_caps & V4L2_CAP_IO_MC)
+		return  *(int *)arg ? -EINVAL : 0;
+
 	return ops->vidioc_s_input(file, fh, *(unsigned int *)arg);
 }
 
 static int v4l_s_output(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
+	struct video_device *vfd = video_devdata(file);
+
+	if (vfd->device_caps & V4L2_CAP_IO_MC)
+		return  *(int *)arg ? -EINVAL : 0;
+
 	return ops->vidioc_s_output(file, fh, *(unsigned int *)arg);
 }
 
@@ -1143,6 +1178,14 @@ static int v4l_enuminput(const struct v4l2_ioctl_ops *ops,
 	if (is_valid_ioctl(vfd, VIDIOC_S_STD))
 		p->capabilities |= V4L2_IN_CAP_STD;
 
+	if (vfd->device_caps & V4L2_CAP_IO_MC) {
+		if (p->index)
+			return -EINVAL;
+		strscpy(p->name, vfd->name, sizeof(p->name));
+		p->type = V4L2_INPUT_TYPE_CAMERA;
+		return 0;
+	}
+
 	return ops->vidioc_enum_input(file, fh, p);
 }
 
@@ -1161,6 +1204,14 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops,
 	if (is_valid_ioctl(vfd, VIDIOC_S_STD))
 		p->capabilities |= V4L2_OUT_CAP_STD;
 
+	if (vfd->device_caps & V4L2_CAP_IO_MC) {
+		if (p->index)
+			return -EINVAL;
+		strscpy(p->name, vfd->name, sizeof(p->name));
+		p->type = V4L2_OUTPUT_TYPE_ANALOG;
+		return 0;
+	}
+
 	return ops->vidioc_enum_output(file, fh, p);
 }
 
@@ -2678,10 +2729,8 @@ DEFINE_V4L_STUB_FUNC(expbuf)
 DEFINE_V4L_STUB_FUNC(g_std)
 DEFINE_V4L_STUB_FUNC(g_audio)
 DEFINE_V4L_STUB_FUNC(s_audio)
-DEFINE_V4L_STUB_FUNC(g_input)
 DEFINE_V4L_STUB_FUNC(g_edid)
 DEFINE_V4L_STUB_FUNC(s_edid)
-DEFINE_V4L_STUB_FUNC(g_output)
 DEFINE_V4L_STUB_FUNC(g_audout)
 DEFINE_V4L_STUB_FUNC(s_audout)
 DEFINE_V4L_STUB_FUNC(g_jpegcomp)
@@ -2730,11 +2779,11 @@ static const struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO(VIDIOC_S_AUDIO, v4l_stub_s_audio, v4l_print_audio, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_QUERYCTRL, v4l_queryctrl, v4l_print_queryctrl, INFO_FL_CTRL | INFO_FL_CLEAR(v4l2_queryctrl, id)),
 	IOCTL_INFO(VIDIOC_QUERYMENU, v4l_querymenu, v4l_print_querymenu, INFO_FL_CTRL | INFO_FL_CLEAR(v4l2_querymenu, index)),
-	IOCTL_INFO(VIDIOC_G_INPUT, v4l_stub_g_input, v4l_print_u32, 0),
+	IOCTL_INFO(VIDIOC_G_INPUT, v4l_g_input, v4l_print_u32, 0),
 	IOCTL_INFO(VIDIOC_S_INPUT, v4l_s_input, v4l_print_u32, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_G_EDID, v4l_stub_g_edid, v4l_print_edid, INFO_FL_ALWAYS_COPY),
 	IOCTL_INFO(VIDIOC_S_EDID, v4l_stub_s_edid, v4l_print_edid, INFO_FL_PRIO | INFO_FL_ALWAYS_COPY),
-	IOCTL_INFO(VIDIOC_G_OUTPUT, v4l_stub_g_output, v4l_print_u32, 0),
+	IOCTL_INFO(VIDIOC_G_OUTPUT, v4l_g_output, v4l_print_u32, 0),
 	IOCTL_INFO(VIDIOC_S_OUTPUT, v4l_s_output, v4l_print_u32, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_ENUMOUTPUT, v4l_enumoutput, v4l_print_enumoutput, INFO_FL_CLEAR(v4l2_output, index)),
 	IOCTL_INFO(VIDIOC_G_AUDOUT, v4l_stub_g_audout, v4l_print_audioout, 0),
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 5f9357dcb060..c793263a3705 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -487,6 +487,8 @@ struct v4l2_capability {
 
 #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
 
+#define V4L2_CAP_IO_MC			0x20000000  /* Is input/output controlled by the media controller */
+
 #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
 
 /*
-- 
Regards,

Laurent Pinchart


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

* [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-19  0:46 [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Laurent Pinchart
  2020-03-19  0:46 ` [PATCH v6 1/5] " Laurent Pinchart
@ 2020-03-19  0:46 ` Laurent Pinchart
  2020-03-19  3:42   ` kbuild test robot
                     ` (3 more replies)
  2020-03-19  0:46 ` [PATCH v6 3/5] rcar-vin: Make use of V4L2_CAP_IO_MC Laurent Pinchart
                   ` (2 subsequent siblings)
  4 siblings, 4 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-19  0:46 UTC (permalink / raw)
  To: linux-media
  Cc: Helen Koike, Hans Verkuil, Sakari Ailus, linux-renesas-soc,
	Niklas Söderlund

The VIDIOC_ENUM_FMT ioctl enumerates all formats supported by a video
node. For MC-centric devices, its behaviour has always been ill-defined,
with drivers implementing one of the following behaviours:

- No support for VIDIOC_ENUM_FMT at all
- Enumerating all formats supported by the video node, regardless of the
  configuration of the pipeline
- Enumerating formats supported by the video node for the active
  configuration of the connected subdevice

The first behaviour is obviously useless for applications. The second
behaviour provides the most information, but doesn't offer a way to find
what formats are compatible with a given pipeline configuration. The
third behaviour fixes that, but with the drawback that applications
can't enumerate all supported formats anymore, and have to modify the
active configuration of the pipeline to enumerate formats.

The situation is messy as none of the implemented behaviours are ideal,
and userspace can't predict what will happen as the behaviour is
driver-specific.

To fix this, let's extend the VIDIOC_ENUM_FMT with a missing capability:
enumerating pixel formats for a given media bus code. The media bus code
is passed through the v4l2_fmtdesc structure in a new mbus_code field
(repurposed from the reserved fields). With this capability in place,
applications can enumerate pixel formats for a given media bus code
without modifying the active configuration of the device.

The current behaviour of the ioctl is preserved when the new mbus_code
field is set to 0, ensuring compatibility with existing userspace. The
API extension is documented as mandatory for MC-centric devices (as
advertised through the V4L2_CAP_IO_MC capability), allowing applications
and compliance tools to easily determine the availability of the
VIDIOC_ENUM_FMT extension.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../media/uapi/v4l/vidioc-enum-fmt.rst        | 19 +++++++++++++++----
 drivers/media/v4l2-core/v4l2-ioctl.c          | 13 +++++++++++--
 include/uapi/linux/videodev2.h                |  3 ++-
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
index 8ca6ab701e4a..82792d8e910b 100644
--- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
@@ -48,10 +48,21 @@ one until ``EINVAL`` is returned. If applicable, drivers shall return
 formats in preference order, where preferred formats are returned before
 (that is, with lower ``index`` value) less-preferred formats.
 
-.. note::
-
-   After switching input or output the list of enumerated image
-   formats may be different.
+If the driver doesn't advertise the ``V4L2_CAP_IO_MC``
+:ref:`device-capabilities <capability>`, applications shall initialize the
+``mbus_code`` field to zero. Drivers shall enumerate all image formats supported
+by the device. The enumerated formats may depend on the active input or output
+of the device.
+
+If the driver advertises the ``V4L2_CAP_IO_MC`` :ref:`device-capabilities
+<capability>`, applications may initialize the ``mbus_code`` to a valid
+:ref:`v4l2_mbus_pixelcode <media bus format code>`. If the ``mbus_code` field
+is not zero, drivers shall restrict enumeration to only the image formats that
+can produce (for video output devices) or be produced from (for video capture
+devices) that media bus code. Regardless of the value of the ``mbus_code``
+field, the enumerated image formats shall not depend on the active
+configuration of the video device or device pipeline. Enumeration shall
+otherwise operate as previously described.
 
 
 .. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 3545a8adf844..b55603964abd 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -264,12 +264,13 @@ static void v4l_print_fmtdesc(const void *arg, bool write_only)
 {
 	const struct v4l2_fmtdesc *p = arg;
 
-	pr_cont("index=%u, type=%s, flags=0x%x, pixelformat=%c%c%c%c, description='%.*s'\n",
+	pr_cont("index=%u, type=%s, flags=0x%x, pixelformat=%c%c%c%c, mbus_code=0x%04x, description='%.*s'\n",
 		p->index, prt_names(p->type, v4l2_type_names),
 		p->flags, (p->pixelformat & 0xff),
 		(p->pixelformat >>  8) & 0xff,
 		(p->pixelformat >> 16) & 0xff,
 		(p->pixelformat >> 24) & 0xff,
+		p->mbus_code,
 		(int)sizeof(p->description), p->description);
 }
 
@@ -1467,12 +1468,20 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_fmtdesc *p = arg;
 	int ret = check_fmt(file, p->type);
+	u32 mbus_code;
 	u32 cap_mask;
 
 	if (ret)
 		return ret;
 	ret = -EINVAL;
 
+	if (p->mbus_code && !(vdev->device_caps & V4L2_CAP_IO_MC))
+		return -EINVAL;
+
+	mbus_code = p->mbus_code;
+	CLEAR_AFTER_FIELD(p, type);
+	p->mbus_code = mbus_code;
+
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
@@ -2752,7 +2761,7 @@ DEFINE_V4L_STUB_FUNC(dv_timings_cap)
 
 static const struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO(VIDIOC_QUERYCAP, v4l_querycap, v4l_print_querycap, 0),
-	IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc, INFO_FL_CLEAR(v4l2_fmtdesc, type)),
+	IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc, 0),
 	IOCTL_INFO(VIDIOC_G_FMT, v4l_g_fmt, v4l_print_format, 0),
 	IOCTL_INFO(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c793263a3705..0ece960844a5 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -779,7 +779,8 @@ struct v4l2_fmtdesc {
 	__u32               flags;
 	__u8		    description[32];   /* Description string */
 	__u32		    pixelformat;       /* Format fourcc      */
-	__u32		    reserved[4];
+	__u32		    mbus_code;		/* Media bus code    */
+	__u32		    reserved[3];
 };
 
 #define V4L2_FMT_FLAG_COMPRESSED		0x0001
-- 
Regards,

Laurent Pinchart


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

* [PATCH v6 3/5] rcar-vin: Make use of V4L2_CAP_IO_MC
  2020-03-19  0:46 [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Laurent Pinchart
  2020-03-19  0:46 ` [PATCH v6 1/5] " Laurent Pinchart
  2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
@ 2020-03-19  0:46 ` Laurent Pinchart
  2020-03-19  0:47 ` [PATCH v6 4/5] staging/intel-ipu3: " Laurent Pinchart
  2020-03-19  0:47 ` [PATCH v6 5/5] vimc: " Laurent Pinchart
  4 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-19  0:46 UTC (permalink / raw)
  To: linux-media
  Cc: Helen Koike, Hans Verkuil, Sakari Ailus, linux-renesas-soc,
	Niklas Söderlund

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Set the V4L2_CAP_IO_MC capability flag and remove the driver specific
vidioc_enum_input, vidioc_g_input and vidioc_s_input callbacks for the
media controller enabled part of the driver.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 5ff565e76bca..f062251250fb 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -751,18 +751,6 @@ static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int rvin_mc_enum_input(struct file *file, void *priv,
-			      struct v4l2_input *i)
-{
-	if (i->index != 0)
-		return -EINVAL;
-
-	i->type = V4L2_INPUT_TYPE_CAMERA;
-	strscpy(i->name, "Camera", sizeof(i->name));
-
-	return 0;
-}
-
 static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
 	.vidioc_querycap		= rvin_querycap,
 	.vidioc_try_fmt_vid_cap		= rvin_mc_try_fmt_vid_cap,
@@ -770,10 +758,6 @@ static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap		= rvin_mc_s_fmt_vid_cap,
 	.vidioc_enum_fmt_vid_cap	= rvin_enum_fmt_vid_cap,
 
-	.vidioc_enum_input		= rvin_mc_enum_input,
-	.vidioc_g_input			= rvin_g_input,
-	.vidioc_s_input			= rvin_s_input,
-
 	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
 	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
 	.vidioc_querybuf		= vb2_ioctl_querybuf,
@@ -945,6 +929,7 @@ int rvin_v4l2_register(struct rvin_dev *vin)
 	vin->format.colorspace = RVIN_DEFAULT_COLORSPACE;
 
 	if (vin->info->use_mc) {
+		vdev->device_caps |= V4L2_CAP_IO_MC;
 		vdev->ioctl_ops = &rvin_mc_ioctl_ops;
 	} else {
 		vdev->ioctl_ops = &rvin_ioctl_ops;
-- 
Regards,

Laurent Pinchart


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

* [PATCH v6 4/5] staging/intel-ipu3: Make use of V4L2_CAP_IO_MC
  2020-03-19  0:46 [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Laurent Pinchart
                   ` (2 preceding siblings ...)
  2020-03-19  0:46 ` [PATCH v6 3/5] rcar-vin: Make use of V4L2_CAP_IO_MC Laurent Pinchart
@ 2020-03-19  0:47 ` Laurent Pinchart
  2020-03-19  0:47 ` [PATCH v6 5/5] vimc: " Laurent Pinchart
  4 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-19  0:47 UTC (permalink / raw)
  To: linux-media
  Cc: Helen Koike, Hans Verkuil, Sakari Ailus, linux-renesas-soc,
	Niklas Söderlund

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Set the V4L2_CAP_IO_MC capability flag and remove the driver specific
vidioc_enum_{input,output}, vidioc_g_{input,output} and
vidioc_s_{input,output} callbacks.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v5:

- Implement mbus_code filtering for format enumeration
---
 drivers/staging/media/ipu3/ipu3-v4l2.c | 64 +++-----------------------
 1 file changed, 7 insertions(+), 57 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
index 569e27b824c8..b39200fe90c9 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -597,6 +597,9 @@ static int enum_fmts(struct v4l2_fmtdesc *f, u32 type)
 {
 	unsigned int i, j;
 
+	if (f->mbus_code != 0 && f->mbus_code != MEDIA_BUS_FMT_FIXED)
+		return -EINVAL;
+
 	for (i = j = 0; i < ARRAY_SIZE(formats); ++i) {
 		if (formats[i].type == type) {
 			if (j == f->index)
@@ -826,6 +829,9 @@ static int imgu_meta_enum_format(struct file *file, void *fh,
 	if (fmt->index > 0 || fmt->type != node->vbq.type)
 		return -EINVAL;
 
+	if (fmt->mbus_code != 0 && fmt->mbus_code != MEDIA_BUS_FMT_FIXED)
+		return -EINVAL;
+
 	strscpy(fmt->description, meta_fmts[i].name, sizeof(fmt->description));
 	fmt->pixelformat = meta_fmts[i].fourcc;
 
@@ -845,54 +851,6 @@ static int imgu_vidioc_g_meta_fmt(struct file *file, void *fh,
 	return 0;
 }
 
-static int imgu_vidioc_enum_input(struct file *file, void *fh,
-				  struct v4l2_input *input)
-{
-	if (input->index > 0)
-		return -EINVAL;
-	strscpy(input->name, "camera", sizeof(input->name));
-	input->type = V4L2_INPUT_TYPE_CAMERA;
-
-	return 0;
-}
-
-static int imgu_vidioc_g_input(struct file *file, void *fh, unsigned int *input)
-{
-	*input = 0;
-
-	return 0;
-}
-
-static int imgu_vidioc_s_input(struct file *file, void *fh, unsigned int input)
-{
-	return input == 0 ? 0 : -EINVAL;
-}
-
-static int imgu_vidioc_enum_output(struct file *file, void *fh,
-				   struct v4l2_output *output)
-{
-	if (output->index > 0)
-		return -EINVAL;
-	strscpy(output->name, "camera", sizeof(output->name));
-	output->type = V4L2_INPUT_TYPE_CAMERA;
-
-	return 0;
-}
-
-static int imgu_vidioc_g_output(struct file *file, void *fh,
-				unsigned int *output)
-{
-	*output = 0;
-
-	return 0;
-}
-
-static int imgu_vidioc_s_output(struct file *file, void *fh,
-				unsigned int output)
-{
-	return output == 0 ? 0 : -EINVAL;
-}
-
 /******************** function pointers ********************/
 
 static struct v4l2_subdev_internal_ops imgu_subdev_internal_ops = {
@@ -965,14 +923,6 @@ static const struct v4l2_ioctl_ops imgu_v4l2_ioctl_ops = {
 	.vidioc_s_fmt_vid_out_mplane = imgu_vidioc_s_fmt,
 	.vidioc_try_fmt_vid_out_mplane = imgu_vidioc_try_fmt,
 
-	.vidioc_enum_output = imgu_vidioc_enum_output,
-	.vidioc_g_output = imgu_vidioc_g_output,
-	.vidioc_s_output = imgu_vidioc_s_output,
-
-	.vidioc_enum_input = imgu_vidioc_enum_input,
-	.vidioc_g_input = imgu_vidioc_g_input,
-	.vidioc_s_input = imgu_vidioc_s_input,
-
 	/* buffer queue management */
 	.vidioc_reqbufs = vb2_ioctl_reqbufs,
 	.vidioc_create_bufs = vb2_ioctl_create_bufs,
@@ -1086,7 +1036,7 @@ static void imgu_node_to_v4l2(u32 node, struct video_device *vdev,
 		vdev->ioctl_ops = &imgu_v4l2_ioctl_ops;
 	}
 
-	vdev->device_caps = V4L2_CAP_STREAMING | cap;
+	vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_IO_MC | cap;
 }
 
 static int imgu_v4l2_subdev_register(struct imgu_device *imgu,
-- 
Regards,

Laurent Pinchart


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

* [PATCH v6 5/5] vimc: Make use of V4L2_CAP_IO_MC
  2020-03-19  0:46 [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Laurent Pinchart
                   ` (3 preceding siblings ...)
  2020-03-19  0:47 ` [PATCH v6 4/5] staging/intel-ipu3: " Laurent Pinchart
@ 2020-03-19  0:47 ` Laurent Pinchart
  4 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-19  0:47 UTC (permalink / raw)
  To: linux-media
  Cc: Helen Koike, Hans Verkuil, Sakari Ailus, linux-renesas-soc,
	Niklas Söderlund

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Set the V4L2_CAP_IO_MC capability flag to report this vimc
inputs/outputs are controlled by the media graph.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Changes since v5:

- Wrap line longer than 80 characters
- Implement mbus_code filtering for format enumeration
---
 drivers/media/platform/vimc/vimc-capture.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index 76c015898cfd..86b3580a746f 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -146,7 +146,12 @@ static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
 static int vimc_cap_enum_fmt_vid_cap(struct file *file, void *priv,
 				     struct v4l2_fmtdesc *f)
 {
-	const struct vimc_pix_map *vpix = vimc_pix_map_by_index(f->index);
+	const struct vimc_pix_map *vpix;
+
+	if (f->mbus_code)
+		vpix = vimc_pix_map_by_code(f->mbus_code);
+	else
+		vpix = vimc_pix_map_by_index(f->index);
 
 	if (!vpix)
 		return -EINVAL;
@@ -447,7 +452,8 @@ struct vimc_ent_device *vimc_cap_add(struct vimc_device *vimc,
 
 	/* Initialize the video_device struct */
 	vdev = &vcap->vdev;
-	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING
+			  | V4L2_CAP_IO_MC;
 	vdev->entity.ops = &vimc_cap_mops;
 	vdev->release = vimc_cap_release;
 	vdev->fops = &vimc_cap_fops;
-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
@ 2020-03-19  3:42   ` kbuild test robot
  2020-03-19  3:52   ` kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2020-03-19  3:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: kbuild-all, linux-media, Helen Koike, Hans Verkuil, Sakari Ailus,
	linux-renesas-soc, Niklas Söderlund

[-- Attachment #1: Type: text/plain, Size: 8025 bytes --]

Hi Laurent,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v5.6-rc6]
[also build test WARNING on next-20200318]
[cannot apply to linuxtv-media/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/v4l2-dev-ioctl-Add-V4L2_CAP_IO_MC/20200319-084846
base:    fb33c6510d5595144d585aa194d377cf74d31911
config: x86_64-randconfig-s1-20200319 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/media/pci/ivtv/ivtv-ioctl.c: In function 'ivtv_enum_fmt_vid_cap':
>> drivers/media/pci/ivtv/ivtv-ioctl.c:925:3: warning: braces around scalar initializer
      { 0, 0, 0, 0 }
      ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:925:3: note: (near initialization for 'hm12.mbus_code')
>> drivers/media/pci/ivtv/ivtv-ioctl.c:925:8: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
           ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:925:8: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:925:11: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
              ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:925:11: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:925:14: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
                 ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:925:14: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:3: warning: braces around scalar initializer
      { 0, 0, 0, 0 }
      ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:3: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:8: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
           ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:8: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:11: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
              ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:11: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:14: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
                 ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:14: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c: In function 'ivtv_enum_fmt_vid_out':
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:3: warning: braces around scalar initializer
      { 0, 0, 0, 0 }
      ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:3: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:8: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
           ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:8: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:11: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
              ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:11: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:14: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
                 ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:14: note: (near initialization for 'hm12.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:3: warning: braces around scalar initializer
      { 0, 0, 0, 0 }
      ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:3: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:8: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
           ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:8: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:11: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
              ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:11: note: (near initialization for 'mpeg.mbus_code')
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:14: warning: excess elements in scalar initializer
      { 0, 0, 0, 0 }
                 ^
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:14: note: (near initialization for 'mpeg.mbus_code')

vim +925 drivers/media/pci/ivtv/ivtv-ioctl.c

1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  919  
3f038d80039f60 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  920  static int ivtv_enum_fmt_vid_cap(struct file *file, void *fh, struct v4l2_fmtdesc *fmt)
3f038d80039f60 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  921  {
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  922  	static const struct v4l2_fmtdesc hm12 = {
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  923  		0, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
368f080b6870e6 drivers/media/video/ivtv/ivtv-ioctl.c Ian Armstrong 2007-11-05  924  		"HM12 (YUV 4:2:0)", V4L2_PIX_FMT_HM12,
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27 @925  		{ 0, 0, 0, 0 }
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  926  	};
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  927  	static const struct v4l2_fmtdesc mpeg = {
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  928  		0, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FMT_FLAG_COMPRESSED,
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  929  		"MPEG", V4L2_PIX_FMT_MPEG,
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  930  		{ 0, 0, 0, 0 }
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  931  	};
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  932  	struct ivtv *itv = fh2id(fh)->itv;
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  933  	struct ivtv_stream *s = &itv->streams[fh2id(fh)->type];
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  934  
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  935  	if (fmt->index)
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  936  		return -EINVAL;
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  937  	if (s->type == IVTV_ENC_STREAM_TYPE_MPG)
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  938  		*fmt = mpeg;
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  939  	else if (s->type == IVTV_ENC_STREAM_TYPE_YUV)
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  940  		*fmt = hm12;
bfd063cebb75d3 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  941  	else
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  942  		return -EINVAL;
3f038d80039f60 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  943  	return 0;
1a0adaf37c30e8 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  944  }
3f038d80039f60 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  945  

:::::: The code at line 925 was first introduced by commit
:::::: 1a0adaf37c30e89e44d1470ef604a930999a5826 V4L/DVB (5345): ivtv driver for Conexant cx23416/cx23415 MPEG encoder/decoder

:::::: TO: Hans Verkuil <hverkuil@xs4all.nl>
:::::: CC: Mauro Carvalho Chehab <mchehab@infradead.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34085 bytes --]

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

* Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
  2020-03-19  3:42   ` kbuild test robot
@ 2020-03-19  3:52   ` kbuild test robot
  2020-03-19  8:24   ` kbuild test robot
  2020-03-23 10:03   ` Sakari Ailus
  3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2020-03-19  3:52 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: kbuild-all, linux-media, Helen Koike, Hans Verkuil, Sakari Ailus,
	linux-renesas-soc, Niklas Söderlund

[-- Attachment #1: Type: text/plain, Size: 7777 bytes --]

Hi Laurent,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v5.6-rc6]
[also build test WARNING on next-20200318]
[cannot apply to linuxtv-media/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/v4l2-dev-ioctl-Add-V4L2_CAP_IO_MC/20200319-084846
base:    fb33c6510d5595144d585aa194d377cf74d31911
config: parisc-allyesconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/media//pci/cx18/cx18-ioctl.c: In function 'cx18_enum_fmt_vid_cap':
>> drivers/media//pci/cx18/cx18-ioctl.c:470:5: warning: braces around scalar initializer
     470 |     "HM12 (YUV 4:1:1)", V4L2_PIX_FMT_HM12, { 0, 0, 0, 0 }
         |     ^~~~~~~~~~~~~~~~~~
   drivers/media//pci/cx18/cx18-ioctl.c:470:5: note: (near initialization for 'formats[0].mbus_code')
>> drivers/media//pci/cx18/cx18-ioctl.c:470:49: warning: excess elements in scalar initializer
     470 |     "HM12 (YUV 4:1:1)", V4L2_PIX_FMT_HM12, { 0, 0, 0, 0 }
         |                                                 ^
   drivers/media//pci/cx18/cx18-ioctl.c:470:49: note: (near initialization for 'formats[0].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:470:52: warning: excess elements in scalar initializer
     470 |     "HM12 (YUV 4:1:1)", V4L2_PIX_FMT_HM12, { 0, 0, 0, 0 }
         |                                                    ^
   drivers/media//pci/cx18/cx18-ioctl.c:470:52: note: (near initialization for 'formats[0].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:470:55: warning: excess elements in scalar initializer
     470 |     "HM12 (YUV 4:1:1)", V4L2_PIX_FMT_HM12, { 0, 0, 0, 0 }
         |                                                       ^
   drivers/media//pci/cx18/cx18-ioctl.c:470:55: note: (near initialization for 'formats[0].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:473:5: warning: braces around scalar initializer
     473 |     "MPEG", V4L2_PIX_FMT_MPEG, { 0, 0, 0, 0 }
         |     ^~~~~~
   drivers/media//pci/cx18/cx18-ioctl.c:473:5: note: (near initialization for 'formats[1].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:473:37: warning: excess elements in scalar initializer
     473 |     "MPEG", V4L2_PIX_FMT_MPEG, { 0, 0, 0, 0 }
         |                                     ^
   drivers/media//pci/cx18/cx18-ioctl.c:473:37: note: (near initialization for 'formats[1].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:473:40: warning: excess elements in scalar initializer
     473 |     "MPEG", V4L2_PIX_FMT_MPEG, { 0, 0, 0, 0 }
         |                                        ^
   drivers/media//pci/cx18/cx18-ioctl.c:473:40: note: (near initialization for 'formats[1].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:473:43: warning: excess elements in scalar initializer
     473 |     "MPEG", V4L2_PIX_FMT_MPEG, { 0, 0, 0, 0 }
         |                                           ^
   drivers/media//pci/cx18/cx18-ioctl.c:473:43: note: (near initialization for 'formats[1].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:476:5: warning: braces around scalar initializer
     476 |     "UYVY 4:2:2", V4L2_PIX_FMT_UYVY, { 0, 0, 0, 0 }
         |     ^~~~~~~~~~~~
   drivers/media//pci/cx18/cx18-ioctl.c:476:5: note: (near initialization for 'formats[2].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:476:43: warning: excess elements in scalar initializer
     476 |     "UYVY 4:2:2", V4L2_PIX_FMT_UYVY, { 0, 0, 0, 0 }
         |                                           ^
   drivers/media//pci/cx18/cx18-ioctl.c:476:43: note: (near initialization for 'formats[2].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:476:46: warning: excess elements in scalar initializer
     476 |     "UYVY 4:2:2", V4L2_PIX_FMT_UYVY, { 0, 0, 0, 0 }
         |                                              ^
   drivers/media//pci/cx18/cx18-ioctl.c:476:46: note: (near initialization for 'formats[2].mbus_code')
   drivers/media//pci/cx18/cx18-ioctl.c:476:49: warning: excess elements in scalar initializer
     476 |     "UYVY 4:2:2", V4L2_PIX_FMT_UYVY, { 0, 0, 0, 0 }
         |                                                 ^
   drivers/media//pci/cx18/cx18-ioctl.c:476:49: note: (near initialization for 'formats[2].mbus_code')

vim +470 drivers/media//pci/cx18/cx18-ioctl.c

1c1e45d17b663d drivers/media/video/cx18/cx18-ioctl.c Hans Verkuil     2008-04-28  464  
3b6fe58f0f1888 drivers/media/video/cx18/cx18-ioctl.c Andy Walls       2008-06-21  465  static int cx18_enum_fmt_vid_cap(struct file *file, void *fh,
3b6fe58f0f1888 drivers/media/video/cx18/cx18-ioctl.c Andy Walls       2008-06-21  466  					struct v4l2_fmtdesc *fmt)
3b6fe58f0f1888 drivers/media/video/cx18/cx18-ioctl.c Andy Walls       2008-06-21  467  {
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  468  	static const struct v4l2_fmtdesc formats[] = {
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  469  		{ 0, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03 @470  		  "HM12 (YUV 4:1:1)", V4L2_PIX_FMT_HM12, { 0, 0, 0, 0 }
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  471  		},
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  472  		{ 1, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FMT_FLAG_COMPRESSED,
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  473  		  "MPEG", V4L2_PIX_FMT_MPEG, { 0, 0, 0, 0 }
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  474  		},
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  475  		{ 2, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  476  		  "UYVY 4:2:2", V4L2_PIX_FMT_UYVY, { 0, 0, 0, 0 }
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  477  		},
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  478  	};
1bf5842fe3b61d drivers/media/video/cx18/cx18-ioctl.c Simon Farnsworth 2011-05-03  479  
b7101de3fff596 drivers/media/video/cx18/cx18-ioctl.c Steven Toth      2011-04-06  480  	if (fmt->index > ARRAY_SIZE(formats) - 1)
1c1e45d17b663d drivers/media/video/cx18/cx18-ioctl.c Hans Verkuil     2008-04-28  481  		return -EINVAL;
1c1e45d17b663d drivers/media/video/cx18/cx18-ioctl.c Hans Verkuil     2008-04-28  482  	*fmt = formats[fmt->index];
1c1e45d17b663d drivers/media/video/cx18/cx18-ioctl.c Hans Verkuil     2008-04-28  483  	return 0;
1c1e45d17b663d drivers/media/video/cx18/cx18-ioctl.c Hans Verkuil     2008-04-28  484  }
1c1e45d17b663d drivers/media/video/cx18/cx18-ioctl.c Hans Verkuil     2008-04-28  485  

:::::: The code at line 470 was first introduced by commit
:::::: 1bf5842fe3b61d2dbbced96dbd27ad26fe93444a [media] cx18: Clean up mmap() support for raw YUV

:::::: TO: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
:::::: CC: Mauro Carvalho Chehab <mchehab@redhat.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60415 bytes --]

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

* Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
  2020-03-19  3:42   ` kbuild test robot
  2020-03-19  3:52   ` kbuild test robot
@ 2020-03-19  8:24   ` kbuild test robot
  2020-03-23 10:03   ` Sakari Ailus
  3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2020-03-19  8:24 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: kbuild-all, linux-media, Helen Koike, Hans Verkuil, Sakari Ailus,
	linux-renesas-soc, Niklas Söderlund

Hi Laurent,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v5.6-rc6]
[also build test WARNING on next-20200318]
[cannot apply to linuxtv-media/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/v4l2-dev-ioctl-Add-V4L2_CAP_IO_MC/20200319-084846
base:    fb33c6510d5595144d585aa194d377cf74d31911
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-181-g83789bbc-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/media/pci/ivtv/ivtv-ioctl.c:925:17: sparse: sparse: bogus scalar initializer
   drivers/media/pci/ivtv/ivtv-ioctl.c:930:17: sparse: sparse: bogus scalar initializer
   drivers/media/pci/ivtv/ivtv-ioctl.c:951:17: sparse: sparse: bogus scalar initializer
   drivers/media/pci/ivtv/ivtv-ioctl.c:956:17: sparse: sparse: bogus scalar initializer
--
>> drivers/media/pci/cx18/cx18-ioctl.c:470:58: sparse: sparse: bogus scalar initializer
   drivers/media/pci/cx18/cx18-ioctl.c:473:46: sparse: sparse: bogus scalar initializer
   drivers/media/pci/cx18/cx18-ioctl.c:476:52: sparse: sparse: bogus scalar initializer

vim +925 drivers/media/pci/ivtv/ivtv-ioctl.c

1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  919  
3f038d80039f60e drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  920  static int ivtv_enum_fmt_vid_cap(struct file *file, void *fh, struct v4l2_fmtdesc *fmt)
3f038d80039f60e drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  921  {
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  922  	static const struct v4l2_fmtdesc hm12 = {
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  923  		0, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
368f080b6870e65 drivers/media/video/ivtv/ivtv-ioctl.c Ian Armstrong 2007-11-05  924  		"HM12 (YUV 4:2:0)", V4L2_PIX_FMT_HM12,
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27 @925  		{ 0, 0, 0, 0 }
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  926  	};
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  927  	static const struct v4l2_fmtdesc mpeg = {
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  928  		0, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FMT_FLAG_COMPRESSED,
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  929  		"MPEG", V4L2_PIX_FMT_MPEG,
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  930  		{ 0, 0, 0, 0 }
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  931  	};
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  932  	struct ivtv *itv = fh2id(fh)->itv;
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  933  	struct ivtv_stream *s = &itv->streams[fh2id(fh)->type];
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  934  
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  935  	if (fmt->index)
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  936  		return -EINVAL;
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  937  	if (s->type == IVTV_ENC_STREAM_TYPE_MPG)
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  938  		*fmt = mpeg;
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  939  	else if (s->type == IVTV_ENC_STREAM_TYPE_YUV)
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  940  		*fmt = hm12;
bfd063cebb75d33 drivers/media/pci/ivtv/ivtv-ioctl.c   Hans Verkuil  2012-10-01  941  	else
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  942  		return -EINVAL;
3f038d80039f60e drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  943  	return 0;
1a0adaf37c30e89 drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2007-04-27  944  }
3f038d80039f60e drivers/media/video/ivtv/ivtv-ioctl.c Hans Verkuil  2008-05-29  945  

:::::: The code at line 925 was first introduced by commit
:::::: 1a0adaf37c30e89e44d1470ef604a930999a5826 V4L/DVB (5345): ivtv driver for Conexant cx23416/cx23415 MPEG encoder/decoder

:::::: TO: Hans Verkuil <hverkuil@xs4all.nl>
:::::: CC: Mauro Carvalho Chehab <mchehab@infradead.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
                     ` (2 preceding siblings ...)
  2020-03-19  8:24   ` kbuild test robot
@ 2020-03-23 10:03   ` Sakari Ailus
  2020-03-23 10:07     ` Laurent Pinchart
  3 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2020-03-23 10:03 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Helen Koike, Hans Verkuil, linux-renesas-soc,
	Niklas Söderlund

Hi Laurent,

On Thu, Mar 19, 2020 at 02:46:58AM +0200, Laurent Pinchart wrote:
> The VIDIOC_ENUM_FMT ioctl enumerates all formats supported by a video
> node. For MC-centric devices, its behaviour has always been ill-defined,
> with drivers implementing one of the following behaviours:
> 
> - No support for VIDIOC_ENUM_FMT at all
> - Enumerating all formats supported by the video node, regardless of the
>   configuration of the pipeline
> - Enumerating formats supported by the video node for the active
>   configuration of the connected subdevice
> 
> The first behaviour is obviously useless for applications. The second
> behaviour provides the most information, but doesn't offer a way to find
> what formats are compatible with a given pipeline configuration. The
> third behaviour fixes that, but with the drawback that applications
> can't enumerate all supported formats anymore, and have to modify the
> active configuration of the pipeline to enumerate formats.
> 
> The situation is messy as none of the implemented behaviours are ideal,
> and userspace can't predict what will happen as the behaviour is
> driver-specific.
> 
> To fix this, let's extend the VIDIOC_ENUM_FMT with a missing capability:
> enumerating pixel formats for a given media bus code. The media bus code
> is passed through the v4l2_fmtdesc structure in a new mbus_code field
> (repurposed from the reserved fields). With this capability in place,
> applications can enumerate pixel formats for a given media bus code
> without modifying the active configuration of the device.
> 
> The current behaviour of the ioctl is preserved when the new mbus_code
> field is set to 0, ensuring compatibility with existing userspace. The
> API extension is documented as mandatory for MC-centric devices (as
> advertised through the V4L2_CAP_IO_MC capability), allowing applications
> and compliance tools to easily determine the availability of the
> VIDIOC_ENUM_FMT extension.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

I'd address setting the reserved fields explicitly in a separate patch,
simply by removing them. As another field in the struct is assigned, the
memory is zeroed and explicit assignment is redundant.

> ---
>  .../media/uapi/v4l/vidioc-enum-fmt.rst        | 19 +++++++++++++++----
>  drivers/media/v4l2-core/v4l2-ioctl.c          | 13 +++++++++++--
>  include/uapi/linux/videodev2.h                |  3 ++-
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> index 8ca6ab701e4a..82792d8e910b 100644
> --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> @@ -48,10 +48,21 @@ one until ``EINVAL`` is returned. If applicable, drivers shall return
>  formats in preference order, where preferred formats are returned before
>  (that is, with lower ``index`` value) less-preferred formats.
>  
> -.. note::
> -
> -   After switching input or output the list of enumerated image
> -   formats may be different.
> +If the driver doesn't advertise the ``V4L2_CAP_IO_MC``
> +:ref:`device-capabilities <capability>`, applications shall initialize the
> +``mbus_code`` field to zero. Drivers shall enumerate all image formats supported
> +by the device. The enumerated formats may depend on the active input or output
> +of the device.
> +
> +If the driver advertises the ``V4L2_CAP_IO_MC`` :ref:`device-capabilities
> +<capability>`, applications may initialize the ``mbus_code`` to a valid
> +:ref:`v4l2_mbus_pixelcode <media bus format code>`. If the ``mbus_code` field
> +is not zero, drivers shall restrict enumeration to only the image formats that
> +can produce (for video output devices) or be produced from (for video capture
> +devices) that media bus code. Regardless of the value of the ``mbus_code``
> +field, the enumerated image formats shall not depend on the active
> +configuration of the video device or device pipeline. Enumeration shall
> +otherwise operate as previously described.
>  
>  
>  .. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 3545a8adf844..b55603964abd 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -264,12 +264,13 @@ static void v4l_print_fmtdesc(const void *arg, bool write_only)
>  {
>  	const struct v4l2_fmtdesc *p = arg;
>  
> -	pr_cont("index=%u, type=%s, flags=0x%x, pixelformat=%c%c%c%c, description='%.*s'\n",
> +	pr_cont("index=%u, type=%s, flags=0x%x, pixelformat=%c%c%c%c, mbus_code=0x%04x, description='%.*s'\n",
>  		p->index, prt_names(p->type, v4l2_type_names),
>  		p->flags, (p->pixelformat & 0xff),
>  		(p->pixelformat >>  8) & 0xff,
>  		(p->pixelformat >> 16) & 0xff,
>  		(p->pixelformat >> 24) & 0xff,
> +		p->mbus_code,
>  		(int)sizeof(p->description), p->description);
>  }
>  
> @@ -1467,12 +1468,20 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>  	struct video_device *vdev = video_devdata(file);
>  	struct v4l2_fmtdesc *p = arg;
>  	int ret = check_fmt(file, p->type);
> +	u32 mbus_code;
>  	u32 cap_mask;
>  
>  	if (ret)
>  		return ret;
>  	ret = -EINVAL;
>  
> +	if (p->mbus_code && !(vdev->device_caps & V4L2_CAP_IO_MC))
> +		return -EINVAL;
> +
> +	mbus_code = p->mbus_code;
> +	CLEAR_AFTER_FIELD(p, type);
> +	p->mbus_code = mbus_code;
> +
>  	switch (p->type) {
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> @@ -2752,7 +2761,7 @@ DEFINE_V4L_STUB_FUNC(dv_timings_cap)
>  
>  static const struct v4l2_ioctl_info v4l2_ioctls[] = {
>  	IOCTL_INFO(VIDIOC_QUERYCAP, v4l_querycap, v4l_print_querycap, 0),
> -	IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc, INFO_FL_CLEAR(v4l2_fmtdesc, type)),
> +	IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc, 0),
>  	IOCTL_INFO(VIDIOC_G_FMT, v4l_g_fmt, v4l_print_format, 0),
>  	IOCTL_INFO(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
>  	IOCTL_INFO(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index c793263a3705..0ece960844a5 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -779,7 +779,8 @@ struct v4l2_fmtdesc {
>  	__u32               flags;
>  	__u8		    description[32];   /* Description string */
>  	__u32		    pixelformat;       /* Format fourcc      */
> -	__u32		    reserved[4];
> +	__u32		    mbus_code;		/* Media bus code    */
> +	__u32		    reserved[3];
>  };
>  
>  #define V4L2_FMT_FLAG_COMPRESSED		0x0001

-- 
Kind regards,

Sakari Ailus

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

* Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-23 10:03   ` Sakari Ailus
@ 2020-03-23 10:07     ` Laurent Pinchart
  2020-03-23 10:12       ` Sakari Ailus
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-23 10:07 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, Helen Koike, Hans Verkuil, linux-renesas-soc,
	Niklas Söderlund

Hi Sakari,

On Mon, Mar 23, 2020 at 12:03:28PM +0200, Sakari Ailus wrote:
> On Thu, Mar 19, 2020 at 02:46:58AM +0200, Laurent Pinchart wrote:
> > The VIDIOC_ENUM_FMT ioctl enumerates all formats supported by a video
> > node. For MC-centric devices, its behaviour has always been ill-defined,
> > with drivers implementing one of the following behaviours:
> > 
> > - No support for VIDIOC_ENUM_FMT at all
> > - Enumerating all formats supported by the video node, regardless of the
> >   configuration of the pipeline
> > - Enumerating formats supported by the video node for the active
> >   configuration of the connected subdevice
> > 
> > The first behaviour is obviously useless for applications. The second
> > behaviour provides the most information, but doesn't offer a way to find
> > what formats are compatible with a given pipeline configuration. The
> > third behaviour fixes that, but with the drawback that applications
> > can't enumerate all supported formats anymore, and have to modify the
> > active configuration of the pipeline to enumerate formats.
> > 
> > The situation is messy as none of the implemented behaviours are ideal,
> > and userspace can't predict what will happen as the behaviour is
> > driver-specific.
> > 
> > To fix this, let's extend the VIDIOC_ENUM_FMT with a missing capability:
> > enumerating pixel formats for a given media bus code. The media bus code
> > is passed through the v4l2_fmtdesc structure in a new mbus_code field
> > (repurposed from the reserved fields). With this capability in place,
> > applications can enumerate pixel formats for a given media bus code
> > without modifying the active configuration of the device.
> > 
> > The current behaviour of the ioctl is preserved when the new mbus_code
> > field is set to 0, ensuring compatibility with existing userspace. The
> > API extension is documented as mandatory for MC-centric devices (as
> > advertised through the V4L2_CAP_IO_MC capability), allowing applications
> > and compliance tools to easily determine the availability of the
> > VIDIOC_ENUM_FMT extension.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> I'd address setting the reserved fields explicitly in a separate patch,
> simply by removing them. As another field in the struct is assigned, the
> memory is zeroed and explicit assignment is redundant.

I'm not sure to follow you, what code are you referring to ?

> > ---
> >  .../media/uapi/v4l/vidioc-enum-fmt.rst        | 19 +++++++++++++++----
> >  drivers/media/v4l2-core/v4l2-ioctl.c          | 13 +++++++++++--
> >  include/uapi/linux/videodev2.h                |  3 ++-
> >  3 files changed, 28 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > index 8ca6ab701e4a..82792d8e910b 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> > @@ -48,10 +48,21 @@ one until ``EINVAL`` is returned. If applicable, drivers shall return
> >  formats in preference order, where preferred formats are returned before
> >  (that is, with lower ``index`` value) less-preferred formats.
> >  
> > -.. note::
> > -
> > -   After switching input or output the list of enumerated image
> > -   formats may be different.
> > +If the driver doesn't advertise the ``V4L2_CAP_IO_MC``
> > +:ref:`device-capabilities <capability>`, applications shall initialize the
> > +``mbus_code`` field to zero. Drivers shall enumerate all image formats supported
> > +by the device. The enumerated formats may depend on the active input or output
> > +of the device.
> > +
> > +If the driver advertises the ``V4L2_CAP_IO_MC`` :ref:`device-capabilities
> > +<capability>`, applications may initialize the ``mbus_code`` to a valid
> > +:ref:`v4l2_mbus_pixelcode <media bus format code>`. If the ``mbus_code` field
> > +is not zero, drivers shall restrict enumeration to only the image formats that
> > +can produce (for video output devices) or be produced from (for video capture
> > +devices) that media bus code. Regardless of the value of the ``mbus_code``
> > +field, the enumerated image formats shall not depend on the active
> > +configuration of the video device or device pipeline. Enumeration shall
> > +otherwise operate as previously described.
> >  
> >  
> >  .. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 3545a8adf844..b55603964abd 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -264,12 +264,13 @@ static void v4l_print_fmtdesc(const void *arg, bool write_only)
> >  {
> >  	const struct v4l2_fmtdesc *p = arg;
> >  
> > -	pr_cont("index=%u, type=%s, flags=0x%x, pixelformat=%c%c%c%c, description='%.*s'\n",
> > +	pr_cont("index=%u, type=%s, flags=0x%x, pixelformat=%c%c%c%c, mbus_code=0x%04x, description='%.*s'\n",
> >  		p->index, prt_names(p->type, v4l2_type_names),
> >  		p->flags, (p->pixelformat & 0xff),
> >  		(p->pixelformat >>  8) & 0xff,
> >  		(p->pixelformat >> 16) & 0xff,
> >  		(p->pixelformat >> 24) & 0xff,
> > +		p->mbus_code,
> >  		(int)sizeof(p->description), p->description);
> >  }
> >  
> > @@ -1467,12 +1468,20 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
> >  	struct video_device *vdev = video_devdata(file);
> >  	struct v4l2_fmtdesc *p = arg;
> >  	int ret = check_fmt(file, p->type);
> > +	u32 mbus_code;
> >  	u32 cap_mask;
> >  
> >  	if (ret)
> >  		return ret;
> >  	ret = -EINVAL;
> >  
> > +	if (p->mbus_code && !(vdev->device_caps & V4L2_CAP_IO_MC))
> > +		return -EINVAL;
> > +
> > +	mbus_code = p->mbus_code;
> > +	CLEAR_AFTER_FIELD(p, type);
> > +	p->mbus_code = mbus_code;
> > +
> >  	switch (p->type) {
> >  	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> >  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> > @@ -2752,7 +2761,7 @@ DEFINE_V4L_STUB_FUNC(dv_timings_cap)
> >  
> >  static const struct v4l2_ioctl_info v4l2_ioctls[] = {
> >  	IOCTL_INFO(VIDIOC_QUERYCAP, v4l_querycap, v4l_print_querycap, 0),
> > -	IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc, INFO_FL_CLEAR(v4l2_fmtdesc, type)),
> > +	IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc, 0),
> >  	IOCTL_INFO(VIDIOC_G_FMT, v4l_g_fmt, v4l_print_format, 0),
> >  	IOCTL_INFO(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
> >  	IOCTL_INFO(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index c793263a3705..0ece960844a5 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -779,7 +779,8 @@ struct v4l2_fmtdesc {
> >  	__u32               flags;
> >  	__u8		    description[32];   /* Description string */
> >  	__u32		    pixelformat;       /* Format fourcc      */
> > -	__u32		    reserved[4];
> > +	__u32		    mbus_code;		/* Media bus code    */
> > +	__u32		    reserved[3];
> >  };
> >  
> >  #define V4L2_FMT_FLAG_COMPRESSED		0x0001

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
  2020-03-23 10:07     ` Laurent Pinchart
@ 2020-03-23 10:12       ` Sakari Ailus
  0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2020-03-23 10:12 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Helen Koike, Hans Verkuil, linux-renesas-soc,
	Niklas Söderlund

On Mon, Mar 23, 2020 at 12:07:27PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Mon, Mar 23, 2020 at 12:03:28PM +0200, Sakari Ailus wrote:
> > On Thu, Mar 19, 2020 at 02:46:58AM +0200, Laurent Pinchart wrote:
> > > The VIDIOC_ENUM_FMT ioctl enumerates all formats supported by a video
> > > node. For MC-centric devices, its behaviour has always been ill-defined,
> > > with drivers implementing one of the following behaviours:
> > > 
> > > - No support for VIDIOC_ENUM_FMT at all
> > > - Enumerating all formats supported by the video node, regardless of the
> > >   configuration of the pipeline
> > > - Enumerating formats supported by the video node for the active
> > >   configuration of the connected subdevice
> > > 
> > > The first behaviour is obviously useless for applications. The second
> > > behaviour provides the most information, but doesn't offer a way to find
> > > what formats are compatible with a given pipeline configuration. The
> > > third behaviour fixes that, but with the drawback that applications
> > > can't enumerate all supported formats anymore, and have to modify the
> > > active configuration of the pipeline to enumerate formats.
> > > 
> > > The situation is messy as none of the implemented behaviours are ideal,
> > > and userspace can't predict what will happen as the behaviour is
> > > driver-specific.
> > > 
> > > To fix this, let's extend the VIDIOC_ENUM_FMT with a missing capability:
> > > enumerating pixel formats for a given media bus code. The media bus code
> > > is passed through the v4l2_fmtdesc structure in a new mbus_code field
> > > (repurposed from the reserved fields). With this capability in place,
> > > applications can enumerate pixel formats for a given media bus code
> > > without modifying the active configuration of the device.
> > > 
> > > The current behaviour of the ioctl is preserved when the new mbus_code
> > > field is set to 0, ensuring compatibility with existing userspace. The
> > > API extension is documented as mandatory for MC-centric devices (as
> > > advertised through the V4L2_CAP_IO_MC capability), allowing applications
> > > and compliance tools to easily determine the availability of the
> > > VIDIOC_ENUM_FMT extension.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > I'd address setting the reserved fields explicitly in a separate patch,
> > simply by removing them. As another field in the struct is assigned, the
> > memory is zeroed and explicit assignment is redundant.
> 
> I'm not sure to follow you, what code are you referring to ?

Have you seen the e-mails from the kbuild bot?

-- 
Sakari Ailus

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

end of thread, other threads:[~2020-03-23 10:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19  0:46 [PATCH v6 0/5] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Laurent Pinchart
2020-03-19  0:46 ` [PATCH v6 1/5] " Laurent Pinchart
2020-03-19  0:46 ` [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices Laurent Pinchart
2020-03-19  3:42   ` kbuild test robot
2020-03-19  3:52   ` kbuild test robot
2020-03-19  8:24   ` kbuild test robot
2020-03-23 10:03   ` Sakari Ailus
2020-03-23 10:07     ` Laurent Pinchart
2020-03-23 10:12       ` Sakari Ailus
2020-03-19  0:46 ` [PATCH v6 3/5] rcar-vin: Make use of V4L2_CAP_IO_MC Laurent Pinchart
2020-03-19  0:47 ` [PATCH v6 4/5] staging/intel-ipu3: " Laurent Pinchart
2020-03-19  0:47 ` [PATCH v6 5/5] vimc: " Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).