* [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
@ 2011-02-17 15:06 Laurent Pinchart
2011-03-02 20:13 ` Mauro Carvalho Chehab
` (2 more replies)
0 siblings, 3 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-02-17 15:06 UTC (permalink / raw)
To: linux-media@vger.kernel.org; +Cc: alsa-devel, Sakari Ailus
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-02-17 15:06 [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver Laurent Pinchart
@ 2011-03-02 20:13 ` Mauro Carvalho Chehab
2011-03-03 9:29 ` Laurent Pinchart
2011-03-03 10:25 ` Laurent Pinchart
2011-03-04 22:16 ` Mauro Carvalho Chehab
2011-03-06 8:34 ` Sakari Ailus
2 siblings, 2 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-02 20:13 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-02 20:13 ` Mauro Carvalho Chehab
@ 2011-03-03 9:29 ` Laurent Pinchart
2011-03-03 10:25 ` Laurent Pinchart
1 sibling, 0 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-03 9:29 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-02 20:13 ` Mauro Carvalho Chehab
2011-03-03 9:29 ` Laurent Pinchart
@ 2011-03-03 10:25 ` Laurent Pinchart
2011-03-04 19:25 ` Mauro Carvalho Chehab
` (2 more replies)
1 sibling, 3 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-03 10:25 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-03 10:25 ` Laurent Pinchart
@ 2011-03-04 19:25 ` Mauro Carvalho Chehab
2011-03-05 13:02 ` Laurent Pinchart
2011-03-04 20:10 ` Mauro Carvalho Chehab
2011-03-04 20:49 ` Mauro Carvalho Chehab
2 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 19:25 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-03 10:25 ` Laurent Pinchart
2011-03-04 19:25 ` Mauro Carvalho Chehab
@ 2011-03-04 20:10 ` Mauro Carvalho Chehab
2011-03-04 20:14 ` David Cohen
2011-03-05 11:52 ` Hans Verkuil
2011-03-04 20:49 ` Mauro Carvalho Chehab
2 siblings, 2 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 20:10 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 20:10 ` Mauro Carvalho Chehab
@ 2011-03-04 20:14 ` David Cohen
2011-03-05 11:52 ` Hans Verkuil
1 sibling, 0 replies; 43+ messages in thread
From: David Cohen @ 2011-03-04 20:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus, Pawel Osciak
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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-03 10:25 ` Laurent Pinchart
2011-03-04 19:25 ` Mauro Carvalho Chehab
2011-03-04 20:10 ` Mauro Carvalho Chehab
@ 2011-03-04 20:49 ` Mauro Carvalho Chehab
2011-03-04 21:31 ` Mauro Carvalho Chehab
2011-03-05 12:03 ` Hans Verkuil
2 siblings, 2 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 20:49 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 20:49 ` Mauro Carvalho Chehab
@ 2011-03-04 21:31 ` Mauro Carvalho Chehab
2011-03-05 12:03 ` Hans Verkuil
1 sibling, 0 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 21:31 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-02-17 15:06 [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver Laurent Pinchart
2011-03-02 20:13 ` Mauro Carvalho Chehab
@ 2011-03-04 22:16 ` Mauro Carvalho Chehab
2011-03-04 22:33 ` David Cohen
2011-03-06 8:34 ` Sakari Ailus
2 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 22:16 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 22:16 ` Mauro Carvalho Chehab
@ 2011-03-04 22:33 ` David Cohen
2011-03-04 22:43 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: David Cohen @ 2011-03-04 22:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus
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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 22:33 ` David Cohen
@ 2011-03-04 22:43 ` Mauro Carvalho Chehab
2011-03-04 22:49 ` David Cohen
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 22:43 UTC (permalink / raw)
To: David Cohen
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 22:43 ` Mauro Carvalho Chehab
@ 2011-03-04 22:49 ` David Cohen
2011-03-04 23:49 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: David Cohen @ 2011-03-04 22:49 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus
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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 22:49 ` David Cohen
@ 2011-03-04 23:49 ` Mauro Carvalho Chehab
2011-03-05 0:40 ` David Cohen
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-04 23:49 UTC (permalink / raw)
To: David Cohen
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 23:49 ` Mauro Carvalho Chehab
@ 2011-03-05 0:40 ` David Cohen
0 siblings, 0 replies; 43+ messages in thread
From: David Cohen @ 2011-03-05 0:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus
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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 20:10 ` Mauro Carvalho Chehab
2011-03-04 20:14 ` David Cohen
@ 2011-03-05 11:52 ` Hans Verkuil
2011-03-05 13:04 ` David Cohen
1 sibling, 1 reply; 43+ messages in thread
From: Hans Verkuil @ 2011-03-05 11:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus, Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 20:49 ` Mauro Carvalho Chehab
2011-03-04 21:31 ` Mauro Carvalho Chehab
@ 2011-03-05 12:03 ` Hans Verkuil
1 sibling, 0 replies; 43+ messages in thread
From: Hans Verkuil @ 2011-03-05 12:03 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus, Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-04 19:25 ` Mauro Carvalho Chehab
@ 2011-03-05 13:02 ` Laurent Pinchart
2011-03-05 18:22 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-05 13:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 11:52 ` Hans Verkuil
@ 2011-03-05 13:04 ` David Cohen
2011-03-05 14:02 ` Hans Verkuil
2011-03-05 14:29 ` Sylwester Nawrocki
0 siblings, 2 replies; 43+ messages in thread
From: David Cohen @ 2011-03-05 13:04 UTC (permalink / raw)
To: Hans Verkuil
Cc: Mauro Carvalho Chehab, Laurent Pinchart,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 13:04 ` David Cohen
@ 2011-03-05 14:02 ` Hans Verkuil
2011-03-05 14:29 ` Sylwester Nawrocki
1 sibling, 0 replies; 43+ messages in thread
From: Hans Verkuil @ 2011-03-05 14:02 UTC (permalink / raw)
To: David Cohen
Cc: Mauro Carvalho Chehab, Laurent Pinchart,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 13:04 ` David Cohen
2011-03-05 14:02 ` Hans Verkuil
@ 2011-03-05 14:29 ` Sylwester Nawrocki
2011-03-05 18:14 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 43+ messages in thread
From: Sylwester Nawrocki @ 2011-03-05 14:29 UTC (permalink / raw)
To: David Cohen
Cc: Hans Verkuil, Mauro Carvalho Chehab, Laurent Pinchart,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 14:29 ` Sylwester Nawrocki
@ 2011-03-05 18:14 ` Mauro Carvalho Chehab
2011-03-05 23:23 ` Sylwester Nawrocki
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-05 18:14 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: David Cohen, Hans Verkuil, Laurent Pinchart,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 13:02 ` Laurent Pinchart
@ 2011-03-05 18:22 ` Mauro Carvalho Chehab
2011-03-05 20:48 ` Laurent Pinchart
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-05 18:22 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 18:22 ` Mauro Carvalho Chehab
@ 2011-03-05 20:48 ` Laurent Pinchart
2011-03-07 11:57 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-05 20:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 18:14 ` Mauro Carvalho Chehab
@ 2011-03-05 23:23 ` Sylwester Nawrocki
2011-03-06 10:56 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: Sylwester Nawrocki @ 2011-03-05 23:23 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: David Cohen, Hans Verkuil, Laurent Pinchart,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-02-17 15:06 [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver Laurent Pinchart
2011-03-02 20:13 ` Mauro Carvalho Chehab
2011-03-04 22:16 ` Mauro Carvalho Chehab
@ 2011-03-06 8:34 ` Sakari Ailus
2011-03-06 10:17 ` Laurent Pinchart
2 siblings, 1 reply; 43+ messages in thread
From: Sakari Ailus @ 2011-03-06 8:34 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media@vger.kernel.org, alsa-devel
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-06 8:34 ` Sakari Ailus
@ 2011-03-06 10:17 ` Laurent Pinchart
2011-03-07 11:56 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-06 10:17 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media@vger.kernel.org, alsa-devel
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 23:23 ` Sylwester Nawrocki
@ 2011-03-06 10:56 ` Mauro Carvalho Chehab
2011-03-06 11:38 ` Laurent Pinchart
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-06 10:56 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: David Cohen, Hans Verkuil, Laurent Pinchart,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-06 10:56 ` Mauro Carvalho Chehab
@ 2011-03-06 11:38 ` Laurent Pinchart
2011-03-06 13:32 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-06 11:38 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-06 11:38 ` Laurent Pinchart
@ 2011-03-06 13:32 ` Mauro Carvalho Chehab
2011-03-06 17:21 ` Laurent Pinchart
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-06 13:32 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-06 13:32 ` Mauro Carvalho Chehab
@ 2011-03-06 17:21 ` Laurent Pinchart
2011-03-07 11:50 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-06 17:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-06 17:21 ` Laurent Pinchart
@ 2011-03-07 11:50 ` Mauro Carvalho Chehab
[not found] ` <201103071302.49323.hansverk@cisco.com>
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-07 11:50 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-06 10:17 ` Laurent Pinchart
@ 2011-03-07 11:56 ` Mauro Carvalho Chehab
2011-03-07 12:08 ` Sakari Ailus
0 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-07 11:56 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Sakari Ailus, linux-media@vger.kernel.org, alsa-devel
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-05 20:48 ` Laurent Pinchart
@ 2011-03-07 11:57 ` Mauro Carvalho Chehab
2011-03-07 12:06 ` David Cohen
` (3 more replies)
0 siblings, 4 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-07 11:57 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 11:57 ` Mauro Carvalho Chehab
@ 2011-03-07 12:06 ` David Cohen
2011-03-07 13:38 ` Laurent Pinchart
` (2 subsequent siblings)
3 siblings, 0 replies; 43+ messages in thread
From: David Cohen @ 2011-03-07 12:06 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel,
Sakari Ailus, Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 11:56 ` Mauro Carvalho Chehab
@ 2011-03-07 12:08 ` Sakari Ailus
0 siblings, 0 replies; 43+ messages in thread
From: Sakari Ailus @ 2011-03-07 12:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Laurent Pinchart, linux-media@vger.kernel.org, alsa-devel
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
[not found] ` <201103071302.49323.hansverk@cisco.com>
@ 2011-03-07 13:00 ` Mauro Carvalho Chehab
2011-03-07 13:04 ` Mauro Carvalho Chehab
2011-03-07 13:46 ` Laurent Pinchart
0 siblings, 2 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-07 13:00 UTC (permalink / raw)
To: Hans Verkuil
Cc: Laurent Pinchart, Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 13:00 ` Mauro Carvalho Chehab
@ 2011-03-07 13:04 ` Mauro Carvalho Chehab
2011-03-07 13:46 ` Laurent Pinchart
1 sibling, 0 replies; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-07 13:04 UTC (permalink / raw)
To: Hans Verkuil
Cc: Laurent Pinchart, Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 11:57 ` Mauro Carvalho Chehab
2011-03-07 12:06 ` David Cohen
@ 2011-03-07 13:38 ` Laurent Pinchart
2011-03-07 22:04 ` Laurent Pinchart
2011-03-11 15:40 ` Mauro Carvalho Chehab
3 siblings, 0 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-07 13:38 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 13:00 ` Mauro Carvalho Chehab
2011-03-07 13:04 ` Mauro Carvalho Chehab
@ 2011-03-07 13:46 ` Laurent Pinchart
1 sibling, 0 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-07 13:46 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Hans Verkuil, Sylwester Nawrocki, David Cohen, Hans Verkuil,
linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 11:57 ` Mauro Carvalho Chehab
2011-03-07 12:06 ` David Cohen
2011-03-07 13:38 ` Laurent Pinchart
@ 2011-03-07 22:04 ` Laurent Pinchart
2011-03-11 15:40 ` Mauro Carvalho Chehab
3 siblings, 0 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-07 22:04 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-07 11:57 ` Mauro Carvalho Chehab
` (2 preceding siblings ...)
2011-03-07 22:04 ` Laurent Pinchart
@ 2011-03-11 15:40 ` Mauro Carvalho Chehab
2011-03-11 15:48 ` Laurent Pinchart
3 siblings, 1 reply; 43+ messages in thread
From: Mauro Carvalho Chehab @ 2011-03-11 15:40 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
2011-03-11 15:40 ` Mauro Carvalho Chehab
@ 2011-03-11 15:48 ` Laurent Pinchart
0 siblings, 0 replies; 43+ messages in thread
From: Laurent Pinchart @ 2011-03-11 15:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org, alsa-devel, Sakari Ailus,
Pawel Osciak
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
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2011-03-11 15:48 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-17 15:06 [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver Laurent Pinchart
2011-03-02 20:13 ` Mauro Carvalho Chehab
2011-03-03 9:29 ` Laurent Pinchart
2011-03-03 10:25 ` Laurent Pinchart
2011-03-04 19:25 ` Mauro Carvalho Chehab
2011-03-05 13:02 ` Laurent Pinchart
2011-03-05 18:22 ` Mauro Carvalho Chehab
2011-03-05 20:48 ` Laurent Pinchart
2011-03-07 11:57 ` Mauro Carvalho Chehab
2011-03-07 12:06 ` David Cohen
2011-03-07 13:38 ` Laurent Pinchart
2011-03-07 22:04 ` Laurent Pinchart
2011-03-11 15:40 ` Mauro Carvalho Chehab
2011-03-11 15:48 ` Laurent Pinchart
2011-03-04 20:10 ` Mauro Carvalho Chehab
2011-03-04 20:14 ` David Cohen
2011-03-05 11:52 ` Hans Verkuil
2011-03-05 13:04 ` David Cohen
2011-03-05 14:02 ` Hans Verkuil
2011-03-05 14:29 ` Sylwester Nawrocki
2011-03-05 18:14 ` Mauro Carvalho Chehab
2011-03-05 23:23 ` Sylwester Nawrocki
2011-03-06 10:56 ` Mauro Carvalho Chehab
2011-03-06 11:38 ` Laurent Pinchart
2011-03-06 13:32 ` Mauro Carvalho Chehab
2011-03-06 17:21 ` Laurent Pinchart
2011-03-07 11:50 ` Mauro Carvalho Chehab
[not found] ` <201103071302.49323.hansverk@cisco.com>
2011-03-07 13:00 ` Mauro Carvalho Chehab
2011-03-07 13:04 ` Mauro Carvalho Chehab
2011-03-07 13:46 ` Laurent Pinchart
2011-03-04 20:49 ` Mauro Carvalho Chehab
2011-03-04 21:31 ` Mauro Carvalho Chehab
2011-03-05 12:03 ` Hans Verkuil
2011-03-04 22:16 ` Mauro Carvalho Chehab
2011-03-04 22:33 ` David Cohen
2011-03-04 22:43 ` Mauro Carvalho Chehab
2011-03-04 22:49 ` David Cohen
2011-03-04 23:49 ` Mauro Carvalho Chehab
2011-03-05 0:40 ` David Cohen
2011-03-06 8:34 ` Sakari Ailus
2011-03-06 10:17 ` Laurent Pinchart
2011-03-07 11:56 ` Mauro Carvalho Chehab
2011-03-07 12:08 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox