[ANN] Agenda for the Cambourne meeting

[ANN] Agenda for the Cambourne meeting

[Laurent Pinchart]

Agenda for V4L2 brainstorm meeting in Cambourne, August 1-5 2011
----------------------------------------------------------------

The next V4L2 brainstorming meeting will take place in Cambourne, UK on August 
1-5 2011. The meeting will be colocated with the Linaro Sprint.

*Reminder*: this is not a summit meeting, conclusions will need to be 
discussed and approved on the mailing list.

The schedule will be slightly different compared to our previous meetings. To 
avoid overlaps with the Linaro Graphics Summit, the V4L2 brainstorming 
sessions will be held on Monday morning, Tuesday morning, Wednesday morning, 
Thursday all day and Friday morning.

As the number of topics to be discussed isn't too large, we will spend the 
first morning to go through all the agenda points and make sure everyone 
understands the problems. If time permits, we will start discussing smaller 
issues. Those who have a vested interest in an agenda item should be prepared 
to explain their take on it and if necessary have a presentation ready. 
Presentations should not take more than half an hour.

On Tuesday, Wednesday and Thursday we will discuss issues in details and try 
to come up with solutions.

On Friday morning we will translate all agenda items into actions.

This approach worked well in the past and it ensures that we end up with
something concrete.

*Attendees*

Cisco Systems Norway:
  Hans Verkuil (Chair) <hverkuil@xs4all.nl>

Ideas On Board:
  Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Intel:
  Tuukka Toivonen <tuukka.toivonen@intel.com>

Nokia:
  Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>

Samsung Poland R&D Center:
  Sylwester Nawrocki <s.nawrocki@samsung.com>
  Tomasz Stanislawski <t.stanislaws@samsung.com>

*Agenda*

To minimize wasted time, please make sure you refresh your memory on every 
topic by (re-)reading RFCs mentioned below.

1) Pipeline configuration, cropping and scaling. This topic has already been
   discussed during the Warsaw meeting in March 2011, but requires more
   brainstorming. (Everyone)

   http://www.mail-archive.com/linux-media@vger.kernel.org/msg26630.html
   http://www.mail-archive.com/linux-media@vger.kernel.org/msg27956.html
   http://www.mail-archive.com/linux-media@vger.kernel.org/msg28490.html
   http://www.mail-archive.com/linux-media@vger.kernel.org/msg34410.html

   Sub-topics that needs to be discussed include

   - Sensor blanking/pixel-clock/frame-rate settings (including enumeration
     and/or discovery)

   - Sub-subdevs and/or subdev hierarchy (Sakari)

   - Binning on sensors (Sakari)

2) Per-frame configuration. This topic has already been discussed during the
   Warsaw meeting in MArch 2011, but requires more brainstorming. (Everyone)

   - Synchronising parameters (e.g. exposure time and gain) on given
     frames. Some sensors support this on hardware. There are many use cases
     which benefit from this, for example this one:

     http://fcam.garage.maemo.org/

   - Frame metadata. It is important for the control algorithms (exposure,
     white balance, for example), to know which sensor settings have been
     used to expose a given frame. Many sensors do support this. Do we want
     to parse this in the kernel or does it belong to user space? The
     metadata formats are mostly sensor dependent.

3) Fast buffer allocation and mapping. (Tuukka)

   http://www.mail-archive.com/linux-media@vger.kernel.org/msg34661.html
   (possibly related)

   Tuukka, could you please elaborate a bit on what you would like to discuss
   exactly ?

4) Media devices, V4L2 subdevs and Linux device model. (Sakari)

     http://www.mail-archive.com/linux-media@vger.kernel.org/msg21831.html
     http://www.mail-archive.com/linux-media@vger.kernel.org/msg34294.html


The first item is the biggest one. We should discuss it on Tuesday morning, 
and continue discussions if time permits on Thursday afternoon. Wednesday 
morning will be spent discussing item 2. Item 3 will be pushed to Thursday 
morning, as it will likely be influenced by the result of the Linaro Graphics 
Summit. Item 4 should follow and be extended to Thursday afternoon.

If time permits other small items can be discussed on Thursday. Please list 
any other topic you would like to address during the meeting in response to 
this e-mail, with links to related documentation and/or RFCs if applicable.

-- 
Regards,

Laurent Pinchart

Re: [ANN] Agenda for the Cambourne meeting

[Laurent Pinchart]

On Tuesday 26 July 2011 16:47:18 Laurent Pinchart wrote:
> Agenda for V4L2 brainstorm meeting in Cambourne, August 1-5 2011
> ----------------------------------------------------------------

[snip]

> *Attendees*
> 
> Cisco Systems Norway:
>   Hans Verkuil (Chair) <hverkuil@xs4all.nl>

Copy and paste can be bad. I'm the meeting's chair this time. Sorry Hans for 
worrying you :-)

> Ideas On Board:
>   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Intel:
>   Tuukka Toivonen <tuukka.toivonen@intel.com>
> 
> Nokia:
>   Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> 
> Samsung Poland R&D Center:
>   Sylwester Nawrocki <s.nawrocki@samsung.com>
>   Tomasz Stanislawski <t.stanislaws@samsung.com>

-- 
Regards,

Laurent Pinchart

[ANN] Meeting minutes of the Cambourne meeting

[Laurent Pinchart]

Hi everybody,

The V4L2 brainstorming meeting held in Cambourne from August the 1st to August 
the 5th was a success. I would like to thank Linaro again, and particularly 
Stephen Doel and Arwen Donaghey, for accommodating us during the whole week.

Here is a summary of the discussions, with preliminary conclusions, ideas, and 
action points.

I encourage all attendants to make their notes available (yes, that might 
require cleaning them up :-)).


