RFC: V4L2 driver for Qualcomm MSM camera.

RFC: V4L2 driver for Qualcomm MSM camera.

[Shuzhen Wang]

Hello, 

This is the architecture overview we put together for Qualcomm MSM camera
support in linux-media tree. Your comments are very much appreciated!


Introduction
============

This is the video4linux driver for Qualcomm MSM (Mobile Station Modem)
camera.

The driver supports video and camera functionality on Qualcomm MSM SoC.
It supports camera sensors provided by OEMs with parallel/MIPI
interfaces, and operates in continuous streaming, snapshot, or video
recording modes.

Hardware description
====================

MSM camera hardware contains the following components:
1. One or more camera sensors controlled via I2C bus, and data outputs
on AXI or MIPI bus.
2. Video Front End (VFE) core is an image signal processing hardware
pipeline that takes the sensor output, does the necessary processing,
and outputs to a buffer for V4L2 application. VFE is clocked by PCLK
(pixel clock) from the sensor.

Software description
====================

The driver encapsulates the low-level hardware management and aligns
the interface to V4L2 driver framework. There is also a user space
camera service daemon which handles events from driver and changes
settings accordingly.

During boot-up, the sensor is probed for and if the probe succeeds
/dev/video0 and /dev/msm_camera/config0 device nodes are created. The
/dev/video0 node is used for video buffer allocation in the kernel and for
receiving V4L2 ioctls for controlling the camera hardware (VFE, sensors).
The /dev/msm_camera/config0 node is used for sending commands and other
statistics available from the hardware to the camera service daemon. Note
that if more than one camera sensor is detected, there will be /dev/video1
and /dev/msm_camera/config1 device nodes created as well.

Design
======

For MSM camera IC, significant portion of image processing and optimization
codes are proprietary, so they cannot sit in kernel space. This plays an
important role when making design decisions.

Our design is to have a light-weighted kernel driver, and put the
proprietary code in a user space daemon. The daemon polls on events
from /dev/msm_camera/config0 device in the form of v4l2_event. The events
could either be asynchronous (generated by the hardware), or synchronous
(control command from the application). Based on the events it receives,
the daemon will send appropriate control commands to the hardware.

   +-------------+        +----------------+
   | Application |        | Service Daemon |
   +-------------+        +----------------+     User Space
 ..........................................................
   +--------------------------------------+      Kernel Space
   |                V4L2                  |
   +--------------------------------------+
   +---------+     +--------+     +-------+
   | VFE/ISP |     | Sensor |     | Flash |
   +---------+     +--------+     +-------+

Power Management
================

None at this point.

SMP/multi-core
==============

Mutex is used for synchronization of threads accessing the driver
simultaneously. Between hardware interrupt handler and threads,
we use spinlock to make sure locking is done properly.

Security
========

None.

Interface
=========

The driver uses V4L2 API for user kernel interaction. Refer to
http://v4l2spec.bytesex.org/spec-single/v4l2.html.

Between camera service daemon and the driver, we have customized IOCTL
commands associated with /dev/msm_camera/config0 node, that controls MSM
camera hardware. The list of IOCTLs are (declarations can be found in
include/media/msm_camera.h):

MSM_CAM_IOCTL_GET_SENSOR_INFO
        Get the basic information such as name, flash support for the
        detected sensor.
MSM_CAM_IOCTL_SENSOR_IO_CFG
        Get or set sensor configurations: fps, line_pf, pixels_pl,
        exposure and gain, etc. The setting is stored in sensor_cfg_data
        structure.
MSM_CAM_IOCTL_CONFIG_VFE
        Change settings of different components of VFE hardware.
MSM_CAM_IOCTL_CTRL_CMD_DONE
        Notify the driver that the ctrl command is finished.
MSM_CAM_IOCTL_RELEASE_STATS_BUFFER
        Notify the driver that the service daemon has done processing the
        stats buffer, and is returning it to the driver.
MSM_CAM_IOCTL_AXI_CONFIG
        Configure AXI bus parameters (frame buffer addresses, offsets) to
        the VFE hardware.

Driver parameters
=================

None.

Config options
==============

MSM_CAMERA in drivers/media/video/msm/Kconfig file

Dependencies
============

PMIC, I2C, Clock, GPIO

User space utilities
====================

A daemon process in the user space called mm-qcamera-daemon is polling
on events from the driver. This daemon is required in order for the V4L2
client application to run, and it's started by the init script.

Other
=====

To do
=====

1. Eliminate creation of /dev/msm_camera/config0 by routing private
ioctls through /dev/video0.
2. Create sub devices for sensor and VFE.



--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Hans Verkuil]

Good to hear from Qualcomm!

I've made some comments below:

On Thursday, December 23, 2010 20:38:04 Shuzhen Wang wrote:
> Hello, 
> 
> This is the architecture overview we put together for Qualcomm MSM camera
> support in linux-media tree. Your comments are very much appreciated!
> 
> 
> Introduction
> ============
> 
> This is the video4linux driver for Qualcomm MSM (Mobile Station Modem)
> camera.
> 
> The driver supports video and camera functionality on Qualcomm MSM SoC.
> It supports camera sensors provided by OEMs with parallel/MIPI
> interfaces, and operates in continuous streaming, snapshot, or video
> recording modes.
> 
> Hardware description
> ====================
> 
> MSM camera hardware contains the following components:
> 1. One or more camera sensors controlled via I2C bus, and data outputs
> on AXI or MIPI bus.
> 2. Video Front End (VFE) core is an image signal processing hardware
> pipeline that takes the sensor output, does the necessary processing,
> and outputs to a buffer for V4L2 application. VFE is clocked by PCLK
> (pixel clock) from the sensor.
> 
> Software description
> ====================
> 
> The driver encapsulates the low-level hardware management and aligns
> the interface to V4L2 driver framework. There is also a user space
> camera service daemon which handles events from driver and changes
> settings accordingly.
> 
> During boot-up, the sensor is probed for and if the probe succeeds
> /dev/video0 and /dev/msm_camera/config0 device nodes are created. The
> /dev/video0 node is used for video buffer allocation in the kernel and for
> receiving V4L2 ioctls for controlling the camera hardware (VFE, sensors).
> The /dev/msm_camera/config0 node is used for sending commands and other
> statistics available from the hardware to the camera service daemon. Note
> that if more than one camera sensor is detected, there will be /dev/video1
> and /dev/msm_camera/config1 device nodes created as well.

Does config control the sensor or the main msm subsystem? Or both?

> 
> Design
> ======
> 
> For MSM camera IC, significant portion of image processing and optimization
> codes are proprietary, so they cannot sit in kernel space. This plays an
> important role when making design decisions.
> 
> Our design is to have a light-weighted kernel driver, and put the
> proprietary code in a user space daemon. The daemon polls on events
> from /dev/msm_camera/config0 device in the form of v4l2_event. The events
> could either be asynchronous (generated by the hardware), or synchronous
> (control command from the application). Based on the events it receives,
> the daemon will send appropriate control commands to the hardware.
> 
>    +-------------+        +----------------+
>    | Application |        | Service Daemon |
>    +-------------+        +----------------+     User Space
>  ..........................................................
>    +--------------------------------------+      Kernel Space
>    |                V4L2                  |
>    +--------------------------------------+
>    +---------+     +--------+     +-------+
>    | VFE/ISP |     | Sensor |     | Flash |
>    +---------+     +--------+     +-------+

Just to repeat what I have discussed with Qualcomm before (so that everyone knows):
I have no problem with proprietary code as long as:

1) the hardware and driver APIs are clearly documented allowing someone else to
make their own algorithms.

2) the initial state of the hardware as set up by the driver is good enough to
capture video in normal lighting conditions. In other words: the daemon should not
be needed for testing the driver. I compare this with cheap webcams that often
need software white balancing to get a decent picture. They still work without
that, but the picture simply doesn't look very good.

We also discussed the daemon in the past. The idea was that it should be called
from libv4l2. Is this still the plan?

> Power Management
> ================
> 
> None at this point.
> 
> SMP/multi-core
> ==============
> 
> Mutex is used for synchronization of threads accessing the driver
> simultaneously. Between hardware interrupt handler and threads,
> we use spinlock to make sure locking is done properly.

