[PATCH] devicetree: Add video bus switch

[PATCH] devicetree: Add video bus switch

[Pavel Machek]

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


N900 contains front and back camera, with a switch between the
two. This adds support for the switch component, and it is now
possible to select between front and back cameras during runtime.

This adds documentation for the devicetree binding.

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>


diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
new file mode 100644
index 0000000..1b9f8e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
@@ -0,0 +1,63 @@
+Video Bus Switch Binding
+========================
+
+This is a binding for a gpio controlled switch for camera interfaces. Such a
+device is used on some embedded devices to connect two cameras to the same
+interface of a image signal processor.
+
+Required properties
+===================
+
+compatible	: must contain "video-bus-switch"
+switch-gpios	: GPIO specifier for the gpio, which can toggle the
+		  selected camera. The GPIO should be configured, so
+		  that a disabled GPIO means, that the first port is
+		  selected.
+
+Required Port nodes
+===================
+
+More documentation on these bindings is available in
+video-interfaces.txt in the same directory.
+
+reg		: The interface:
+		  0 - port for image signal processor
+		  1 - port for first camera sensor
+		  2 - port for second camera sensor
+
+Example
+=======
+
+video-bus-switch {
+	compatible = "video-bus-switch"
+	switch-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			csi_switch_in: endpoint {
+				remote-endpoint = <&csi_isp>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			csi_switch_out1: endpoint {
+				remote-endpoint = <&csi_cam1>;
+			};
+		};
+
+		port@2 {
+			reg = <2>;
+
+			csi_switch_out2: endpoint {
+				remote-endpoint = <&csi_cam2>;
+			};
+		};
+	};
+};



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sakari Ailus]

Hi Pavel,

My apologies for the delays in reviewing. Feel free to ping me in the future
if this happens. :-)

On Fri, Feb 03, 2017 at 01:35:08PM +0100, Pavel Machek wrote:
> 
> N900 contains front and back camera, with a switch between the
> two. This adds support for the switch component, and it is now
> possible to select between front and back cameras during runtime.
> 
> This adds documentation for the devicetree binding.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> 
> diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> new file mode 100644
> index 0000000..1b9f8e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> @@ -0,0 +1,63 @@
> +Video Bus Switch Binding
> +========================
> +
> +This is a binding for a gpio controlled switch for camera interfaces. Such a
> +device is used on some embedded devices to connect two cameras to the same
> +interface of a image signal processor.
> +
> +Required properties
> +===================
> +
> +compatible	: must contain "video-bus-switch"

How generic is this? Should we have e.g. nokia,video-bus-switch? And if so,
change the file name accordingly.

> +switch-gpios	: GPIO specifier for the gpio, which can toggle the
> +		  selected camera. The GPIO should be configured, so
> +		  that a disabled GPIO means, that the first port is
> +		  selected.
> +
> +Required Port nodes
> +===================
> +
> +More documentation on these bindings is available in
> +video-interfaces.txt in the same directory.
> +
> +reg		: The interface:
> +		  0 - port for image signal processor
> +		  1 - port for first camera sensor
> +		  2 - port for second camera sensor

I'd say this must be pretty much specific to the one in N900. You could have
more ports. Or you could say that ports beyond 0 are camera sensors. I guess
this is good enough for now though, it can be changed later on with the
source if a need arises.

Btw. was it still considered a problem that the endpoint properties for the
sensors can be different? With the g_routing() pad op which is to be added,
the ISP driver (should actually go to a framework somewhere) could parse the
graph and find the proper endpoint there.

I don't think we need to wait for that now, but this is how the problem
could be solved going forward.

> +
> +Example
> +=======
> +
> +video-bus-switch {
> +	compatible = "video-bus-switch"
> +	switch-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			csi_switch_in: endpoint {
> +				remote-endpoint = <&csi_isp>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			csi_switch_out1: endpoint {
> +				remote-endpoint = <&csi_cam1>;
> +			};
> +		};
> +
> +		port@2 {
> +			reg = <2>;
> +
> +			csi_switch_out2: endpoint {
> +				remote-endpoint = <&csi_cam2>;
> +			};
> +		};
> +	};
> +};
> 
> 
> 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

Hi!

> My apologies for the delays in reviewing. Feel free to ping me in the future
> if this happens. :-)

No problem :-). If you could review the C-code, too... that would be
nice. It should be in your inbox somewhere (and I attached it below,
without the dts part).


> > +Required properties
> > +===================
> > +
> > +compatible	: must contain "video-bus-switch"
> 
> How generic is this? Should we have e.g. nokia,video-bus-switch? And if so,
> change the file name accordingly.

Generic for "single GPIO controls the switch", AFAICT. But that should
be common enough...

> > +reg		: The interface:
> > +		  0 - port for image signal processor
> > +		  1 - port for first camera sensor
> > +		  2 - port for second camera sensor
> 
> I'd say this must be pretty much specific to the one in N900. You could have
> more ports. Or you could say that ports beyond 0 are camera sensors. I guess
> this is good enough for now though, it can be changed later on with the
> source if a need arises.

Well, I'd say that selecting between two sensors is going to be the
common case. If someone needs more than two, it will no longer be
simple GPIO, so we'll have some fixing to do.

> Btw. was it still considered a problem that the endpoint properties for the
> sensors can be different? With the g_routing() pad op which is to be added,
> the ISP driver (should actually go to a framework somewhere) could parse the
> graph and find the proper endpoint there.

I don't know about g_routing. I added g_endpoint_config method that
passes the configuration, and that seems to work for me.

I don't see g_routing in next-20170201 . Is there place to look?

Best regards,
								Pavel

---

N900 contains front and back camera, with a switch between the
two. This adds support for the switch component, and it is now
possible to select between front and back cameras during runtime.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>


diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index ce4a96f..a4b509e 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -91,6 +91,16 @@ config VIDEO_OMAP3_DEBUG
 	---help---
 	  Enable debug messages on OMAP 3 camera controller driver.
 
+config VIDEO_BUS_SWITCH
+	tristate "Video Bus switch"
+	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	depends on MEDIA_CONTROLLER
+	depends on OF
+	---help---
+	  Driver for a GPIO controlled video bus switch, which is used to
+	  connect two camera sensors to the same port a the image signal
+	  processor.
+
 config VIDEO_PXA27x
 	tristate "PXA27x Quick Capture Interface driver"
 	depends on VIDEO_DEV && HAS_DMA
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 40b18d1..8eafc27 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -11,6 +11,8 @@ obj-$(CONFIG_VIDEO_MMP_CAMERA) += marvell-ccic/
 obj-$(CONFIG_VIDEO_OMAP3)	+= omap3isp/
 obj-$(CONFIG_VIDEO_PXA27x)	+= pxa_camera.o
 
+obj-$(CONFIG_VIDEO_BUS_SWITCH) += video-bus-switch.o
+
 obj-$(CONFIG_VIDEO_VIU) += fsl-viu.o
 
 obj-$(CONFIG_VIDEO_VIVID)		+= vivid/
