[PATCH RFC] soc-camera: add API documentation

[PATCH RFC] soc-camera: add API documentation

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

---

Please, comment, suggest improvements.

diff --git a/Documentation/video4linux/soc-camera.txt b/Documentation/video4linux/soc-camera.txt
new file mode 100644
index 0000000..b58956a
--- /dev/null
+++ b/Documentation/video4linux/soc-camera.txt
@@ -0,0 +1,109 @@
+			Soc-Camera Subsystem
+			====================
+
+Purpose of the soc-camera subsystem
+-----------------------------------
+
+The soc-camera subsystem provides a unified API between camera host drivers and
+camera sensor drivers. It implements a V4L2 interface to the user, currently
+only the mmap method is supported.
+
+Originally this subsystem has been written to connect drivers for System-on-Chip
+(SoC) video capture interfaces with drivers for CMOS camera sensor chips to
+enable the reuse of sensor drivers with various hosts. The subsystem has been
+designed to support multiple camera host interfaces and multiple cameras per
+interface, although most applications have only one camera sensor. It has also
+been proposed to use this subsystem for USB or PCI based cameras, where there is
+a clear separation between the card / adapter and the sensor. For example, when
+there is an internal i2c bus in the card and the sensor is controlled over it.
+
+Existing drivers
+----------------
+
+Currently there are two host drivers in the mainline: pxa_camera.c for PXA27x
+SoCs and sh_mobile_ceu_camera.c for SuperH SoCs, and four sensor drivers:
+mt9m001.c, mt9m111.c, mt9v022.c and a generic soc_camera_platform.c driver.
+Please, use these driver as examples when developing new ones.
+
+Camera host API
+---------------
+
+A host camera driver is registered using the
+
+soc_camera_host_register(struct soc_camera_host *);
+
+function. The host object can be initialized as follows:
+
+static struct soc_camera_host pxa_soc_camera_host = {
+	.drv_name	= PXA_CAM_DRV_NAME,
+	.ops		= &pxa_soc_camera_host_ops,
+};
+
+All camera host methods are passed in a struct soc_camera_host_ops:
+
+static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
+	.owner		= THIS_MODULE,
+	.add		= pxa_camera_add_device,
+	.remove		= pxa_camera_remove_device,
+	.suspend	= pxa_camera_suspend,
+	.resume		= pxa_camera_resume,
+	.set_fmt_cap	= pxa_camera_set_fmt_cap,
+	.try_fmt_cap	= pxa_camera_try_fmt_cap,
+	.init_videobuf	= pxa_camera_init_videobuf,
+	.reqbufs	= pxa_camera_reqbufs,
+	.poll		= pxa_camera_poll,
+	.querycap	= pxa_camera_querycap,
+	.try_bus_param	= pxa_camera_try_bus_param,
+	.set_bus_param	= pxa_camera_set_bus_param,
+};
+
+.add and .remove methods are called when a sensor is attached to or detached
+from the host, apart from performing host-internal tasks they shall also call
+sensor driver's .init and .release methods respectively. .suspend and .resume
+methods implement host's power-management functionality and its their
+responsibility to call respective sensor's methods. .try_bus_param and
+.set_bus_param are used to negotiate physical connection parameters between the
+host and the sensor. .init_videobuf is called by soc-camera core when a
+video-device is opened, further video-buffer management is implemented completely
+by the specific camera host driver. The rest of the methods are called from
+respective V4L2 operations.
+
+Camera API
+----------
+
+Sensor drivers can use struct soc_camera_link, typically provided by the
+platform, and used to specify to which camera host bus the sensor is connected,
+and arbitrarily provide platform .power and .reset methods for the camera.
+soc_camera_device_register() and soc_camera_device_unregister() functions are
+used to add a sensor driver to or remove one from the system. The registration
+function takes a pointer to struct soc_camera_device as the only parameter.
+This struct can be initialized as follows:
+
+	/* link to driver operations */
+	icd->ops	= &mt9m001_ops;
+	/* link to the underlying physical (e.g., i2c) device */
+	icd->control	= &client->dev;
+	/* window geometry */
+	icd->x_min	= 20;
+	icd->y_min	= 12;
+	icd->x_current	= 20;
+	icd->y_current	= 12;
+	icd->width_min	= 48;
+	icd->width_max	= 1280;
+	icd->height_min	= 32;
+	icd->height_max	= 1024;
+	icd->y_skip_top	= 1;
+	/* camera bus ID, typically obtained from platform data */
+	icd->iface	= icl->bus_id;
+
+struct soc_camera_ops provides .probe and .remove methods, which are called by
+the soc-camera core, when a camera is matched against or removed from a camera
+host bus, .init, .release, .suspend, and .resume are called from the camera host
+driver as discussed above. Other members of this struct provide respective V4L2
+functionality.
+
+struct soc_camera_device also links to an array of struct soc_camera_data_format,
+listing pixel formats, supported by the camera.
+
+--
+Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

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

Re: [PATCH RFC] soc-camera: add API documentation

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> +Existing drivers
> +----------------
> +
> +Currently there are two host drivers in the mainline: pxa_camera.c for PXA27x
> +SoCs and sh_mobile_ceu_camera.c for SuperH SoCs, and four sensor drivers:
> +mt9m001.c, mt9m111.c, mt9v022.c and a generic soc_camera_platform.c driver.
> +Please, use these driver as examples when developing new ones.
                           ^
                           maybe an s here ?
> +Camera host API
> +---------------
> +
> +A host camera driver is registered using the
> +
> +soc_camera_host_register(struct soc_camera_host *);
> +
> +function. The host object can be initialized as follows:
> +
> +static struct soc_camera_host pxa_soc_camera_host = {
> +	.drv_name	= PXA_CAM_DRV_NAME,
> +	.ops		= &pxa_soc_camera_host_ops,
> +};
> +
> +All camera host methods are passed in a struct soc_camera_host_ops:
> +
> +static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
> +	.owner		= THIS_MODULE,
> +	.add		= pxa_camera_add_device,
> +	.remove		= pxa_camera_remove_device,
> +	.suspend	= pxa_camera_suspend,
> +	.resume		= pxa_camera_resume,
> +	.set_fmt_cap	= pxa_camera_set_fmt_cap,
> +	.try_fmt_cap	= pxa_camera_try_fmt_cap,
> +	.init_videobuf	= pxa_camera_init_videobuf,
> +	.reqbufs	= pxa_camera_reqbufs,
> +	.poll		= pxa_camera_poll,
> +	.querycap	= pxa_camera_querycap,
> +	.try_bus_param	= pxa_camera_try_bus_param,
> +	.set_bus_param	= pxa_camera_set_bus_param,
> +};
> +
> +.add and .remove methods are called when a sensor is attached to or detached
> +from the host, apart from performing host-internal tasks they shall also call
> +sensor driver's .init and .release methods respectively. .suspend and .resume
> +methods implement host's power-management functionality and its their
> +responsibility to call respective sensor's methods. .try_bus_param and
> +.set_bus_param are used to negotiate physical connection parameters between the
> +host and the sensor. .init_videobuf is called by soc-camera core when a
> +video-device is opened, further video-buffer management is implemented completely
> +by the specific camera host driver. The rest of the methods are called from
> +respective V4L2 operations.

Maybe a concrete example to help people with concepts of SoCs, hosts, host
driver, camera driver, and so on ...
Like (to be translated to correct english) :

---
Let's assume our hardware system is :
 - an Intel CPU, the PXA270, which does provide a hardware bus for camera
 - a camera chip, which is a Micron MT9M001 CMOS sensor
 - the wires connecting the camera chip (8 for data + 4 control) to the PXA270

The driver involved in camera management are :
 - the host driver, in pxa_camera.c, which handles any camera attached to the
 pxa270
 - the camera driver, in mt9m001.c, which interacts with the camera giving it
 orders to poweron/poweroff, choose output definition, ...
 - the glue binding the host driver to the camera driver (soc_camera.c)
---

Note that it may be only me, and my personnal obsession of real examples to
illustrate. I really like the explanation in that doc :)

--
Robert

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

Re: [PATCH RFC] soc-camera: add API documentation

[Robert Schwebel]

Hi Guennadi,

On Wed, Aug 20, 2008 at 11:40:39AM +0200, Guennadi Liakhovetski wrote:
> +Existing drivers
> +----------------
> +
> +Currently there are two host drivers in the mainline: pxa_camera.c for PXA27x
> +SoCs and sh_mobile_ceu_camera.c for SuperH SoCs, and four sensor drivers:
> +mt9m001.c, mt9m111.c, mt9v022.c and a generic soc_camera_platform.c driver.
> +Please, use these driver as examples when developing new ones.

I wouldn't include this kind of cross-link between a generic
documentation and what's in the tree. The tree is moving quickly and
people tend to forget to update the docs then.

rsc
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

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

[PATCH v2] soc-camera: add API documentation

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

---

Addressed comments to the previous version - thanks! And RFC is gone now - 
if there are no objections this time, I will push it through my hg tree.