Take a look at the new core-assisted locking scheme implemented for 2.6.37.
This might simplify your driver. Just FYI.
 
> Security
> ========
> 
> None.
> 
> Interface
> =========
> 
> The driver uses V4L2 API for user kernel interaction. Refer to
> http://v4l2spec.bytesex.org/spec-single/v4l2.html.

That's really, really old. This is much more up to date:

http://linuxtv.org/downloads/v4l-dvb-apis/

And the very latest build every day is always available here:

http://www.xs4all.nl/~hverkuil/spec/media.html

> 
> Between camera service daemon and the driver, we have customized IOCTL
> commands associated with /dev/msm_camera/config0 node, that controls MSM
> camera hardware. The list of IOCTLs are (declarations can be found in
> include/media/msm_camera.h):
> 
> MSM_CAM_IOCTL_GET_SENSOR_INFO
>         Get the basic information such as name, flash support for the
>         detected sensor.
> MSM_CAM_IOCTL_SENSOR_IO_CFG
>         Get or set sensor configurations: fps, line_pf, pixels_pl,
>         exposure and gain, etc. The setting is stored in sensor_cfg_data
>         structure.
> MSM_CAM_IOCTL_CONFIG_VFE
>         Change settings of different components of VFE hardware.
> MSM_CAM_IOCTL_CTRL_CMD_DONE
>         Notify the driver that the ctrl command is finished.
> MSM_CAM_IOCTL_RELEASE_STATS_BUFFER
>         Notify the driver that the service daemon has done processing the
>         stats buffer, and is returning it to the driver.
> MSM_CAM_IOCTL_AXI_CONFIG
>         Configure AXI bus parameters (frame buffer addresses, offsets) to
>         the VFE hardware.

Laurent Pinchart has a patch series adding support for device nodes for
sub-devices. The only reason that series isn't merged yet is that there are
no merged drivers that need it. You should use those patches to implement
these ioctls in sub-devices. I guess you probably want to create a subdev
for the VFE hardware. The SENSOR_INFO ioctl seems like something that can
be implemented using the upcoming media controller framework.

My guess is that these ioctls will need some work.
 
> Driver parameters
> =================
> 
> None.
> 
> Config options
> ==============
> 
> MSM_CAMERA in drivers/media/video/msm/Kconfig file
> 
> Dependencies
> ============
> 
> PMIC, I2C, Clock, GPIO
> 
> User space utilities
> ====================
> 
> A daemon process in the user space called mm-qcamera-daemon is polling
> on events from the driver. This daemon is required in order for the V4L2
> client application to run, and it's started by the init script.
> 
> Other
> =====
> 
> To do
> =====
> 
> 1. Eliminate creation of /dev/msm_camera/config0 by routing private
> ioctls through /dev/video0.
> 2. Create sub devices for sensor and VFE.

It's more likely that the private ioctls will go through subdev device nodes.

That's really what they were designed for.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

RE: RFC: V4L2 driver for Qualcomm MSM camera.

[Shuzhen Wang]

Hello, Hans,

Thank you very much for the comments!

I don't have answers to all of your comments, but will reply as much as I
can.
Since a lot of folks are on break here, we will have feedbacks for the other
ones 
after the holiday.


> -----Original Message-----
> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Hans Verkuil
> 
> Does config control the sensor or the main msm subsystem? Or both?
> 

It controls both. 

> Just to repeat what I have discussed with Qualcomm before (so that
> everyone knows):
> I have no problem with proprietary code as long as:
> 
> 1) the hardware and driver APIs are clearly documented allowing someone
> else to
> make their own algorithms.
>

Yes, we will have the APIs clearly documented for the hardware and driver. 
 
> 2) the initial state of the hardware as set up by the driver is good
> enough to
> capture video in normal lighting conditions. In other words: the daemon
> should not
> be needed for testing the driver. I compare this with cheap webcams
> that often
> need software white balancing to get a decent picture. They still work
> without
> that, but the picture simply doesn't look very good.
> 

I agree that it's a very good idea to be able to run the driver without
daemon.
Our challenge is that we have all the hardware pipeline configuration in the
daemon. The driver doesn't know how to configure the pipeline as a whole
based on
user specification, it only cares about the configuration of each individual
component. 

> We also discussed the daemon in the past. The idea was that it should
> be called
> from libv4l2. Is this still the plan?
>

[to be commented on later]

> Take a look at the new core-assisted locking scheme implemented for
> 2.6.37.
> This might simplify your driver. Just FYI.
> 

We will take a look at this.

> Laurent Pinchart has a patch series adding support for device nodes for
> sub-devices. The only reason that series isn't merged yet is that there
> are
> no merged drivers that need it. You should use those patches to
> implement
> these ioctls in sub-devices. I guess you probably want to create a
> subdev
> for the VFE hardware. The SENSOR_INFO ioctl seems like something that
> can
> be implemented using the upcoming media controller framework.
> 
> My guess is that these ioctls will need some work.
> 
> 
> It's more likely that the private ioctls will go through subdev device
> nodes.
> 
> That's really what they were designed for.
> 

Agreed. We will make the sensor and VFE hardware related calls go through 
subdebv device nodes.


Thanks,
Shuzhen

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Mauro Carvalho Chehab]

Em 24-12-2010 09:19, Hans Verkuil escreveu:
> Good to hear from Qualcomm!
> 
> I've made some comments below:
> 
> On Thursday, December 23, 2010 20:38:04 Shuzhen Wang wrote:
>> Hello, 
>>
>> This is the architecture overview we put together for Qualcomm MSM camera
>> support in linux-media tree. Your comments are very much appreciated!
>>
>>
>> Introduction
>> ============
>>
>> This is the video4linux driver for Qualcomm MSM (Mobile Station Modem)
>> camera.
>>
>> The driver supports video and camera functionality on Qualcomm MSM SoC.
>> It supports camera sensors provided by OEMs with parallel/MIPI
>> interfaces, and operates in continuous streaming, snapshot, or video
>> recording modes.
>>
>> Hardware description
>> ====================
>>
>> MSM camera hardware contains the following components:
>> 1. One or more camera sensors controlled via I2C bus, and data outputs
>> on AXI or MIPI bus.
>> 2. Video Front End (VFE) core is an image signal processing hardware
>> pipeline that takes the sensor output, does the necessary processing,
>> and outputs to a buffer for V4L2 application. VFE is clocked by PCLK
>> (pixel clock) from the sensor.
>>
>> Software description
>> ====================
>>
>> The driver encapsulates the low-level hardware management and aligns
>> the interface to V4L2 driver framework. There is also a user space
>> camera service daemon which handles events from driver and changes
>> settings accordingly.
>>
>> During boot-up, the sensor is probed for and if the probe succeeds
>> /dev/video0 and /dev/msm_camera/config0 device nodes are created. The
>> /dev/video0 node is used for video buffer allocation in the kernel and for
>> receiving V4L2 ioctls for controlling the camera hardware (VFE, sensors).
>> The /dev/msm_camera/config0 node is used for sending commands and other
>> statistics available from the hardware to the camera service daemon. Note
>> that if more than one camera sensor is detected, there will be /dev/video1
>> and /dev/msm_camera/config1 device nodes created as well.
> 
> Does config control the sensor or the main msm subsystem? Or both?
> 
>>
>> Design
>> ======
>>
>> For MSM camera IC, significant portion of image processing and optimization
>> codes are proprietary, so they cannot sit in kernel space. This plays an
>> important role when making design decisions.
>>
>> Our design is to have a light-weighted kernel driver, and put the
>> proprietary code in a user space daemon. The daemon polls on events
>> from /dev/msm_camera/config0 device in the form of v4l2_event. The events
>> could either be asynchronous (generated by the hardware), or synchronous
>> (control command from the application). Based on the events it receives,
>> the daemon will send appropriate control commands to the hardware.
>>
>>    +-------------+        +----------------+
>>    | Application |        | Service Daemon |
>>    +-------------+        +----------------+     User Space
>>  ..........................................................
>>    +--------------------------------------+      Kernel Space
>>    |                V4L2                  |
>>    +--------------------------------------+
>>    +---------+     +--------+     +-------+
>>    | VFE/ISP |     | Sensor |     | Flash |
>>    +---------+     +--------+     +-------+
> 
> Just to repeat what I have discussed with Qualcomm before (so that everyone knows):
> I have no problem with proprietary code as long as:
> 
> 1) the hardware and driver APIs are clearly documented allowing someone else to
> make their own algorithms.
> 
> 2) the initial state of the hardware as set up by the driver is good enough to
> capture video in normal lighting conditions. In other words: the daemon should not
> be needed for testing the driver. I compare this with cheap webcams that often
> need software white balancing to get a decent picture. They still work without
> that, but the picture simply doesn't look very good.
> 
> We also discussed the daemon in the past. The idea was that it should be called
> from libv4l2. Is this still the plan?
> 
>> Power Management
>> ================
>>
>> None at this point.
>>
>> SMP/multi-core
>> ==============
>>
>> Mutex is used for synchronization of threads accessing the driver
>> simultaneously. Between hardware interrupt handler and threads,
>> we use spinlock to make sure locking is done properly.
> 
> Take a look at the new core-assisted locking scheme implemented for 2.6.37.
> This might simplify your driver. Just FYI.
>  
>> Security
>> ========
>>
>> None.
>>
>> Interface
>> =========
>>
>> The driver uses V4L2 API for user kernel interaction. Refer to
>> http://v4l2spec.bytesex.org/spec-single/v4l2.html.
> 
> That's really, really old. This is much more up to date:
> 
> http://linuxtv.org/downloads/v4l-dvb-apis/
> 
> And the very latest build every day is always available here:
> 
> http://www.xs4all.nl/~hverkuil/spec/media.html
>>
>> Between camera service daemon and the driver, we have customized IOCTL
>> commands associated with /dev/msm_camera/config0 node, that controls MSM
>> camera hardware. The list of IOCTLs are (declarations can be found in
>> include/media/msm_camera.h):
>>
>> MSM_CAM_IOCTL_GET_SENSOR_INFO
>>         Get the basic information such as name, flash support for the
>>         detected sensor.

