Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver

[Mauro Carvalho Chehab <mchehab@redhat.com>]

Hi Laurent,

Em 17-02-2011 13:06, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> The following changes since commit 85e2efbb1db9a18d218006706d6e4fbeb0216213:
> 
>   Linux 2.6.38-rc5 (2011-02-15 19:23:45 -0800)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/media.git media-0005-omap3isp

I've added the patches that looked ok on my eyes at:

http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/media_controller

There are just a few small adjustments on a few of them, as I've commented.
I prefer if you do them on separate patches, to save my work of not needing
to review the entire series again.

The ones still pending on my quilt tree are:

0030-v4l-subdev-Generic-ioctl-support.patch
0040-omap3isp-OMAP3-ISP-core.patch
0041-omap3isp-Video-devices-and-buffers-queue.patch
0042-omap3isp-CCP2-CSI2-receivers.patch
0043-omap3isp-CCDC-preview-engine-and-resizer.patch
0044-omap3isp-Statistics.patch
0045-omap3isp-Kconfig-and-Makefile.patch
0046-omap3isp-Add-set-performance-callback-in-isp-platfor.patch

with the following diffstat:

 Documentation/video4linux/v4l2-framework.txt       |    5 +
 MAINTAINERS                                        |    6 +
 drivers/media/video/Kconfig                        |   13 +
 drivers/media/video/Makefile                       |    2 +
 drivers/media/video/omap3-isp/Makefile             |   13 +
 drivers/media/video/omap3-isp/cfa_coef_table.h     |   61 +
 drivers/media/video/omap3-isp/gamma_table.h        |   90 +
 drivers/media/video/omap3-isp/isp.c                | 2220 +++++++++++++++++++
 drivers/media/video/omap3-isp/isp.h                |  428 ++++
 drivers/media/video/omap3-isp/ispccdc.c            | 2268 ++++++++++++++++++++
 drivers/media/video/omap3-isp/ispccdc.h            |  219 ++
 drivers/media/video/omap3-isp/ispccp2.c            | 1173 ++++++++++
 drivers/media/video/omap3-isp/ispccp2.h            |   98 +
 drivers/media/video/omap3-isp/ispcsi2.c            | 1317 ++++++++++++
 drivers/media/video/omap3-isp/ispcsi2.h            |  166 ++
 drivers/media/video/omap3-isp/ispcsiphy.c          |  247 +++
 drivers/media/video/omap3-isp/ispcsiphy.h          |   74 +
 drivers/media/video/omap3-isp/isph3a.h             |  117 +
 drivers/media/video/omap3-isp/isph3a_aewb.c        |  374 ++++
 drivers/media/video/omap3-isp/isph3a_af.c          |  429 ++++
 drivers/media/video/omap3-isp/isphist.c            |  520 +++++
 drivers/media/video/omap3-isp/isphist.h            |   40 +
 drivers/media/video/omap3-isp/isppreview.c         | 2113 ++++++++++++++++++
 drivers/media/video/omap3-isp/isppreview.h         |  214 ++
 drivers/media/video/omap3-isp/ispqueue.c           | 1153 ++++++++++
 drivers/media/video/omap3-isp/ispqueue.h           |  187 ++
 drivers/media/video/omap3-isp/ispreg.h             | 1589 ++++++++++++++
 drivers/media/video/omap3-isp/ispresizer.c         | 1693 +++++++++++++++
 drivers/media/video/omap3-isp/ispresizer.h         |  147 ++
 drivers/media/video/omap3-isp/ispstat.c            | 1092 ++++++++++
 drivers/media/video/omap3-isp/ispstat.h            |  169 ++
 drivers/media/video/omap3-isp/ispvideo.c           | 1255 +++++++++++
 drivers/media/video/omap3-isp/ispvideo.h           |  202 ++
 drivers/media/video/omap3-isp/luma_enhance_table.h |   42 +
 drivers/media/video/omap3-isp/noise_filter_table.h |   30 +
 drivers/media/video/v4l2-subdev.c                  |    2 +-
 drivers/media/video/videobuf-dma-contig.c          |    2 +-
 include/linux/Kbuild                               |    1 +
 38 files changed, 19769 insertions(+), 2 deletions(-)

I used quilt for all patches, except for the one patch with some gifs, where I did a
git cherry-pick. So, the imported patches should be ok. Of course, it doesn't hurt
do double check.

The main issue with the omap3isp is due to the presence of private ioctl's that
I don't have a clear idea about what they are really doing. 

I couldn't see any documentation about them on a very quick look. While I suspect
that they are used only for 3A, I have no means of being sure about that. 

Also, as I've said several times, while I don't like, I have nothing against 
having some ioctls that would be used by a vendor to implement their own 3A software 
algorithms that he may need to hide for some reason or have any patents applied to
the algorithm, but only if:
	1) such algorithms are implemented on userspace;
	2) the userspace API used by them is fully documented, in order
to allow that someone else with enough motivation and spare time may
want to implement his own algorithm (including an open-source one);
	3) there are no patents denying or charging for the usage and/or
distribution/redistribution of the Kernel with the provided kernel driver;
	4) if the device works with a reasonable quality without them
(by reasonable I mean like a cheap webcam, where libv4l could use his
set of 3A algorithms to provide a good quality).

Assuming that all those private ioctl's are really for 3A, it is ok for me
to accept such ioctls after being sure that the above applies. I'm not sure
how to check (4), as, while I have 2 omap boards here (a Beagleboard and a
gumstix), none of them have any sensor.


Cheers,
Mauro.