diff --git a/drivers/media/platform/video-bus-switch.c b/drivers/media/platform/video-bus-switch.c
new file mode 100644
index 0000000..6400cfc
--- /dev/null
+++ b/drivers/media/platform/video-bus-switch.c
@@ -0,0 +1,387 @@
+/*
+ * Generic driver for video bus switches
+ *
+ * Copyright (C) 2015 Sebastian Reichel <sre@kernel.org>
+ * Copyright (C) 2016 Pavel Machek <pavel@ucw.cz>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#define DEBUG
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/gpio/consumer.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-of.h>
+
+/*
+ * TODO:
+ * isp_subdev_notifier_complete() calls v4l2_device_register_subdev_nodes()
+ */
+
+#define CSI_SWITCH_SUBDEVS 2
+#define CSI_SWITCH_PORTS 3
+
+enum vbs_state {
+	CSI_SWITCH_DISABLED = 0,
+	CSI_SWITCH_PORT_1 = 1,
+	CSI_SWITCH_PORT_2 = 2,
+};
+
+struct vbs_src_pads {
+	struct media_entity *src;
+	int src_pad;
+};
+
+struct vbs_data {
+	struct gpio_desc *swgpio;
+	struct v4l2_subdev subdev;
+	struct v4l2_async_notifier notifier;
+	struct media_pad pads[CSI_SWITCH_PORTS];
+	struct vbs_src_pads src_pads[CSI_SWITCH_PORTS];
+	struct v4l2_of_endpoint vep[CSI_SWITCH_PORTS];
+	enum vbs_state state;
+};
+
+struct vbs_async_subdev {
+	struct v4l2_subdev *sd;
+	struct v4l2_async_subdev asd;
+	u8 port;
+};
+
+static int vbs_of_parse_nodes(struct device *dev, struct vbs_data *pdata)
+{
+	struct v4l2_async_notifier *notifier = &pdata->notifier;
+	struct device_node *node = NULL;
+
+	notifier->subdevs = devm_kcalloc(dev, CSI_SWITCH_SUBDEVS,
+		sizeof(*notifier->subdevs), GFP_KERNEL);
+	if (!notifier->subdevs)
+		return -ENOMEM;
+
+	notifier->num_subdevs = 0;
+	while (notifier->num_subdevs < CSI_SWITCH_SUBDEVS &&
+	       (node = of_graph_get_next_endpoint(dev->of_node, node))) {
+		struct v4l2_of_endpoint vep;
+		struct vbs_async_subdev *ssd;
+
+		/* skip first port (connected to isp) */
+		v4l2_of_parse_endpoint(node, &vep);
+		if (vep.base.port == 0) {
+			struct device_node *ispnode;
+
+			ispnode = of_graph_get_remote_port_parent(node);
+			if (!ispnode) {
+				dev_warn(dev, "bad remote port parent\n");
+				return -EINVAL;
+			}
+
+			of_node_put(node);
+			continue;
+		}
+
+		ssd = devm_kzalloc(dev, sizeof(*ssd), GFP_KERNEL);
+		if (!ssd) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		ssd->port = vep.base.port;
+
+		notifier->subdevs[notifier->num_subdevs] = &ssd->asd;
+
+		ssd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+		of_node_put(node);
+		if (!ssd->asd.match.of.node) {
+			dev_warn(dev, "bad remote port parent\n");
+			return -EINVAL;
+		}
+
+		ssd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+		pdata->vep[notifier->num_subdevs] = vep;
+		notifier->num_subdevs++;
+	}
+
+	return notifier->num_subdevs;
+}
+
+static int vbs_registered(struct v4l2_subdev *sd)
+{
+	struct v4l2_device *v4l2_dev = sd->v4l2_dev;
+	struct vbs_data *pdata;
+	int err;
+
+	dev_dbg(sd->dev, "registered, init notifier...\n");
+
+	pdata = v4l2_get_subdevdata(sd);
+
+	err = v4l2_async_notifier_register(v4l2_dev, &pdata->notifier);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static struct v4l2_subdev *vbs_get_remote_subdev(struct v4l2_subdev *sd)
+{
+	struct vbs_data *pdata = v4l2_get_subdevdata(sd);
+	struct media_entity *src;
+
+	if (pdata->state == CSI_SWITCH_DISABLED)
+		return ERR_PTR(-ENXIO);
+
+	src = pdata->src_pads[pdata->state].src;
+
+	return media_entity_to_v4l2_subdev(src);
+}
+
+static int vbs_link_setup(struct media_entity *entity,
+			  const struct media_pad *local,
+			  const struct media_pad *remote, u32 flags)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct vbs_data *pdata = v4l2_get_subdevdata(sd);
+	bool enable = flags & MEDIA_LNK_FL_ENABLED;
+
+	if (local->index > CSI_SWITCH_PORTS - 1)
+		return -ENXIO;
+
+	/* no configuration needed on source port */
+	if (local->index == 0)
+		return 0;
+
+	if (!enable) {
+		if (local->index == pdata->state) {
+			pdata->state = CSI_SWITCH_DISABLED;
+
+			/* Make sure we have both cameras enabled */
+			gpiod_set_value(pdata->swgpio, 1);
+			return 0;
+		} else {
+			return -EINVAL;
+		}
+	}
+
+	/* there can only be one active sink at the same time */
+	if (pdata->state != CSI_SWITCH_DISABLED)
+		return -EBUSY;
+
+	dev_dbg(sd->dev, "Link setup: going to config %d\n", local->index);
+
+	gpiod_set_value(pdata->swgpio, local->index == CSI_SWITCH_PORT_2);
+	pdata->state = local->index;
+
+	sd = vbs_get_remote_subdev(sd);
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+
+	pdata->subdev.ctrl_handler = sd->ctrl_handler;
+
+	return 0;
+}
+
+static int vbs_subdev_notifier_bound(struct v4l2_async_notifier *async,
+				     struct v4l2_subdev *subdev,
+				     struct v4l2_async_subdev *asd)
+{
+	struct vbs_data *pdata = container_of(async,
+		struct vbs_data, notifier);
+	struct vbs_async_subdev *ssd =
+		container_of(asd, struct vbs_async_subdev, asd);
+	struct media_entity *sink = &pdata->subdev.entity;
+	struct media_entity *src = &subdev->entity;
+	int sink_pad = ssd->port;
+	int src_pad;
+
+	if (sink_pad >= sink->num_pads) {
+		dev_err(pdata->subdev.dev, "no sink pad in internal entity!\n");
+		return -EINVAL;
+	}
+
+	for (src_pad = 0; src_pad < subdev->entity.num_pads; src_pad++) {
+		if (subdev->entity.pads[src_pad].flags & MEDIA_PAD_FL_SOURCE)
+			break;
+	}
+
+	if (src_pad >= src->num_pads) {
+		dev_err(pdata->subdev.dev, "no source pad in external entity\n");
+		return -EINVAL;
+	}
+
+	pdata->src_pads[sink_pad].src = src;
+	pdata->src_pads[sink_pad].src_pad = src_pad;
+	ssd->sd = subdev;
+
+	return 0;
+}
+
+static int vbs_subdev_notifier_complete(struct v4l2_async_notifier *async)
+{
+	struct vbs_data *pdata = container_of(async, struct vbs_data, notifier);
+	struct media_entity *sink = &pdata->subdev.entity;
+	int sink_pad;
+
+	for (sink_pad = 1; sink_pad < CSI_SWITCH_PORTS; sink_pad++) {
+		struct media_entity *src = pdata->src_pads[sink_pad].src;
+		int src_pad = pdata->src_pads[sink_pad].src_pad;
+		int err;
+
+		err = media_create_pad_link(src, src_pad, sink, sink_pad, 0);
+		if (err < 0)
+			return err;
+
+		dev_dbg(pdata->subdev.dev, "create link: %s[%d] -> %s[%d])\n",
+			src->name, src_pad, sink->name, sink_pad);
+	}
+
+	return v4l2_device_register_subdev_nodes(pdata->subdev.v4l2_dev);
+}
+
+static int vbs_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct v4l2_subdev *subdev = vbs_get_remote_subdev(sd);
+
+	if (IS_ERR(subdev))
+		return PTR_ERR(subdev);
+
+	return v4l2_subdev_call(subdev, video, s_stream, enable);
+}
+
+static int vbs_g_endpoint_config(struct v4l2_subdev *sd, struct v4l2_of_endpoint *cfg)
+{
+	struct vbs_data *pdata = v4l2_get_subdevdata(sd);
+	dev_dbg(sd->dev, "vbs_g_endpoint_config... active port is %d\n", pdata->state);
+	*cfg = pdata->vep[pdata->state - 1];
+
+	return 0;
+}
+
+
+static const struct v4l2_subdev_internal_ops vbs_internal_ops = {
+	.registered = &vbs_registered,
+};
+
+static const struct media_entity_operations vbs_media_ops = {
+	.link_setup = vbs_link_setup,
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+/* subdev video operations */
+static const struct v4l2_subdev_video_ops vbs_video_ops = {
+	.s_stream = vbs_s_stream,
+	.g_endpoint_config = vbs_g_endpoint_config,
+};
+
+static const struct v4l2_subdev_ops vbs_ops = {
+	.video = &vbs_video_ops,
+};
+
+static int video_bus_switch_probe(struct platform_device *pdev)
+{
+	struct vbs_data *pdata;
+	int err = 0;
+
+	/* platform data */
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_dbg(&pdev->dev, "video-bus-switch: not enough memory\n");
+		return -ENOMEM;
+	}
+	platform_set_drvdata(pdev, pdata);
+
+	/* switch gpio */
+	pdata->swgpio = devm_gpiod_get(&pdev->dev, "switch", GPIOD_OUT_HIGH);
+	if (IS_ERR(pdata->swgpio)) {
+		err = PTR_ERR(pdata->swgpio);
+		dev_err(&pdev->dev, "Failed to request gpio: %d\n", err);
+		return err;
+	}
+
+	/* find sub-devices */
+	err = vbs_of_parse_nodes(&pdev->dev, pdata);
+	if (err < 0) {
+		dev_err(&pdev->dev, "Failed to parse nodes: %d\n", err);
+		return err;
+	}
+
+	pdata->state = CSI_SWITCH_DISABLED;
+	pdata->notifier.bound = vbs_subdev_notifier_bound;
+	pdata->notifier.complete = vbs_subdev_notifier_complete;
+
+	/* setup subdev */
+	pdata->pads[0].flags = MEDIA_PAD_FL_SOURCE;
+	pdata->pads[1].flags = MEDIA_PAD_FL_SINK;
+	pdata->pads[2].flags = MEDIA_PAD_FL_SINK;
+
+	v4l2_subdev_init(&pdata->subdev, &vbs_ops);
+	pdata->subdev.dev = &pdev->dev;
+	pdata->subdev.owner = pdev->dev.driver->owner;
+	strncpy(pdata->subdev.name, dev_name(&pdev->dev), V4L2_SUBDEV_NAME_SIZE);
+	v4l2_set_subdevdata(&pdata->subdev, pdata);
+	pdata->subdev.entity.function = MEDIA_ENT_F_SWITCH;
+	pdata->subdev.entity.flags |= MEDIA_ENT_F_SWITCH;
+	pdata->subdev.entity.ops = &vbs_media_ops;
+	pdata->subdev.internal_ops = &vbs_internal_ops;
+	err = media_entity_pads_init(&pdata->subdev.entity, CSI_SWITCH_PORTS,
+				pdata->pads);
+	if (err < 0) {
+		dev_err(&pdev->dev, "Failed to init media entity: %d\n", err);
+		return err;
+	}
+
+	/* register subdev */
+	err = v4l2_async_register_subdev(&pdata->subdev);
+	if (err < 0) {
+		dev_err(&pdev->dev, "Failed to register v4l2 subdev: %d\n", err);
+		media_entity_cleanup(&pdata->subdev.entity);
+		return err;
+	}
+
+	dev_info(&pdev->dev, "video-bus-switch registered\n");
+
+	return 0;
+}
+
+static int video_bus_switch_remove(struct platform_device *pdev)
+{
+	struct vbs_data *pdata = platform_get_drvdata(pdev);
+
+	v4l2_async_notifier_unregister(&pdata->notifier);
+	v4l2_async_unregister_subdev(&pdata->subdev);
+	media_entity_cleanup(&pdata->subdev.entity);
+
+	return 0;
+}
+
+static const struct of_device_id video_bus_switch_of_match[] = {
+	{ .compatible = "video-bus-switch" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, video_bus_switch_of_match);
+
+static struct platform_driver video_bus_switch_driver = {
+	.driver = {
+		.name	= "video-bus-switch",
+		.of_match_table = video_bus_switch_of_match,
+	},
+	.probe		= video_bus_switch_probe,
+	.remove		= video_bus_switch_remove,
+};
+
+module_platform_driver(video_bus_switch_driver);
+
+MODULE_AUTHOR("Sebastian Reichel <sre@kernel.org>");
+MODULE_DESCRIPTION("Video Bus Switch");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:video-bus-switch");
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index cf778c5..448dbb5 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -25,6 +25,7 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-mediabus.h>
+#include <media/v4l2-of.h>
 
 /* generic v4l2_device notify callback notification values */
 #define V4L2_SUBDEV_IR_RX_NOTIFY		_IOW('v', 0, u32)
@@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
 			     const struct v4l2_mbus_config *cfg);
 	int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
 			   unsigned int *size);