diff -u /dev/null a/Documentation/video4linux/soc-camera.txt
--- /dev/null	2008-08-28 10:21:05.503011301 +0200
+++ a/Documentation/video4linux/soc-camera.txt	2008-08-28 16:38:12.000000000 +0200
@@ -0,0 +1,123 @@
+			Soc-Camera Subsystem
+			====================
+
+Terminology
+-----------
+
+The following terms are used in this document:
+ - camera / camera device / camera sensor - a video-camera sensor chip, capable
+   of connecting to a variety of systems and interfaces, typically uses i2c for
+   control and configuration, and a parallel or a serial bus for data.
+ - camera host - an interface, to which a camera is connected. Typically a
+   specialised interface, present on many SoCs, e.g., PXA27x and PXA3xx, SuperH,
+   AVR32, i.MX27, i.MX31.
+ - camera host bus - a connection between a camera host and a camera. Can be
+   parallel or serial, consists of data and control lines, e.g., clock, vertical
+   and horisontal synchronization signals.
+
+Purpose of the soc-camera subsystem
+-----------------------------------
+
+The soc-camera subsystem provides a unified API between camera host drivers and
+camera sensor drivers. It implements a V4L2 interface to the user, currently
+only the mmap method is supported.
+
+Originally this subsystem has been written to connect drivers for System-on-Chip
+(SoC) video capture interfaces with drivers for CMOS camera sensor chips to
+enable the reuse of sensor drivers with various hosts. The subsystem has been
+designed to support multiple camera host interfaces and multiple cameras per
+interface, although most applications have only one camera sensor. It has also
+been proposed to use this subsystem for USB or PCI based cameras, where there is
+a clear separation between the card / adapter and the sensor. For example, when
+there is an internal i2c bus in the card and the sensor is controlled over it.
+
+Existing drivers
+----------------
+
+As of 2.6.27-rc4 there are two host drivers in the mainline: pxa_camera.c for
+PXA27x SoCs and sh_mobile_ceu_camera.c for SuperH SoCs, and four sensor drivers:
+mt9m001.c, mt9m111.c, mt9v022.c and a generic soc_camera_platform.c driver. This
+list is not supposed to be updated, look for more examples in your tree.
+
+Camera host API
+---------------
+
+A host camera driver is registered using the
+
+soc_camera_host_register(struct soc_camera_host *);
+
+function. The host object can be initialized as follows:
+
+static struct soc_camera_host pxa_soc_camera_host = {
+	.drv_name	= PXA_CAM_DRV_NAME,
+	.ops		= &pxa_soc_camera_host_ops,
+};
+
+All camera host methods are passed in a struct soc_camera_host_ops:
+
+static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
+	.owner		= THIS_MODULE,
+	.add		= pxa_camera_add_device,
+	.remove		= pxa_camera_remove_device,
+	.suspend	= pxa_camera_suspend,
+	.resume		= pxa_camera_resume,
+	.set_fmt_cap	= pxa_camera_set_fmt_cap,
+	.try_fmt_cap	= pxa_camera_try_fmt_cap,
+	.init_videobuf	= pxa_camera_init_videobuf,
+	.reqbufs	= pxa_camera_reqbufs,
+	.poll		= pxa_camera_poll,
+	.querycap	= pxa_camera_querycap,
+	.try_bus_param	= pxa_camera_try_bus_param,
+	.set_bus_param	= pxa_camera_set_bus_param,
+};
+
+.add and .remove methods are called when a sensor is attached to or detached
+from the host, apart from performing host-internal tasks they shall also call
+sensor driver's .init and .release methods respectively. .suspend and .resume
+methods implement host's power-management functionality and its their
+responsibility to call respective sensor's methods. .try_bus_param and
+.set_bus_param are used to negotiate physical connection parameters between the
+host and the sensor. .init_videobuf is called by soc-camera core when a
+video-device is opened, further video-buffer management is implemented completely
+by the specific camera host driver. The rest of the methods are called from
+respective V4L2 operations.
+
+Camera API
+----------
+
+Sensor drivers can use struct soc_camera_link, typically provided by the
+platform, and used to specify to which camera host bus the sensor is connected,
+and arbitrarily provide platform .power and .reset methods for the camera.
+soc_camera_device_register() and soc_camera_device_unregister() functions are
+used to add a sensor driver to or remove one from the system. The registration
+function takes a pointer to struct soc_camera_device as the only parameter.
+This struct can be initialized as follows:
+
+	/* link to driver operations */
+	icd->ops	= &mt9m001_ops;
+	/* link to the underlying physical (e.g., i2c) device */
+	icd->control	= &client->dev;
+	/* window geometry */
+	icd->x_min	= 20;
+	icd->y_min	= 12;
+	icd->x_current	= 20;
+	icd->y_current	= 12;
+	icd->width_min	= 48;
+	icd->width_max	= 1280;
+	icd->height_min	= 32;
+	icd->height_max	= 1024;
+	icd->y_skip_top	= 1;
+	/* camera bus ID, typically obtained from platform data */
+	icd->iface	= icl->bus_id;
+
+struct soc_camera_ops provides .probe and .remove methods, which are called by
+the soc-camera core, when a camera is matched against or removed from a camera
+host bus, .init, .release, .suspend, and .resume are called from the camera host
+driver as discussed above. Other members of this struct provide respective V4L2
+functionality.
+
+struct soc_camera_device also links to an array of struct soc_camera_data_format,
+listing pixel formats, supported by the camera.
+
+--
+Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

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

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

[Hans Verkuil]

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 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.

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.

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.

Bottomline is that when you say "It has also been proposed to use this 
subsystem for USB or PCI based cameras, where there is a clear 
separation between the card / adapter and the sensor.", then I would 
not immediately agree to that. Not without further discussions, at 
least.

It's only fairly recently that I have started digging into these issues, 
mostly due to my recent cleanup work which touched a lot of code all 
over. I'm rather disconcerted about the, how shall I put 
it, 'narrow-minded' approach to solving a problem that I've seen all 
over the place. The chosen solution works for that product but it is 
often not general enough to be used elsewhere. I can't blame anyone due 
to the fact that there is so little guidance from the v4l2 framework. 
But I'm trying to change that.

Anyway, when it comes to this documentation I'd say that I think that 
the line "It has also been proposed to..." has no place here. You are 
documenting what is, not what might (or might not!) be. In addition 
it's 'horizontal', not 'horisontal'. :-)

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.

Regards,

	Hans


On Thursday 28 August 2008 16:49:42 Guennadi Liakhovetski wrote:
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>
> ---
>
> Addressed comments to the previous version - thanks! And RFC is gone
> now - if there are no objections this time, I will push it through my
> hg tree.
>
> diff -u /dev/null a/Documentation/video4linux/soc-camera.txt
> --- /dev/null	2008-08-28 10:21:05.503011301 +0200
> +++ a/Documentation/video4linux/soc-camera.txt	2008-08-28
> 16:38:12.000000000 +0200 @@ -0,0 +1,123 @@
> +			Soc-Camera Subsystem
> +			====================
> +
> +Terminology
> +-----------
> +
> +The following terms are used in this document:
> + - camera / camera device / camera sensor - a video-camera sensor
> chip, capable +   of connecting to a variety of systems and
> interfaces, typically uses i2c for +   control and configuration, and
> a parallel or a serial bus for data. + - camera host - an interface,
> to which a camera is connected. Typically a +   specialised
> interface, present on many SoCs, e.g., PXA27x and PXA3xx, SuperH, +  
> AVR32, i.MX27, i.MX31.
> + - camera host bus - a connection between a camera host and a
> camera. Can be +   parallel or serial, consists of data and control
> lines, e.g., clock, vertical +   and horisontal synchronization
> signals.
> +
> +Purpose of the soc-camera subsystem
> +-----------------------------------
> +
> +The soc-camera subsystem provides a unified API between camera host
> drivers and +camera sensor drivers. It implements a V4L2 interface to
> the user, currently +only the mmap method is supported.
> +
> +Originally this subsystem has been written to connect drivers for
> System-on-Chip +(SoC) video capture interfaces with drivers for CMOS
> camera sensor chips to +enable the reuse of sensor drivers with
> various hosts. The subsystem has been +designed to support multiple
> camera host interfaces and multiple cameras per +interface, although
> most applications have only one camera sensor. It has also +been
> proposed to use this subsystem for USB or PCI based cameras, where
> there is +a clear separation between the card / adapter and the
> sensor. For example, when +there is an internal i2c bus in the card
> and the sensor is controlled over it. +
> +Existing drivers
> +----------------
> +
> +As of 2.6.27-rc4 there are two host drivers in the mainline:
> pxa_camera.c for +PXA27x SoCs and sh_mobile_ceu_camera.c for SuperH
> SoCs, and four sensor drivers: +mt9m001.c, mt9m111.c, mt9v022.c and a
> generic soc_camera_platform.c driver. This +list is not supposed to
> be updated, look for more examples in your tree. +
> +Camera host API
> +---------------
> +
> +A host camera driver is registered using the
> +
> +soc_camera_host_register(struct soc_camera_host *);
> +
> +function. The host object can be initialized as follows:
> +
> +static struct soc_camera_host pxa_soc_camera_host = {
> +	.drv_name	= PXA_CAM_DRV_NAME,
> +	.ops		= &pxa_soc_camera_host_ops,
> +};
> +
> +All camera host methods are passed in a struct soc_camera_host_ops:
> +
> +static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
> +	.owner		= THIS_MODULE,
> +	.add		= pxa_camera_add_device,
> +	.remove		= pxa_camera_remove_device,
> +	.suspend	= pxa_camera_suspend,
> +	.resume		= pxa_camera_resume,
> +	.set_fmt_cap	= pxa_camera_set_fmt_cap,
> +	.try_fmt_cap	= pxa_camera_try_fmt_cap,
> +	.init_videobuf	= pxa_camera_init_videobuf,
> +	.reqbufs	= pxa_camera_reqbufs,
> +	.poll		= pxa_camera_poll,
> +	.querycap	= pxa_camera_querycap,
> +	.try_bus_param	= pxa_camera_try_bus_param,
> +	.set_bus_param	= pxa_camera_set_bus_param,
> +};
> +
> +.add and .remove methods are called when a sensor is attached to or
> detached +from the host, apart from performing host-internal tasks
> they shall also call +sensor driver's .init and .release methods
> respectively. .suspend and .resume +methods implement host's
> power-management functionality and its their +responsibility to call
> respective sensor's methods. .try_bus_param and +.set_bus_param are
> used to negotiate physical connection parameters between the +host
> and the sensor. .init_videobuf is called by soc-camera core when a
> +video-device is opened, further video-buffer management is
> implemented completely +by the specific camera host driver. The rest
> of the methods are called from +respective V4L2 operations.
> +
> +Camera API
> +----------
> +
> +Sensor drivers can use struct soc_camera_link, typically provided by
> the +platform, and used to specify to which camera host bus the
> sensor is connected, +and arbitrarily provide platform .power and
> .reset methods for the camera. +soc_camera_device_register() and
> soc_camera_device_unregister() functions are +used to add a sensor
> driver to or remove one from the system. The registration +function
> takes a pointer to struct soc_camera_device as the only parameter.
> +This struct can be initialized as follows:
> +
> +	/* link to driver operations */
> +	icd->ops	= &mt9m001_ops;
> +	/* link to the underlying physical (e.g., i2c) device */
> +	icd->control	= &client->dev;
> +	/* window geometry */
> +	icd->x_min	= 20;
> +	icd->y_min	= 12;
> +	icd->x_current	= 20;
> +	icd->y_current	= 12;
> +	icd->width_min	= 48;
> +	icd->width_max	= 1280;
> +	icd->height_min	= 32;
> +	icd->height_max	= 1024;
> +	icd->y_skip_top	= 1;
> +	/* camera bus ID, typically obtained from platform data */
> +	icd->iface	= icl->bus_id;
> +
> +struct soc_camera_ops provides .probe and .remove methods, which are
> called by +the soc-camera core, when a camera is matched against or
> removed from a camera +host bus, .init, .release, .suspend, and
> .resume are called from the camera host +driver as discussed above.
> Other members of this struct provide respective V4L2 +functionality.
> +
> +struct soc_camera_device also links to an array of struct
> soc_camera_data_format, +listing pixel formats, supported by the
> camera.
> +
> +--
> +Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>
> --
> video4linux-list mailing list
> Unsubscribe
> mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list


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

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

