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

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

[Laurent Pinchart]

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

Antti Koskipaa (1):
      v4l: v4l2_subdev userspace crop API

David Cohen (1):
      omap3isp: Statistics

Laurent Pinchart (36):
      v4l: Share code between video_usercopy and video_ioctl2
      v4l: subdev: Don't require core operations
      v4l: subdev: Add device node support
      v4l: subdev: Uninline the v4l2_subdev_init function
      v4l: subdev: Control ioctls support
      media: Media device node support
      media: Media device
      media: Entities, pads and links
      media: Entity use count
      media: Media device information query
      media: Entities, pads and links enumeration
      media: Links setup
      media: Pipelines and media streams
      v4l: Add a media_device pointer to the v4l2_device structure
      v4l: Make video_device inherit from media_entity
      v4l: Make v4l2_subdev inherit from media_entity
      v4l: Move the media/v4l2-mediabus.h header to include/linux
      v4l: Replace enums with fixed-sized fields in public structure
      v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
      v4l: Group media bus pixel codes by types and sort them alphabetically
      v4l: subdev: Add new file operations
      v4l: v4l2_subdev pad-level operations
      v4l: v4l2_subdev userspace format API - documentation binary files
      v4l: v4l2_subdev userspace format API
      v4l: v4l2_subdev userspace frame interval API
      v4l: subdev: Generic ioctl support
      v4l: Add subdev sensor g_skip_frames operation
      v4l: Add 8-bit YUYV on 16-bit bus and SGRBG10 media bus pixel codes
      v4l: Add remaining RAW10 patterns w DPCM pixel code variants
      v4l: Add missing 12 bits bayer media bus formats
      v4l: Add 12 bits bayer pixel formats
      omap3: Add function to register omap3isp platform device structure
      omap3isp: Video devices and buffers queue
      omap3isp: CCP2/CSI2 receivers
      omap3isp: CCDC, preview engine and resizer
      omap3isp: Kconfig and Makefile

Sakari Ailus (3):
      v4l: subdev: Events support
      media: Entity graph traversal
      omap3isp: OMAP3 ISP core

Sergio Aguirre (2):
      omap3: Remove unusued ISP CBUFF resource
      omap2: Fix camera resources for multiomap

Stanimir Varbanov (1):
      v4l: Create v4l2 subdev file handle structure

Tuukka Toivonen (1):
      ARM: OMAP3: Update Camera ISP definitions for OMAP3630

 Documentation/ABI/testing/sysfs-bus-media          |    6 +
 Documentation/DocBook/Makefile                     |    5 +-
 Documentation/DocBook/media-entities.tmpl          |   50 +
 Documentation/DocBook/media.tmpl                   |    3 +
 Documentation/DocBook/v4l/bayer.pdf                |  Bin 0 -> 12116 bytes
 Documentation/DocBook/v4l/bayer.png                |  Bin 0 -> 9725 bytes
 Documentation/DocBook/v4l/dev-subdev.xml           |  313 +++
 Documentation/DocBook/v4l/media-controller.xml     |   89 +
 Documentation/DocBook/v4l/media-func-close.xml     |   59 +
 Documentation/DocBook/v4l/media-func-ioctl.xml     |  116 +
 Documentation/DocBook/v4l/media-func-open.xml      |   94 +
 .../DocBook/v4l/media-ioc-device-info.xml          |  133 ++
 .../DocBook/v4l/media-ioc-enum-entities.xml        |  308 +++
 Documentation/DocBook/v4l/media-ioc-enum-links.xml |  207 ++
 Documentation/DocBook/v4l/media-ioc-setup-link.xml |   93 +
 Documentation/DocBook/v4l/pipeline.pdf             |  Bin 0 -> 20276 bytes
 Documentation/DocBook/v4l/pipeline.png             |  Bin 0 -> 12130 bytes
 Documentation/DocBook/v4l/subdev-formats.xml       | 2467 
++++++++++++++++++++
 Documentation/DocBook/v4l/v4l2.xml                 |    7 +
 Documentation/DocBook/v4l/vidioc-streamon.xml      |    9 +
 .../v4l/vidioc-subdev-enum-frame-interval.xml      |  152 ++
 .../DocBook/v4l/vidioc-subdev-enum-frame-size.xml  |  154 ++
 .../DocBook/v4l/vidioc-subdev-enum-mbus-code.xml   |  119 +
 Documentation/DocBook/v4l/vidioc-subdev-g-crop.xml |  155 ++
 Documentation/DocBook/v4l/vidioc-subdev-g-fmt.xml  |  180 ++
 .../DocBook/v4l/vidioc-subdev-g-frame-interval.xml |  141 ++
 Documentation/media-framework.txt                  |  353 +++
 Documentation/video4linux/v4l2-framework.txt       |  127 +-
 MAINTAINERS                                        |    6 +
 arch/arm/mach-omap2/devices.c                      |   63 +-
 arch/arm/mach-omap2/devices.h                      |   19 +
 arch/arm/plat-omap/include/plat/omap34xx.h         |   16 +-
 drivers/media/Kconfig                              |   22 +
 drivers/media/Makefile                             |    6 +
 drivers/media/media-device.c                       |  382 +++
 drivers/media/media-devnode.c                      |  321 +++
 drivers/media/media-entity.c                       |  536 +++++
 drivers/media/video/Kconfig                        |   13 +
 drivers/media/video/Makefile                       |    4 +-
 drivers/media/video/mt9m001.c                      |    2 +-
 drivers/media/video/mt9v022.c                      |    4 +-
 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                |  427 ++++
 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           | 1264 ++++++++++
 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/ov6650.c                       |   10 +-
 drivers/media/video/sh_mobile_csi2.c               |    6 +-
 drivers/media/video/soc_mediabus.c                 |    2 +-
 drivers/media/video/v4l2-dev.c                     |   76 +-
 drivers/media/video/v4l2-device.c                  |   84 +-
 drivers/media/video/v4l2-ioctl.c                   |  216 +--
 drivers/media/video/v4l2-subdev.c                  |  332 +++
 include/linux/Kbuild                               |    4 +
 include/linux/media.h                              |  132 ++
 include/linux/omap3isp.h                           |  646 +++++
 include/linux/v4l2-mediabus.h                      |  108 +
 include/linux/v4l2-subdev.h                        |  141 ++
 include/linux/videodev2.h                          |    4 +
 include/media/media-device.h                       |   95 +
 include/media/media-devnode.h                      |   97 +
 include/media/media-entity.h                       |  151 ++
 include/media/soc_mediabus.h                       |    3 +-
 include/media/v4l2-dev.h                           |   25 +-
 include/media/v4l2-device.h                        |   10 +
 include/media/v4l2-ioctl.h                         |    3 +
 include/media/v4l2-mediabus.h                      |   61 +-
 include/media/v4l2-subdev.h                        |  111 +-
 94 files changed, 28496 insertions(+), 303 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-media
 create mode 100644 Documentation/DocBook/v4l/bayer.pdf
 create mode 100644 Documentation/DocBook/v4l/bayer.png
 create mode 100644 Documentation/DocBook/v4l/dev-subdev.xml
 create mode 100644 Documentation/DocBook/v4l/media-controller.xml
 create mode 100644 Documentation/DocBook/v4l/media-func-close.xml
 create mode 100644 Documentation/DocBook/v4l/media-func-ioctl.xml
 create mode 100644 Documentation/DocBook/v4l/media-func-open.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-device-info.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-enum-entities.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-enum-links.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-setup-link.xml
 create mode 100644 Documentation/DocBook/v4l/pipeline.pdf
 create mode 100644 Documentation/DocBook/v4l/pipeline.png
 create mode 100644 Documentation/DocBook/v4l/subdev-formats.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-enum-frame-
interval.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-enum-frame-
size.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-enum-mbus-code.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-g-crop.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-g-fmt.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-g-frame-
interval.xml
 create mode 100644 Documentation/media-framework.txt
 create mode 100644 arch/arm/mach-omap2/devices.h
 create mode 100644 drivers/media/media-device.c
 create mode 100644 drivers/media/media-devnode.c
 create mode 100644 drivers/media/media-entity.c
 create mode 100644 drivers/media/video/omap3-isp/Makefile
 create mode 100644 drivers/media/video/omap3-isp/cfa_coef_table.h
 create mode 100644 drivers/media/video/omap3-isp/gamma_table.h
 create mode 100644 drivers/media/video/omap3-isp/isp.c
 create mode 100644 drivers/media/video/omap3-isp/isp.h
 create mode 100644 drivers/media/video/omap3-isp/ispccdc.c
 create mode 100644 drivers/media/video/omap3-isp/ispccdc.h
 create mode 100644 drivers/media/video/omap3-isp/ispccp2.c
 create mode 100644 drivers/media/video/omap3-isp/ispccp2.h
 create mode 100644 drivers/media/video/omap3-isp/ispcsi2.c
 create mode 100644 drivers/media/video/omap3-isp/ispcsi2.h
 create mode 100644 drivers/media/video/omap3-isp/ispcsiphy.c
 create mode 100644 drivers/media/video/omap3-isp/ispcsiphy.h
 create mode 100644 drivers/media/video/omap3-isp/isph3a.h
 create mode 100644 drivers/media/video/omap3-isp/isph3a_aewb.c
 create mode 100644 drivers/media/video/omap3-isp/isph3a_af.c
 create mode 100644 drivers/media/video/omap3-isp/isphist.c
 create mode 100644 drivers/media/video/omap3-isp/isphist.h
 create mode 100644 drivers/media/video/omap3-isp/isppreview.c
 create mode 100644 drivers/media/video/omap3-isp/isppreview.h
 create mode 100644 drivers/media/video/omap3-isp/ispqueue.c
 create mode 100644 drivers/media/video/omap3-isp/ispqueue.h
 create mode 100644 drivers/media/video/omap3-isp/ispreg.h
 create mode 100644 drivers/media/video/omap3-isp/ispresizer.c
 create mode 100644 drivers/media/video/omap3-isp/ispresizer.h
 create mode 100644 drivers/media/video/omap3-isp/ispstat.c
 create mode 100644 drivers/media/video/omap3-isp/ispstat.h
 create mode 100644 drivers/media/video/omap3-isp/ispvideo.c
 create mode 100644 drivers/media/video/omap3-isp/ispvideo.h
 create mode 100644 drivers/media/video/omap3-isp/luma_enhance_table.h
 create mode 100644 drivers/media/video/omap3-isp/noise_filter_table.h
 create mode 100644 drivers/media/video/v4l2-subdev.c
 create mode 100644 include/linux/media.h
 create mode 100644 include/linux/omap3isp.h
 create mode 100644 include/linux/v4l2-mediabus.h
 create mode 100644 include/linux/v4l2-subdev.h
 create mode 100644 include/media/media-device.h
 create mode 100644 include/media/media-devnode.h
 create mode 100644 include/media/media-entity.h

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 17-02-2011 13:06, Laurent Pinchart escreveu:
>       v4l: Share code between video_usercopy and video_ioctl2

Two hunks failed on this patch:

$ quilt push
Applying patch patches/0951-v4l-Share-code-between-video_usercopy-and-video_ioct.patch
patching file drivers/media/video/v4l2-ioctl.c
Hunk #1 FAILED at 325.
Hunk #2 succeeded at 332 (offset -33 lines).
Hunk #3 succeeded at 368 (offset -33 lines).
Hunk #4 succeeded at 397 (offset -33 lines).
Hunk #5 FAILED at 1982.
2 out of 5 hunks FAILED -- rejects in file drivers/media/video/v4l2-ioctl.c
Patch patches/0951-v4l-Share-code-between-video_usercopy-and-video_ioct.patch does not apply (enforce with -f)

Basically, the code for video_ioctl2 suffered some recent changes. I suspect that the
blamed patch is this one:

cff7fbc6 (Pawel Osciak           2010-12-23 04:15:27 -0300 2342)        bool    has_array_args;
cff7fbc6 (Pawel Osciak           2010-12-23 04:15:27 -0300 2343)        size_t  array_size = 0;

Could you please fix the merge conflicts?

Thanks!
Mauro.

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

[Laurent Pinchart]

Hi Mauro,

On Wednesday 02 March 2011 21:13:31 Mauro Carvalho Chehab wrote:
> Em 17-02-2011 13:06, Laurent Pinchart escreveu:
> >       v4l: Share code between video_usercopy and video_ioctl2
> 
> Two hunks failed on this patch:
> 
> $ quilt push
> Applying patch
> patches/0951-v4l-Share-code-between-video_usercopy-and-video_ioct.patch
> patching file drivers/media/video/v4l2-ioctl.c
> Hunk #1 FAILED at 325.
> Hunk #2 succeeded at 332 (offset -33 lines).
> Hunk #3 succeeded at 368 (offset -33 lines).
> Hunk #4 succeeded at 397 (offset -33 lines).
> Hunk #5 FAILED at 1982.
> 2 out of 5 hunks FAILED -- rejects in file drivers/media/video/v4l2-ioctl.c
> Patch
> patches/0951-v4l-Share-code-between-video_usercopy-and-video_ioct.patch
> does not apply (enforce with -f)
> 
> Basically, the code for video_ioctl2 suffered some recent changes. I
> suspect that the blamed patch is this one:
> 
> cff7fbc6 (Pawel Osciak           2010-12-23 04:15:27 -0300 2342)       
> bool    has_array_args; cff7fbc6 (Pawel Osciak           2010-12-23
> 04:15:27 -0300 2343)        size_t  array_size = 0;
> 
> Could you please fix the merge conflicts?

Sure. I'm working on that right now, I'll send a new pull request.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Mauro,

The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:

  [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)

are available in the git repository at:
  git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp

The branch has been rebased on top of the latest for_v2.6.39 branch, with the
v4l2-ioctl.c conflict resolved.

Antti Koskipaa (1):
      v4l: v4l2_subdev userspace crop API

David Cohen (1):
      omap3isp: Statistics

