All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
@ 2019-10-15 14:35 Niklas Söderlund
  2019-10-15 14:35 ` [PATCH 1/3] " Niklas Söderlund
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-10-15 14:35 UTC (permalink / raw)
  To: Helen Koike, Hans Verkuil, Sakari Ailus, Laurent Pinchart, linux-media
  Cc: linux-renesas-soc, Niklas Söderlund

Hi,

First of this series is based on work done by Helen in 2017 [1], I have 
synced with her that it's OK for me to takeover the work.

This series 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.

Patch 1/3 adds the core parts of this change by adding a new video 
device capability flag V4L2_CAP_IO_MC which if set provides helper 
implementations for the get, set and enum inputs and outputs ioctrls 
freeing the video device driver from the need to implement them.

Patch 2/3 and 3/3 converts the R-Car VIN and Intel IPU3 drivers to use 
this new flag and delete the now redundant boiler plate code. I'm sure 
more video device drivers can make use of this new flag but as I can 
only test on these two platforms I have limited my changes to those.

1. https://patchwork.linuxtv.org/patch/41857/

Niklas Söderlund (3):
  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

 .../media/uapi/v4l/vidioc-querycap.rst        |  3 +
 .../media/videodev2.h.rst.exceptions          |  1 +
 drivers/media/platform/rcar-vin/rcar-v4l2.c   | 17 +---
 drivers/media/v4l2-core/v4l2-dev.c            | 24 +++--
 drivers/media/v4l2-core/v4l2-ioctl.c          | 87 ++++++++++++++++++-
 drivers/staging/media/ipu3/ipu3-v4l2.c        | 60 +------------
 include/uapi/linux/videodev2.h                |  2 +
 7 files changed, 110 insertions(+), 84 deletions(-)

-- 
2.23.0


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

* [PATCH 1/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
  2019-10-15 14:35 [PATCH 0/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Niklas Söderlund
@ 2019-10-15 14:35 ` Niklas Söderlund
  2019-10-16 13:56   ` Hans Verkuil
  2019-10-15 14:35 ` [PATCH 2/3] rcar-vin: Make use of V4L2_CAP_IO_MC Niklas Söderlund
  2019-10-15 14:35 ` [PATCH 3/3] staging/intel-ipu3: " Niklas Söderlund
  2 siblings, 1 reply; 6+ messages in thread
From: Niklas Söderlund @ 2019-10-15 14:35 UTC (permalink / raw)
  To: Helen Koike, Hans Verkuil, Sakari Ailus, Laurent Pinchart, linux-media
  Cc: linux-renesas-soc, Niklas Söderlund

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, ioctls for get, set and enum inputs and outputs
are automatically enabled and programmed to call helper function

Signed-off-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 .../media/uapi/v4l/vidioc-querycap.rst        |  3 +
 .../media/videodev2.h.rst.exceptions          |  1 +
 drivers/media/v4l2-core/v4l2-dev.c            | 24 +++--
 drivers/media/v4l2-core/v4l2-ioctl.c          | 87 ++++++++++++++++++-
 include/uapi/linux/videodev2.h                |  2 +
 5 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 5f9930195d624c73..8b621ecb906afe96 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -264,6 +264,9 @@ specification the ioctl returns an ``EINVAL`` error code.
     * - ``V4L2_CAP_TOUCH``
       - 0x10000000
       - This is a touch device.
+    * - ``V4L2_CAP_IO_MC``
+      - 0x20000000
+      - The inputs and/or outputs of this device are controlled by 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 b58e381bdf7bd38a..6c78a79fa45de283 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -173,6 +173,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 4037689a945a5330..851e645414600941 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -702,22 +702,34 @@ 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);
 			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);
 			SET_VALID_IOCTL(ops, VIDIOC_QUERY_DV_TIMINGS, vidioc_query_dv_timings);
 			SET_VALID_IOCTL(ops, VIDIOC_S_EDID, vidioc_s_edid);
+			if (vdev->device_caps & V4L2_CAP_IO_MC) {
+				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);
+			}
 		}
 		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);
 			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);
+			if (vdev->device_caps & V4L2_CAP_IO_MC) {
+				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);
+			}
 		}
 		if (ops->vidioc_g_parm || (vdev->vfl_type == VFL_TYPE_GRABBER &&
 					ops->vidioc_g_std))
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 51b912743f0f4f6f..e4d2ec4ccd49f65e 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1075,6 +1075,72 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
 	return ret;
 }
 
+static int v4l2_ioctl_enum_input_mc(struct file *file, void *priv,
+				    struct v4l2_input *i)
+{
+	struct video_device *vfd = video_devdata(file);
+
+	if (i->index > 0)
+		return -EINVAL;
+
+	memset(i, 0, sizeof(*i));
+	strlcpy(i->name, vfd->name, sizeof(i->name));
+	i->type = V4L2_INPUT_TYPE_CAMERA;
+
+	return 0;
+}
+
+static int v4l2_ioctl_enum_output_mc(struct file *file, void *priv,
+				     struct v4l2_output *o)
+{
+	struct video_device *vfd = video_devdata(file);
+
+	if (o->index > 0)
+		return -EINVAL;
+
+	memset(o, 0, sizeof(*o));
+	strlcpy(o->name, vfd->name, sizeof(o->name));
+	o->type = V4L2_OUTPUT_TYPE_ANALOG;
+
+	return 0;
+}
+
+static int v4l2_ioctl_g_input_mc(struct file *file, void *priv, unsigned int *i)
+{
+	*i = 0;
+	return 0;
+}
+#define v4l2_ioctl_g_output_mc v4l2_ioctl_g_input_mc
+
+static int v4l2_ioctl_s_input_mc(struct file *file, void *priv, unsigned int i)
+{
+	return i ? -EINVAL : 0;
+}
+#define v4l2_ioctl_s_output_mc v4l2_ioctl_s_input_mc
+
+
+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)
+		return v4l2_ioctl_g_input_mc(file, fh, arg);
+
+	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)
+		return v4l2_ioctl_g_output_mc(file, fh, arg);
+
+	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)
 {
@@ -1084,12 +1150,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 v4l2_ioctl_s_input_mc(file, fh, *(unsigned int *)arg);
+
 	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 v4l2_ioctl_s_output_mc(file, fh, *(unsigned int *)arg);
+
 	return ops->vidioc_s_output(file, fh, *(unsigned int *)arg);
 }
 
@@ -1133,6 +1208,9 @@ 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)
+		return v4l2_ioctl_enum_input_mc(file, fh, p);
+
 	return ops->vidioc_enum_input(file, fh, p);
 }
 
@@ -1151,6 +1229,9 @@ 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)
+		return v4l2_ioctl_enum_output_mc(file, fh, p);
+
 	return ops->vidioc_enum_output(file, fh, p);
 }
 
@@ -2663,10 +2744,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)
@@ -2715,11 +2794,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 b3c0961b62a0cba7..8c86f6f5b3d06b26 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 */
 
 /*
-- 
2.23.0


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

* [PATCH 2/3] rcar-vin: Make use of V4L2_CAP_IO_MC
  2019-10-15 14:35 [PATCH 0/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Niklas Söderlund
  2019-10-15 14:35 ` [PATCH 1/3] " Niklas Söderlund