[Sakari Ailus]

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.

(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.

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).

Best regards,

-- 
Sakari Ailus
sakari.ailus@nokia.com

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

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

[Hans Verkuil]

On Friday 29 August 2008 14:17:08 Sakari Ailus wrote:
> Hi,
>
> (I'm adding Mohit Jalori at TI to Cc:.)

Thank you, I should have done that myself.

>
> 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.

In general it is quite noticable that a lot of the camera code in 
v4l-dvb was originally written as out-of-tree code. So developers 
(quite naturally) did their own thing since getting the devices to work 
was more important than creating generic frameworks. But recently a lot 
of those drivers were merged and the lack of consistency becomes much 
more obvious now.

> > 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.

I prefer strong typing whereever possible. Basically this solution 
allows you to subdivide the full range of operations into several 
interfaces and drivers can choose to implement one or more of those as 
needed. If a driver doesn't do anything with audio, then there is no 
need to define the audio interface, so it doesn't take up any space 
either (except for a single NULL v4l2_client_audio_ops pointer).

And it was indeed my intention to add a union as well to the client_ops 
where you can put in mutually-exclusive interfaces. And quite possibly 
a union as well where you can put in driver-specific interfaces. E.g.:

// forward references
// If you want to use these ops, then you
// have to include drv_foo.h first for the full definition
struct drv_foo_ops;
struct drv_bar_ops;

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;
	int drv_id;
	union {
		const struct drv_foo_ops *foo;
		const struct drv_bar_ops *bar;
	} drv;
};

Since v4l2_client_ops is completely an internal API it can be changed 
fairly easily if it turns out that the interfaces need to be rearranged 
for more optimal use.

>
> > 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.
>
> (Please correct me if I'm mistaken somewhere above. And otherwise,
> too. :))

ISP = In-System Programming?

I definitely agree that an effort should be made to unify these two. It 
seems to be the logical approach. But what I am even more interested in 
is to work on embedding all the v4l drivers into a larger framework of 
code v4l types providing core v4l services.

So by using v4l2_client as the basic type for all client drivers 
(sensors, lens controllers, audio muxes, video encoders/decoders, you 
name it), it becomes possible to write basic services for that as well: 
for example my v4l-dvb-ng tree also contains a v4l2_device type which 
embodies a v4l device instance (e.g. a webcam, a PCI framegrabber, 
etc). This structure has a list where all the v4l2_clients register 
themselves, and they are automatically unloaded when the v4l device is 
unloaded. In addition, the v4l2_device can be registered with a single 
global list of v4l2_devices.

It's all pretty simple, but every driver is reinventing this stuff again 
and again. And the quality of those implementations differs enormously.

As far as I can see it would be relatively easy to implement both soc 
and v4l2-int-device on top of v4l2_client: several ops already fall 
within either core_ops or video_ops, the remainder can initially go 
into a soc_ops or int_ops. Later these two can be merged into one or 
more ops structs.

> 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.
>
> 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).

What makes a huge difference is that all these interfaces are internal 
to the kernel. So it is perfectly fine to start with a suboptimal 
interface and refine it as more devices are added.

Note that I am not a webcam/sensor expert, so let me know if I 
misunderstand some concept or if I clearly don't know what I'm talking 
about :-), but I do have a pretty good overview of the v4l drivers by 
now and the one thing that became abundantly clear to me was that the 
lack of proper v4l core services is a serious problem. It's something 
that needs to be implemented urgently IMHO.

Regards,

	Hans

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

[PATCH v3] soc-camera: add API documentation

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

---

Ok, more comments addressed, thanks to all again, we might even get it in 
for 2.6.27? It certainly will not cause any regressions:-)

diff -u /dev/null b/Documentation/video4linux/soc-camera.txt
--- /dev/null	2008-08-29 08:10:06.483001029 +0200
+++ b/Documentation/video4linux/soc-camera.txt	2008-08-29 19:56:41.000000000 +0200
@@ -0,0 +1,120 @@
+			Soc-Camera Subsystem
+			====================
+
+Terminology
+-----------
+
+The following terms are used in this document:
+ - camera / camera device / camera sensor - a video-camera sensor chip, capable
+   of connecting to a variety of systems and interfaces, typically uses i2c for
+   control and configuration, and a parallel or a serial bus for data.
+ - camera host - an interface, to which a camera is connected. Typically a
+   specialised interface, present on many SoCs, e.g., PXA27x and PXA3xx, SuperH,
+   AVR32, i.MX27, i.MX31.
+ - camera host bus - a connection between a camera host and a camera. Can be
+   parallel or serial, consists of data and control lines, e.g., clock, vertical
+   and horizontal synchronization signals.
+
+Purpose of the soc-camera subsystem
+-----------------------------------
+
+The soc-camera subsystem provides a unified API between camera host drivers and
+camera sensor drivers. It implements a V4L2 interface to the user, currently
+only the mmap method is supported.
+
+This subsystem has been written to connect drivers for System-on-Chip (SoC)
+video capture interfaces with drivers for CMOS camera sensor chips to enable
+the reuse of sensor drivers with various hosts. The subsystem has been designed
+to support multiple camera host interfaces and multiple cameras per interface,
+although most applications have only one camera sensor.
+
+Existing drivers
+----------------
+
+As of 2.6.27-rc4 there are two host drivers in the mainline: pxa_camera.c for
+PXA27x SoCs and sh_mobile_ceu_camera.c for SuperH SoCs, and four sensor drivers:
+mt9m001.c, mt9m111.c, mt9v022.c and a generic soc_camera_platform.c driver. This
+list is not supposed to be updated, look for more examples in your tree.
+
+Camera host API
+---------------
+
+A host camera driver is registered using the
+
+soc_camera_host_register(struct soc_camera_host *);
+
+function. The host object can be initialized as follows:
+
+static struct soc_camera_host pxa_soc_camera_host = {
+	.drv_name	= PXA_CAM_DRV_NAME,
+	.ops		= &pxa_soc_camera_host_ops,
+};
+
+All camera host methods are passed in a struct soc_camera_host_ops:
+
+static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
+	.owner		= THIS_MODULE,
+	.add		= pxa_camera_add_device,
+	.remove		= pxa_camera_remove_device,
+	.suspend	= pxa_camera_suspend,
+	.resume		= pxa_camera_resume,
+	.set_fmt_cap	= pxa_camera_set_fmt_cap,
+	.try_fmt_cap	= pxa_camera_try_fmt_cap,
+	.init_videobuf	= pxa_camera_init_videobuf,
+	.reqbufs	= pxa_camera_reqbufs,
+	.poll		= pxa_camera_poll,
+	.querycap	= pxa_camera_querycap,
+	.try_bus_param	= pxa_camera_try_bus_param,
+	.set_bus_param	= pxa_camera_set_bus_param,
+};
+
+.add and .remove methods are called when a sensor is attached to or detached
+from the host, apart from performing host-internal tasks they shall also call
+sensor driver's .init and .release methods respectively. .suspend and .resume
+methods implement host's power-management functionality and its their
+responsibility to call respective sensor's methods. .try_bus_param and
+.set_bus_param are used to negotiate physical connection parameters between the
+host and the sensor. .init_videobuf is called by soc-camera core when a
+video-device is opened, further video-buffer management is implemented completely
+by the specific camera host driver. The rest of the methods are called from
+respective V4L2 operations.
+
+Camera API
+----------
+
+Sensor drivers can use struct soc_camera_link, typically provided by the
+platform, and used to specify to which camera host bus the sensor is connected,
+and arbitrarily provide platform .power and .reset methods for the camera.
+soc_camera_device_register() and soc_camera_device_unregister() functions are
+used to add a sensor driver to or remove one from the system. The registration
+function takes a pointer to struct soc_camera_device as the only parameter.
+This struct can be initialized as follows:
+
+	/* link to driver operations */
+	icd->ops	= &mt9m001_ops;
+	/* link to the underlying physical (e.g., i2c) device */
+	icd->control	= &client->dev;
+	/* window geometry */
+	icd->x_min	= 20;
+	icd->y_min	= 12;
+	icd->x_current	= 20;
+	icd->y_current	= 12;
+	icd->width_min	= 48;
+	icd->width_max	= 1280;
+	icd->height_min	= 32;
+	icd->height_max	= 1024;
+	icd->y_skip_top	= 1;
+	/* camera bus ID, typically obtained from platform data */
+	icd->iface	= icl->bus_id;
+
+struct soc_camera_ops provides .probe and .remove methods, which are called by
+the soc-camera core, when a camera is matched against or removed from a camera
+host bus, .init, .release, .suspend, and .resume are called from the camera host
+driver as discussed above. Other members of this struct provide respective V4L2
+functionality.
+
+struct soc_camera_device also links to an array of struct soc_camera_data_format,
+listing pixel formats, supported by the camera.
+
+--
+Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

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