Pipeline configuration
----------------------

 Preliminary conclusions:

 - SUBDEV_ENUM_FRAME_SIZE should enumerate frame sizes that can be obtained
   through binning/skipping, limited to "common sizes".
 - Binning must always be preferred over skipping.
 - SUBDEV_S_FMT selects the frame size at pads, and thus configures scaling.
 - When sensors support cropping on both the pixel array and on the output,
   SUBDEV_S_SELECTION with a new "pixel array" target could be used. Whether
   this is needed isn't known.
 - SUBDEV_G_SELECTION is used to get the pixel array size.
 - When entities include controls that require cropping for internal reasons,
   the recommended behaviour is for the driver to transparently add cropping
   when the control is disabled, in order to allow userspace to enable/disable
   the control at runtime.
 - Cropping and flipping may change format for Bayer sensors. This is allowed,
   but must fail with -EBUSY when streaming.

 Actions:

 - (Pre-requisite) video device node G/S_SELECTION API. (Tomasz)
 - Specify the SUBDEV_G/S_SELECTION ioctl on subdev pads. (Laurent, Sakari)
 - Add a "keep-pipeline" flag to SUBDEV_S_SELECTION. (Laurent, Sakari)
 - Deprecated the SUBDEV_G/S_CROP ioctl on subdev pads. (Laurent, Sakari)
 - Document the pipeline setup behaviour: (Laurent, Sakari)
   - From sink pad to source pad inside subdevs.
   - From device to memory between subdevs (but controlled by userspace, so
     that's not mandatory).


Pixel clock and blanking
------------------------

 Preliminary conclusions:

 - Pixel clock(s) and blanking will be exported through controls on subdev
   nodes.
 - The pixel array pixel clock is needed by userspace.
 - Hosts/bridges/ISPs need pixel clock and blanking information to validate
   pipelines.

 Actions:

 - CSI2 and CCP2 bus frequencies will be selectable use integer menu controls.
   (Sakari)
 - Add an integer menu control type, replacing the name with a 64-bit integer.
   (Sakari, Hans)
 - Research which pixel clock(s) to expose based on the SMIA sensor.
   (Sakari)
 - Add two new internal subdev pad operations to get and set clocks and
   blanking.
   (Laurent, Sakari)


Per-frame configuration
-----------------------

 Use cases:

 - Controls:
   - Exposure/gain/focus bracketing
   - Flash + exposure
 - Output devices:
   - Switching buffer format on the fly on output devices (TI OMAP3 DSS)
   - Codecs support (forcing I-frames, changing quantization parameters)
   - Smooth digital composing
 - Capture devices:
   - Smooth digital zoom

 Preliminary conclusions:

 - ioctls related to per-frame configuration are limited to S_CTRL, S_FMT and
   S_SELECTION.
 - We should not emulate per-frame configuration in kernel drivers for
   hardware that don't support it.

 Preliminary ideas (no consensus):

 - Configuring sensors for exposure/gain/focus bracketing can be done through
   a sensor-specific ioctl on subdev nodes.
 - Per-buffer configuration is easier than per-frame configuration:
   - No need to keep a configuration queue in the driver.
   - It can be difficult for drivers to determine which frame comes up when.
 - Add *_PER_FRAME ioctls (based on the frame sequence number).
 - Use a configuration plane with the multi-plane API.

 Actions:

 - Think about it.


Per-frame meta-data
-------------------

 Preliminary conclusions:

 - Don't add software-based metadata in the kernel. Only export metadata
   generated by the hardware. Pre-processing hardware-generated metadata in
   the kernel is allowed.
 - If we can't find a better API, add a "flash" buffer flag to indicate that
   the buffer has been exposed to flash.
 - Drivers shouldn't parse meta-data. libv4l should parse it.

 Possible implementations:

 - Use a meta-data video device.
 - Use a meta-data plane with the multi-plane API.
 - Use a per-frame control API.

 Actions:

 - Think about it.


Shared buffers
--------------

 Actions:

 - Add support for the shared buffers proof-of-concept API to fbdev and vivi.
   (Tomasz)


Subdevs hierachy, Linux device model
------------------------------------

 Preliminary conclusions:

 - With the move to device tree on ARM (and other platforms), I2C, SPI and
   platform subdevs should be created from board code, not from bridge/host
   drivers.
 - Bus notifiers should be used by bridge/host drivers to wait for all
   required subdevs. V4L2 core should provide helper functions.
 - struct clk should be used to handle clocks provided by hosts to subdevs.

 Actions:

 - Work on a proof-of-concept implementation of the new subdevs registration
   mechanism and send an RFC (whoever needs it first).
 - Work on a proof-of-concept clock handling using struct clk with the OMAP3
   ISP driver (Laurent).


-- 
Regards,

Laurent Pinchart

Re: [ANN] Meeting minutes of the Cambourne meeting

[Subash Patel]

Hi Sakari,

I have a point with the pixel clock. During discussion we found that 
pixel clock get/set is required for user space to do fine control over 
the frame-rate etc. What if the user sets the pixel array clock which is 
above the system/if bus clock? Suppose we are setting the pixel clock 
(which user space sets) to higher rate at sensor array, but for some 
reason the bus cannot handle that rate (either low speed or loaded) or 
lower PCLK at say CSI2 interface is being set. Are we not going to loose 
data due to this? Also, there would be data validation overhead in 
driver on what is acceptable PCLK values for a particular sensor on an 
interface etc.

I am still not favoring user space controlling this, and wish driver 
decides this for a given frame-rate requested by the user space :)

Frame-rate   resolution  HSYNC  VSYNC  PCLK(array)  PCLK (i/f bus) ...

Let user space control only first two, and driver decide rest (PCLK can 
be different at different ISP h/w units though)

Regards,
Subash

 > Pixel clock and blanking
 > ------------------------
 >
 >   Preliminary conclusions:
 >
 >   - Pixel clock(s) and blanking will be exported through controls on 
subdev
 >     nodes.
 >   - The pixel array pixel clock is needed by userspace.
 >   - Hosts/bridges/ISPs need pixel clock and blanking information to 