@ 2019-10-15 14:35 ` Niklas Söderlund
  2019-10-15 14:35 ` [PATCH 3/3] staging/intel-ipu3: " Niklas Söderlund
  2 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-10-15 14:35 UTC (permalink / raw)
  To: Helen Koike, Hans Verkuil, Sakari Ailus, Laurent Pinchart, linux-media
  Cc: linux-renesas-soc, Niklas Söderlund

Set the V4L2_CAP_IO_MC capability flag and remove the driver specific
enum, get and set input callbacks for the media controller enabled mode
of the driver.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 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 9a9b89c0dc0b3be4..dfcf60d7523e3f4f 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -714,18 +714,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,
@@ -733,10 +721,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,
@@ -908,6 +892,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;
-- 
2.23.0


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

* [PATCH 3/3] staging/intel-ipu3: Make use of V4L2_CAP_IO_MC
  2019-10-15 14:35 [PATCH 0/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Niklas Söderlund
  2019-10-15 14:35 ` [PATCH 1/3] " Niklas Söderlund
  2019-10-15 14:35 ` [PATCH 2/3] rcar-vin: Make use of V4L2_CAP_IO_MC Niklas Söderlund
@ 2019-10-15 14:35 ` Niklas Söderlund
  2 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-10-15 14:35 UTC (permalink / raw)
  To: Helen Koike, Hans Verkuil, Sakari Ailus, Laurent Pinchart, linux-media
  Cc: linux-renesas-soc, Niklas Söderlund

Set the V4L2_CAP_IO_MC capability flag and remove the driver specific
enum, get and set input callbacks.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/staging/media/ipu3/ipu3-v4l2.c | 60 +-------------------------
 1 file changed, 2 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
index 3c7ad1eed4343a2a..bae0f7212e640ee2 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -845,54 +845,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 +917,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,
@@ -1062,7 +1006,7 @@ static void imgu_node_to_v4l2(u32 node, struct video_device *vdev,
 
 	switch (node) {
 	case IMGU_NODE_IN:
-		cap = V4L2_CAP_VIDEO_OUTPUT_MPLANE;
+		cap = V4L2_CAP_VIDEO_OUTPUT_MPLANE | V4L2_CAP_IO_MC;
 		f->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
 		vdev->ioctl_ops = &imgu_v4l2_ioctl_ops;
 		break;
@@ -1081,7 +1025,7 @@ static void imgu_node_to_v4l2(u32 node, struct video_device *vdev,
 		imgu_css_meta_fmt_set(&f->fmt.meta);
 		break;
 	default:
-		cap = V4L2_CAP_VIDEO_CAPTURE_MPLANE;
+		cap = V4L2_CAP_VIDEO_CAPTURE_MPLANE | V4L2_CAP_IO_MC;
 		f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 		vdev->ioctl_ops = &imgu_v4l2_ioctl_ops;
 	}
-- 
2.23.0


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

* Re: [PATCH 1/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
  2019-10-15 14:35 ` [PATCH 1/3] " Niklas Söderlund
@ 2019-10-16 13:56   ` Hans Verkuil
  2019-11-15 23:47     ` Niklas Söderlund
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2019-10-16 13:56 UTC (permalink / raw)
  To: Niklas Söderlund, Helen Koike, Sakari Ailus,
	Laurent Pinchart, linux-media
  Cc: linux-renesas-soc

On 10/15/19 4:35 PM, Niklas Söderlund wrote:
> 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, ioctls for get, set and enum inputs and outputs
> are automatically enabled and programmed to call helper function
> 
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  .../media/uapi/v4l/vidioc-querycap.rst        |  3 +
>  .../media/videodev2.h.rst.exceptions          |  1 +
>  drivers/media/v4l2-core/v4l2-dev.c            | 24 +++--
>  drivers/media/v4l2-core/v4l2-ioctl.c          | 87 ++++++++++++++++++-
>  include/uapi/linux/videodev2.h                |  2 +
>  5 files changed, 107 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> index 5f9930195d624c73..8b621ecb906afe96 100644
> --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> @@ -264,6 +264,9 @@ specification the ioctl returns an ``EINVAL`` error code.
>      * - ``V4L2_CAP_TOUCH``
>        - 0x10000000
>        - This is a touch device.
> +    * - ``V4L2_CAP_IO_MC``
> +      - 0x20000000
> +      - The inputs and/or outputs of this device are controlled by 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 b58e381bdf7bd38a..6c78a79fa45de283 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -173,6 +173,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 4037689a945a5330..851e645414600941 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -702,22 +702,34 @@ 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);
>  			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);
>  			SET_VALID_IOCTL(ops, VIDIOC_QUERY_DV_TIMINGS, vidioc_query_dv_timings);
>  			SET_VALID_IOCTL(ops, VIDIOC_S_EDID, vidioc_s_edid);
> +			if (vdev->device_caps & V4L2_CAP_IO_MC) {
> +				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);
> +			}
>  		}
>  		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);
>  			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);
> +			if (vdev->device_caps & V4L2_CAP_IO_MC) {
> +				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);
> +			}
>  		}
>  		if (ops->vidioc_g_parm || (vdev->vfl_type == VFL_TYPE_GRABBER &&
>  					ops->vidioc_g_std))
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 51b912743f0f4f6f..e4d2ec4ccd49f65e 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1075,6 +1075,72 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
>  	return ret;
>  }
>  
> +static int v4l2_ioctl_enum_input_mc(struct file *file, void *priv,
> +				    struct v4l2_input *i)
> +{
> +	struct video_device *vfd = video_devdata(file);
> +
> +	if (i->index > 0)
> +		return -EINVAL;
> +
> +	memset(i, 0, sizeof(*i));
> +	strlcpy(i->name, vfd->name, sizeof(i->name));
> +	i->type = V4L2_INPUT_TYPE_CAMERA;

Does this...

> +
> +	return 0;
> +}
> +
> +static int v4l2_ioctl_enum_output_mc(struct file *file, void *priv,
> +				     struct v4l2_output *o)
> +{
> +	struct video_device *vfd = video_devdata(file);
> +
> +	if (o->index > 0)
> +		return -EINVAL;
> +
> +	memset(o, 0, sizeof(*o));
> +	strlcpy(o->name, vfd->name, sizeof(o->name));
> +	o->type = V4L2_OUTPUT_TYPE_ANALOG;

... and this make sense for devices like this?

I was wondering if we shouldn't make aliases:

	V4L2_INPUT_TYPE_VIDEO = V4L2_INPUT_TYPE_CAMERA,

and

	V4L2_OUTPUT_TYPE_VIDEO = V4L2_OUTPUT_TYPE_ANALOG,

This wouldn't change the API, but it avoids using these really outdated
CAMERA/ANALOG words.

But it is perhaps something for a separate patch.

> +
> +	return 0;
> +}
> +
> +static int v4l2_ioctl_g_input_mc(struct file *file, void *priv, unsigned int *i)
> +{
> +	*i = 0;
> +	return 0;
> +}
> +#define v4l2_ioctl_g_output_mc v4l2_ioctl_g_input_mc
> +
> +static int v4l2_ioctl_s_input_mc(struct file *file, void *priv, unsigned int i)
> +{
> +	return i ? -EINVAL : 0;
> +}