Re: [PATCH v3] soc-camera: add API documentation

[Hans Verkuil]

On Friday 29 August 2008 20:16:31 Guennadi Liakhovetski wrote:
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>
> ---
>
> Ok, more comments addressed, thanks to all again, we might even get
> it in for 2.6.27? It certainly will not cause any regressions:-)

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

And apologies for hijacking your post for me rant :-)

Regards,

	Hans

>
> diff -u /dev/null b/Documentation/video4linux/soc-camera.txt
> --- /dev/null	2008-08-29 08:10:06.483001029 +0200
> +++ b/Documentation/video4linux/soc-camera.txt	2008-08-29
> 19:56:41.000000000 +0200 @@ -0,0 +1,120 @@
> +			Soc-Camera Subsystem
> +			====================
> +
> +Terminology
> +-----------
> +
> +The following terms are used in this document:
> + - camera / camera device / camera sensor - a video-camera sensor
> chip, capable +   of connecting to a variety of systems and
> interfaces, typically uses i2c for +   control and configuration, and
> a parallel or a serial bus for data. + - camera host - an interface,
> to which a camera is connected. Typically a +   specialised
> interface, present on many SoCs, e.g., PXA27x and PXA3xx, SuperH, +  
> AVR32, i.MX27, i.MX31.
> + - camera host bus - a connection between a camera host and a
> camera. Can be +   parallel or serial, consists of data and control
> lines, e.g., clock, vertical +   and horizontal synchronization
> signals.
> +
> +Purpose of the soc-camera subsystem
> +-----------------------------------
> +
> +The soc-camera subsystem provides a unified API between camera host
> drivers and +camera sensor drivers. It implements a V4L2 interface to
> the user, currently +only the mmap method is supported.
> +
> +This subsystem has been written to connect drivers for
> System-on-Chip (SoC) +video capture interfaces with drivers for CMOS
> camera sensor chips to enable +the reuse of sensor drivers with
> various hosts. The subsystem has been designed +to support multiple
> camera host interfaces and multiple cameras per interface, +although
> most applications have only one camera sensor.
> +
> +Existing drivers
> +----------------
> +
> +As of 2.6.27-rc4 there are two host drivers in the mainline:
> pxa_camera.c for +PXA27x SoCs and sh_mobile_ceu_camera.c for SuperH
> SoCs, and four sensor drivers: +mt9m001.c, mt9m111.c, mt9v022.c and a
> generic soc_camera_platform.c driver. This +list is not supposed to
> be updated, look for more examples in your tree. +
> +Camera host API
> +---------------
> +
> +A host camera driver is registered using the
> +
> +soc_camera_host_register(struct soc_camera_host *);
> +
> +function. The host object can be initialized as follows:
> +
> +static struct soc_camera_host pxa_soc_camera_host = {
> +	.drv_name	= PXA_CAM_DRV_NAME,
> +	.ops		= &pxa_soc_camera_host_ops,
> +};
> +
> +All camera host methods are passed in a struct soc_camera_host_ops:
> +
> +static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
> +	.owner		= THIS_MODULE,
> +	.add		= pxa_camera_add_device,
> +	.remove		= pxa_camera_remove_device,
> +	.suspend	= pxa_camera_suspend,
> +	.resume		= pxa_camera_resume,
> +	.set_fmt_cap	= pxa_camera_set_fmt_cap,
> +	.try_fmt_cap	= pxa_camera_try_fmt_cap,
> +	.init_videobuf	= pxa_camera_init_videobuf,
> +	.reqbufs	= pxa_camera_reqbufs,
> +	.poll		= pxa_camera_poll,
> +	.querycap	= pxa_camera_querycap,
> +	.try_bus_param	= pxa_camera_try_bus_param,
> +	.set_bus_param	= pxa_camera_set_bus_param,
> +};
> +
> +.add and .remove methods are called when a sensor is attached to or
> detached +from the host, apart from performing host-internal tasks
> they shall also call +sensor driver's .init and .release methods
> respectively. .suspend and .resume +methods implement host's
> power-management functionality and its their +responsibility to call
> respective sensor's methods. .try_bus_param and +.set_bus_param are
> used to negotiate physical connection parameters between the +host
> and the sensor. .init_videobuf is called by soc-camera core when a
> +video-device is opened, further video-buffer management is
> implemented completely +by the specific camera host driver. The rest
> of the methods are called from +respective V4L2 operations.
> +
> +Camera API
> +----------
> +
> +Sensor drivers can use struct soc_camera_link, typically provided by
> the +platform, and used to specify to which camera host bus the
> sensor is connected, +and arbitrarily provide platform .power and
> .reset methods for the camera. +soc_camera_device_register() and
> soc_camera_device_unregister() functions are +used to add a sensor
> driver to or remove one from the system. The registration +function
> takes a pointer to struct soc_camera_device as the only parameter.
> +This struct can be initialized as follows:
> +
> +	/* link to driver operations */
> +	icd->ops	= &mt9m001_ops;
> +	/* link to the underlying physical (e.g., i2c) device */
> +	icd->control	= &client->dev;
> +	/* window geometry */
> +	icd->x_min	= 20;
> +	icd->y_min	= 12;
> +	icd->x_current	= 20;
> +	icd->y_current	= 12;
> +	icd->width_min	= 48;
> +	icd->width_max	= 1280;
> +	icd->height_min	= 32;
> +	icd->height_max	= 1024;
> +	icd->y_skip_top	= 1;
> +	/* camera bus ID, typically obtained from platform data */
> +	icd->iface	= icl->bus_id;
> +
> +struct soc_camera_ops provides .probe and .remove methods, which are
> called by +the soc-camera core, when a camera is matched against or
> removed from a camera +host bus, .init, .release, .suspend, and
> .resume are called from the camera host +driver as discussed above.
> Other members of this struct provide respective V4L2 +functionality.
> +
> +struct soc_camera_device also links to an array of struct
> soc_camera_data_format, +listing pixel formats, supported by the
> camera.
> +
> +--
> +Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>


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

Re: [PATCH v3] soc-camera: add API documentation

[Guennadi Liakhovetski]

On Fri, 29 Aug 2008, Hans Verkuil wrote:

> On Friday 29 August 2008 20:16:31 Guennadi Liakhovetski wrote:
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >
> > ---
> >
> > Ok, more comments addressed, thanks to all again, we might even get
> > it in for 2.6.27? It certainly will not cause any regressions:-)
> 
> Acked-by: Hans Verkuil <hverkuil@xs4all.nl>
> 
> And apologies for hijacking your post for me rant :-)

No problem at all. I am glad it served the purpose. It is clear we need an 
API for the SoC camera interface, or a USB dongle, or a PCI card on one 
side and a CMOS sensor, or a frame-grabber, or you-name-them on the other 
side. I might well imagine, that the current soc-camera API is not a final 
solution, i.e., it will not suit all these purposes, so, I am all for a 
better more generic interface. I only hope the transition will not be too 
painful... And I suspect, we will get a couple more soc-camera drivers by 
the time the new API gets implemented, so, I very much hope we will not 
have to largely re-write them all:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

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

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

[Jean-Francois Moine]

On Fri, 2008-08-29 at 18:34 -0300, Mauro Carvalho Chehab wrote:
	[split]
