On Thursday 15 December 2011 22:50:33 Sakari Ailus wrote:
> On Thu, Dec 15, 2011 at 01:54:52PM +0100, Laurent Pinchart wrote:
> > On Thursday 15 December 2011 12:53:03 Sakari Ailus wrote:
> > > On Thu, Dec 15, 2011 at 11:28:06AM +0100, Laurent Pinchart wrote:
> > > > On Thursday 15 December 2011 10:50:34 Sakari Ailus wrote:
> > > > > Configure CSI-2 phy based on platform data in the ISP driver rather
> > > > > than in platform code.
> > > > >
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
> >
> > [snip]
> >
> > > > > diff --git a/drivers/media/video/omap3isp/ispcsiphy.c
> > > > > b/drivers/media/video/omap3isp/ispcsiphy.c index 5be37ce..52af308
> > > > > 100644 --- a/drivers/media/video/omap3isp/ispcsiphy.c
> > > > > +++ b/drivers/media/video/omap3isp/ispcsiphy.c
> > > > > @@ -28,6 +28,8 @@
> >
> > [snip]
> >
> > > > > +int omap3isp_csiphy_config(struct isp_device *isp,
> > > > > + struct v4l2_subdev *csi2_subdev,
> > > > > + struct v4l2_subdev *sensor,
> > > > > + struct v4l2_mbus_framefmt *sensor_fmt)
> > > >
> > > > The number of lanes can depend on the format. Wouldn't it be better
> > > > to add a subdev operation to query the sensor for its bus
> > > > configuration instead of relying on ISP platform data ?
> > >
> > > In principle, yes. That's an interesting point; how this kind of
> > > information would best be delivered?
> >
> > There are two separate information that need to be delivered:
> >
> > - how the lanes are connected on the board
> > - which lanes are used by the sensor, and for what purpose
> >
> > The first information must be supplied through platform data, either to
> > the sensor driver or the OMAP3 ISP driver (or both). As the second
> > information
>
> Both, and both of them may require configuring it. I don't know sensors
> that allow it, but the CSI-2 receiver is flexible in lane mapping.
>
> > comes from the sensor, my idea was to provide the first to the sensor,
> > and to query the sensor in the OMAP3 ISP driver for the full
> > configuration.
>
> In theory, at least, configurations with less lanes need to be specified
> separately. There may be limitations on how the lanes can be used, say,
> using two out of three lanes may require leaving a aparticular lane unused.
In theory, I fully agree. This brings additional complexity for use cases that
might never exist though. Do you think we need to support it from the very
beginning ?
> > > The number of lanes might be something the user would want to touch,
> > > but I'm not entirely sure. You achieve more functionality by providing
> > > that flexibility to the user but I don't see need for configuring that
> > > --- still getting the number of lanes could be interesting.
> >
> > If we want to expose such configuration I think we should do it on the
> > sensor, not the ISP.
>
> I agree.
>
> ...
>
> > > > > @@ -355,6 +358,22 @@ static int isp_video_validate_pipeline(struct
> > > > > isp_pipeline *pipe) fmt_source.format.height !=
> > > > > fmt_sink.format.height)
> > > > >
> > > > > return -EPIPE;
> > > > >
> > > > > + /* Configure CSI-2 receiver based on sensor format. */
> > > > > + if (_subdev == &isp->isp_csi2a.subdev
> > > > > + || _subdev == &isp->isp_csi2c.subdev) {
> > > > > + if (cpu_is_omap3630()) {
> > > > > + /*
> > > > > + * FIXME: CSI-2 is supported only on
> > > > > + * the 3630!
> > > > > + */
> > > >
> > > > Is it ? Or do you mean by the driver ? What would it take to support
> > > > it on OMAP34xx and OMAP35xx ?
> > >
> > > I have no way to test it on the OMAP 3430 since I have no CSI-2 sensor
> > > connected to it. As a matter of fact I've never had one, so I don't
> > > really know.
> >
> > What about assuming it works on the 34xx and 35xx as well ?
>
> I'll change it so.
Thanks
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