Laurent Pinchart (36):
      v4l: Share code between video_usercopy and video_ioctl2
      v4l: subdev: Don't require core operations
      v4l: subdev: Add device node support
      v4l: subdev: Uninline the v4l2_subdev_init function
      v4l: subdev: Control ioctls support
      media: Media device node support
      media: Media device
      media: Entities, pads and links
      media: Entity use count
      media: Media device information query
      media: Entities, pads and links enumeration
      media: Links setup
      media: Pipelines and media streams
      v4l: Add a media_device pointer to the v4l2_device structure
      v4l: Make video_device inherit from media_entity
      v4l: Make v4l2_subdev inherit from media_entity
      v4l: Move the media/v4l2-mediabus.h header to include/linux
      v4l: Replace enums with fixed-sized fields in public structure
      v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
      v4l: Group media bus pixel codes by types and sort them alphabetically
      v4l: subdev: Add new file operations
      v4l: v4l2_subdev pad-level operations
      v4l: v4l2_subdev userspace format API - documentation binary files
      v4l: v4l2_subdev userspace format API
      v4l: v4l2_subdev userspace frame interval API
      v4l: subdev: Generic ioctl support
      v4l: Add subdev sensor g_skip_frames operation
      v4l: Add 8-bit YUYV on 16-bit bus and SGRBG10 media bus pixel codes
      v4l: Add remaining RAW10 patterns w DPCM pixel code variants
      v4l: Add missing 12 bits bayer media bus formats
      v4l: Add 12 bits bayer pixel formats
      omap3: Add function to register omap3isp platform device structure
      omap3isp: Video devices and buffers queue
      omap3isp: CCP2/CSI2 receivers
      omap3isp: CCDC, preview engine and resizer
      omap3isp: Kconfig and Makefile

Sakari Ailus (3):
      v4l: subdev: Events support
      media: Entity graph traversal
      omap3isp: OMAP3 ISP core

Sergio Aguirre (2):
      omap3: Remove unusued ISP CBUFF resource
      omap2: Fix camera resources for multiomap

Stanimir Varbanov (1):
      v4l: Create v4l2 subdev file handle structure

Tuukka Toivonen (1):
      ARM: OMAP3: Update Camera ISP definitions for OMAP3630

 Documentation/ABI/testing/sysfs-bus-media          |    6 +
 Documentation/DocBook/Makefile                     |    5 +-
 Documentation/DocBook/media-entities.tmpl          |   50 +
 Documentation/DocBook/media.tmpl                   |    3 +
 Documentation/DocBook/v4l/bayer.pdf                |  Bin 0 -> 12116 bytes
 Documentation/DocBook/v4l/bayer.png                |  Bin 0 -> 9725 bytes
 Documentation/DocBook/v4l/dev-subdev.xml           |  313 +++
 Documentation/DocBook/v4l/media-controller.xml     |   89 +
 Documentation/DocBook/v4l/media-func-close.xml     |   59 +
 Documentation/DocBook/v4l/media-func-ioctl.xml     |  116 +
 Documentation/DocBook/v4l/media-func-open.xml      |   94 +
 .../DocBook/v4l/media-ioc-device-info.xml          |  133 ++
 .../DocBook/v4l/media-ioc-enum-entities.xml        |  308 +++
 Documentation/DocBook/v4l/media-ioc-enum-links.xml |  207 ++
 Documentation/DocBook/v4l/media-ioc-setup-link.xml |   93 +
 Documentation/DocBook/v4l/pipeline.pdf             |  Bin 0 -> 20276 bytes
 Documentation/DocBook/v4l/pipeline.png             |  Bin 0 -> 12130 bytes
 Documentation/DocBook/v4l/subdev-formats.xml       | 2467 ++++++++++++++++++++
 Documentation/DocBook/v4l/v4l2.xml                 |    7 +
 Documentation/DocBook/v4l/vidioc-streamon.xml      |    9 +
 .../v4l/vidioc-subdev-enum-frame-interval.xml      |  152 ++
 .../DocBook/v4l/vidioc-subdev-enum-frame-size.xml  |  154 ++
 .../DocBook/v4l/vidioc-subdev-enum-mbus-code.xml   |  119 +
 Documentation/DocBook/v4l/vidioc-subdev-g-crop.xml |  155 ++
 Documentation/DocBook/v4l/vidioc-subdev-g-fmt.xml  |  180 ++
 .../DocBook/v4l/vidioc-subdev-g-frame-interval.xml |  141 ++
 Documentation/media-framework.txt                  |  353 +++
 Documentation/video4linux/v4l2-framework.txt       |  127 +-
 MAINTAINERS                                        |    6 +
 arch/arm/mach-omap2/devices.c                      |   63 +-
 arch/arm/mach-omap2/devices.h                      |   19 +
 arch/arm/plat-omap/include/plat/omap34xx.h         |   16 +-
 drivers/media/Kconfig                              |   22 +
 drivers/media/Makefile                             |    6 +
 drivers/media/media-device.c                       |  382 +++
 drivers/media/media-devnode.c                      |  321 +++
 drivers/media/media-entity.c                       |  536 +++++
 drivers/media/video/Kconfig                        |   13 +
 drivers/media/video/Makefile                       |    4 +-
 drivers/media/video/mt9m001.c                      |    2 +-
 drivers/media/video/mt9v022.c                      |    4 +-
 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                |  427 ++++
 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           | 1264 ++++++++++
 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/ov6650.c                       |   10 +-
 drivers/media/video/sh_mobile_csi2.c               |    6 +-
 drivers/media/video/soc_mediabus.c                 |    2 +-
 drivers/media/video/v4l2-dev.c                     |   76 +-
 drivers/media/video/v4l2-device.c                  |   84 +-
 drivers/media/video/v4l2-ioctl.c                   |  109 +-
 drivers/media/video/v4l2-subdev.c                  |  332 +++
 include/linux/Kbuild                               |    4 +
 include/linux/media.h                              |  132 ++
 include/linux/omap3isp.h                           |  646 +++++
 include/linux/v4l2-mediabus.h                      |  108 +
 include/linux/v4l2-subdev.h                        |  141 ++
 include/linux/videodev2.h                          |    4 +
 include/media/media-device.h                       |   95 +
 include/media/media-devnode.h                      |   97 +
 include/media/media-entity.h                       |  151 ++
 include/media/soc_mediabus.h                       |    3 +-
 include/media/v4l2-dev.h                           |   25 +-
 include/media/v4l2-device.h                        |   10 +
 include/media/v4l2-mediabus.h                      |   61 +-
 include/media/v4l2-subdev.h                        |  111 +-
 93 files changed, 28434 insertions(+), 255 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-media
 create mode 100644 Documentation/DocBook/v4l/bayer.pdf
 create mode 100644 Documentation/DocBook/v4l/bayer.png
 create mode 100644 Documentation/DocBook/v4l/dev-subdev.xml
 create mode 100644 Documentation/DocBook/v4l/media-controller.xml
 create mode 100644 Documentation/DocBook/v4l/media-func-close.xml
 create mode 100644 Documentation/DocBook/v4l/media-func-ioctl.xml
 create mode 100644 Documentation/DocBook/v4l/media-func-open.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-device-info.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-enum-entities.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-enum-links.xml
 create mode 100644 Documentation/DocBook/v4l/media-ioc-setup-link.xml
 create mode 100644 Documentation/DocBook/v4l/pipeline.pdf
 create mode 100644 Documentation/DocBook/v4l/pipeline.png
 create mode 100644 Documentation/DocBook/v4l/subdev-formats.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-enum-frame-interval.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-enum-frame-size.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-enum-mbus-code.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-g-crop.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-g-fmt.xml
 create mode 100644 Documentation/DocBook/v4l/vidioc-subdev-g-frame-interval.xml
 create mode 100644 Documentation/media-framework.txt
 create mode 100644 arch/arm/mach-omap2/devices.h
 create mode 100644 drivers/media/media-device.c
 create mode 100644 drivers/media/media-devnode.c
 create mode 100644 drivers/media/media-entity.c
 create mode 100644 drivers/media/video/omap3-isp/Makefile
 create mode 100644 drivers/media/video/omap3-isp/cfa_coef_table.h
 create mode 100644 drivers/media/video/omap3-isp/gamma_table.h
 create mode 100644 drivers/media/video/omap3-isp/isp.c
 create mode 100644 drivers/media/video/omap3-isp/isp.h
 create mode 100644 drivers/media/video/omap3-isp/ispccdc.c
 create mode 100644 drivers/media/video/omap3-isp/ispccdc.h
 create mode 100644 drivers/media/video/omap3-isp/ispccp2.c
 create mode 100644 drivers/media/video/omap3-isp/ispccp2.h
 create mode 100644 drivers/media/video/omap3-isp/ispcsi2.c
 create mode 100644 drivers/media/video/omap3-isp/ispcsi2.h
 create mode 100644 drivers/media/video/omap3-isp/ispcsiphy.c
 create mode 100644 drivers/media/video/omap3-isp/ispcsiphy.h
 create mode 100644 drivers/media/video/omap3-isp/isph3a.h
 create mode 100644 drivers/media/video/omap3-isp/isph3a_aewb.c
 create mode 100644 drivers/media/video/omap3-isp/isph3a_af.c
 create mode 100644 drivers/media/video/omap3-isp/isphist.c
 create mode 100644 drivers/media/video/omap3-isp/isphist.h
 create mode 100644 drivers/media/video/omap3-isp/isppreview.c
 create mode 100644 drivers/media/video/omap3-isp/isppreview.h
 create mode 100644 drivers/media/video/omap3-isp/ispqueue.c
 create mode 100644 drivers/media/video/omap3-isp/ispqueue.h
 create mode 100644 drivers/media/video/omap3-isp/ispreg.h
 create mode 100644 drivers/media/video/omap3-isp/ispresizer.c
 create mode 100644 drivers/media/video/omap3-isp/ispresizer.h
 create mode 100644 drivers/media/video/omap3-isp/ispstat.c
 create mode 100644 drivers/media/video/omap3-isp/ispstat.h
 create mode 100644 drivers/media/video/omap3-isp/ispvideo.c
 create mode 100644 drivers/media/video/omap3-isp/ispvideo.h
 create mode 100644 drivers/media/video/omap3-isp/luma_enhance_table.h
 create mode 100644 drivers/media/video/omap3-isp/noise_filter_table.h
 create mode 100644 drivers/media/video/v4l2-subdev.c
 create mode 100644 include/linux/media.h
 create mode 100644 include/linux/omap3isp.h
 create mode 100644 include/linux/v4l2-mediabus.h
 create mode 100644 include/linux/v4l2-subdev.h
 create mode 100644 include/media/media-device.h
 create mode 100644 include/media/media-devnode.h
 create mode 100644 include/media/media-entity.h

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
> 
>   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
> 
> The branch has been rebased on top of the latest for_v2.6.39 branch, with the
> v4l2-ioctl.c conflict resolved.
> 
> Antti Koskipaa (1):
>       v4l: v4l2_subdev userspace crop API
> 
> David Cohen (1):
>       omap3isp: Statistics
> 
> Laurent Pinchart (36):
>       v4l: Share code between video_usercopy and video_ioctl2
>       v4l: subdev: Don't require core operations
>       v4l: subdev: Add device node support
>       v4l: subdev: Uninline the v4l2_subdev_init function
>       v4l: subdev: Control ioctls support
>       media: Media device node support
>       media: Media device
>       media: Entities, pads and links
>       media: Entity use count
>       media: Media device information query

Hi Laurent,

You're using 'M' for the media control ioctl's, but I'm not seeing any patch
adding that range to the ioctl-number:
	Documentation/ioctl/ioctl-number.txt

In particular, according with the text, 'M' is used by several other drivers,
including audio, with doesn't sounds like a good idea to me.

So, please send me a patch, at the end of the series, reserving an unused range 
for the media controller and replacing the ioctl numbers that you've already
added to the new ioctl group.

Thanks!
Mauro

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

[Laurent Pinchart]

Hi Mauro,

Thanks for the review. Let me address all your concerns in a single mail.

- ioctl numbers

I'll send you a patch that reserves a range in Documentation/ioctl/ioctl-
number.txt and update include/linux/media.h accordingly.

- private ioctls

As already explained by David, the private ioctls are used to control advanced 
device features that can't be handled by V4L2 controls at the moment (such as 
setting a gamma correction table). Using those ioctls is not mandatory, and 
the device will work correctly without them (albeit with a non optimal image 
quality).

David said he will submit a patch to document the ioctls.

- media bus formats

As Hans explained, there's no 1:1 relationship between media bus formats and 
pixel formats.

- FOURCC and media bus codes documentation

I forgot to document some of them. I'll send a new patch that adds the missing 
documentation.


Is there any other issue I need to address ? My understanding is that there's 
no need to rebase the existing patches, is that correct ?

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 05-03-2011 10:02, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> Thanks for the review. Let me address all your concerns in a single mail.
> 
> - ioctl numbers
> 
> I'll send you a patch that reserves a range in Documentation/ioctl/ioctl-
> number.txt and update include/linux/media.h accordingly.

Ok, thanks.
> 
> - private ioctls
> 
> As already explained by David, the private ioctls are used to control advanced 
> device features that can't be handled by V4L2 controls at the moment (such as 
> setting a gamma correction table). Using those ioctls is not mandatory, and 
> the device will work correctly without them (albeit with a non optimal image 
> quality).
> 
> David said he will submit a patch to document the ioctls.

Ok.

> - media bus formats
> 
> As Hans explained, there's no 1:1 relationship between media bus formats and 
> pixel formats.

Yet, there are some relationship between them. See my comments on my previous email.

> - FOURCC and media bus codes documentation
> 
> I forgot to document some of them. I'll send a new patch that adds the missing 
> documentation.

Ok.
> 
> 
> Is there any other issue I need to address ? 

Nothing else, in the patches I've analysed so far. I'll take a look at the remaining
omap3isp after receiving the documentation for the private ioctl's.

> My understanding is that there's 
> no need to rebase the existing patches, is that correct ?

Yes, it is correct. Just send the new patches to be applied at the end of the series.
I'll eventually reorder them if needed to avoid breaking git bisect.

Thanks!
Mauro.

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

[Laurent Pinchart]

Hi Mauro,