These could be exported: it is very common to have just one input or output,
and many drivers have such trivial implementations.

Calling it v4l2_ioctl_g/s_single_input and making it available for everyone would
be nice.

Alternatively (and perhaps even better?) drivers can just leave ops_vidioc_g/s_in/output
to NULL and in that case the core assumes that there is just a single input/output.

> +#define v4l2_ioctl_s_output_mc v4l2_ioctl_s_input_mc
> +
> +
> +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)
> +		return v4l2_ioctl_g_input_mc(file, fh, arg);
> +
> +	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)
> +		return v4l2_ioctl_g_output_mc(file, fh, arg);
> +
> +	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)
>  {
> @@ -1084,12 +1150,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 v4l2_ioctl_s_input_mc(file, fh, *(unsigned int *)arg);
> +
>  	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 v4l2_ioctl_s_output_mc(file, fh, *(unsigned int *)arg);
> +
>  	return ops->vidioc_s_output(file, fh, *(unsigned int *)arg);
>  }
>  
> @@ -1133,6 +1208,9 @@ 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)
> +		return v4l2_ioctl_enum_input_mc(file, fh, p);
> +
>  	return ops->vidioc_enum_input(file, fh, p);
>  }
>  
> @@ -1151,6 +1229,9 @@ 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)
> +		return v4l2_ioctl_enum_output_mc(file, fh, p);
> +
>  	return ops->vidioc_enum_output(file, fh, p);
>  }
>  
> @@ -2663,10 +2744,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)
> @@ -2715,11 +2794,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 b3c0961b62a0cba7..8c86f6f5b3d06b26 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,

	Hans

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