validate
 >     pipelines.
 >
 >   Actions:
 >
 >   - CSI2 and CCP2 bus frequencies will be selectable use integer menu 
controls.
 >     (Sakari)
 >   - Add an integer menu control type, replacing the name with a 
64-bit integer.
 >     (Sakari, Hans)
 >   - Research which pixel clock(s) to expose based on the SMIA sensor.
 >     (Sakari)
 >   - Add two new internal subdev pad operations to get and set clocks and
 >     blanking.
 >     (Laurent, Sakari)

Re: [ANN] Meeting minutes of the Cambourne meeting

[Sakari Ailus]

Subash Patel wrote:
> Hi Sakari,

Hi Subash,

> I have a point with the pixel clock. During discussion we found that
> pixel clock get/set is required for user space to do fine control over
> the frame-rate etc. What if the user sets the pixel array clock which is
> above the system/if bus clock? Suppose we are setting the pixel clock

The pixel array clock should be calculated by the driver based on CSI-2
bus frequency (user specified), lanes (from board data), binning,
skipping and crop. This is since there are typically limitations for its
value, and the sensor driver can come up with a "best value" for it,
based on the information above. Exactly how, that depends on the sensor
driver and the sensor.

So the pixel array clock is read-only for the user, and the frame rate
can then be chosen using the blanking configuration.

> (which user space sets) to higher rate at sensor array, but for some
> reason the bus cannot handle that rate (either low speed or loaded) or
> lower PCLK at say CSI2 interface is being set. Are we not going to loose
> data due to this? Also, there would be data validation overhead in
> driver on what is acceptable PCLK values for a particular sensor on an
> interface etc.

This is something that must be handled independently of the way the
sensor pixel clock is configured. Typically the limitation is on either
the bus frequency or the pixel rate on the bus.

This actually can be better avoided when the user has a chance to choose
the bus frequency explicitly rather than receive just something the
driver happens to produce based on frame rate and resolution settings.

> I am still not favoring user space controlling this, and wish driver
> decides this for a given frame-rate requested by the user space :)
> 
> Frame-rate   resolution  HSYNC  VSYNC  PCLK(array)  PCLK (i/f bus) ...

You can still do that, but it comes with limitations. Any fixed set of
the above parameters is very hardware and use case dependent.

> Let user space control only first two, and driver decide rest (PCLK can
> be different at different ISP h/w units though)

I'm definitely not against this. We do have drivers which use this kind
of interface already and some vendors do not even provide enough
information to write a driver for their sensor offering any other kind
of interface.

Cheers,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [ANN] Meeting minutes of the Cambourne meeting

[Sakari Ailus]

nitesh moundekar wrote:
> Hi all,

Hi Nitesh,

> I am worried about direction v4l2 is taking. It looks against the basic
> principle of driver i.e. hardware abstraction. So i think giving out pixel
> clock, binning, skipping, bayer pattern, etc device varying features to user
> space questionable. We can try to remain generic and proprietary or internal
> device information can be exposed at subdev level or via sysfs.

Welcome to the world of embedded devices...

What this would provide you is a way to configure sensors in a generic
way at low level without enforcing policies or putting artificial
limitations in place while being able to better gain information on the
capabilities of the devices in user space.

This level of control is essential when implementing digital cameras, be
they high end or low end in terms of hardware. If you're not doing that,
then this interface might not be relevant to you.

Also, this is not meant by any means to replace existing interfaces used
by applications.

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [ANN] Meeting minutes of the Cambourne meeting

[Sakari Ailus]

nitesh moundekar wrote:
> Hi Sakari,

Hi Nitesh,

> So without touching these controls, drivers should be able to work with
> default or internal settings calculated from frame rate and resolution. And
> when application like DSLR wants more control it can access those controls.

The current interface is provided from V4L2 subdevs, so the application
using that can be expected to know something of the system already.

It may be up to drivers to decide what do they implement and what they
do not. We'll have to see how generic the new way of configuring the
sensors is; my hope is that practically all raw bayer sensors (not the
SoC ones!) could be configured this way. Lack of information from
manufacturer could limit the ability to write such drivers for sensors,
though.

With such a system, an user space algorithm library (a plugin for
libv4l) will be needed in any case to come up with a fully functional
system useful for generic applications, and it may well be that this
interface will be used from the plugins.

Alternatively the old interface could be implemented using a wrapper
library for all drivers providing the new sensor configuration
interface. There would be a list of default modes, some of which could
be more board dependent than others.

Regards,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [ANN] Meeting minutes of the Cambourne meeting

[Tuukka Toivonen]

On Tuesday, August 09, 2011 06:36:19 pm nitesh moundekar wrote:
> I am worried about direction v4l2 is taking. It looks against the basic
> principle of driver i.e. hardware abstraction. 

There definitely should be an API which is hardware independent.
However, the problem is that the hardware is so flexible that an
hardware independent API needs to be necessarily enforce policies
and possibly do complex image processing which is against the kernel
philosophy and belongs to userspace.

As I see it, it is necessary to provide more or less HW-dependent
API to kernel V4L2. What we need is an userspace library abstracting
that to simple HW-independent API. I think that libv4l is a very
good way to do that, since it still provides the V4L2 interface
but can do things in userspace.

- Tuukka
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Sakari Ailus]

On Mon, Aug 08, 2011 at 05:50:06PM +0200, Laurent Pinchart wrote:
> Hi everybody,

Hi, all!

> The V4L2 brainstorming meeting held in Cambourne from August the 1st to August 
> the 5th was a success. I would like to thank Linaro again, and particularly 
> Stephen Doel and Arwen Donaghey, for accommodating us during the whole week.
> 
> Here is a summary of the discussions, with preliminary conclusions, ideas, and 
> action points.

Thanks for the notes!

...