+	int (*g_endpoint_config)(struct v4l2_subdev *sd,
+			    struct v4l2_of_endpoint *cfg);
 };
 
 /**
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4890787..94648ab 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -147,6 +147,7 @@ struct media_device_info {
  * MEDIA_ENT_F_IF_VID_DECODER and/or MEDIA_ENT_F_IF_AUD_DECODER.
  */
 #define MEDIA_ENT_F_TUNER		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 5)
+#define MEDIA_ENT_F_SWITCH		(MEDIA_ENT_F_OLD_SUBDEV_BASE + 6)
 
 #define MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN	MEDIA_ENT_F_OLD_SUBDEV_BASE
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sakari Ailus]

Hi Pavel,

On Fri, Feb 03, 2017 at 10:06:10PM +0100, Pavel Machek wrote:
> Hi!
> 
> > My apologies for the delays in reviewing. Feel free to ping me in the future
> > if this happens. :-)
> 
> No problem :-). If you could review the C-code, too... that would be
> nice. It should be in your inbox somewhere (and I attached it below,
> without the dts part).
> 
> 
> > > +Required properties
> > > +===================
> > > +
> > > +compatible	: must contain "video-bus-switch"
> > 
> > How generic is this? Should we have e.g. nokia,video-bus-switch? And if so,
> > change the file name accordingly.
> 
> Generic for "single GPIO controls the switch", AFAICT. But that should
> be common enough...

Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.

> 
> > > +reg		: The interface:
> > > +		  0 - port for image signal processor
> > > +		  1 - port for first camera sensor
> > > +		  2 - port for second camera sensor
> > 
> > I'd say this must be pretty much specific to the one in N900. You could have
> > more ports. Or you could say that ports beyond 0 are camera sensors. I guess
> > this is good enough for now though, it can be changed later on with the
> > source if a need arises.
> 
> Well, I'd say that selecting between two sensors is going to be the
> common case. If someone needs more than two, it will no longer be
> simple GPIO, so we'll have some fixing to do.

It could be two GPIOs --- that's how the GPIO I2C mux works.

But I'd be surprised if someone ever uses something like that again. ;-)

> 
> > Btw. was it still considered a problem that the endpoint properties for the
> > sensors can be different? With the g_routing() pad op which is to be added,
> > the ISP driver (should actually go to a framework somewhere) could parse the
> > graph and find the proper endpoint there.
> 
> I don't know about g_routing. I added g_endpoint_config method that
> passes the configuration, and that seems to work for me.
> 
> I don't see g_routing in next-20170201 . Is there place to look?

I think there was a patch by Laurent to LMML quite some time ago. I suppose
that set will be repicked soonish.

I don't really object using g_endpoint_config() as a temporary solution; I'd
like to have Laurent's opinion on that though. Another option is to wait,
but we've already waited a looong time (as in total).

I'll reply to the other patch containing the code.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

Hi!

> > > > +Required properties
> > > > +===================
> > > > +
> > > > +compatible	: must contain "video-bus-switch"
> > > 
> > > How generic is this? Should we have e.g. nokia,video-bus-switch? And if so,
> > > change the file name accordingly.
> > 
> > Generic for "single GPIO controls the switch", AFAICT. But that should
> > be common enough...
> 
> Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.

Ok, done. I also fixed the english a bit.