On Saturday 05 March 2011 19:22:28 Mauro Carvalho Chehab wrote:
> Em 05-03-2011 10:02, Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > Thanks for the review. Let me address all your concerns in a single mail.
> > 
> > - ioctl numbers
> > 
> > I'll send you a patch that reserves a range in Documentation/ioctl/ioctl-
> > number.txt and update include/linux/media.h accordingly.
> 
> Ok, thanks.

"media: Pick a free ioctls range" at the top of the 
http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media-2.6.39-0005-
omap3isp branch

> > - private ioctls
> > 
> > As already explained by David, the private ioctls are used to control
> > advanced device features that can't be handled by V4L2 controls at the
> > moment (such as setting a gamma correction table). Using those ioctls is
> > not mandatory, and the device will work correctly without them (albeit
> > with a non optimal image quality).
> > 
> > David said he will submit a patch to document the ioctls.
> 
> Ok.

Working on that.

> > - media bus formats
> > 
> > As Hans explained, there's no 1:1 relationship between media bus formats
> > and pixel formats.
> 
> Yet, there are some relationship between them. See my comments on my
> previous email.

Let's continue the discussion in the mail thread.

> > - FOURCC and media bus codes documentation
> > 
> > I forgot to document some of them. I'll send a new patch that adds the
> > missing documentation.
> 
> Ok.

"v4l: Add documentation for the 12 bits bayer pixel formats"
"v4l: Fix 12 bits bayer media bus format documentation"

in the 
http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media-2.6.39-0004-
v4l-misc branch.

> > Is there any other issue I need to address ?
> 
> Nothing else, in the patches I've analysed so far. I'll take a look at the
> remaining omap3isp after receiving the documentation for the private
> ioctl's.
> 
> > My understanding is that there's
> > no need to rebase the existing patches, is that correct ?
> 
> Yes, it is correct. Just send the new patches to be applied at the end of
> the series. I'll eventually reorder them if needed to avoid breaking git
> bisect.

Please squash "v4l: Add documentation for the 12 bits bayer pixel formats" 
with "v4l: Add 12 bits bayer pixel formats" and "v4l: Fix 12 bits bayer media 
bus format documentation" with "v4l: Add missing 12 bits bayer media bus 
formats" when applying to keep the history clean. You can discard the commit 
message of the two new patches.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 05-03-2011 17:48, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> On Saturday 05 March 2011 19:22:28 Mauro Carvalho Chehab wrote:
>> Em 05-03-2011 10:02, Laurent Pinchart escreveu:
>>> Hi Mauro,
>>>
>>> Thanks for the review. Let me address all your concerns in a single mail.
>>>
>>> - ioctl numbers
>>>
>>> I'll send you a patch that reserves a range in Documentation/ioctl/ioctl-
>>> number.txt and update include/linux/media.h accordingly.
>>
>> Ok, thanks.
> 
> "media: Pick a free ioctls range" at the top of the 
> http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media-2.6.39-0005-
> omap3isp branch

Added in the end of my quilt series.
> 
>>> - private ioctls
>>>
>>> As already explained by David, the private ioctls are used to control
>>> advanced device features that can't be handled by V4L2 controls at the
>>> moment (such as setting a gamma correction table). Using those ioctls is
>>> not mandatory, and the device will work correctly without them (albeit
>>> with a non optimal image quality).
>>>
>>> David said he will submit a patch to document the ioctls.
>>
>> Ok.
> 
> Working on that.

Laurent/David, any news on that?

>>> - media bus formats
>>>
>>> As Hans explained, there's no 1:1 relationship between media bus formats
>>> and pixel formats.
>>
>> Yet, there are some relationship between them. See my comments on my
>> previous email.
> 
> Let's continue the discussion in the mail thread.
> 
>>> - FOURCC and media bus codes documentation
>>>
>>> I forgot to document some of them. I'll send a new patch that adds the
>>> missing documentation.
>>
>> Ok.
> 
> "v4l: Add documentation for the 12 bits bayer pixel formats"
> "v4l: Fix 12 bits bayer media bus format documentation"
> 
> in the 
> http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media-2.6.39-0004-
> v4l-misc branch.
> 
>>> Is there any other issue I need to address ?
>>
>> Nothing else, in the patches I've analysed so far. I'll take a look at the
>> remaining omap3isp after receiving the documentation for the private
>> ioctl's.
>>
>>> My understanding is that there's
>>> no need to rebase the existing patches, is that correct ?
>>
>> Yes, it is correct. Just send the new patches to be applied at the end of
>> the series. I'll eventually reorder them if needed to avoid breaking git
>> bisect.
> 
> Please squash "v4l: Add documentation for the 12 bits bayer pixel formats" 
> with "v4l: Add 12 bits bayer pixel formats" and "v4l: Fix 12 bits bayer media 
> bus format documentation" with "v4l: Add missing 12 bits bayer media bus 
> formats" when applying to keep the history clean. You can discard the commit 
> message of the two new patches.

Added both patches and folded them as requested, and added the remaining
patches after my review. The new tree is at:

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

The pending issues for merging it to the main devel branch are:
	- omap3isp private control description;
	- a chapter describing how *MBUS* and fourcc formats are related;
	- a description about how to lock between MBUS/fourcc get/set format;
	- a renaming patch to make directory name and file names consistent.

Thanks,
Mauro.

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

[David Cohen]

On Mon, Mar 7, 2011 at 1:57 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 05-03-2011 17:48, Laurent Pinchart escreveu:
>> Hi Mauro,

Hi Mauro,

>>
>> On Saturday 05 March 2011 19:22:28 Mauro Carvalho Chehab wrote:
>>> Em 05-03-2011 10:02, Laurent Pinchart escreveu:
>>>> Hi Mauro,
>>>>
>>>> Thanks for the review. Let me address all your concerns in a single mail.
>>>>
>>>> - ioctl numbers
>>>>
>>>> I'll send you a patch that reserves a range in Documentation/ioctl/ioctl-
>>>> number.txt and update include/linux/media.h accordingly.
>>>
>>> Ok, thanks.
>>
>> "media: Pick a free ioctls range" at the top of the
>> http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media-2.6.39-0005-
>> omap3isp branch
>
> Added in the end of my quilt series.
>>
>>>> - private ioctls
>>>>
>>>> As already explained by David, the private ioctls are used to control
>>>> advanced device features that can't be handled by V4L2 controls at the
>>>> moment (such as setting a gamma correction table). Using those ioctls is
>>>> not mandatory, and the device will work correctly without them (albeit
>>>> with a non optimal image quality).
>>>>
>>>> David said he will submit a patch to document the ioctls.
>>>
>>> Ok.
>>
>> Working on that.
>
> Laurent/David, any news on that?

Yes. Sakari has written a first documentation and I'm complementing
with statistic's specific usage information.

Regards,

David

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

[Laurent Pinchart]

Hi Mauro,

On Monday 07 March 2011 12:57:30 Mauro Carvalho Chehab wrote:
> Em 05-03-2011 17:48, Laurent Pinchart escreveu:

[snip]

> Added both patches and folded them as requested, and added the remaining
> patches after my review. The new tree is at:
> 
> http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/med
> ia_controller
> 
> The pending issues for merging it to the main devel branch are:
> 	- omap3isp private control description;

Still working on that, I expect to send it this evening.

> 	- a chapter describing how *MBUS* and fourcc formats are related;

This still needs to be discussed, there's no agreement on that yet.

> 	- a description about how to lock between MBUS/fourcc get/set format;

>From Documentation/media-framework.txt:

"If other operations need to be disallowed on streaming entities (such as
changing entities configuration parameters) drivers can explictly check the
media_entity stream_count field to find out if an entity is streaming. This
operation must be done with the media_device graph_mutex held."

So it's already there :-) And the media_entity_pipeline_start() function makes 
it easy to implement in bridge driver.

> 	- a renaming patch to make directory name and file names consistent.

Done. I've pushed the modified patches to the media-2.6.39-0005-omap3isp 
branch.

The media-2.6.39-0004-v4l-misc branch has also been rebased to squash the 
format documentation patches as you did in your tree. There's no need to pull 
anything from it.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Mauro,

On Monday 07 March 2011 12:57:30 Mauro Carvalho Chehab wrote:
> Em 05-03-2011 17:48, Laurent Pinchart escreveu:
> > On Saturday 05 March 2011 19:22:28 Mauro Carvalho Chehab wrote:
> >> Em 05-03-2011 10:02, Laurent Pinchart escreveu:

[snip]

> >>> - private ioctls
> >>> 
> >>> As already explained by David, the private ioctls are used to control
> >>> advanced device features that can't be handled by V4L2 controls at the
> >>> moment (such as setting a gamma correction table). Using those ioctls
> >>> is not mandatory, and the device will work correctly without them
> >>> (albeit with a non optimal image quality).
> >>> 
> >>> David said he will submit a patch to document the ioctls.
> >> 
> >> Ok.
> > 
> > Working on that.
> 
> Laurent/David, any news on that?

"omap3isp: Add documentation" in
http://git.linuxtv.org/pinchartl/media.git?a=shortlog;h=refs/heads/media-2.6.39-0005-
omap3isp

You can take all the OMAP3 ISP patches from that branch.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 07-03-2011 08:57, Mauro Carvalho Chehab escreveu:
> Em 05-03-2011 17:48, Laurent Pinchart escreveu:

> Added both patches and folded them as requested, and added the remaining
> patches after my review. The new tree is at:
> 
> http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/media_controller
> 
> The pending issues for merging it to the main devel branch are:
> 	- omap3isp private control description;
> 	- a renaming patch to make directory name and file names consistent.

Tree updated with the patches from:
	git://linuxtv.org/pinchartl/media.git media-for-mauro

> 	- a chapter describing how *MBUS* and fourcc formats are related;

Still needed. For now, I'll merge what we currently have at the master devel tree,
but we still need such chapter to be written, in order to have the media controller
for .39.

> 	- a description about how to lock between MBUS/fourcc get/set format;

We had some discussions about it, but we didn't reach to a conclusion. I'd like
to see something documented at the v4l framework about that. IMO, this is a good
theme for the V4L "brainstorm" meeting.

I also like to have a patch adding such docs for .39.

Could you please double-check if everything went fine on my merge at:
	http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/media_controller
Before I merge it at the media-tree?

Thanks,
Mauro.

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

[Laurent Pinchart]

Hi Mauro,

On Friday 11 March 2011 16:40:42 Mauro Carvalho Chehab wrote:
> Em 07-03-2011 08:57, Mauro Carvalho Chehab escreveu:
> > Em 05-03-2011 17:48, Laurent Pinchart escreveu:
> > 
> > Added both patches and folded them as requested, and added the remaining
> > patches after my review. The new tree is at:
> > 
> > http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/m
> > edia_controller
> > 
> > The pending issues for merging it to the main devel branch are:
> > 	- omap3isp private control description;
> > 	- a renaming patch to make directory name and file names consistent.
> 
> Tree updated with the patches from:
> 	git://linuxtv.org/pinchartl/media.git media-for-mauro

Thanks.

> > 	- a chapter describing how *MBUS* and fourcc formats are related;
> 
> Still needed. For now, I'll merge what we currently have at the master
> devel tree, but we still need such chapter to be written, in order to have
> the media controller for .39.

I plan to discuss this topic during the brainstorming meeting.

> > 	- a description about how to lock between MBUS/fourcc get/set format;
> 
> We had some discussions about it, but we didn't reach to a conclusion.

Documentation/media-framework.txt already states that

"If other operations need to be disallowed on streaming entities (such as
changing entities configuration parameters) drivers can explictly check the
media_entity stream_count field to find out if an entity is streaming. This
operation must be done with the media_device graph_mutex held."

> I'd like to see something documented at the v4l framework about that. IMO,
> this is a good theme for the V4L "brainstorm" meeting.
>
> I also like to have a patch adding such docs for .39.

OK, we'll discuss the topic during the meeting and I'll work on documentation.
 
> Could you please double-check if everything went fine on my merge at:
> 	http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/me
> dia_controller Before I merge it at the media-tree?

Everything is there. Thanks a lot.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
> 
>   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
> 
> The branch has been rebased on top of the latest for_v2.6.39 branch, with the
> v4l2-ioctl.c conflict resolved.
> 
> Antti Koskipaa (1):
>       v4l: v4l2_subdev userspace crop API
> 
> David Cohen (1):
>       omap3isp: Statistics
> 
> Laurent Pinchart (36):
>       v4l: Share code between video_usercopy and video_ioctl2
>       v4l: subdev: Don't require core operations
>       v4l: subdev: Add device node support
>       v4l: subdev: Uninline the v4l2_subdev_init function
>       v4l: subdev: Control ioctls support
>       media: Media device node support
>       media: Media device
>       media: Entities, pads and links
>       media: Entity use count
>       media: Media device information query
>       media: Entities, pads and links enumeration
>       media: Links setup
>       media: Pipelines and media streams
>       v4l: Add a media_device pointer to the v4l2_device structure
>       v4l: Make video_device inherit from media_entity
>       v4l: Make v4l2_subdev inherit from media_entity
>       v4l: Move the media/v4l2-mediabus.h header to include/linux
>       v4l: Replace enums with fixed-sized fields in public structure
>       v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
>       v4l: Group media bus pixel codes by types and sort them alphabetically

The presence of those mediabus names against the traditional fourcc codes
at the API adds some mess to the media controller. Not sure how to solve,
but maybe the best way is to add a table at the V4L2 API associating each
media bus format to the corresponding V4L2 fourcc codes.

Cheers,
Mauro

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

[David Cohen]

Hi Mauro,