> Pixel clock and blanking
> ------------------------
> 
>  Preliminary conclusions:
> 
>  - Pixel clock(s) and blanking will be exported through controls on subdev
>    nodes.
>  - The pixel array pixel clock is needed by userspace.
>  - Hosts/bridges/ISPs need pixel clock and blanking information to validate
>    pipelines.

I have a small addition to this in my notes:


Pixel array and bus configuration for sensors
---------------------------------------------

The CSI-2 bus frequency will receive an integer menu control. Together with
the binning, skipping, scaling and CSI-2 output bits-per-pixel information,
this allows the sensor driver to calculate the value of the "best pixel
rate" in the sensor, which will be a read-only int64 control.

Based on pixel clock, image width, height and ranges on vertical and
horizontal blanking, the user can define the frame rate. Vertical and
horizontal blanking are implemented as integer controls.

Integer menu controls are easy to add; this will be implemented by making
the name field in v4l2_querymenu an anonymous union. (I actually have
patches for this but haven't tested them yet. I'll send them once I have
time for that.)


Cheers,

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [ANN] Meeting minutes of the Cambourne meeting

[Sylwester Nawrocki]

Hi Laurent,

On 08/08/2011 05:50 PM, Laurent Pinchart wrote:
> Subdevs hierachy, Linux device model
> ------------------------------------
> 
>   Preliminary conclusions:
> 
>   - With the move to device tree on ARM (and other platforms), I2C, SPI and
>     platform subdevs should be created from board code, not from bridge/host
>     drivers.
>   - Bus notifiers should be used by bridge/host drivers to wait for all
>     required subdevs. V4L2 core should provide helper functions.
>   - struct clk should be used to handle clocks provided by hosts to subdevs.

I have been investigating recently possible ways to correct the external clock
handling in Samsung FIMC driver and this led me up to the device tree stuff. 
I.e. in order to be able to register any I2C client device there is a need
to enable its master clock at the v4l2 host/bridge driver.

There is an issue that the v4l2_device (host)/v4l2_subdev hierarchy is not
reflected by the linux device tree model, e.g. the host might be a platform
device while the client an I2C client device. Thus a proper device/driver 
registration order is not assured by the device driver core from v4l2 POV.

I thought about embedding some API in a struct v4l2_device for the subdevs
to be able to get their master clock(s) as they need it. But this would work
only when a v4l2_device and v4l2_subdev are matched (registered) before I2C
client's probe(), or alternatively subdev_internal_ops::registered() callback,
is called. 

Currently such requirement is satisfied when the I2C client/v4l2 subdev
devices are registered from within a v4l2 bridge/host driver initialization
routine. But we may need to stop doing this to adhere to the DT rules.

I guess above you didn't really mean to create subdevs from board code?
The I2C client registration is now done at the I2C bus drivers, using the OF
helpers to retrieve the child devices list from fdt.

I guess we could try to create some sort of replacement for
v4l2_i2c_new_subdev_board() function in linux/drivers/of/* (of_i2c.c ?),
similar to of_i2c_register_devices().

But first we would have somehow to make sure the host drivers are registered
and initialized first. I'm not sure how to do it.
Plus such a new subdev registration method would have to obtain a relevant
struct v4l2_device object reference during the process; which is getting
a bit cumbersome..

Also, if we used a 'struct clk' to handle clocks provided by hosts to subdevs,
could we use any subdev operation callback to pass a reference to such object
from host to subdev? I doubt since the clock may be needed in the subdev before
it is allocated and fully initialized, (i.e. available in the host).

If we have embedded a 'struct clk' pointer into struct v4l2_device, it would
have probably to be an array of clocks and the subdev would have to be able to
find out which clock applies to it. 

So I thought about doing something like:

diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index d61febf..9888f7d 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -54,6 +54,7 @@ struct v4l2_device {
        /* notify callback called by some sub-devices. */
        void (*notify)(struct v4l2_subdev *sd,
                        unsigned int notification, void *arg);
+       const struct clk * (*clock_get)(struct v4l2_subdev *sd);
        /* The control handler. May be NULL. */
        struct v4l2_ctrl_handler *ctrl_handler;
        /* Device's priority state */

This would allow the host to return proper clock for a subdev.
But it won't work unless the initialization order is assured..

> 
>   Actions:
> 
>   - Work on a proof-of-concept implementation of the new subdevs registration
>     mechanism and send an RFC (whoever needs it first).
>   - Work on a proof-of-concept clock handling using struct clk with the OMAP3
>     ISP driver (Laurent).

---
Regards,
Sylwester

Re: [ANN] Meeting minutes of the Cambourne meeting

[Laurent Pinchart]

Hi Sylwester,

On Sunday 28 August 2011 13:28:23 Sylwester Nawrocki wrote:
> On 08/08/2011 05:50 PM, Laurent Pinchart wrote:
> > Subdevs hierachy, Linux device model
> > ------------------------------------
> > 
> >   Preliminary conclusions:
> >   
> >   - With the move to device tree on ARM (and other platforms), I2C, SPI
> >   and
> >   
> >     platform subdevs should be created from board code, not from
> >     bridge/host drivers.
> >   
> >   - Bus notifiers should be used by bridge/host drivers to wait for all
> >   
> >     required subdevs. V4L2 core should provide helper functions.
> >   
> >   - struct clk should be used to handle clocks provided by hosts to
> >   subdevs.
> 
> I have been investigating recently possible ways to correct the external
> clock handling in Samsung FIMC driver and this led me up to the device
> tree stuff. I.e. in order to be able to register any I2C client device
> there is a need to enable its master clock at the v4l2 host/bridge driver.

