Hi Steve,

On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:

On 01/24/2017 04:02 AM, Philipp Zabel wrote:

quoted

On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:

quoted

quoted

+
+int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config
*cfg)
+{
+	struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
+	struct media_pad *pad;
+	int ret;
+
+	if (vidsw->active == -1) {
+		dev_err(sd->dev, "no configuration for inactive mux\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Retrieve media bus configuration from the entity connected to the
+	 * active input
+	 */
+	pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
+	if (pad) {
+		sd = media_entity_to_v4l2_subdev(pad->entity);
+		ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
+		if (ret == -ENOIOCTLCMD)
+			pad = NULL;
+		else if (ret < 0) {
+			dev_err(sd->dev, "failed to get source 

configuration\n");

quoted

quoted

quoted

+			return ret;
+		}
+	}
+	if (!pad) {
+		/* Mirror the input side on the output side */
+		cfg->type = vidsw->endpoint[vidsw->active].bus_type;
+		if (cfg->type == V4L2_MBUS_PARALLEL ||
+		    cfg->type == V4L2_MBUS_BT656)
+			cfg->flags = vidsw->endpoint[vidsw-

active].bus.parallel.flags;

quoted

quoted

quoted

+	}
+
+	return 0;
+}

I am not certain this op is needed at all. In the current kernel this op
is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious).
Normally this information should come from the device tree and there
should be no need for this op.

My (tentative) long-term plan was to get rid of this op.

If you don't need it, then I recommend it is removed.

Hi Hans, the imx-media driver was only calling g_mbus_config to the camera
sensor, and it was doing that to determine the sensor's bus type. This info
was already available from parsing a v4l2_of_endpoint from the sensor node.
So it was simple to remove the g_mbus_config calls, and instead rely on the
parsed sensor v4l2_of_endpoint.

That's not a good point. The imx-media driver must not parse the sensor DT 
node as it is not aware of what bindings the sensor is compatible with. 
Information must instead be queried from the sensor subdev at runtime, through 
the g_mbus_config() operation.

Of course, if you can get the information from the imx-media DT node, that's 
certainly an option. It's only information provided by the sensor driver that 
you have no choice but query using a subdev operation.

quoted

We currently use this to make the CSI capture interface understand
whether its source from the MIPI CSI-2 or from the parallel bus. That is
probably something that should be fixed, but I'm not quite sure how.

The Synopsys DesignWare MIPI CSI-2 reciever turns the incoming MIPI
CSI-2 signal into a 32-bit parallel pixel bus plus some signals for the
MIPI specific metadata (virtual channel, data type).

Then the CSI2IPU gasket turns this input bus into four separate parallel
16-bit pixel buses plus an 8-bit "mct_di" bus for each of them, that
carries the MIPI metadata. The incoming data is split into the four
outputs according to the MIPI virtual channel.

Two of these 16-bit + 8-bit parallel buses are routed through a
multiplexer before finally arriving at the CSI on the other side.

We need to configure the CSI to either use or ignore the data from the
8-bit mct_di bus depending on whether the source of the mux is
configured to the MIPI CSI-2 receiver / CSI2IPU gasket, or to a parallel
input.

Philipp, from my experience, the CSI_MIPI_DI register (configured
by ipu_csi_set_mipi_datatype()) can only be given a virtual channel 0,
otherwise no data is received from the MIPI CSI-2 sensor, regardless
of the virtual channel the sensor is transmitting over.

So it seems the info on the 8-bit mct_di buses generated by the CSI2IPU
gasket are ignored by the CSI's, at least the virtual channel number is
ignored.

For example, if the sensor is transmitting on vc 1, the gasket routes
the sensor data to the parallel bus to CSI1. But if CSI_MIPI_DI on CSI1
is written with vc 1, no data is received.

Steve

quoted

Currently we let g_mbus_config pretend that even the internal 32-bit +
metadata and 16-bit + 8-bit metadata parallel buses are of type
V4L2_MBUS_CSI so that the CSI can ask the mux, which propagates to the
CSI-2 receiver, if connected.

Without g_mbus_config we'd need to get that information from somewhere
else. One possibility would be to extend MEDIA_BUS formats to describe
these "parallelized MIPI data" buses separately.

-- 
Regards,

Laurent Pinchart