> At the other side of the coin, we have lots of drivers that don't use any API
> to split sensors from the bridge drivers, being GSPCA the newest example. I
> believe that splitting sensors from bridge drivers will benefit to reduce the
> amount of coding size on such drivers, and help to fix bugs that are common to
> several webcams. Of course, such conversion would be a huge task.

I think this subject was already discussed.

If you look at the gspca code, you will see that many sensors are used
by different bridges. Indeed, some values are closed from each other,
but what to do with the differences? (the initial values of the sensor
registers seem to be tied to the physical wires, signal levels and
capabilities of the bridges)

Also, the way to load the sensor registers is very different from one
bridge to an other one (direct write, with one or many values in one
write, paging, bridge packets with different headers/tails..).

Eventually, looking again at the code, you will see that the sensor
exchanges are only a small part of the code.

Cheers.

-- 
Ken ar c'hentañ |             ** Breizh ha Linux atav! **
Jef             |               http://moinejf.free.fr/


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

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

[Jean-Francois Moine]

On Sat, 2008-08-30 at 07:03 -0300, Mauro Carvalho Chehab wrote:
	[snip]
> Yes, but still, this is something that it is still painful: gspca has lots of
> "magic" constants inside. Things like (from etoms.c):
> 	reg_w_val(gspca_dev, 0x80, 0x00);
	[snip]
> It is something really hard to understand why reg 3 needs value 1. OK, I know that
> most of this came from snooping URB's at another system driver, but, if there
> were bugs at the reversed-engineered driver (and there are bugs there also),
> you won't have any glue to debug.

Anyway you do it, I (we) have almost no information about the webcam
bridges and sensors. Sometimes, we find some documents and then, the
spec calls RSVD (reserved) some registers which are later differently
initialized for such and such bridge!

> It is much clearer if you look, for example, at ov7670:
> 	{ REG_CLKRC, 0x1 },	/* OV: clock scale (30 fps) */
> 	{ REG_TSLB,  0x04 },	/* OV */
	[snip]
> Ok, it is not fair to compare with ov7670, since I suspect that OLPC got the
> datasheets for that device. Yet, having a driver for each sensor makes easier
> to add more detailed info at that driver, if later the device manufacturer
> opens for us their datasheets, like several manufacturers already did.

I don't think so. First, I don't think the device manufacturers will
give information about their product: they prefer to sell opaque
drivers. Then, as, at job, I built firmware for embedded boards, I know
that a master/slave interface may be very complex and not easy to
explain. Also, once the driver works, why would you change anything?

> > If you look at the gspca code, you will see that many sensors are used
> > by different bridges. Indeed, some values are closed from each other,
> > but what to do with the differences? (the initial values of the sensor
> > registers seem to be tied to the physical wires, signal levels and
> > capabilities of the bridges)
> 
> I can see two ways:
	[snip]

Why do you want the drivers to be so complexified? I already tried to
look at such drivers, and I could not find where was the used code. With
gspca, all the webcam dependant stuff is located in one file only.

> The real gain is not the size reduction. Having a generic module for a sensor
> have some benefits:
> 
> 1) All common stuff being there means that it will be easier to maintain, since
> a bug found on one module should be replied on other modules;

I never saw that!

> 2) Newer drivers may be written faster, since they may use the existing code.

Usually, newer drivers are written because they do not fall into some
existing code.

> I'm not saying that we _need_ to do this change for gspca. I'm just saying that
> this is a _good_ improvement to the code. I know that this change
> requires lots of janitors effort with little gain to the end users.

I would go to an other way: instead of developping general functions
which have to be customized, why don't you use the generic gspca driver
as a base for all the USB video devices (including TV)?

Cheers.

-- 
Ken ar c'hentañ |             ** Breizh ha Linux atav! **
Jef             |               http://moinejf.free.fr/


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

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

[Hans Verkuil]

On Saturday 30 August 2008 16:58:46 Jean-Francois Moine wrote:
> On Sat, 2008-08-30 at 07:03 -0300, Mauro Carvalho Chehab wrote:
> 	[snip]
>
> > Yes, but still, this is something that it is still painful: gspca
> > has lots of "magic" constants inside. Things like (from etoms.c):
> > 	reg_w_val(gspca_dev, 0x80, 0x00);
>
> 	[snip]
>
> > It is something really hard to understand why reg 3 needs value 1.
> > OK, I know that most of this came from snooping URB's at another
> > system driver, but, if there were bugs at the reversed-engineered
> > driver (and there are bugs there also), you won't have any glue to
> > debug.
>
> Anyway you do it, I (we) have almost no information about the webcam
> bridges and sensors. Sometimes, we find some documents and then, the
> spec calls RSVD (reserved) some registers which are later differently
> initialized for such and such bridge!

I think it makes a big difference whether you have full documentation or 
not. In the first case you have all the information you need to make a 
generic sensor driver, in the second case that will be much harder and 
probably for little gain.

> > It is much clearer if you look, for example, at ov7670:
> > 	{ REG_CLKRC, 0x1 },	/* OV: clock scale (30 fps) */
> > 	{ REG_TSLB,  0x04 },	/* OV */
>
> 	[snip]
>
> > Ok, it is not fair to compare with ov7670, since I suspect that
> > OLPC got the datasheets for that device. Yet, having a driver for
> > each sensor makes easier to add more detailed info at that driver,
> > if later the device manufacturer opens for us their datasheets,
> > like several manufacturers already did.
>
> I don't think so. First, I don't think the device manufacturers will
> give information about their product: they prefer to sell opaque
> drivers. Then, as, at job, I built firmware for embedded boards, I
> know that a master/slave interface may be very complex and not easy
> to explain. Also, once the driver works, why would you change
> anything?

It is true that it is getting easier to obtain datasheets. So that 
definitely helps. And having reusable drivers rather than drivers tied 
to a specific product is preferable in the long run. Improvements to 
that driver will benefit all products that use that driver, as opposed 
to only that particular product. It's not the only consideration, but 
definitely a very important one.

> > > If you look at the gspca code, you will see that many sensors are
> > > used by different bridges. Indeed, some values are closed from
> > > each other, but what to do with the differences? (the initial
> > > values of the sensor registers seem to be tied to the physical
> > > wires, signal levels and capabilities of the bridges)

In general you need a function that you can call to setup such things. 
The same happens with video encoders like the saa7115: depending on how 
it is wired up you have to setup the routing pins, clock frequencies 
and similar settings. Not terribly difficult to do.

> >
> > I can see two ways:
>
> 	[snip]
>
> Why do you want the drivers to be so complexified? I already tried to
> look at such drivers, and I could not find where was the used code.
> With gspca, all the webcam dependant stuff is located in one file
> only.

I can only speak for how the i2c modules are organized that the video 
capture drivers use. Most of these drivers use one or more i2c modules 
to process audio and/or video. Depending on the particular card model 
you load the right combination of modules, but after that you can 
program them all in the same way since they all have the same API. It 
works great there because it makes it easy to add support to a new card 
and also because several of these i2c drivers are quite complex and you 
really do not want to have them duplicated. Also, we have the 
datasheets for pretty much all of them.

> > The real gain is not the size reduction. Having a generic module
> > for a sensor have some benefits:
> >
> > 1) All common stuff being there means that it will be easier to
> > maintain, since a bug found on one module should be replied on
> > other modules;
>
> I never saw that!

Not yet, perhaps.

> > 2) Newer drivers may be written faster, since they may use the
> > existing code.
>
> Usually, newer drivers are written because they do not fall into some
> existing code.
>
> > I'm not saying that we _need_ to do this change for gspca. I'm just
> > saying that this is a _good_ improvement to the code. I know that
> > this change requires lots of janitors effort with little gain to
> > the end users.
>
> I would go to an other way: instead of developping general functions
> which have to be customized, why don't you use the generic gspca
> driver as a base for all the USB video devices (including TV)?

No thank you, especially not for TV drivers. I'll bet that if you would 
have had full documentation from the start for all the devices you 
probably wouldn't have set it up like this.

It is basic good design to have separate drivers (or at least sources) 
for each chip. Due to lack of documentation that isn't always possible, 
so then you have to fall back to an alternative.

Another reason why having a single driver for each chip is good is that 
it allows the community to steadily improve that driver. Usually the 
first implementation will do just the basics. Later additions implement 
the remaining interesting features that most chips offer in one way or 
another. You probably wouldn't do that if the support for the same chip 
was implemented in multiple places. And those improvements will benefit 
all products that use that chip.

Having said all this, I honestly do not know how easy or hard it really 
is to write a generic sensor API to be used with those sensors for 
which a datasheet is available. I don't know enough about webcam 
drivers right now. But I do know that it is a very good long-term 
strategy.

Regards,

	Hans

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

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

[Sakari Ailus]

ext Hans Verkuil wrote:
> I prefer strong typing whereever possible. Basically this solution 
> allows you to subdivide the full range of operations into several 
> interfaces and drivers can choose to implement one or more of those as 
> needed. If a driver doesn't do anything with audio, then there is no 
> need to define the audio interface, so it doesn't take up any space 
> either (except for a single NULL v4l2_client_audio_ops pointer).

How about caller, does the caller need to ensure that a driver actually
implements a function or how should this be handled?

Compared to v4l2-int-if --- if there's no slave, you'll get -ENODEV,
and in case of missing command, you get -ENOIOCTLCMD. That's not in the
tree yet but it's part of OMAP 3 camera driver patches.