To be completely generic, the subdev master clock can come from anywhere, not 
only from the V4L2 host/bridge (although that's the usual case).

> There is an issue that the v4l2_device (host)/v4l2_subdev hierarchy is not
> reflected by the linux device tree model, e.g. the host might be a platform
> device while the client an I2C client device. Thus a proper device/driver
> registration order is not assured by the device driver core from v4l2 POV.
> 
> I thought about embedding some API in a struct v4l2_device for the subdevs
> to be able to get their master clock(s) as they need it. But this would
> work only when a v4l2_device and v4l2_subdev are matched (registered)
> before I2C client's probe(), or alternatively
> subdev_internal_ops::registered() callback, is called.
> 
> Currently such requirement is satisfied when the I2C client/v4l2 subdev
> devices are registered from within a v4l2 bridge/host driver initialization
> routine. But we may need to stop doing this to adhere to the DT rules.

Right, that's my understanding as well.

> I guess above you didn't really mean to create subdevs from board code?
> The I2C client registration is now done at the I2C bus drivers, using the
> OF helpers to retrieve the child devices list from fdt.

I meant registering the I2C board information from board code (for non-DT 
platforms) or from the device tree (for DT platforms) instead of V4L2 
host/bridge drivers.

> I guess we could try to create some sort of replacement for
> v4l2_i2c_new_subdev_board() function in linux/drivers/of/* (of_i2c.c ?),
> similar to of_i2c_register_devices().
> 
> But first we would have somehow to make sure the host drivers are registered
> and initialized first. I'm not sure how to do it.
> Plus such a new subdev registration method would have to obtain a relevant
> struct v4l2_device object reference during the process; which is getting
> a bit cumbersome..
> 
> Also, if we used a 'struct clk' to handle clocks provided by hosts to
> subdevs, could we use any subdev operation callback to pass a reference to
> such object from host to subdev? I doubt since the clock may be needed in
> the subdev before it is allocated and fully initialized, (i.e. available
> in the host).
> 
> If we have embedded a 'struct clk' pointer into struct v4l2_device, it
> would have probably to be an array of clocks and the subdev would have to
> be able to find out which clock applies to it.
> 
> So I thought about doing something like:
> 
> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
> index d61febf..9888f7d 100644
> --- a/include/media/v4l2-device.h
> +++ b/include/media/v4l2-device.h
> @@ -54,6 +54,7 @@ struct v4l2_device {
>         /* notify callback called by some sub-devices. */
>         void (*notify)(struct v4l2_subdev *sd,
>                         unsigned int notification, void *arg);
> +       const struct clk * (*clock_get)(struct v4l2_subdev *sd);
>         /* The control handler. May be NULL. */
>         struct v4l2_ctrl_handler *ctrl_handler;
>         /* Device's priority state */
> 
> This would allow the host to return proper clock for a subdev.
> But it won't work unless the initialization order is assured..

My idea was to let the kernel register all devices based on the DT or board 
code. When the V4L2 host/bridge driver gets registered, it will then call a 
V4L2 core function with a list of subdevs it needs. The V4L2 core would store 
that information and react to bus notifier events to notify the V4L2 
host/bridge driver when all subdevs are present. At that point the host/bridge 
driver will get hold of all the subdevs and call (probably through the V4L2 
core) their .registered operation. That's where the subdevs will get access to 
their clock using clk_get().

This is really a rough idea, we will probably run into unexpected issues. I'm 
not even sure if this can work out in the end, but I don't really see another 
clean solution for now.

-- 
Regards,

Laurent Pinchart

Re: [ANN] Meeting minutes of the Cambourne meeting

[Guennadi Liakhovetski]

On Mon, 29 Aug 2011, Laurent Pinchart wrote:

[snip]

> My idea was to let the kernel register all devices based on the DT or board 
> code. When the V4L2 host/bridge driver gets registered, it will then call a 
> V4L2 core function with a list of subdevs it needs. The V4L2 core would store 
> that information and react to bus notifier events to notify the V4L2 
> host/bridge driver when all subdevs are present. At that point the host/bridge 
> driver will get hold of all the subdevs and call (probably through the V4L2 
> core) their .registered operation. That's where the subdevs will get access to 
> their clock using clk_get().

Correct me, if I'm wrong, but this seems to be the case of sensor (and 
other i2c-client) drivers having to succeed their probe() methods without 
being able to actually access the hardware?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [ANN] Meeting minutes of the Cambourne meeting

[Laurent Pinchart]

Hi Guennadi,

On Tuesday 30 August 2011 00:20:09 Guennadi Liakhovetski wrote:
> On Mon, 29 Aug 2011, Laurent Pinchart wrote:
> 
> [snip]
> 
> > My idea was to let the kernel register all devices based on the DT or
> > board code. When the V4L2 host/bridge driver gets registered, it will
> > then call a V4L2 core function with a list of subdevs it needs. The V4L2
> > core would store that information and react to bus notifier events to
> > notify the V4L2 host/bridge driver when all subdevs are present. At that
> > point the host/bridge driver will get hold of all the subdevs and call
> > (probably through the V4L2 core) their .registered operation. That's
> > where the subdevs will get access to their clock using clk_get().
> 
> Correct me, if I'm wrong, but this seems to be the case of sensor (and
> other i2c-client) drivers having to succeed their probe() methods without
> being able to actually access the hardware?

That's right. I'd love to find a better way :-) Note that this is already the 
case for many subdev drivers that probe the hardware in the .registered() 
operation instead of the probe() method.

-- 
Regards,

Laurent Pinchart

Re: [ANN] Meeting minutes of the Cambourne meeting

[Guennadi Liakhovetski]

