Re: [RFC] media: i2c: ov5640: Implement get_mbus_config

[Leonardo Costa]

Hi Sakari,

On Mon, Sep 21, 2026 at 11:02:13PM +0300, Sakari Ailus wrote:
> Hi Leonardo,
> 
> On Mon, Sep 21, 2026 at 03:04:59PM -0300, Leonardo Costa wrote:
> > Hi all,
> > 
> > Some time ago, we had sent a patch that implemented the .get_mbus_config
> > function for the OV5640 camera. This change was necessary for the camera to
> > work with the i.MX6 after the v5.18 release.
> > 
> >     https://lore.kernel.org/all/20230306063649.7387-1-marcel@xxxxxxxxxxxx/T/#u
> > 
> > The patch stirred some discussion, since .get_mbus_config wasn't supposed to be
> > implemented on drivers that don't have dynamic lane configuration. There were
> > proposals of implementing it in other points of the camera pipeline, but no
> > conclusion was reached.
> > 
> > We are planning to send the overlays for this camera for the Apalis iMX6, but
> > we verified that this patch is still needed for the camera to work on the
> > current mainline. Below are the commands to configure the pipeline, which
> > explicitly require a .get_mbus_config from the camera driver.
> > 
> >     root@apalis-imx6-11367581:~# media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
> >     root@apalis-imx6-11367581:~# media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
> >     root@apalis-imx6-11367581:~# media-ctl -l "'ipu1_csi1':2 -> 'ipu1_csi1 capture':0[1]"
> >     root@apalis-imx6-11367581:~# media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/1920x1080 field:none]"
> >     root@apalis-imx6-11367581:~# media-ctl -V "'imx6-mipi-csi2':2 [fmt:UYVY8_1X16/1920x1080 field:none]"
> >     [   47.438237] ipu1_csi1: entity ov5640 1-003c does not implement get_mbus_config()
> >     [   47.438265] ipu1_csi1: failed to get upstream media bus configuration
> >     root@apalis-imx6-11367581:~# media-ctl -V "'ipu1_csi1':2 [fmt:UYVY8_1X16/1920x1080 field:none]"
> >     Unable to setup formats: Inappropriate ioctl for device (25)
> >     [   62.616177] ipu1_csi1: entity ov5640 1-003c does not implement get_mbus_config()
> >     [   62.616204] ipu1_csi1: failed to get upstream media bus configuration
> > 
> > I am not very familiar with this subsystem, and it's been years since this
> > discussion took place, so I wanted to know what are your thoughts about this
> > patch and what the correct approach would be here. Was there any change that
> > would make this patch ok to be applied today? Do you think this still should
> > be included somewhere else on the pipeline?
> 
> My objection to the approach was about adding code that does very little or
> nothing to potentially a rather large number of drivers.
> 
> Since that we've gotten v4l2_get_active_data_lanes() that however seems to
> be used by the imx-mipi-csis driver only. Could using that solve the
> problem you have?

Hmm, looking at the implementation of v4l2_get_active_data_lanes it
seems to actually still use .get_mbus_config, and falls back to a
maximum value passed as an argument.

Furthermore, the error comes from imx-media-csi.c, and it doesn't seem
to be reading the number of lanes in the config, but rather the type of
the mbus (all the uses of the gotten mbus_cfg boil down to checking the
value of mbus_cfg.type). The receiver (imx6-mipi-csi2.c) actually seems
to get the number of lanes statically, and handle well the case where the
camera doesn't implement .get_mbus_config.

To test this, I hard-coded mbus_cfg->type = V4L2_MBUS_CSI2_DPHY inside
csi_get_upstream_mbus_config(), and the test above worked with that. So
as far as I understand it, imx-media-csi.c really only needs to know
what the type of the bus is.

The csi_get_upstream_mbus_config function already identifies whether
it's connected directly to the receiver or the mux, see the switch
statement below. If I understand correctly, in the case where it's
connected to the receiver directly, this is already known to be CSI-2,
so (I think) we can set the type value directly in this case.

For the mux I am not entirely sure. From the "Figure 19-1. CSI2IPU
gasket connectivity" figure in the IMX6DQRM TRM [1] (the same one Jacopo
referenced on the other thread), the mux's possible inputs seem to be
well defined to be either the receiver itself or the parallel interface.
Given this, maybe we could similarly infer the bus type from the
sd->grp_id gotten from the mux.

static int csi_get_upstream_mbus_config(struct csi_priv *priv,
					struct v4l2_mbus_config *mbus_cfg)
{
    ...
	switch (sd->grp_id) {
	case IMX_MEDIA_GRP_ID_CSI_MUX: // <------ Mux
		sd = imx_media_pipeline_subdev(&sd->entity,
					       IMX_MEDIA_GRP_ID_CSI2,
					       true);
        ...
		break;
	case IMX_MEDIA_GRP_ID_CSI2: // <------- Receiver
		break;
	default:
		...
		break;
	}
...
}

[1] https://www.nxp.com/webapp/Download?colCode=IMX6DQRM

What are your thoughts on this?

Kind regards,
Leonardo