Commands in v4l2-int-if could be arranged so that sensor commands are 
grouped together, although they would still share the same namespace, 
unlike in v4l2_client.

> And it was indeed my intention to add a union as well to the client_ops 
> where you can put in mutually-exclusive interfaces. And quite possibly 
> a union as well where you can put in driver-specific interfaces. E.g.:
> 
> // forward references
> // If you want to use these ops, then you
> // have to include drv_foo.h first for the full definition
> struct drv_foo_ops;
> struct drv_bar_ops;
> 
> 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;
> 	int drv_id;
> 	union {
> 		const struct drv_foo_ops *foo;
> 		const struct drv_bar_ops *bar;
> 	} drv;
> };
> 
> Since v4l2_client_ops is completely an internal API it can be changed 
> fairly easily if it turns out that the interfaces need to be rearranged 
> for more optimal use.

Union drv will grow huge over time if the interface becomes widely used. 
And there's a dependency to every driver, too. :I

>> 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.
>>
>> (Please correct me if I'm mistaken somewhere above. And otherwise,
>> too. :))
> 
> ISP = In-System Programming?

ISP stands for Image Signal Processor. It's the block in OMAP 3 that the 
sensors interface to and which does some image processing, too, e.g. 
noise filtering and lens shading correction.

> I definitely agree that an effort should be made to unify these two. It 
> seems to be the logical approach. But what I am even more interested in 
> is to work on embedding all the v4l drivers into a larger framework of 
> code v4l types providing core v4l services.
> 
> So by using v4l2_client as the basic type for all client drivers 
> (sensors, lens controllers, audio muxes, video encoders/decoders, you 
> name it), it becomes possible to write basic services for that as well: 
> for example my v4l-dvb-ng tree also contains a v4l2_device type which 
> embodies a v4l device instance (e.g. a webcam, a PCI framegrabber, 
> etc). This structure has a list where all the v4l2_clients register 
> themselves, and they are automatically unloaded when the v4l device is 
> unloaded. In addition, the v4l2_device can be registered with a single 
> global list of v4l2_devices.
> 
> It's all pretty simple, but every driver is reinventing this stuff again 
> and again. And the quality of those implementations differs enormously.
> 
> As far as I can see it would be relatively easy to implement both soc 
> and v4l2-int-device on top of v4l2_client: several ops already fall 
> within either core_ops or video_ops, the remainder can initially go 
> into a soc_ops or int_ops. Later these two can be merged into one or 
> more ops structs.

 From your last mail I understood that you found the existing 
implementations of frameworks that offer some kind of abstraction 
between devices that make one V4L device more or less bad. I think we 
should first evaluate what is wrong in the current approaches so we 
don't repeat the mistakes done in them.

v4l2_client approach solves more or less the same problem than 
v4l2-int-device, as far as I understand.

SoC camera is a generic camera driver but it does have its own calling 
convention. So the current convention could be replaced but a generic 
camera driver is still needed.

-- 
Sakari Ailus
sakari.ailus@nokia.com


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

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

[Hans Verkuil]

On Friday 05 September 2008 18:08:43 Sakari Ailus wrote:
> ext Hans Verkuil wrote:
> > I prefer strong typing whereever possible. Basically this solution
> > allows you to subdivide the full range of operations into several
> > interfaces and drivers can choose to implement one or more of those
> > as needed. If a driver doesn't do anything with audio, then there
> > is no need to define the audio interface, so it doesn't take up any
> > space either (except for a single NULL v4l2_client_audio_ops
> > pointer).
>
> How about caller, does the caller need to ensure that a driver
> actually implements a function or how should this be handled?

Easiest would be a something like this:

// Note that client->ops is always non-zero, you would have
// gotten a BUG_ON earlier if it wasn't.
#define v4l2_client_call(client, o, f, args...) \
({ \
        int err = -ENOIOCTLCMD; \
        if (client && client->ops->o && client->ops->o->f) \
                err = client->ops->o->f(client , ##args); \
        err; \
})


struct v4l2_control ctrl = { ... };

int result = v4l2_client_call(client, core, s_ctrl, &ctrl);

Alternatively, if you know which client driver you are accessing you can 
call it directly without checking:

int result = client->ops->core->s_ctrl(client, &ctrl);

There is little advantage in using the second variant so I would prefer 
the first.

There is also a similar define that will call all registered clients 
with the same arguments. Similar to i2c_clients_command but not limited 
to i2c clients. That obviously also checks whether a function is 
present.

> Compared to v4l2-int-if --- if there's no slave, you'll get -ENODEV,
> and in case of missing command, you get -ENOIOCTLCMD. That's not in
> the tree yet but it's part of OMAP 3 camera driver patches.

In what use case can you ever have no slave? I can easily return -ENODEV 
if client == NULL, but isn't the absence of a slave a severe error so 
that you would have bailed out much earlier? Just wondering.

> Commands in v4l2-int-if could be arranged so that sensor commands are
> grouped together, although they would still share the same namespace,
> unlike in v4l2_client.
>
> > And it was indeed my intention to add a union as well to the
> > client_ops where you can put in mutually-exclusive interfaces. And
> > quite possibly a union as well where you can put in driver-specific
> > interfaces. E.g.:
> >
> > // forward references
> > // If you want to use these ops, then you
> > // have to include drv_foo.h first for the full definition
> > struct drv_foo_ops;
> > struct drv_bar_ops;
> >
> > 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;
> > 	int drv_id;
> > 	union {
> > 		const struct drv_foo_ops *foo;
> > 		const struct drv_bar_ops *bar;
> > 	} drv;
> > };
> >
> > Since v4l2_client_ops is completely an internal API it can be
> > changed fairly easily if it turns out that the interfaces need to
> > be rearranged for more optimal use.
>
> Union drv will grow huge over time if the interface becomes widely
> used. And there's a dependency to every driver, too. :I

There no such dependency. The v4l2-client.h only needs to provide a 
single forward struct definition. Only if you actually use that 
drv_foo_ops will you need to include the header containing that 
definition.

In general clients should stick to the main ops structs and only add a 
client specific one if it is really needed. That said, I'm not sure 
whether it is such a bad idea to have client specific ops structs. My 
original idea was to add an ioctl type function to the core ops that 
could be used to make client specific commands. However, with an ops 
struct you have strong typing and a more natural way of calling 
clients. That's actually quite nice.

> >> 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.
> >>
> >> (Please correct me if I'm mistaken somewhere above. And otherwise,
> >> too. :))
> >
> > ISP = In-System Programming?
>
> ISP stands for Image Signal Processor. It's the block in OMAP 3 that
> the sensors interface to and which does some image processing, too,
> e.g. noise filtering and lens shading correction.

Ah, thanks.

> > I definitely agree that an effort should be made to unify these
> > two. It seems to be the logical approach. But what I am even more
> > interested in is to work on embedding all the v4l drivers into a
> > larger framework of code v4l types providing core v4l services.
> >
> > So by using v4l2_client as the basic type for all client drivers
> > (sensors, lens controllers, audio muxes, video encoders/decoders,
> > you name it), it becomes possible to write basic services for that
> > as well: for example my v4l-dvb-ng tree also contains a v4l2_device
> > type which embodies a v4l device instance (e.g. a webcam, a PCI
> > framegrabber, etc). This structure has a list where all the
> > v4l2_clients register themselves, and they are automatically
> > unloaded when the v4l device is unloaded. In addition, the
> > v4l2_device can be registered with a single global list of
> > v4l2_devices.
> >
> > It's all pretty simple, but every driver is reinventing this stuff
> > again and again. And the quality of those implementations differs
> > enormously.
> >
> > As far as I can see it would be relatively easy to implement both
> > soc and v4l2-int-device on top of v4l2_client: several ops already
> > fall within either core_ops or video_ops, the remainder can
> > initially go into a soc_ops or int_ops. Later these two can be
> > merged into one or more ops structs.
>
>  From your last mail I understood that you found the existing
> implementations of frameworks that offer some kind of abstraction
> between devices that make one V4L device more or less bad. I think we
> should first evaluate what is wrong in the current approaches so we
> don't repeat the mistakes done in them.

The basic idea is the same in all approaches. However, see the following 
shortcomings in the existing implementations:

1) They are not generic, but built to solve a specific problem. To be 
specific: soc-camera.h and v4l2-int-device.h were written for sensors. 
By contrast, the ioctl-type interface used in most other i2c drivers 
(e.g. saa7115, msp3400, etc) is generic.

2) Using 'ops'-like structs is the preferred way of doing such things 
inside the kernel. It is fast, the ops structs themselves can be setup 
as static const and initialized at compile time, and you have strong 
typing. Of the three only soc-camera uses this.

3) Independent of the whatever bus is used by the client driver. This is 
true for soc-camera and v4l2-int-device, but not for the ioctl 
interface used by all other i2c drivers. That relies on the i2c core.

4) Having a single v4l2_client struct as the gateway for all client 
drivers makes it much easier to create supporting functions in the 
v4l2-core that will take over some of the boring bits. For example, 
something like G_CHIP_IDENT can be implemented by a generic 
v4l2_client_g_chip_ident function and by adding a chip_ident and 
chip_rev field to the v4l2_client struct. That function can do all the 
needed checks for you. The same is true for control handling. All 
drivers whether i2c or pci/usb basically need to implement the same 
checks. That can be done much smarter, but it makes only sense if all 
drivers use the same building blocks. I have more ideas, but this gives 
you an idea.