On Fri, Mar 4, 2011 at 10:10 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
>> Hi Mauro,
>>
>> The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
>>
>>   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
>>
>> are available in the git repository at:
>>   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
>>
>> The branch has been rebased on top of the latest for_v2.6.39 branch, with the
>> v4l2-ioctl.c conflict resolved.
>>
>> Antti Koskipaa (1):
>>       v4l: v4l2_subdev userspace crop API
>>
>> David Cohen (1):
>>       omap3isp: Statistics
>>
>> Laurent Pinchart (36):
>>       v4l: Share code between video_usercopy and video_ioctl2
>>       v4l: subdev: Don't require core operations
>>       v4l: subdev: Add device node support
>>       v4l: subdev: Uninline the v4l2_subdev_init function
>>       v4l: subdev: Control ioctls support
>>       media: Media device node support
>>       media: Media device
>>       media: Entities, pads and links
>>       media: Entity use count
>>       media: Media device information query
>>       media: Entities, pads and links enumeration
>>       media: Links setup
>>       media: Pipelines and media streams
>>       v4l: Add a media_device pointer to the v4l2_device structure
>>       v4l: Make video_device inherit from media_entity
>>       v4l: Make v4l2_subdev inherit from media_entity
>>       v4l: Move the media/v4l2-mediabus.h header to include/linux
>>       v4l: Replace enums with fixed-sized fields in public structure
>>       v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
>>       v4l: Group media bus pixel codes by types and sort them alphabetically
>
> The presence of those mediabus names against the traditional fourcc codes
> at the API adds some mess to the media controller. Not sure how to solve,
> but maybe the best way is to add a table at the V4L2 API associating each
> media bus format to the corresponding V4L2 fourcc codes.

That would be good. OMAP2 camera driver also needs such table.

Br,

David

>
> Cheers,
> 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: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver

[Hans Verkuil]

On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
> > 
> >   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
> > 
> > are available in the git repository at:
> >   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
> > 
> > The branch has been rebased on top of the latest for_v2.6.39 branch, with the
> > v4l2-ioctl.c conflict resolved.
> > 
> > Antti Koskipaa (1):
> >       v4l: v4l2_subdev userspace crop API
> > 
> > David Cohen (1):
> >       omap3isp: Statistics
> > 
> > Laurent Pinchart (36):
> >       v4l: Share code between video_usercopy and video_ioctl2
> >       v4l: subdev: Don't require core operations
> >       v4l: subdev: Add device node support
> >       v4l: subdev: Uninline the v4l2_subdev_init function
> >       v4l: subdev: Control ioctls support
> >       media: Media device node support
> >       media: Media device
> >       media: Entities, pads and links
> >       media: Entity use count
> >       media: Media device information query
> >       media: Entities, pads and links enumeration
> >       media: Links setup
> >       media: Pipelines and media streams
> >       v4l: Add a media_device pointer to the v4l2_device structure
> >       v4l: Make video_device inherit from media_entity
> >       v4l: Make v4l2_subdev inherit from media_entity
> >       v4l: Move the media/v4l2-mediabus.h header to include/linux
> >       v4l: Replace enums with fixed-sized fields in public structure
> >       v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
> >       v4l: Group media bus pixel codes by types and sort them alphabetically
> 
> The presence of those mediabus names against the traditional fourcc codes
> at the API adds some mess to the media controller. Not sure how to solve,
> but maybe the best way is to add a table at the V4L2 API associating each
> media bus format to the corresponding V4L2 fourcc codes.

You can't do that in general. Only for specific hardware platforms. If you
could do it, then we would have never bothered creating these mediabus fourccs.

How a mediabus fourcc translates to a pixelcode (== memory format) depends
entirely on the hardware capabilities (mostly that of the DMA engine).

A generic V4L2 application will never use mediabus fourcc codes. It's only used
by drivers and applications written specifically for that hardware and using
/dev/v4l-subdevX devices.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

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

[David Cohen]

Hi Hans,

On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
>> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
>> > Hi Mauro,
>> >
>> > The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
>> >
>> >   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
>> >
>> > are available in the git repository at:
>> >   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
>> >
>> > The branch has been rebased on top of the latest for_v2.6.39 branch, with the
>> > v4l2-ioctl.c conflict resolved.
>> >
>> > Antti Koskipaa (1):
>> >       v4l: v4l2_subdev userspace crop API
>> >
>> > David Cohen (1):
>> >       omap3isp: Statistics
>> >
>> > Laurent Pinchart (36):
>> >       v4l: Share code between video_usercopy and video_ioctl2
>> >       v4l: subdev: Don't require core operations
>> >       v4l: subdev: Add device node support
>> >       v4l: subdev: Uninline the v4l2_subdev_init function
>> >       v4l: subdev: Control ioctls support
>> >       media: Media device node support
>> >       media: Media device
>> >       media: Entities, pads and links
>> >       media: Entity use count
>> >       media: Media device information query
>> >       media: Entities, pads and links enumeration
>> >       media: Links setup
>> >       media: Pipelines and media streams
>> >       v4l: Add a media_device pointer to the v4l2_device structure
>> >       v4l: Make video_device inherit from media_entity
>> >       v4l: Make v4l2_subdev inherit from media_entity
>> >       v4l: Move the media/v4l2-mediabus.h header to include/linux
>> >       v4l: Replace enums with fixed-sized fields in public structure
>> >       v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
>> >       v4l: Group media bus pixel codes by types and sort them alphabetically
>>
>> The presence of those mediabus names against the traditional fourcc codes
>> at the API adds some mess to the media controller. Not sure how to solve,
>> but maybe the best way is to add a table at the V4L2 API associating each
>> media bus format to the corresponding V4L2 fourcc codes.
>
> You can't do that in general. Only for specific hardware platforms. If you
> could do it, then we would have never bothered creating these mediabus fourccs.
>
> How a mediabus fourcc translates to a pixelcode (== memory format) depends
> entirely on the hardware capabilities (mostly that of the DMA engine).

May I ask you one question here? (not entirely related to this patch set).
Why pixelcode != mediabus fourcc?
e.g. OMAP2 camera driver talks to sensor through subdev interface and
sets its own output pixelformat depending on sensor's mediabus fourcc.
So it needs a translation table mbus_pixelcode -> pixelformat. Why
can't it be pixelformat -> pixelformat ?

Regards,

David

>
> A generic V4L2 application will never use mediabus fourcc codes. It's only used
> by drivers and applications written specifically for that hardware and using
> /dev/v4l-subdevX devices.
>
> Regards,
>
>        Hans
>
> --
> Hans Verkuil - video4linux developer - sponsored by Cisco
> --
> 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: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver

[Hans Verkuil]

On Saturday, March 05, 2011 14:04:21 David Cohen wrote:
> Hi Hans,
> 
> On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
> >> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> >> > Hi Mauro,
> >> >
> >> > The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
> >> >
> >> >   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
> >> >
> >> > are available in the git repository at:
> >> >   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
> >> >
> >> > The branch has been rebased on top of the latest for_v2.6.39 branch, with the
> >> > v4l2-ioctl.c conflict resolved.
> >> >
> >> > Antti Koskipaa (1):
> >> >       v4l: v4l2_subdev userspace crop API
> >> >
> >> > David Cohen (1):
> >> >       omap3isp: Statistics
> >> >
> >> > Laurent Pinchart (36):
> >> >       v4l: Share code between video_usercopy and video_ioctl2
> >> >       v4l: subdev: Don't require core operations
> >> >       v4l: subdev: Add device node support
> >> >       v4l: subdev: Uninline the v4l2_subdev_init function
> >> >       v4l: subdev: Control ioctls support
> >> >       media: Media device node support
> >> >       media: Media device
> >> >       media: Entities, pads and links
> >> >       media: Entity use count
> >> >       media: Media device information query
> >> >       media: Entities, pads and links enumeration
> >> >       media: Links setup
> >> >       media: Pipelines and media streams
> >> >       v4l: Add a media_device pointer to the v4l2_device structure
> >> >       v4l: Make video_device inherit from media_entity
> >> >       v4l: Make v4l2_subdev inherit from media_entity
> >> >       v4l: Move the media/v4l2-mediabus.h header to include/linux
> >> >       v4l: Replace enums with fixed-sized fields in public structure
> >> >       v4l: Rename V4L2_MBUS_FMT_GREY8_1X8 to V4L2_MBUS_FMT_Y8_1X8
> >> >       v4l: Group media bus pixel codes by types and sort them alphabetically
> >>
> >> The presence of those mediabus names against the traditional fourcc codes
> >> at the API adds some mess to the media controller. Not sure how to solve,
> >> but maybe the best way is to add a table at the V4L2 API associating each
> >> media bus format to the corresponding V4L2 fourcc codes.
> >
> > You can't do that in general. Only for specific hardware platforms. If you
> > could do it, then we would have never bothered creating these mediabus fourccs.
> >
> > How a mediabus fourcc translates to a pixelcode (== memory format) depends
> > entirely on the hardware capabilities (mostly that of the DMA engine).
> 
> May I ask you one question here? (not entirely related to this patch set).
> Why pixelcode != mediabus fourcc?
> e.g. OMAP2 camera driver talks to sensor through subdev interface and
> sets its own output pixelformat depending on sensor's mediabus fourcc.

The media bus deals with how pixels are transferred over a physical bus.
For example 10-bit RGB samples. But how those samples end up in memory
is quite a different story: depending on the (DMA) hardware this might
end up as a 24-bit RGB format (8 bits per sample), or a multi-planar format
where one plane has the top 8 bits and another plane the lower 2 bits, or
it might go through a built-in colorspace convertor, and let's not forget
the endianness issues you get once you DMA into memory.

So mediabus formats are quite different from memory formats. In certain
simple cases the mapping may be straightforward, but not in the general case.

In a nutshell: mediabus deals with how two devices stream media over a bus
between one another, whereas pixelformats deal with how the media is encoded
in memory.

Hope this helps.

Regards,

	Hans

> So it needs a translation table mbus_pixelcode -> pixelformat. Why
> can't it be pixelformat -> pixelformat ?
> 
> Regards,
> 
> David
> 
> >
> > A generic V4L2 application will never use mediabus fourcc codes. It's only used
> > by drivers and applications written specifically for that hardware and using
> > /dev/v4l-subdevX devices.
> >
> > Regards,
> >
> >        Hans
> >
> > --
> > Hans Verkuil - video4linux developer - sponsored by Cisco
> > --
> > 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: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver

[Sylwester Nawrocki]

Hi David,

On 03/05/2011 02:04 PM, David Cohen wrote:
> Hi Hans,
> 
> On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuil<hverkuil@xs4all.nl>  wrote:
>> On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
>>> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
...
>>>>        v4l: Group media bus pixel codes by types and sort them alphabetically
>>>
>>> The presence of those mediabus names against the traditional fourcc codes
>>> at the API adds some mess to the media controller. Not sure how to solve,
>>> but maybe the best way is to add a table at the V4L2 API associating each
>>> media bus format to the corresponding V4L2 fourcc codes.
>>
>> You can't do that in general. Only for specific hardware platforms. If you
>> could do it, then we would have never bothered creating these mediabus fourccs.
>>
>> How a mediabus fourcc translates to a pixelcode (== memory format) depends
>> entirely on the hardware capabilities (mostly that of the DMA engine).
> 
> May I ask you one question here? (not entirely related to this patch set).
> Why pixelcode != mediabus fourcc?
> e.g. OMAP2 camera driver talks to sensor through subdev interface and
> sets its own output pixelformat depending on sensor's mediabus fourcc.
> So it needs a translation table mbus_pixelcode ->  pixelformat. Why
> can't it be pixelformat ->  pixelformat ?
> 

Let me try to explain, struct v4l2_mbus_framefmt::code (pixelcode)
describes how data is transfered/sampled on the camera parallel or serial bus.
It defines bus width, data alignment and how many data samples form a single
pixel.

struct v4l2_pix_format::pixelformat (fourcc) on the other hand describes how
the image data is stored in memory.
 
As Hans pointed out there is not always a 1:1 correspondence, e.g. 

1. Both V4L2_MBUS_FMT_YUYV8_1x16 and V4L2_MBUS_FMT_YUYV8_2x8 may being 
translating to V4L2_PIX_FMT_YUYV fourcc,

2. Or the DMA engine in the camera host interface might be capable of
converting single V4L2_MBUS_FMT_RGB555 pixelcode to V4L2_PIX_FMT_RGB555
and V4L2_PIX_FMT_RGB565 fourcc's. So the user can choose any of them they
seem most suitable and the hardware takes care of the conversion. 

What translations are available really depends on the hardware, so how
could we define a standard translation table? IMO it should be realized
in each driver on an individual basis.

Regards,
Sylwester

> Regards,
> 
> David
> 
>>
>> A generic V4L2 application will never use mediabus fourcc codes. It's only used
>> by drivers and applications written specifically for that hardware and using
>> /dev/v4l-subdevX devices.
>>
>> Regards,
>>
>>         Hans
>>

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

[Mauro Carvalho Chehab]

Em 05-03-2011 11:29, Sylwester Nawrocki escreveu:
> Hi David,
> 
> On 03/05/2011 02:04 PM, David Cohen wrote:
>> Hi Hans,
>>
>> On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuil<hverkuil@xs4all.nl>  wrote:
>>> On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
>>>> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> ...
>>>>>        v4l: Group media bus pixel codes by types and sort them alphabetically
>>>>
>>>> The presence of those mediabus names against the traditional fourcc codes
>>>> at the API adds some mess to the media controller. Not sure how to solve,
>>>> but maybe the best way is to add a table at the V4L2 API associating each
>>>> media bus format to the corresponding V4L2 fourcc codes.
>>>
>>> You can't do that in general. Only for specific hardware platforms. If you
>>> could do it, then we would have never bothered creating these mediabus fourccs.
>>>
>>> How a mediabus fourcc translates to a pixelcode (== memory format) depends
>>> entirely on the hardware capabilities (mostly that of the DMA engine).
>>
>> May I ask you one question here? (not entirely related to this patch set).
>> Why pixelcode != mediabus fourcc?
>> e.g. OMAP2 camera driver talks to sensor through subdev interface and
>> sets its own output pixelformat depending on sensor's mediabus fourcc.
>> So it needs a translation table mbus_pixelcode ->  pixelformat. Why
>> can't it be pixelformat ->  pixelformat ?
>>
> 
> Let me try to explain, struct v4l2_mbus_framefmt::code (pixelcode)
> describes how data is transfered/sampled on the camera parallel or serial bus.
> It defines bus width, data alignment and how many data samples form a single
> pixel.
> 
> struct v4l2_pix_format::pixelformat (fourcc) on the other hand describes how
> the image data is stored in memory.
>  
> As Hans pointed out there is not always a 1:1 correspondence, e.g. 

The relation may not be 1:1 but they are related.

It should be documented somehow how those are related, otherwise, the API
will be obscure. 

