RE: [PATCH v2] soc-camera: add API documentation

["Jalori, Mohit" <mjalori@xxxxxx>]

> -----Original Message-----
> From: Sakari Ailus [mailto:sakari.ailus@xxxxxxxxx]
> Sent: Friday, August 29, 2008 7:17 AM
> To: ext Hans Verkuil
> Cc: video4linux-list@xxxxxxxxxx; Mauro Carvalho Chehab; Guennadi
> Liakhovetski; Jalori, Mohit
> Subject: Re: [PATCH v2] soc-camera: add API documentation
> 
> Hi,
> 
> (I'm adding Mohit Jalori at TI to Cc:.)
> 
> ext Hans Verkuil wrote:
> > This is not really a comment on this patch, but more on sensor APIs
> in
> > general. I'm beginning to be concerned about the many different
> > approaches. We have soc-camera, v4l2-int-device.h which some of the
> > omap drivers want to use, gspca which basically sees the bridge
> driver
> > and sensor as one unit, and probably other attempts that I do not
> know
> > about.
> 
> I agree. Although I didn't know about the rest beyond SoC camera and
> v4l2-int-if.
> 
> I'm kind of responsible for the v4l2-int-device stuff. At the time
> there
> was no SoC camera and it was my attempt to break the connection
> between
> the camera controller and the sensor. It also aims to be usable
> outside
> camera V4L devices.
> 
> Guennadi published SoC camera before he knew about v4l2-int-if and so
> we
> had two approaches. Lately I haven't had time to pay attention to new
> developments which I'm a bit sorry for.
> 
> > I am strongly in favor of discussing this situation during the
> Portland
> > Linux Plumbers Conference where at least Mauro, Hans de Goede, Manju
> > from TI and myself will be present.
> 
> I'm unlikely to be able to participate. :I
> 
> > Lately I've been experimenting with improving the V4L2 framework.
> > Prototyping code can be found in my hg/~hverkuil/v4l-dvb-ng/ tree.
> It
> > provides a new generic 'ops' structure (see media/v4l2-client.h)
> that
> > can be used both by i2c drivers and non-i2c driver alike, that is
> easy
> > to extend and that will work for all client drivers, not just
> sensors.
> > And it can be implemented in existing i2c drivers without requiring
> > that the current PCI/USB drivers that use them need to be converted
> at
> > the same time.
> >
> > The client ops structure is this:
> >
> > struct v4l2_client_ops {
> >         const struct v4l2_client_core_ops  *core;
> >         const struct v4l2_client_tuner_ops *tuner;
> >         const struct v4l2_client_audio_ops *audio;
> >         const struct v4l2_client_video_ops *video;
> > };
> >
> > Basically it's a bunch of (possibly NULL) pointers to other ops
> > structures. The pointers can by NULL so that client drivers that are
> > video only do not need to implement e.g. audio ops. It is easy to
> > extend this with other ops structs like a sensor_ops. It probably
> fits
> > fairly well with soc_camera which does something similar, except
> that
> > this approach is general.
> 
> The major diff to v4l2-int-if is that this defines device types. I
> guess
> this way it's easier to define what kind of ioctls are expected to
> work
> for clients. Although I didn't want to define such list as I wasn't
> quite sure I could come up with a definite one at the time. So now
> it's
> actually defined by what the master expects but the size of the
> structure holding the slave function calls does not grow even if the
> slave calls themselves change or more calls are added. It was due to
> this argument that ioctl-like interface was selected for v4l2-int-if
> instead. Think of all the ioctls supported by V4L2.
> 
> struct v4l2_client_ops should at least be a union. But this should not
> be a problem as long as the clients have a type.
> 
> > Remember, it is only prototyping code, but I consider it to be a
> very
> > promising approach. There is a serious lack of V4L2 core support
> which
> > means that drivers tend to re-invent the wheel and all to often that
> > wheel looks suspiciously square to me.
> 
> It is not completely obvious to me, at least, what are the calls that
> are requred to be implemented by the sensor driver. The current
> sensors
> that are supported by Linux (at least the ones I know) are so-called
> smart sensors. All the control algorithms (AE, AWB and possibly AF)
> are
> run directly on the microcontroller inside the sensor module itself.
> 
> The direction, as far as I see, is towards separating the sensor from
> the image processing. An example of this is the OMAP 3 ISP. The OMAP 3
> camera driver is able to work with smart sensors, too, but it's meant
> to
> be used with raw sensors that just produce raw bayer matrix.
> 
> The bayer matrix input is converted (to YUV422, for example) and
> scaled
> based on user requirements.
> 
> The ISP with a raw bayer sensor does not implement the same
> functionality that smart sensors offer. It only produces statistics
> (OMAP 3 ISP, for example) that are useful for the actual control
> algorithms (AE, AWB and AF). One of the consequences are that the
> algorithms are run on host system and these algorithms require
> additional information, something that used to be handled only inside
> the smart sensor. It's not that much different, though.
> 
> > Sorry for the 'rant' about the state the V4L2 sub-system is in, it's
> not
> > a reflection on your patch, it only provided the trigger for this
> > outburst of mine.
> 
> :)
> 
> I've been thinking it could make sense to unify v4l2-int-if and SoC
> camera efforts in longer term.
> 
> Although the approach in SoC camera is somewhat different than in
> v4l2-int-if they share some similarities. V4l2-int-if tries to define
> common set of commands for commanding different hardware devices that
> make one V4L2 device, e.g. /dev/video0. SoC camera, OTOH, is a
> hardware-independent camera driver that can interface with different
> camera controllers and image sensors.
> 
> Interestingly, the concepts used in v4l2-int-if and SoC camera are
> quite
> similar. Roughly equivalent pieces can be found easily:
> 
> v4l2-int-if + OMAP 3 camera		SoC camera
> 
> OMAP 3 camera driver (int if master)	SoC camera driver
> OMAP 3 ISP driver			host
> sensor (int if slave)            	device
> lens (int if slave)
> flash (int if slave)
> 
> Control flow:
> 
> SoC camera
> |    \
> |     \
> |      \
> |       \
> host    device
> 
> OMAP 3 camera driver
> |    |        |   \
> |    |        |    \
> |    |        |     \
> |    |        |      \
> |    sensor   lens    flash
> |    |
> |    |
> |    machine/platform specific code
> |      /
> |     /
> |    /
> |   /
> |  /
> ISP
> 
> Additionally both can employ the use of machine specific glue code if
> needed. It might be that direct OMAP 3 camera driver -> ISP control
> path should go away. There are only a few places where this is done at
> the moment.
> 