Hmm... It may be interesting to have this as a standard ioctl for webcams.

>> MSM_CAM_IOCTL_SENSOR_IO_CFG
>>         Get or set sensor configurations: fps, line_pf, pixels_pl,
>>         exposure and gain, etc. The setting is stored in sensor_cfg_data
>>         structure.

This doesn't make much sense to me as-is. The V4L2 API can set fps, exposure, 
gain and other things. Please only use private ioctl's for those things that
aren't provided elsewhere and can't be mapped into some CTRL.

>> MSM_CAM_IOCTL_CONFIG_VFE
>>         Change settings of different components of VFE hardware.

Hard to analyze it, as you didn't provide any details ;) 

Maybe the media controller API will be the right place for it. As Hans pointed,
the hardware should be able to work without private ioctl's and/or media
controller stuff.

>> MSM_CAM_IOCTL_CTRL_CMD_DONE
>>         Notify the driver that the ctrl command is finished.

Just looking at the ioctl name, this doesn't make much sense. If you open a 
device on normal way, the ioctl it will block until the operation is completed.

Could you please provide more details about it?

>> MSM_CAM_IOCTL_RELEASE_STATS_BUFFER
>>         Notify the driver that the service daemon has done processing the
>>         stats buffer, and is returning it to the driver.

The media controller API is the right place for things like stats.

>> MSM_CAM_IOCTL_AXI_CONFIG
>>         Configure AXI bus parameters (frame buffer addresses, offsets) to
>>         the VFE hardware.

Hard to analyze it, as you didn't provide any details ;) 

The same comments I did for MSM_CAM_IOCTL_CONFIG_VFE apply here.

> Laurent Pinchart has a patch series adding support for device nodes for
> sub-devices. The only reason that series isn't merged yet is that there are
> no merged drivers that need it. You should use those patches to implement
> these ioctls in sub-devices. I guess you probably want to create a subdev
> for the VFE hardware. The SENSOR_INFO ioctl seems like something that can
> be implemented using the upcoming media controller framework.
> 
> My guess is that these ioctls will need some work.
>  
>> Driver parameters
>> =================
>>
>> None.
>>
>> Config options
>> ==============
>>
>> MSM_CAMERA in drivers/media/video/msm/Kconfig file
>>
>> Dependencies
>> ============
>>
>> PMIC, I2C, Clock, GPIO
>>
>> User space utilities
>> ====================
>>
>> A daemon process in the user space called mm-qcamera-daemon is polling
>> on events from the driver. This daemon is required in order for the V4L2
>> client application to run, and it's started by the init script.
>>
>> Other
>> =====
>>
>> To do
>> =====
>>
>> 1. Eliminate creation of /dev/msm_camera/config0 by routing private
>> ioctls through /dev/video0.
>> 2. Create sub devices for sensor and VFE.
> 
> It's more likely that the private ioctls will go through subdev device nodes.
> 
> That's really what they were designed for.
> 
> Regards,
> 
> 	Hans
> 

RE: RFC: V4L2 driver for Qualcomm MSM camera.

[Shuzhen Wang]

> -----Original Message-----
> From: Mauro Carvalho Chehab [mailto:mchehab@redhat.com]
> Sent: Monday, December 27, 2010 4:12 AM
> To: Hans Verkuil
> Cc: Shuzhen Wang; linux-media@vger.kernel.org; hzhong@codeaurora.org
> Subject: Re: RFC: V4L2 driver for Qualcomm MSM camera.
> 
> Em 24-12-2010 09:19, Hans Verkuil escreveu:
> >> MSM_CAM_IOCTL_SENSOR_IO_CFG
> >>         Get or set sensor configurations: fps, line_pf, pixels_pl,
> >>         exposure and gain, etc. The setting is stored in
> sensor_cfg_data
> >>         structure.
> 
> This doesn't make much sense to me as-is. The V4L2 API can set fps,
> exposure,
> gain and other things. Please only use private ioctl's for those things
> that
> aren't provided elsewhere and can't be mapped into some CTRL.
> 

In our design, all these private ioctls are only called from the service
daemon, so they are transparent to the application. For example, when a
standard V4L2 API is called from the app to change fps, it gets translated
to MSM_CAM_IOCTL_SENSOR_IO_CFG in the daemon, and sent to the sensor
hardware.

> >> MSM_CAM_IOCTL_CONFIG_VFE
> >>         Change settings of different components of VFE hardware.
> 
> Hard to analyze it, as you didn't provide any details ;)
> 
> Maybe the media controller API will be the right place for it. As Hans
> pointed,
> the hardware should be able to work without private ioctl's and/or
> media
> controller stuff.
> 

Because all the private ioctl's are only called from daemon, they are not
very big concern here IMHO. The fact that a lot of stuff is done in daemon
does make it harder to decouple. 

MSM_CAM_IOCTL_CONFIG_VFE ioctl calls pass in a structure like this:
struct msm_vfe_cfg_cmd {
        int cmd_type;
        uint16_t length;
        void *value;
};
Where cmd_type indicates what component of the VFE pipeline to configure,
For example, enable/disable stats, VFE buffers configuration, demosaic,
color conversion/correction, etc. The value field will contain the
appropriate data for the said cmd_type.

> >> MSM_CAM_IOCTL_CTRL_CMD_DONE
> >>         Notify the driver that the ctrl command is finished.
> 
> Just looking at the ioctl name, this doesn't make much sense. If you
> open a
> device on normal way, the ioctl it will block until the operation is
> completed.
> 
> Could you please provide more details about it?

The idea is that the kernel driver delegates the control command to the
service daemon ( by means of v4l2_event ). The V4L2 control command call
from the app is blocked until the service daemon is done with operation.

For example, for a VIDIOC_S_CTRL, the driver wraps the v4l2_ctrl structure
in a v4l2_event, publishes it to the daemon, and blocks. The daemon then
calls either MSM_CAM_IOCTL_CONFIG_VFE or MSM_CAM_IOCTL_SENSOR_IO_CFG or
both to configure the hardware. Once thoese ioctls return, it then call 
MSM_CAM_IOCTL_CTRL_CMD_DONE to notify the driver so that it can wake up
the application.