Of course, the output format may be different than the internal formats, 
since some bridges have format converters.

> 
> 1. Both V4L2_MBUS_FMT_YUYV8_1x16 and V4L2_MBUS_FMT_YUYV8_2x8 may being 
> translating to V4L2_PIX_FMT_YUYV fourcc,

Ok, so there is a relationship between them.

> 2. Or the DMA engine in the camera host interface might be capable of
> converting single V4L2_MBUS_FMT_RGB555 pixelcode to V4L2_PIX_FMT_RGB555
> and V4L2_PIX_FMT_RGB565 fourcc's. So the user can choose any of them they
> seem most suitable and the hardware takes care of the conversion. 

No. You can't create an additional bit. if V4L2_MBUS_FMT_RGB555 provides 5
bits for all color channels, the only corresponding format is V4L2_PIX_FMT_RGB555,
as there's no way to get 6 bits for green, if the hardware sampled it with
5 bits. Ok, some bridge may fill with 0 an "extra" bit for green, but this
is the bridge doing a format conversion.

In general, for all RGB formats, a mapping between MBUS_FMT_RGBxxx and the
corresponding fourcc formats could be mapped on two formats only: 
V4L2_PIX_FMT_RGBxxx or V4L2_PIX_FMT_BGRxxx.
 
> What translations are available really depends on the hardware, so how
> could we define a standard translation table? IMO it should be realized
> in each driver on an individual basis.

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

[Sylwester Nawrocki]

Hi Mauro,

On 03/05/2011 07:14 PM, Mauro Carvalho Chehab wrote:
> Em 05-03-2011 11:29, Sylwester Nawrocki escreveu:
>> Hi David,
>>
>> On 03/05/2011 02:04 PM, David Cohen wrote:
>>> Hi Hans,
>>>
>>> On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuil<hverkuil@xs4all.nl>   wrote:
>>>> On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
>>>>> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
>> ...
>>>>>>         v4l: Group media bus pixel codes by types and sort them alphabetically
>>>>>
>>>>> The presence of those mediabus names against the traditional fourcc codes
>>>>> at the API adds some mess to the media controller. Not sure how to solve,
>>>>> but maybe the best way is to add a table at the V4L2 API associating each
>>>>> media bus format to the corresponding V4L2 fourcc codes.
>>>>
>>>> You can't do that in general. Only for specific hardware platforms. If you
>>>> could do it, then we would have never bothered creating these mediabus fourccs.
>>>>
>>>> How a mediabus fourcc translates to a pixelcode (== memory format) depends
>>>> entirely on the hardware capabilities (mostly that of the DMA engine).
>>>
>>> May I ask you one question here? (not entirely related to this patch set).
>>> Why pixelcode != mediabus fourcc?
>>> e.g. OMAP2 camera driver talks to sensor through subdev interface and
>>> sets its own output pixelformat depending on sensor's mediabus fourcc.
>>> So it needs a translation table mbus_pixelcode ->   pixelformat. Why
>>> can't it be pixelformat ->   pixelformat ?
>>>
>>
>> Let me try to explain, struct v4l2_mbus_framefmt::code (pixelcode)
>> describes how data is transfered/sampled on the camera parallel or serial bus.
>> It defines bus width, data alignment and how many data samples form a single
>> pixel.
>>
>> struct v4l2_pix_format::pixelformat (fourcc) on the other hand describes how
>> the image data is stored in memory.
>>
>> As Hans pointed out there is not always a 1:1 correspondence, e.g.
> 
> The relation may not be 1:1 but they are related.
> 
> It should be documented somehow how those are related, otherwise, the API
> will be obscure.

Yeah, I agree this is a good point now when the media bus formats have become
public. Perhaps by a misunderstanding I just thought it is all more about some
utility functions in the v4l core rather than the documentation.

> 
> Of course, the output format may be different than the internal formats,
> since some bridges have format converters.
> 
>>
>> 1. Both V4L2_MBUS_FMT_YUYV8_1x16 and V4L2_MBUS_FMT_YUYV8_2x8 may being
>> translating to V4L2_PIX_FMT_YUYV fourcc,
> 
> Ok, so there is a relationship between them.
> 
>> 2. Or the DMA engine in the camera host interface might be capable of
>> converting single V4L2_MBUS_FMT_RGB555 pixelcode to V4L2_PIX_FMT_RGB555
>> and V4L2_PIX_FMT_RGB565 fourcc's. So the user can choose any of them they
>> seem most suitable and the hardware takes care of the conversion.
> 
> No. You can't create an additional bit. if V4L2_MBUS_FMT_RGB555 provides 5
> bits for all color channels, the only corresponding format is V4L2_PIX_FMT_RGB555,
> as there's no way to get 6 bits for green, if the hardware sampled it with
> 5 bits. Ok, some bridge may fill with 0 an "extra" bit for green, but this
> is the bridge doing a format conversion.
> 
> In general, for all RGB formats, a mapping between MBUS_FMT_RGBxxx and the
> corresponding fourcc formats could be mapped on two formats only:
> V4L2_PIX_FMT_RGBxxx or V4L2_PIX_FMT_BGRxxx.

OK, that might not have been a good example, of one mbus pixel code to many
fourccs relationship. 
There will be always conversion between media bus pixelcode and fourccs 
as they are in completely different domains. And the method of conversion 
from media bus formats may be an intrinsic property of a bridge, changing
across various bridges, e.g. different endianness may be used.

So I think in general it is good to clearly specify the relationships 
in the API but we need to be aware of varying "correlation ratio" across 
the formats and that we should perhaps operate on ranges rather than single
formats. Perhaps the API should provide guidelines of which formats should
be used when to obtain best results.

> 
>> What translations are available really depends on the hardware, so how
>> could we define a standard translation table? IMO it should be realized
>> in each driver on an individual basis.

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

[Mauro Carvalho Chehab]

Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
> Hi Mauro,
> 
> On 03/05/2011 07:14 PM, Mauro Carvalho Chehab wrote:
>> Em 05-03-2011 11:29, Sylwester Nawrocki escreveu:
>>> Hi David,
>>>
>>> On 03/05/2011 02:04 PM, David Cohen wrote:
>>>> Hi Hans,
>>>>
>>>> On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuil<hverkuil@xs4all.nl>   wrote:
>>>>> On Friday, March 04, 2011 21:10:05 Mauro Carvalho Chehab wrote:
>>>>>> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
>>> ...
>>>>>>>         v4l: Group media bus pixel codes by types and sort them alphabetically
>>>>>>
>>>>>> The presence of those mediabus names against the traditional fourcc codes
>>>>>> at the API adds some mess to the media controller. Not sure how to solve,
>>>>>> but maybe the best way is to add a table at the V4L2 API associating each
>>>>>> media bus format to the corresponding V4L2 fourcc codes.
>>>>>
>>>>> You can't do that in general. Only for specific hardware platforms. If you
>>>>> could do it, then we would have never bothered creating these mediabus fourccs.
>>>>>
>>>>> How a mediabus fourcc translates to a pixelcode (== memory format) depends
>>>>> entirely on the hardware capabilities (mostly that of the DMA engine).
>>>>
>>>> May I ask you one question here? (not entirely related to this patch set).
>>>> Why pixelcode != mediabus fourcc?
>>>> e.g. OMAP2 camera driver talks to sensor through subdev interface and
>>>> sets its own output pixelformat depending on sensor's mediabus fourcc.
>>>> So it needs a translation table mbus_pixelcode ->   pixelformat. Why
>>>> can't it be pixelformat ->   pixelformat ?
>>>>
>>>
>>> Let me try to explain, struct v4l2_mbus_framefmt::code (pixelcode)
>>> describes how data is transfered/sampled on the camera parallel or serial bus.
>>> It defines bus width, data alignment and how many data samples form a single
>>> pixel.
>>>
>>> struct v4l2_pix_format::pixelformat (fourcc) on the other hand describes how
>>> the image data is stored in memory.
>>>
>>> As Hans pointed out there is not always a 1:1 correspondence, e.g.
>>
>> The relation may not be 1:1 but they are related.
>>
>> It should be documented somehow how those are related, otherwise, the API
>> will be obscure.
> 
> Yeah, I agree this is a good point now when the media bus formats have become
> public. Perhaps by a misunderstanding I just thought it is all more about some
> utility functions in the v4l core rather than the documentation.

Yes, now you got my point. 
 
>>
>> Of course, the output format may be different than the internal formats,
>> since some bridges have format converters.
>>
>>>
>>> 1. Both V4L2_MBUS_FMT_YUYV8_1x16 and V4L2_MBUS_FMT_YUYV8_2x8 may being
>>> translating to V4L2_PIX_FMT_YUYV fourcc,
>>
>> Ok, so there is a relationship between them.
>>
>>> 2. Or the DMA engine in the camera host interface might be capable of
>>> converting single V4L2_MBUS_FMT_RGB555 pixelcode to V4L2_PIX_FMT_RGB555
>>> and V4L2_PIX_FMT_RGB565 fourcc's. So the user can choose any of them they
>>> seem most suitable and the hardware takes care of the conversion.
>>
>> No. You can't create an additional bit. if V4L2_MBUS_FMT_RGB555 provides 5
>> bits for all color channels, the only corresponding format is V4L2_PIX_FMT_RGB555,
>> as there's no way to get 6 bits for green, if the hardware sampled it with
>> 5 bits. Ok, some bridge may fill with 0 an "extra" bit for green, but this
>> is the bridge doing a format conversion.
>>
>> In general, for all RGB formats, a mapping between MBUS_FMT_RGBxxx and the
>> corresponding fourcc formats could be mapped on two formats only:
>> V4L2_PIX_FMT_RGBxxx or V4L2_PIX_FMT_BGRxxx.
> 
> OK, that might not have been a good example, of one mbus pixel code to many
> fourccs relationship. 
> There will be always conversion between media bus pixelcode and fourccs 
> as they are in completely different domains. And the method of conversion 
> from media bus formats may be an intrinsic property of a bridge, changing
> across various bridges, e.g. different endianness may be used.
> 
> So I think in general it is good to clearly specify the relationships 
> in the API but we need to be aware of varying "correlation ratio" across 
> the formats and that we should perhaps operate on ranges rather than single
> formats. Perhaps the API should provide guidelines of which formats should
> be used when to obtain best results.

It makes sense to operate in ranges are you're proposing.

A somewhat unrelated question that occurred to me today: what happens when a 
format change happens while streaming? 

Considering that some formats need more bits than others, this could lead into 
buffer overflows, either internally at the device or externally, on bridges 
that just forward whatever it receives to the DMA buffers (there are some that 
just does that). I didn't see anything inside the mc code preventing such
condition to happen, and probably implementing it won't be an easy job. So,
one alternative would be to require some special CAPS if userspace tries to
set the mbus format directly, or to recommend userspace to create media
controller nodes with 0600 permission.

Comments?

Thanks,
Mauro.

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

[Laurent Pinchart]

Hi Mauro,

On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
>
> A somewhat unrelated question that occurred to me today: what happens when
> a format change happens while streaming?
> 
> Considering that some formats need more bits than others, this could lead
> into buffer overflows, either internally at the device or externally, on
> bridges that just forward whatever it receives to the DMA buffers (there
> are some that just does that). I didn't see anything inside the mc code
> preventing such condition to happen, and probably implementing it won't be
> an easy job. So, one alternative would be to require some special CAPS if
> userspace tries to set the mbus format directly, or to recommend userspace
> to create media controller nodes with 0600 permission.

That's not really a media controller issue. Whether formats can be changed 
during streaming is a driver decision. The OMAP3 ISP driver won't allow 
formats to be changed during streaming. If the hardware allows for such format 
changes, drivers can implement support for that and make sure that no buffer 
overflow will occur.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 06-03-2011 08:38, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
>> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
>>
>> A somewhat unrelated question that occurred to me today: what happens when
>> a format change happens while streaming?
>>
>> Considering that some formats need more bits than others, this could lead
>> into buffer overflows, either internally at the device or externally, on
>> bridges that just forward whatever it receives to the DMA buffers (there
>> are some that just does that). I didn't see anything inside the mc code
>> preventing such condition to happen, and probably implementing it won't be
>> an easy job. So, one alternative would be to require some special CAPS if
>> userspace tries to set the mbus format directly, or to recommend userspace
>> to create media controller nodes with 0600 permission.
> 
> That's not really a media controller issue. Whether formats can be changed 
> during streaming is a driver decision. The OMAP3 ISP driver won't allow 
> formats to be changed during streaming. If the hardware allows for such format 
> changes, drivers can implement support for that and make sure that no buffer 
> overflow will occur.

Such issues is caused by having two API's that allow format changes, one that
does it device-based, and another one doing it subdev-based.

Ok, drivers can implementing locks to prevent such troubles, but, without
the core providing a reliable mechanism, it is hard to implement a
correct lock. 

For example, let's suppose that some driver is using mt9m111 subdev (I just picked 
one random sensor that supports lots of MBUS formats). There's nothing
there preventing a subdev call for it to change mbus format while streaming.
Worse than that, the sensor driver has no way to block it, as it doesn't
know that the bridge driver is streaming or not.

The code at subdev_do_ioctl() is just:

case VIDIOC_SUBDEV_S_FMT: {
        struct v4l2_subdev_format *format = arg;

        if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
            format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
                return -EINVAL;

        if (format->pad >= sd->entity.num_pads)
                return -EINVAL;
 
        return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
}

So, mc core won't be preventing it.

So, I can't see how such subdev request would be implementing a logic to
return -EBUSY on those cases.

Mauro.

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

[Laurent Pinchart]

Hi Mauro,