> v4l2_client approach solves more or less the same problem than
> v4l2-int-device, as far as I understand.
>
> SoC camera is a generic camera driver but it does have its own
> calling convention. So the current convention could be replaced but a
> generic camera driver is still needed.

Obviously. It's the calling convention that I'm looking at. Among 
others, since this is just part of what I want to do.

Regards,

	Hans

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

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

[Guennadi Liakhovetski]

Hi Hans,

On Thu, 28 Aug 2008, Hans Verkuil wrote:

> 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.

[...]

Finally I took a couple of minutes and looked at your v4l-dvb-ng tree - I 
really did:-) Do I understand it right, that you currently only have some 
infrastructure in place (v4l2-client.h, v4l2-client.c, v4l2-common.c) and 
some client drivers like saa717x.c, but no "host" (USB or PCI or SoC) 
drivers and no APIs for host / client communication? And it is exactly 
this part that makes this generalisation so interesting. How you let the 
host driver specify what interface it has to the client (slave serial, or 
master parallel, 8 or 10 bits...) and what data (pixel) formats it is 
prepared to accept. Like:

Host: I have a XYZ-compatible device at i2c-address II
Client: XYZ-device version ABC detected at i2c-address II
User: need format U
Client: device is capabale of formats O, P, Q over bus types L, M, N
Host: suggest you send me format O using bus type L, I convert it to U

Or even

Core: Host: enumerate capabilities
Core: Client: enumerate capabilities
Core: Host: prepare pipe-X converting O on L to U
Core: Client: start sending O over L

(note, I'm not saying soc-camera can handle all these negotiation 
perfectly as it stands now). I like your struct v4l2_client_ops, and I 
think it is not very different from what soc-camera is doing in this 
regard at the moment - but only the video part. So, it should be possible 
to convert the soc-camera interface to your v4l2_client. Then, I think, we 
could think about the host-client negotiation and maybe design it in a way 
similar to what soc-camera is doing ATM?

Note, I am away the whole next week, and quite possibly will not have 
proper internet connection, only back on 9 October.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

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

Interfaces for composite devices (was: Re: [PATCH v2] soc-camera: add API documentation)

[Sakari Ailus]

Hello, Hans!

I'm Cc:ing to Sergio Aguirre and Hari Nagalla as they are working with 
the OMAP 3 ISP and camera drivers at TI now. I'm also removing Mohit.

ext Hans Verkuil wrote:
> On Friday 05 September 2008 18:08:43 Sakari Ailus wrote:
>> How about caller, does the caller need to ensure that a driver
>> actually implements a function or how should this be handled?
> 
> Easiest would be a something like this:
> 
> // Note that client->ops is always non-zero, you would have
> // gotten a BUG_ON earlier if it wasn't.
> #define v4l2_client_call(client, o, f, args...) \
> ({ \
>         int err = -ENOIOCTLCMD; \
>         if (client && client->ops->o && client->ops->o->f) \
>                 err = client->ops->o->f(client , ##args); \
>         err; \
> })
> 
> 
> struct v4l2_control ctrl = { ... };
> 
> int result = v4l2_client_call(client, core, s_ctrl, &ctrl);
> 
> Alternatively, if you know which client driver you are accessing you can 
> call it directly without checking:
> 
> int result = client->ops->core->s_ctrl(client, &ctrl);
> 
> There is little advantage in using the second variant so I would prefer 
> the first.

I agree. Usually you don't want to know anything useless about the client.

> There is also a similar define that will call all registered clients 
> with the same arguments. Similar to i2c_clients_command but not limited 
> to i2c clients. That obviously also checks whether a function is 
> present.

I hope this would still be limited to a single v4l device, right? :)

That would be actually needed in the OMAP 3 camera as it doesn't want to
know which client implements a given control, for example.

>> Compared to v4l2-int-if --- if there's no slave, you'll get -ENODEV,
>> and in case of missing command, you get -ENOIOCTLCMD. That's not in
>> the tree yet but it's part of OMAP 3 camera driver patches.
> 
> In what use case can you ever have no slave? I can easily return -ENODEV 
> if client == NULL, but isn't the absence of a slave a severe error so 
> that you would have bailed out much earlier? Just wondering.

There's a valid use case for that.

The OMAP 3 camera driver can drive a lenses and a flash, too. But lens
or flash might not be part of every camera device. For example a
secondary camera in a device could be without these.

> In general clients should stick to the main ops structs and only add a 
> client specific one if it is really needed. That said, I'm not sure 
> whether it is such a bad idea to have client specific ops structs. My 
> original idea was to add an ioctl type function to the core ops that 
> could be used to make client specific commands. However, with an ops 
> struct you have strong typing and a more natural way of calling 
> clients. That's actually quite nice.

I would prefer the ioctl approach at least in some cases. Why not to
allow both?

Just as an example, consider the OMAP 3 camera again. A slave can quite
well implement a private interface that it wants to export to userspace,
like the OMAP 3 ISP driver does have ioctls for configuring the
statistics collection and for obtaining the statistics. The camera
driver does not need nor want to be involved with this. The statistics 
are used by the automatic white balance, exposure and focus algorithms.

I don't recognise need for private ops at the moment that would be
useful to slave. If the master driver is supposed to be generic it
really should not use directly any private operations.

Dependencies between slaves can be handled in the board code instead. In 
that case the dependencies are not a problem because they already are 
inherent to that machine.

>>  From your last mail I understood that you found the existing
>> implementations of frameworks that offer some kind of abstraction
>> between devices that make one V4L device more or less bad. I think we
>> should first evaluate what is wrong in the current approaches so we
>> don't repeat the mistakes done in them.
> 
> The basic idea is the same in all approaches. However, see the following 
> shortcomings in the existing implementations:
> 
> 1) They are not generic, but built to solve a specific problem. To be 
> specific: soc-camera.h and v4l2-int-device.h were written for sensors. 

Not quite; v4l2-int-device is supposed to work with any other kind of
devices as well. There are no dependencies to sensors.

But I agree that different types of slaves are not differentiated so
this could cause problems. The other thing is that during about one year 
no-one else has started to use this interface except the OMAP 1 camera 
driver and one sensor driver that is used with the first.

If we go with v4l2_client then I suppose we should forget about
v4l2-int-if and maybe SoC camera also and convert all the drivers to use
v4l2_client instead of unifying the first two. Instead if something
would be done to old code that would be to convert it to use v4l2_client
instead. That task probably would not be very big as the concepts are
quite similar; just the implementation differs.

(I still should take a closer look at the v4l2_client, sorry.)

Sincerely,

-- 
Sakari Ailus
sakari.ailus@nokia.com

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

Re: Interfaces for composite devices (was: Re: [PATCH v2] soc-camera: add API documentation)

[Hans Verkuil]

Hi Sakari!

On Monday 29 September 2008 10:33:36 Sakari Ailus wrote:
> Hello, Hans!
>
> I'm Cc:ing to Sergio Aguirre and Hari Nagalla as they are working
> with the OMAP 3 ISP and camera drivers at TI now. I'm also removing
> Mohit.
>
> ext Hans Verkuil wrote:
> > On Friday 05 September 2008 18:08:43 Sakari Ailus wrote:
> >> How about caller, does the caller need to ensure that a driver
> >> actually implements a function or how should this be handled?
> >
> > Easiest would be a something like this:
> >
> > // Note that client->ops is always non-zero, you would have
> > // gotten a BUG_ON earlier if it wasn't.
> > #define v4l2_client_call(client, o, f, args...) \
> > ({ \
> >         int err = -ENOIOCTLCMD; \
> >         if (client && client->ops->o && client->ops->o->f) \
> >                 err = client->ops->o->f(client , ##args); \
> >         err; \
> > })
> >
> >
> > struct v4l2_control ctrl = { ... };
> >
> > int result = v4l2_client_call(client, core, s_ctrl, &ctrl);
> >
> > Alternatively, if you know which client driver you are accessing
> > you can call it directly without checking:
> >
> > int result = client->ops->core->s_ctrl(client, &ctrl);
> >
> > There is little advantage in using the second variant so I would
> > prefer the first.
>
> I agree. Usually you don't want to know anything useless about the
> client.
>
> > There is also a similar define that will call all registered
> > clients with the same arguments. Similar to i2c_clients_command but
> > not limited to i2c clients. That obviously also checks whether a
> > function is present.
>
> I hope this would still be limited to a single v4l device, right? :)

Obviously :-)

> That would be actually needed in the OMAP 3 camera as it doesn't want
> to know which client implements a given control, for example.

Right. During the LPC conference the suggestion was made to allow the 
bridge driver to assign a group ID to clients and use that group ID to 
call only clients belonging to that group. It allows the bridge driver 
finer-grained control over which clients should be called. In the 
current proposal there is only a choice between calling one or all 
clients, having a group ID fixes this limitation. I will implement 
this.

> >> Compared to v4l2-int-if --- if there's no slave, you'll get
> >> -ENODEV, and in case of missing command, you get -ENOIOCTLCMD.
> >> That's not in the tree yet but it's part of OMAP 3 camera driver
> >> patches.
> >
> > In what use case can you ever have no slave? I can easily return
> > -ENODEV if client == NULL, but isn't the absence of a slave a
> > severe error so that you would have bailed out much earlier? Just
> > wondering.
>
> There's a valid use case for that.
>
> The OMAP 3 camera driver can drive a lenses and a flash, too. But
> lens or flash might not be part of every camera device. For example a
> secondary camera in a device could be without these.