> >> MSM_CAM_IOCTL_AXI_CONFIG
> >>         Configure AXI bus parameters (frame buffer addresses,
> offsets) to
> >>         the VFE hardware.
> 
> Hard to analyze it, as you didn't provide any details ;)
> 
> The same comments I did for MSM_CAM_IOCTL_CONFIG_VFE apply here.

This registers buffers with VFE hardware. Like all other private ioctls, 
It's called from the daemon. 


Thanks!
-Shuzhen

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Laurent Pinchart]

Hi,

On Tuesday 28 December 2010 19:35:05 Shuzhen Wang wrote:
> > -----Original Message-----
> > From: Mauro Carvalho Chehab [mailto:mchehab@redhat.com]
> > Sent: Monday, December 27, 2010 4:12 AM
> > To: Hans Verkuil
> > Cc: Shuzhen Wang; linux-media@vger.kernel.org; hzhong@codeaurora.org
> > Subject: Re: RFC: V4L2 driver for Qualcomm MSM camera.
> > 
> > Em 24-12-2010 09:19, Hans Verkuil escreveu:
> > >> MSM_CAM_IOCTL_SENSOR_IO_CFG
> > >> 
> > >>         Get or set sensor configurations: fps, line_pf, pixels_pl,
> > >>         exposure and gain, etc. The setting is stored in
> > 
> > sensor_cfg_data
> > 
> > >>         structure.
> > 
> > This doesn't make much sense to me as-is. The V4L2 API can set fps,
> > exposure,
> > gain and other things. Please only use private ioctl's for those things
> > that
> > aren't provided elsewhere and can't be mapped into some CTRL.
> 
> In our design, all these private ioctls are only called from the service
> daemon, so they are transparent to the application. For example, when a
> standard V4L2 API is called from the app to change fps, it gets translated
> to MSM_CAM_IOCTL_SENSOR_IO_CFG in the daemon, and sent to the sensor
> hardware.

Please don't do that. The daemon should use the standard V4L2 API. 
Applications must be able to use those APIs directly if they wish to do so. 
Using the daemon must be completely optional for applications that don't want 
software-assisted algorithms such as 3A.

> > >> MSM_CAM_IOCTL_CONFIG_VFE
> > >> 
> > >>         Change settings of different components of VFE hardware.
> > 
> > Hard to analyze it, as you didn't provide any details ;)
> > 
> > Maybe the media controller API will be the right place for it. As Hans
> > pointed,
> > the hardware should be able to work without private ioctl's and/or
> > media
> > controller stuff.
> 
> Because all the private ioctl's are only called from daemon, they are not
> very big concern here IMHO. The fact that a lot of stuff is done in daemon
> does make it harder to decouple.
> 
> MSM_CAM_IOCTL_CONFIG_VFE ioctl calls pass in a structure like this:
> struct msm_vfe_cfg_cmd {
>         int cmd_type;
>         uint16_t length;
>         void *value;
> };
> Where cmd_type indicates what component of the VFE pipeline to configure,
> For example, enable/disable stats, VFE buffers configuration, demosaic,
> color conversion/correction, etc. The value field will contain the
> appropriate data for the said cmd_type.
> 
> > >> MSM_CAM_IOCTL_CTRL_CMD_DONE
> > >> 
> > >>         Notify the driver that the ctrl command is finished.
> > 
> > Just looking at the ioctl name, this doesn't make much sense. If you
> > open a
> > device on normal way, the ioctl it will block until the operation is
> > completed.
> > 
> > Could you please provide more details about it?
> 
> The idea is that the kernel driver delegates the control command to the
> service daemon ( by means of v4l2_event ). The V4L2 control command call
> from the app is blocked until the service daemon is done with operation.

-EINVAL. No, no and no again. That's a completely broken design. Go back to 
the black board and create a real Linux driver.

If you want a daemon to isolate proprietary code, fine, but it must be totally 
optional. Make the application communicate with the daemon (through a libv4l 
plugin for instance), and have the daemon use the V4L2 API to communicate with 
the hardware.

I will strongly NAK any implementation that requires a daemon.

> For example, for a VIDIOC_S_CTRL, the driver wraps the v4l2_ctrl structure
> in a v4l2_event, publishes it to the daemon, and blocks. The daemon then
> calls either MSM_CAM_IOCTL_CONFIG_VFE or MSM_CAM_IOCTL_SENSOR_IO_CFG or
> both to configure the hardware. Once thoese ioctls return, it then call
> MSM_CAM_IOCTL_CTRL_CMD_DONE to notify the driver so that it can wake up
> the application.
> 
> > >> MSM_CAM_IOCTL_AXI_CONFIG
> > >> 
> > >>         Configure AXI bus parameters (frame buffer addresses,
> > 
> > offsets) to
> > 
> > >>         the VFE hardware.
> > 
> > Hard to analyze it, as you didn't provide any details ;)
> > 
> > The same comments I did for MSM_CAM_IOCTL_CONFIG_VFE apply here.
> 
> This registers buffers with VFE hardware. Like all other private ioctls,
> It's called from the daemon.

-- 
Regards,

Laurent Pinchart

RE: RFC: V4L2 driver for Qualcomm MSM camera.

[Shuzhen Wang]

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Tuesday, December 28, 2010 12:24 PM
> To: Shuzhen Wang
> Cc: 'Mauro Carvalho Chehab'; 'Hans Verkuil'; linux-
> media@vger.kernel.org; hzhong@codeaurora.org; Yan, Yupeng
> Subject: Re: RFC: V4L2 driver for Qualcomm MSM camera.
>
> I will strongly NAK any implementation that requires a daemon.
> 

We understand the motivation behind making the daemon optional.
However there are restrictions from legal perspective, which we
don't know how to get around.

A simplest video streaming data flow with MSM ISP is like this:

Sensor -> ISP Hardware pipeline -> videobuf

The procedure to set up ISP pipeline is proprietary and cannot
be open sourced. Without proper pipeline configuration, streaming
won't work. And That's why we require the daemon. 
 
Thank,
Shuzhen

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Haibo Zhong]

On 1/3/2011 6:37 PM, Shuzhen Wang wrote:
>> -----Original Message-----
>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>> Sent: Tuesday, December 28, 2010 12:24 PM
>> To: Shuzhen Wang
>> Cc: 'Mauro Carvalho Chehab'; 'Hans Verkuil'; linux-
>> media@vger.kernel.org; hzhong@codeaurora.org; Yan, Yupeng
>> Subject: Re: RFC: V4L2 driver for Qualcomm MSM camera.
>>
>> I will strongly NAK any implementation that requires a daemon.
>>
>
> We understand the motivation behind making the daemon optional.
> However there are restrictions from legal perspective, which we
> don't know how to get around.
>
> A simplest video streaming data flow with MSM ISP is like this:
>
> Sensor ->  ISP Hardware pipeline ->  videobuf
>
> The procedure to set up ISP pipeline is proprietary and cannot
> be open sourced. Without proper pipeline configuration, streaming
> won't work. And That's why we require the daemon.

Laurent/Hans/Mauro,

We are working on and will provide more design information on Qualcomm 
MSM ISP design and explain the legal concern of the daemon implementation.

The underlined idea is to comply to V4L2 architecture with MSM solution. 
In the meantime, Laurent, can you share with your major concern about 
the Daemon?

Thanks,
Jeff

>
> Thank,
> Shuzhen
>
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> --
>

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Laurent Pinchart]

Hi Shuzhen,

On Tuesday 04 January 2011 03:37:10 Shuzhen Wang wrote:
> On Tuesday, December 28, 2010 12:24 PM Laurent Pinchart wrote:
> > 
> > I will strongly NAK any implementation that requires a daemon.
> 
> We understand the motivation behind making the daemon optional.
> However there are restrictions from legal perspective, which we
> don't know how to get around.
> 
> A simplest video streaming data flow with MSM ISP is like this:
> 
> Sensor -> ISP Hardware pipeline -> videobuf
> 
> The procedure to set up ISP pipeline is proprietary and cannot
> be open sourced. Without proper pipeline configuration, streaming
> won't work. And That's why we require the daemon.

Then I'm afraid you simply won't be able to provide an open-source Linux 
driver.