* Re: [PATCH 1/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC
  2019-10-16 13:56   ` Hans Verkuil
@ 2019-11-15 23:47     ` Niklas Söderlund
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-11-15 23:47 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Helen Koike, Sakari Ailus, Laurent Pinchart, linux-media,
	linux-renesas-soc

Hi Hans,

Thanks for your feedback.

On 2019-10-16 15:56:20 +0200, Hans Verkuil wrote:
> On 10/15/19 4:35 PM, Niklas Söderlund wrote:
> > 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, ioctls for get, set and enum inputs and outputs
> > are automatically enabled and programmed to call helper function
> > 
> > Signed-off-by: Helen Koike <helen.koike@collabora.com>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  .../media/uapi/v4l/vidioc-querycap.rst        |  3 +
> >  .../media/videodev2.h.rst.exceptions          |  1 +
> >  drivers/media/v4l2-core/v4l2-dev.c            | 24 +++--
> >  drivers/media/v4l2-core/v4l2-ioctl.c          | 87 ++++++++++++++++++-
> >  include/uapi/linux/videodev2.h                |  2 +
> >  5 files changed, 107 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> > index 5f9930195d624c73..8b621ecb906afe96 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> > @@ -264,6 +264,9 @@ specification the ioctl returns an ``EINVAL`` error code.
> >      * - ``V4L2_CAP_TOUCH``
> >        - 0x10000000
> >        - This is a touch device.
> > +    * - ``V4L2_CAP_IO_MC``
> > +      - 0x20000000
> > +      - The inputs and/or outputs of this device are controlled by 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 b58e381bdf7bd38a..6c78a79fa45de283 100644
> > --- a/Documentation/media/videodev2.h.rst.exceptions
> > +++ b/Documentation/media/videodev2.h.rst.exceptions
> > @@ -173,6 +173,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 4037689a945a5330..851e645414600941 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -702,22 +702,34 @@ 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);
> >  			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);
> >  			SET_VALID_IOCTL(ops, VIDIOC_QUERY_DV_TIMINGS, vidioc_query_dv_timings);
> >  			SET_VALID_IOCTL(ops, VIDIOC_S_EDID, vidioc_s_edid);
> > +			if (vdev->device_caps & V4L2_CAP_IO_MC) {
> > +				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);
> > +			}
> >  		}
> >  		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);
> >  			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);
> > +			if (vdev->device_caps & V4L2_CAP_IO_MC) {
> > +				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);
> > +			}
> >  		}
> >  		if (ops->vidioc_g_parm || (vdev->vfl_type == VFL_TYPE_GRABBER &&
> >  					ops->vidioc_g_std))
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 51b912743f0f4f6f..e4d2ec4ccd49f65e 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1075,6 +1075,72 @@ static int v4l_querycap(const struct 
> > v4l2_ioctl_ops *ops,
> >  	return ret;
> >  }
> >  
> > +static int v4l2_ioctl_enum_input_mc(struct file *file, void *priv,
> > +				    struct v4l2_input *i)
> > +{
> > +	struct video_device *vfd = video_devdata(file);
> > +
> > +	if (i->index > 0)
> > +		return -EINVAL;
> > +
> > +	memset(i, 0, sizeof(*i));
> > +	strlcpy(i->name, vfd->name, sizeof(i->name));
> > +	i->type = V4L2_INPUT_TYPE_CAMERA;
> 
> Does this...
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int v4l2_ioctl_enum_output_mc(struct file *file, void *priv,
> > +				     struct v4l2_output *o)
> > +{
> > +	struct video_device *vfd = video_devdata(file);
> > +
> > +	if (o->index > 0)
> > +		return -EINVAL;
> > +
> > +	memset(o, 0, sizeof(*o));
> > +	strlcpy(o->name, vfd->name, sizeof(o->name));
> > +	o->type = V4L2_OUTPUT_TYPE_ANALOG;
> 
> ... and this make sense for devices like this?
> 
> I was wondering if we shouldn't make aliases:
> 
> 	V4L2_INPUT_TYPE_VIDEO = V4L2_INPUT_TYPE_CAMERA,
> 
> and
> 
> 	V4L2_OUTPUT_TYPE_VIDEO = V4L2_OUTPUT_TYPE_ANALOG,
> 
> This wouldn't change the API, but it avoids using these really outdated
> CAMERA/ANALOG words.
> 
> But it is perhaps something for a separate patch.

I like it, but I think it is indeed something for another patch. I made 
a note of this and will get back to this on top of this series, if 
someone don't beat me to it.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int v4l2_ioctl_g_input_mc(struct file *file, void *priv, unsigned int *i)
> > +{
> > +	*i = 0;
> > +	return 0;
> > +}
> > +#define v4l2_ioctl_g_output_mc v4l2_ioctl_g_input_mc
> > +
> > +static int v4l2_ioctl_s_input_mc(struct file *file, void *priv, unsigned int i)
> > +{
> > +	return i ? -EINVAL : 0;
> > +}
> 
> These could be exported: it is very common to have just one input or output,
> and many drivers have such trivial implementations.
> 
> Calling it v4l2_ioctl_g/s_single_input and making it available for everyone would
> be nice.
> 
> Alternatively (and perhaps even better?) drivers can just leave ops_vidioc_g/s_in/output
> to NULL and in that case the core assumes that there is just a single input/output.

I really like this idea and will use it in v2.

> 
> > +#define v4l2_ioctl_s_output_mc v4l2_ioctl_s_input_mc
> > +
> > +
> > +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)
> > +		return v4l2_ioctl_g_input_mc(file, fh, arg);
> > +
> > +	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)
> > +		return v4l2_ioctl_g_output_mc(file, fh, arg);
> > +
> > +	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)
> >  {
> > @@ -1084,12 +1150,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 v4l2_ioctl_s_input_mc(file, fh, *(unsigned int *)arg);
> > +
> >  	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 v4l2_ioctl_s_output_mc(file, fh, *(unsigned int *)arg);
> > +
> >  	return ops->vidioc_s_output(file, fh, *(unsigned int *)arg);
> >  }
> >  
> > @@ -1133,6 +1208,9 @@ 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)
> > +		return v4l2_ioctl_enum_input_mc(file, fh, p);
> > +
> >  	return ops->vidioc_enum_input(file, fh, p);
> >  }
> >  
> > @@ -1151,6 +1229,9 @@ 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)
> > +		return v4l2_ioctl_enum_output_mc(file, fh, p);
> > +
> >  	return ops->vidioc_enum_output(file, fh, p);
> >  }
> >  
> > @@ -2663,10 +2744,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)
> > @@ -2715,11 +2794,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 b3c0961b62a0cba7..8c86f6f5b3d06b26 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,
> 
> 	Hans

-- 
Regards,
Niklas Söderlund

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

end of thread, other threads:[~2019-11-15 23:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-15 14:35 [PATCH 0/3] v4l2-dev/ioctl: Add V4L2_CAP_IO_MC Niklas Söderlund
2019-10-15 14:35 ` [PATCH 1/3] " Niklas Söderlund
2019-10-16 13:56   ` Hans Verkuil
2019-11-15 23:47     ` Niklas Söderlund
2019-10-15 14:35 ` [PATCH 2/3] rcar-vin: Make use of V4L2_CAP_IO_MC Niklas Söderlund
2019-10-15 14:35 ` [PATCH 3/3] staging/intel-ipu3: " Niklas Söderlund

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.