> > > > +reg		: The interface:
> > > > +		  0 - port for image signal processor
> > > > +		  1 - port for first camera sensor
> > > > +		  2 - port for second camera sensor
> > > 
> > > I'd say this must be pretty much specific to the one in N900. You could have
> > > more ports. Or you could say that ports beyond 0 are camera sensors. I guess
> > > this is good enough for now though, it can be changed later on with the
> > > source if a need arises.
> > 
> > Well, I'd say that selecting between two sensors is going to be the
> > common case. If someone needs more than two, it will no longer be
> > simple GPIO, so we'll have some fixing to do.
> 
> It could be two GPIOs --- that's how the GPIO I2C mux works.
> 
> But I'd be surprised if someone ever uses something like that
> again. ;-)

I'd say.. lets handle that when we see hardware like that.

> > > Btw. was it still considered a problem that the endpoint properties for the
> > > sensors can be different? With the g_routing() pad op which is to be added,
> > > the ISP driver (should actually go to a framework somewhere) could parse the
> > > graph and find the proper endpoint there.
> > 
> > I don't know about g_routing. I added g_endpoint_config method that
> > passes the configuration, and that seems to work for me.
> > 
> > I don't see g_routing in next-20170201 . Is there place to look?
> 
> I think there was a patch by Laurent to LMML quite some time ago. I suppose
> that set will be repicked soonish.
> 
> I don't really object using g_endpoint_config() as a temporary solution; I'd
> like to have Laurent's opinion on that though. Another option is to wait,
> but we've already waited a looong time (as in total).

Laurent, do you have some input here? We have simple "2 cameras
connected to one signal processor" situation here. We need some way of
passing endpoint configuration from the sensors through the switch. I
did this:

> > @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> >                          const struct v4l2_mbus_config *cfg);
> >     int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> >                        unsigned int *size);
> > +   int (*g_endpoint_config)(struct v4l2_subdev *sd,
> > +                       struct v4l2_of_endpoint *cfg);

Google of g_routing tells me:

9) Highly reconfigurable hardware - Julien Beraud

- 44 sub-devices connected with an interconnect.
- As long as formats match, any sub-device could be connected to any
- other sub-device through a link.
- The result is 44 * 44 links at worst.
- A switch sub-device proposed as the solution to model the
- interconnect. The sub-devices are connected to the switch
- sub-devices through the hardware links that connect to the
- interconnect.
- The switch would be controlled through new IOCTLs S_ROUTING and
- G_ROUTING.
- Patches available:
 http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip

but the patches are from 2005. So I guess I'll need some guidance here...

> I'll reply to the other patch containing the code.

Ok, thanks.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sakari Ailus]

Hi Pavel,

On Sat, Feb 04, 2017 at 10:56:10PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > > +Required properties
> > > > > +===================
> > > > > +
> > > > > +compatible	: must contain "video-bus-switch"
> > > > 
> > > > How generic is this? Should we have e.g. nokia,video-bus-switch? And if so,
> > > > change the file name accordingly.
> > > 
> > > Generic for "single GPIO controls the switch", AFAICT. But that should
> > > be common enough...
> > 
> > Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.
> 
> Ok, done. I also fixed the english a bit.
> 
> > > > > +reg		: The interface:
> > > > > +		  0 - port for image signal processor
> > > > > +		  1 - port for first camera sensor
> > > > > +		  2 - port for second camera sensor
> > > > 
> > > > I'd say this must be pretty much specific to the one in N900. You could have
> > > > more ports. Or you could say that ports beyond 0 are camera sensors. I guess
> > > > this is good enough for now though, it can be changed later on with the
> > > > source if a need arises.
> > > 
> > > Well, I'd say that selecting between two sensors is going to be the
> > > common case. If someone needs more than two, it will no longer be
> > > simple GPIO, so we'll have some fixing to do.
> > 
> > It could be two GPIOs --- that's how the GPIO I2C mux works.
> > 
> > But I'd be surprised if someone ever uses something like that
> > again. ;-)
> 
> I'd say.. lets handle that when we see hardware like that.

Yes. :-)

> 
> > > > Btw. was it still considered a problem that the endpoint properties for the
> > > > sensors can be different? With the g_routing() pad op which is to be added,
> > > > the ISP driver (should actually go to a framework somewhere) could parse the
> > > > graph and find the proper endpoint there.
> > > 
> > > I don't know about g_routing. I added g_endpoint_config method that
> > > passes the configuration, and that seems to work for me.
> > > 
> > > I don't see g_routing in next-20170201 . Is there place to look?
> > 
> > I think there was a patch by Laurent to LMML quite some time ago. I suppose
> > that set will be repicked soonish.
> > 
> > I don't really object using g_endpoint_config() as a temporary solution; I'd
> > like to have Laurent's opinion on that though. Another option is to wait,
> > but we've already waited a looong time (as in total).
> 
> Laurent, do you have some input here? We have simple "2 cameras
> connected to one signal processor" situation here. We need some way of
> passing endpoint configuration from the sensors through the switch. I
> did this:
> 
> > > @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> > >                          const struct v4l2_mbus_config *cfg);
> > >     int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> > >                        unsigned int *size);
> > > +   int (*g_endpoint_config)(struct v4l2_subdev *sd,
> > > +                       struct v4l2_of_endpoint *cfg);
> 
> Google of g_routing tells me:
> 
> 9) Highly reconfigurable hardware - Julien Beraud
> 
> - 44 sub-devices connected with an interconnect.
> - As long as formats match, any sub-device could be connected to any
> - other sub-device through a link.
> - The result is 44 * 44 links at worst.
> - A switch sub-device proposed as the solution to model the
> - interconnect. The sub-devices are connected to the switch
> - sub-devices through the hardware links that connect to the
> - interconnect.
> - The switch would be controlled through new IOCTLs S_ROUTING and
> - G_ROUTING.
> - Patches available:
>  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> 
> but the patches are from 2005. So I guess I'll need some guidance here...

Yeah, that's where it began (2015?), but right now I can only suggest to
wait until there's more. My estimate is within next couple of weeks /
months. But it won't be years.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

Hi!

> > 9) Highly reconfigurable hardware - Julien Beraud
> > 
> > - 44 sub-devices connected with an interconnect.
> > - As long as formats match, any sub-device could be connected to any
> > - other sub-device through a link.
> > - The result is 44 * 44 links at worst.
> > - A switch sub-device proposed as the solution to model the
> > - interconnect. The sub-devices are connected to the switch
> > - sub-devices through the hardware links that connect to the
> > - interconnect.
> > - The switch would be controlled through new IOCTLs S_ROUTING and
> > - G_ROUTING.
> > - Patches available:
> >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > 
> > but the patches are from 2005. So I guess I'll need some guidance here...
> 
> Yeah, that's where it began (2015?), but right now I can only suggest to
> wait until there's more. My estimate is within next couple of weeks /
> months. But it won't be years.

Ok, week or two would be ok, couple of months is not. And all I need
is single hook in common structure.

So if g_endpoint_config hook looks sane to _you_, I suggest we simply
proceed. Now, maybe Mauro Carvalho Chehab <mchehab@s-opensource.com>
or Laurent or Julien will want a different solution, but
then... they'll have to suggest something doable now, not in couple of
months.

Does that sound like a plan?

Mauro added to cc list, so we can get some input.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sebastian Reichel]

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

Hi,

On Sun, Feb 05, 2017 at 10:12:20PM +0100, Pavel Machek wrote:
> > > 9) Highly reconfigurable hardware - Julien Beraud
> > > 
> > > - 44 sub-devices connected with an interconnect.
> > > - As long as formats match, any sub-device could be connected to any
> > > - other sub-device through a link.
> > > - The result is 44 * 44 links at worst.
> > > - A switch sub-device proposed as the solution to model the
> > > - interconnect. The sub-devices are connected to the switch
> > > - sub-devices through the hardware links that connect to the
> > > - interconnect.
> > > - The switch would be controlled through new IOCTLs S_ROUTING and
> > > - G_ROUTING.
> > > - Patches available:
> > >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > > 
> > > but the patches are from 2005. So I guess I'll need some guidance here...
> > 
> > Yeah, that's where it began (2015?), but right now I can only suggest to
> > wait until there's more. My estimate is within next couple of weeks /
> > months. But it won't be years.
> 
> Ok, week or two would be ok, couple of months is not. And all I need
> is single hook in common structure.
> 
> So if g_endpoint_config hook looks sane to _you_, I suggest we simply
> proceed. Now, maybe Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>
> or Laurent or Julien will want a different solution, but
> then... they'll have to suggest something doable now, not in couple of
> months.
> 
> Does that sound like a plan?
> 
> Mauro added to cc list, so we can get some input.