On Tue, 30 Aug 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday 30 August 2011 00:20:09 Guennadi Liakhovetski wrote:
> > On Mon, 29 Aug 2011, Laurent Pinchart wrote:
> > 
> > [snip]
> > 
> > > My idea was to let the kernel register all devices based on the DT or
> > > board code. When the V4L2 host/bridge driver gets registered, it will
> > > then call a V4L2 core function with a list of subdevs it needs. The V4L2
> > > core would store that information and react to bus notifier events to
> > > notify the V4L2 host/bridge driver when all subdevs are present. At that
> > > point the host/bridge driver will get hold of all the subdevs and call
> > > (probably through the V4L2 core) their .registered operation. That's
> > > where the subdevs will get access to their clock using clk_get().
> > 
> > Correct me, if I'm wrong, but this seems to be the case of sensor (and
> > other i2c-client) drivers having to succeed their probe() methods without
> > being able to actually access the hardware?
> 
> That's right. I'd love to find a better way :-) Note that this is already the 
> case for many subdev drivers that probe the hardware in the .registered() 
> operation instead of the probe() method.

Then why do you think it is better, than adding devices from bridge 
drivers? Think about hotpluggable devices - drivers create devices all the 
time - USB etc. Why cannot we do the same? As a historic reference: 
soc-camera used to do this too before - probe without hardware access and 
"really-probe" after the host turns on the clock. Then we switched to 
registering devices later. I like the present approach better.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [ANN] Meeting minutes of the Cambourne meeting

[Mark Brown]

On Tue, Aug 30, 2011 at 12:20:09AM +0200, Guennadi Liakhovetski wrote:
> On Mon, 29 Aug 2011, Laurent Pinchart wrote:

> > My idea was to let the kernel register all devices based on the DT or board 
> > code. When the V4L2 host/bridge driver gets registered, it will then call a 
> > V4L2 core function with a list of subdevs it needs. The V4L2 core would store 
> > that information and react to bus notifier events to notify the V4L2 
> > host/bridge driver when all subdevs are present. At that point the host/bridge 
> > driver will get hold of all the subdevs and call (probably through the V4L2 
> > core) their .registered operation. That's where the subdevs will get access to 
> > their clock using clk_get().

> Correct me, if I'm wrong, but this seems to be the case of sensor (and 
> other i2c-client) drivers having to succeed their probe() methods without 
> being able to actually access the hardware?

The events should only be generated after the probe() has succeeded so
if the driver talks to the hardware then it can fail probe() if need be.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Grant Likely]

On Tue, Aug 30, 2011 at 02:41:48PM +0100, Mark Brown wrote:
> On Tue, Aug 30, 2011 at 12:20:09AM +0200, Guennadi Liakhovetski wrote:
> > On Mon, 29 Aug 2011, Laurent Pinchart wrote:
> 
> > > My idea was to let the kernel register all devices based on the DT or board 
> > > code. When the V4L2 host/bridge driver gets registered, it will then call a 
> > > V4L2 core function with a list of subdevs it needs. The V4L2 core would store 
> > > that information and react to bus notifier events to notify the V4L2 
> > > host/bridge driver when all subdevs are present. At that point the host/bridge 

Sounds a lot like what ASoC is currently doing.

> > > driver will get hold of all the subdevs and call (probably through the V4L2 
> > > core) their .registered operation. That's where the subdevs will get access to 
> > > their clock using clk_get().
> 
> > Correct me, if I'm wrong, but this seems to be the case of sensor (and 
> > other i2c-client) drivers having to succeed their probe() methods without 
> > being able to actually access the hardware?

It indeed sounds like that, which also concerns me.  ASoC and other
subsystems have exactly the same problem where the 'device' is
actually an aggregate of multiple devices attached to different
busses.  My personal opinion is that the best way to handle this is to
support deferred probing so that a driver can fail with -EAGAIN if all
the resources that it requires are not available immediately, and have
the driver core retry the probe after other devices have successfully
probed.

I've got prototype code for this, but it needs some more work before
being mainlined.

> The events should only be generated after the probe() has succeeded so
> if the driver talks to the hardware then it can fail probe() if need be.

I'm a bit confused here.  Which events are you referring to, and which
.probe call? (the i2c/spi/whatever probe, or the aggregate v4l2 probe?)

Re: [ANN] Meeting minutes of the Cambourne meeting

[Mark Brown]

On Tue, Aug 30, 2011 at 07:56:09AM -0600, Grant Likely wrote:
> On Tue, Aug 30, 2011 at 02:41:48PM +0100, Mark Brown wrote:

> > The events should only be generated after the probe() has succeeded so
> > if the driver talks to the hardware then it can fail probe() if need be.

> I'm a bit confused here.  Which events are you referring to, and which
> .probe call? (the i2c/spi/whatever probe, or the aggregate v4l2 probe?)

There's some driver model core level notifiers that are generated when
things manage to bind (postdating all the ASoC stuff for this IIRC, and
not covering the suspend/resume ordering issues).  Actually, thinking
about it they may be per bus.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Guennadi Liakhovetski]

Hi Grant

On Tue, 30 Aug 2011, Grant Likely wrote:

> On Tue, Aug 30, 2011 at 02:41:48PM +0100, Mark Brown wrote:
> > On Tue, Aug 30, 2011 at 12:20:09AM +0200, Guennadi Liakhovetski wrote:
> > > On Mon, 29 Aug 2011, Laurent Pinchart wrote:
> > 
> > > > My idea was to let the kernel register all devices based on the DT or board 
> > > > code. When the V4L2 host/bridge driver gets registered, it will then call a 
> > > > V4L2 core function with a list of subdevs it needs. The V4L2 core would store 
> > > > that information and react to bus notifier events to notify the V4L2 
> > > > host/bridge driver when all subdevs are present. At that point the host/bridge 
> 
> Sounds a lot like what ASoC is currently doing.
> 
> > > > driver will get hold of all the subdevs and call (probably through the V4L2 
> > > > core) their .registered operation. That's where the subdevs will get access to 
> > > > their clock using clk_get().
> > 
> > > Correct me, if I'm wrong, but this seems to be the case of sensor (and 
> > > other i2c-client) drivers having to succeed their probe() methods without 
> > > being able to actually access the hardware?
> 
> It indeed sounds like that, which also concerns me.  ASoC and other
> subsystems have exactly the same problem where the 'device' is
> actually an aggregate of multiple devices attached to different
> busses.  My personal opinion is that the best way to handle this is to
> support deferred probing