On Sunday 06 March 2011 14:32:44 Mauro Carvalho Chehab wrote:
> Em 06-03-2011 08:38, Laurent Pinchart escreveu:
> > On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
> >> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
> >> 
> >> A somewhat unrelated question that occurred to me today: what happens
> >> when a format change happens while streaming?
> >> 
> >> Considering that some formats need more bits than others, this could
> >> lead into buffer overflows, either internally at the device or
> >> externally, on bridges that just forward whatever it receives to the
> >> DMA buffers (there are some that just does that). I didn't see anything
> >> inside the mc code preventing such condition to happen, and probably
> >> implementing it won't be an easy job. So, one alternative would be to
> >> require some special CAPS if userspace tries to set the mbus format
> >> directly, or to recommend userspace to create media controller nodes
> >> with 0600 permission.
> > 
> > That's not really a media controller issue. Whether formats can be
> > changed during streaming is a driver decision. The OMAP3 ISP driver
> > won't allow formats to be changed during streaming. If the hardware
> > allows for such format changes, drivers can implement support for that
> > and make sure that no buffer overflow will occur.
> 
> Such issues is caused by having two API's that allow format changes, one
> that does it device-based, and another one doing it subdev-based.
> 
> Ok, drivers can implementing locks to prevent such troubles, but, without
> the core providing a reliable mechanism, it is hard to implement a
> correct lock.
> 
> For example, let's suppose that some driver is using mt9m111 subdev (I just
> picked one random sensor that supports lots of MBUS formats). There's
> nothing there preventing a subdev call for it to change mbus format while
> streaming. Worse than that, the sensor driver has no way to block it, as
> it doesn't know that the bridge driver is streaming or not.
> 
> The code at subdev_do_ioctl() is just:
> 
> case VIDIOC_SUBDEV_S_FMT: {
>         struct v4l2_subdev_format *format = arg;
> 
>         if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
>             format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
>                 return -EINVAL;
> 
>         if (format->pad >= sd->entity.num_pads)
>                 return -EINVAL;
> 
>         return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
> }
> 
> So, mc core won't be preventing it.
> 
> So, I can't see how such subdev request would be implementing a logic to
> return -EBUSY on those cases.

Drivers can use the media_device graph_mutex to serialize format and stream 
management calls. A finer grain locking mechanism implemented in the core 
might be better, but we're not stuck without a solution at the moment.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 06-03-2011 14:21, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> On Sunday 06 March 2011 14:32:44 Mauro Carvalho Chehab wrote:
>> Em 06-03-2011 08:38, Laurent Pinchart escreveu:
>>> On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
>>>> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
>>>>
>>>> A somewhat unrelated question that occurred to me today: what happens
>>>> when a format change happens while streaming?
>>>>
>>>> Considering that some formats need more bits than others, this could
>>>> lead into buffer overflows, either internally at the device or
>>>> externally, on bridges that just forward whatever it receives to the
>>>> DMA buffers (there are some that just does that). I didn't see anything
>>>> inside the mc code preventing such condition to happen, and probably
>>>> implementing it won't be an easy job. So, one alternative would be to
>>>> require some special CAPS if userspace tries to set the mbus format
>>>> directly, or to recommend userspace to create media controller nodes
>>>> with 0600 permission.
>>>
>>> That's not really a media controller issue. Whether formats can be
>>> changed during streaming is a driver decision. The OMAP3 ISP driver
>>> won't allow formats to be changed during streaming. If the hardware
>>> allows for such format changes, drivers can implement support for that
>>> and make sure that no buffer overflow will occur.
>>
>> Such issues is caused by having two API's that allow format changes, one
>> that does it device-based, and another one doing it subdev-based.
>>
>> Ok, drivers can implementing locks to prevent such troubles, but, without
>> the core providing a reliable mechanism, it is hard to implement a
>> correct lock.
>>
>> For example, let's suppose that some driver is using mt9m111 subdev (I just
>> picked one random sensor that supports lots of MBUS formats). There's
>> nothing there preventing a subdev call for it to change mbus format while
>> streaming. Worse than that, the sensor driver has no way to block it, as
>> it doesn't know that the bridge driver is streaming or not.
>>
>> The code at subdev_do_ioctl() is just:
>>
>> case VIDIOC_SUBDEV_S_FMT: {
>>         struct v4l2_subdev_format *format = arg;
>>
>>         if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
>>             format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
>>                 return -EINVAL;
>>
>>         if (format->pad >= sd->entity.num_pads)
>>                 return -EINVAL;
>>
>>         return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
>> }
>>
>> So, mc core won't be preventing it.
>>
>> So, I can't see how such subdev request would be implementing a logic to
>> return -EBUSY on those cases.
> 
> Drivers can use the media_device graph_mutex to serialize format and stream 
> management calls. A finer grain locking mechanism implemented in the core 
> might be better, but we're not stuck without a solution at the moment.

Ok, i see. This is not the best world, as I suspect that developers will
just try to enable media controller for a few devices without taking enough
care to avoid buffer overflows.
While we don't have a better way for doing it, please add a note at the kernel 
api doc saying about that, briefly describing how to properly lock it, because
this is not obvious at all.

Cheers,
Mauro

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

[Mauro Carvalho Chehab]

Em 07-03-2011 09:02, Hans Verkuil escreveu:
> On Monday, March 07, 2011 12:50:28 Mauro Carvalho Chehab wrote:
> 
>> Em 06-03-2011 14:21, Laurent Pinchart escreveu:
> 
>> > Hi Mauro,
> 
>> >
> 
>> > On Sunday 06 March 2011 14:32:44 Mauro Carvalho Chehab wrote:
> 
>> >> Em 06-03-2011 08:38, Laurent Pinchart escreveu:
> 
>> >>> On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
> 
>> >>>> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
> 
>> >>>>
> 
>> >>>> A somewhat unrelated question that occurred to me today: what happens
> 
>> >>>> when a format change happens while streaming?
> 
>> >>>>
> 
>> >>>> Considering that some formats need more bits than others, this could
> 
>> >>>> lead into buffer overflows, either internally at the device or
> 
>> >>>> externally, on bridges that just forward whatever it receives to the
> 
>> >>>> DMA buffers (there are some that just does that). I didn't see anything
> 
>> >>>> inside the mc code preventing such condition to happen, and probably
> 
>> >>>> implementing it won't be an easy job. So, one alternative would be to
> 
>> >>>> require some special CAPS if userspace tries to set the mbus format
> 
>> >>>> directly, or to recommend userspace to create media controller nodes
> 
>> >>>> with 0600 permission.
> 
>> >>>
> 
>> >>> That's not really a media controller issue. Whether formats can be
> 
>> >>> changed during streaming is a driver decision. The OMAP3 ISP driver
> 
>> >>> won't allow formats to be changed during streaming. If the hardware
> 
>> >>> allows for such format changes, drivers can implement support for that
> 
>> >>> and make sure that no buffer overflow will occur.
> 
>> >>
> 
>> >> Such issues is caused by having two API's that allow format changes, one
> 
>> >> that does it device-based, and another one doing it subdev-based.
> 
>> >>
> 
>> >> Ok, drivers can implementing locks to prevent such troubles, but, without
> 
>> >> the core providing a reliable mechanism, it is hard to implement a
> 
>> >> correct lock.
> 
>> >>
> 
>> >> For example, let's suppose that some driver is using mt9m111 subdev (I just
> 
>> >> picked one random sensor that supports lots of MBUS formats). There's
> 
>> >> nothing there preventing a subdev call for it to change mbus format while
> 
>> >> streaming. Worse than that, the sensor driver has no way to block it, as
> 
>> >> it doesn't know that the bridge driver is streaming or not.
> 
>> >>
> 
>> >> The code at subdev_do_ioctl() is just:
> 
>> >>
> 
>> >> case VIDIOC_SUBDEV_S_FMT: {
> 
>> >> struct v4l2_subdev_format *format = arg;
> 
>> >>
> 
>> >> if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> 
>> >> format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> 
>> >> return -EINVAL;
> 
>> >>
> 
>> >> if (format->pad >= sd->entity.num_pads)
> 
>> >> return -EINVAL;
> 
>> >>
> 
>> >> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
> 
>> >> }
> 
>> >>
> 
>> >> So, mc core won't be preventing it.
> 
>> >>
> 
>> >> So, I can't see how such subdev request would be implementing a logic to
> 
>> >> return -EBUSY on those cases.
> 
>> >
> 
>> > Drivers can use the media_device graph_mutex to serialize format and stream
> 
>> > management calls. A finer grain locking mechanism implemented in the core
> 
>> > might be better, but we're not stuck without a solution at the moment.
> 
> Am I missing something here? Isn't it as simple as remembering whether the
> 
> subdev is in streaming mode (s_stream(1) was called) or not? When streaming
> 
> any attempt to change the format should return an error (unless the hardware
> 
> can handle it, of course).
> 
> This is the same as for the 'regular' V4L2 API.

Not all subdevs implement s_stream, and I suspect that not all bridge drivers
calls it. The random example I've looked didn't implement (mt9m111.c), but even
some that implements it (like mt9m001.c) currently don't store the stream status
or use it to prevent a format change.

At the moment we open the possibility to directly access the subdev, 
developers might think that all they need to use the new API is to enable
the subdev to create subdev nodes (btw, the first mc patch series were enabling
it by default). However, opening subdev access without address such issues will
lead into a security breach, as buffer overflows will happen if hardware can't 
handle format changes in the middle of a streaming [1].

Also, a lock there will only work if properly implemented at the bridge driver,
as a bridge driver that implement the media controller should implement something
like the following sequence (at VIDIOC_REQBUFS):

	lock_format_changes_at_subdev();			/* step 1 */
	get_subdev_formats();					/* step 2 */
	program_bridge_to follow_subdev_format_and_s_fmt();	/* step 3 */
	reserve_memory();					/* step 4 */
	start_streaming();					/* step 5 */


In the above, s_stream should be called at the step 1, and not at step 5, as,
otherwise, a race condition will happen, if a MBUS format change happens between
step 1 and 5.

Cheers,
Mauro.

[1] Btw, is there any hardware that supports a random change at the input format provided
by a subdevice without any need of reconfiguring anything, and keeping providing the
same output format? It seems doubtful for me, as hardware would need to have a format
auto-detection logic, and some changes are impossible to track (for example, changing
chroma order from YUYV to YVYU or from RGB to BGR can't be auto-detected). Perhaps the
better would be to just block such changes while streaming.


Cheers,
Mauro

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

[Mauro Carvalho Chehab]

Em 07-03-2011 10:00, Mauro Carvalho Chehab escreveu:
> Em 07-03-2011 09:02, Hans Verkuil escreveu:
>> On Monday, March 07, 2011 12:50:28 Mauro Carvalho Chehab wrote:
>>
>>> Em 06-03-2011 14:21, Laurent Pinchart escreveu:
>>
>>>> Hi Mauro,
>>
>>>>
>>
>>>> On Sunday 06 March 2011 14:32:44 Mauro Carvalho Chehab wrote:
>>
>>>>> Em 06-03-2011 08:38, Laurent Pinchart escreveu:
>>
>>>>>> On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
>>
>>>>>>> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
>>
>>>>>>>
>>
>>>>>>> A somewhat unrelated question that occurred to me today: what happens
>>
>>>>>>> when a format change happens while streaming?
>>
>>>>>>>
>>
>>>>>>> Considering that some formats need more bits than others, this could
>>
>>>>>>> lead into buffer overflows, either internally at the device or
>>
>>>>>>> externally, on bridges that just forward whatever it receives to the
>>
>>>>>>> DMA buffers (there are some that just does that). I didn't see anything
>>
>>>>>>> inside the mc code preventing such condition to happen, and probably
>>
>>>>>>> implementing it won't be an easy job. So, one alternative would be to
>>
>>>>>>> require some special CAPS if userspace tries to set the mbus format
>>
>>>>>>> directly, or to recommend userspace to create media controller nodes
>>
>>>>>>> with 0600 permission.
>>
>>>>>>
>>
>>>>>> That's not really a media controller issue. Whether formats can be
>>
>>>>>> changed during streaming is a driver decision. The OMAP3 ISP driver
>>
>>>>>> won't allow formats to be changed during streaming. If the hardware
>>
>>>>>> allows for such format changes, drivers can implement support for that
>>
>>>>>> and make sure that no buffer overflow will occur.
>>
>>>>>
>>
>>>>> Such issues is caused by having two API's that allow format changes, one
>>
>>>>> that does it device-based, and another one doing it subdev-based.
>>
>>>>>
>>
>>>>> Ok, drivers can implementing locks to prevent such troubles, but, without
>>
>>>>> the core providing a reliable mechanism, it is hard to implement a
>>
>>>>> correct lock.
>>
>>>>>
>>
>>>>> For example, let's suppose that some driver is using mt9m111 subdev (I just
>>
>>>>> picked one random sensor that supports lots of MBUS formats). There's
>>
>>>>> nothing there preventing a subdev call for it to change mbus format while
>>
>>>>> streaming. Worse than that, the sensor driver has no way to block it, as
>>
>>>>> it doesn't know that the bridge driver is streaming or not.
>>
>>>>>
>>
>>>>> The code at subdev_do_ioctl() is just:
>>
>>>>>
>>
>>>>> case VIDIOC_SUBDEV_S_FMT: {
>>
>>>>> struct v4l2_subdev_format *format = arg;
>>
>>>>>
>>
>>>>> if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
>>
>>>>> format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
>>
>>>>> return -EINVAL;
>>
>>>>>
>>
>>>>> if (format->pad >= sd->entity.num_pads)
>>
>>>>> return -EINVAL;
>>
>>>>>
>>
>>>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
>>
>>>>> }
>>
>>>>>
>>
>>>>> So, mc core won't be preventing it.
>>
>>>>>
>>
>>>>> So, I can't see how such subdev request would be implementing a logic to
>>
>>>>> return -EBUSY on those cases.
>>
>>>>
>>
>>>> Drivers can use the media_device graph_mutex to serialize format and stream
>>
>>>> management calls. A finer grain locking mechanism implemented in the core
>>
>>>> might be better, but we're not stuck without a solution at the moment.
>>
>> Am I missing something here? Isn't it as simple as remembering whether the
>>
>> subdev is in streaming mode (s_stream(1) was called) or not? When streaming
>>
>> any attempt to change the format should return an error (unless the hardware
>>
>> can handle it, of course).
>>
>> This is the same as for the 'regular' V4L2 API.
> 
> Not all subdevs implement s_stream, and I suspect that not all bridge drivers
> calls it. The random example I've looked didn't implement (mt9m111.c), but even
> some that implements it (like mt9m001.c) currently don't store the stream status
> or use it to prevent a format change.
> 
> At the moment we open the possibility to directly access the subdev, 
> developers might think that all they need to use the new API is to enable
> the subdev to create subdev nodes (btw, the first mc patch series were enabling
> it by default). However, opening subdev access without address such issues will
> lead into a security breach, as buffer overflows will happen if hardware can't 
> handle format changes in the middle of a streaming [1].
> 
> Also, a lock there will only work if properly implemented at the bridge driver,
> as a bridge driver that implement the media controller should implement something
> like the following sequence (at VIDIOC_REQBUFS):
> 
> 	lock_format_changes_at_subdev();			/* step 1 */
> 	get_subdev_formats();					/* step 2 */
> 	program_bridge_to follow_subdev_format_and_s_fmt();	/* step 3 */
> 	reserve_memory();					/* step 4 */
> 	start_streaming();					/* step 5 */
> 
> 
> In the above, s_stream should be called at the step 1, and not at step 5, as,
> otherwise, a race condition will happen, if a MBUS format change happens between
> step 1 and 5.