side note: It's an kernel-internal API only used by the media
subsystem. Code can be easily updated to use an improved API
at any time.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Laurent Pinchart]

Hi Pavel,

On Saturday, 4 February 2017 23:56:10 EET Pavel Machek wrote:
> Hi!
> 
> >>>> +Required properties
> >>>> +===================
> >>>> +
> >>>> +compatible	: must contain "video-bus-switch"
> >>> 
> >>> How generic is this? Should we have e.g. nokia,video-bus-switch? And
> >>> if so, change the file name accordingly.
> >> 
> >> Generic for "single GPIO controls the switch", AFAICT. But that should
> >> be common enough...
> > 
> > Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.
> 
> Ok, done. I also fixed the english a bit.
> 
> >>>> +reg		: The interface:
> >>>> +		  0 - port for image signal processor
> >>>> +		  1 - port for first camera sensor
> >>>> +		  2 - port for second camera sensor
> >>> 
> >>> I'd say this must be pretty much specific to the one in N900. You
> >>> could have more ports. Or you could say that ports beyond 0 are
> >>> camera sensors. I guess this is good enough for now though, it can be
> >>> changed later on with the source if a need arises.
> >> 
> >> Well, I'd say that selecting between two sensors is going to be the
> >> common case. If someone needs more than two, it will no longer be
> >> simple GPIO, so we'll have some fixing to do.
> > 
> > It could be two GPIOs --- that's how the GPIO I2C mux works.
> > 
> > But I'd be surprised if someone ever uses something like that
> > again. ;-)
> 
> I'd say.. lets handle that when we see hardware like that.
> 
> >>> Btw. was it still considered a problem that the endpoint properties
> >>> for the sensors can be different? With the g_routing() pad op which is
> >>> to be added, the ISP driver (should actually go to a framework
> >>> somewhere) could parse the graph and find the proper endpoint there.
> >> 
> >> I don't know about g_routing. I added g_endpoint_config method that
> >> passes the configuration, and that seems to work for me.
> >> 
> >> I don't see g_routing in next-20170201 . Is there place to look?
> > 
> > I think there was a patch by Laurent to LMML quite some time ago. I
> > suppose that set will be repicked soonish.
> > 
> > I don't really object using g_endpoint_config() as a temporary solution;
> > I'd like to have Laurent's opinion on that though. Another option is to
> > wait, but we've already waited a looong time (as in total).
> 
> Laurent, do you have some input here? We have simple "2 cameras
> connected to one signal processor" situation here. We need some way of
> passing endpoint configuration from the sensors through the switch. I
> did this:

Could you give me a bit more information about the platform you're targeting: 
how the switch is connected, what kind of switch it is, and what endpoint 
configuration data you need ?

> >> @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> >>                          const struct v4l2_mbus_config *cfg);
> >>     int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> >>                        unsigned int *size);
> >> +   int (*g_endpoint_config)(struct v4l2_subdev *sd,
> >> +                       struct v4l2_of_endpoint *cfg);
> 
> Google of g_routing tells me:
> 
> 9) Highly reconfigurable hardware - Julien Beraud
> 
> - 44 sub-devices connected with an interconnect.
> - As long as formats match, any sub-device could be connected to any
> - other sub-device through a link.
> - The result is 44 * 44 links at worst.
> - A switch sub-device proposed as the solution to model the
> - interconnect. The sub-devices are connected to the switch
> - sub-devices through the hardware links that connect to the
> - interconnect.
> - The switch would be controlled through new IOCTLs S_ROUTING and
> - G_ROUTING.
> - Patches available:
>  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> 
> but the patches are from 2005. So I guess I'll need some guidance here...

You made me feel very old for a moment. The patches are from 2015 :-)

> > I'll reply to the other patch containing the code.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Add video bus switch

[Sakari Ailus]

On Wed, Dec 20, 2017 at 07:54:12PM +0200, Laurent Pinchart wrote:
> Hi Pavel,
> 
> On Saturday, 4 February 2017 23:56:10 EET Pavel Machek wrote:
> > Hi!
> > 
> > >>>> +Required properties
> > >>>> +===================
> > >>>> +
> > >>>> +compatible	: must contain "video-bus-switch"
> > >>> 
> > >>> How generic is this? Should we have e.g. nokia,video-bus-switch? And
> > >>> if so, change the file name accordingly.
> > >> 
> > >> Generic for "single GPIO controls the switch", AFAICT. But that should
> > >> be common enough...
> > > 
> > > Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.
> > 
> > Ok, done. I also fixed the english a bit.
> > 
> > >>>> +reg		: The interface:
> > >>>> +		  0 - port for image signal processor
> > >>>> +		  1 - port for first camera sensor
> > >>>> +		  2 - port for second camera sensor
> > >>> 
> > >>> I'd say this must be pretty much specific to the one in N900. You
> > >>> could have more ports. Or you could say that ports beyond 0 are
> > >>> camera sensors. I guess this is good enough for now though, it can be
> > >>> changed later on with the source if a need arises.
> > >> 
> > >> Well, I'd say that selecting between two sensors is going to be the
> > >> common case. If someone needs more than two, it will no longer be
> > >> simple GPIO, so we'll have some fixing to do.
> > > 
> > > It could be two GPIOs --- that's how the GPIO I2C mux works.
> > > 
> > > But I'd be surprised if someone ever uses something like that
> > > again. ;-)
> > 
> > I'd say.. lets handle that when we see hardware like that.
> > 
> > >>> Btw. was it still considered a problem that the endpoint properties
> > >>> for the sensors can be different? With the g_routing() pad op which is
> > >>> to be added, the ISP driver (should actually go to a framework
> > >>> somewhere) could parse the graph and find the proper endpoint there.
> > >> 
> > >> I don't know about g_routing. I added g_endpoint_config method that
> > >> passes the configuration, and that seems to work for me.
> > >> 
> > >> I don't see g_routing in next-20170201 . Is there place to look?
> > > 
> > > I think there was a patch by Laurent to LMML quite some time ago. I
> > > suppose that set will be repicked soonish.
> > > 
> > > I don't really object using g_endpoint_config() as a temporary solution;
> > > I'd like to have Laurent's opinion on that though. Another option is to
> > > wait, but we've already waited a looong time (as in total).
> > 
> > Laurent, do you have some input here? We have simple "2 cameras
> > connected to one signal processor" situation here. We need some way of
> > passing endpoint configuration from the sensors through the switch. I
> > did this:
> 
> Could you give me a bit more information about the platform you're targeting: 
> how the switch is connected, what kind of switch it is, and what endpoint 
> configuration data you need ?
> 
> > >> @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> > >>                          const struct v4l2_mbus_config *cfg);
> > >>     int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> > >>                        unsigned int *size);
> > >> +   int (*g_endpoint_config)(struct v4l2_subdev *sd,
> > >> +                       struct v4l2_of_endpoint *cfg);
> > 
> > Google of g_routing tells me:
> > 
> > 9) Highly reconfigurable hardware - Julien Beraud
> > 
> > - 44 sub-devices connected with an interconnect.
> > - As long as formats match, any sub-device could be connected to any
> > - other sub-device through a link.
> > - The result is 44 * 44 links at worst.
> > - A switch sub-device proposed as the solution to model the
> > - interconnect. The sub-devices are connected to the switch
> > - sub-devices through the hardware links that connect to the
> > - interconnect.
> > - The switch would be controlled through new IOCTLs S_ROUTING and
> > - G_ROUTING.
> > - Patches available:
> >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > 
> > but the patches are from 2005. So I guess I'll need some guidance here...
> 
> You made me feel very old for a moment. The patches are from 2015 :-)