The purpose of a driver is to allow the system to access the hardware. Whether 
the code lives in the Linux kernel or in userspace is irrelevant here. If part 
of your driver is closed source, then it can't be called open-source and it 
can't be included in the mainline Linux kernel (there's a precedent on the 
same issue in the Direct Rendering Manager subsystem, open-source kernel code 
has been refused because it depended on closed-source userspace components).

If you want your hardware supported in the mainline Linux kernel you have to 
play by the rules and provide a complete open-source driver. You need to sort 
it out with your legal department and get a green light on disclosing 
information on how to setup the ISP pipeline.

-- 
Regards,

Laurent Pinchart

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Laurent Pinchart]

Hi Jeff,

On Tuesday 04 January 2011 07:14:00 Haibo Zhong wrote:
> On 1/3/2011 6:37 PM, Shuzhen Wang wrote:
> > On Tuesday, December 28, 2010 12:24 PM Laurent Pinchart wrote:
> >> 
> >> I will strongly NAK any implementation that requires a daemon.
> > 
> > We understand the motivation behind making the daemon optional.
> > However there are restrictions from legal perspective, which we
> > don't know how to get around.
> > 
> > A simplest video streaming data flow with MSM ISP is like this:
> > 
> > Sensor ->  ISP Hardware pipeline ->  videobuf
> > 
> > The procedure to set up ISP pipeline is proprietary and cannot
> > be open sourced. Without proper pipeline configuration, streaming
> > won't work. And That's why we require the daemon.
> 
> Laurent/Hans/Mauro,
> 
> We are working on and will provide more design information on Qualcomm
> MSM ISP design and explain the legal concern of the daemon implementation.
> 
> The underlined idea is to comply to V4L2 architecture with MSM solution.

That's a good first step, but I'm afraid it's not enough. If you want your 
driver to be included in the mainline Linux kernel (and its derivative 
distribution kernels such as the MeeGo kernel for instance) all the code 
needed to access and use the hardware must be open-source.

This of course doesn't preclude you from providing a closed-source userspace 
implementation of proprietary hardware-assisted image processing algorithms 
for instance (as a library or as a daemon).

> In the meantime, Laurent, can you share with your major concern about the
> Daemon?

I have two concerns.

- The daemon makes code required to use the hardware closed-source, making the 
driver closed-source (whether the kernel-side code is licensed under the GPL 
or not is irrelevant). I would have the exact same opinion if the required 
userspace proprietary code was provided as a library, so this concern is not 
specific to the implementation being in the form of a daemon.

- The daemon makes the kernel-side driver architecture more complex for no 
reason. Assuming you can make all the driver open-source in the future and 
want to keep proprietary userspace image processing code closed-source, the 
driver architecture must not be designed solely to support that use case. The 
driver should be clean and lean, and the proprietary code must then come as a 
user of the driver, not the other way around.

As a summary, having part of the driver closed-source is a no-go, and having 
part of the kernel driver API designed and used to support closed-source 
components only is a no-go as well.

-- 
Regards,

Laurent Pinchart

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Hans Verkuil]

> Hi Jeff,
>
> On Tuesday 04 January 2011 07:14:00 Haibo Zhong wrote:
>> On 1/3/2011 6:37 PM, Shuzhen Wang wrote:
>> > On Tuesday, December 28, 2010 12:24 PM Laurent Pinchart wrote:
>> >>
>> >> I will strongly NAK any implementation that requires a daemon.
>> >
>> > We understand the motivation behind making the daemon optional.
>> > However there are restrictions from legal perspective, which we
>> > don't know how to get around.
>> >
>> > A simplest video streaming data flow with MSM ISP is like this:
>> >
>> > Sensor ->  ISP Hardware pipeline ->  videobuf
>> >
>> > The procedure to set up ISP pipeline is proprietary and cannot
>> > be open sourced. Without proper pipeline configuration, streaming
>> > won't work. And That's why we require the daemon.
>>
>> Laurent/Hans/Mauro,
>>
>> We are working on and will provide more design information on Qualcomm
>> MSM ISP design and explain the legal concern of the daemon
>> implementation.
>>
>> The underlined idea is to comply to V4L2 architecture with MSM solution.
>
> That's a good first step, but I'm afraid it's not enough. If you want your
> driver to be included in the mainline Linux kernel (and its derivative
> distribution kernels such as the MeeGo kernel for instance) all the code
> needed to access and use the hardware must be open-source.
>
> This of course doesn't preclude you from providing a closed-source
> userspace
> implementation of proprietary hardware-assisted image processing
> algorithms
> for instance (as a library or as a daemon).
>
>> In the meantime, Laurent, can you share with your major concern about
>> the
>> Daemon?
>
> I have two concerns.
>
> - The daemon makes code required to use the hardware closed-source, making
> the
> driver closed-source (whether the kernel-side code is licensed under the
> GPL
> or not is irrelevant). I would have the exact same opinion if the required
> userspace proprietary code was provided as a library, so this concern is
> not
> specific to the implementation being in the form of a daemon.
>
> - The daemon makes the kernel-side driver architecture more complex for no
> reason. Assuming you can make all the driver open-source in the future and
> want to keep proprietary userspace image processing code closed-source,
> the
> driver architecture must not be designed solely to support that use case.
> The
> driver should be clean and lean, and the proprietary code must then come
> as a
> user of the driver, not the other way around.
>
> As a summary, having part of the driver closed-source is a no-go, and
> having
> part of the kernel driver API designed and used to support closed-source
> components only is a no-go as well.

I don't entirely understand the whole discussion: in Helsinki this was
discussed extensively with Jeff Zhong and I thought we all agreed on how
to implement the driver: using a libv4l plugin which communicates to a
daemon for your proprietary ISP code.

Also, the driver should work without any proprietary code (obviously with
sub-optimal picture quality, but you should get something out of the
hardware). The driver API should be well-documented when it comes to any
custom ioctls/controls. This makes it possible for sufficiently motivated
developers to write open source libv4l plugins. It was my understanding
that the proprietary code was about determining the optimal ISP settings,
not about getting the ISP hardware to work.

Regards,

        Hans

>
> --
> Regards,
>
> Laurent Pinchart
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Sakari Ailus]

Hello,

Shuzhen Wang wrote:
>> -----Original Message-----
>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>> Sent: Tuesday, December 28, 2010 12:24 PM
>> To: Shuzhen Wang
>> Cc: 'Mauro Carvalho Chehab'; 'Hans Verkuil'; linux-
>> media@vger.kernel.org; hzhong@codeaurora.org; Yan, Yupeng
>> Subject: Re: RFC: V4L2 driver for Qualcomm MSM camera.
>>
>> I will strongly NAK any implementation that requires a daemon.
>>
> 
> We understand the motivation behind making the daemon optional.
> However there are restrictions from legal perspective, which we
> don't know how to get around.
> 
> A simplest video streaming data flow with MSM ISP is like this:
> 
> Sensor -> ISP Hardware pipeline -> videobuf
> 
> The procedure to set up ISP pipeline is proprietary and cannot
> be open sourced. Without proper pipeline configuration, streaming
> won't work. And That's why we require the daemon. 

Why not? In my opinion the pipeline configuration is quite basic
functionality present also in the OMAP3 ISP, for example. This should be
available through the Media controller interface.

In general, the driver should expose APIs best suited for configuring
the ISP hardware. Existing APIs should be used as much as possible.
There should be no untyped blob passing in its API.

Regards,

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

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Mauro Carvalho Chehab]