Yes, that's also what I think should be done. But I was thinking about a 
slightly different approach - a dependency-based probing. I.e., you should 
be able to register a device, depending on another one (parent?), and only 
after the latter one has successfully probed, the driver core should be 
allowed to probe the child. Of course, devices can depend on multiple 
other devices, so, a single parent might not be enough.

Thanks
Guennadi

> so that a driver can fail with -EAGAIN if all
> the resources that it requires are not available immediately, and have
> the driver core retry the probe after other devices have successfully
> probed.
> 
> I've got prototype code for this, but it needs some more work before
> being mainlined.
> 
> > The events should only be generated after the probe() has succeeded so
> > if the driver talks to the hardware then it can fail probe() if need be.
> 
> I'm a bit confused here.  Which events are you referring to, and which
> .probe call? (the i2c/spi/whatever probe, or the aggregate v4l2 probe?)
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [ANN] Meeting minutes of the Cambourne meeting

[Grant Likely]

On Tue, Aug 30, 2011 at 8:03 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi Grant
>
> On Tue, 30 Aug 2011, Grant Likely wrote:
>
>> On Tue, Aug 30, 2011 at 02:41:48PM +0100, Mark Brown wrote:
>> > On Tue, Aug 30, 2011 at 12:20:09AM +0200, Guennadi Liakhovetski wrote:
>> > > On Mon, 29 Aug 2011, Laurent Pinchart wrote:
>> >
>> > > > My idea was to let the kernel register all devices based on the DT or board
>> > > > code. When the V4L2 host/bridge driver gets registered, it will then call a
>> > > > V4L2 core function with a list of subdevs it needs. The V4L2 core would store
>> > > > that information and react to bus notifier events to notify the V4L2
>> > > > host/bridge driver when all subdevs are present. At that point the host/bridge
>>
>> Sounds a lot like what ASoC is currently doing.
>>
>> > > > driver will get hold of all the subdevs and call (probably through the V4L2
>> > > > core) their .registered operation. That's where the subdevs will get access to
>> > > > their clock using clk_get().
>> >
>> > > Correct me, if I'm wrong, but this seems to be the case of sensor (and
>> > > other i2c-client) drivers having to succeed their probe() methods without
>> > > being able to actually access the hardware?
>>
>> It indeed sounds like that, which also concerns me.  ASoC and other
>> subsystems have exactly the same problem where the 'device' is
>> actually an aggregate of multiple devices attached to different
>> busses.  My personal opinion is that the best way to handle this is to
>> support deferred probing
>
> Yes, that's also what I think should be done. But I was thinking about a
> slightly different approach - a dependency-based probing. I.e., you should
> be able to register a device, depending on another one (parent?), and only
> after the latter one has successfully probed, the driver core should be
> allowed to probe the child. Of course, devices can depend on multiple
> other devices, so, a single parent might not be enough.

Yes, a dependency system would be lovely... but it gets really complex
in a hurry, especially when faced with heterogeneous device
registrations.  A deferral system ends up being really simple to
implement and probably work just as well.

g.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Laurent Pinchart]

On Tuesday 30 August 2011 17:18:31 Grant Likely wrote:
> On Tue, Aug 30, 2011 at 8:03 AM, Guennadi Liakhovetski
> 
> <g.liakhovetski@gmx.de> wrote:
> > Hi Grant
> > 
> > On Tue, 30 Aug 2011, Grant Likely wrote:
> >> On Tue, Aug 30, 2011 at 02:41:48PM +0100, Mark Brown wrote:
> >> > On Tue, Aug 30, 2011 at 12:20:09AM +0200, Guennadi Liakhovetski wrote:
> >> > > On Mon, 29 Aug 2011, Laurent Pinchart wrote:
> >> > > > My idea was to let the kernel register all devices based on the DT
> >> > > > or board code. When the V4L2 host/bridge driver gets registered,
> >> > > > it will then call a V4L2 core function with a list of subdevs it
> >> > > > needs. The V4L2 core would store that information and react to
> >> > > > bus notifier events to notify the V4L2 host/bridge driver when
> >> > > > all subdevs are present. At that point the host/bridge
> >> 
> >> Sounds a lot like what ASoC is currently doing.
> >> 
> >> > > > driver will get hold of all the subdevs and call (probably through
> >> > > > the V4L2 core) their .registered operation. That's where the
> >> > > > subdevs will get access to their clock using clk_get().
> >> > > 
> >> > > Correct me, if I'm wrong, but this seems to be the case of sensor
> >> > > (and other i2c-client) drivers having to succeed their probe()
> >> > > methods without being able to actually access the hardware?
> >> 
> >> It indeed sounds like that, which also concerns me.  ASoC and other
> >> subsystems have exactly the same problem where the 'device' is
> >> actually an aggregate of multiple devices attached to different
> >> busses.  My personal opinion is that the best way to handle this is to
> >> support deferred probing
> > 
> > Yes, that's also what I think should be done. But I was thinking about a
> > slightly different approach - a dependency-based probing. I.e., you
> > should be able to register a device, depending on another one (parent?),
> > and only after the latter one has successfully probed, the driver core
> > should be allowed to probe the child. Of course, devices can depend on
> > multiple other devices, so, a single parent might not be enough.
> 
> Yes, a dependency system would be lovely... but it gets really complex
> in a hurry, especially when faced with heterogeneous device
> registrations.  A deferral system ends up being really simple to
> implement and probably work just as well.

The core issue is that physical device trees, clock trees, power trees and 
logical device tress are not always aligned. Instanciating devices based on 
the parent-child device relationships will always lead to situations where a 
device probe() method will be called with clocks or power sources not 
available yet.