In time: assuming that s_stream would implement such lock. 

Also: it this is a mandatory requirement, it should be part of API documentation.
Otherwise, we'll have subdevs that will implement the lock using one way, and others
using a different way, creating an mess at the subdevs in a way that some subdevs
will work with bridge A, but not with bridge B, that has a different requirement
for such lock.

Cheers,
Mauro.

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

[Laurent Pinchart]

Hi Mauro,

On Monday 07 March 2011 14:00:20 Mauro Carvalho Chehab wrote:
> Em 07-03-2011 09:02, Hans Verkuil escreveu:
> > On Monday, March 07, 2011 12:50:28 Mauro Carvalho Chehab wrote:
> >> Em 06-03-2011 14:21, Laurent Pinchart escreveu:
> >> > On Sunday 06 March 2011 14:32:44 Mauro Carvalho Chehab wrote:
> >> >> Em 06-03-2011 08:38, Laurent Pinchart escreveu:
> >> >>> On Sunday 06 March 2011 11:56:04 Mauro Carvalho Chehab wrote:
> >> >>>> Em 05-03-2011 20:23, Sylwester Nawrocki escreveu:
> >> >>>> 
> >> >>>> A somewhat unrelated question that occurred to me today: what
> >> >>>> happens when a format change happens while streaming?
> >> >>>> 
> >> >>>> Considering that some formats need more bits than others, this could
> >> >>>> lead into buffer overflows, either internally at the device or
> >> >>>> externally, on bridges that just forward whatever it receives to the
> >> >>>> DMA buffers (there are some that just does that). I didn't see
> >> >>>> anything inside the mc code preventing such condition to happen, and
> >> >>>> probably implementing it won't be an easy job. So, one alternative
> >> >>>> would be to require some special CAPS if userspace tries to set the
> >> >>>> mbus format directly, or to recommend userspace to create media
> >> >>>> controller nodes with 0600 permission.
> >> >>> 
> >> >>> That's not really a media controller issue. Whether formats can be
> >> >>> changed during streaming is a driver decision. The OMAP3 ISP driver
> >> >>> won't allow formats to be changed during streaming. If the hardware
> >> >>> allows for such format changes, drivers can implement support for
> >> >>> that and make sure that no buffer overflow will occur.
> >> >> 
> >> >> Such issues is caused by having two API's that allow format changes,
> >> >> one that does it device-based, and another one doing it subdev-based.
> >> >> 
> >> >> Ok, drivers can implementing locks to prevent such troubles, but,
> >> >> without the core providing a reliable mechanism, it is hard to
> >> >> implement a correct lock.
> >> >> 
> >> >> For example, let's suppose that some driver is using mt9m111 subdev
> >> >> (I just picked one random sensor that supports lots of MBUS formats).
> >> >> There's nothing there preventing a subdev call for it to change mbus
> >> >> format while streaming. Worse than that, the sensor driver has no way
> >> >> to block it, as it doesn't know that the bridge driver is streaming or
> >> >> not.
> >> >> 
> >> >> The code at subdev_do_ioctl() is just:
> >> >> 
> >> >> case VIDIOC_SUBDEV_S_FMT: {
> >> >> struct v4l2_subdev_format *format = arg;
> >> >> 
> >> >> if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> >> >> format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> >> >> return -EINVAL;
> >> >> 
> >> >> if (format->pad >= sd->entity.num_pads)
> >> >> return -EINVAL;
> >> >> 
> >> >> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
> >> >> }
> >> >> 
> >> >> So, mc core won't be preventing it.
> >> >> 
> >> >> So, I can't see how such subdev request would be implementing a logic
> >> >> to return -EBUSY on those cases.
> >> > 
> >> > Drivers can use the media_device graph_mutex to serialize format and
> >> > stream management calls. A finer grain locking mechanism implemented in
> >> > the core might be better, but we're not stuck without a solution at the
> >> > moment.
> > 
> > Am I missing something here? Isn't it as simple as remembering whether
> > the subdev is in streaming mode (s_stream(1) was called) or not? When
> > streaming any attempt to change the format should return an error (unless
> > the hardware can handle it, of course).
> > 
> > This is the same as for the 'regular' V4L2 API.
> 
> Not all subdevs implement s_stream, and I suspect that not all bridge
> drivers calls it. The random example I've looked didn't implement
> (mt9m111.c), but even some that implements it (like mt9m001.c) currently
> don't store the stream status or use it to prevent a format change.

Those drivers don't implement subdev pad-level operations, so they won't work 
with the media controller anyway. They will need to be fixed, and locking can 
then be implemented.

> At the moment we open the possibility to directly access the subdev,
> developers might think that all they need to use the new API is to enable
> the subdev to create subdev nodes (btw, the first mc patch series were
> enabling it by default). However, opening subdev access without address
> such issues will lead into a security breach, as buffer overflows will
> happen if hardware can't handle format changes in the middle of a
> streaming [1].
> 
> Also, a lock there will only work if properly implemented at the bridge
> driver, as a bridge driver that implement the media controller should
> implement something like the following sequence (at VIDIOC_REQBUFS):
> 
> 	lock_format_changes_at_subdev();			/* step 1 */
> 	get_subdev_formats();					/* step 2 */
> 	program_bridge_to follow_subdev_format_and_s_fmt();	/* step 3 */
> 	reserve_memory();					/* step 4 */
> 	start_streaming();					/* step 5 */
> 
> 
> In the above, s_stream should be called at the step 1, and not at step 5,
> as, otherwise, a race condition will happen, if a MBUS format change
> happens between step 1 and 5.

s_stream should be called at step 5, but media_entity_pipeline_start() should 
be called at step 1. Subdev drivers can then check the media_entity 
stream_count field (protected by the media_device graph_mutex lock) and refuse 
to change formats if the stream count is > 0. This is explained in 
Documentation/media-framework.txt, and the OMAP3 ISP driver implements this.

Sensor drivers will obviously need to be fixed, but that's not a real issue as 
they won't work with the OMAP3 ISP as-is anyway.

> [1] Btw, is there any hardware that supports a random change at the input
> format provided by a subdevice without any need of reconfiguring anything,
> and keeping providing the same output format? It seems doubtful for me, as
> hardware would need to have a format auto-detection logic, and some
> changes are impossible to track (for example, changing chroma order from
> YUYV to YVYU or from RGB to BGR can't be auto-detected). Perhaps the
> better would be to just block such changes while streaming.

I think it makes sense to just prevent those changes, especially as we have no 
real use case for such dynamic reconfiguration. Let's not try to over-engineer 
a solution that nobody will use.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
> 
>   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
...
> Laurent Pinchart (36):
...
>       v4l: subdev: Generic ioctl supportFrom 57b36ef1b9733124f3e04e6e2c06cf358051e209 Mon Sep 17 00:00:00 2001

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Fri, 26 Feb 2010 16:23:10 +0100
Subject: v4l: subdev: Generic ioctl support
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>

Instead of returning an error when receiving an ioctl call with an
unsupported command, forward the call to the subdev core::ioctl handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 Documentation/video4linux/v4l2-framework.txt |    5 +++++
 drivers/media/video/v4l2-subdev.c            |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 77d96f4..f2df31b 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -405,6 +405,11 @@ VIDIOC_UNSUBSCRIBE_EVENT
 	To properly support events, the poll() file operation is also
 	implemented.
 
+Private ioctls
+
+	All ioctls not in the above list are passed directly to the sub-device
+	driver through the core::ioctl operation.
+
 
 I2C sub-device drivers
 ----------------------
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 6e76f73..0b80644 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -276,7 +276,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	}
 #endif
 	default:
-		return -ENOIOCTLCMD;
+		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
 	}
 
 	return 0;
-- 
1.7.1

I don't like to apply a patch like that without a very good explanation
about why it is needed and where it is used. Private ioctls are generally
a very bad idea, as they lack proper documentation.

Also, we may quickly loose the control about what ioctl's are valid for
subdevs, as the same code could also be used to accept a video (or audio)
ioctl directly into a subdev. 

So, IMO, the better is to manually add ioctl's there as they're
needed inside subdevs. I'll not apply this patch on my tree for now.

Is it currently needed for omap3isp? If so, what are the used ioctls
inside omap3isp?

>       v4l: Add subdev sensor g_skip_frames operation
>       v4l: Add 8-bit YUYV on 16-bit bus and SGRBG10 media bus pixel codes
>       v4l: Add remaining RAW10 patterns w DPCM pixel code variants
>       v4l: Add missing 12 bits bayer media bus formats
>       v4l: Add 12 bits bayer pixel formats

Had you document all those newly-added formats at the API? Is there a way
to double check if something is missed there? With the V4L2 API, as we
add videodev2.h header, and we create dynamic links between the .h file
and the specs, DocBook warns if some FOURCC is missed. From our experience,
it is common that people add stuff at the header files, but forget to add
the corresponding documentation for it. We need something similar for
MBUS formats, as I suspect that we'll also have lots of additions there.

Ok, I finished the review of the 36 media controller patches. I'll now
start to dig into omap3isp patches.

Cheers,
Mauro

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

[Mauro Carvalho Chehab]

Em 04-03-2011 17:49, Mauro Carvalho Chehab escreveu:
> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
>> > Hi Mauro,
>> > 
>> > The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
>> > 
>> >   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
>> > 
>> > are available in the git repository at:
>> >   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
> ...
>> > Laurent Pinchart (36):
> ...
>> >       v4l: subdev: Generic ioctl supportFrom 57b36ef1b9733124f3e04e6e2c06cf358051e209 Mon Sep 17 00:00:00 2001
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Date: Fri, 26 Feb 2010 16:23:10 +0100
> Subject: v4l: subdev: Generic ioctl support
> Cc: Linux Media Mailing List <linux-media@vger.kernel.org>
> 
> Instead of returning an error when receiving an ioctl call with an
> unsupported command, forward the call to the subdev core::ioctl handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> ---
>  Documentation/video4linux/v4l2-framework.txt |    5 +++++
>  drivers/media/video/v4l2-subdev.c            |    2 +-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
> index 77d96f4..f2df31b 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -405,6 +405,11 @@ VIDIOC_UNSUBSCRIBE_EVENT
>  	To properly support events, the poll() file operation is also
>  	implemented.
>  
> +Private ioctls
> +
> +	All ioctls not in the above list are passed directly to the sub-device
> +	driver through the core::ioctl operation.
> +
>  
>  I2C sub-device drivers
>  ----------------------
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index 6e76f73..0b80644 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -276,7 +276,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	}
>  #endif
>  	default:
> -		return -ENOIOCTLCMD;
> +		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
>  	}
>  
>  	return 0;

> I don't like to apply a patch like that without a very good explanation
> about why it is needed and where it is used. Private ioctls are generally
> a very bad idea, as they lack proper documentation.
> 
> Also, we may quickly loose the control about what ioctl's are valid for
> subdevs, as the same code could also be used to accept a video (or audio)
> ioctl directly into a subdev. 
> 
> So, IMO, the better is to manually add ioctl's there as they're
> needed inside subdevs. I'll not apply this patch on my tree for now.
> 
> Is it currently needed for omap3isp? If so, what are the used ioctls
> inside omap3isp?

Ok, I got my answer at the omap3isp patches:

# + * Private IOCTLs
# + * 
# + * VIDIOC_OMAP3ISP_CCDC_CFG: Set CCDC configuration
# + * VIDIOC_OMAP3ISP_PRV_CFG: Set preview engine configuration
# + * VIDIOC_OMAP3ISP_AEWB_CFG: Set AEWB module configuration
# + * VIDIOC_OMAP3ISP_HIST_CFG: Set histogram module configuration
# + * VIDIOC_OMAP3ISP_AF_CFG: Set auto-focus module configuration
# + * VIDIOC_OMAP3ISP_STAT_REQ: Read statistics (AEWB/AF/histogram) data
# + * VIDIOC_OMAP3ISP_STAT_EN: Enable/disable a statistics module

What are those ioctl's meant to do? In special, they seem to
be needed to make the device work. As I don't have any device here to
test, it is hard to be sure about that, so I'm tempted to nack this 
patch as-is.

Also, where are those private ioctl's documented?

Cheers,
Mauro.

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

[Hans Verkuil]

On Friday, March 04, 2011 21:49:32 Mauro Carvalho Chehab wrote:
> Em 03-03-2011 07:25, Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > The following changes since commit 88a763df226facb74fdb254563e30e9efb64275c:
> > 
> >   [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300)
> > 
> > are available in the git repository at:
> >   git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp
> ...
> > Laurent Pinchart (36):
> ...
> >       v4l: subdev: Generic ioctl supportFrom 57b36ef1b9733124f3e04e6e2c06cf358051e209 Mon Sep 17 00:00:00 2001
> 
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Date: Fri, 26 Feb 2010 16:23:10 +0100
> Subject: v4l: subdev: Generic ioctl support
> Cc: Linux Media Mailing List <linux-media@vger.kernel.org>
> 
> Instead of returning an error when receiving an ioctl call with an
> unsupported command, forward the call to the subdev core::ioctl handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> ---
>  Documentation/video4linux/v4l2-framework.txt |    5 +++++
>  drivers/media/video/v4l2-subdev.c            |    2 +-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
> index 77d96f4..f2df31b 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -405,6 +405,11 @@ VIDIOC_UNSUBSCRIBE_EVENT
>  	To properly support events, the poll() file operation is also
>  	implemented.
>  
> +Private ioctls
> +
> +	All ioctls not in the above list are passed directly to the sub-device
> +	driver through the core::ioctl operation.
> +
>  
>  I2C sub-device drivers
>  ----------------------
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index 6e76f73..0b80644 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -276,7 +276,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	}
>  #endif
>  	default:
> -		return -ENOIOCTLCMD;
> +		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
>  	}
>  
>  	return 0;
> -- 
> 1.7.1
> 
> I don't like to apply a patch like that without a very good explanation
> about why it is needed and where it is used. Private ioctls are generally
> a very bad idea, as they lack proper documentation.