There are up-to-date patches here:

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

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Add video bus switch

[Ivaylo Dimitrov]

Hi,

On 20.12.2017 19:54, Laurent Pinchart wrote:
> Hi Pavel,
> 
> On Saturday, 4 February 2017 23:56:10 EET Pavel Machek wrote:
>> Hi!
>>
>>>>>> +Required properties
>>>>>> +===================
>>>>>> +
>>>>>> +compatible	: must contain "video-bus-switch"
>>>>>
>>>>> How generic is this? Should we have e.g. nokia,video-bus-switch? And
>>>>> if so, change the file name accordingly.
>>>>
>>>> Generic for "single GPIO controls the switch", AFAICT. But that should
>>>> be common enough...
>>>
>>> Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.
>>
>> Ok, done. I also fixed the english a bit.
>>
>>>>>> +reg		: The interface:
>>>>>> +		  0 - port for image signal processor
>>>>>> +		  1 - port for first camera sensor
>>>>>> +		  2 - port for second camera sensor
>>>>>
>>>>> I'd say this must be pretty much specific to the one in N900. You
>>>>> could have more ports. Or you could say that ports beyond 0 are
>>>>> camera sensors. I guess this is good enough for now though, it can be
>>>>> changed later on with the source if a need arises.
>>>>
>>>> Well, I'd say that selecting between two sensors is going to be the
>>>> common case. If someone needs more than two, it will no longer be
>>>> simple GPIO, so we'll have some fixing to do.
>>>
>>> It could be two GPIOs --- that's how the GPIO I2C mux works.
>>>
>>> But I'd be surprised if someone ever uses something like that
>>> again. ;-)
>>
>> I'd say.. lets handle that when we see hardware like that.
>>
>>>>> Btw. was it still considered a problem that the endpoint properties
>>>>> for the sensors can be different? With the g_routing() pad op which is
>>>>> to be added, the ISP driver (should actually go to a framework
>>>>> somewhere) could parse the graph and find the proper endpoint there.
>>>>
>>>> I don't know about g_routing. I added g_endpoint_config method that
>>>> passes the configuration, and that seems to work for me.
>>>>
>>>> I don't see g_routing in next-20170201 . Is there place to look?
>>>
>>> I think there was a patch by Laurent to LMML quite some time ago. I
>>> suppose that set will be repicked soonish.
>>>
>>> I don't really object using g_endpoint_config() as a temporary solution;
>>> I'd like to have Laurent's opinion on that though. Another option is to
>>> wait, but we've already waited a looong time (as in total).
>>
>> Laurent, do you have some input here? We have simple "2 cameras
>> connected to one signal processor" situation here. We need some way of
>> passing endpoint configuration from the sensors through the switch. I
>> did this:
> 
> Could you give me a bit more information about the platform you're targeting:
> how the switch is connected, what kind of switch it is, and what endpoint


http://plan9.stanleylieber.com/hardware/n900/n900.schematics.pdf, on 
page 2, see N5801 and N5802.

> configuration data you need ?
> 
>>>> @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
>>>>                           const struct v4l2_mbus_config *cfg);
>>>>      int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
>>>>                         unsigned int *size);
>>>> +   int (*g_endpoint_config)(struct v4l2_subdev *sd,
>>>> +                       struct v4l2_of_endpoint *cfg);
>>
>> Google of g_routing tells me:
>>
>> 9) Highly reconfigurable hardware - Julien Beraud
>>
>> - 44 sub-devices connected with an interconnect.
>> - As long as formats match, any sub-device could be connected to any
>> - other sub-device through a link.
>> - The result is 44 * 44 links at worst.
>> - A switch sub-device proposed as the solution to model the
>> - interconnect. The sub-devices are connected to the switch
>> - sub-devices through the hardware links that connect to the
>> - interconnect.
>> - The switch would be controlled through new IOCTLs S_ROUTING and
>> - G_ROUTING.
>> - Patches available:
>>   http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
>>
>> but the patches are from 2005. So I guess I'll need some guidance here...
> 
> You made me feel very old for a moment. The patches are from 2015 :-)
> 
>>> I'll reply to the other patch containing the code.
> 

Regards,
Ivo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

Hi!

> > > I don't really object using g_endpoint_config() as a temporary solution;
> > > I'd like to have Laurent's opinion on that though. Another option is to
> > > wait, but we've already waited a looong time (as in total).
> > 
> > Laurent, do you have some input here? We have simple "2 cameras
> > connected to one signal processor" situation here. We need some way of
> > passing endpoint configuration from the sensors through the switch. I
> > did this:
> 
> Could you give me a bit more information about the platform you're targeting: 
> how the switch is connected, what kind of switch it is, and what endpoint 
> configuration data you need ?

Platform is Nokia N900, Ivaylo already gave pointer to schematics.

Switch is controlled using GPIO, and basically there's CSI
configuration that would normally be in the device tree, but now we
have two CSI configurations to select from...

> > 9) Highly reconfigurable hardware - Julien Beraud
> > 
> > - 44 sub-devices connected with an interconnect.
> > - As long as formats match, any sub-device could be connected to any
> > - other sub-device through a link.
> > - The result is 44 * 44 links at worst.
> > - A switch sub-device proposed as the solution to model the
> > - interconnect. The sub-devices are connected to the switch
> > - sub-devices through the hardware links that connect to the
> > - interconnect.
> > - The switch would be controlled through new IOCTLs S_ROUTING and
> > - G_ROUTING.
> > - Patches available:
> >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > 
> > but the patches are from 2005. So I guess I'll need some guidance here...
> 
> You made me feel very old for a moment. The patches are from 2015 :-)

Sorry about that :-).
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Pali Rohár]

On Friday 03 February 2017 13:35:08 Pavel Machek wrote:
> N900 contains front and back camera, with a switch between the
> two. This adds support for the switch component, and it is now
> possible to select between front and back cameras during runtime.

IIRC for controlling cameras on N900 there are two GPIOs. Should not you
have both in switch driver?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

On Fri 2017-02-03 14:32:19, Pali Rohár wrote:
> On Friday 03 February 2017 13:35:08 Pavel Machek wrote:
> > N900 contains front and back camera, with a switch between the
> > two. This adds support for the switch component, and it is now
> > possible to select between front and back cameras during runtime.
> 
> IIRC for controlling cameras on N900 there are two GPIOs. Should not you
> have both in switch driver?

I guess you recall wrongly :-). Switch seems to work. The issue was
with switch GPIO also serving as reset GPIO for one sensor, or
something like that, if _I_ recall correctly ;-).

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sebastian Reichel]

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

Hi,

On Fri, Feb 03, 2017 at 10:07:28PM +0100, Pavel Machek wrote:
> On Fri 2017-02-03 14:32:19, Pali Rohár wrote:
> > On Friday 03 February 2017 13:35:08 Pavel Machek wrote:
> > > N900 contains front and back camera, with a switch between the
> > > two. This adds support for the switch component, and it is now
> > > possible to select between front and back cameras during runtime.
> > 
> > IIRC for controlling cameras on N900 there are two GPIOs. Should
> > not you have both in switch driver?
>
> I guess you recall wrongly :-). Switch seems to work. The issue was
> with switch GPIO also serving as reset GPIO for one sensor, or
> something like that, if _I_ recall correctly ;-).

I have a schematic in my master thesis, which shows how the camera
sensors are connected to the SoC. The PDF is available here:

https://www.uni-oldenburg.de/fileadmin/user_upload/informatik/ag/svs/download/thesis/Reichel_Sebastian.pdf

The schematic is on page 37 (or 45 if your PDF reader does not
use different numbers for the preamble stuff).

--Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Rob Herring]

On Fri, Feb 03, 2017 at 01:35:08PM +0100, Pavel Machek wrote:
> 
> N900 contains front and back camera, with a switch between the
> two. This adds support for the switch component, and it is now
> possible to select between front and back cameras during runtime.
> 
> This adds documentation for the devicetree binding.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> 
> diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> new file mode 100644
> index 0000000..1b9f8e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> @@ -0,0 +1,63 @@
> +Video Bus Switch Binding
> +========================

