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

[Laurent Pinchart]

On Tue, Sep 22, 2026 at 11:12:45AM -0500, Frank Li wrote:
> On Tue, Sep 22, 2026 at 09:32:41AM -0300, Leonardo Costa wrote:
> > On Mon, Sep 21, 2026 at 11:02:13PM +0300, Sakari Ailus wrote:
> > > On Mon, Sep 21, 2026 at 03:04:59PM -0300, Leonardo Costa wrote:
> > > > Hi all,
> 
> Add G.N zhou and Cai Robby.
> 
> > > > 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.

On a side node, the CSI-2 receiver driver should be decoupled from the
rest of the imx6-media-csi driver.

Replacing the two helpers it uses (imx_media_init_mbus_fmt() and
imx_media_init_state()) is the easy part.  The imx_media_init_mbus_fmt()
call should actually be dropped, as the csi2_dev.format_mbus field
should be dropped too and replaced by usage of the subdev active state
API.

The harder part is to drop usage of grp_id. It will require some
research and experimentation.

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

There's also a test of mbus_cfg->bus.parallel.bus_width in
is_parallel_16bit_bus().

> > 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;
> > 	}
> > ...
> > }

As I wrote above, we need to drop setting grp_id in the CSI-2 receiver
driver. Given that the mux inputs are well known, I would prefer
inferring the source type from the mux's routing configuration. The
upside is that it will remove two out of the three locations that test
for IMX_MEDIA_GRP_ID_CSI2, easing removal of IMX_MEDIA_GRP_ID_CSI2.

> >
> > [1] https://www.nxp.com/webapp/Download?colCode=IMX6DQRM
> >
> > What are your thoughts on this?

-- 
Regards,

Laurent Pinchart