* [PATCH v2] media: i2c: ov5640: Implement get_mbus_config @ 2023-03-06 6:36 Marcel Ziswiler 2023-03-14 12:13 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Marcel Ziswiler @ 2023-03-06 6:36 UTC (permalink / raw) To: linux-media Cc: Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel 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, }; static const struct v4l2_subdev_ops ov5640_subdev_ops = { -- 2.36.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-06 6:36 [PATCH v2] media: i2c: ov5640: Implement get_mbus_config Marcel Ziswiler @ 2023-03-14 12:13 ` Sakari Ailus 2023-03-14 12:32 ` Francesco Dolcini 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2023-03-14 12:13 UTC (permalink / raw) To: Marcel Ziswiler Cc: linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel 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. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-14 12:13 ` Sakari Ailus @ 2023-03-14 12:32 ` Francesco Dolcini 2023-03-14 12:45 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Francesco Dolcini @ 2023-03-14 12:32 UTC (permalink / raw) To: Sakari Ailus Cc: Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-14 12:32 ` Francesco Dolcini @ 2023-03-14 12:45 ` Sakari Ailus 2023-03-14 12:59 ` Francesco Dolcini 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2023-03-14 12:45 UTC (permalink / raw) To: Francesco Dolcini Cc: Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel Hi Francesco, On Tue, Mar 14, 2023 at 01:32:16PM +0100, Francesco Dolcini wrote: > 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. Not even for staging drivers. The driver should be fixed to get that information from the endpoint instead. 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. -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-14 12:45 ` Sakari Ailus @ 2023-03-14 12:59 ` Francesco Dolcini 2023-03-20 8:48 ` Jacopo Mondi 0 siblings, 1 reply; 21+ messages in thread From: Francesco Dolcini @ 2023-03-14 12:59 UTC (permalink / raw) To: Sakari Ailus Cc: Francesco Dolcini, Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch +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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-14 12:59 ` Francesco Dolcini @ 2023-03-20 8:48 ` Jacopo Mondi 2023-03-20 9:00 ` Sakari Ailus 2023-03-20 11:13 ` Francesco Dolcini 0 siblings, 2 replies; 21+ messages in thread From: Jacopo Mondi @ 2023-03-20 8:48 UTC (permalink / raw) To: Francesco Dolcini Cc: Sakari Ailus, Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 8:48 ` Jacopo Mondi @ 2023-03-20 9:00 ` Sakari Ailus 2023-03-20 9:26 ` Laurent Pinchart 2023-03-20 11:13 ` Francesco Dolcini 1 sibling, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2023-03-20 9:00 UTC (permalink / raw) To: Jacopo Mondi Cc: Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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? -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 9:00 ` Sakari Ailus @ 2023-03-20 9:26 ` Laurent Pinchart 2023-03-20 9:37 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Laurent Pinchart @ 2023-03-20 9:26 UTC (permalink / raw) To: Sakari Ailus Cc: Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, Philipp Zabel, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 9:26 ` Laurent Pinchart @ 2023-03-20 9:37 ` Sakari Ailus 2023-03-20 9:55 ` Laurent Pinchart 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2023-03-20 9:37 UTC (permalink / raw) To: Laurent Pinchart Cc: Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, Philipp Zabel, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 9:37 ` Sakari Ailus @ 2023-03-20 9:55 ` Laurent Pinchart 2023-03-20 10:15 ` Sakari Ailus 2023-03-20 11:26 ` Philipp Zabel 0 siblings, 2 replies; 21+ messages in thread From: Laurent Pinchart @ 2023-03-20 9:55 UTC (permalink / raw) To: Sakari Ailus Cc: Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, Philipp Zabel, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 9:55 ` Laurent Pinchart @ 2023-03-20 10:15 ` Sakari Ailus 2023-03-20 13:32 ` Philipp Zabel 2023-03-20 11:26 ` Philipp Zabel 1 sibling, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2023-03-20 10:15 UTC (permalink / raw) To: Laurent Pinchart Cc: Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, Philipp Zabel, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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. -- Kind regrads, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 10:15 ` Sakari Ailus @ 2023-03-20 13:32 ` Philipp Zabel 2023-03-20 14:00 ` Laurent Pinchart 0 siblings, 1 reply; 21+ messages in thread From: Philipp Zabel @ 2023-03-20 13:32 UTC (permalink / raw) To: Sakari Ailus, Laurent Pinchart Cc: Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 13:32 ` Philipp Zabel @ 2023-03-20 14:00 ` Laurent Pinchart 2023-03-20 18:31 ` Dave Stevenson 2023-03-20 21:53 ` Sakari Ailus 0 siblings, 2 replies; 21+ messages in thread From: Laurent Pinchart @ 2023-03-20 14:00 UTC (permalink / raw) To: Philipp Zabel Cc: Sakari Ailus, Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 14:00 ` Laurent Pinchart @ 2023-03-20 18:31 ` Dave Stevenson 2023-03-20 21:53 ` Laurent Pinchart 2023-03-20 21:53 ` Sakari Ailus 1 sibling, 1 reply; 21+ messages in thread From: Dave Stevenson @ 2023-03-20 18:31 UTC (permalink / raw) To: Laurent Pinchart Cc: Philipp Zabel, Sakari Ailus, Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 18:31 ` Dave Stevenson @ 2023-03-20 21:53 ` Laurent Pinchart 0 siblings, 0 replies; 21+ messages in thread From: Laurent Pinchart @ 2023-03-20 21:53 UTC (permalink / raw) To: Dave Stevenson Cc: Philipp Zabel, Sakari Ailus, Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 14:00 ` Laurent Pinchart 2023-03-20 18:31 ` Dave Stevenson @ 2023-03-20 21:53 ` Sakari Ailus 1 sibling, 0 replies; 21+ messages in thread From: Sakari Ailus @ 2023-03-20 21:53 UTC (permalink / raw) To: Laurent Pinchart Cc: Philipp Zabel, Jacopo Mondi, Francesco Dolcini, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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). -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 9:55 ` Laurent Pinchart 2023-03-20 10:15 ` Sakari Ailus @ 2023-03-20 11:26 ` Philipp Zabel 2023-03-20 12:47 ` Jacopo Mondi 1 sibling, 1 reply; 21+ messages in thread From: Philipp Zabel @ 2023-03-20 11:26 UTC (permalink / raw) To: Laurent Pinchart, Sakari Ailus Cc: Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 11:26 ` Philipp Zabel @ 2023-03-20 12:47 ` Jacopo Mondi 2023-03-20 13:28 ` Philipp Zabel 0 siblings, 1 reply; 21+ messages in thread From: Jacopo Mondi @ 2023-03-20 12:47 UTC (permalink / raw) To: Philipp Zabel Cc: Laurent Pinchart, Sakari Ailus, Jacopo Mondi, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 12:47 ` Jacopo Mondi @ 2023-03-20 13:28 ` Philipp Zabel 0 siblings, 0 replies; 21+ messages in thread From: Philipp Zabel @ 2023-03-20 13:28 UTC (permalink / raw) To: Jacopo Mondi Cc: Laurent Pinchart, Sakari Ailus, Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, kernel, Francesco Dolcini, Aishwarya Kothari, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 8:48 ` Jacopo Mondi 2023-03-20 9:00 ` Sakari Ailus @ 2023-03-20 11:13 ` Francesco Dolcini 2023-03-22 13:53 ` Aishwarya Kothari 1 sibling, 1 reply; 21+ messages in thread From: Francesco Dolcini @ 2023-03-20 11:13 UTC (permalink / raw) To: Jacopo Mondi, Aishwarya Kothari Cc: Francesco Dolcini, Sakari Ailus, Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] media: i2c: ov5640: Implement get_mbus_config 2023-03-20 11:13 ` Francesco Dolcini @ 2023-03-22 13:53 ` Aishwarya Kothari 0 siblings, 0 replies; 21+ messages in thread From: Aishwarya Kothari @ 2023-03-22 13:53 UTC (permalink / raw) To: Francesco Dolcini, Jacopo Mondi, Aishwarya Kothari Cc: Sakari Ailus, Marcel Ziswiler, linux-media, Laurent Pinchart, Philipp Zabel, kernel, Sakari Ailus, Francesco Dolcini, Marcel Ziswiler, Mauro Carvalho Chehab, Steve Longerbeam, linux-kernel, Marco Felsch On 20.03.23 12:13, Francesco Dolcini 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? >>> >> 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 > > It worked on the v5.18 without this patch. Aishwarya ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-03-22 13:53 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-06 6:36 [PATCH v2] media: i2c: ov5640: Implement get_mbus_config Marcel Ziswiler 2023-03-14 12:13 ` Sakari Ailus 2023-03-14 12:32 ` Francesco Dolcini 2023-03-14 12:45 ` Sakari Ailus 2023-03-14 12:59 ` Francesco Dolcini 2023-03-20 8:48 ` Jacopo Mondi 2023-03-20 9:00 ` Sakari Ailus 2023-03-20 9:26 ` Laurent Pinchart 2023-03-20 9:37 ` Sakari Ailus 2023-03-20 9:55 ` Laurent Pinchart 2023-03-20 10:15 ` Sakari Ailus 2023-03-20 13:32 ` Philipp Zabel 2023-03-20 14:00 ` Laurent Pinchart 2023-03-20 18:31 ` Dave Stevenson 2023-03-20 21:53 ` Laurent Pinchart 2023-03-20 21:53 ` Sakari Ailus 2023-03-20 11:26 ` Philipp Zabel 2023-03-20 12:47 ` Jacopo Mondi 2023-03-20 13:28 ` Philipp Zabel 2023-03-20 11:13 ` Francesco Dolcini 2023-03-22 13:53 ` Aishwarya Kothari
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).