Hi,
On Thu, Dec 22, 2016 at 11:42:26PM +0100, Pavel Machek wrote:
> On Thu 2016-12-22 15:32:44, Sebastian Reichel wrote:
> > Hi Pavel,
> >
> > On Thu, Dec 22, 2016 at 02:39:38PM +0100, Pavel Machek wrote:
> > > N900 contains front and back camera, with a switch between the
> > > two. This adds support for the swich component.
> > >
> > > Signed-off-by: Sebastian Reichel <sre@xxxxxxxxxx>
> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@xxxxxxxxx>
> > > Signed-off-by: Pavel Machek <pavel@xxxxxx>
> > >
> > > --
> > >
> > > I see this needs dts documentation, anything else than needs to be
> > > done?
> >
> > Yes. This driver takes care of the switch gpio, but the cameras also
> > use different bus settings. Currently omap3isp gets the bus-settings
> > from the link connected to the CCP2 port in DT at probe time (*).
> >
> > So there are two general problems:
> >
> > 1. Settings must be applied before the streaming starts instead of
> > at probe time, since the settings may change (based one the selected
> > camera). That should be fairly easy to implement by just moving the
> > code to the s_stream callback as far as I can see.
> >
> > 2. omap3isp should try to get the bus settings from using a callback
> > in the connected driver instead of loading it from DT. Then the
> > video-bus-switch can load the bus-settings from its downstream links
> > in DT and propagate the correct ones to omap3isp based on the
> > selected port. The DT loading part should actually remain in omap3isp
> > as fallback, in case it does not find a callback in the connected driver.
> > That way everything is backward compatible and the DT variant is
> > nice for 1-on-1 scenarios.
>
> So... did I understood it correctly? (Needs some work to be done...)
I had a quick look and yes, that's basically what I had in mind to
solve the issue. If callback is not available the old system should
be used of course.
> [...]
>
> static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> diff --git a/drivers/media/platform/video-bus-switch.c b/drivers/media/platform/video-bus-switch.c
> index 1a5d944..3a2d442 100644
> --- a/drivers/media/platform/video-bus-switch.c
> +++ b/drivers/media/platform/video-bus-switch.c
> @@ -247,12 +247,21 @@ static int vbs_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct v4l2_subdev *subdev = vbs_get_remote_subdev(sd);
>
> + /* FIXME: we need to set the GPIO here */
> +
The gpio is set when the pad is selected, so no need to do it again.
The gpio selection actually works with your branch (assuming its
based on Ivo's).
> if (IS_ERR(subdev))
> return PTR_ERR(subdev);
>
> return v4l2_subdev_call(subdev, video, s_stream, enable);
> }
>
> +static int vbs_g_endpoint_config(struct v4l2_subdev *sd, struct isp_bus_cfg *cfg)
> +{
> + printk("vbs_g_endpoint_config...\n");
> + return 0;
> +}
Would be nice to find something more abstract than isp_bus_cfg,
which is specific to omap3isp.
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
