| Message ID | 20230306063649.7387-1-marcel@ziswiler.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] media: i2c: ov5640: Implement get_mbus_config | expand |
Hi Marcel, On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > Implement the introduced get_mbus_config operation to report the > config of the MIPI CSI-2, BT.656 and Parallel interface. > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > --- > > Changes in v2: > - Take care of MIPI CSI-2, BT.656 and Parallel interface as > pointed out by Jacopo. Thanks! > > drivers/media/i2c/ov5640.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 1536649b9e90..43373416fcba 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -3774,6 +3774,24 @@ static int ov5640_init_cfg(struct v4l2_subdev *sd, > return 0; > } > > +static int ov5640_get_mbus_config(struct v4l2_subdev *sd, > + unsigned int pad, > + struct v4l2_mbus_config *cfg) > +{ > + struct ov5640_dev *sensor = to_ov5640_dev(sd); > + > + cfg->type = sensor->ep.bus_type; > + if (ov5640_is_csi2(sensor)) { > + cfg->bus.mipi_csi2.num_data_lanes = > + sensor->ep.bus.mipi_csi2.num_data_lanes; > + cfg->bus.mipi_csi2.flags = sensor->ep.bus.mipi_csi2.flags; > + } else { > + cfg->bus.parallel.flags = sensor->ep.bus.parallel.flags; > + } > + > + return 0; > +} > + > static const struct v4l2_subdev_core_ops ov5640_core_ops = { > .log_status = v4l2_ctrl_subdev_log_status, > .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > @@ -3794,6 +3812,7 @@ static const struct v4l2_subdev_pad_ops ov5640_pad_ops = { > .get_selection = ov5640_get_selection, > .enum_frame_size = ov5640_enum_frame_size, > .enum_frame_interval = ov5640_enum_frame_interval, > + .get_mbus_config = ov5640_get_mbus_config, What's the reasoning for this patch? Drivers that don't have e.g. dynamic lane configuration shouldn't need to implement get_mbus_config.
Hello Sakari, On Tue, Mar 14, 2023 at 02:13:46PM +0200, Sakari Ailus wrote: > On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > Implement the introduced get_mbus_config operation to report the > > config of the MIPI CSI-2, BT.656 and Parallel interface. > > > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > --- > > > > Changes in v2: > > - Take care of MIPI CSI-2, BT.656 and Parallel interface as > > pointed out by Jacopo. Thanks! > > > > drivers/media/i2c/ov5640.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > > index 1536649b9e90..43373416fcba 100644 > > --- a/drivers/media/i2c/ov5640.c > > +++ b/drivers/media/i2c/ov5640.c > > @@ -3774,6 +3774,24 @@ static int ov5640_init_cfg(struct v4l2_subdev *sd, > > return 0; > > } > > > > +static int ov5640_get_mbus_config(struct v4l2_subdev *sd, > > + unsigned int pad, > > + struct v4l2_mbus_config *cfg) > > +{ > > + struct ov5640_dev *sensor = to_ov5640_dev(sd); > > + > > + cfg->type = sensor->ep.bus_type; > > + if (ov5640_is_csi2(sensor)) { > > + cfg->bus.mipi_csi2.num_data_lanes = > > + sensor->ep.bus.mipi_csi2.num_data_lanes; > > + cfg->bus.mipi_csi2.flags = sensor->ep.bus.mipi_csi2.flags; > > + } else { > > + cfg->bus.parallel.flags = sensor->ep.bus.parallel.flags; > > + } > > + > > + return 0; > > +} > > + > > static const struct v4l2_subdev_core_ops ov5640_core_ops = { > > .log_status = v4l2_ctrl_subdev_log_status, > > .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > > @@ -3794,6 +3812,7 @@ static const struct v4l2_subdev_pad_ops ov5640_pad_ops = { > > .get_selection = ov5640_get_selection, > > .enum_frame_size = ov5640_enum_frame_size, > > .enum_frame_interval = ov5640_enum_frame_interval, > > + .get_mbus_config = ov5640_get_mbus_config, > > What's the reasoning for this patch? Without this it's not possible to use it on i.MX6, drivers/staging/media/imx/imx6-mipi-csi2.c requires it, some more details from Jacopo here [0]. Everything used to work fine up to v5.18, after that kernel version various changes broke it [1][2] (I assume you are pretty much aware of the history here, you commented on a few emails). [0] https://lore.kernel.org/all/20230128100611.7ulsfqqqgscg54gy@uno.localdomain/ [1] https://lore.kernel.org/all/081cc2d3-1f3a-6c14-6dc7-53f976be7b2b@gmail.com/ [2] https://lore.kernel.org/all/cacfe146-101b-35b3-5f66-1a1cabfd342f@gmail.com/ > Drivers that don't have e.g. dynamic lane configuration shouldn't need to > implement get_mbus_config. Francesco
+Marco On Tue, Mar 14, 2023 at 02:45:53PM +0200, Sakari Ailus wrote: > On Tue, Mar 14, 2023 at 01:32:16PM +0100, Francesco Dolcini wrote: > > On Tue, Mar 14, 2023 at 02:13:46PM +0200, Sakari Ailus wrote: > > > On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > > > > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > > > Implement the introduced get_mbus_config operation to report the > > > > config of the MIPI CSI-2, BT.656 and Parallel interface. > > > > > > > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > > > What's the reasoning for this patch? > > > > Without this it's not possible to use it on i.MX6, > > drivers/staging/media/imx/imx6-mipi-csi2.c requires it, some more > > details from Jacopo here [0]. > > > > Everything used to work fine up to v5.18, after that kernel version > > various changes broke it [1][2] (I assume you are pretty much aware of > > the history here, you commented on a few emails). > > > > [0] https://lore.kernel.org/all/20230128100611.7ulsfqqqgscg54gy@uno.localdomain/ > > [1] https://lore.kernel.org/all/081cc2d3-1f3a-6c14-6dc7-53f976be7b2b@gmail.com/ > > [2] https://lore.kernel.org/all/cacfe146-101b-35b3-5f66-1a1cabfd342f@gmail.com/ > > > > > Drivers that don't have e.g. dynamic lane configuration shouldn't need to > > > implement get_mbus_config. > > Not even for staging drivers. The driver should be fixed to get that > information from the endpoint instead. This seems exactly the opposite of what commit 7318abface48 ("media: imx: Use get_mbus_config instead of parsing upstream DT endpoints") did. Given that I am somehow confused, but I am not that familiar with this subsystem, so I guess this is expected :-). Can someone provide some additional hint here? > I don't object having a helper in the framework to do this though. There > are many receiver drivers that need this to work with those devices that > have variable number of lanes. Thanks in advance, Francesco
Hello On Tue, Mar 14, 2023 at 01:59:06PM +0100, Francesco Dolcini wrote: > +Marco > > On Tue, Mar 14, 2023 at 02:45:53PM +0200, Sakari Ailus wrote: > > On Tue, Mar 14, 2023 at 01:32:16PM +0100, Francesco Dolcini wrote: > > > On Tue, Mar 14, 2023 at 02:13:46PM +0200, Sakari Ailus wrote: > > > > On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > > > > > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > > > > > Implement the introduced get_mbus_config operation to report the > > > > > config of the MIPI CSI-2, BT.656 and Parallel interface. > > > > > > > > > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > > > > > What's the reasoning for this patch? > > > > > > Without this it's not possible to use it on i.MX6, > > > drivers/staging/media/imx/imx6-mipi-csi2.c requires it, some more > > > details from Jacopo here [0]. > > > > > > Everything used to work fine up to v5.18, after that kernel version > > > various changes broke it [1][2] (I assume you are pretty much aware of > > > the history here, you commented on a few emails). > > > > > > [0] https://lore.kernel.org/all/20230128100611.7ulsfqqqgscg54gy@uno.localdomain/ > > > [1] https://lore.kernel.org/all/081cc2d3-1f3a-6c14-6dc7-53f976be7b2b@gmail.com/ > > > [2] https://lore.kernel.org/all/cacfe146-101b-35b3-5f66-1a1cabfd342f@gmail.com/ > > > > > > > Drivers that don't have e.g. dynamic lane configuration shouldn't need to > > > > implement get_mbus_config. > > > > Not even for staging drivers. The driver should be fixed to get that > > information from the endpoint instead. > > This seems exactly the opposite of what commit > 7318abface48 ("media: imx: Use get_mbus_config instead of parsing upstream DT endpoints") > did. > > Given that I am somehow confused, but I am not that familiar with this > subsystem, so I guess this is expected :-). Can someone provide some > additional hint here? > As per my understanding, the i.MX6 IPU CSI driver connects to the CSI-2 receiver and/or two video muxes. One figure's worth a thousands words: "Figure 19-1. CSI2IPU gasket connectivity" of the IMX6DQRM TRM. So the local endpoint might not provide the required information on the bus configuration as it connects to a video-mux. That's why the imx_media_pipeline_subdev() helper is used in csi_get_upstream_mbus_config(). My gut feeling is that it would be better to always call get_mbus_config() on the next subdev (the mux or the CSI-2 rx) and there parse the local endpoint as it's the mux or the CSI-2 rx that connect to the actual source. To be honest my understanding is that this patch has always been needed to work on imx6 and this is not a regression but something that was kept as an out-of-tree patch downstream. Is this correct or is this a regression ? The above mentioned patches that move fwnode matching to endpoints don't seem related, don't they ? Thanks j > > I don't object having a helper in the framework to do this though. There > > are many receiver drivers that need this to work with those devices that > > have variable number of lanes. > > Thanks in advance, > Francesco >
Hi Jacopo, On Mon, Mar 20, 2023 at 09:48:44AM +0100, Jacopo Mondi wrote: > Hello > > On Tue, Mar 14, 2023 at 01:59:06PM +0100, Francesco Dolcini wrote: > > +Marco > > > > On Tue, Mar 14, 2023 at 02:45:53PM +0200, Sakari Ailus wrote: > > > On Tue, Mar 14, 2023 at 01:32:16PM +0100, Francesco Dolcini wrote: > > > > On Tue, Mar 14, 2023 at 02:13:46PM +0200, Sakari Ailus wrote: > > > > > On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > > > > > > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > > > > > > > Implement the introduced get_mbus_config operation to report the > > > > > > config of the MIPI CSI-2, BT.656 and Parallel interface. > > > > > > > > > > > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > > > > > > > What's the reasoning for this patch? > > > > > > > > Without this it's not possible to use it on i.MX6, > > > > drivers/staging/media/imx/imx6-mipi-csi2.c requires it, some more > > > > details from Jacopo here [0]. > > > > > > > > Everything used to work fine up to v5.18, after that kernel version > > > > various changes broke it [1][2] (I assume you are pretty much aware of > > > > the history here, you commented on a few emails). > > > > > > > > [0] https://lore.kernel.org/all/20230128100611.7ulsfqqqgscg54gy@uno.localdomain/ > > > > [1] https://lore.kernel.org/all/081cc2d3-1f3a-6c14-6dc7-53f976be7b2b@gmail.com/ > > > > [2] https://lore.kernel.org/all/cacfe146-101b-35b3-5f66-1a1cabfd342f@gmail.com/ > > > > > > > > > Drivers that don't have e.g. dynamic lane configuration shouldn't need to > > > > > implement get_mbus_config. > > > > > > Not even for staging drivers. The driver should be fixed to get that > > > information from the endpoint instead. > > > > This seems exactly the opposite of what commit > > 7318abface48 ("media: imx: Use get_mbus_config instead of parsing upstream DT endpoints") > > did. > > > > Given that I am somehow confused, but I am not that familiar with this > > subsystem, so I guess this is expected :-). Can someone provide some > > additional hint here? > > > > As per my understanding, the i.MX6 IPU CSI driver connects to the > CSI-2 receiver and/or two video muxes. One figure's worth a thousands > words: "Figure 19-1. CSI2IPU gasket connectivity" of the IMX6DQRM TRM. I don't have that document. > > So the local endpoint might not provide the required information on > the bus configuration as it connects to a video-mux. > > That's why the imx_media_pipeline_subdev() helper is used in > csi_get_upstream_mbus_config(). > > My gut feeling is that it would be better to always call > get_mbus_config() on the next subdev (the mux or the CSI-2 rx) and > there parse the local endpoint as it's the mux or the CSI-2 rx that > connect to the actual source. Isn't this still a different endpoint in DT? I understand you have a single pad with two links?
On Mon, Mar 20, 2023 at 11:00:36AM +0200, Sakari Ailus wrote: > On Mon, Mar 20, 2023 at 09:48:44AM +0100, Jacopo Mondi wrote: > > On Tue, Mar 14, 2023 at 01:59:06PM +0100, Francesco Dolcini wrote: > > > On Tue, Mar 14, 2023 at 02:45:53PM +0200, Sakari Ailus wrote: > > > > On Tue, Mar 14, 2023 at 01:32:16PM +0100, Francesco Dolcini wrote: > > > > > On Tue, Mar 14, 2023 at 02:13:46PM +0200, Sakari Ailus wrote: > > > > > > On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > > > > > > > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > > > > > > > > > Implement the introduced get_mbus_config operation to report the > > > > > > > config of the MIPI CSI-2, BT.656 and Parallel interface. > > > > > > > > > > > > > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > > > > > > > > > What's the reasoning for this patch? > > > > > > > > > > Without this it's not possible to use it on i.MX6, > > > > > drivers/staging/media/imx/imx6-mipi-csi2.c requires it, some more > > > > > details from Jacopo here [0]. > > > > > > > > > > Everything used to work fine up to v5.18, after that kernel version > > > > > various changes broke it [1][2] (I assume you are pretty much aware of > > > > > the history here, you commented on a few emails). > > > > > > > > > > [0] https://lore.kernel.org/all/20230128100611.7ulsfqqqgscg54gy@uno.localdomain/ > > > > > [1] https://lore.kernel.org/all/081cc2d3-1f3a-6c14-6dc7-53f976be7b2b@gmail.com/ > > > > > [2] https://lore.kernel.org/all/cacfe146-101b-35b3-5f66-1a1cabfd342f@gmail.com/ > > > > > > > > > > > Drivers that don't have e.g. dynamic lane configuration shouldn't need to > > > > > > implement get_mbus_config. > > > > > > > > Not even for staging drivers. The driver should be fixed to get that > > > > information from the endpoint instead. > > > > > > This seems exactly the opposite of what commit > > > 7318abface48 ("media: imx: Use get_mbus_config instead of parsing upstream DT endpoints") > > > did. > > > > > > Given that I am somehow confused, but I am not that familiar with this > > > subsystem, so I guess this is expected :-). Can someone provide some > > > additional hint here? > > > > > > > As per my understanding, the i.MX6 IPU CSI driver connects to the > > CSI-2 receiver and/or two video muxes. One figure's worth a thousands > > words: "Figure 19-1. CSI2IPU gasket connectivity" of the IMX6DQRM TRM. > > I don't have that document. https://www.nxp.com/webapp/Download?colCode=IMX6DQRM You'll need a user account on nxp.com though. > > So the local endpoint might not provide the required information on > > the bus configuration as it connects to a video-mux. > > > > That's why the imx_media_pipeline_subdev() helper is used in > > csi_get_upstream_mbus_config(). > > > > My gut feeling is that it would be better to always call > > get_mbus_config() on the next subdev (the mux or the CSI-2 rx) and > > there parse the local endpoint as it's the mux or the CSI-2 rx that > > connect to the actual source. > > Isn't this still a different endpoint in DT? I understand you have a single > pad with two links? In a (simplified) nutshell, ---------+ +----------+ +---------+ +-----+ +-----+ | Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | | Sensor | | | | Gasket | | | | | ---------+ +----------+ +---------+ +-----+ +-----+ All those blocks, except for the gasket, have a node in DT. The IPU driver needs to know the number of CSI-2 data lanes, which is encoded in the data-lanes DT property present in both the sensor output endpoint and the CSI-2 RX input endpoint, but not the other endpoints in the pipeline.
Hi Laurent, On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > In a (simplified) nutshell, > > ---------+ +----------+ +---------+ +-----+ +-----+ > | Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > | Sensor | | | | Gasket | | | | | > ---------+ +----------+ +---------+ +-----+ +-----+ Thank you, this is helpful. I suppose the mux here at least won't actively do anything to the data. So presumably its endpoint won't contain the active configuration, but its superset. > > All those blocks, except for the gasket, have a node in DT. > > The IPU driver needs to know the number of CSI-2 data lanes, which is > encoded in the data-lanes DT property present in both the sensor output > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > the pipeline. This doesn't yet explain why the sensor would need to implement get_mbus_config if its bus configuration remains constant. I suppose those blocks in between would probably need something to convey their active configuration from upstream sub-devices.
On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > Hi Laurent, > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > In a (simplified) nutshell, > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > | Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > | Sensor | | | | Gasket | | | | | > > ---------+ +----------+ +---------+ +-----+ +-----+ > > Thank you, this is helpful. > > I suppose the mux here at least won't actively do anything to the data. So > presumably its endpoint won't contain the active configuration, but its > superset. > > > > > All those blocks, except for the gasket, have a node in DT. > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > encoded in the data-lanes DT property present in both the sensor output > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > the pipeline. > > This doesn't yet explain why the sensor would need to implement > get_mbus_config if its bus configuration remains constant. If I recall correctly, the IPU driver calls .g_mbus_config() on the camera sensor to get the number of lanes, as it can't get it from its own endpoint. That's a hack, and as Jacopo proposed, calling .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver can then get the value from its own endpoint, without requiring all sensor drivers to implement .g_mbus_config(). > I suppose those blocks in between would probably need something to convey > their active configuration from upstream sub-devices.
Hi Laurent, On Mon, Mar 20, 2023 at 11:55:14AM +0200, Laurent Pinchart wrote: > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > Hi Laurent, > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > In a (simplified) nutshell, > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > | Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > | Sensor | | | | Gasket | | | | | > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > Thank you, this is helpful. > > > > I suppose the mux here at least won't actively do anything to the data. So > > presumably its endpoint won't contain the active configuration, but its > > superset. > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > encoded in the data-lanes DT property present in both the sensor output > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > the pipeline. > > > > This doesn't yet explain why the sensor would need to implement > > get_mbus_config if its bus configuration remains constant. > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > camera sensor to get the number of lanes, as it can't get it from its > own endpoint. That's a hack, and as Jacopo proposed, calling > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > can then get the value from its own endpoint, without requiring all > sensor drivers to implement .g_mbus_config(). The g_mbus_config op could be implemented by the CSI2IPU and mux, by simply requesting the information from the upstream sub-device. No hacks would be needed.
On Mon, Mar 20, 2023 at 09:48:44AM +0100, Jacopo Mondi wrote: > On Tue, Mar 14, 2023 at 01:59:06PM +0100, Francesco Dolcini wrote: > > On Tue, Mar 14, 2023 at 02:45:53PM +0200, Sakari Ailus wrote: > > > On Tue, Mar 14, 2023 at 01:32:16PM +0100, Francesco Dolcini wrote: > > > > On Tue, Mar 14, 2023 at 02:13:46PM +0200, Sakari Ailus wrote: > > > > > On Mon, Mar 06, 2023 at 07:36:49AM +0100, Marcel Ziswiler wrote: > > > > > > From: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > > > > > > > Implement the introduced get_mbus_config operation to report the > > > > > > config of the MIPI CSI-2, BT.656 and Parallel interface. > > > > > > > > > > > > Signed-off-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> > > > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > > > > > > > What's the reasoning for this patch? > > > > > > > > Without this it's not possible to use it on i.MX6, > > > > drivers/staging/media/imx/imx6-mipi-csi2.c requires it, some more > > > > details from Jacopo here [0]. > > > > > > > > Everything used to work fine up to v5.18, after that kernel version > > > > various changes broke it [1][2] (I assume you are pretty much aware of > > > > the history here, you commented on a few emails). > > > > > > > > [0] https://lore.kernel.org/all/20230128100611.7ulsfqqqgscg54gy@uno.localdomain/ > > > > [1] https://lore.kernel.org/all/081cc2d3-1f3a-6c14-6dc7-53f976be7b2b@gmail.com/ > > > > [2] https://lore.kernel.org/all/cacfe146-101b-35b3-5f66-1a1cabfd342f@gmail.com/ > > > > > > > > > Drivers that don't have e.g. dynamic lane configuration shouldn't need to > > > > > implement get_mbus_config. > > > > > > Not even for staging drivers. The driver should be fixed to get that > > > information from the endpoint instead. > > > > This seems exactly the opposite of what commit > > 7318abface48 ("media: imx: Use get_mbus_config instead of parsing upstream DT endpoints") > > did. > > > > Given that I am somehow confused, but I am not that familiar with this > > subsystem, so I guess this is expected :-). Can someone provide some > > additional hint here? > > > To be honest my understanding is that this patch has always been > needed to work on imx6 and this is not a regression but something that > was kept as an out-of-tree patch downstream. Is this correct or is > this a regression ? I confirm that v5.18 was/is fine. Aishwarya: correct? In the end you tested it, not me :-) Francesco
On Mo, 2023-03-20 at 11:55 +0200, Laurent Pinchart wrote: > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > Hi Laurent, > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > In a (simplified) nutshell, > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > Sensor | | | | Gasket | | | | | > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > Thank you, this is helpful. > > > > I suppose the mux here at least won't actively do anything to the data. So > > presumably its endpoint won't contain the active configuration, but its > > superset. > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > encoded in the data-lanes DT property present in both the sensor output > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > the pipeline. > > > > This doesn't yet explain why the sensor would need to implement > > get_mbus_config if its bus configuration remains constant. > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > camera sensor to get the number of lanes, as it can't get it from its > own endpoint. That's a hack, and as Jacopo proposed, calling > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > can then get the value from its own endpoint, without requiring all > sensor drivers to implement .g_mbus_config(). The IPU driver doesn't call get_mbus_config, the CSI-2 RX driver does (csi2_get_active_lanes in imx6-mipi-csi2.c). It could just fall back to looking at its own endpoint if the upstream driver does not implement get_mbus_config. Of course that will only help if the DT contains this information and all connected lanes are active. regards Philipp
Hi Philipp On Mon, Mar 20, 2023 at 12:26:26PM +0100, Philipp Zabel wrote: > On Mo, 2023-03-20 at 11:55 +0200, Laurent Pinchart wrote: > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > Hi Laurent, > > > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > In a (simplified) nutshell, > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > Sensor | | | | Gasket | | | | | > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > Thank you, this is helpful. > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > presumably its endpoint won't contain the active configuration, but its > > > superset. > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > encoded in the data-lanes DT property present in both the sensor output > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > the pipeline. > > > > > > This doesn't yet explain why the sensor would need to implement > > > get_mbus_config if its bus configuration remains constant. > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > camera sensor to get the number of lanes, as it can't get it from its > > own endpoint. That's a hack, and as Jacopo proposed, calling > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > can then get the value from its own endpoint, without requiring all > > sensor drivers to implement .g_mbus_config(). > > The IPU driver doesn't call get_mbus_config, the CSI-2 RX driver does Am I confusing IPU CSI with CSI-2 ? https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/imx/imx-media-csi.c#L211 > (csi2_get_active_lanes in imx6-mipi-csi2.c). It could just fall back to > looking at its own endpoint if the upstream driver does not implement > get_mbus_config. > > Of course that will only help if the DT contains this information and > all connected lanes are active. We should assume DTs are correct, otherwise we're screwed most of the times... > > regards > Philipp
On Mo, 2023-03-20 at 13:47 +0100, Jacopo Mondi wrote: > Hi Philipp > > On Mon, Mar 20, 2023 at 12:26:26PM +0100, Philipp Zabel wrote: > > On Mo, 2023-03-20 at 11:55 +0200, Laurent Pinchart wrote: > > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > > Hi Laurent, > > > > > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > > In a (simplified) nutshell, > > > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > > Sensor | | | | Gasket | | | | | > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > > Thank you, this is helpful. > > > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > > presumably its endpoint won't contain the active configuration, but its > > > > superset. > > > > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > > encoded in the data-lanes DT property present in both the sensor output > > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > > the pipeline. > > > > > > > > This doesn't yet explain why the sensor would need to implement > > > > get_mbus_config if its bus configuration remains constant. > > > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > > camera sensor to get the number of lanes, as it can't get it from its > > > own endpoint. That's a hack, and as Jacopo proposed, calling > > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > > can then get the value from its own endpoint, without requiring all > > > sensor drivers to implement .g_mbus_config(). > > > > The IPU driver doesn't call get_mbus_config, the CSI-2 RX driver does > > Am I confusing IPU CSI with CSI-2 ? > https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/imx/imx-media-csi.c#L211 What the eyesight ... Sorry for the confusion. You are right, it's right there. Yes, that is IPU CSI calling get_mbus_config. I had git grepped for get_mbus_config this morning, and was convinced I only found the call in imx-media-csi2, which is where the number of lanes is evaluated. The call in imx-media-csi.c is used to determine whether the sensor is parallel or CSI-2 and whether the CSI has to operate in bypass mode (for 16-bit parallel bus). regards Philipp
On Mo, 2023-03-20 at 12:15 +0200, Sakari Ailus wrote: > Hi Laurent, > > On Mon, Mar 20, 2023 at 11:55:14AM +0200, Laurent Pinchart wrote: > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > Hi Laurent, > > > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > In a (simplified) nutshell, > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > Sensor | | | | Gasket | | | | | > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > Thank you, this is helpful. > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > presumably its endpoint won't contain the active configuration, but its > > > superset. > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > encoded in the data-lanes DT property present in both the sensor output > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > the pipeline. > > > > > > This doesn't yet explain why the sensor would need to implement > > > get_mbus_config if its bus configuration remains constant. > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > camera sensor to get the number of lanes, as it can't get it from its > > own endpoint. That's a hack, and as Jacopo proposed, calling > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > can then get the value from its own endpoint, without requiring all > > sensor drivers to implement .g_mbus_config(). > > The g_mbus_config op could be implemented by the CSI2IPU and mux, by simply > requesting the information from the upstream sub-device. No hacks would be > needed. I think implementing get_mbus_config on the mux might be enough. The IPU driver already recognizes the CSI-2 RX by its grp_id and could infer that it has to use MIPI CSI-2 mode from that. The video-mux would have to forward get_mbus_config to its active upstream port and if the upstream sensor does not implement it read bus width from the active upstream endpoint. regards Philipp
On Mon, Mar 20, 2023 at 02:32:25PM +0100, Philipp Zabel wrote: > On Mo, 2023-03-20 at 12:15 +0200, Sakari Ailus wrote: > > On Mon, Mar 20, 2023 at 11:55:14AM +0200, Laurent Pinchart wrote: > > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > > In a (simplified) nutshell, > > > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > > Sensor | | | | Gasket | | | | | > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > > Thank you, this is helpful. > > > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > > presumably its endpoint won't contain the active configuration, but its > > > > superset. > > > > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > > encoded in the data-lanes DT property present in both the sensor output > > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > > the pipeline. > > > > > > > > This doesn't yet explain why the sensor would need to implement > > > > get_mbus_config if its bus configuration remains constant. > > > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > > camera sensor to get the number of lanes, as it can't get it from its > > > own endpoint. That's a hack, and as Jacopo proposed, calling > > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > > can then get the value from its own endpoint, without requiring all > > > sensor drivers to implement .g_mbus_config(). > > > > The g_mbus_config op could be implemented by the CSI2IPU and mux, by simply > > requesting the information from the upstream sub-device. No hacks would be > > needed. > > I think implementing get_mbus_config on the mux might be enough. The > IPU driver already recognizes the CSI-2 RX by its grp_id and could > infer that it has to use MIPI CSI-2 mode from that. > > The video-mux would have to forward get_mbus_config to its active > upstream port and if the upstream sensor does not implement it read bus > width from the active upstream endpoint. I'm fine with implementing it in the mux as well, but I think we can take a shortcut here and call it on the CSI-2 RX from the IPU driver, as the IPU driver knows about the architecture of the whole pipeline.
On Mon, 20 Mar 2023 at 14:00, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Mon, Mar 20, 2023 at 02:32:25PM +0100, Philipp Zabel wrote: > > On Mo, 2023-03-20 at 12:15 +0200, Sakari Ailus wrote: > > > On Mon, Mar 20, 2023 at 11:55:14AM +0200, Laurent Pinchart wrote: > > > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > > > In a (simplified) nutshell, > > > > > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > > > Sensor | | | | Gasket | | | | | > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > > > > Thank you, this is helpful. > > > > > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > > > presumably its endpoint won't contain the active configuration, but its > > > > > superset. > > > > > > > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > > > encoded in the data-lanes DT property present in both the sensor output > > > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > > > the pipeline. > > > > > > > > > > This doesn't yet explain why the sensor would need to implement > > > > > get_mbus_config if its bus configuration remains constant. > > > > > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > > > camera sensor to get the number of lanes, as it can't get it from its > > > > own endpoint. That's a hack, and as Jacopo proposed, calling > > > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > > > can then get the value from its own endpoint, without requiring all > > > > sensor drivers to implement .g_mbus_config(). > > > > > > The g_mbus_config op could be implemented by the CSI2IPU and mux, by simply > > > requesting the information from the upstream sub-device. No hacks would be > > > needed. > > > > I think implementing get_mbus_config on the mux might be enough. The > > IPU driver already recognizes the CSI-2 RX by its grp_id and could > > infer that it has to use MIPI CSI-2 mode from that. > > > > The video-mux would have to forward get_mbus_config to its active > > upstream port and if the upstream sensor does not implement it read bus > > width from the active upstream endpoint. > > I'm fine with implementing it in the mux as well, but I think we can > take a shortcut here and call it on the CSI-2 RX from the IPU driver, as > the IPU driver knows about the architecture of the whole pipeline. FWIW I have made use of video-mux and implementing g_mbus_config on it for D-PHY switch chips[1] where the different input ports may have different configurations. I'll admit that I've made the easy assumption that it's CSI-2 D-PHY in and out, when it could quite legitimately be working with any of the other bus types. I had been intending to send this[2] upstream when I get a chance, but am I reading imx6q.dtsi[3] correctly in that the mux is accepting parallel on some ports and CSI-2 on others? The mux hardware is therefore far more than just a simple switch between inputs? Although as this is after the CSI2 rx block, this is effectively parallel data within the SoC, therefore is the configuration and get_mbus_config really relevant? I'd like to understand how this is being used on imx6 before trying to rework my patch into a generic solution. Thanks Dave [1] eg Arducam's 2 and 4 port muxes - https://www.arducam.com/product-category/camera-multiplexers/, which IIRC use OnSemi's FSA644 https://www.onsemi.com/products/interfaces/analog-switches/fsa644 [2] https://github.com/raspberrypi/linux/commit/bf653318475cf4db0ec59e92139f477f7cc0a544 [3] https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6q.dtsi#L349
Hi Laurent, On Mon, Mar 20, 2023 at 04:00:12PM +0200, Laurent Pinchart wrote: > On Mon, Mar 20, 2023 at 02:32:25PM +0100, Philipp Zabel wrote: > > On Mo, 2023-03-20 at 12:15 +0200, Sakari Ailus wrote: > > > On Mon, Mar 20, 2023 at 11:55:14AM +0200, Laurent Pinchart wrote: > > > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > > > In a (simplified) nutshell, > > > > > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > > > Sensor | | | | Gasket | | | | | > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > > > > Thank you, this is helpful. > > > > > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > > > presumably its endpoint won't contain the active configuration, but its > > > > > superset. > > > > > > > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > > > encoded in the data-lanes DT property present in both the sensor output > > > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > > > the pipeline. > > > > > > > > > > This doesn't yet explain why the sensor would need to implement > > > > > get_mbus_config if its bus configuration remains constant. > > > > > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > > > camera sensor to get the number of lanes, as it can't get it from its > > > > own endpoint. That's a hack, and as Jacopo proposed, calling > > > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > > > can then get the value from its own endpoint, without requiring all > > > > sensor drivers to implement .g_mbus_config(). > > > > > > The g_mbus_config op could be implemented by the CSI2IPU and mux, by simply > > > requesting the information from the upstream sub-device. No hacks would be > > > needed. > > > > I think implementing get_mbus_config on the mux might be enough. The > > IPU driver already recognizes the CSI-2 RX by its grp_id and could > > infer that it has to use MIPI CSI-2 mode from that. > > > > The video-mux would have to forward get_mbus_config to its active > > upstream port and if the upstream sensor does not implement it read bus > > width from the active upstream endpoint. > > I'm fine with implementing it in the mux as well, but I think we can > take a shortcut here and call it on the CSI-2 RX from the IPU driver, as > the IPU driver knows about the architecture of the whole pipeline. If that's the case then I guess that's fine. But can these drivers be used elsewhere than with IMX6? It'd be safest to implement g_mbus_config for the all the way to CSI-2 RX. But if the answer to the question is "no", then making that shortcut should be fine (and this can be always reworked if need be).
Hi Dave, On Mon, Mar 20, 2023 at 06:31:04PM +0000, Dave Stevenson wrote: > On Mon, 20 Mar 2023 at 14:00, Laurent Pinchart wrote: > > On Mon, Mar 20, 2023 at 02:32:25PM +0100, Philipp Zabel wrote: > > > On Mo, 2023-03-20 at 12:15 +0200, Sakari Ailus wrote: > > > > On Mon, Mar 20, 2023 at 11:55:14AM +0200, Laurent Pinchart wrote: > > > > > On Mon, Mar 20, 2023 at 11:37:33AM +0200, Sakari Ailus wrote: > > > > > > On Mon, Mar 20, 2023 at 11:26:02AM +0200, Laurent Pinchart wrote: > > > > > > > In a (simplified) nutshell, > > > > > > > > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > > Camera | --> | CSI-2 RX | --> | CSI2IPU | --> | Mux | --> | IPU | > > > > > > > > Sensor | | | | Gasket | | | | | > > > > > > > ---------+ +----------+ +---------+ +-----+ +-----+ > > > > > > > > > > > > Thank you, this is helpful. > > > > > > > > > > > > I suppose the mux here at least won't actively do anything to the data. So > > > > > > presumably its endpoint won't contain the active configuration, but its > > > > > > superset. > > > > > > > > > > > > > > > > > > > > All those blocks, except for the gasket, have a node in DT. > > > > > > > > > > > > > > The IPU driver needs to know the number of CSI-2 data lanes, which is > > > > > > > encoded in the data-lanes DT property present in both the sensor output > > > > > > > endpoint and the CSI-2 RX input endpoint, but not the other endpoints in > > > > > > > the pipeline. > > > > > > > > > > > > This doesn't yet explain why the sensor would need to implement > > > > > > get_mbus_config if its bus configuration remains constant. > > > > > > > > > > If I recall correctly, the IPU driver calls .g_mbus_config() on the > > > > > camera sensor to get the number of lanes, as it can't get it from its > > > > > own endpoint. That's a hack, and as Jacopo proposed, calling > > > > > .g_mbus_config() on the CSI-2 RX would be better, as the CSI-2 RX driver > > > > > can then get the value from its own endpoint, without requiring all > > > > > sensor drivers to implement .g_mbus_config(). > > > > > > > > The g_mbus_config op could be implemented by the CSI2IPU and mux, by simply > > > > requesting the information from the upstream sub-device. No hacks would be > > > > needed. > > > > > > I think implementing get_mbus_config on the mux might be enough. The > > > IPU driver already recognizes the CSI-2 RX by its grp_id and could > > > infer that it has to use MIPI CSI-2 mode from that. > > > > > > The video-mux would have to forward get_mbus_config to its active > > > upstream port and if the upstream sensor does not implement it read bus > > > width from the active upstream endpoint. > > > > I'm fine with implementing it in the mux as well, but I think we can > > take a shortcut here and call it on the CSI-2 RX from the IPU driver, as > > the IPU driver knows about the architecture of the whole pipeline. > > FWIW I have made use of video-mux and implementing g_mbus_config on it > for D-PHY switch chips[1] where the different input ports may have > different configurations. I'll admit that I've made the easy > assumption that it's CSI-2 D-PHY in and out, when it could quite > legitimately be working with any of the other bus types. That's a use case I hadn't considered. .get_mbus_config() makes sense. > I had been intending to send this[2] upstream when I get a chance, but > am I reading imx6q.dtsi[3] correctly in that the mux is accepting > parallel on some ports and CSI-2 on others? The mux hardware is > therefore far more than just a simple switch between inputs? Although > as this is after the CSI2 rx block, this is effectively parallel data > within the SoC, therefore is the configuration and get_mbus_config > really relevant? The exact hardware architecture isn't clear, but I indeed expect the mux to receive parallel data from the CSI-2 RX and parallel inputs. I had a closer look at the IPU driver code, and realized I was wrong when I mentioned it needed to know the number of lanes. What the IPU need is the bus type (CSI-2, BT656 or parallel) and, for parallel buses, the bus width. It may be possible to refactor the IPU driver code to replace that information with the media bus code in some places, but not everywhere. Whether the input comes from the CSI-2 RX could be deduced internally from the selected mux input, but we can't differentiate BT656 from parallel without querying the mux's input bus type. Calling .get_mbus_config() on the mux is thus required, and the mux driver should implement the operation. > I'd like to understand how this is being used on imx6 before trying to > rework my patch into a generic solution. > > Thanks > Dave > > [1] eg Arducam's 2 and 4 port muxes - https://www.arducam.com/product-category/camera-multiplexers/, which > IIRC use OnSemi's FSA644 https://www.onsemi.com/products/interfaces/analog-switches/fsa644 > [2] https://github.com/raspberrypi/linux/commit/bf653318475cf4db0ec59e92139f477f7cc0a544 > [3] https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6q.dtsi#L349
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 1536649b9e90..43373416fcba 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -3774,6 +3774,24 @@ static int ov5640_init_cfg(struct v4l2_subdev *sd, return 0; } +static int ov5640_get_mbus_config(struct v4l2_subdev *sd, + unsigned int pad, + struct v4l2_mbus_config *cfg) +{ + struct ov5640_dev *sensor = to_ov5640_dev(sd); + + cfg->type = sensor->ep.bus_type; + if (ov5640_is_csi2(sensor)) { + cfg->bus.mipi_csi2.num_data_lanes = + sensor->ep.bus.mipi_csi2.num_data_lanes; + cfg->bus.mipi_csi2.flags = sensor->ep.bus.mipi_csi2.flags; + } else { + cfg->bus.parallel.flags = sensor->ep.bus.parallel.flags; + } + + return 0; +} + static const struct v4l2_subdev_core_ops ov5640_core_ops = { .log_status = v4l2_ctrl_subdev_log_status, .subscribe_event = v4l2_ctrl_subdev_subscribe_event, @@ -3794,6 +3812,7 @@ static const struct v4l2_subdev_pad_ops ov5640_pad_ops = { .get_selection = ov5640_get_selection, .enum_frame_size = ov5640_enum_frame_size, .enum_frame_interval = ov5640_enum_frame_interval, + .get_mbus_config = ov5640_get_mbus_config, }; static const struct v4l2_subdev_ops ov5640_subdev_ops = {