I think I would prefer it if the bridge driver does the test against 
client == NULL. But I will have to think about this a bit more.

> > In general clients should stick to the main ops structs and only
> > add a client specific one if it is really needed. That said, I'm
> > not sure whether it is such a bad idea to have client specific ops
> > structs. My original idea was to add an ioctl type function to the
> > core ops that could be used to make client specific commands.
> > However, with an ops struct you have strong typing and a more
> > natural way of calling clients. That's actually quite nice.
>
> I would prefer the ioctl approach at least in some cases. Why not to
> allow both?
>
> Just as an example, consider the OMAP 3 camera again. A slave can
> quite well implement a private interface that it wants to export to
> userspace, like the OMAP 3 ISP driver does have ioctls for
> configuring the statistics collection and for obtaining the
> statistics. The camera driver does not need nor want to be involved
> with this. The statistics are used by the automatic white balance,
> exposure and focus algorithms.

Good point. I'll keep the ioctl for this purpose.

> I don't recognise need for private ops at the moment that would be
> useful to slave. If the master driver is supposed to be generic it
> really should not use directly any private operations.
>
> Dependencies between slaves can be handled in the board code instead.
> In that case the dependencies are not a problem because they already
> are inherent to that machine.
>
> >>  From your last mail I understood that you found the existing
> >> implementations of frameworks that offer some kind of abstraction
> >> between devices that make one V4L device more or less bad. I think
> >> we should first evaluate what is wrong in the current approaches
> >> so we don't repeat the mistakes done in them.
> >
> > The basic idea is the same in all approaches. However, see the
> > following shortcomings in the existing implementations:
> >
> > 1) They are not generic, but built to solve a specific problem. To
> > be specific: soc-camera.h and v4l2-int-device.h were written for
> > sensors.
>
> Not quite; v4l2-int-device is supposed to work with any other kind of
> devices as well. There are no dependencies to sensors.

You are correct.

> But I agree that different types of slaves are not differentiated so
> this could cause problems. The other thing is that during about one
> year no-one else has started to use this interface except the OMAP 1
> camera driver and one sensor driver that is used with the first.
>
> If we go with v4l2_client then I suppose we should forget about
> v4l2-int-if and maybe SoC camera also and convert all the drivers to
> use v4l2_client instead of unifying the first two. Instead if
> something would be done to old code that would be to convert it to
> use v4l2_client instead. That task probably would not be very big as
> the concepts are quite similar; just the implementation differs.
>
> (I still should take a closer look at the v4l2_client, sorry.)
>
> Sincerely,

During the LPC conference everyone liked the ideas behind v4l2_client 
and v4l2_device and the decision was made that I'll go ahead with it. 
It will be renamed to media_client since the same approach can also be 
used in the dvb subsystem in the future, so it is not v4l specific 
(although it will be initially).

I hope to get most of it into v4l-dvb in the next month. Once that's 
done it shouldn't be difficult to replace v4l2-int-if with media_client 
and to modify soc-camera to start using it as well. That should unify 
all these internal interfaces and simplify future drivers.

Regards,

	Hans

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

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

[Hans Verkuil]

On Sunday 28 September 2008 00:38:23 Guennadi Liakhovetski wrote:
> Hi Hans,
>
> On Thu, 28 Aug 2008, Hans Verkuil wrote:
> > 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.
>
> [...]
>
> Finally I took a couple of minutes and looked at your v4l-dvb-ng tree
> - I really did:-) Do I understand it right, that you currently only
> have some infrastructure in place (v4l2-client.h, v4l2-client.c,
> v4l2-common.c) and some client drivers like saa717x.c, but no "host"
> (USB or PCI or SoC) drivers and no APIs for host / client
> communication? And it is exactly this part that makes this
> generalisation so interesting. How you let the host driver specify
> what interface it has to the client (slave serial, or master
> parallel, 8 or 10 bits...) and what data (pixel) formats it is
> prepared to accept. Like:
>
> Host: I have a XYZ-compatible device at i2c-address II
> Client: XYZ-device version ABC detected at i2c-address II
> User: need format U
> Client: device is capabale of formats O, P, Q over bus types L, M, N
> Host: suggest you send me format O using bus type L, I convert it to
> U
>
> Or even
>
> Core: Host: enumerate capabilities
> Core: Client: enumerate capabilities
> Core: Host: prepare pipe-X converting O on L to U
> Core: Client: start sending O over L
>
> (note, I'm not saying soc-camera can handle all these negotiation
> perfectly as it stands now). I like your struct v4l2_client_ops, and
> I think it is not very different from what soc-camera is doing in
> this regard at the moment - but only the video part. So, it should be
> possible to convert the soc-camera interface to your v4l2_client.
> Then, I think, we could think about the host-client negotiation and
> maybe design it in a way similar to what soc-camera is doing ATM?
>
> Note, I am away the whole next week, and quite possibly will not have
> proper internet connection, only back on 9 October.

My current prototype implementation uses ivtv as its test driver (since 
I maintain ivtv that was the logical choice for me).

Since ivtv is an MPEG encoder driver there are obviously no sensor 
ioctls implemented. But when v4l2_client is used for sensor drivers I 
expect that a 'const struct v4l2_client_sensor_ops *sensor;' field will 
be added that contains the sensor-specific ops. Remember that it is an 
internal API so it is easy to modify/extend these ops.

v4l2_client/v4l2_device are meant to provide a consistent framework of 
working with client drivers. Almost all v4l2 drivers need this and very 
few (if any) drivers do this correctly. The simple act of unloading a 
client driver while the bridge driver is still loaded is usually enough 
to at best prevent the device from working correctly and at worst to 
create an oops. In addition upcoming changes to the i2c core will make 
this even harder so it becomes essential to create a unified framework 
for this so that you can create utility functions that do the hard part 
of loading and registering client drivers.

This close dependency between the bridge and client drivers is pretty 
much unique for v4l2 and the lack of proper support for this meant that 
most drivers do it wrong. Most people do not realize this since it for 
normal usage it works fine, but as soon as you start messing around 
with rmmod you will run into problems. Combine that with the fact that 
i2c modules can be delayed-loaded in the future and you really need 
better support in the v4l2 framework since that's a non-trivial problem 
to solve.

Note that during the LPC conference it was decided that I can go ahead 
with implementing this, so you can expect to see media_client and 
media_device structs appearing in v4l-dvb during the next month.

Regards,

	Hans

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

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

[Guennadi Liakhovetski]

On Mon, 29 Sep 2008, Hans Verkuil wrote:

> > Core: Host: enumerate capabilities
> > Core: Client: enumerate capabilities
> > Core: Host: prepare pipe-X converting O on L to U
> > Core: Client: start sending O over L
> >
> > (note, I'm not saying soc-camera can handle all these negotiation
> > perfectly as it stands now). I like your struct v4l2_client_ops, and
> > I think it is not very different from what soc-camera is doing in
> > this regard at the moment - but only the video part. So, it should be
> > possible to convert the soc-camera interface to your v4l2_client.
> > Then, I think, we could think about the host-client negotiation and
> > maybe design it in a way similar to what soc-camera is doing ATM?
> >
> > Note, I am away the whole next week, and quite possibly will not have
> > proper internet connection, only back on 9 October.
> 
> My current prototype implementation uses ivtv as its test driver (since 
> I maintain ivtv that was the logical choice for me).
> 
> Since ivtv is an MPEG encoder driver there are obviously no sensor 
> ioctls implemented. But when v4l2_client is used for sensor drivers I 
> expect that a 'const struct v4l2_client_sensor_ops *sensor;' field will 
> be added that contains the sensor-specific ops. Remember that it is an 
> internal API so it is easy to modify/extend these ops.
> 
> v4l2_client/v4l2_device are meant to provide a consistent framework of 
> working with client drivers. Almost all v4l2 drivers need this and very 
> few (if any) drivers do this correctly. The simple act of unloading a 
> client driver while the bridge driver is still loaded is usually enough 
> to at best prevent the device from working correctly and at worst to 
> create an oops. In addition upcoming changes to the i2c core will make 
> this even harder so it becomes essential to create a unified framework 
> for this so that you can create utility functions that do the hard part 
> of loading and registering client drivers.
> 
> This close dependency between the bridge and client drivers is pretty 
> much unique for v4l2 and the lack of proper support for this meant that 
> most drivers do it wrong. Most people do not realize this since it for 
> normal usage it works fine, but as soon as you start messing around 
> with rmmod you will run into problems. Combine that with the fact that 
> i2c modules can be delayed-loaded in the future and you really need 
> better support in the v4l2 framework since that's a non-trivial problem 
> to solve.

FWIW, with soc-camera all loading and unloading has been tested and worked 
- at least at the time I was developing it. The central module allowes 
arbitrary loading order, and modules use-count is increased as long as 
components are in use.

> Note that during the LPC conference it was decided that I can go ahead 
> with implementing this, so you can expect to see media_client and 
> media_device structs appearing in v4l-dvb during the next month.

Good, looking forward to it! Just, please, make sure to think about 
negotiation issues I mentioned before. I think, we want some standard API 
for those too, at least as much as possible.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

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