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

[Frank Li <Frank.li@oss.nxp.com>]

On Tue, Sep 22, 2026 at 09:32:41AM -0300, Leonardo Costa wrote:
> 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,

Add G.N zhou and Cai Robby.

Frank

> > >
> > > 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@ziswiler.com/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