[alsa-devel] [GIT PULL FOR 2.6.39] Media controller and OMAP3 ISP driver
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
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.
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_ioctl2Two 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.
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
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
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 ?
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.
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... 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... 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.
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... 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... 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/medi...
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.
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... 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
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.
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... omap3isp
You can take all the OMAP3 ISP patches from that branch.
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/medi...
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/medi... Before I merge it at the media-tree?
Thanks, Mauro.
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.
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
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
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
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
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
Hi David,
On 03/05/2011 02:04 PM, David Cohen wrote:
Hi Hans,
On Sat, Mar 5, 2011 at 1:52 PM, Hans Verkuilhverkuil@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 alphabeticallyThe 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
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 Verkuilhverkuil@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 alphabeticallyThe 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.
- 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.
- 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.
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 Verkuilhverkuil@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 alphabeticallyThe 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.
- 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.
- 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.
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 Verkuilhverkuil@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.
- 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.
- 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.
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.
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.
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.
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
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.
Regards,
Hans
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
-- 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
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
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.
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.
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;
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 2001From: 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.
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 2001From: 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 operationv4l: 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 formatsHad 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
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/medi...
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.
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/medi...
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
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/medi...
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 orderto 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/ordistribution/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
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/medi...
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
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/medi...
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 orderto 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/ordistribution/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
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/medi...
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
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,
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.
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
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,
participants (8)
-
David Cohen -
Hans Verkuil -
Hans Verkuil -
Laurent Pinchart -
Mauro Carvalho Chehab -
Mauro Carvalho Chehab -
Sakari Ailus -
Sylwester Nawrocki