Yes I agree it makes sense to remove the dependency between the ISP and OMAP3 camera driver. We tried to minimize that as much as possible. Place where this dependency exists can reorganized. If you look at the ISP calls from camera driver we try to keep them as close to the V4L2 i/f calls as possible to make this porting easy if we can come with this extension.

> (Please correct me if I'm mistaken somewhere above. And otherwise,
> too. :))
> 
> I think few hardware-specific things are ending up in the OMAP 3
> camera
> driver. On that side it seems we could perhaps even make OMAP 3 camera
> driver a generic camera driver that could support lens and flash
> devices
> as well.
> 

Yes that is possible. With the patches going out today only statistics stuff might be still there but other I think is pretty much cleaned out of the driver.

> Making the ISP driver a v4l2-int-if slave would help a lot --- this
> would remove the camera driver -> ISP driver dependency. The ISP
> driver
> would be the fourth slave for the camera driver in that case. But it
> required a standard interface, too. In this case it might make sense
> to
> separate image processing capabilities from the bare sensor hw
> interface
> (i.e. CCP2 receiver).

It would be helpful to have that kind of i/f especially once choices are available for ISPs. Most ISP will be doing similar operations (in different ways) so perhaps it is a good idea to standardize these.

> 
> Best regards,
> 
> --
> Sakari Ailus
> sakari.ailus@xxxxxxxxx


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list