I'd call it a mux rather than switch.

BTW, there's a new mux-controller binding under review you might look 
at. It would only be needed here if the mux ctrl also controls other 
things.

> +
> +This is a binding for a gpio controlled switch for camera interfaces. Such a
> +device is used on some embedded devices to connect two cameras to the same
> +interface of a image signal processor.
> +
> +Required properties
> +===================
> +
> +compatible	: must contain "video-bus-switch"

video-bus-gpio-mux

> +switch-gpios	: GPIO specifier for the gpio, which can toggle the

mux-gpios to align with existing GPIO controlled muxes.

> +		  selected camera. The GPIO should be configured, so
> +		  that a disabled GPIO means, that the first port is
> +		  selected.
> +
> +Required Port nodes
> +===================
> +
> +More documentation on these bindings is available in
> +video-interfaces.txt in the same directory.
> +
> +reg		: The interface:
> +		  0 - port for image signal processor
> +		  1 - port for first camera sensor
> +		  2 - port for second camera sensor

This could be used for display side as well. So describe these just as 
inputs and outputs.

Rob

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

On Wed 2017-02-08 15:36:09, Rob Herring wrote:
> On Fri, Feb 03, 2017 at 01:35:08PM +0100, Pavel Machek wrote:
> > 
> > N900 contains front and back camera, with a switch between the
> > two. This adds support for the switch component, and it is now
> > possible to select between front and back cameras during runtime.
> > 
> > This adds documentation for the devicetree binding.
> > 
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > 
> > diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> > new file mode 100644
> > index 0000000..1b9f8e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> > @@ -0,0 +1,63 @@
> > +Video Bus Switch Binding
> > +========================
> 
> I'd call it a mux rather than switch.

It is a switch, not a multiplexor (
https://en.wikipedia.org/wiki/Multiplexing ). Only one camera can
operate at a time.

> BTW, there's a new mux-controller binding under review you might look 
> at. It would only be needed here if the mux ctrl also controls other 
> things.

Do you have a pointer?

> > +Required Port nodes
> > +===================
> > +
> > +More documentation on these bindings is available in
> > +video-interfaces.txt in the same directory.
> > +
> > +reg		: The interface:
> > +		  0 - port for image signal processor
> > +		  1 - port for first camera sensor
> > +		  2 - port for second camera sensor
> 
> This could be used for display side as well. So describe these just as 
> inputs and outputs.

I'd prefer not to confuse people. I guess that would be 0 -- output
port, 1, 2 -- input ports... But this is media data, are you sure it
is good idea to change this?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Rob Herring]

On Wed, Feb 8, 2017 at 4:30 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Wed 2017-02-08 15:36:09, Rob Herring wrote:
>> On Fri, Feb 03, 2017 at 01:35:08PM +0100, Pavel Machek wrote:
>> >
>> > N900 contains front and back camera, with a switch between the
>> > two. This adds support for the switch component, and it is now
>> > possible to select between front and back cameras during runtime.
>> >
>> > This adds documentation for the devicetree binding.
>> >
>> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
>> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
>> >
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
>> > new file mode 100644
>> > index 0000000..1b9f8e0
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
>> > @@ -0,0 +1,63 @@
>> > +Video Bus Switch Binding
>> > +========================
>>
>> I'd call it a mux rather than switch.
>
> It is a switch, not a multiplexor (
> https://en.wikipedia.org/wiki/Multiplexing ). Only one camera can
> operate at a time.

It's no different than an i2c mux. It's one at a time.

>
>> BTW, there's a new mux-controller binding under review you might look
>> at. It would only be needed here if the mux ctrl also controls other
>> things.
>
> Do you have a pointer?

Let me Google that for you:

>
>> > +Required Port nodes
>> > +===================
>> > +
>> > +More documentation on these bindings is available in
>> > +video-interfaces.txt in the same directory.
>> > +
>> > +reg                : The interface:
>> > +             0 - port for image signal processor
>> > +             1 - port for first camera sensor
>> > +             2 - port for second camera sensor
>>
>> This could be used for display side as well. So describe these just as
>> inputs and outputs.
>
> I'd prefer not to confuse people. I guess that would be 0 -- output
> port, 1, 2 -- input ports... But this is media data, are you sure it
> is good idea to change this?

And I'd prefer something that can be reused by others.

>
>                                                                         Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] devicetree: Add video bus switch

[Rob Herring]

On Thu, Feb 9, 2017 at 5:02 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Feb 8, 2017 at 4:30 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> On Wed 2017-02-08 15:36:09, Rob Herring wrote:
>>> On Fri, Feb 03, 2017 at 01:35:08PM +0100, Pavel Machek wrote:
>>> >
>>> > N900 contains front and back camera, with a switch between the
>>> > two. This adds support for the switch component, and it is now
>>> > possible to select between front and back cameras during runtime.
>>> >
>>> > This adds documentation for the devicetree binding.
>>> >
>>> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
>>> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>>> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>> >
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
>>> > new file mode 100644
>>> > index 0000000..1b9f8e0
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
>>> > @@ -0,0 +1,63 @@
>>> > +Video Bus Switch Binding
>>> > +========================
>>>
>>> I'd call it a mux rather than switch.
>>
>> It is a switch, not a multiplexor (
>> https://en.wikipedia.org/wiki/Multiplexing ). Only one camera can
>> operate at a time.
>
> It's no different than an i2c mux. It's one at a time.
>
>>
>>> BTW, there's a new mux-controller binding under review you might look
>>> at. It would only be needed here if the mux ctrl also controls other
>>> things.
>>
>> Do you have a pointer?
>
> Let me Google that for you:

https://lwn.net/Articles/713971/

>
>>
>>> > +Required Port nodes
>>> > +===================
>>> > +
>>> > +More documentation on these bindings is available in
>>> > +video-interfaces.txt in the same directory.
>>> > +
>>> > +reg                : The interface:
>>> > +             0 - port for image signal processor
>>> > +             1 - port for first camera sensor
>>> > +             2 - port for second camera sensor
>>>
>>> This could be used for display side as well. So describe these just as
>>> inputs and outputs.
>>
>> I'd prefer not to confuse people. I guess that would be 0 -- output
>> port, 1, 2 -- input ports... But this is media data, are you sure it
>> is good idea to change this?
>
> And I'd prefer something that can be reused by others.
>
>>
>>                                                                         Pavel
>> --
>> (english) http://www.livejournal.com/~pavelmachek
>> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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


> >>> > diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> >>> > new file mode 100644
> >>> > index 0000000..1b9f8e0
> >>> > --- /dev/null
> >>> > +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> >>> > @@ -0,0 +1,63 @@
> >>> > +Video Bus Switch Binding
> >>> > +========================
> >>>
> >>> I'd call it a mux rather than switch.
> >>
> >> It is a switch, not a multiplexor (
> >> https://en.wikipedia.org/wiki/Multiplexing ). Only one camera can
> >> operate at a time.
> >
> > It's no different than an i2c mux. It's one at a time.

Take a look at the wikipedia. If you do "one at a time" at 100Hz, you
can claim it is time-domain multiplex. But we are plain switching the
cameras. It takes second (or so) to setup the pipeline.

This is not multiplex.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sakari Ailus]

Hi Pavel,

On Fri, Feb 10, 2017 at 08:54:35PM +0100, Pavel Machek wrote:
> 
> > >>> > diff --git a/Documentation/devicetree/bindings/media/video-bus-switch.txt b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> > >>> > new file mode 100644
> > >>> > index 0000000..1b9f8e0
> > >>> > --- /dev/null
> > >>> > +++ b/Documentation/devicetree/bindings/media/video-bus-switch.txt
> > >>> > @@ -0,0 +1,63 @@
> > >>> > +Video Bus Switch Binding
> > >>> > +========================
> > >>>
> > >>> I'd call it a mux rather than switch.
> > >>
> > >> It is a switch, not a multiplexor (
> > >> https://en.wikipedia.org/wiki/Multiplexing ). Only one camera can
> > >> operate at a time.
> > >
> > > It's no different than an i2c mux. It's one at a time.
> 
> Take a look at the wikipedia. If you do "one at a time" at 100Hz, you
> can claim it is time-domain multiplex. But we are plain switching the
> cameras. It takes second (or so) to setup the pipeline.
> 
> This is not multiplex.

The functionality is still the same, isn't it? Does it change what it is if
the frequency might be 100 Hz or 0,01 Hz?

I was a bit annoyed for having to have two drivers for switching the source
(one for GPIO, another for syscon / register), where both of the drivers
would be essentially the same with the minor exception of having a slightly
different means to toggle the mux setting.

The MUX framework adds an API for controlling the MUX. Thus we'll need only
a single driver that uses the MUX framework API for V4L2. As an added bonus,
V4L2 would be in line with the rest of the MUX usage in the kernel.

The set appears to already contain a GPIO MUX. What's needed would be to use
the MUX API instead of direct GPIOs usage.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

Hi!

> > Take a look at the wikipedia. If you do "one at a time" at 100Hz, you
> > can claim it is time-domain multiplex. But we are plain switching the
> > cameras. It takes second (or so) to setup the pipeline.
> > 
> > This is not multiplex.
> 
> The functionality is still the same, isn't it? Does it change what it is if
> the frequency might be 100 Hz or 0,01 Hz?

Well. In your living your you can have a switch, which is switch at
much less than 0.01Hz. You can also have a dimmer, which is a PWM,
which is switch at 100Hz or so. So yes, I'd say switch and mux are
different things.

> I was a bit annoyed for having to have two drivers for switching the source
> (one for GPIO, another for syscon / register), where both of the drivers
> would be essentially the same with the minor exception of having a slightly
> different means to toggle the mux setting.

Well, most of the video-bus-switch is the video4linux glue. Actual
switching is very very small part. So.. where is the other driver?
Looks like we have the same problem.

> The MUX framework adds an API for controlling the MUX. Thus we'll need only
> a single driver that uses the MUX framework API for V4L2. As an added bonus,
> V4L2 would be in line with the rest of the MUX usage in the kernel.
> 
> The set appears to already contain a GPIO MUX. What's needed would be to use
> the MUX API instead of direct GPIOs usage.

If there's a driver that already does switching for video4linux
devices? Do you have a pointer?

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Sakari Ailus]

Hi Pavel,

On Mon, Feb 13, 2017 at 10:54:20AM +0100, Pavel Machek wrote:
> Hi!
> 
> > > Take a look at the wikipedia. If you do "one at a time" at 100Hz, you
> > > can claim it is time-domain multiplex. But we are plain switching the
> > > cameras. It takes second (or so) to setup the pipeline.
> > > 
> > > This is not multiplex.
> > 
> > The functionality is still the same, isn't it? Does it change what it is if
> > the frequency might be 100 Hz or 0,01 Hz?
> 
> Well. In your living your you can have a switch, which is switch at
> much less than 0.01Hz. You can also have a dimmer, which is a PWM,
> which is switch at 100Hz or so. So yes, I'd say switch and mux are
> different things.

Light switches are mostly on/off switches. It'd be interesting to have a
light switch that you could use to light either of the light bulbs in a room
but not to switch both of them on at the same time. Or off... :-)

