Re: [PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control
Hi Laurent, Thanks for the review. On Tue, Sep 25, 2012 at 9:30 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Prabhakar, Thank you for the patch. On Tuesday 25 September 2012 18:29:25 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com --- drivers/media/i2c/mt9p031.c | 27 drivers/media/i2c/mt9t001.c | 33 +++--- drivers/media/i2c/mt9v032.c | 46 3 files changed, 66 insertions(+), 40 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 2c0f407..a45c2ea 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev, * V4L2 subdev control operations */ -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001) -#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002) -#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003) -#define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1004) -#define V4L2_CID_BLC_DIGITAL_OFFSET (V4L2_CID_USER_BASE | 0x1005) +#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1001) +#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1002) +#define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1003) +#define V4L2_CID_BLC_DIGITAL_OFFSET (V4L2_CID_USER_BASE | 0x1004) Let's not change the value of the other device-specific controls. Ok static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) { @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] = { static const struct v4l2_ctrl_config mt9p031_ctrls[] = { { .ops= mt9p031_ctrl_ops, - .id = V4L2_CID_TEST_PATTERN, - .type = V4L2_CTRL_TYPE_MENU, - .name = Test Pattern, - .min= 0, - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, - .step = 0, - .def= 0, - .flags = 0, - .menu_skip_mask = 0, - .qmenu = mt9p031_test_pattern_menu, - }, { - .ops= mt9p031_ctrl_ops, .id = V4L2_CID_BLC_AUTO, .type = V4L2_CTRL_TYPE_BOOLEAN, .name = BLC, Auto, @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031-model = did-driver_data; mt9p031-reset = -1; - v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5); + v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6); v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops, V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN, @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client, v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops, V4L2_CID_PIXEL_RATE, pdata-target_freq, pdata-target_freq, 1, pdata-target_freq); + v4l2_ctrl_new_std_menu_items(mt9p031-ctrls, mt9p031_ctrl_ops, + V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0, + 0, mt9p031_test_pattern_menu); for (i = 0; i ARRAY_SIZE(mt9p031_ctrls); ++i) v4l2_ctrl_new_custom(mt9p031-ctrls, mt9p031_ctrls[i], NULL); diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c index 6d343ad..16eac3f 100644 --- a/drivers/media/i2c/mt9t001.c +++ b/drivers/media/i2c/mt9t001.c @@ -124,6 +124,7 @@ struct mt9t001 { u16 output_control; u16 black_level; + bool test_pattern; }; static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd) @@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev *subdev, * V4L2 subdev control operations */ -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001) -#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002) -#define V4L2_CID_BLACK_LEVEL_OFFSET (V4L2_CID_USER_BASE | 0x1003) -#define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004) +#define V4L2_CID_PARAMETRIC_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001) That's a bit of a long name. What about V4L2_CID_TEST_PATTERN_COLOR ? Ok. +#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002) +#define V4L2_CID_BLACK_LEVEL_OFFSET (V4L2_CID_USER_BASE | 0x1003) +#define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004) #define V4L2_CID_GAIN_RED(V4L2_CTRL_CLASS_CAMERA | 0x1001) #define V4L2_CID_GAIN_GREEN_RED (V4L2_CTRL_CLASS_CAMERA | 0x1002) @@ -485,8 +486,15 @@
[PATCH v2] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control
From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Paul Gortmaker paul.gortma...@windriver.com Cc: Jean Delvare kh...@linux-fr.org --- Changes for v2: 1: Fixed review comments pointed by Laurent. drivers/media/i2c/mt9p031.c | 19 +-- drivers/media/i2c/mt9t001.c | 23 +++ drivers/media/i2c/mt9v032.c | 34 -- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 2c0f407..e328332 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -574,7 +574,6 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev, * V4L2 subdev control operations */ -#define V4L2_CID_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001) #define V4L2_CID_BLC_AUTO (V4L2_CID_USER_BASE | 0x1002) #define V4L2_CID_BLC_TARGET_LEVEL (V4L2_CID_USER_BASE | 0x1003) #define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1004) @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] = { static const struct v4l2_ctrl_config mt9p031_ctrls[] = { { .ops= mt9p031_ctrl_ops, - .id = V4L2_CID_TEST_PATTERN, - .type = V4L2_CTRL_TYPE_MENU, - .name = Test Pattern, - .min= 0, - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, - .step = 0, - .def= 0, - .flags = 0, - .menu_skip_mask = 0, - .qmenu = mt9p031_test_pattern_menu, - }, { - .ops= mt9p031_ctrl_ops, .id = V4L2_CID_BLC_AUTO, .type = V4L2_CTRL_TYPE_BOOLEAN, .name = BLC, Auto, @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031-model = did-driver_data; mt9p031-reset = -1; - v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5); + v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6); v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops, V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN, @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client, v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops, V4L2_CID_PIXEL_RATE, pdata-target_freq, pdata-target_freq, 1, pdata-target_freq); + v4l2_ctrl_new_std_menu_items(mt9p031-ctrls, mt9p031_ctrl_ops, + V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0, + 0, mt9p031_test_pattern_menu); for (i = 0; i ARRAY_SIZE(mt9p031_ctrls); ++i) v4l2_ctrl_new_custom(mt9p031-ctrls, mt9p031_ctrls[i], NULL); diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c index 6d343ad..23f4e0a 100644 --- a/drivers/media/i2c/mt9t001.c +++ b/drivers/media/i2c/mt9t001.c @@ -371,7 +371,7 @@ static int mt9t001_set_crop(struct v4l2_subdev *subdev, * V4L2 subdev control operations */ -#define V4L2_CID_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001) +#define V4L2_CID_TEST_PATTERN_COLOR(V4L2_CID_USER_BASE | 0x1001) #define V4L2_CID_BLACK_LEVEL_AUTO (V4L2_CID_USER_BASE | 0x1002) #define V4L2_CID_BLACK_LEVEL_OFFSET(V4L2_CID_USER_BASE | 0x1003) #define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004) @@ -485,14 +485,12 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl) return mt9t001_write(client, MT9T001_SHUTTER_WIDTH_HIGH, ctrl-val 16); - case V4L2_CID_TEST_PATTERN: - ret = mt9t001_set_output_control(mt9t001, + return mt9t001_set_output_control(mt9t001, ctrl-val ? 0 : MT9T001_OUTPUT_CONTROL_TEST_DATA, ctrl-val ? MT9T001_OUTPUT_CONTROL_TEST_DATA : 0); - if (ret 0) - return ret; + case V4L2_CID_TEST_PATTERN_COLOR: return mt9t001_write(client, MT9T001_TEST_DATA, ctrl-val 2); case V4L2_CID_BLACK_LEVEL_AUTO: @@ -533,12 +531,17 @@ static struct v4l2_ctrl_ops mt9t001_ctrl_ops = { .s_ctrl = mt9t001_s_ctrl, }; +static const char * const mt9t001_test_pattern_menu[] = { + Disabled, + Enabled, +}; + static const struct v4l2_ctrl_config mt9t001_ctrls[] = { { .ops= mt9t001_ctrl_ops, - .id = V4L2_CID_TEST_PATTERN, + .id = V4L2_CID_TEST_PATTERN_COLOR,
Re: [PATCHv8 18/26] v4l: add buffer exporting via dmabuf
On Tue September 25 2012 18:30:43 Tomasz Stanislawski wrote: Hi Hans, Thank you for review. Please refer to the comments below. On 08/22/2012 01:41 PM, Hans Verkuil wrote: On Tue August 14 2012 17:34:48 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/video/v4l2-compat-ioctl32.c |1 + drivers/media/video/v4l2-dev.c|1 + drivers/media/video/v4l2-ioctl.c | 15 +++ include/linux/videodev2.h | 26 ++ include/media/v4l2-ioctl.h|2 ++ 5 files changed, 45 insertions(+) [snip] diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index dffd3c9..c4e8c7e 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -458,6 +458,14 @@ static void v4l_print_buffer(const void *arg, bool write_only) tc-type, tc-flags, tc-frames, *(__u32 *)tc-userbits); } +static void v4l_print_exportbuffer(const void *arg, bool write_only) +{ + const struct v4l2_exportbuffer *p = arg; + + pr_cont(fd=%d, mem_offset=%lx, flags=%lx\n, + p-fd, (unsigned long)p-mem_offset, (unsigned long)p-flags); Why the unsigned long casts? It is needed to avoid compiler warnings on machines where %lx is not compatible with u32. Why not use %x instead of %lx? +} + static void v4l_print_create_buffers(const void *arg, bool write_only) { const struct v4l2_create_buffers *p = arg; [snip] diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 7f918dc..b5d058b 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -688,6 +688,31 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000 +/** + * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor + * + * @fd: file descriptor associated with DMABUF (set by driver) + * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in struct + *v4l2_buffer::m.offset (for single-plane formats) or + *v4l2_plane::m.offset (for multi-planar formats) + * @flags:flags for newly created file, currently only O_CLOEXEC is + *supported, refer to manual of open syscall for more details + * + * Contains data used for exporting a video buffer as DMABUF file descriptor. + * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to userspace). All + * reserved fields must be set to zero. The field reserved0 is expected to + * become a structure 'type' allowing an alternative layout of the structure + * content. Therefore this field should not be used for any other extensions. + */ +struct v4l2_exportbuffer { + __u32 fd; + __u32 reserved0; + __u32 mem_offset; + __u32 flags; + __u32 reserved[12]; +}; OK, I realized that we also need a type field here: you need the type field (same as in v4l2_buffer) to know which queue the mem_offset refers to. For M2M devices you have two queues, so you need this information. I do not agree with you. The mmap() does not need buffer_type. So VIDIOC_EXPBUF should not need the field either. Please refer to patch [PATCHv8 26/26] v4l: s5p-mfc: support for dmabuf exporting for example how to deal without buffer_type. Is there any reason not to use __u32 memory instead of __u32 reserved0? I really dislike 'reserved0'. It's also very inconsistent with the other buffer ioctls which all have type+memory fields. The type is not needed for exporting if mem_offset is available like in mmap() case. The memory is not needed because exporting is available only for MMAP buffers. Today, yes. Perhaps not in the future. mem_offset uniquely identifies a buffer, but only in the MMAP case. That happens to be the only one we support at the moment, but in the future we might want to support others as well. I see two ways to describe a buffer for exporting: a) by mem_offset b) by (buffer_type, index, plane_index) tuple Actually a (buffer_type, index, plane_index, memory) tuple. For know I prefer to implement only method (a) to avoid single vs. multi plane madness. Moreover it guarantees that only MMAP buffers are exported. You see that as an advantage, I see that as a disadvantage. Because b is more future-proof than a. And while you need to provide more
Re: Gain controls in v4l2-ctrl framework
Hi All, On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Please let me know for any addition/deletion. Regards, --Prabhakar Lad Thanks and Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HVR 4000 and DVB-API
Le Tue, 25 Sep 2012 22:27:55 +0300, Rémi Denis-Courmont r...@remlab.net a écrit : Thanks for the answer. Le lundi 24 septembre 2012 10:51:23, Dominique Michel a écrit : The WinTV HVR-4000-HD is a multi-tuners TV card with 2 dvb tuners. It look like its driver doesn't have been updated to the new DVB-API. Multi-standard frontends required DVB API version 5.5. That is found in kernel versions 3.2 and later. So you might need to update the kernel. If you already have, then well, you need to get someone to update the driver. I have kernel 3.4.5 with the in-kernel dvb driver, so it must be OK. I just checked the kernel config, and it seam OK too. But just to be sure, do you know if it is some specific kernel option that need to be enabled in order to get the DVB API version 5.5, or is it the only one that is in the kernel? Also the application needs to be updated to support DVBv5.5 too. I don't know which versions of VDR support multi-standard frontends, if any as yet. In the main time, I will ask on the vdr forum. vdr-1.6.0 here, the last stable version. -- We have the heroes we deserve. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi All. On 09/25/2012 11:44 PM, Prabhakar Lad wrote: Hi All, On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Please let me know for any addition/deletion. I thought the consensus was that we would also need a V4L2_CID_GAIN_GREEN, to handle devices for which there are not two separate greens. Also, should there be a V4L2_CID_GREEN_RED_OFFSET and V4L2_CID_GREEN_BLUE_OFFSET, for consistency and to handle hardware that has such offsets? (Perhaps I missed an email in this thread, but I thought I caught them all.) Regards, --Prabhakar Lad Cheers, Chris MacGregor (the Seattle one) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi Chris, On Wed, Sep 26, 2012 at 12:23 PM, Chris MacGregor ch...@cybermato.com wrote: Hi All. On 09/25/2012 11:44 PM, Prabhakar Lad wrote: Hi All, On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Please let me know for any addition/deletion. I thought the consensus was that we would also need a V4L2_CID_GAIN_GREEN, to handle devices for which there are not two separate greens. Ok, I'll add it too. Also, should there be a V4L2_CID_GREEN_RED_OFFSET and V4L2_CID_GREEN_BLUE_OFFSET, for consistency and to handle hardware that has such offsets? +1 (But I would like opinions form others too for these control) (Perhaps I missed an email in this thread, but I thought I caught them all.) May be I missed out :( Thanks And Regards, --Prabhakar Lad Regards, --Prabhakar Lad Cheers, Chris MacGregor (the Seattle one) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 API PATCH 4/4] tuner-core: map audmode to STEREO for radio devices.
On Wed September 26 2012 04:29:33 Mauro Carvalho Chehab wrote: Em Tue, 25 Sep 2012 15:45:00 +0200 Hans Verkuil hansv...@cisco.com escreveu: On Tue 25 September 2012 15:33:40 Mauro Carvalho Chehab wrote: Em Fri, 14 Sep 2012 13:15:36 +0200 Hans Verkuil hans.verk...@cisco.com escreveu: Fixes a v4l2-compliance error: setting audmode to a value other than mono or stereo for a radio device should map to MODE_STEREO. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/tuner-core.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index b5a819a..ea71371 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -1235,8 +1235,11 @@ static int tuner_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) if (set_mode(t, vt-type)) return 0; - if (t-mode == V4L2_TUNER_RADIO) + if (t-mode == V4L2_TUNER_RADIO) { t-audmode = vt-audmode; + if (t-audmode V4L2_TUNER_MODE_STEREO) + t-audmode = V4L2_TUNER_MODE_STEREO; NACK. It is not a core's task to fix driver's bugs. It would be ok to have here a WARN_ON(), but, if a driver is reporting a wrong radio audmode, the fix should be there at the drivers, and not here at the core. tuner-core *is* the driver. Not really... it is a driver's glue between the real I2C driver and the bridge driver. A bridge driver just passes v4l2_tuner on to the subdev driver(s), and it is the subdev driver such as tuner-core that needs to process the audmode as specified by the user. Which in this case means mapping audmodes that are invalid when in radio mode to something that is valid as per the spec. Well, when the user is requesting an invalid mode, it should just return -EINVAL. It makes sense to add a check there at tuner-core to reject audmode if userspace is requesting, for example, a second language[1]. My interpretation of the spec is that it will map invalid audmodes to valid audmodes. From the VIDIOC_S_TUNER documentation: The selected audio mode, see Table A.89, “Tuner Audio Modes” for valid values. The audio mode does not affect audio subprogram detection, and like a control it does not automatically change unless the requested mode is invalid or unsupported. See Table A.90, “Tuner Audio Matrix” for possible results when the selected and received audio programs do not match. So my interpretation is that if an audmode is provided that is not valid for the given device, then the device maps it to something valid rather than returning an error. The error code list only states that -EINVAL is returned if the index field is out-of-bounds, not for invalid audmodes. I think this makes sense as well, otherwise apps would have to laboriously check which audmodes are supported before they can call S_TUNER. It's much easier to just give the 'best' audmode and let the driver downgrade if it isn't supported. This is what happens today anyway, so we can't change that behavior. But the one thing that should work is that the actual audmode is returned when calling G_TUNER, which is why the current tuner-core fails with v4l2-compliance. [1] Yet, I think that digital audio standards allow more than one audio channels. So, this may require to be pushed down into the drivers in some future. What is invalid actually depends on the device. For example, AM ISA drivers don't support stereo. Ok, all tuners supported by tuner-core are FM. Even so, some of them may not support stereo[2]. [2] afaikt, some designs with tuner xc2028 don't support stereo. The driver currently doesn't handle such border cases, but the point is that such checks should happen at driver's level. 99% of all those tuner drivers do support stereo, so let's do this simple check in tuner-core so we don't have to fix all of them. The spec is also clear that radio devices only support mono or stereo audmodes. Those tuner drivers that only support mono can easily enforce that explicitly. Or they could, if tuner-core would copy back the audmode value after calling analog_ops-set_params(). Just as we do basic checks in v4l2-ioctl.c, so we can do basic checks in tuner-core as well. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] [media] exynos-gsc: Remove linux/version.h header file inclusion
version.h is not needed for these files. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/exynos-gsc/gsc-core.c |1 - drivers/media/platform/exynos-gsc/gsc-m2m.c |1 - 2 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index c5c7625..90a6c55 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -12,7 +12,6 @@ #include linux/module.h #include linux/kernel.h -#include linux/version.h #include linux/types.h #include linux/errno.h #include linux/bug.h diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index 2589cae..a4f327e 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -12,7 +12,6 @@ #include linux/module.h #include linux/kernel.h -#include linux/version.h #include linux/types.h #include linux/errno.h #include linux/bug.h -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] [media] exynos-gsc: Add missing static storage class specifiers
Fixes the following warnings: drivers/media/platform/exynos-gsc/gsc-core.c:313:5: warning: symbol 'get_plane_info' was not declared. Should it be static? drivers/media/platform/exynos-gsc/gsc-core.c:746:28: warning: symbol 'gsc_ctrl_ops' was not declared. Should it be static? drivers/media/platform/exynos-gsc/gsc-m2m.c:102:5: warning: symbol 'gsc_fill_addr' was not declared. Should it be static? drivers/media/platform/exynos-gsc/gsc-m2m.c:252:16: warning: symbol 'gsc_m2m_qops' was not declared. Should it be static? Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/exynos-gsc/gsc-core.c |4 ++-- drivers/media/platform/exynos-gsc/gsc-m2m.c |4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 90a6c55..bfec9e6 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -310,7 +310,7 @@ int gsc_enum_fmt_mplane(struct v4l2_fmtdesc *f) return 0; } -u32 get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index) +static u32 get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index) { if (frm-addr.y == addr) { *index = 0; @@ -743,7 +743,7 @@ static int gsc_s_ctrl(struct v4l2_ctrl *ctrl) return ret; } -const struct v4l2_ctrl_ops gsc_ctrl_ops = { +static const struct v4l2_ctrl_ops gsc_ctrl_ops = { .s_ctrl = gsc_s_ctrl, }; diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index a4f327e..3c7f005 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -99,7 +99,7 @@ static void gsc_m2m_job_abort(void *priv) gsc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR); } -int gsc_fill_addr(struct gsc_ctx *ctx) +static int gsc_fill_addr(struct gsc_ctx *ctx) { struct gsc_frame *s_frame, *d_frame; struct vb2_buffer *vb = NULL; @@ -249,7 +249,7 @@ static void gsc_m2m_buf_queue(struct vb2_buffer *vb) v4l2_m2m_buf_queue(ctx-m2m_ctx, vb); } -struct vb2_ops gsc_m2m_qops = { +static struct vb2_ops gsc_m2m_qops = { .queue_setup = gsc_m2m_queue_setup, .buf_prepare = gsc_m2m_buf_prepare, .buf_queue = gsc_m2m_buf_queue, -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
On Wed, Sep 26, 2012 at 12:14:36PM +0530, Prabhakar Lad wrote: Hi All, On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Hi Prabhakar, As these are low level controls, I wonder whether it would make sense to make a difference between digital and analogue gain. I admit I'm not quite as certain whether there's such a large difference as there is for global gains for the camera control algorithms. Which ones do you need now? Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi Sakari, On Wed, Sep 26, 2012 at 1:12 PM, Sakari Ailus sakari.ai...@iki.fi wrote: On Wed, Sep 26, 2012 at 12:14:36PM +0530, Prabhakar Lad wrote: Hi All, On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Hi Prabhakar, As these are low level controls, I wonder whether it would make sense to make a difference between digital and analogue gain. I admit I'm not quite as certain whether there's such a large difference as there is for global gains for the camera control algorithms. Which ones do you need now? Currently I am need of following, 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET Regards, --Prabhakar Lad Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v3.7] Add back lost tda9875 copyright
When the separate tda9875 driver was merged into tvaudio the copyright line of the tda9875 driver was dropped inadvertently. Add it back. Regards, Hans The following changes since commit 4313902ebe33155209472215c62d2f29d117be29: [media] ivtv-alsa, ivtv: Connect ivtv PCM capture stream to ivtv-alsa interface driver (2012-09-18 13:29:07 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git tvaudio for you to fetch changes up to d4c90825a394f0bb3858516757c427e19cdfe224: tvaudio: add back lost tda9875 copyright. (2012-09-25 09:44:46 +0200) Hans Verkuil (1): tvaudio: add back lost tda9875 copyright. drivers/media/i2c/tvaudio.c |4 1 file changed, 4 insertions(+) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
On Wed, Sep 26, 2012 at 01:16:08PM +0530, Prabhakar Lad wrote: ... Currently I am need of following, 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET Are they analogue or digital? -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: wrong image after resizer with mt9v034 sensor
Hi Laurent 2012/9/25 Laurent Pinchart laurent.pinch...@ideasonboard.com: Hi Enric, On Tuesday 25 September 2012 13:23:20 Enric Balletbò i Serra wrote: 2012/9/25 Laurent Pinchart laurent.pinch...@ideasonboard.com: On Tuesday 25 September 2012 09:44:42 Enric Balletbò i Serra wrote: 2012/9/25 Laurent Pinchart laurent.pinch...@ideasonboard.com: On Monday 24 September 2012 15:49:01 Enric Balletbò i Serra wrote: 2012/9/24 Laurent Pinchart laurent.pinch...@ideasonboard.com: On Monday 24 September 2012 10:33:42 Enric Balletbò i Serra wrote: Hi everybody, I'm trying to add support for MT9V034 Aptina image sensor to current mainline, as a base of my current work I start using the latest omap3isp-next branch from Laurent's git tree [1]. The MT9V034 image sensor is very similar to MT9V032 sensor, so I modified current driver to accept MT9V034 sensor adding the chip ID. The driver recognizes the sensor and I'm able to capture some frames. I started capturing directly frames using the pipeline Sensor - CCDC ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:1-OMAP3 ISP CCDC output:0[1]' ./media-ctl -f 'mt9v032 3-005c:0 [SGRBG10 752x480]' ./media-ctl -f 'OMAP3 ISP CCDC:1 [SGRBG10 752x480]' # Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 # ./yavta -p -f SGRBG10 -s 752x480 -n 4 --capture=3 /dev/video2 --file=img-#.bin To convert to jpg I used bayer2rgb [2] program executing following command, $ convert -size 752x480 GRBG_BAYER:./img-00.bin img-00.jpg And the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-sensor.jp g Seems good, so I tried to use following pipeline Sensor - CCDC - Preview - Resizer ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:2-OMAP3 ISP preview:0[1]' ./media-ctl -l 'OMAP3 ISP preview:1-OMAP3 ISP resizer:0[1]' ./media-ctl -l 'OMAP3 ISP resizer:1-OMAP3 ISP resizer output:0[1]' ./media-ctl -V 'mt9v032 3-005c:0[SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:0 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:2 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP preview:1 [UYVY 752x480]' ./media-ctl -V 'OMAP3 ISP resizer:1 [UYVY 752x480]' # Set Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 ./yavta -f UYVY -s 752x480 --capture=3 --file=img-#.uyvy /dev/video6 I used 'convert' program to pass from UYVY to jpg, $ convert -size 752x480 img-00.uyvy img-00.jpg and the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-resizer.j pg As you can see, the image is wrong and I'm not sure if the problem is from the sensor, from the previewer, from the resizer or from my conversion. Anyone have idea where should I look ? Or which is the source of the problem ? Could you please post the output of all the above media-ctl and yavta runs, as well as the captured raw binary frame ? Of course, The log configuring the pipeline Sensor - CCDC is http://pastebin.com/WX8ex5x2 and the raw image can be found http://downloads.isee.biz/pub/files/patterns/img-00.bin It looks like D9 and D8 have trouble keeping their high-level. Possible reasons would be conflicts on the signal lines (with something actively driving them to a low-level, a pull-down wouldn't have such an effect), faulty cable/solder joints (but I doubt that), or sampling the data on the wrong edge. In that case don't be the first image also wrong ? (the image that outputs from sensor /dev/video2) Yes, it should be, and http://downloads.isee.biz/pub/files/patterns/img-00.bin is corrupted. That's the image captured at the CCDC output, isn't it ? Yes it is. http://downloads.isee.biz/pub/files/patterns/img-from-sensor.jpg How did you capture that one ? This image is the img-00.bin (that you say is corrupted) converted to RGB using bayer2rgb [2] program. So seems I'm using wrong tools to convert images. How you known that this file is corrupted ? Please, could you provide the tools that you use ? I'm using raw2rgbpnm (https://gitorious.org/raw2rgbpnm). If you look at the binary file in a hex editor you'll see that the MSBs are corrupted, instead of being stable in the 01 and 02 regions (pixels 256-511 and 512-752 on each line, so bytes 512-1023 and 1024-1503) they oscillate between 00 and 01, and 00 and 02 respectively. Thanks for the explanation, I really appreciate it. I'll investigate a bit more
re: [media] v4l2-subdev: add support for the new edid ioctls
Hi Hans, The patch ed45ce2cc0b3: [media] v4l2-subdev: add support for the new edid ioctls from Aug 10, 2012, needs an overflow check the same as the other cases in that switch statement. drivers/media/v4l2-core/v4l2-ioctl.c 2200 case VIDIOC_SUBDEV_G_EDID: 2201 case VIDIOC_SUBDEV_S_EDID: { 2202 struct v4l2_subdev_edid *edid = parg; 2203 2204 if (edid-blocks) { 2205 *user_ptr = (void __user *)edid-edid; 2206 *kernel_ptr = (void *)edid-edid; 2207 *array_size = edid-blocks * 128; ^^ This can overflow. 2208 ret = 1; 2209 } 2210 break; 2211 } regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
On Wed, Sep 26, 2012 at 1:24 PM, Sakari Ailus sakari.ai...@iki.fi wrote: On Wed, Sep 26, 2012 at 01:16:08PM +0530, Prabhakar Lad wrote: ... Currently I am need of following, 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET Are they analogue or digital? digital Regards, --Prabhakar -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media] v4l2-subdev: add support for the new edid ioctls
On Wed 26 September 2012 10:01:06 Dan Carpenter wrote: Hi Hans, The patch ed45ce2cc0b3: [media] v4l2-subdev: add support for the new edid ioctls from Aug 10, 2012, needs an overflow check the same as the other cases in that switch statement. drivers/media/v4l2-core/v4l2-ioctl.c 2200 case VIDIOC_SUBDEV_G_EDID: 2201 case VIDIOC_SUBDEV_S_EDID: { 2202 struct v4l2_subdev_edid *edid = parg; 2203 2204 if (edid-blocks) { 2205 *user_ptr = (void __user *)edid-edid; 2206 *kernel_ptr = (void *)edid-edid; 2207 *array_size = edid-blocks * 128; ^^ This can overflow. 2208 ret = 1; 2209 } 2210 break; 2211 } True. Thanks for reporting this! I'll make a fix for it. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v3.7] Add missing vidioc-subdev-g-edid.xml.
On Tue 25 September 2012 13:56:34 Hans Verkuil wrote: Hi Mauro, As requested! I've respun this tree, fixing one documentation bug (the max value for 'blocks' is 256, not 255) and adding an overflow check in v4l2-ioctl.c as reported by Dan Carpenter: http://www.mail-archive.com/linux-media@vger.kernel.org/msg52640.html Regards, Hans Regards, Hans The following changes since commit 4313902ebe33155209472215c62d2f29d117be29: [media] ivtv-alsa, ivtv: Connect ivtv PCM capture stream to ivtv-alsa interface driver (2012-09-18 13:29:07 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git docfix for you to fetch changes up to 369832c0cb2cd8df37d4854997d31978a286348e: DocBook: add missing vidioc-subdev-g-edid.xml. (2012-09-25 13:54:34 +0200) Hans Verkuil (1): DocBook: add missing vidioc-subdev-g-edid.xml. Documentation/DocBook/media/v4l/v4l2.xml |1 + Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml | 152 2 files changed, 153 insertions(+) create mode 100644 Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: wrong image after resizer with mt9v034 sensor
Hi Enric, On Wednesday 26 September 2012 09:57:53 Enric Balletbò i Serra wrote: [snip] You had reason. Checking the data lines of the camera bus with an oscilloscope I see I had a problem, exactly in D8 /D9 data lines. I'm curious, how have you fixed that ? Now I can capture images but the color is still wrong, see the following image captured with pipeline SENSOR - CCDC OUTPUT http://downloads.isee.biz/pub/files/patterns/img-01.pnm Now the image was converted using : ./raw2rgbpnm -s 752x480 -f SGRBG10 img-01.bin img-01.pnm And the raw data can be found here: http://downloads.isee.biz/pub/files/patterns/img-01.bin Any idea where I can look ? Thanks. Your sensors produces BGGR data if I'm not mistaken, not GRBG. raw2rgbpnm doesn't support BGGR (yet), but the OMAP3 ISP preview engine can convert that to YUV since v3.5. Just make your sensor driver expose the right media bus format and configure the pipeline accordingly. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] s5p-tv: Fix potential NULL pointer dereference error
On 09/24/2012 10:52 AM, Tomasz Stanislawski wrote: On 09/22/2012 09:39 AM, Sachin Kamat wrote: When mdev is NULL, the error print statement will try to dereference the NULL pointer. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s5p-tv/mixer_drv.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/platform/s5p-tv/mixer_drv.c b/drivers/media/platform/s5p-tv/mixer_drv.c index a15ca05..ca0f297 100644 --- a/drivers/media/platform/s5p-tv/mixer_drv.c +++ b/drivers/media/platform/s5p-tv/mixer_drv.c @@ -384,7 +384,7 @@ static int __devinit mxr_probe(struct platform_device *pdev) mdev = kzalloc(sizeof *mdev, GFP_KERNEL); if (!mdev) { -mxr_err(mdev, not enough memory.\n); +dev_err(dev, not enough memory.\n); ret = -ENOMEM; goto fail; } Acked-by: Tomasz Stanislawski t.stanisl...@samsung.com Applied, thanks! Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH Resend] [media] s5p-fimc: Fix incorrect condition in fimc_lite_reqbufs()
On 09/26/2012 05:53 AM, Sachin Kamat wrote: Fixes a typo in a conditional evaluation. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Applied, thanks! -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control
Hi Prabhakar, Thanks for the patch. On Wednesday 26 September 2012 12:05:10 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Paul Gortmaker paul.gortma...@windriver.com Cc: Jean Delvare kh...@linux-fr.org --- Changes for v2: 1: Fixed review comments pointed by Laurent. drivers/media/i2c/mt9p031.c | 19 +-- drivers/media/i2c/mt9t001.c | 23 +++ drivers/media/i2c/mt9v032.c | 34 -- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 2c0f407..e328332 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -574,7 +574,6 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev, * V4L2 subdev control operations */ -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001) #define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002) #define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003) #define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1004) @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] = { static const struct v4l2_ctrl_config mt9p031_ctrls[] = { { .ops= mt9p031_ctrl_ops, - .id = V4L2_CID_TEST_PATTERN, - .type = V4L2_CTRL_TYPE_MENU, - .name = Test Pattern, - .min= 0, - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, - .step = 0, - .def= 0, - .flags = 0, - .menu_skip_mask = 0, - .qmenu = mt9p031_test_pattern_menu, - }, { - .ops= mt9p031_ctrl_ops, .id = V4L2_CID_BLC_AUTO, .type = V4L2_CTRL_TYPE_BOOLEAN, .name = BLC, Auto, @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031-model = did-driver_data; mt9p031-reset = -1; - v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5); + v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6); v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops, V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN, @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client, v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops, V4L2_CID_PIXEL_RATE, pdata-target_freq, pdata-target_freq, 1, pdata-target_freq); + v4l2_ctrl_new_std_menu_items(mt9p031-ctrls, mt9p031_ctrl_ops, + V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0, + 0, mt9p031_test_pattern_menu); for (i = 0; i ARRAY_SIZE(mt9p031_ctrls); ++i) v4l2_ctrl_new_custom(mt9p031-ctrls, mt9p031_ctrls[i], NULL); diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c index 6d343ad..23f4e0a 100644 --- a/drivers/media/i2c/mt9t001.c +++ b/drivers/media/i2c/mt9t001.c @@ -371,7 +371,7 @@ static int mt9t001_set_crop(struct v4l2_subdev *subdev, * V4L2 subdev control operations */ -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001) +#define V4L2_CID_TEST_PATTERN_COLOR (V4L2_CID_USER_BASE | 0x1001) #define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002) #define V4L2_CID_BLACK_LEVEL_OFFSET (V4L2_CID_USER_BASE | 0x1003) #define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004) @@ -485,14 +485,12 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl) return mt9t001_write(client, MT9T001_SHUTTER_WIDTH_HIGH, ctrl-val 16); - No need to remove the blank line here. case V4L2_CID_TEST_PATTERN: - ret = mt9t001_set_output_control(mt9t001, + return mt9t001_set_output_control(mt9t001, ctrl-val ? 0 : MT9T001_OUTPUT_CONTROL_TEST_DATA, ctrl-val ? MT9T001_OUTPUT_CONTROL_TEST_DATA : 0); - if (ret 0) - return ret; + case V4L2_CID_TEST_PATTERN_COLOR: return mt9t001_write(client, MT9T001_TEST_DATA, ctrl-val 2); case V4L2_CID_BLACK_LEVEL_AUTO: @@ -533,12 +531,17 @@ static struct v4l2_ctrl_ops mt9t001_ctrl_ops = { .s_ctrl = mt9t001_s_ctrl, }; +static const char * const mt9t001_test_pattern_menu[] = { + Disabled, + Enabled, +}; + static const struct v4l2_ctrl_config mt9t001_ctrls[] = { { .ops
Re: [PATCH 1/2] [media] exynos-gsc: Remove linux/version.h header file inclusion
On 09/26/2012 09:18 AM, Sachin Kamat wrote: version.h is not needed for these files. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Applied both, thank you. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Timestamps and V4L2
Hi Sakari, On Tuesday 25 September 2012 23:12:00 Sakari Ailus wrote: Hans Verkuil wrote: On Tue 25 September 2012 12:48:01 Laurent Pinchart wrote: On Tuesday 25 September 2012 08:47:45 Hans Verkuil wrote: On Tue September 25 2012 02:00:55 Laurent Pinchart wrote: BTW, I think we should also fix the description of the timestamp in the spec. Currently it says: For input streams this is the system time (as returned by the gettimeofday() function) when the first data byte was captured. For output streams the data will not be displayed before this time, secondary to the nominal frame rate determined by the current video standard in enqueued order. Applications can for example zero this field to display frames as soon as possible. The driver stores the time at which the first data byte was actually sent out in the timestamp field. This permits applications to monitor the drift between the video and system clock. To my knowledge all capture drivers set the timestamp to the time the *last* data byte was captured, not the first. The uvcvideo driver uses the time the first image packet is received :-) Most other drivers use the time the last byte was *received*, not captured. Unless the hardware buffers more than a few lines there is very little difference between the time the last byte was received and when it was captured. But you are correct, it is typically the time the last byte was received. Should we signal this as well? First vs last byte? Or shall we standardize? My personal opinion would be to change the spec to say what almost every driver does: it's the timestamp from the moment the last pixel has been received. We have the frame sync event for telling when the frame starts btw. The same event could be used for signalling whenever a given line starts. I don't see frame end fitting to that quite as nicely but I guess it could be possible. The uvcvideo driver can timestamp the buffers with the system time at which the first packet in the frame is received, but has no way to generate a frame start event: the frame start event should correspond to the time the frame starts, not to the time the first packet in the frame is received. That information isn't available to the driver. BTW, the human mind is amazingly tolerant when it comes to A/V synchronization. Audio can be up to 50 ms ahead of the video and up to I believe 120 ms lagging behind the video before most people will notice. So being off by one frame won't be noticable at all. I wonder if this is what most DVD players do. What they do is not pretty. The difference could be more, though. ;-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 API PATCH 4/4] tuner-core: map audmode to STEREO for radio devices.
Em Wed, 26 Sep 2012 09:03:13 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On Wed September 26 2012 04:29:33 Mauro Carvalho Chehab wrote: Em Tue, 25 Sep 2012 15:45:00 +0200 Hans Verkuil hansv...@cisco.com escreveu: On Tue 25 September 2012 15:33:40 Mauro Carvalho Chehab wrote: Em Fri, 14 Sep 2012 13:15:36 +0200 Hans Verkuil hans.verk...@cisco.com escreveu: Fixes a v4l2-compliance error: setting audmode to a value other than mono or stereo for a radio device should map to MODE_STEREO. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/tuner-core.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index b5a819a..ea71371 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -1235,8 +1235,11 @@ static int tuner_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) if (set_mode(t, vt-type)) return 0; - if (t-mode == V4L2_TUNER_RADIO) + if (t-mode == V4L2_TUNER_RADIO) { t-audmode = vt-audmode; + if (t-audmode V4L2_TUNER_MODE_STEREO) + t-audmode = V4L2_TUNER_MODE_STEREO; NACK. It is not a core's task to fix driver's bugs. It would be ok to have here a WARN_ON(), but, if a driver is reporting a wrong radio audmode, the fix should be there at the drivers, and not here at the core. tuner-core *is* the driver. Not really... it is a driver's glue between the real I2C driver and the bridge driver. A bridge driver just passes v4l2_tuner on to the subdev driver(s), and it is the subdev driver such as tuner-core that needs to process the audmode as specified by the user. Which in this case means mapping audmodes that are invalid when in radio mode to something that is valid as per the spec. Well, when the user is requesting an invalid mode, it should just return -EINVAL. It makes sense to add a check there at tuner-core to reject audmode if userspace is requesting, for example, a second language[1]. My interpretation of the spec is that it will map invalid audmodes to valid audmodes. From the VIDIOC_S_TUNER documentation: The selected audio mode, see Table A.89, “Tuner Audio Modes” for valid values. The audio mode does not affect audio subprogram detection, and like a control it does not automatically change unless the requested mode is invalid or unsupported. See Table A.90, “Tuner Audio Matrix” for possible results when the selected and received audio programs do not match. So my interpretation is that if an audmode is provided that is not valid for the given device, then the device maps it to something valid rather than returning an error. The error code list only states that -EINVAL is returned if the index field is out-of-bounds, not for invalid audmodes. I think this makes sense as well, otherwise apps would have to laboriously check which audmodes are supported before they can call S_TUNER. It's much easier to just give the 'best' audmode and let the driver downgrade if it isn't supported. This is what happens today anyway, so we can't change that behavior. But the one thing that should work is that the actual audmode is returned when calling G_TUNER, which is why the current tuner-core fails with v4l2-compliance. Ok, you convinced me on that. Please be more verbose at the patch description, describing why it is falling back to a different mode. Also, please change that: + if (t-audmode V4L2_TUNER_MODE_STEREO) + t-audmode = V4L2_TUNER_MODE_STEREO; to something like: if (t-audmode != V4L2_TUNER_MODE_STEREO t-audmode != V4L2_TUNER_MODE_MONO) t-audmode = V4L2_TUNER_MODE_STEREO; We use those enums/defines to not having to remember the actual numbers associated with them. By using operators like greater/lower than, people will actually need to dig into the videodev2.h, in order to know what's covered there. Besides that, the compiler will likely optimize it to greater than anyway, as audmode is unsigned. [1] Yet, I think that digital audio standards allow more than one audio channels. So, this may require to be pushed down into the drivers in some future. What is invalid actually depends on the device. For example, AM ISA drivers don't support stereo. Ok, all tuners supported by tuner-core are FM. Even so, some of them may not support stereo[2]. [2] afaikt, some designs with tuner xc2028 don't support stereo. The driver currently doesn't handle such border cases, but the point is that such checks should happen at driver's level. 99% of all those tuner
[PATCH 0/5] media: ov7670: driver cleanup and support for ov7674.
The following series includes all the changes discussed in [1] that don't affect either bridge drivers that use ov7670 or soc-camera framework For this reason they are considered non controversial and sent separately. At least 1 more series will follow in order to implement all features described in [1]. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] media: ov7670: add support for ov7675.
ov7675 and ov7670 share the same registers but there is no way to distinguish them at runtime. However, they require different tweaks to achieve the desired resolution. For this reason this patch adds a new ov7675 entry to the ov7670_id table. Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c | 164 ++-- 1 file changed, 111 insertions(+), 53 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index e7c82b2..0478a7b 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -183,6 +183,10 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); #define REG_HAECC7 0xaa/* Hist AEC/AGC control 7 */ #define REG_BD60MAX0xab/* 60hz banding step limit */ +enum ov7670_model { + MODEL_OV7670 = 0, + MODEL_OV7675, +}; /* * Information we maintain about a known sensor. @@ -198,6 +202,7 @@ struct ov7670_info { int clock_speed;/* External clock speed (MHz) */ u8 clkrc; /* Clock divider value */ bool use_smbus; /* Use smbus I/O instead of I2C */ + enum ov7670_model model; }; static inline struct ov7670_info *to_state(struct v4l2_subdev *sd) @@ -652,7 +657,7 @@ static struct regval_list ov7670_qcif_regs[] = { { 0xff, 0xff }, }; -static struct ov7670_win_size { +struct ov7670_win_size { int width; int height; unsigned char com7_bit; @@ -661,57 +666,105 @@ static struct ov7670_win_size { int vstart; /* sense to humans, but evidently the sensor */ int vstop; /* will do the right thing... */ struct regval_list *regs; /* Regs to tweak */ -/* h/vref stuff */ -} ov7670_win_sizes[] = { - /* VGA */ - { - .width = VGA_WIDTH, - .height = VGA_HEIGHT, - .com7_bit = COM7_FMT_VGA, - .hstart = 158, /* These values from */ - .hstop = 14, /* Omnivision */ - .vstart = 10, - .vstop = 490, - .regs = NULL, - }, - /* CIF */ - { - .width = CIF_WIDTH, - .height = CIF_HEIGHT, - .com7_bit = COM7_FMT_CIF, - .hstart = 170, /* Empirically determined */ - .hstop = 90, - .vstart = 14, - .vstop = 494, - .regs = NULL, - }, - /* QVGA */ +}; + +static struct ov7670_win_size ov7670_win_sizes[2][4] = { + /* ov7670 */ { - .width = QVGA_WIDTH, - .height = QVGA_HEIGHT, - .com7_bit = COM7_FMT_QVGA, - .hstart = 168, /* Empirically determined */ - .hstop = 24, - .vstart = 12, - .vstop = 492, - .regs = NULL, + /* VGA */ + { + .width = VGA_WIDTH, + .height = VGA_HEIGHT, + .com7_bit = COM7_FMT_VGA, + .hstart = 158, /* These values from */ + .hstop = 14, /* Omnivision */ + .vstart = 10, + .vstop = 490, + .regs = NULL, + }, + /* CIF */ + { + .width = CIF_WIDTH, + .height = CIF_HEIGHT, + .com7_bit = COM7_FMT_CIF, + .hstart = 170, /* Empirically determined */ + .hstop = 90, + .vstart = 14, + .vstop = 494, + .regs = NULL, + }, + /* QVGA */ + { + .width = QVGA_WIDTH, + .height = QVGA_HEIGHT, + .com7_bit = COM7_FMT_QVGA, + .hstart = 168, /* Empirically determined */ + .hstop = 24, + .vstart = 12, + .vstop = 492, + .regs = NULL, + }, + /* QCIF */ + { + .width = QCIF_WIDTH, + .height = QCIF_HEIGHT, + .com7_bit = COM7_FMT_VGA, /* see comment above */ + .hstart = 456, /* Empirically determined */ + .hstop
[PATCH 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
'min_height' and 'min_width' are variables that allow to specify the minimum resolution that the sensor will achieve. This patch make v4l2 fmt callbacks consider this parameters in order to return valid data to user space. Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c | 22 +++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 0478a7b..627fe5f 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -812,10 +812,11 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd, struct ov7670_format_struct **ret_fmt, struct ov7670_win_size **ret_wsize) { - int index; + int index, i; struct ov7670_win_size *wsize; struct ov7670_info *info = to_state(sd); int n_win_sizes = ARRAY_SIZE(ov7670_win_sizes[info-model]); + int win_sizes_limit = n_win_sizes; for (index = 0; index N_OV7670_FMTS; index++) if (ov7670_formats[index].mbus_code == fmt-code) @@ -831,15 +832,30 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd, * Fields: the OV devices claim to be progressive. */ fmt-field = V4L2_FIELD_NONE; + + /* +* Don't consider values that don't match min_height and min_width +* constraints. +*/ + if (info-min_width || info-min_height) + for (i = 0; i n_win_sizes; i++) { + wsize = ov7670_win_sizes[info-model] + i; + + if (wsize-width info-min_width || + wsize-height info-min_height) { + win_sizes_limit = i; + break; + } + } /* * Round requested image size down to the nearest * we support, but not below the smallest. */ for (wsize = ov7670_win_sizes[info-model]; -wsize ov7670_win_sizes[info-model] + n_win_sizes; wsize++) +wsize ov7670_win_sizes[info-model] + win_sizes_limit; wsize++) if (fmt-width = wsize-width fmt-height = wsize-height) break; - if (wsize = ov7670_win_sizes[info-model] + n_win_sizes) + if (wsize = ov7670_win_sizes[info-model] + win_sizes_limit) wsize--; /* Take the smallest one */ if (ret_wsize != NULL) *ret_wsize = wsize; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] media: ov7670: add possibility to bypass pll for ov7675.
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c | 24 ++-- include/media/ov7670.h |1 + 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 175fbfc..54fb535 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -210,6 +210,7 @@ struct ov7670_info { int clock_speed;/* External clock speed (MHz) */ u8 clkrc; /* Clock divider value */ bool use_smbus; /* Use smbus I/O instead of I2C */ + bool pll_bypass; enum ov7670_model model; }; @@ -778,7 +779,12 @@ static void ov7670_get_framerate(struct v4l2_subdev *sd, { struct ov7670_info *info = to_state(sd); u32 clkrc = info-clkrc; - u32 pll_factor = PLL_FACTOR; + int pll_factor; + + if (info-pll_bypass) + pll_factor = 1; + else + pll_factor = PLL_FACTOR; clkrc++; if (info-fmt-mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8) @@ -794,7 +800,7 @@ static int ov7670_set_framerate(struct v4l2_subdev *sd, { struct ov7670_info *info = to_state(sd); u32 clkrc; - u32 pll_factor = PLL_FACTOR; + int pll_factor; int ret; /* @@ -804,6 +810,16 @@ static int ov7670_set_framerate(struct v4l2_subdev *sd, * pixclk = clock_speed / (clkrc + 1) * PLLfactor * */ + if (info-pll_bypass) { + pll_factor = 1; + ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS); + } else { + pll_factor = PLL_FACTOR; + ret = ov7670_write(sd, REG_DBLV, DBLV_X4); + } + if (ret 0) + return ret; + if (tpf-numerator == 0 || tpf-denominator == 0) { clkrc = 0; } else { @@ -831,6 +847,7 @@ static int ov7670_set_framerate(struct v4l2_subdev *sd, ret = ov7670_write(sd, REG_CLKRC, info-clkrc); if (ret 0) return ret; + return ov7670_write(sd, REG_DBLV, DBLV_X4); } @@ -1689,6 +1706,9 @@ static int ov7670_probe(struct i2c_client *client, if (config-clock_speed) info-clock_speed = config-clock_speed; + + if (config-pll_bypass id-driver_data != MODEL_OV7670) + info-pll_bypass = true; } /* Make sure it's an ov7670 */ diff --git a/include/media/ov7670.h b/include/media/ov7670.h index b133bc1..a68c8bb 100644 --- a/include/media/ov7670.h +++ b/include/media/ov7670.h @@ -15,6 +15,7 @@ struct ov7670_config { int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ bool use_smbus; /* Use smbus I/O instead of I2C */ + bool pll_bypass;/* Choose whether to bypass the PLL */ }; #endif -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] media: ov7670: calculate framerate properly for ov7675.
According to the datasheet ov7675 uses a formula to achieve the desired framerate that is different from the operations done in the current code. In fact, this formula should apply to ov7670 too. This would mean that current code is wrong but, in order to preserve compatibility, the new formula will be used for ov7675 only. Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c | 122 ++-- 1 file changed, 105 insertions(+), 17 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 627fe5f..175fbfc 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -47,6 +47,8 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); */ #define OV7670_I2C_ADDR 0x42 +#define PLL_FACTOR 4 + /* Registers */ #define REG_GAIN 0x00/* Gain lower 8 bits (rest in vref) */ #define REG_BLUE 0x01/* blue gain */ @@ -164,6 +166,12 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); #define REG_GFIX 0x69/* Fix gain control */ +#define REG_DBLV 0x6b/* PLL control an debugging */ +#define DBLV_BYPASS0x00/* Bypass PLL */ +#define DBLV_X40x01/* clock x4 */ +#define DBLV_X60x10/* clock x6 */ +#define DBLV_X80x11/* clock x8 */ + #define REG_REG76 0x76/* OV's name */ #define R76_BLKPCOR0x80/* Black pixel correction enable */ #define R76_WHTPCOR0x40/* White pixel correction enable */ @@ -765,6 +773,67 @@ static struct ov7670_win_size ov7670_win_sizes[2][4] = { } }; +static void ov7670_get_framerate(struct v4l2_subdev *sd, +struct v4l2_fract *tpf) +{ + struct ov7670_info *info = to_state(sd); + u32 clkrc = info-clkrc; + u32 pll_factor = PLL_FACTOR; + + clkrc++; + if (info-fmt-mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8) + clkrc = (clkrc 1); + + tpf-numerator = 1; + tpf-denominator = (5 * pll_factor * info-clock_speed) / + (4 * clkrc); +} + +static int ov7670_set_framerate(struct v4l2_subdev *sd, +struct v4l2_fract *tpf) +{ + struct ov7670_info *info = to_state(sd); + u32 clkrc; + u32 pll_factor = PLL_FACTOR; + int ret; + + /* +* The formula is fps = 5/4*pixclk for YUV/RGB and +* fps = 5/2*pixclk for RAW. +* +* pixclk = clock_speed / (clkrc + 1) * PLLfactor +* +*/ + if (tpf-numerator == 0 || tpf-denominator == 0) { + clkrc = 0; + } else { + clkrc = (5 * pll_factor * info-clock_speed * tpf-numerator) / + (4 * tpf-denominator); + if (info-fmt-mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8) + clkrc = (clkrc 1); + clkrc--; + } + + /* +* The datasheet claims that clkrc = 0 will divide the input clock by 1 +* but we've checked with an oscilloscope that it divides by 2 instead. +* So, if clkrc = 0 just bypass the divider. +*/ + if (clkrc = 0) + clkrc = CLK_EXT; + else if (clkrc CLK_SCALE) + clkrc = CLK_SCALE; + info-clkrc = clkrc; + + /* Recalculate frame rate */ + ov7670_get_framerate(sd, tpf); + + ret = ov7670_write(sd, REG_CLKRC, info-clkrc); + if (ret 0) + return ret; + return ov7670_write(sd, REG_DBLV, DBLV_X4); +} + /* * Store a set of start/stop values into the camera. */ @@ -939,10 +1008,15 @@ static int ov7670_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms) memset(cp, 0, sizeof(struct v4l2_captureparm)); cp-capability = V4L2_CAP_TIMEPERFRAME; - cp-timeperframe.numerator = 1; - cp-timeperframe.denominator = info-clock_speed; - if ((info-clkrc CLK_EXT) == 0 (info-clkrc CLK_SCALE) 1) - cp-timeperframe.denominator /= (info-clkrc CLK_SCALE); + if (info-model == MODEL_OV7670) { + /* legacy */ + cp-timeperframe.numerator = 1; + cp-timeperframe.denominator = info-clock_speed; + if ((info-clkrc CLK_EXT) == 0 (info-clkrc CLK_SCALE) 1) + cp-timeperframe.denominator /= (info-clkrc CLK_SCALE); + } else { + ov7670_get_framerate(sd, cp-timeperframe); + } return 0; } @@ -958,18 +1032,23 @@ static int ov7670_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms) if (cp-extendedmode != 0) return -EINVAL; - if (tpf-numerator == 0 || tpf-denominator == 0) - div = 1; /* Reset to full rate */ - else - div = (tpf-numerator * info-clock_speed) / tpf-denominator; - if (div == 0) - div = 1; - else if (div CLK_SCALE) - div = CLK_SCALE; -
[PATCH 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c |8 include/media/ov7670.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 54fb535..f7e4341 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -211,6 +211,7 @@ struct ov7670_info { u8 clkrc; /* Clock divider value */ bool use_smbus; /* Use smbus I/O instead of I2C */ bool pll_bypass; + bool pclk_hb_disable; enum ov7670_model model; }; @@ -1709,6 +1710,9 @@ static int ov7670_probe(struct i2c_client *client, if (config-pll_bypass id-driver_data != MODEL_OV7670) info-pll_bypass = true; + + if (config-pclk_hb_disable) + info-pclk_hb_disable = true; } /* Make sure it's an ov7670 */ @@ -1735,6 +1739,10 @@ static int ov7670_probe(struct i2c_client *client, tpf.denominator = 30; ov7670_set_framerate(sd, tpf); } + + if (info-pclk_hb_disable) + ov7670_write(sd, REG_COM10, COM10_PCLK_HB); + return 0; } diff --git a/include/media/ov7670.h b/include/media/ov7670.h index a68c8bb..1913d51 100644 --- a/include/media/ov7670.h +++ b/include/media/ov7670.h @@ -16,6 +16,7 @@ struct ov7670_config { int clock_speed;/* External clock speed (MHz) */ bool use_smbus; /* Use smbus I/O instead of I2C */ bool pll_bypass;/* Choose whether to bypass the PLL */ + bool pclk_hb_disable; /* Disable toggling pixclk during horizontal blanking */ }; #endif -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 18/26] v4l: add buffer exporting via dmabuf
Hi Hans, Thank your for your comments. On 09/26/2012 08:39 AM, Hans Verkuil wrote: On Tue September 25 2012 18:30:43 Tomasz Stanislawski wrote: Hi Hans, Thank you for review. Please refer to the comments below. On 08/22/2012 01:41 PM, Hans Verkuil wrote: On Tue August 14 2012 17:34:48 Tomasz Stanislawski wrote: This patch adds extension to V4L2 api. It allow to export a mmap buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by mmap and return a file descriptor on success. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/video/v4l2-compat-ioctl32.c |1 + drivers/media/video/v4l2-dev.c|1 + drivers/media/video/v4l2-ioctl.c | 15 +++ include/linux/videodev2.h | 26 ++ include/media/v4l2-ioctl.h|2 ++ 5 files changed, 45 insertions(+) [snip] diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index dffd3c9..c4e8c7e 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -458,6 +458,14 @@ static void v4l_print_buffer(const void *arg, bool write_only) tc-type, tc-flags, tc-frames, *(__u32 *)tc-userbits); } +static void v4l_print_exportbuffer(const void *arg, bool write_only) +{ + const struct v4l2_exportbuffer *p = arg; + + pr_cont(fd=%d, mem_offset=%lx, flags=%lx\n, + p-fd, (unsigned long)p-mem_offset, (unsigned long)p-flags); Why the unsigned long casts? It is needed to avoid compiler warnings on machines where %lx is not compatible with u32. Why not use %x instead of %lx? Ok. '%x' or '%08x' should be compatible with u32. Hopefully, no one is using V4L2 on 16-bit machines :). +} + static void v4l_print_create_buffers(const void *arg, bool write_only) { const struct v4l2_create_buffers *p = arg; [snip] diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 7f918dc..b5d058b 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -688,6 +688,31 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000 +/** + * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor + * + * @fd: file descriptor associated with DMABUF (set by driver) + * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in struct + *v4l2_buffer::m.offset (for single-plane formats) or + *v4l2_plane::m.offset (for multi-planar formats) + * @flags:flags for newly created file, currently only O_CLOEXEC is + *supported, refer to manual of open syscall for more details + * + * Contains data used for exporting a video buffer as DMABUF file descriptor. + * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to userspace). All + * reserved fields must be set to zero. The field reserved0 is expected to + * become a structure 'type' allowing an alternative layout of the structure + * content. Therefore this field should not be used for any other extensions. + */ +struct v4l2_exportbuffer { + __u32 fd; + __u32 reserved0; + __u32 mem_offset; + __u32 flags; + __u32 reserved[12]; +}; OK, I realized that we also need a type field here: you need the type field (same as in v4l2_buffer) to know which queue the mem_offset refers to. For M2M devices you have two queues, so you need this information. I do not agree with you. The mmap() does not need buffer_type. So VIDIOC_EXPBUF should not need the field either. Please refer to patch [PATCHv8 26/26] v4l: s5p-mfc: support for dmabuf exporting for example how to deal without buffer_type. Is there any reason not to use __u32 memory instead of __u32 reserved0? I really dislike 'reserved0'. It's also very inconsistent with the other buffer ioctls which all have type+memory fields. The type is not needed for exporting if mem_offset is available like in mmap() case. The memory is not needed because exporting is available only for MMAP buffers. Today, yes. Perhaps not in the future. mem_offset uniquely identifies a buffer, but only in the MMAP case. That happens to be the only one we support at the moment, but in the future we might want to support others as well. I see two ways to describe a buffer for exporting: a) by mem_offset b) by (buffer_type, index, plane_index) tuple Actually a (buffer_type, index, plane_index, memory) tuple. Why the memory goes into the tuple? Is it possible that the queue can work in two modes at the same time? Is it possible that userptr and mmap buffers both with index 0 can coexist
Re: [RFCv1 API PATCH 4/4] tuner-core: map audmode to STEREO for radio devices.
On Wed 26 September 2012 11:35:27 Mauro Carvalho Chehab wrote: Em Wed, 26 Sep 2012 09:03:13 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On Wed September 26 2012 04:29:33 Mauro Carvalho Chehab wrote: Em Tue, 25 Sep 2012 15:45:00 +0200 Hans Verkuil hansv...@cisco.com escreveu: On Tue 25 September 2012 15:33:40 Mauro Carvalho Chehab wrote: Em Fri, 14 Sep 2012 13:15:36 +0200 Hans Verkuil hans.verk...@cisco.com escreveu: Fixes a v4l2-compliance error: setting audmode to a value other than mono or stereo for a radio device should map to MODE_STEREO. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/tuner-core.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index b5a819a..ea71371 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -1235,8 +1235,11 @@ static int tuner_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) if (set_mode(t, vt-type)) return 0; - if (t-mode == V4L2_TUNER_RADIO) + if (t-mode == V4L2_TUNER_RADIO) { t-audmode = vt-audmode; + if (t-audmode V4L2_TUNER_MODE_STEREO) + t-audmode = V4L2_TUNER_MODE_STEREO; NACK. It is not a core's task to fix driver's bugs. It would be ok to have here a WARN_ON(), but, if a driver is reporting a wrong radio audmode, the fix should be there at the drivers, and not here at the core. tuner-core *is* the driver. Not really... it is a driver's glue between the real I2C driver and the bridge driver. A bridge driver just passes v4l2_tuner on to the subdev driver(s), and it is the subdev driver such as tuner-core that needs to process the audmode as specified by the user. Which in this case means mapping audmodes that are invalid when in radio mode to something that is valid as per the spec. Well, when the user is requesting an invalid mode, it should just return -EINVAL. It makes sense to add a check there at tuner-core to reject audmode if userspace is requesting, for example, a second language[1]. My interpretation of the spec is that it will map invalid audmodes to valid audmodes. From the VIDIOC_S_TUNER documentation: The selected audio mode, see Table A.89, “Tuner Audio Modes” for valid values. The audio mode does not affect audio subprogram detection, and like a control it does not automatically change unless the requested mode is invalid or unsupported. See Table A.90, “Tuner Audio Matrix” for possible results when the selected and received audio programs do not match. So my interpretation is that if an audmode is provided that is not valid for the given device, then the device maps it to something valid rather than returning an error. The error code list only states that -EINVAL is returned if the index field is out-of-bounds, not for invalid audmodes. I think this makes sense as well, otherwise apps would have to laboriously check which audmodes are supported before they can call S_TUNER. It's much easier to just give the 'best' audmode and let the driver downgrade if it isn't supported. This is what happens today anyway, so we can't change that behavior. But the one thing that should work is that the actual audmode is returned when calling G_TUNER, which is why the current tuner-core fails with v4l2-compliance. Ok, you convinced me on that. Please be more verbose at the patch description, describing why it is falling back to a different mode. Also, please change that: + if (t-audmode V4L2_TUNER_MODE_STEREO) + t-audmode = V4L2_TUNER_MODE_STEREO; to something like: if (t-audmode != V4L2_TUNER_MODE_STEREO t-audmode != V4L2_TUNER_MODE_MONO) t-audmode = V4L2_TUNER_MODE_STEREO; We use those enums/defines to not having to remember the actual numbers associated with them. By using operators like greater/lower than, people will actually need to dig into the videodev2.h, in order to know what's covered there. Besides that, the compiler will likely optimize it to greater than anyway, as audmode is unsigned. [1] Yet, I think that digital audio standards allow more than one audio channels. So, this may require to be pushed down into the drivers in some future. What is invalid actually depends on the device. For example, AM ISA drivers don't support stereo. Ok, all tuners supported by tuner-core are FM. Even so, some of them may not support stereo[2]. [2] afaikt, some designs with tuner xc2028 don't
Re: HVR 4000 and DVB-API
Le Wed, 26 Sep 2012 09:48:41 +0200, Dominique Michel dominique.mic...@vtxnet.ch a écrit : Le Tue, 25 Sep 2012 22:27:55 +0300, Rémi Denis-Courmont r...@remlab.net a écrit : Thanks for the answer. Le lundi 24 septembre 2012 10:51:23, Dominique Michel a écrit : The WinTV HVR-4000-HD is a multi-tuners TV card with 2 dvb tuners. It look like its driver doesn't have been updated to the new DVB-API. Multi-standard frontends required DVB API version 5.5. That is found in kernel versions 3.2 and later. So you might need to update the kernel. If you already have, then well, you need to get someone to update the driver. I have kernel 3.4.5 with the in-kernel dvb driver, so it must be OK. I get 2 frontends, so the driver was not updated in order to use only 1 frontend and calls like DTV_ENUM_DELSYS and FE_SET_PROPERTY / DTV_DELIVERY_SYSTEM. The driver is cx88_dvb from Chris Pascoe and Gerd Knorr. Also the application needs to be updated to support DVBv5.5 too. I don't know which versions of VDR support multi-standard frontends, if any as yet. In the main time, I will ask on the vdr forum. vdr-1.6.0 here, the last stable version. vdr = 1.7.23 is needed too. -- We have the heroes we deserve. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] em28xx: Replace memcpy with struct assignment
This kind of memcpy() is error-prone and its replacement with a struct assignment is prefered. Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- drivers/media/usb/em28xx/em28xx-cards.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index ca62b99..1a07857 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -2203,7 +2203,7 @@ EXPORT_SYMBOL_GPL(em28xx_tuner_callback); static inline void em28xx_set_model(struct em28xx *dev) { - memcpy(dev-board, em28xx_boards[dev-model], sizeof(dev-board)); + dev-board = em28xx_boards[dev-model]; /* Those are the default values for the majority of boards Use those values if not specified otherwise at boards entry -- 1.7.8.6 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 API PATCH 2/4] v4l2-ctrls: add a notify callback.
Hi Hans, Thanks for the patch. On Friday 14 September 2012 13:15:34 Hans Verkuil wrote: Sometimes platform/bridge drivers need to be notified when a control from a subdevice changes value. In order to support this a notify callback was added. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- Documentation/video4linux/v4l2-controls.txt | 22 ++ drivers/media/v4l2-core/v4l2-ctrls.c| 25 include/media/v4l2-ctrls.h | 25 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt index 43da22b..cecaff8 100644 --- a/Documentation/video4linux/v4l2-controls.txt +++ b/Documentation/video4linux/v4l2-controls.txt @@ -687,14 +687,20 @@ a control of this type whenever the first control belonging to a new control class is added. -Proposals for Extensions - +Adding Notify Callbacks +=== + +Sometimes the platform or bridge driver needs to be notified when a control +from a sub-device driver changes. You can set a notify callback by calling +this function: -Some ideas for future extensions to the spec: +void v4l2_ctrl_notify(struct v4l2_ctrl *ctrl, + void (*notify)(struct v4l2_ctrl *ctrl, void *priv), void *priv); -1) Add a V4L2_CTRL_FLAG_HEX to have values shown as hexadecimal instead of -decimal. Useful for e.g. video_mute_yuv. +Whenever the give control changes value the notify callback will be called +with a pointer to the control and the priv pointer that was passed with +v4l2_ctrl_notify. Note that the control's handler lock is held when the +notify function is called. -2) It is possible to mark in the controls array which controls have been -successfully written and which failed by for example adding a bit to the -control ID. Not sure if it is worth the effort, though. +There can be only one notify function per control handler. Any attempt +to set another notify function will cause a WARN_ON. diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index f400035..43061e1 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1160,6 +1160,8 @@ static void new_to_cur(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, send_event(fh, ctrl, (changed ? V4L2_EVENT_CTRL_CH_VALUE : 0) | (update_inactive ? V4L2_EVENT_CTRL_CH_FLAGS : 0)); + if (ctrl-call_notify changed ctrl-handler-notify) + ctrl-handler-notify(ctrl, ctrl-handler-notify_priv); } } @@ -2628,6 +2630,29 @@ int v4l2_ctrl_s_ctrl_int64(struct v4l2_ctrl *ctrl, s64 val) } EXPORT_SYMBOL(v4l2_ctrl_s_ctrl_int64); +void v4l2_ctrl_notify(struct v4l2_ctrl *ctrl, v4l2_ctrl_notify_fnc notify, void *priv) +{ + if (ctrl == NULL) + return; Isn't the caller supposed not to set ctrl to NULL ? A crash is easier to notice than a silent failure during development. + if (notify == NULL) { + ctrl-call_notify = 0; + return; + } + /* Only one notifier is allowed. Should we ever need to support +multiple notifiers, then some sort of linked list of notifiers +should be implemented. But I don't see any real reason to implement +that now. If you think you need multiple notifiers, then contact +the linux-media mailinglist. */ + if (WARN_ON(ctrl-handler-notify + (ctrl-handler-notify != notify || + ctrl-handler-notify_priv != priv))) + return; I'm not sure whether I like that. It feels a bit hackish. Wouldn't it be better to register the notifier with the handler explictly just once and then enable/disable notifications on a per-control basis ? + ctrl-handler-notify = notify; + ctrl-handler-notify_priv = priv; + ctrl-call_notify = 1; +} +EXPORT_SYMBOL(v4l2_ctrl_notify); + static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems) { struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev-fh-ctrl_handler, sev-id); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 6890f5e..4484fd3 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -53,6 +53,8 @@ struct v4l2_ctrl_ops { int (*s_ctrl)(struct v4l2_ctrl *ctrl); }; +typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv); + /** struct v4l2_ctrl - The control structure. * @node: The list node. * @ev_subs:The list of control event subscriptions. @@ -72,6 +74,8 @@ struct v4l2_ctrl_ops { * set this flag directly. * @has_volatiles: If set, then one or more members of the cluster are volatile. * Drivers should never touch this flag. + * @call_notify: If set,
Re: [PATCH] media-ctl: Fix build error with newer autotools
Hi Gary, On Monday 24 September 2012 08:34:24 Gary Thomas wrote: Rename configure.in to be configure.ac - required for newer versions of autotools (older versions silently handled this, now it's an error) Signed-off-by: Gary Thomas g...@mlbassoc.com Thanks for the patch. Applied and pushed. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] uvc: Add return code check at vb2_queue_init()
Hi Ezequiel, Thanks for the patch. On Monday 17 September 2012 10:49:50 Ezequiel Garcia wrote: This function returns an integer and it's mandatory to check the return code. Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- drivers/media/usb/uvc/uvc_queue.c |8 ++-- drivers/media/usb/uvc/uvc_video.c |4 +++- drivers/media/usb/uvc/uvcvideo.h |2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 5577381..2cec818 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -122,16 +122,20 @@ static struct vb2_ops uvc_queue_qops = { .buf_finish = uvc_buffer_finish, }; -void uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, +int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, int drop_corrupted) { + int rc; + Please use ret instead of rc. Other than that the patch looks good to me. queue-queue.type = type; queue-queue.io_modes = VB2_MMAP | VB2_USERPTR; queue-queue.drv_priv = queue; queue-queue.buf_struct_size = sizeof(struct uvc_buffer); queue-queue.ops = uvc_queue_qops; queue-queue.mem_ops = vb2_vmalloc_memops; - vb2_queue_init(queue-queue); + rc = vb2_queue_init(queue-queue); + if (rc) + return rc; mutex_init(queue-mutex); spin_lock_init(queue-irqlock); diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 1c15b42..57c3076 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1755,7 +1755,9 @@ int uvc_video_init(struct uvc_streaming *stream) atomic_set(stream-active, 0); /* Initialize the video buffers queue. */ - uvc_queue_init(stream-queue, stream-type, !uvc_no_drop_param); + ret = uvc_queue_init(stream-queue, stream-type, !uvc_no_drop_param); + if (ret) + return ret; /* Alternate setting 0 should be the default, yet the XBox Live Vision * Cam (and possibly other devices) crash or otherwise misbehave if diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 3764040..af216ec 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -600,7 +600,7 @@ extern struct uvc_driver uvc_driver; extern struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id); /* Video buffers queue management. */ -extern void uvc_queue_init(struct uvc_video_queue *queue, +extern int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, int drop_corrupted); extern int uvc_alloc_buffers(struct uvc_video_queue *queue, struct v4l2_requestbuffers *rb); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] uvc: Add return code check at vb2_queue_init()
This function returns an integer and it's mandatory to check the return code. Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- Changes from v1: * Change variable name rc - ret * Add return 0 on successful uvc_queue_init drivers/media/usb/uvc/uvc_queue.c | 10 -- drivers/media/usb/uvc/uvc_video.c |4 +++- drivers/media/usb/uvc/uvcvideo.h |2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 5577381..18a91fa 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -122,21 +122,27 @@ static struct vb2_ops uvc_queue_qops = { .buf_finish = uvc_buffer_finish, }; -void uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, +int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, int drop_corrupted) { + int ret; + queue-queue.type = type; queue-queue.io_modes = VB2_MMAP | VB2_USERPTR; queue-queue.drv_priv = queue; queue-queue.buf_struct_size = sizeof(struct uvc_buffer); queue-queue.ops = uvc_queue_qops; queue-queue.mem_ops = vb2_vmalloc_memops; - vb2_queue_init(queue-queue); + ret = vb2_queue_init(queue-queue); + if (ret) + return ret; mutex_init(queue-mutex); spin_lock_init(queue-irqlock); INIT_LIST_HEAD(queue-irqqueue); queue-flags = drop_corrupted ? UVC_QUEUE_DROP_CORRUPTED : 0; + + return 0; } /* - diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 1c15b42..57c3076 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1755,7 +1755,9 @@ int uvc_video_init(struct uvc_streaming *stream) atomic_set(stream-active, 0); /* Initialize the video buffers queue. */ - uvc_queue_init(stream-queue, stream-type, !uvc_no_drop_param); + ret = uvc_queue_init(stream-queue, stream-type, !uvc_no_drop_param); + if (ret) + return ret; /* Alternate setting 0 should be the default, yet the XBox Live Vision * Cam (and possibly other devices) crash or otherwise misbehave if diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 3764040..af216ec 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -600,7 +600,7 @@ extern struct uvc_driver uvc_driver; extern struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id); /* Video buffers queue management. */ -extern void uvc_queue_init(struct uvc_video_queue *queue, +extern int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, int drop_corrupted); extern int uvc_alloc_buffers(struct uvc_video_queue *queue, struct v4l2_requestbuffers *rb); -- 1.7.8.6 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ISPsupport
Hi John, On Monday 10 September 2012 20:00:54 John Tobias wrote: Hi all, I tried devel-ISPSUPPORT-IPIPE and devel-ISPSUPPORT, It would help you you told use what hardware you're running on, what kernel version you're using, and what devel-ISPSUPPORT-IPIPE and devel-ISPSUPPORT are. the kernel detected my image sensor (ov5650). But, when I execute the yavta /dev/video0 -c4 -n1 -s2592x1944 -fSGRBG10 -Fov5650-2592x1944-#.bin I was getting Unable to start streaming: Invalid argument (22).. I would like to know if anyone here can guide me a bit in order to have a working environment?. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: use of timestamp/sequence in v4l2_buffer
Hi, On Thursday 13 September 2012 21:44:48 Sakari Ailus wrote: On Tue, Sep 04, 2012 at 12:38:06PM +0200, Hans Verkuil wrote: Hi all, During the Media Workshop last week we discussed how the timestamp and sequence fields in struct v4l2_buffer should be used. While trying to document the exact behavior I realized that there are a few missing pieces. Open questions with regards to the sequence field: 1) Should the first frame that was captured or displayed after starting streaming for the first time always start with sequence index 0? In my opinion it should. I agree. Agreed. 2) Should the sequence counter be reset to 0 after a STREAMOFF? Or should it only be reset to 0 after REQBUFS/CREATE_BUFS is called? Definitely not after CREATE_BUFS. Streaming may be ongoing when the IOCTL is called, and this will cause a great deal of trouble. I'd propose resetting it to zero at streamon (or streamoff) time. Agreed. 3) Should the sequence counter behave differently for mem2mem devices? With the current definition both the capture and display sides of a mem2mem device just have their own independent sequence counter. They need to be independent, as one frame can be encoded/decoded to several frames (or the other way around). 4) If frames are skipped, should the sequence counter skip as well? In my opinion it shouldn't. Do you mean skipping incrementing the counter, or skipping the frame number? :-) In my opinion the sequence number must be increased in this case. Not doing so would make it difficult to figure out frames have been skipped in the first place. That may be important in some cases. I can't think of any adverse effects this could result; the OMAP 3 ISP driver does so, for example. I agree, I think the sequence counter should be incremented for skipped frames. Not all drivers can detect frame skips though, or how many frames are skipped. On the side of additional positive effects, consider the following quite realistic scenario: A frame is skipped on a single buffer queue of a device producing two streams of the same source, e.g. a sensor, whereas no frame is skipped on the second video buffer queue. Not incrementing the sequence would make the sequence numbers out-of-sync, and the situation would even be difficult to detect from the user space --- frame sequence numbers are important in associating buffers from different streams to the same original captured image. 5) Should the sequence counter always be monotonically increasing? I think it should. With the exception of the above, in my opinion yes. I.e. you're not supposed to have a decrease in field_count until it wraps around. Open questions with regards to the timestamp field: 6) For output devices the timestamp field can be used to determine when to transmit the frame. In practice there are no output drivers that support this. It is also unclear how this would work: if the timestamp is 1 hour into the future, should the driver hold on to that frame for one hour? If another frame is queued with a timestamp that's earlier than the previous frame, should that frame be output first? I am inclined to drop this behavior from the spec. Should we get drivers that actually implement this, then we need to clarify the spec and add a new capability flag somewhere to tell userspace that you can actually use the timestamp for this purpose. 7) Should the timestamp field always be monotonically increasing? Or it is possible to get timestamps that jump around? This makes sense for encoders that create B-frames referring to frames captured earlier than an I-frame. And for decoders that need to hold a key frame longer than others. (Or to save system memory resources, return it to the user space with the proposed READ_ONLY flag not agreed on yet.) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Integrate camera interface of OMAP3530 in Angstrom Linux
Hi Andreas, On Thursday 06 September 2012 20:00:04 Andreas Nagel wrote: Hello, I am using an embedded module called TAO-3530 from Technexion, which has an OMAP3530 processor. This processor has a camera interface, which is part of the ISP submodule. For an ongoing project I want to capture a video signal from this interface. After several days of excessive research I still don't know, how to access it. I configured the Angstrom kernel (2.6.32), so that the driver for OMAP 3 camera controller (and all other OMAP 3 related things) is integrated, but I don't see any new device nodes in the filesystem. You should upgrade to a much more recent kernel, as the driver for the OMAP3 ISP included in the Angstrom 2.6.32 kernel is just bad legacy code that should be burried in a very deep cave. I also found some rumors, that the Media Controller Framework or driver provides the device node /dev/media0, but I was not able to install it. You will need to upgrade your kernel for that. I'd advise going for the latest mainline kernel. I use OpenEmbedded, but I don't have a recipe for Media Controller. On the Angstrom website ( http://www.angstrom-distribution.org/repo/ ) there's actually a package called media-ctl, but due to the missing recipe, i can't install it. Can't say, I am an expert in OE. Can you help me point out, what's necessary to make the camera interface accessible? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] uvc: Add return code check at vb2_queue_init()
On Wednesday 26 September 2012 08:30:34 Ezequiel Garcia wrote: This function returns an integer and it's mandatory to check the return code. Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Ezequiel Garcia elezegar...@gmail.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Applied to my tree, thank you. --- Changes from v1: * Change variable name rc - ret * Add return 0 on successful uvc_queue_init drivers/media/usb/uvc/uvc_queue.c | 10 -- drivers/media/usb/uvc/uvc_video.c |4 +++- drivers/media/usb/uvc/uvcvideo.h |2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 5577381..18a91fa 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -122,21 +122,27 @@ static struct vb2_ops uvc_queue_qops = { .buf_finish = uvc_buffer_finish, }; -void uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, +int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, int drop_corrupted) { + int ret; + queue-queue.type = type; queue-queue.io_modes = VB2_MMAP | VB2_USERPTR; queue-queue.drv_priv = queue; queue-queue.buf_struct_size = sizeof(struct uvc_buffer); queue-queue.ops = uvc_queue_qops; queue-queue.mem_ops = vb2_vmalloc_memops; - vb2_queue_init(queue-queue); + ret = vb2_queue_init(queue-queue); + if (ret) + return ret; mutex_init(queue-mutex); spin_lock_init(queue-irqlock); INIT_LIST_HEAD(queue-irqqueue); queue-flags = drop_corrupted ? UVC_QUEUE_DROP_CORRUPTED : 0; + + return 0; } /* --- -- diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 1c15b42..57c3076 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1755,7 +1755,9 @@ int uvc_video_init(struct uvc_streaming *stream) atomic_set(stream-active, 0); /* Initialize the video buffers queue. */ - uvc_queue_init(stream-queue, stream-type, !uvc_no_drop_param); + ret = uvc_queue_init(stream-queue, stream-type, !uvc_no_drop_param); + if (ret) + return ret; /* Alternate setting 0 should be the default, yet the XBox Live Vision * Cam (and possibly other devices) crash or otherwise misbehave if diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 3764040..af216ec 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -600,7 +600,7 @@ extern struct uvc_driver uvc_driver; extern struct uvc_entity *uvc_entity_by_id(struct uvc_device *dev, int id); /* Video buffers queue management. */ -extern void uvc_queue_init(struct uvc_video_queue *queue, +extern int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type, int drop_corrupted); extern int uvc_alloc_buffers(struct uvc_video_queue *queue, struct v4l2_requestbuffers *rb); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] media: ov7670: driver cleanup and support for ov7674.
On 26 September 2012 11:47, Javier Martin javier.mar...@vista-silicon.com wrote: The following series includes all the changes discussed in [1] that don't affect either bridge drivers that use ov7670 or soc-camera framework For this reason they are considered non controversial and sent separately. At least 1 more series will follow in order to implement all features described in [1]. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html Support is for ov7675, not ov7674, sorry for the typo. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-usb: print small buffers via %*ph
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/media/usb/dvb-usb/a800.c |2 +- drivers/media/usb/dvb-usb/cinergyT2-core.c |3 +-- drivers/media/usb/dvb-usb/dibusb-common.c |2 +- drivers/media/usb/dvb-usb/digitv.c |2 +- drivers/media/usb/dvb-usb/dtt200u.c|2 +- drivers/media/usb/dvb-usb/m920x.c |2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/media/usb/dvb-usb/a800.c b/drivers/media/usb/dvb-usb/a800.c index 8d7fef8..83684ed 100644 --- a/drivers/media/usb/dvb-usb/a800.c +++ b/drivers/media/usb/dvb-usb/a800.c @@ -93,7 +93,7 @@ static int a800_rc_query(struct dvb_usb_device *d, u32 *event, int *state) /* call the universal NEC remote processor, to find out the key's state and event */ dvb_usb_nec_rc_key_to_event(d,key,event,state); if (key[0] != 0) - deb_rc(key: %x %x %x %x %x\n,key[0],key[1],key[2],key[3],key[4]); + deb_rc(key: %*ph\n, 5, key); ret = 0; out: kfree(key); diff --git a/drivers/media/usb/dvb-usb/cinergyT2-core.c b/drivers/media/usb/dvb-usb/cinergyT2-core.c index 0a98548..9fd1527 100644 --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c @@ -172,8 +172,7 @@ static int cinergyt2_rc_query(struct dvb_usb_device *d, u32 *event, int *state) if (*event != d-last_event) st-rc_counter = 0; - deb_rc(key: %x %x %x %x %x\n, - key[0], key[1], key[2], key[3], key[4]); + deb_rc(key: %*ph\n, 5, key); } return 0; } diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c index a76bbb2..af0d432 100644 --- a/drivers/media/usb/dvb-usb/dibusb-common.c +++ b/drivers/media/usb/dvb-usb/dibusb-common.c @@ -473,7 +473,7 @@ int dibusb_rc_query(struct dvb_usb_device *d, u32 *event, int *state) dvb_usb_generic_rw(d,cmd,1,key,5,0); dvb_usb_nec_rc_key_to_event(d,key,event,state); if (key[0] != 0) - deb_info(key: %x %x %x %x %x\n,key[0],key[1],key[2],key[3],key[4]); + deb_info(key: %*ph\n, 5, key); return 0; } EXPORT_SYMBOL(dibusb_rc_query); diff --git a/drivers/media/usb/dvb-usb/digitv.c b/drivers/media/usb/dvb-usb/digitv.c index ff34419..772bde3 100644 --- a/drivers/media/usb/dvb-usb/digitv.c +++ b/drivers/media/usb/dvb-usb/digitv.c @@ -253,7 +253,7 @@ static int digitv_rc_query(struct dvb_usb_device *d, u32 *event, int *state) } if (key[0] != 0) - deb_rc(key: %x %x %x %x %x\n,key[0],key[1],key[2],key[3],key[4]); + deb_rc(key: %*ph\n, 5, key); return 0; } diff --git a/drivers/media/usb/dvb-usb/dtt200u.c b/drivers/media/usb/dvb-usb/dtt200u.c index 66f205c..c357fb3 100644 --- a/drivers/media/usb/dvb-usb/dtt200u.c +++ b/drivers/media/usb/dvb-usb/dtt200u.c @@ -84,7 +84,7 @@ static int dtt200u_rc_query(struct dvb_usb_device *d, u32 *event, int *state) dvb_usb_generic_rw(d,cmd,1,key,5,0); dvb_usb_nec_rc_key_to_event(d,key,event,state); if (key[0] != 0) - deb_info(key: %x %x %x %x %x\n,key[0],key[1],key[2],key[3],key[4]); + deb_info(key: %*ph\n, 5, key); return 0; } diff --git a/drivers/media/usb/dvb-usb/m920x.c b/drivers/media/usb/dvb-usb/m920x.c index 288af29..661bb75 100644 --- a/drivers/media/usb/dvb-usb/m920x.c +++ b/drivers/media/usb/dvb-usb/m920x.c @@ -358,7 +358,7 @@ static int m920x_firmware_download(struct usb_device *udev, const struct firmwar if ((ret = m920x_read(udev, M9206_FILTER, 0x0, 0x8000, read, 4)) != 0) goto done; - deb(%x %x %x %x\n, read[0], read[1], read[2], read[3]); + deb(%*ph\n, 4, read); if ((ret = m920x_read(udev, M9206_FW, 0x0, 0x0, read, 1)) != 0) goto done; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: wrong image after resizer with mt9v034 sensor
Hi Laurent, 2012/9/26 Laurent Pinchart laurent.pinch...@ideasonboard.com: Hi Enric, On Wednesday 26 September 2012 09:57:53 Enric Balletbò i Serra wrote: [snip] You had reason. Checking the data lines of the camera bus with an oscilloscope I see I had a problem, exactly in D8 /D9 data lines. I'm curious, how have you fixed that ? The board had a pull-down 4k7 resistor which I removed in these lines (D8/D9). The board is prepared to accept sensors from 8 to 12 bits, lines from D8 to D12 have a pull-down resistor to tie down the line by default. With the oscilloscope I saw that D8/D9 had problems to go to high level like you said, then I checked the schematic and I saw these resistors. Now I can capture images but the color is still wrong, see the following image captured with pipeline SENSOR - CCDC OUTPUT http://downloads.isee.biz/pub/files/patterns/img-01.pnm Now the image was converted using : ./raw2rgbpnm -s 752x480 -f SGRBG10 img-01.bin img-01.pnm And the raw data can be found here: http://downloads.isee.biz/pub/files/patterns/img-01.bin Any idea where I can look ? Thanks. Your sensors produces BGGR data if I'm not mistaken, not GRBG. raw2rgbpnm doesn't support BGGR (yet), but the OMAP3 ISP preview engine can convert that to YUV since v3.5. Just make your sensor driver expose the right media bus format and configure the pipeline accordingly. The datasheet (p.10,11) says that the Pixel Color Pattern is as follows. direction n 4321 .. GB GB GB GB .. RG RG RG RG So seems you're right, if the first byte is on the right the sensor produces BGGR. But for some reason the mt9v032 driver uses GRBG data. Maybe is related with following lines which writes register 0x0D Read Mode (p.26,27) and presumably flips row or column bytes (not sure about this I need to check) 334 /* Configure the window size and row/column bin */ 335 hratio = DIV_ROUND_CLOSEST(crop-width, format-width); 336 vratio = DIV_ROUND_CLOSEST(crop-height, format-height); 337 338 ret = mt9v032_write(client, MT9V032_READ_MODE, 339 (hratio - 1) MT9V032_READ_MODE_ROW_BIN_SHIFT | 340 (vratio - 1) MT9V032_READ_MODE_COLUMN_BIN_SHIFT); Nonetheless, I changed the driver to configure for BGGR pattern. Using the Sensor-CCDC-Preview-Resizer pipeline I captured the data with yavta and converted using raw2rgbpnm program. ./raw2rgbpnm -s 752x480 -f UYVY img-01.uyvy img-01.pnm and the result is http://downloads.isee.biz/pub/files/patterns/img-02.pnm http://downloads.isee.biz/pub/files/patterns/img-02.bin The image looks better than older, not perfect, but better. The image is only a bit yellowish. Could be this a hardware issue ? We are close to ... Regards, Enric -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi. On 09/26/2012 12:42 AM, Sakari Ailus wrote: On Wed, Sep 26, 2012 at 12:14:36PM +0530, Prabhakar Lad wrote: Hi All, On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Hi Prabhakar, As these are low level controls, I wonder whether it would make sense to make a difference between digital and analogue gain. I admit I'm not quite as certain whether there's such a large difference as there is for global gains for the camera control algorithms. Sorry to make this more complicated, but the Aptina MT9P031, for instance (datasheet at http://www.aptina.com/assets/downloadDocument.do?id=865 - see page 35), has Digital Gain, an Analog Multiplier, and Analog Gain (for each of R, Gr, Gb, and B). For each color channel, there is one register, with the bits divided up into the three gain types. Furthermore, the different gain types have different units (increments). Currently (at least in the last version I've used), the driver hides all this and provides a single gain control, and prioritizes which gain types are adjusted at different user-level gain settings in accordance with the datasheet recommendations (e.g. keep the analog gain between 1 and 4 for best noise performance, and use the multiplier for gains between 4 and 8). This seems very sensible. If we try to distinguish between analog and digital gains in the control definitions, what should this driver do? And what about the multiplier? I suppose it could be hidden by the driver as part of the analog gain, as the driver currently does for the entire gain... Cheers, Chris -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi Chris, On Wednesday 26 September 2012 07:42:41 Chris MacGregor wrote: On 09/26/2012 12:42 AM, Sakari Ailus wrote: On Wed, Sep 26, 2012 at 12:14:36PM +0530, Prabhakar Lad wrote: On Sun, Sep 23, 2012 at 4:56 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I am listing out the gain controls which is the outcome of above discussion:- 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET 6: V4L2_CID_BLUE_OFFSET 7: V4L2_CID_RED_OFFSET 8: V4L2_CID_GREEN_OFFSET Hi Prabhakar, As these are low level controls, I wonder whether it would make sense to make a difference between digital and analogue gain. I admit I'm not quite as certain whether there's such a large difference as there is for global gains for the camera control algorithms. Sorry to make this more complicated, but the Aptina MT9P031, for instance (datasheet at http://www.aptina.com/assets/downloadDocument.do?id=865 - see page 35), has Digital Gain, an Analog Multiplier, and Analog Gain (for each of R, Gr, Gb, and B). For each color channel, there is one register, with the bits divided up into the three gain types. Furthermore, the different gain types have different units (increments). Currently (at least in the last version I've used), the driver hides all this and provides a single gain control, and prioritizes which gain types are adjusted at different user-level gain settings in accordance with the datasheet recommendations (e.g. keep the analog gain between 1 and 4 for best noise performance, and use the multiplier for gains between 4 and 8). This seems very sensible. I think it should be fine for now. If we later find out that a user space application really needs to control the analog and digital gains individually and precisely we can always split the controls then. For now I think a single gain control (per channel) that groups analog and digital gains should be enough. If we try to distinguish between analog and digital gains in the control definitions, what should this driver do? And what about the multiplier? I suppose it could be hidden by the driver as part of the analog gain, as the driver currently does for the entire gain... -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v3 0/5] s5p-fimc: Add interleaved image data capture support
Hi All, This patch series adds device/vendor specific media bus pixel code section and defines S5C73MX camera specific media bus pixel code, along with corresponding fourcc. The third patch adds support for MIPI-CSI2 Embedded Data capture in Samsung S5P/Exynos MIPI-CSIS device. It depends on patch [PATCH RFC] V4L: Add s_rx_buffer subdev video operation. The fourth patch extends s5p-fimc driver to allow it to support 2-planar V4L2_PIX_FMT_S5C_UYVY_JPG format. More details can be found in the patch summary. The [get/set]_frame_desc subdev callback are used only to retrieve from a sensor subdev required buffer size. It depends on patch [PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks The fifth patch adds [get/set]_frame_desc op handlers to the m5mols driver as an example. I prepared also similar patch for S5C73M3 sensor where 2 frame description entries are used, but that driver is not yet mainlined due to a few missing items in V4L2 required to fully control it, so I didn't include that patch in this series. Changes since v2: - addressed review comments on patches adding the media bus pixel code and the fourcc, - minor cleanup of patch 4/5. This series with all dependencies can be found in git repository git://git.infradead.org/users/kmpark/linux-samsung v4l-framedesc (http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/v4l-framedesc) Thanks, Sylwester Sylwester Nawrocki (5): V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition s5p-csis: Add support for non-image data packets capture s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc m5mols: Implement .get_frame_desc subdev callback Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/pixfmt.xml | 28 + Documentation/DocBook/media/v4l/subdev-formats.xml | 44 +++ drivers/media/i2c/m5mols/m5mols.h | 9 ++ drivers/media/i2c/m5mols/m5mols_capture.c | 3 + drivers/media/i2c/m5mols/m5mols_core.c | 47 +++ drivers/media/i2c/m5mols/m5mols_reg.h | 1 + drivers/media/platform/s5p-fimc/fimc-capture.c | 135 ++--- drivers/media/platform/s5p-fimc/fimc-core.c| 19 ++- drivers/media/platform/s5p-fimc/fimc-core.h| 28 - drivers/media/platform/s5p-fimc/fimc-reg.c | 23 +++- drivers/media/platform/s5p-fimc/fimc-reg.h | 3 +- drivers/media/platform/s5p-fimc/mipi-csis.c| 59 - include/linux/v4l2-mediabus.h | 5 + include/linux/videodev2.h | 1 + 15 files changed, 376 insertions(+), 33 deletions(-) -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v3 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
This patch adds media bus pixel code for the interleaved JPEG/UYVY image format used by S5C73MX Samsung cameras. This interleaved image data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data. It also defines an experimental vendor and device specific media bus formats section and adds related DocBook documentation. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Documentation/DocBook/media/v4l/compat.xml | 4 ++ Documentation/DocBook/media/v4l/subdev-formats.xml | 44 ++ include/linux/v4l2-mediabus.h | 5 +++ 3 files changed, 53 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 802c1ab..b12cddb 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2612,6 +2612,10 @@ ioctls./para listitem paraExporting DMABUF files using VIDIOC-EXPBUF; ioctl./para /listitem +listitem + paraVendor and device specific media bus pixel formats. + xref linkend=v4l2-mbus-vendor-spec-fmts /./para +/listitem /itemizedlist /section diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..a0a9364 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -2565,5 +2565,49 @@ /tgroup /table /section + +section id=v4l2-mbus-vendor-spec-fmts + titleVendor and Device Specific Formats/title + + note + titleExperimental/title + paraThis is an link linkend=experimentalexperimental/link +interface and may change in the future./para + /note + + paraThis section lists complex data formats that are either vendor or + device specific. + /para + + paraThe following table lists the existing vendor and device specific + formats./para + + table pgwide=0 frame=none id=v4l2-mbus-pixelcode-vendor-specific + titleVendor and device specific formats/title + tgroup cols=3 + colspec colname=id align=left / + colspec colname=code align=left/ + colspec colname=remarks align=left/ + thead + row + entryIdentifier/entry + entryCode/entry + entryComments/entry + /row + /thead + tbody valign=top + row id=V4L2-MBUS-FMT-S5C-UYVY-JPEG-1X8 + entryV4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8/entry + entry0x5001/entry + entry + Interleaved raw UYVY and JPEG image format with embedded + meta-data used by Samsung S3C73MX camera sensors. + /entry + /row + /tbody + /tgroup + /table +/section + /section /section diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h index 5ea7f75..7d64e0e 100644 --- a/include/linux/v4l2-mediabus.h +++ b/include/linux/v4l2-mediabus.h @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode { /* JPEG compressed formats - next is 0x4002 */ V4L2_MBUS_FMT_JPEG_1X8 = 0x4001, + + /* Vendor specific formats - next is 0x5002 */ + + /* S5C73M3 sensor specific interleaved UYVY and JPEG */ + V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x5001, }; /** -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v3 2/5] V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition
This patch adds definition of the Samsung S5C73M3 camera specific image format. V4L2_PIX_FMT_S5C_UYVY_JPG is a two-planar format, the first plane contains interleaved UYVY and JPEG data followed by meta-data. The second plane contains additional meta-data needed for extracting JPEG and UYVY data stream from the first plane. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Documentation/DocBook/media/v4l/pixfmt.xml | 28 include/linux/videodev2.h | 1 + 2 files changed, 29 insertions(+) diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml index 1ddbfab..21284ba 100644 --- a/Documentation/DocBook/media/v4l/pixfmt.xml +++ b/Documentation/DocBook/media/v4l/pixfmt.xml @@ -996,6 +996,34 @@ the other bits are set to 0./entry entryOld 6-bit greyscale format. Only the most significant 6 bits of each byte are used, the other bits are set to 0./entry /row + row id=V4L2-PIX-FMT-S5C-UYVY-JPG + entryconstantV4L2_PIX_FMT_S5C_UYVY_JPG/constant/entry + entry'S5CI'/entry + entryTwo-planar format used by Samsung S5C73MX cameras. The +first plane contains interleaved JPEG and UYVY image data, followed by meta data +in form of an array of offsets to the UYVY data blocks. The actual pointer array +follows immediately the interleaved JPEG/UYVY data, the number of entries in +this array equals the height of the UYVY image. Each entry is a 4-byte unsigned +integer in big endian order and it's an offset to a single pixel line of the +UYVY image. The first plane can start either with JPEG or UYVY data chunk. The +size of a single UYVY block equals the UYVY image's width multiplied by 2. The +size of a JPEG chunk depends on the image and can vary with each line. +paraThe second plane, at an offset of 4084 bytes, contains a 4-byte offset to +the pointer array in the first plane. This offset is followed by a 4-byte value +indicating size of the pointer array. All numbers in the second plane are also +in big endian order. Remaining data in the first plane is undefined. The +information in the second plane allows to easily find location of the pointer +array, which can be different for each frame. The size of the pointer array is +constant for given UYVY image height./para +paraIn order to extract UYVY and JPEG frames an application can initially set +a data pointer to the start of first plane and then add an offset from the first +entry of the pointers table. Such a pointer indicates start of an UYVY image +pixel line. Whole UYVY line can be copied to a separate buffer. These steps +should be repeated for each line, i.e. the number of entries in the pointer +array. Anything what's in between the UYVY lines is JPEG data and should be +concatenated to form the JPEG stream. /para +/entry + /row /tbody /tgroup /table diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 8d29bb2..6c82ff5 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -436,6 +436,7 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_KONICA420 v4l2_fourcc('K', 'O', 'N', 'I') /* YUV420 planar in blocks of 256 pixels */ #define V4L2_PIX_FMT_JPGL v4l2_fourcc('J', 'P', 'G', 'L') /* JPEG-Lite */ #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */ +#define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'J') /* S5C73M3 interleaved UYVY/JPEG */ /* * F O R M A T E N U M E R A T I O N -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v3 3/5] s5p-csis: Add support for non-image data packets capture
MIPI-CSI has internal memory mapped buffers for the frame embedded (non-image) data. There are two buffers, for even and odd frames which need to be saved after an interrupt is raised. The packet data buffers size is 4 KiB and there is no status register in the hardware where the actual non-image data size can be read from. Hence the driver copies whole packet data buffer into a buffer provided by the FIMC driver. This will form a separate plane in the user buffer. When FIMC DMA engine is stopped by the driver due the to user space not keeping up with buffer de-queuing the MIPI-CSIS will still run, however it must discard data which is not captured by FIMC. Which frames are actually captured by MIPI-CSIS is determined by means of the s_rx_buffer subdev callback. When it is not called after a single embedded data frame has been captured and copied and before next embedded data frame interrupt occurrs, subsequent embedded data frames will be dropped. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-fimc/mipi-csis.c | 51 + 1 file changed, 51 insertions(+) diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c index c1c5c86..306410f 100644 --- a/drivers/media/platform/s5p-fimc/mipi-csis.c +++ b/drivers/media/platform/s5p-fimc/mipi-csis.c @@ -98,6 +98,11 @@ MODULE_PARM_DESC(debug, Debug level (0-2)); #define CSIS_MAX_PIX_WIDTH 0x #define CSIS_MAX_PIX_HEIGHT0x +/* Non-image packet data buffers */ +#define S5PCSIS_PKTDATA_ODD0x2000 +#define S5PCSIS_PKTDATA_EVEN 0x3000 +#define S5PCSIS_PKTDATA_SIZE SZ_4K + enum { CSIS_CLK_MUX, CSIS_CLK_GATE, @@ -144,6 +149,11 @@ static const struct s5pcsis_event s5pcsis_events[] = { }; #define S5PCSIS_NUM_EVENTS ARRAY_SIZE(s5pcsis_events) +struct csis_pktbuf { + u32 *data; + unsigned int len; +}; + /** * struct csis_state - the driver's internal state data structure * @lock: mutex serializing the subdev and power management operations, @@ -160,6 +170,7 @@ static const struct s5pcsis_event s5pcsis_events[] = { * @csis_fmt: current CSIS pixel format * @format: common media bus format for the source and sink pad * @slock: spinlock protecting structure members below + * @pkt_buf: the frame embedded (non-image) data buffer * @events: MIPI-CSIS event (error) counters */ struct csis_state { @@ -177,6 +188,7 @@ struct csis_state { struct v4l2_mbus_framefmt format; struct spinlock slock; + struct csis_pktbuf pkt_buf; struct s5pcsis_event events[S5PCSIS_NUM_EVENTS]; }; @@ -534,6 +546,21 @@ static int s5pcsis_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, return 0; } +static int s5pcsis_s_rx_buffer(struct v4l2_subdev *sd, void *buf, + unsigned int *size) +{ + struct csis_state *state = sd_to_csis_state(sd); + unsigned long flags; + + spin_lock_irqsave(state-slock, flags); + *size = min_t(unsigned int, *size, S5PCSIS_PKTDATA_SIZE); + state-pkt_buf.data = buf; + state-pkt_buf.len = *size; + spin_unlock_irqrestore(state-slock, flags); + + return 0; +} + static int s5pcsis_log_status(struct v4l2_subdev *sd) { struct csis_state *state = sd_to_csis_state(sd); @@ -571,6 +598,7 @@ static struct v4l2_subdev_pad_ops s5pcsis_pad_ops = { }; static struct v4l2_subdev_video_ops s5pcsis_video_ops = { + .s_rx_buffer = s5pcsis_s_rx_buffer, .s_stream = s5pcsis_s_stream, }; @@ -580,16 +608,39 @@ static struct v4l2_subdev_ops s5pcsis_subdev_ops = { .video = s5pcsis_video_ops, }; +static void s5pcsis_pkt_copy(struct csis_state *state, u32 *buf, +u32 offset, u32 size) +{ + int i; + + for (i = 0; i S5PCSIS_PKTDATA_SIZE; i += 4, buf++) + *buf = s5pcsis_read(state, offset + i); +} + static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) { struct csis_state *state = dev_id; + struct csis_pktbuf *pktbuf = state-pkt_buf; unsigned long flags; u32 status; status = s5pcsis_read(state, S5PCSIS_INTSRC); + s5pcsis_write(state, S5PCSIS_INTSRC, status); spin_lock_irqsave(state-slock, flags); + if ((status S5PCSIS_INTSRC_NON_IMAGE_DATA) pktbuf-data) { + u32 offset; + + if (status S5PCSIS_INTSRC_EVEN) + offset = S5PCSIS_PKTDATA_EVEN; + else + offset = S5PCSIS_PKTDATA_ODD; + + s5pcsis_pkt_copy(state, pktbuf-data, offset, pktbuf-len); + pktbuf-data = NULL; + } + /* Update the event/error counters */ if ((status S5PCSIS_INTSRC_ERRORS) || debug) {
[PATCH RFC v3 4/5] s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc
The V4L2_PIX_FMT_S5C_YUYV_JPG image formats consists of 2 planes, the first containing interleaved JPEG/YUYV data and the second containing meta data describing the interleaving method. The image data is transferred with MIPI-CSI User Defined Byte-Based Data 1 type and is captured to memory by FIMC DMA engine. The meta data is transferred using MIPI-CSI2 Embedded 8-bit non Image Data and it is captured in the MIPI-CSI slave device and copied to the bridge provided buffer. To make sure the size of allocated buffers is correct for the subdevs configuration when VIDIOC_STREAMON ioctl is invoked, an additional check is added at the video pipeline validation function. Flag FMT_FLAGS_COMPRESSED indicates the buffer size must be retrieved from a sensor subdev. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-fimc/fimc-capture.c | 135 + drivers/media/platform/s5p-fimc/fimc-core.c| 19 +++- drivers/media/platform/s5p-fimc/fimc-core.h| 28 - drivers/media/platform/s5p-fimc/fimc-reg.c | 23 - drivers/media/platform/s5p-fimc/fimc-reg.h | 3 +- drivers/media/platform/s5p-fimc/mipi-csis.c| 8 +- 6 files changed, 183 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c index 8a0908b..304b6fd 100644 --- a/drivers/media/platform/s5p-fimc/fimc-capture.c +++ b/drivers/media/platform/s5p-fimc/fimc-capture.c @@ -177,7 +177,9 @@ static int fimc_capture_config_update(struct fimc_ctx *ctx) void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf) { + struct v4l2_subdev *csis = fimc-pipeline.subdevs[IDX_CSIS]; struct fimc_vid_cap *cap = fimc-vid_cap; + struct fimc_frame *f = cap-ctx-d_frame; struct fimc_vid_buffer *v_buf; struct timeval *tv; struct timespec ts; @@ -216,6 +218,25 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf) if (++cap-buf_index = FIMC_MAX_OUT_BUFS) cap-buf_index = 0; } + /* +* Set up a buffer at MIPI-CSIS if current image format +* requires the frame embedded data capture. +*/ + if (f-fmt-mdataplanes !list_empty(cap-active_buf_q)) { + unsigned int plane = ffs(f-fmt-mdataplanes) - 1; + unsigned int size = f-payload[plane]; + s32 index = fimc_hw_get_frame_index(fimc); + void *vaddr; + + list_for_each_entry(v_buf, cap-active_buf_q, list) { + if (v_buf-index != index) + continue; + vaddr = vb2_plane_vaddr(v_buf-vb, plane); + v4l2_subdev_call(csis, video, s_rx_buffer, +vaddr, size); + break; + } + } if (cap-active_buf_cnt == 0) { if (deq_buf) @@ -351,6 +372,8 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *pfmt, unsigned int size = (wh * fmt-depth[i]) / 8; if (pixm) sizes[i] = max(size, pixm-plane_fmt[i].sizeimage); + else if (fimc_fmt_is_user_defined(fmt-color)) + sizes[i] = frame-payload[i]; else sizes[i] = max_t(u32, size, frame-payload[i]); @@ -611,10 +634,10 @@ static struct fimc_fmt *fimc_capture_try_format(struct fimc_ctx *ctx, u32 mask = FMT_FLAGS_CAM; struct fimc_fmt *ffmt; - /* Color conversion from/to JPEG is not supported */ + /* Conversion from/to JPEG or User Defined format is not supported */ if (code ctx-s_frame.fmt pad == FIMC_SD_PAD_SOURCE - fimc_fmt_is_jpeg(ctx-s_frame.fmt-color)) - *code = V4L2_MBUS_FMT_JPEG_1X8; + fimc_fmt_is_user_defined(ctx-s_frame.fmt-color)) + *code = ctx-s_frame.fmt-mbus_code; if (fourcc *fourcc != V4L2_PIX_FMT_JPEG pad != FIMC_SD_PAD_SINK) mask |= FMT_FLAGS_M2M; @@ -628,18 +651,19 @@ static struct fimc_fmt *fimc_capture_try_format(struct fimc_ctx *ctx, *fourcc = ffmt-fourcc; if (pad == FIMC_SD_PAD_SINK) { - max_w = fimc_fmt_is_jpeg(ffmt-color) ? + max_w = fimc_fmt_is_user_defined(ffmt-color) ? pl-scaler_dis_w : pl-scaler_en_w; /* Apply the camera input interface pixel constraints */ v4l_bound_align_image(width, max_t(u32, *width, 32), max_w, 4, height, max_t(u32, *height, 32), FIMC_CAMIF_MAX_HEIGHT, - fimc_fmt_is_jpeg(ffmt-color) ? 3 : 1, +
[PATCH RFC v3 5/5] m5mols: Implement .get_frame_desc subdev callback
.get_frame_desc can be used by host interface driver to query properties of captured frames, e.g. required memory buffer size. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/i2c/m5mols/m5mols.h | 9 ++ drivers/media/i2c/m5mols/m5mols_capture.c | 3 ++ drivers/media/i2c/m5mols/m5mols_core.c| 47 +++ drivers/media/i2c/m5mols/m5mols_reg.h | 1 + 4 files changed, 60 insertions(+) diff --git a/drivers/media/i2c/m5mols/m5mols.h b/drivers/media/i2c/m5mols/m5mols.h index 15d3a4f..de3b755 100644 --- a/drivers/media/i2c/m5mols/m5mols.h +++ b/drivers/media/i2c/m5mols/m5mols.h @@ -19,6 +19,13 @@ #include media/v4l2-subdev.h #include m5mols_reg.h + +/* An amount of data transmitted in addition to the value + * determined by CAPP_JPEG_SIZE_MAX register. + */ +#define M5MOLS_JPEG_TAGS_SIZE 0x2 +#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * SZ_1M) + extern int m5mols_debug; enum m5mols_restype { @@ -67,12 +74,14 @@ struct m5mols_exif { /** * struct m5mols_capture - Structure for the capture capability * @exif: EXIF information + * @buf_size: internal JPEG frame buffer size, in bytes * @main: size in bytes of the main image * @thumb: size in bytes of the thumb image, if it was accompanied * @total: total size in bytes of the produced image */ struct m5mols_capture { struct m5mols_exif exif; + unsigned int buf_size; u32 main; u32 thumb; u32 total; diff --git a/drivers/media/i2c/m5mols/m5mols_capture.c b/drivers/media/i2c/m5mols/m5mols_capture.c index cb243bd..ab34cce 100644 --- a/drivers/media/i2c/m5mols/m5mols_capture.c +++ b/drivers/media/i2c/m5mols/m5mols_capture.c @@ -105,6 +105,7 @@ static int m5mols_capture_info(struct m5mols_info *info) int m5mols_start_capture(struct m5mols_info *info) { + unsigned int framesize = info-cap.buf_size - M5MOLS_JPEG_TAGS_SIZE; struct v4l2_subdev *sd = info-sd; int ret; @@ -121,6 +122,8 @@ int m5mols_start_capture(struct m5mols_info *info) if (!ret) ret = m5mols_write(sd, CAPP_MAIN_IMAGE_SIZE, info-resolution); if (!ret) + ret = m5mols_write(sd, CAPP_JPEG_SIZE_MAX, framesize); + if (!ret) ret = m5mols_set_mode(info, REG_CAPTURE); if (!ret) /* Wait until a frame is captured to ISP internal memory */ diff --git a/drivers/media/i2c/m5mols/m5mols_core.c b/drivers/media/i2c/m5mols/m5mols_core.c index 933014f..c780689 100644 --- a/drivers/media/i2c/m5mols/m5mols_core.c +++ b/drivers/media/i2c/m5mols/m5mols_core.c @@ -599,6 +599,51 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, return ret; } +static int m5mols_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad, +struct v4l2_mbus_frame_desc *fd) +{ + struct m5mols_info *info = to_m5mols(sd); + + if (pad != 0 || fd == NULL) + return -EINVAL; + + mutex_lock(info-lock); + /* +* .get_frame_desc is only used for compressed formats, +* thus we always return the capture frame parameters here. +*/ + fd-entry[0].length = info-cap.buf_size; + fd-entry[0].pixelcode = info-ffmt[M5MOLS_RESTYPE_CAPTURE].code; + mutex_unlock(info-lock); + + fd-entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX; + fd-num_entries = 1; + + return 0; +} + +static int m5mols_set_frame_desc(struct v4l2_subdev *sd, unsigned int pad, +struct v4l2_mbus_frame_desc *fd) +{ + struct m5mols_info *info = to_m5mols(sd); + struct v4l2_mbus_framefmt *mf = info-ffmt[M5MOLS_RESTYPE_CAPTURE]; + + if (pad != 0 || fd == NULL) + return -EINVAL; + + fd-entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX; + fd-num_entries = 1; + fd-entry[0].length = clamp_t(u32, fd-entry[0].length, + mf-width * mf-height, + M5MOLS_MAIN_JPEG_SIZE_MAX); + mutex_lock(info-lock); + info-cap.buf_size = fd-entry[0].length; + mutex_unlock(info-lock); + + return 0; +} + + static int m5mols_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, struct v4l2_subdev_mbus_code_enum *code) @@ -615,6 +660,8 @@ static struct v4l2_subdev_pad_ops m5mols_pad_ops = { .enum_mbus_code = m5mols_enum_mbus_code, .get_fmt= m5mols_get_fmt, .set_fmt= m5mols_set_fmt, + .get_frame_desc = m5mols_get_frame_desc, + .set_frame_desc = m5mols_set_frame_desc, }; /** diff --git a/drivers/media/i2c/m5mols/m5mols_reg.h b/drivers/media/i2c/m5mols/m5mols_reg.h index 14d4be7..58d8027 100644 --- a/drivers/media/i2c/m5mols/m5mols_reg.h +++
Re: [PATCH 1/5] media: ov7670: add support for ov7675.
This is going to have to be quick, sorry... On Wed, 26 Sep 2012 11:47:53 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: +static struct ov7670_win_size ov7670_win_sizes[2][4] = { + /* ov7670 */ I must confess I don't like this; now we've got constants in an array that was automatically sized before and ov7670_win_sizes[info-model] everywhere. I'd suggest a separate array for each device and an ov7670_get_wsizes(model) function. + /* CIF - WARNING: not tested for ov7675 */ + { ...and this is part of why I don't like it. My experience with this particular sensor says that, if it's not tested, it hasn't yet seen the magic-number tweaking required to actually make it work. Please don't claim to support formats that you don't know actually work, or I'll get stuck with the bug reports :) + .width = CIF_WIDTH, + .height = CIF_HEIGHT, + .com7_bit = COM7_FMT_CIF, + .hstart = 170, /* Empirically determined */ + .hstop = 90, + .vstart = 14, + .vstop = 494, + .regs = NULL, + }, jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
On Wed, 26 Sep 2012 11:47:54 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: 'min_height' and 'min_width' are variables that allow to specify the minimum resolution that the sensor will achieve. This patch make v4l2 fmt callbacks consider this parameters in order to return valid data to user space. I'd have preferred to do this differently, perhaps backing toward larger sizes if the minimum turns out to be violated. But so be it... Acked-by: Jonathan Corbet cor...@lwn.net jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] media: ov7670: calculate framerate properly for ov7675.
On Wed, 26 Sep 2012 11:47:55 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: According to the datasheet ov7675 uses a formula to achieve the desired framerate that is different from the operations done in the current code. In fact, this formula should apply to ov7670 too. This would mean that current code is wrong but, in order to preserve compatibility, the new formula will be used for ov7675 only. At this point I couldn't tell you what the real situation is; it's been a while and there's always a fair amount of black magic involved with ov7670 configuration. I do appreciate attention to not breaking existing users. +static void ov7670_get_framerate(struct v4l2_subdev *sd, + struct v4l2_fract *tpf) This bugs me, though. It's called ov7670_get_framerate() but it's getting the rate for the ov7675 - confusing. Meanwhile the real ov7670 code remains inline while ov7675 has its own function. Please make two functions, one of which is ov7675_get_framerate(), and call the right one for the model. Same for the set functions, obviously. Maybe what's really needed is a structure full of sensor-specific operations? The get_wsizes() function could go there too. That would take a lot of if statements out of the code. + /* + * The datasheet claims that clkrc = 0 will divide the input clock by 1 + * but we've checked with an oscilloscope that it divides by 2 instead. + * So, if clkrc = 0 just bypass the divider. + */ Thanks for documenting this kind of thing. jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] media: ov7670: add possibility to bypass pll for ov7675.
On Wed, 26 Sep 2012 11:47:56 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: Signed-off-by: Javier Martin javier.mar...@vista-silicon.com This one needs a changelog - what does bypassing the PLL do and why might you want to do it? Otherwise: Acked-by: Jonathan Corbet cor...@lwn.net jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
On Wed, 26 Sep 2012 11:47:57 +0200 Javier Martin javier.mar...@vista-silicon.com wrote: Signed-off-by: Javier Martin javier.mar...@vista-silicon.com --- drivers/media/i2c/ov7670.c |8 include/media/ov7670.h |1 + 2 files changed, 9 insertions(+) Again, needs a changelog. Otherwise Acked-by: Jonathan Corbet cor...@lwn.net jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mmotm 2012-09-25-17-06 uploaded (media/tuners/fc2580)
On 09/25/2012 05:07 PM, a...@linux-foundation.org wrote: The mm-of-the-moment snapshot 2012-09-25-17-06 has been uploaded to http://www.ozlabs.org/~akpm/mmotm/ mmotm-readme.txt says README for mm-of-the-moment: http://www.ozlabs.org/~akpm/mmotm/ on i386: ERROR: __udivdi3 [drivers/media/tuners/fc2580.ko] undefined! ERROR: __umoddi3 [drivers/media/tuners/fc2580.ko] undefined! -- ~Randy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Wed Sep 26 19:00:22 CEST 2012 git hash:3f70e1f598a6be4277e71516a98457fd3bddfbd0 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-sh: ERRORS linux-git-x86_64: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: ERRORS linux-2.6.38.2-x86_64: ERRORS linux-2.6.39.1-x86_64: ERRORS linux-3.0-x86_64: ERRORS linux-3.1-x86_64: ERRORS linux-3.2.1-x86_64: ERRORS linux-3.3-x86_64: ERRORS linux-3.4-x86_64: ERRORS linux-3.5-x86_64: ERRORS linux-3.6-rc2-x86_64: ERRORS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: ERRORS linux-2.6.38.2-i686: ERRORS linux-2.6.39.1-i686: ERRORS linux-3.0-i686: ERRORS linux-3.1-i686: ERRORS linux-3.2.1-i686: ERRORS linux-3.3-i686: ERRORS linux-3.4-i686: ERRORS linux-3.5-i686: ERRORS linux-3.6-rc2-i686: ERRORS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mmotm 2012-09-25-17-06 uploaded (media/tuners/fc2580)
Il 26/09/2012 19:26, Randy Dunlap ha scritto: On 09/25/2012 05:07 PM, a...@linux-foundation.org wrote: The mm-of-the-moment snapshot 2012-09-25-17-06 has been uploaded to http://www.ozlabs.org/~akpm/mmotm/ mmotm-readme.txt says README for mm-of-the-moment: http://www.ozlabs.org/~akpm/mmotm/ on i386: ERROR: __udivdi3 [drivers/media/tuners/fc2580.ko] undefined! ERROR: __umoddi3 [drivers/media/tuners/fc2580.ko] undefined! Fixed here: http://patchwork.linuxtv.org/patch/14619/ Regards, Gianluca -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Timestamps and V4L2
Hi Laurent, Laurent Pinchart wrote: On Tuesday 25 September 2012 23:12:00 Sakari Ailus wrote: Hans Verkuil wrote: On Tue 25 September 2012 12:48:01 Laurent Pinchart wrote: On Tuesday 25 September 2012 08:47:45 Hans Verkuil wrote: On Tue September 25 2012 02:00:55 Laurent Pinchart wrote: BTW, I think we should also fix the description of the timestamp in the spec. Currently it says: For input streams this is the system time (as returned by the gettimeofday() function) when the first data byte was captured. For output streams the data will not be displayed before this time, secondary to the nominal frame rate determined by the current video standard in enqueued order. Applications can for example zero this field to display frames as soon as possible. The driver stores the time at which the first data byte was actually sent out in the timestamp field. This permits applications to monitor the drift between the video and system clock. To my knowledge all capture drivers set the timestamp to the time the *last* data byte was captured, not the first. The uvcvideo driver uses the time the first image packet is received :-) Most other drivers use the time the last byte was *received*, not captured. Unless the hardware buffers more than a few lines there is very little difference between the time the last byte was received and when it was captured. But you are correct, it is typically the time the last byte was received. Should we signal this as well? First vs last byte? Or shall we standardize? My personal opinion would be to change the spec to say what almost every driver does: it's the timestamp from the moment the last pixel has been received. We have the frame sync event for telling when the frame starts btw. The same event could be used for signalling whenever a given line starts. I don't see frame end fitting to that quite as nicely but I guess it could be possible. The uvcvideo driver can timestamp the buffers with the system time at which the first packet in the frame is received, but has no way to generate a frame start event: the frame start event should correspond to the time the frame starts, not to the time the first packet in the frame is received. That information isn't available to the driver. Aren't the two about equal, apart from the possible delays caused by the USB bus? The spec says about the frame sync event that it's Triggered immediately when the reception of a frame has begun. Cheers, -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi Laurent and Chris, Laurent Pinchart wrote: On Wednesday 26 September 2012 07:42:41 Chris MacGregor wrote: ... Sorry to make this more complicated, but the Aptina MT9P031, for instance (datasheet at http://www.aptina.com/assets/downloadDocument.do?id=865 - see page 35), has Digital Gain, an Analog Multiplier, and Analog Gain (for each of R, Gr, Gb, and B). For each color channel, there is one register, with the bits divided up into the three gain types. Furthermore, the different gain types have different units (increments). Currently (at least in the last version I've used), the driver hides all this and provides a single gain control, and prioritizes which gain types are adjusted at different user-level gain settings in accordance with the datasheet recommendations (e.g. keep the analog gain between 1 and 4 for best noise performance, and use the multiplier for gains between 4 and 8). This seems very sensible. I think it should be fine for now. If we later find out that a user space application really needs to control the analog and digital gains individually and precisely we can always split the controls then. For now I think a single gain control (per channel) that groups analog and digital gains should be enough. Agreed. I think there's much less reason to keep them separate for per-colour controls (compared to global gain), so little it's probably not worth it. Cheers, -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: davinci: vpif: add check for NULL handler
Em Thu, 16 Aug 2012 19:32:00 +0530 Prabhakar Lad prabhakar@ti.com escreveu: It is amazing how many SOB's/acks are in this patch and nobody asked you to provide a patch description... the subject just tells what the code is also telling. Could you please provide a better patch description? Thanks! Mauro From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com --- drivers/media/video/davinci/vpif_capture.c | 12 +++- drivers/media/video/davinci/vpif_display.c | 14 -- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c index 266025e..a87b7a5 100644 --- a/drivers/media/video/davinci/vpif_capture.c +++ b/drivers/media/video/davinci/vpif_capture.c @@ -311,12 +311,14 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } /* configure 1 or 2 channel mode */ - ret = vpif_config_data-setup_input_channel_mode - (vpif-std_info.ycmux_mode); + if (vpif_config_data-setup_input_channel_mode) { + ret = vpif_config_data-setup_input_channel_mode + (vpif-std_info.ycmux_mode); - if (ret 0) { - vpif_dbg(1, debug, can't set vpif channel mode\n); - return ret; + if (ret 0) { + vpif_dbg(1, debug, can't set vpif channel mode\n); + return ret; + } } /* Call vpif_set_params function to set the parameters and addresses */ diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c index e129c98..1e35f92 100644 --- a/drivers/media/video/davinci/vpif_display.c +++ b/drivers/media/video/davinci/vpif_display.c @@ -280,12 +280,14 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } /* clock settings */ - ret = - vpif_config_data-set_clock(ch-vpifparams.std_info.ycmux_mode, - ch-vpifparams.std_info.hd_sd); - if (ret 0) { - vpif_err(can't set clock\n); - return ret; + if (vpif_config_data-set_clock) { + ret = + vpif_config_data-set_clock(ch-vpifparams.std_info.ycmux_mode, + ch-vpifparams.std_info.hd_sd); + if (ret 0) { + vpif_err(can't set clock\n); + return ret; + } } /* set the parameters and addresses */ Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params
Em Mon, 13 Aug 2012 19:43:29 +0530 Manjunath Hadli manjunath.ha...@ti.com escreveu: Hi Dror, Thanks for the patch. Mauro, I'll queue this patch for v3.7 through my tree. Sure. On Thursday 09 August 2012 12:03 PM, Dror Cohen wrote: This patch address the issue that a function named config_vpif_params should be vpif_config_params. However this name is shared with two structures defined already. So I changed the structures to config_vpif_params (origin was vpif_config_params) v2 changes: softer wording in description and the structs are now defined without _t Hmm... I didn't understand what you're wanting with this change. Before this patch, there are: v4l@pedra ~/v4l/patchwork $ git grep config_vpif_params drivers/media/platform/davinci/vpif.c:/* config_vpif_params drivers/media/platform/davinci/vpif.c:static void config_vpif_params(struct vpif_params *vpifparams, drivers/media/platform/davinci/vpif.c:config_vpif_params(vpifparams, channel_id, found); v4l@pedra ~/v4l/patchwork $ git grep vpif_config_params drivers/media/platform/davinci/vpif_capture.c:static struct vpif_config_params config_params = { drivers/media/platform/davinci/vpif_capture.h:struct vpif_config_params { drivers/media/platform/davinci/vpif_display.c:static struct vpif_config_params config_params = { drivers/media/platform/davinci/vpif_display.h:struct vpif_config_params { After that, there are: v4l@pedra ~/v4l/patchwork $ git grep vpif_config_params drivers/media/platform/davinci/vpif.c:/* vpif_config_params drivers/media/platform/davinci/vpif.c:static void vpif_config_params(struct vpif_params *vpifparams, drivers/media/platform/davinci/vpif.c:vpif_config_params(vpifparams, channel_id, found); v4l@pedra ~/v4l/patchwork $ git grep config_vpif_params drivers/media/platform/davinci/vpif_capture.c:static struct config_vpif_params config_params = { drivers/media/platform/davinci/vpif_capture.h:struct config_vpif_params { drivers/media/platform/davinci/vpif_display.c:static struct config_vpif_params config_params = { drivers/media/platform/davinci/vpif_display.h:struct config_vpif_params { So, I can't really see any improvement on avoiding duplicate names. IMHO, the better would be to name those functions as: vpif: vpif_config_params (or, even better, vpif_core_config_params) vpif_capture: vpif_capture_config_params vpif_display: vpif_display_config_params This way, duplication will be avoided and will avoid the confusing inversion between vpif and config. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC, updated] Fix DVB ioctls failing if frontend open/closed too fast
On Wed, Aug 01, 2012 at 12:22:16AM +0200, Juergen Lock wrote: That likely fixes this MythTV ticket: http://code.mythtv.org/trac/ticket/10830 (which btw affects all usb tuners I tested as well, pctv452e, dib0700, af9015) pctv452e is still possibly broken with MythTV even after this fix; it does work with VDR here tho despite I2C errors. Reduced testcase: http://people.freebsd.org/~nox/tmp/ioctltst.c Thanx to devinheitmueller and crope from #linuxtv for helping with this fix! :) Updated patch for media tree for_3.7: Signed-off-by: Juergen Lock n...@jelal.kn-bremen.de --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -603,6 +603,7 @@ static int dvb_frontend_thread(void *dat enum dvbfe_algo algo; bool re_tune = false; + bool semheld = false; dev_dbg(fe-dvb-device, %s:\n, __func__); @@ -626,6 +627,8 @@ restart: if (kthread_should_stop() || dvb_frontend_is_exiting(fe)) { /* got signal or quitting */ + if (!down_interruptible (fepriv-sem)) + semheld = true; fepriv-exit = DVB_FE_NORMAL_EXIT; break; } @@ -741,6 +744,8 @@ restart: fepriv-exit = DVB_FE_NO_EXIT; mb(); + if (semheld) + up(fepriv-sem); dvb_frontend_wakeup(fe); return 0; } @@ -1819,16 +1824,20 @@ static int dvb_frontend_ioctl(struct fil int err = -ENOTTY; dev_dbg(fe-dvb-device, %s: (%d)\n, __func__, _IOC_NR(cmd)); - if (fepriv-exit != DVB_FE_NO_EXIT) + if (down_interruptible (fepriv-sem)) + return -ERESTARTSYS; + + if (fepriv-exit != DVB_FE_NO_EXIT) { + up(fepriv-sem); return -ENODEV; + } if ((file-f_flags O_ACCMODE) == O_RDONLY (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT || -cmd == FE_DISEQC_RECV_SLAVE_REPLY)) +cmd == FE_DISEQC_RECV_SLAVE_REPLY)) { + up(fepriv-sem); return -EPERM; - - if (down_interruptible (fepriv-sem)) - return -ERESTARTSYS; + } if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)) err = dvb_frontend_ioctl_properties(file, cmd, parg); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hang in Technotrend TT-connect CT-3650
Hi all, I'm new to the list so feel free to direct me somewhere else if the following is OT. I get a hard lock on my machine, no response to Alt-Sysrq T, dead to the world, when running the mentioned card with a Conax cam to record encrypted DVB-T channels. I have had the thing running through a night and recording a couple of shows, but mostly it locks up within seconds from the start of recording, with no pattern that I can see I am running gentoo, and I have tried both the latest stable kernel with its modules, the latest unstable kernel with its modules and the modules from git as per http://linuxtv.org/wiki/index.php/How_to_Obtain,_Build_and_Install_V4L-DVB_Device_Drivers, the Basic approach. My current kernel is: # uname -a Linux medisin 3.5.4-gentoo #1 PREEMPT Wed Sep 26 14:23:09 CEST 2012 i686 Intel(R) Pentium(R) 4 CPU 2.00GHz GenuineIntel GNU/Linux Should I maybe loose the preempt ? The last messages I get in my logs is mythbackend: TVRecEvent tv_rec.cpp:3989 (TuningNewRecorder) - TVRec(7): rec-GetPathname(): '/apub/media/mythtv/2201_20120926203000.mpg' kernel: [ 98.144753] ttusb2: tt3650_ci_read_cam_control 0x01 - 0 0x00 debug: medisin kernel: [ 98.144753] ttusb2: tt3650_ci_read_cam_control 0x01 - 0 0x00 Everything up to that point seems hunky dory, such as 30 seconds earlier : [ 54.232760] ttusb2: tt3650_ci_init [ 59.244586] ttusb2: tt3650_ci_slot_reset 0 [ 61.447592] ttusb2: tt3650_ci_read_attribute_mem - 0 0x00 [ 61.447842] ttusb2: tt3650_ci_read_attribute_mem 0002 - 0 0x00 [ 71.045833] ttusb2: tt3650_ci_set_video_port 0 0 [ 71.047997] ttusb2: tt3650_ci_slot_reset 0 [ 73.156515] ttusb2: tt3650_ci_read_attribute_mem - 0 0x1d [ 73.156765] ttusb2: tt3650_ci_read_attribute_mem 0002 - 0 0x04 [ 71.045833] ttusb2: tt3650_ci_set_video_port 0 0 [ 71.047997] ttusb2: tt3650_ci_slot_reset 0 and these go on ... I have miles of logs, just tell me what to post :-) Other interesting points about the system, in no particular order the box is a fairly old system, which has had some issues. I have repaced some popped caps, I believe I found them all. I have removed the usb flat-bed scanner so the draw on power should be /less/ than before, considering the Technotrend has its own power. I have made sure that lsusb says usb 2.0 on the bus. What else should I check ? Whith hope, Håkon :-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v8] media: add v4l2 subdev driver for S5K4ECGX sensor
Em Thu, 13 Sep 2012 12:02:14 +0100 Sangwook Lee sangwook@linaro.org escreveu: This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Lee sangwook@linaro.org Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com Cc: Francesco Lavra francescolavra...@gmail.com Cc: Scott Bambrough scott.bambro...@linaro.org Cc: Homin Lee suap...@insignal.co.kr ... +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ + const struct firmware *fw; + const u8 *ptr; + int err, i, regs_num; + struct i2c_client *client = v4l2_get_subdevdata(sd); + u16 val; + u32 addr, crc, crc_file, addr_inc = 0; + + err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); The patch looks correct on my eyes... Yet, calling request_firmware() might not be the right thing to do. The thing is that newer versions of udev refuse to load firmware synchronously during probe/init time. As this function is actually called by s_power, maybe this driver doesn't suffer from that new udev behavior, so, I'll be merging it as-is. However, I suggest you to take a deeper review on that and, if possible, test it with the latest udev. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] V4L: Add s_rx_buffer subdev video operation
Hi Sylwester, Sylwester Nawrocki wrote: On 09/24/2012 08:26 PM, Sakari Ailus wrote: On Mon, Sep 24, 2012 at 06:51:41PM +0200, Sylwester Nawrocki wrote: On 09/24/2012 03:44 PM, Sakari Ailus wrote: How about useing a separate video buffer queue for the purpose? That would provide a nice way to pass it to the user space where it's needed. It'd also play nicely together with the frame layout descriptors. It's tempting, but doing frame synchronisation in user space in this case would have been painful, if at all possible in reliable manner. It would have significantly complicate applications and the drivers. Let's face it: applications that are interested in this information have to do exactly the same frame number matching with the statistics buffers. Just stitching the data to the same video buffer isn't a generic solution. Let me list disadvantages of using separate buffer queue: 1. significant complication of the driver: - need to add video node support with all it's memory and file ops, That's not much of code since the driver already does it once. - more complicated VIDIOC_STREAMON logic, MIPI CSI receiver needs to care about the data pipeline details (power, streaming,..); Power management complication is about non-existent, streaming likely require ensuring that STREAMON IOCTL has been issued on both before streaming is really started. 2. more processing overhead due second /dev/video handling; True. 3. much more complex device handling in user space. Somewhat, yes. All this for virtually nothing but 2 x 4-byte integers we are interested in in the Embedded Data stream. And advantages: 1. More generic solution, no need to invent new fourcc's for standard image data formats with metadata (new fourcc is needed anyway for the device- specific image data (JPEG/YUV/.../YUV/JPEG/meta-data, and we can choose to use multi-planar only for non-standard formats and separate meta-data buffer queue for others); 2. Probably other host IF/ISP drivers would implement it this way, or would they ? 3. What else could be added ? One more advantage is that you'll get the metadata immediately to the user space after it's been written to the system memory; no need to wait for the rest of the frame. That may be interesting sometimes. Currently I don't see justification for using separate video node as the frame embedded frame grabber. I don't expect it to be useful for us in future, not for the ISPs that process sensor data separately from the host CPUs. Moreover, this MIPI-CSIS device has maximum buffer size of only 4 KiB, which effectively limits its possible use cases. That's your hardware. Most CSI-2 receivers I've seen either write the metadata to the same buffer or to a different memory area equivalent to the video buffer where the image is stored. I don't think there is a need to force host drivers to use either separate buffer queues or multi-planar APIs. Especially in case of non-standard hybrid data formats. I'm ready to discuss separate buffer queue approach if we have real use case for it. I don't think these two methods are exclusive. I agree with that. It should be made possible for the user to decide which one to use. The hardware may also limit possibilities since if it just recognises a single buffer, there's not much that the software can do in that case. Multi-plane buffers are the only option in that case I guess. Until then I would prefer not to live with an awkward solution. I think it would be good to be able to have plane-specific formats on multi-plane buffers. Thus we wouldn't end up having a new pixel format out of every metadata / image format pair. And there will be many, many of those and the applications certainly don't want to care about the combinations themselves. VIDIOC_STREAMON, VIDIOC_QBUF/DQBUF calls would have been at least roughly synchronized, and applications would have to know somehow which video nodes needs to be opened together. I guess things like that could be abstracted in a library, but what do we really gain for such effort ? And now I can just ask kernel for 2-planar buffers where everything is in place.. That's equally good --- some hardware can only do that after all, but do you need the callback in that case, if there's a single destination buffer anyway? Wouldn't the frame layout descriptor have enough information to do this? There is as many buffers as user requested with REQBUFS. In VSYNC interrupt I meant separately allocated and mapped memory areas related to a single frame. of one device there is a buffer configured for the other device. With each frame interrupt there is a different buffer used, the one that the DMA engine actually writes data to. Data copying happens from the MIPI-CSIS internal ioremapped buffer to a buffer owned by the host interface driver. And the callback is used for dynamically switching buffers at the subdev. So... your CSI-2
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Wed Sep 26 20:33:55 CEST 2012 git hash:3f70e1f598a6be4277e71516a98457fd3bddfbd0 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-sh: ERRORS linux-git-x86_64: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-rc2-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-rc2-i686: WARNINGS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] OMAP 3 CSI-2 configuration
Hi Paul and Laurent, This is an update to an old patchset for CSI-2 configuration for OMAP 3430 and 3630, and the corresponding patch to the ISP driver. Both have been tested on the 3630 only so far. Additional patches for the N9(50) camera support that mostly aren't yet upstreamable, are available in rm696-016/002-devel branch here: URL:https://git.gitorious.org/omap3camera/mainline.git Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data
Configure CSI-2 phy based on platform data in the ISP driver. For that, the new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same was configured from the board code. This patch is dependent on omap3: Provide means for changing CSI2 PHY configuration. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/platform/omap3isp/isp.h |3 - drivers/media/platform/omap3isp/ispcsiphy.c | 161 +++ drivers/media/platform/omap3isp/ispcsiphy.h | 10 -- 3 files changed, 90 insertions(+), 84 deletions(-) diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h index 8be7487..a2f992c 100644 --- a/drivers/media/platform/omap3isp/isp.h +++ b/drivers/media/platform/omap3isp/isp.h @@ -127,9 +127,6 @@ struct isp_reg { struct isp_platform_callback { u32 (*set_xclk)(struct isp_device *isp, u32 xclk, u8 xclksel); - int (*csiphy_config)(struct isp_csiphy *phy, -struct isp_csiphy_dphy_cfg *dphy, -struct isp_csiphy_lanes_cfg *lanes); }; /* diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c index 348f67e..1d16e66 100644 --- a/drivers/media/platform/omap3isp/ispcsiphy.c +++ b/drivers/media/platform/omap3isp/ispcsiphy.c @@ -28,41 +28,13 @@ #include linux/device.h #include linux/regulator/consumer.h +#include mach/control.h + #include isp.h #include ispreg.h #include ispcsiphy.h /* - * csiphy_lanes_config - Configuration of CSIPHY lanes. - * - * Updates HW configuration. - * Called with phy-mutex taken. - */ -static void csiphy_lanes_config(struct isp_csiphy *phy) -{ - unsigned int i; - u32 reg; - - reg = isp_reg_readl(phy-isp, phy-cfg_regs, ISPCSI2_PHY_CFG); - - for (i = 0; i phy-num_data_lanes; i++) { - reg = ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) | -ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1)); - reg |= (phy-lanes.data[i].pol - ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1)); - reg |= (phy-lanes.data[i].pos - ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1)); - } - - reg = ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK | -ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK); - reg |= phy-lanes.clk.pol ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT; - reg |= phy-lanes.clk.pos ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT; - - isp_reg_writel(phy-isp, reg, phy-cfg_regs, ISPCSI2_PHY_CFG); -} - -/* * csiphy_power_autoswitch_enable * @enable: Sets or clears the autoswitch function enable flag. */ @@ -107,46 +79,32 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32 power) } /* - * csiphy_dphy_config - Configure CSI2 D-PHY parameters. - * - * Called with phy-mutex taken. + * TCLK values are OK at their reset values */ -static void csiphy_dphy_config(struct isp_csiphy *phy) -{ - u32 reg; - - /* Set up ISPCSIPHY_REG0 */ - reg = isp_reg_readl(phy-isp, phy-phy_regs, ISPCSIPHY_REG0); - - reg = ~(ISPCSIPHY_REG0_THS_TERM_MASK | -ISPCSIPHY_REG0_THS_SETTLE_MASK); - reg |= phy-dphy.ths_term ISPCSIPHY_REG0_THS_TERM_SHIFT; - reg |= phy-dphy.ths_settle ISPCSIPHY_REG0_THS_SETTLE_SHIFT; - - isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG0); - - /* Set up ISPCSIPHY_REG1 */ - reg = isp_reg_readl(phy-isp, phy-phy_regs, ISPCSIPHY_REG1); - - reg = ~(ISPCSIPHY_REG1_TCLK_TERM_MASK | -ISPCSIPHY_REG1_TCLK_MISS_MASK | -ISPCSIPHY_REG1_TCLK_SETTLE_MASK); - reg |= phy-dphy.tclk_term ISPCSIPHY_REG1_TCLK_TERM_SHIFT; - reg |= phy-dphy.tclk_miss ISPCSIPHY_REG1_TCLK_MISS_SHIFT; - reg |= phy-dphy.tclk_settle ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT; - - isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG1); -} +#define TCLK_TERM 0 +#define TCLK_MISS 1 +#define TCLK_SETTLE14 -static int csiphy_config(struct isp_csiphy *phy, -struct isp_csiphy_dphy_cfg *dphy, -struct isp_csiphy_lanes_cfg *lanes) +static int omap3isp_csiphy_config(struct isp_csiphy *phy) { + struct isp_csi2_device *csi2 = phy-csi2; + struct isp_pipeline *pipe = to_isp_pipeline(csi2-subdev.entity); + struct isp_v4l2_subdevs_group *subdevs = pipe-external-host_priv; + struct isp_csiphy_lanes_cfg *lanes; + int csi2_ddrclk_khz; unsigned int used_lanes = 0; unsigned int i; + unsigned int phy_num; + u32 reg; + + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1 + || subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + lanes = subdevs-bus.ccp2.lanecfg; + else + lanes = subdevs-bus.csi2.lanecfg; /* Clock and data lanes verification */ - for (i = 0; i
[PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration
The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to the actual CSI-2 receivers outside the ISP itself. Allow changing this configuration from the ISP driver. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- arch/arm/mach-omap2/control.c | 86 arch/arm/mach-omap2/control.h | 15 + arch/arm/mach-omap2/include/mach/control.h | 13 3 files changed, 114 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-omap2/include/mach/control.h diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c index 3223b81..11bb900 100644 --- a/arch/arm/mach-omap2/control.c +++ b/arch/arm/mach-omap2/control.c @@ -12,9 +12,12 @@ */ #undef DEBUG +#include linux/export.h #include linux/kernel.h #include linux/io.h +#include mach/control.h + #include plat/hardware.h #include plat/sdrc.h @@ -607,4 +610,87 @@ int omap3_ctrl_save_padconf(void) return 0; } +static int omap3630_ctrl_csi2_phy_cfg(u32 phy, u32 flags) +{ + u32 cam_phy_ctrl = + omap_ctrl_readl(OMAP3630_CONTROL_CAMERA_PHY_CTRL); + u32 shift, mode; + + switch (phy) { + case OMAP3_CTRL_CSI2_PHY1_CCP2B: + cam_phy_ctrl = ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2; + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT; + break; + case OMAP3_CTRL_CSI2_PHY1_CSI2C: + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT; + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY; + break; + case OMAP3_CTRL_CSI2_PHY2_CCP2B: + cam_phy_ctrl |= OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2; + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT; + break; + case OMAP3_CTRL_CSI2_PHY2_CSI2A: + shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT; + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY; + break; + default: + pr_warn(bad phy %d\n, phy); + return -EINVAL; + } + + /* Select data/clock or data/strobe mode for CCP2 */ + switch (phy) { + case OMAP3_CTRL_CSI2_PHY1_CCP2B: + case OMAP3_CTRL_CSI2_PHY2_CCP2B: + if (flags OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE) + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE; + else + mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK; + break; + } + + cam_phy_ctrl = ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK shift); + cam_phy_ctrl |= mode shift; + + omap_ctrl_writel(cam_phy_ctrl, +OMAP3630_CONTROL_CAMERA_PHY_CTRL); + + return 0; +} + +static int omap3430_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags) +{ + uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ + | OMAP343X_CONTROL_CSIRXFE_RESET; + + /* Nothing to configure here. */ + if (phy == OMAP3_CTRL_CSI2_PHY2_CSI2A) + return 0; + + if (phy != OMAP3_CTRL_CSI2_PHY1_CCP2B) + return -EINVAL; + + if (!on) { + omap_ctrl_writel(0, OMAP343X_CONTROL_CSIRXFE); + return 0; + } + + if (flags OMAP3_CTRL_CSI2_CFG_CCP2_DATA_STROBE) + csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM; + + omap_ctrl_writel(csirxfe, OMAP343X_CONTROL_CSIRXFE); + + return 0; +} + +int omap3_ctrl_csi2_phy_cfg(u32 phy, bool on, u32 flags) +{ + if (cpu_is_omap3630() on) + return omap3630_ctrl_csi2_phy_cfg(phy, flags); + if (cpu_is_omap3430()) + return omap3430_ctrl_csi2_phy_cfg(phy, on, flags); + return 0; +} +EXPORT_SYMBOL_GPL(omap3_ctrl_csi2_phy_cfg); + #endif /* CONFIG_ARCH_OMAP3 CONFIG_PM */ diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h index b8cdc85..7b2ee5d 100644 --- a/arch/arm/mach-omap2/control.h +++ b/arch/arm/mach-omap2/control.h @@ -132,6 +132,11 @@ #define OMAP343X_CONTROL_MEM_DFTRW1(OMAP2_CONTROL_GENERAL + 0x000c) #define OMAP343X_CONTROL_DEVCONF1 (OMAP2_CONTROL_GENERAL + 0x0068) #define OMAP343X_CONTROL_CSIRXFE (OMAP2_CONTROL_GENERAL + 0x006c) +#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV (1 7) +#define OMAP343X_CONTROL_CSIRXFE_RESENABLE (1 8) +#define OMAP343X_CONTROL_CSIRXFE_SELFORM (1 10) +#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ(1 12) +#define OMAP343X_CONTROL_CSIRXFE_RESET (1 13) #define OMAP343X_CONTROL_SEC_STATUS(OMAP2_CONTROL_GENERAL + 0x0070) #define OMAP343X_CONTROL_SEC_ERR_STATUS(OMAP2_CONTROL_GENERAL + 0x0074) #define OMAP343X_CONTROL_SEC_ERR_STATUS_DEBUG (OMAP2_CONTROL_GENERAL + 0x0078) @@ -189,6 +194,16 @@ #define OMAP3630_CONTROL_FUSE_OPP50_VDD2
Re: [RFC PATCH v8] media: add v4l2 subdev driver for S5K4ECGX sensor
On 09/26/2012 10:32 PM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 12:02:14 +0100 Sangwook Leesangwook@linaro.org escreveu: This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Leesangwook@linaro.org Reviewed-by: Sylwester Nawrockis.nawro...@samsung.com Cc: Francesco Lavrafrancescolavra...@gmail.com Cc: Scott Bambroughscott.bambro...@linaro.org Cc: Homin Leesuap...@insignal.co.kr ... +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ +const struct firmware *fw; +const u8 *ptr; +int err, i, regs_num; +struct i2c_client *client = v4l2_get_subdevdata(sd); +u16 val; +u32 addr, crc, crc_file, addr_inc = 0; + +err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); The patch looks correct on my eyes... Yet, calling request_firmware() might not be the right thing to do. The thing is that newer versions of udev refuse to load firmware synchronously during probe/init time. As this function is actually called by s_power, maybe this driver doesn't suffer from that new udev behavior, so, I'll be merging it as-is. However, I suggest you to take a deeper review on that and, if possible, test it with the latest udev. True, it's indeed a bit tricky. The host interface driver this sensor driver has been tested with calls s_power on a subdev only in response to a video device open(). During probe only subdev's .registered() callback is called. And there is no request_firmware() there, as it is not needed to boot the sensor's MCU. The firmware is really just a set of settings that are normally needed only before streaming needs to be started. We have analysed this issue and hopefully there should be no request_firmware() calls from within probe(), as long as the host interface driver doesn't call s_power from it's probe() callback. I'm afraid it's not the case for all bridge drivers though. If needed we could probably try and use request_firmware_nowait(), but it seems better to me to make sure the bridge drivers don't call s_power from within their probe() callback. -- Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] omap3isp: Configure CSI-2 phy based on platform data
Moi Sakari * Sakari Ailus sakari.ai...@iki.fi [120926 14:51]: Configure CSI-2 phy based on platform data in the ISP driver. For that, the new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same was configured from the board code. This patch is dependent on omap3: Provide means for changing CSI2 PHY configuration. Can you please do one more patch to get rid of the last remaining cpu_is_omap check in drivers/media/platform/omap3isp/isp.c? That data should come from platform_data (or device tree) as we going to make cpu_is_omap privat to mach-omap2. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
qv4l2-bug / libv4lconvert API issue
Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? Regards, Frank -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Timestamps and V4L2
Hi Laurent, On 09/25/2012 02:35 AM, Laurent Pinchart wrote: Does the clock type need to be selectable for mem-to-mem devices ? Do device- specific timestamps make sense there ? I'd like to clarify one thing here, i.e. if we select device-specific timestamps how should the v4l2_buffer::timestamp field behave ? Are these two things exclusive ? Or should v4l2_buffer::timestamp be valid even if device-specific timestamps are enabled ? With regards to your question, I think device-specific timestamps make sense for mem-to-mem devices. Maybe not for the very simple ones, that process buffers 1-to-1, but codecs may need it. I was told the Exynos/ S5P Multi Format Codec device has some register the timestamps could be read from, but it's currently not used by the s5p-mfc driver. Kamil might provide more details on that. I guess if capture and output devices can have their timestamping clocks selectable it should be also possible for mem-to-mem devices. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
HD-PVR fails consistently on Linux, works on Windows
I recently purchased a Hauppauge HD-PVR (the 1212 version, label on bottom 49001LF, Rev F2). I have consistent capture failures on Linux where data from the device simply stops, generally within a few minutes of starting a capture. Yet, the device works flawlessly on Windows with the same USB and component cables, same power supply, and same physical position. This suggests that the device itself has acceptable power, is not overheating, etc. I'll detail below the testing I've done thus far and would appreciate any suggestions on how to further test or address the problem. The good news is that I have a highly reproducible failure on Linux, but then that's the bad news too. Thanks. Keith -- Linux tests -- I started trying to use the HD-PVR directly with my MythTV backend. I have subsequently switched all of my testing to simple direct captures from the /dev/video? device using /bin/cat to eliminate as many variables as possible. I've done a large number of tests with combinations of the following: OS: gentoo 3.4.7, gentoo 3.5.4 HD-PVR firmware: 1.5.7.0 (0x15), 1.7.1.30059 (0x1e) Input resolution: fixed to 720p, fixed to 1080i, floating based on input USB ports: motherboard ports on Intel DP45SG, motherboard ports on MSI X58 Pro-E, ports on SIIG USB PCIe card Captures fail consistently. I've verified that the HD-PVR is the only device on the USB bus and that the bus shows as Linux Foundation 2.0 root hub in all tests. I've increased the debug output level for the hdpvr driver to 6 (/sys/module/hdpvr/parameters/hdpvr_debug) and collected the following: Sep 21 17:01:00 mythbe kernel: [535043.504450] usb 9-1: New USB device found, idVendor=2040, idProduct=4903 Sep 21 17:01:00 mythbe kernel: [535043.504453] usb 9-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Sep 21 17:01:00 mythbe kernel: [535043.504456] usb 9-1: Product: Hauppauge HD PVR Sep 21 17:01:00 mythbe kernel: [535043.504458] usb 9-1: Manufacturer: AMBA Sep 21 17:01:00 mythbe kernel: [535043.504459] usb 9-1: SerialNumber: 00A6DD48 Sep 21 17:01:00 mythbe kernel: [535043.504523] usb 9-1: ep 0x1 - rounding interval to 32768 microframes, ep desc says 0 microframes Sep 21 17:01:00 mythbe kernel: [535043.504528] usb 9-1: ep 0x81 - rounding interval to 32768 microframes, ep desc says 0 microframes Sep 21 17:01:01 mythbe kernel: [535043.703947] hdpvr 9-1:1.0: firmware version 0x15 dated Jun 17 2010 09:26:53 Sep 21 17:01:01 mythbe kernel: [535043.889144] IR keymap rc-hauppauge not found Sep 21 17:01:01 mythbe kernel: [535043.889146] Registered IR keymap rc-empty Sep 21 17:01:01 mythbe kernel: [535043.889190] input: i2c IR (HD-PVR) as /devices/virtual/rc/rc5/input16 Sep 21 17:01:01 mythbe kernel: [535043.889415] rc5: i2c IR (HD-PVR) as /devices/virtual/rc/rc5 Sep 21 17:01:01 mythbe kernel: [535043.889417] ir-kbd-i2c: i2c IR (HD-PVR) detected at i2c-8/8-0071/ir0 [Hauppage HD PVR I2C] Sep 21 17:01:01 mythbe kernel: [535043.889518] hdpvr 9-1:1.0: device now attached to video6 Sep 21 17:01:01 mythbe kernel: [535043.889534] usbcore: registered new interface driver hdpvr Sep 21 17:05:11 mythbe kernel: [535293.776318] hdpvr 9-1:1.0: video signal: 1920x1080@30hz Sep 21 17:05:14 mythbe kernel: [535297.312589] hdpvr 9-1:1.0: encoder start control request returned 0 Sep 21 17:05:15 mythbe kernel: [535297.670830] hdpvr 9-1:1.0: config call request for value 0x700 returned 1 Sep 21 17:05:15 mythbe kernel: [535297.670833] hdpvr 9-1:1.0: streaming started Sep 21 17:05:15 mythbe kernel: [535297.670839] hdpvr 9-1:1.0: hdpvr_read:442 buffer stat: 64 free, 0 proc Sep 21 17:05:15 mythbe kernel: [535297.670882] hdpvr 9-1:1.0: hdpvr_submit_buffers:209 buffer stat: 0 free, 64 proc Sep 21 17:05:15 mythbe kernel: [535297.709079] hdpvr 9-1:1.0: hdpvr_read:502 buffer stat: 1 free, 63 proc Sep 21 17:05:15 mythbe kernel: [535297.709088] hdpvr 9-1:1.0: hdpvr_submit_buffers:209 buffer stat: 0 free, 64 proc (many repeats of the above two line sequence) Sep 21 17:17:09 mythbe kernel: [536011.936858] hdpvr 9-1:1.0: hdpvr_read:502 buffer stat: 1 free, 63 proc Sep 21 17:17:09 mythbe kernel: [536011.936866] hdpvr 9-1:1.0: hdpvr_submit_buffers:209 buffer stat: 0 free, 64 proc Sep 21 17:17:36 mythbe kernel: [536038.853044] hdpvr 9-1:1.0: config call request for value 0x800 returned -110 Sep 21 17:17:36 mythbe kernel: [536038.853052] hdpvr 9-1:1.0: transmit worker exited Sep 21 17:17:36 mythbe kernel: [536038.996035] hdpvr 9-1:1.0: used 0 urbs to empty device buffers If I understand correctly, this is showing a ETIMEDOUT error. When I've looked at the cat with strace, it is always blocked on a read. So, it seems like the HD-PVR just stops sending. I also ran a USB capture with wireshark and see much the same thing. While I haven't tried to decode the USB packets, the pattern is that the HD-PVR sends, the host sends a message/ack, this pattern repeats, and then nothing. The majority of the failures occur in less
Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params
Hi Dror, On Thu, Sep 27, 2012 at 1:50 AM, Mauro Carvalho Chehab mche...@infradead.org wrote: Em Mon, 13 Aug 2012 19:43:29 +0530 Manjunath Hadli manjunath.ha...@ti.com escreveu: Hi Dror, Thanks for the patch. Mauro, I'll queue this patch for v3.7 through my tree. Sure. On Thursday 09 August 2012 12:03 PM, Dror Cohen wrote: This patch address the issue that a function named config_vpif_params should be vpif_config_params. However this name is shared with two structures defined already. So I changed the structures to config_vpif_params (origin was vpif_config_params) v2 changes: softer wording in description and the structs are now defined without _t Hmm... I didn't understand what you're wanting with this change. Before this patch, there are: v4l@pedra ~/v4l/patchwork $ git grep config_vpif_params drivers/media/platform/davinci/vpif.c:/* config_vpif_params drivers/media/platform/davinci/vpif.c:static void config_vpif_params(struct vpif_params *vpifparams, drivers/media/platform/davinci/vpif.c:config_vpif_params(vpifparams, channel_id, found); v4l@pedra ~/v4l/patchwork $ git grep vpif_config_params drivers/media/platform/davinci/vpif_capture.c:static struct vpif_config_params config_params = { drivers/media/platform/davinci/vpif_capture.h:struct vpif_config_params { drivers/media/platform/davinci/vpif_display.c:static struct vpif_config_params config_params = { drivers/media/platform/davinci/vpif_display.h:struct vpif_config_params { After that, there are: v4l@pedra ~/v4l/patchwork $ git grep vpif_config_params drivers/media/platform/davinci/vpif.c:/* vpif_config_params drivers/media/platform/davinci/vpif.c:static void vpif_config_params(struct vpif_params *vpifparams, drivers/media/platform/davinci/vpif.c:vpif_config_params(vpifparams, channel_id, found); v4l@pedra ~/v4l/patchwork $ git grep config_vpif_params drivers/media/platform/davinci/vpif_capture.c:static struct config_vpif_params config_params = { drivers/media/platform/davinci/vpif_capture.h:struct config_vpif_params { drivers/media/platform/davinci/vpif_display.c:static struct config_vpif_params config_params = { drivers/media/platform/davinci/vpif_display.h:struct config_vpif_params { So, I can't really see any improvement on avoiding duplicate names. IMHO, the better would be to name those functions as: vpif: vpif_config_params (or, even better, vpif_core_config_params) vpif_capture: vpif_capture_config_params vpif_display: vpif_display_config_params This way, duplication will be avoided and will avoid the confusing inversion between vpif and config. I agree with Mauro here, Can you do the above changes and post a v2.( Rebase it on http://git.linuxtv.org/media_tree.git/shortlog/refs/heads/staging/for_v3.7 branch) Regards, --Prabhakar Lad Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: davinci: vpif: add check for NULL handler
Hi Mauro, On Thu, Sep 27, 2012 at 1:37 AM, Mauro Carvalho Chehab mche...@infradead.org wrote: Em Thu, 16 Aug 2012 19:32:00 +0530 Prabhakar Lad prabhakar@ti.com escreveu: It is amazing how many SOB's/acks are in this patch and nobody asked you to provide a patch description... the subject just tells what the code is also telling. Could you please provide a better patch description? My bad I'll post a v2. Regards, --Prabhakar Lad Thanks! Mauro From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com --- drivers/media/video/davinci/vpif_capture.c | 12 +++- drivers/media/video/davinci/vpif_display.c | 14 -- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c index 266025e..a87b7a5 100644 --- a/drivers/media/video/davinci/vpif_capture.c +++ b/drivers/media/video/davinci/vpif_capture.c @@ -311,12 +311,14 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } /* configure 1 or 2 channel mode */ - ret = vpif_config_data-setup_input_channel_mode - (vpif-std_info.ycmux_mode); + if (vpif_config_data-setup_input_channel_mode) { + ret = vpif_config_data-setup_input_channel_mode + (vpif-std_info.ycmux_mode); - if (ret 0) { - vpif_dbg(1, debug, can't set vpif channel mode\n); - return ret; + if (ret 0) { + vpif_dbg(1, debug, can't set vpif channel mode\n); + return ret; + } } /* Call vpif_set_params function to set the parameters and addresses */ diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c index e129c98..1e35f92 100644 --- a/drivers/media/video/davinci/vpif_display.c +++ b/drivers/media/video/davinci/vpif_display.c @@ -280,12 +280,14 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } /* clock settings */ - ret = - vpif_config_data-set_clock(ch-vpifparams.std_info.ycmux_mode, - ch-vpifparams.std_info.hd_sd); - if (ret 0) { - vpif_err(can't set clock\n); - return ret; + if (vpif_config_data-set_clock) { + ret = + vpif_config_data-set_clock(ch-vpifparams.std_info.ycmux_mode, + ch-vpifparams.std_info.hd_sd); + if (ret 0) { + vpif_err(can't set clock\n); + return ret; + } } /* set the parameters and addresses */ Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] media: davinci: vpif: add check for NULL handler
From: Lad, Prabhakar prabhakar@ti.com for da850/omap-l138, there is no need to setup_input_channel_mode() and set_clock(), to avoid adding dummy code in board file just returning zero add a check in the driver itself to call the handler only if its not NULL. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com Cc: Mauro Carvalho Chehab mche...@infradead.org --- Changes for v2: 1: Added patch description, pointed by Mauro. drivers/media/platform/davinci/vpif_capture.c | 13 +++-- drivers/media/platform/davinci/vpif_display.c | 13 +++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index 13aa46d..7b5e09a 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -311,12 +311,13 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } /* configure 1 or 2 channel mode */ - ret = vpif_config_data-setup_input_channel_mode - (vpif-std_info.ycmux_mode); - - if (ret 0) { - vpif_dbg(1, debug, can't set vpif channel mode\n); - return ret; + if (vpif_config_data-setup_input_channel_mode) { + ret = vpif_config_data- + setup_input_channel_mode(vpif-std_info.ycmux_mode); + if (ret 0) { + vpif_dbg(1, debug, can't set vpif channel mode\n); + return ret; + } } /* Call vpif_set_params function to set the parameters and addresses */ diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index e401aea..e8a0286 100644 --- a/drivers/media/platform/davinci/vpif_display.c +++ b/drivers/media/platform/davinci/vpif_display.c @@ -280,12 +280,13 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } /* clock settings */ - ret = - vpif_config_data-set_clock(ch-vpifparams.std_info.ycmux_mode, - ch-vpifparams.std_info.hd_sd); - if (ret 0) { - vpif_err(can't set clock\n); - return ret; + if (vpif_config_data-set_clock) { + ret = vpif_config_data-set_clock(ch-vpifparams.std_info. + ycmux_mode, ch-vpifparams.std_info.hd_sd); + if (ret 0) { + vpif_err(can't set clock\n); + return ret; + } } /* set the parameters and addresses */ -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html