Em 04-01-2011 08:40, Hans Verkuil escreveu:
>> Hi Jeff,
>>
>> On Tuesday 04 January 2011 07:14:00 Haibo Zhong wrote:
>>> On 1/3/2011 6:37 PM, Shuzhen Wang wrote:
>>>> On Tuesday, December 28, 2010 12:24 PM Laurent Pinchart wrote:
>>>>>
>>>>> I will strongly NAK any implementation that requires a daemon.
>>>>
>>>> We understand the motivation behind making the daemon optional.
>>>> However there are restrictions from legal perspective, which we
>>>> don't know how to get around.
>>>>
>>>> A simplest video streaming data flow with MSM ISP is like this:
>>>>
>>>> Sensor ->  ISP Hardware pipeline ->  videobuf
>>>>
>>>> The procedure to set up ISP pipeline is proprietary and cannot
>>>> be open sourced. Without proper pipeline configuration, streaming
>>>> won't work. And That's why we require the daemon.
>>>
>>> Laurent/Hans/Mauro,
>>>
>>> We are working on and will provide more design information on Qualcomm
>>> MSM ISP design and explain the legal concern of the daemon
>>> implementation.
>>>
>>> The underlined idea is to comply to V4L2 architecture with MSM solution.
>>
>> That's a good first step, but I'm afraid it's not enough. If you want your
>> driver to be included in the mainline Linux kernel (and its derivative
>> distribution kernels such as the MeeGo kernel for instance) all the code
>> needed to access and use the hardware must be open-source.
>>
>> This of course doesn't preclude you from providing a closed-source
>> userspace
>> implementation of proprietary hardware-assisted image processing
>> algorithms
>> for instance (as a library or as a daemon).
>>
>>> In the meantime, Laurent, can you share with your major concern about
>>> the
>>> Daemon?
>>
>> I have two concerns.
>>
>> - The daemon makes code required to use the hardware closed-source, making
>> the
>> driver closed-source (whether the kernel-side code is licensed under the
>> GPL
>> or not is irrelevant). I would have the exact same opinion if the required
>> userspace proprietary code was provided as a library, so this concern is
>> not
>> specific to the implementation being in the form of a daemon.
>>
>> - The daemon makes the kernel-side driver architecture more complex for no
>> reason. Assuming you can make all the driver open-source in the future and
>> want to keep proprietary userspace image processing code closed-source,
>> the
>> driver architecture must not be designed solely to support that use case.
>> The
>> driver should be clean and lean, and the proprietary code must then come
>> as a
>> user of the driver, not the other way around.
>>
>> As a summary, having part of the driver closed-source is a no-go, and
>> having
>> part of the kernel driver API designed and used to support closed-source
>> components only is a no-go as well.
> 
> I don't entirely understand the whole discussion: in Helsinki this was
> discussed extensively with Jeff Zhong and I thought we all agreed on how
> to implement the driver: using a libv4l plugin which communicates to a
> daemon for your proprietary ISP code.

I also had the same understanding from Helsinki's meeting.

> Also, the driver should work without any proprietary code (obviously with
> sub-optimal picture quality, but you should get something out of the
> hardware). The driver API should be well-documented when it comes to any
> custom ioctls/controls. This makes it possible for sufficiently motivated
> developers to write open source libv4l plugins. It was my understanding
> that the proprietary code was about determining the optimal ISP settings,
> not about getting the ISP hardware to work.

Look: we don't mind if you have some daemon/library/userspace utils with some
picture enhancement algorithm that you may have developed and have legal
concerns to protect them. The best way for doing that is probably via libv4l,
as other userspace apps will use it, but you can do whatever you want.

However, the driver should be open source for being accepted upstream. 
In other words, all access to the hardware should be done via the open 
source driver, and using a public, well documented, kernel<=>userspace API.
That means that any application should be able to use it, without requiring
any daemon.

So, it is OK if your driver have some well-documented ways to expose image
statistics and some way to adjust the device parameters to enhance image
quality, and have some proprietary, closed source daemon/library that will 
use those statistics to enhance the image. Of course, such driver, to be open,
should allow someone to write a different code to do similar enhancements,
as Hans pointed.

That's said, I can't understand why you're afraid of doing that. Hardware 
initialization is very specific to a given hardware, and I don't see how
open the initialization sequence for your hardware would cause any legal
damage, or benefit your competitors: It is just a sequence of reads/writes. 
All drivers do it. The IP that you likely want to protect is the way you 
design your hardware and the image algorithms that you have to produce a 
clearer image. This won't be exposed by an open-source hardware, for 
two reasons:
	1) They are not needed to make the driver work;
	2) It is not a kernel task to do image conversion, handle auto-focus,
do face track, do white balancing, etc. The kernel driver should just expose
whatever feature the hardware have and let some userspace program to use it.

So, if the hardware has white balancing, the driver should provide a control
for the application to turn it on or off. 

With libv4l, you can even add a software white balancing control, that could,
for example, read some hardware-provided statistics and use some red/green/blue 
gains and/or other hardware-provided controls. As discussed in Helsinki's meeting, 
libv4l can even call some proprietary code to actually implement such logic.

Mauro.

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Markus Rechberger]

On Tue, Jan 4, 2011 at 3:29 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 04-01-2011 08:40, Hans Verkuil escreveu:
>>> Hi Jeff,
>>>
>>> On Tuesday 04 January 2011 07:14:00 Haibo Zhong wrote:
>>>> On 1/3/2011 6:37 PM, Shuzhen Wang wrote:
>>>>> On Tuesday, December 28, 2010 12:24 PM Laurent Pinchart wrote:
>>>>>>
>>>>>> I will strongly NAK any implementation that requires a daemon.
>>>>>
>>>>> We understand the motivation behind making the daemon optional.
>>>>> However there are restrictions from legal perspective, which we
>>>>> don't know how to get around.
>>>>>
>>>>> A simplest video streaming data flow with MSM ISP is like this:
>>>>>
>>>>> Sensor ->  ISP Hardware pipeline ->  videobuf
>>>>>
>>>>> The procedure to set up ISP pipeline is proprietary and cannot
>>>>> be open sourced. Without proper pipeline configuration, streaming
>>>>> won't work. And That's why we require the daemon.
>>>>
>>>> Laurent/Hans/Mauro,
>>>>
>>>> We are working on and will provide more design information on Qualcomm
>>>> MSM ISP design and explain the legal concern of the daemon
>>>> implementation.
>>>>
>>>> The underlined idea is to comply to V4L2 architecture with MSM solution.
>>>
>>> That's a good first step, but I'm afraid it's not enough. If you want your
>>> driver to be included in the mainline Linux kernel (and its derivative
>>> distribution kernels such as the MeeGo kernel for instance) all the code
>>> needed to access and use the hardware must be open-source.
>>>
>>> This of course doesn't preclude you from providing a closed-source
>>> userspace
>>> implementation of proprietary hardware-assisted image processing
>>> algorithms
>>> for instance (as a library or as a daemon).
>>>
>>>> In the meantime, Laurent, can you share with your major concern about
>>>> the
>>>> Daemon?
>>>
>>> I have two concerns.
>>>
>>> - The daemon makes code required to use the hardware closed-source, making
>>> the
>>> driver closed-source (whether the kernel-side code is licensed under the
>>> GPL
>>> or not is irrelevant). I would have the exact same opinion if the required
>>> userspace proprietary code was provided as a library, so this concern is
>>> not
>>> specific to the implementation being in the form of a daemon.
>>>
>>> - The daemon makes the kernel-side driver architecture more complex for no
>>> reason. Assuming you can make all the driver open-source in the future and
>>> want to keep proprietary userspace image processing code closed-source,
>>> the
>>> driver architecture must not be designed solely to support that use case.
>>> The
>>> driver should be clean and lean, and the proprietary code must then come
>>> as a
>>> user of the driver, not the other way around.
>>>
>>> As a summary, having part of the driver closed-source is a no-go, and
>>> having
>>> part of the kernel driver API designed and used to support closed-source
>>> components only is a no-go as well.
>>
>> I don't entirely understand the whole discussion: in Helsinki this was
>> discussed extensively with Jeff Zhong and I thought we all agreed on how
>> to implement the driver: using a libv4l plugin which communicates to a
>> daemon for your proprietary ISP code.
>
> I also had the same understanding from Helsinki's meeting.
>
>> Also, the driver should work without any proprietary code (obviously with
>> sub-optimal picture quality, but you should get something out of the
>> hardware). The driver API should be well-documented when it comes to any
>> custom ioctls/controls. This makes it possible for sufficiently motivated
>> developers to write open source libv4l plugins. It was my understanding
>> that the proprietary code was about determining the optimal ISP settings,
>> not about getting the ISP hardware to work.
>
> Look: we don't mind if you have some daemon/library/userspace utils with some
> picture enhancement algorithm that you may have developed and have legal
> concerns to protect them. The best way for doing that is probably via libv4l,
> as other userspace apps will use it, but you can do whatever you want.
>
> However, the driver should be open source for being accepted upstream.
> In other words, all access to the hardware should be done via the open
> source driver, and using a public, well documented, kernel<=>userspace API.
> That means that any application should be able to use it, without requiring
> any daemon.
>
> So, it is OK if your driver have some well-documented ways to expose image
> statistics and some way to adjust the device parameters to enhance image
> quality, and have some proprietary, closed source daemon/library that will
> use those statistics to enhance the image. Of course, such driver, to be open,
> should allow someone to write a different code to do similar enhancements,
> as Hans pointed.
>
> That's said, I can't understand why you're afraid of doing that.