I wonder if everyone would be happy with a statement saying that it's a
on / on switch which is used to implement a multiplexer?

> 
> > I was a bit annoyed for having to have two drivers for switching the source
> > (one for GPIO, another for syscon / register), where both of the drivers
> > would be essentially the same with the minor exception of having a slightly
> > different means to toggle the mux setting.
> 
> Well, most of the video-bus-switch is the video4linux glue. Actual
> switching is very very small part. So.. where is the other driver?
> Looks like we have the same problem.

It's here, slightly hidden in plain sight in the same patch with the MUX
framework:

<URL:https://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1328763.html>

> 
> > The MUX framework adds an API for controlling the MUX. Thus we'll need only
> > a single driver that uses the MUX framework API for V4L2. As an added bonus,
> > V4L2 would be in line with the rest of the MUX usage in the kernel.
> > 
> > The set appears to already contain a GPIO MUX. What's needed would be to use
> > the MUX API instead of direct GPIOs usage.
> 
> If there's a driver that already does switching for video4linux
> devices? Do you have a pointer?

I don't think there's one. But with MUX API, we'll be fine using a single
driver instead of two (other one for syscon on iMX).

Cc Steve and Philipp.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

On Mon 2017-02-13 12:20:35, Sakari Ailus wrote:
> Hi Pavel,
> 
> On Mon, Feb 13, 2017 at 10:54:20AM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > > Take a look at the wikipedia. If you do "one at a time" at 100Hz, you
> > > > can claim it is time-domain multiplex. But we are plain switching the
> > > > cameras. It takes second (or so) to setup the pipeline.
> > > > 
> > > > This is not multiplex.
> > > 
> > > The functionality is still the same, isn't it? Does it change what it is if
> > > the frequency might be 100 Hz or 0,01 Hz?
> > 
> > Well. In your living your you can have a switch, which is switch at
> > much less than 0.01Hz. You can also have a dimmer, which is a PWM,
> > which is switch at 100Hz or so. So yes, I'd say switch and mux are
> > different things.
> 
> Light switches are mostly on/off switches. It'd be interesting to have a
> light switch that you could use to light either of the light bulbs in a room
> but not to switch both of them on at the same time. Or off... :-)
> 
> I wonder if everyone would be happy with a statement saying that it's a
> on / on switch which is used to implement a multiplexer?

I believe the difference is the timescale. If it switches "slow" it is
a switch. If it switches fast, it is a dimmer, mux, or something....

Anyway, someone else was faster, so they get to name their creation...

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

> > +
> > +This is a binding for a gpio controlled switch for camera interfaces. Such a
> > +device is used on some embedded devices to connect two cameras to the same
> > +interface of a image signal processor.
> > +
> > +Required properties
> > +===================
> > +
> > +compatible	: must contain "video-bus-switch"
> 
> video-bus-gpio-mux

Sakari already asked for rename here. I believe I waited reasonable
time, but got no input from you, so I did rename it. Now you decide on
different name.

Can we either get timely reactions or less bikeshedding?

Thanks,

                                                                Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] devicetree: Add video bus switch

[Rob Herring]

On Wed, Feb 8, 2017 at 4:34 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> > +
>> > +This is a binding for a gpio controlled switch for camera interfaces. Such a
>> > +device is used on some embedded devices to connect two cameras to the same
>> > +interface of a image signal processor.
>> > +
>> > +Required properties
>> > +===================
>> > +
>> > +compatible : must contain "video-bus-switch"
>>
>> video-bus-gpio-mux
>
> Sakari already asked for rename here. I believe I waited reasonable
> time, but got no input from you, so I did rename it. Now you decide on
> different name.
>
> Can we either get timely reactions or less bikeshedding?

You mean less than 5 days because I don't see any other version of
this? But in short, no, you can't.

Rob

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

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

On Thu 2017-02-09 16:58:29, Rob Herring wrote:
> On Wed, Feb 8, 2017 at 4:34 PM, Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> wrote:
> >> > +
> >> > +This is a binding for a gpio controlled switch for camera interfaces. Such a
> >> > +device is used on some embedded devices to connect two cameras to the same
> >> > +interface of a image signal processor.
> >> > +
> >> > +Required properties
> >> > +===================
> >> > +
> >> > +compatible : must contain "video-bus-switch"
> >>
> >> video-bus-gpio-mux
> >
> > Sakari already asked for rename here. I believe I waited reasonable
> > time, but got no input from you, so I did rename it. Now you decide on
> > different name.
> >
> > Can we either get timely reactions or less bikeshedding?
> 
> You mean less than 5 days because I don't see any other version of
> this? But in short, no, you can't.

Could we switch device tree bindings from "cc: subsystem, to: device
tree" to "to: subsystem, cc: device tree" mode? Currently it takes
more effort to merge the device tree parts than the relevant driver,
and that is not quite good.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]