A dependency system is tempting but will be very complex to implement 
properly, especially when faced with cyclic dependencies. For instance the 
OMAP3 ISP driver requires the camera sensor device to be present to proceed, 
and the camera sensor requires a clock provided by the OMAP3 ISP. To solve 
this we need to probe the OMAP3 ISP first, have it register its clock devices, 
and then wait until all sensors become available.

A probe deferral system is probably simpler, but it will have its share of 
problems as well. In the above example, if the sensor is probed first, the 
driver can return -EAGAIN in the probe() method as the clock isn't available 
yet (I'm not sure how to differentiate between "not available yet" and "not 
present in the system" though). However, if the OMAP3 ISP is probed first, 
returning -EAGAIN in its probe() method won't really help, as we need to 
register the clock before waiting for the sensor.

-- 
Regards,

Laurent Pinchart

Re: [ANN] Meeting minutes of the Cambourne meeting

[Mark Brown]

On Tue, Aug 30, 2011 at 05:42:55PM +0200, Laurent Pinchart wrote:

> A dependency system is tempting but will be very complex to implement 
> properly, especially when faced with cyclic dependencies. For instance the 
> OMAP3 ISP driver requires the camera sensor device to be present to proceed, 
> and the camera sensor requires a clock provided by the OMAP3 ISP. To solve 
> this we need to probe the OMAP3 ISP first, have it register its clock devices, 
> and then wait until all sensors become available.

With composite devices like that where the borad has sufficient
interesting stuff on it representing the board itself as a device (this
is what ASoC does).

> A probe deferral system is probably simpler, but it will have its share of 
> problems as well. In the above example, if the sensor is probed first, the 
> driver can return -EAGAIN in the probe() method as the clock isn't available 
> yet (I'm not sure how to differentiate between "not available yet" and "not 
> present in the system" though). However, if the OMAP3 ISP is probed first, 
> returning -EAGAIN in its probe() method won't really help, as we need to 
> register the clock before waiting for the sensor.

Having a device for the camera subsystem as a whole breaks this loop as
the probe of that device triggers the overall system probe.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Laurent Pinchart]

Hi Mark,

On Tuesday 30 August 2011 17:46:42 Mark Brown wrote:
> On Tue, Aug 30, 2011 at 05:42:55PM +0200, Laurent Pinchart wrote:
> > A dependency system is tempting but will be very complex to implement
> > properly, especially when faced with cyclic dependencies. For instance
> > the OMAP3 ISP driver requires the camera sensor device to be present to
> > proceed, and the camera sensor requires a clock provided by the OMAP3
> > ISP. To solve this we need to probe the OMAP3 ISP first, have it
> > register its clock devices, and then wait until all sensors become
> > available.
> 
> With composite devices like that where the borad has sufficient
> interesting stuff on it representing the board itself as a device (this
> is what ASoC does).
> 
> > A probe deferral system is probably simpler, but it will have its share
> > of problems as well. In the above example, if the sensor is probed
> > first, the driver can return -EAGAIN in the probe() method as the clock
> > isn't available yet (I'm not sure how to differentiate between "not
> > available yet" and "not present in the system" though). However, if the
> > OMAP3 ISP is probed first, returning -EAGAIN in its probe() method won't
> > really help, as we need to register the clock before waiting for the
> > sensor.
> 
> Having a device for the camera subsystem as a whole breaks this loop as
> the probe of that device triggers the overall system probe.

The exact same idea crossed my mind after hitting the "Send" button :-)

Would such a device be included in the DT ? My understanding is that the DT 
should only describe the hardware.

-- 
Regards,

Laurent Pinchart

Re: [ANN] Meeting minutes of the Cambourne meeting

[Mark Brown]

On Tue, Aug 30, 2011 at 10:12:30PM +0200, Laurent Pinchart wrote:

> Would such a device be included in the DT ? My understanding is that the DT 
> should only describe the hardware.

For ASoC they will be, the view is that the schematic for the board is
sufficiently interesting to count as hardware.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Laurent Pinchart]

On Tuesday 30 August 2011 22:35:59 Grant Likely wrote:
> On Aug 30, 2011 2:19 PM, "Mark Brown" wrote:
> > On Tue, Aug 30, 2011 at 10:12:30PM +0200, Laurent Pinchart wrote:
> > > Would such a device be included in the DT ? My understanding is that
> > > the DT should only describe the hardware.
> > 
> > For ASoC they will be, the view is that the schematic for the board is
> > sufficiently interesting to count as hardware.
> 
> Yes. Aggregate devices are sufficiently complex that there is a strong
> argument for using a node to describe one.

Is there any document or sample implementation ?

-- 
Regards,

Laurent Pinchart

Re: [ANN] Meeting minutes of the Cambourne meeting

[Mark Brown]

On Tue, Aug 30, 2011 at 10:37:21PM +0200, Laurent Pinchart wrote:
> On Tuesday 30 August 2011 22:35:59 Grant Likely wrote:

> > Yes. Aggregate devices are sufficiently complex that there is a strong
> > argument for using a node to describe one.

> Is there any document or sample implementation ?

ASoC.  Unfortunately there are several rather annoying problems in the
Linux device model which make this substantially less straightforward
than it might otherwise be, the main ones being waiting for everything
to probe and making sure everything suspends and resumes in the correct
order.

Re: [ANN] Meeting minutes of the Cambourne meeting

[Guennadi Liakhovetski]

On Tue, 30 Aug 2011, Laurent Pinchart wrote:

> A dependency system is tempting but will be very complex to implement 
> properly, especially when faced with cyclic dependencies. For instance the 
> OMAP3 ISP driver requires the camera sensor device to be present to proceed, 

Switching to a notifier instead of waiting in probe() might be a good idea 
(TM).

> and the camera sensor requires a clock provided by the OMAP3 ISP. To solve 
> this we need to probe the OMAP3 ISP first, have it register its clock devices, 
> and then wait until all sensors become available.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/