Multiple reasons
1. They want to control their drivers and not have to take care about
the kernel release cycles.
2. With a thin kernel layer (if needed) they can still modify the
hardware and just need
to update the userspace part of it. This will make their product
better supported accross different
kernelversions.
3. easier debugging of course
4. things are going userspace.. see PCI/e virtualization, drivers can
be in Userspace which
also increases the system stability (and portability - my comment it's
easier to port userspace
drivers to other operating systems than kernelspace drivers we already
did that).

(note I'm not addressing opensource or closed source here, just the
advantage of userspace drivers).

Some people who just entered the year 1990 a few days ago don't see
those advantages maybe :-)

Markus

> Hardware
> initialization is very specific to a given hardware, and I don't see how
> open the initialization sequence for your hardware would cause any legal
> damage, or benefit your competitors: It is just a sequence of reads/writes.
> All drivers do it. The IP that you likely want to protect is the way you
> design your hardware and the image algorithms that you have to produce a
> clearer image. This won't be exposed by an open-source hardware, for
> two reasons:
>        1) They are not needed to make the driver work;
>        2) It is not a kernel task to do image conversion, handle auto-focus,
> do face track, do white balancing, etc. The kernel driver should just expose
> whatever feature the hardware have and let some userspace program to use it.
>
> So, if the hardware has white balancing, the driver should provide a control
> for the application to turn it on or off.
>
> With libv4l, you can even add a software white balancing control, that could,
> for example, read some hardware-provided statistics and use some red/green/blue
> gains and/or other hardware-provided controls. As discussed in Helsinki's meeting,
> libv4l can even call some proprietary code to actually implement such logic.
>
> Mauro.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: RFC: V4L2 driver for Qualcomm MSM camera.

[Yan, Yupeng]

We will exploring the usage of libv4l2...however we still have the difficulties to open-source hardware: our ISP and sensors, will need help on how to address such issues.

-----Original Message-----
From: Markus Rechberger [mailto:mrechberger@gmail.com] 
Sent: Tuesday, January 04, 2011 10:08 AM
To: Mauro Carvalho Chehab
Cc: Hans Verkuil; Laurent Pinchart; Haibo Zhong; Shuzhen Wang; linux-media@vger.kernel.org; Yan, Yupeng; Zhong, Jeff
Subject: Re: RFC: V4L2 driver for Qualcomm MSM camera.

On Tue, Jan 4, 2011 at 3:29 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 04-01-2011 08:40, Hans Verkuil escreveu:
>>> Hi Jeff,
>>>
>>> On Tuesday 04 January 2011 07:14:00 Haibo Zhong wrote:
>>>> On 1/3/2011 6:37 PM, Shuzhen Wang wrote:
>>>>> On Tuesday, December 28, 2010 12:24 PM Laurent Pinchart wrote:
>>>>>>
>>>>>> I will strongly NAK any implementation that requires a daemon.
>>>>>
>>>>> We understand the motivation behind making the daemon optional.
>>>>> However there are restrictions from legal perspective, which we
>>>>> don't know how to get around.
>>>>>
>>>>> A simplest video streaming data flow with MSM ISP is like this:
>>>>>
>>>>> Sensor ->  ISP Hardware pipeline ->  videobuf
>>>>>
>>>>> The procedure to set up ISP pipeline is proprietary and cannot
>>>>> be open sourced. Without proper pipeline configuration, streaming
>>>>> won't work. And That's why we require the daemon.
>>>>
>>>> Laurent/Hans/Mauro,
>>>>
>>>> We are working on and will provide more design information on Qualcomm
>>>> MSM ISP design and explain the legal concern of the daemon
>>>> implementation.
>>>>
>>>> The underlined idea is to comply to V4L2 architecture with MSM solution.
>>>
>>> That's a good first step, but I'm afraid it's not enough. If you want your
>>> driver to be included in the mainline Linux kernel (and its derivative
>>> distribution kernels such as the MeeGo kernel for instance) all the code
>>> needed to access and use the hardware must be open-source.
>>>
>>> This of course doesn't preclude you from providing a closed-source
>>> userspace
>>> implementation of proprietary hardware-assisted image processing
>>> algorithms
>>> for instance (as a library or as a daemon).
>>>
>>>> In the meantime, Laurent, can you share with your major concern about
>>>> the
>>>> Daemon?
>>>
>>> I have two concerns.
>>>
>>> - The daemon makes code required to use the hardware closed-source, making
>>> the
>>> driver closed-source (whether the kernel-side code is licensed under the
>>> GPL
>>> or not is irrelevant). I would have the exact same opinion if the required
>>> userspace proprietary code was provided as a library, so this concern is
>>> not
>>> specific to the implementation being in the form of a daemon.
>>>
>>> - The daemon makes the kernel-side driver architecture more complex for no
>>> reason. Assuming you can make all the driver open-source in the future and
>>> want to keep proprietary userspace image processing code closed-source,
>>> the
>>> driver architecture must not be designed solely to support that use case.
>>> The
>>> driver should be clean and lean, and the proprietary code must then come
>>> as a
>>> user of the driver, not the other way around.
>>>
>>> As a summary, having part of the driver closed-source is a no-go, and
>>> having
>>> part of the kernel driver API designed and used to support closed-source
>>> components only is a no-go as well.
>>
>> I don't entirely understand the whole discussion: in Helsinki this was
>> discussed extensively with Jeff Zhong and I thought we all agreed on how
>> to implement the driver: using a libv4l plugin which communicates to a
>> daemon for your proprietary ISP code.
>
> I also had the same understanding from Helsinki's meeting.
>
>> Also, the driver should work without any proprietary code (obviously with
>> sub-optimal picture quality, but you should get something out of the
>> hardware). The driver API should be well-documented when it comes to any
>> custom ioctls/controls. This makes it possible for sufficiently motivated
>> developers to write open source libv4l plugins. It was my understanding
>> that the proprietary code was about determining the optimal ISP settings,
>> not about getting the ISP hardware to work.
>
> Look: we don't mind if you have some daemon/library/userspace utils with some
> picture enhancement algorithm that you may have developed and have legal
> concerns to protect them. The best way for doing that is probably via libv4l,
> as other userspace apps will use it, but you can do whatever you want.
>
> However, the driver should be open source for being accepted upstream.
> In other words, all access to the hardware should be done via the open
> source driver, and using a public, well documented, kernel<=>userspace API.
> That means that any application should be able to use it, without requiring
> any daemon.
>
> So, it is OK if your driver have some well-documented ways to expose image
> statistics and some way to adjust the device parameters to enhance image
> quality, and have some proprietary, closed source daemon/library that will
> use those statistics to enhance the image. Of course, such driver, to be open,
> should allow someone to write a different code to do similar enhancements,
> as Hans pointed.
>
> That's said, I can't understand why you're afraid of doing that.

Multiple reasons
1. They want to control their drivers and not have to take care about
the kernel release cycles.
2. With a thin kernel layer (if needed) they can still modify the
hardware and just need
to update the userspace part of it. This will make their product
better supported accross different
kernelversions.
3. easier debugging of course
4. things are going userspace.. see PCI/e virtualization, drivers can
be in Userspace which
also increases the system stability (and portability - my comment it's
easier to port userspace
drivers to other operating systems than kernelspace drivers we already
did that).

(note I'm not addressing opensource or closed source here, just the
advantage of userspace drivers).

Some people who just entered the year 1990 a few days ago don't see
those advantages maybe :-)

Markus