Private ioctls are perfectly valid in sub-devices as they will only be used
by applications that are written specifically for that hardware. Provided of
course that they are documented reasonably well (the less public documentation
there is, the more detailed the documentation should be) and that it is possible
to still use the hardware (if in a more limited capability) without using them.

> Also, we may quickly loose the control about what ioctl's are valid for
> subdevs, as the same code could also be used to accept a video (or audio)
> ioctl directly into a subdev. 

That's why we do reviews. It's pretty easy to review: you just have to pay
special attention to the ioctl core op in the subdev driver.

> So, IMO, the better is to manually add ioctl's there as they're
> needed inside subdevs. I'll not apply this patch on my tree for now.

Yuck, then we pollute the core v4l2-subdev.c file with hardware-specific
ioctls and subdev ops.

We do the same for V4L2 drivers (vidioc_default), which works quite well.
Obviously, there are problems with private ioctls in some legacy drivers.
But in those days there was much less reviewing going on.

Regards,

	Hans

> Is it currently needed for omap3isp? If so, what are the used ioctls
> inside omap3isp?
> 
> > >       v4l: Add subdev sensor g_skip_frames operation
> >       v4l: Add 8-bit YUYV on 16-bit bus and SGRBG10 media bus pixel codes
> >       v4l: Add remaining RAW10 patterns w DPCM pixel code variants
> >       v4l: Add missing 12 bits bayer media bus formats
> >       v4l: Add 12 bits bayer pixel formats
> 
> Had you document all those newly-added formats at the API? Is there a way
> to double check if something is missed there? With the V4L2 API, as we
> add videodev2.h header, and we create dynamic links between the .h file
> and the specs, DocBook warns if some FOURCC is missed. From our experience,
> it is common that people add stuff at the header files, but forget to add
> the corresponding documentation for it. We need something similar for
> MBUS formats, as I suspect that we'll also have lots of additions there.
> 
> Ok, I finished the review of the 36 media controller patches. I'll now
> start to dig into omap3isp patches.
> 
> Cheers,
> 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
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

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

[Mauro Carvalho Chehab]

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.

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

[David Cohen]

Hi Mauro,

On Sat, Mar 5, 2011 at 12:16 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> 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;

Yes.

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

The API is pretty close to what is found on public OMAP3 TRM. I'd say
it's almost to fill registers through a userspace API.

>        3) there are no patents denying or charging for the usage and/or
> distribution/redistribution of the Kernel with the provided kernel driver;

I'd say there's no patent / charge for usage or redistribution. But
that's lawyer stuff. :/

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

It depends on the sensor as well, but in general should work with a
reasonable quality without using statistic modules.

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

The private ioctl are used mostly on statistic modules (3A and
Histogram). But it's used for CCDC and Preview modules configuration
too.

Regards,

David

>
>
> Cheers,
> 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: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver

[Mauro Carvalho Chehab]

Em 04-03-2011 19:33, David Cohen escreveu:
> Hi Mauro,
> 
> On Sat, Mar 5, 2011 at 12:16 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> 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;
> 
> Yes.
> 
>>        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);
> 
> The API is pretty close to what is found on public OMAP3 TRM. I'd say
> it's almost to fill registers through a userspace API.

Ok, so a simple patch adding a txt file documenting those private ioctls
to Documentation/video4linux explaining how to use them and pointing to 
OMAP3 TRM documentation is enough.

>>        3) there are no patents denying or charging for the usage and/or
>> distribution/redistribution of the Kernel with the provided kernel driver;
> 
> I'd say there's no patent / charge for usage or redistribution. But
> that's lawyer stuff. :/
> 
>>        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).
> 
> It depends on the sensor as well, but in general should work with a
> reasonable quality without using statistic modules.
> 
>>
>> 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.
> 
> The private ioctl are used mostly on statistic modules (3A and
> Histogram). But it's used for CCDC and Preview modules configuration
> too.

The issue here is: what if no CCDC and no Preview initialization ever happen?

Cheers,
Mauro

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

[David Cohen]

On Sat, Mar 5, 2011 at 12:43 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 04-03-2011 19:33, David Cohen escreveu:
>> Hi Mauro,
>>
>> On Sat, Mar 5, 2011 at 12:16 AM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> 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;
>>
>> Yes.
>>
>>>        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);
>>
>> The API is pretty close to what is found on public OMAP3 TRM. I'd say
>> it's almost to fill registers through a userspace API.
>
> Ok, so a simple patch adding a txt file documenting those private ioctls
> to Documentation/video4linux explaining how to use them and pointing to
> OMAP3 TRM documentation is enough.

Ok. I'll provide a patch for it.

>
>>>        3) there are no patents denying or charging for the usage and/or
>>> distribution/redistribution of the Kernel with the provided kernel driver;
>>
>> I'd say there's no patent / charge for usage or redistribution. But
>> that's lawyer stuff. :/
>>
>>>        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).
>>
>> It depends on the sensor as well, but in general should work with a
>> reasonable quality without using statistic modules.
>>
>>>
>>> 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.
>>
>> The private ioctl are used mostly on statistic modules (3A and
>> Histogram). But it's used for CCDC and Preview modules configuration
>> too.
>
> The issue here is: what if no CCDC and no Preview initialization ever happen?

Currently the driver does not support such situation. Either all
modules successfully initialize or ISP driver doesn't probe correctly.

Regards,

Davd

>
> Cheers,
> Mauro
>

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

[Mauro Carvalho Chehab]

Em 04-03-2011 19:49, David Cohen escreveu:
> On Sat, Mar 5, 2011 at 12:43 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 04-03-2011 19:33, David Cohen escreveu:
>>> Hi Mauro,
>>>
>>> On Sat, Mar 5, 2011 at 12:16 AM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> 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;
>>>
>>> Yes.
>>>
>>>>        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);
>>>
>>> The API is pretty close to what is found on public OMAP3 TRM. I'd say
>>> it's almost to fill registers through a userspace API.
>>
>> Ok, so a simple patch adding a txt file documenting those private ioctls
>> to Documentation/video4linux explaining how to use them and pointing to
>> OMAP3 TRM documentation is enough.
> 
> Ok. I'll provide a patch for it.

Ok, thanks.
> 
>>
>>>>        3) there are no patents denying or charging for the usage and/or
>>>> distribution/redistribution of the Kernel with the provided kernel driver;
>>>
>>> I'd say there's no patent / charge for usage or redistribution. But
>>> that's lawyer stuff. :/
>>>
>>>>        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).
>>>
>>> It depends on the sensor as well, but in general should work with a
>>> reasonable quality without using statistic modules.
>>>
>>>>
>>>> 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.
>>>
>>> The private ioctl are used mostly on statistic modules (3A and
>>> Histogram). But it's used for CCDC and Preview modules configuration
>>> too.
>>
>> The issue here is: what if no CCDC and no Preview initialization ever happen?
> 
> Currently the driver does not support such situation. Either all
> modules successfully initialize or ISP driver doesn't probe correctly.

I'm not sure if I understood (or if you understood my question ;) )

What I'm asking is: what happens if the private ioctl's for CCDC and Preview
initialization are never called (or any other private/subdev ioctl)? 
The device will work or not?

Cheers,
Mauro

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

[David Cohen]

On Sat, Mar 5, 2011 at 1:49 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 04-03-2011 19:49, David Cohen escreveu:
>> On Sat, Mar 5, 2011 at 12:43 AM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 04-03-2011 19:33, David Cohen escreveu:
>>>> Hi Mauro,
>>>>
>>>> On Sat, Mar 5, 2011 at 12:16 AM, Mauro Carvalho Chehab
>>>> <mchehab@redhat.com> wrote:
>>>>> 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;
>>>>
>>>> Yes.
>>>>
>>>>>        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);
>>>>
>>>> The API is pretty close to what is found on public OMAP3 TRM. I'd say
>>>> it's almost to fill registers through a userspace API.
>>>
>>> Ok, so a simple patch adding a txt file documenting those private ioctls
>>> to Documentation/video4linux explaining how to use them and pointing to
>>> OMAP3 TRM documentation is enough.
>>
>> Ok. I'll provide a patch for it.
>
> Ok, thanks.
>>
>>>
>>>>>        3) there are no patents denying or charging for the usage and/or
>>>>> distribution/redistribution of the Kernel with the provided kernel driver;
>>>>
>>>> I'd say there's no patent / charge for usage or redistribution. But
>>>> that's lawyer stuff. :/
>>>>
>>>>>        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).
>>>>
>>>> It depends on the sensor as well, but in general should work with a
>>>> reasonable quality without using statistic modules.
>>>>
>>>>>
>>>>> 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.
>>>>
>>>> The private ioctl are used mostly on statistic modules (3A and
>>>> Histogram). But it's used for CCDC and Preview modules configuration
>>>> too.
>>>
>>> The issue here is: what if no CCDC and no Preview initialization ever happen?
>>
>> Currently the driver does not support such situation. Either all
>> modules successfully initialize or ISP driver doesn't probe correctly.
>
> I'm not sure if I understood (or if you understood my question ;) )
>
> What I'm asking is: what happens if the private ioctl's for CCDC and Preview
> initialization are never called (or any other private/subdev ioctl)?
> The device will work or not?

It seems I misunderstood you question for the first time. The answer
is yes. Some functionalities will be disabled and others initialized
with valid default values.

Br,

David

>
> Cheers,
> Mauro
>

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

[Sakari Ailus]

Hi Laurent,

Many thanks for the pull req!

On Thu, Feb 17, 2011 at 04:06:58PM +0100, Laurent Pinchart wrote:
...
>  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           | 1264 ++++++++++
>  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 +
...
>  include/linux/Kbuild                               |    4 +
>  include/linux/media.h                              |  132 ++
>  include/linux/omap3isp.h                           |  646 +++++

What about renaming the directory omap3isp for the sake of consistency? 
The header file is called omap3isp.h and omap3isp is the prefix used in 
the driver for exported symbols.

My apologies for not bringing this up earlier.

Regards,

-- 
Sakari Ailus
sakari dot ailus at retiisi dot org dot uk

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

[Laurent Pinchart]

Hi Sakari,

On Sunday 06 March 2011 09:34:50 Sakari Ailus wrote:
> Hi Laurent,
> 
> Many thanks for the pull req!
> 
> On Thu, Feb 17, 2011 at 04:06:58PM +0100, Laurent Pinchart wrote:
> ...
> 
> >  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           | 1264 ++++++++++
> >  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 +
> 
> ...
> 
> >  include/linux/Kbuild                               |    4 +
> >  include/linux/media.h                              |  132 ++
> >  include/linux/omap3isp.h                           |  646 +++++
> 
> What about renaming the directory omap3isp for the sake of consistency?
> The header file is called omap3isp.h and omap3isp is the prefix used in
> the driver for exported symbols.

I'm fine with both. If Mauro prefers omap3-isp, I can update the patches.

> My apologies for not bringing this up earlier.

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em 06-03-2011 07:17, Laurent Pinchart escreveu:
> Hi Sakari,
> 
> On Sunday 06 March 2011 09:34:50 Sakari Ailus wrote:
>> Hi Laurent,
>>
>> Many thanks for the pull req!
>>
>> On Thu, Feb 17, 2011 at 04:06:58PM +0100, Laurent Pinchart wrote:
>> ...
>>
>>>  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           | 1264 ++++++++++
>>>  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 +
>>
>> ...
>>
>>>  include/linux/Kbuild                               |    4 +
>>>  include/linux/media.h                              |  132 ++
>>>  include/linux/omap3isp.h                           |  646 +++++
>>
>> What about renaming the directory omap3isp for the sake of consistency?
>> The header file is called omap3isp.h and omap3isp is the prefix used in
>> the driver for exported symbols.
> 
> I'm fine with both. If Mauro prefers omap3-isp, I can update the patches.

Probably, omap3-isp would be better, but I'm fine if you prefere omap3isp.

Cheers,
Mauro

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

[Sakari Ailus]

On Mon, Mar 07, 2011 at 08:56:31AM -0300, Mauro Carvalho Chehab wrote:
> Em 06-03-2011 07:17, Laurent Pinchart escreveu:
> > Hi Sakari,
> > 
> > On Sunday 06 March 2011 09:34:50 Sakari Ailus wrote:
> >> Hi Laurent,
> >>
> >> Many thanks for the pull req!
> >>
> >> On Thu, Feb 17, 2011 at 04:06:58PM +0100, Laurent Pinchart wrote:
> >> ...
> >>
> >>>  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           | 1264 ++++++++++
> >>>  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 +
> >>
> >> ...
> >>
> >>>  include/linux/Kbuild                               |    4 +
> >>>  include/linux/media.h                              |  132 ++
> >>>  include/linux/omap3isp.h                           |  646 +++++
> >>
> >> What about renaming the directory omap3isp for the sake of consistency?
> >> The header file is called omap3isp.h and omap3isp is the prefix used in
> >> the driver for exported symbols.
> > 
> > I'm fine with both. If Mauro prefers omap3-isp, I can update the patches.
> 
> Probably, omap3-isp would be better, but I'm fine if you prefere omap3isp.

Hi Mauro, Laurent,

I'm also fine with omap3-isp. My point was that we should be consistent in
naming. If the symbol prefix and the file / directory names are a little
different that is certainly not an issue to me. So the change to the current
state of the patchset would be that the header file was be called
omap3-isp.h, right?

Cheers,

-- 
Sakari Ailus
sakari dot ailus at retiisi dot org dot uk