> Hardware
> initialization is very specific to a given hardware, and I don't see how
> open the initialization sequence for your hardware would cause any legal
> damage, or benefit your competitors: It is just a sequence of reads/writes.
> All drivers do it. The IP that you likely want to protect is the way you
> design your hardware and the image algorithms that you have to produce a
> clearer image. This won't be exposed by an open-source hardware, for
> two reasons:
>        1) They are not needed to make the driver work;
>        2) It is not a kernel task to do image conversion, handle auto-focus,
> do face track, do white balancing, etc. The kernel driver should just expose
> whatever feature the hardware have and let some userspace program to use it.
>
> So, if the hardware has white balancing, the driver should provide a control
> for the application to turn it on or off.
>
> With libv4l, you can even add a software white balancing control, that could,
> for example, read some hardware-provided statistics and use some red/green/blue
> gains and/or other hardware-provided controls. As discussed in Helsinki's meeting,
> libv4l can even call some proprietary code to actually implement such logic.
>
> Mauro.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Laurent Pinchart]

Hi,

On Tuesday 04 January 2011 20:37:31 Yan, Yupeng wrote:
> We will exploring the usage of libv4l2...however we still have the
> difficulties to open-source hardware: our ISP and sensors, will need help
> on how to address such issues.

I suppose you mean open-sourcing hardware driver. There's no requirement to 
open-source the hardware at this point, although I would love to see that 
happening :-)

Let's start with sensors, it's the easiest. Sensor drivers must not depend on 
the ISP driver, and the ISP driver must not depend on sensor drivers. They 
communicate together through the media controller and the V4L2 subdev APIs.

You will obviously need to test your ISP driver with at least one sensor, but 
you're not required to submit the sensor driver along with the ISP driver 
(although it would be nice if you did so).

Correct me if I'm wrong, but I don't think Qualcomm produces video sensors, so 
no Qualcomm confidential information would be disclosed in a sensor driver. If 
the sensor manufacturer hasn't released a sensor driver, or sensor 
specifications covered by a license that allows a GPL driver to be written, 
you will need to sort it out with the manufacturer. Note that public 
documentation is not strictly required to release an open-source driver (see 
the SMIA++ driver written by Nokia for instance), but that would be 
appreciated.

For the ISP the problem is not that complex either. From what I understand 
Qualcomm is scared that publishing an open-source driver will backfire. I can 
see several reasons (I should probably say myths instead of reasons) for that 
fear:

- "Competitors will steal my code". They can't. The code is highly hardware 
specific and doesn't contain much added value. It can't be reused as such. 
Even if code could be reused, it couldn't be "stolen" as it will be covered by 
the GPLv2.

- "Competitors will get key information about my hardware". You need to ask 
yourself what you consider as key information. The fact that the output width 
is stored in register 0x12345678 is hardly key information (if it is, there's 
a more basic issue). The way statistics are computed might be. Based on my 
experience with ISPs key information isn't disclosed by the driver. The 
hardware algorithms (such as color conversion, faulty pixels correction, 
statistics gathering, ...) implementation are usually not disclosed by the 
code.

- "Competitors will sue me for patent infringement". This goes along with the 
previous point. Patented ISP features are usually the ones you consider as key 
information, and details are thus not disclosed by the driver. Note that I 
don't care here about trivial patents with broad claims such as "a system to 
capture an image to memory". Even without any open-source driver infringement 
can still easily be proven (and the patent can easily be invalidated, although 
that costs money - Qualcomm should join the Open Invention Network :-)).

None of those reasons are valid compared to the benefits you get from an open-
source Linux driver.

-- 
Regards,

Laurent Pinchart

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Yupeng Yan]

Thanks for the comments - certainly good arguments to our lawyers :-)... 
actually the information of how to config the ISP HW is requested to be 
protected for now, we are working on certain degree of openness.

The HW config code is just a small part of the user space (daemon) 
tasks, the user space code also processes ISP states, carries out 3A 
algorithm and perform post processing features, etc. which will have to 
be protected, this is the reason why the daemon is used in QC solution.


On 1/4/2011 4:15 PM, Laurent Pinchart wrote:
> Hi,
>
> On Tuesday 04 January 2011 20:37:31 Yan, Yupeng wrote:
>> We will exploring the usage of libv4l2...however we still have the
>> difficulties to open-source hardware: our ISP and sensors, will need help
>> on how to address such issues.
> I suppose you mean open-sourcing hardware driver. There's no requirement to
> open-source the hardware at this point, although I would love to see that
> happening :-)
>
> Let's start with sensors, it's the easiest. Sensor drivers must not depend on
> the ISP driver, and the ISP driver must not depend on sensor drivers. They
> communicate together through the media controller and the V4L2 subdev APIs.
>
> You will obviously need to test your ISP driver with at least one sensor, but
> you're not required to submit the sensor driver along with the ISP driver
> (although it would be nice if you did so).
>
> Correct me if I'm wrong, but I don't think Qualcomm produces video sensors, so
> no Qualcomm confidential information would be disclosed in a sensor driver. If
> the sensor manufacturer hasn't released a sensor driver, or sensor
> specifications covered by a license that allows a GPL driver to be written,
> you will need to sort it out with the manufacturer. Note that public
> documentation is not strictly required to release an open-source driver (see
> the SMIA++ driver written by Nokia for instance), but that would be
> appreciated.
>
> For the ISP the problem is not that complex either. From what I understand
> Qualcomm is scared that publishing an open-source driver will backfire. I can
> see several reasons (I should probably say myths instead of reasons) for that
> fear:
>
> - "Competitors will steal my code". They can't. The code is highly hardware
> specific and doesn't contain much added value. It can't be reused as such.
> Even if code could be reused, it couldn't be "stolen" as it will be covered by
> the GPLv2.
>
> - "Competitors will get key information about my hardware". You need to ask
> yourself what you consider as key information. The fact that the output width
> is stored in register 0x12345678 is hardly key information (if it is, there's
> a more basic issue). The way statistics are computed might be. Based on my
> experience with ISPs key information isn't disclosed by the driver. The
> hardware algorithms (such as color conversion, faulty pixels correction,
> statistics gathering, ...) implementation are usually not disclosed by the
> code.
>
> - "Competitors will sue me for patent infringement". This goes along with the
> previous point. Patented ISP features are usually the ones you consider as key
> information, and details are thus not disclosed by the driver. Note that I
> don't care here about trivial patents with broad claims such as "a system to
> capture an image to memory". Even without any open-source driver infringement
> can still easily be proven (and the patent can easily be invalidated, although
> that costs money - Qualcomm should join the Open Invention Network :-)).
>
> None of those reasons are valid compared to the benefits you get from an open-
> source Linux driver.
>

Re: RFC: V4L2 driver for Qualcomm MSM camera.

[Laurent Pinchart]

Hi,

On Friday 07 January 2011 01:03:58 Yupeng Yan wrote:
> Thanks for the comments - certainly good arguments to our lawyers :-)...
> actually the information of how to config the ISP HW is requested to be
> protected for now, we are working on certain degree of openness.
> 
> The HW config code is just a small part of the user space (daemon)
> tasks, the user space code also processes ISP states, carries out 3A
> algorithm and perform post processing features, etc. which will have to
> be protected, this is the reason why the daemon is used in QC solution.

That's fine. Having closed-source userspace 3A and post-processing isn't an 
issue as far as the Linux driver is concerned.

We have the exact same requirements for the OMAP3. The 3A and image quality 
enhancement algorithms are implemented in userspace, in a library that can be 
used by applications (currently either through an open-source LGPL GStreamer 
element that links to the closed-source library, or through a libv4l plugin).

There is no legal need to implement this in a daemon. LGPL software can be 
linked to closed-source libraries, there is no risk there. If you insist on 
implementing a daemon (I really can't understand why that would be needed), 
the best solution would be to provide a library that applications can use to 
communicate with the daemon. That library (and the daemon) should handle 3A 
and post-processing tasks only. All ISP control (configuring the pipeline(s), 
starting/stopping the stream(s), handling the video buffers, getting/setting 
controls, reading statistics data, ...) must be implemented in the open-source 
kernel driver.

-- 
Regards,

Laurent Pinchart