Re: HVR-1600 QAM recordings with slight glitches in them
On 29-04-12 00:21, Brian J. Murrell wrote: On 12-04-28 04:39 PM, Brian J. Murrell wrote: I typically have one more splitter downstream from that 3 way splitter which is a 4 way splitter to feed all of the tuners on my Mythtv box and introducing that splitter reduces the SNR at the HVR-1600 to between "13c" and "13e" (31.6 - 31.8 dB). Interestingly enough, I moved the Myth backend to it's usual home, in the basement, right next to the incoming cable signal and replaced that 25' run that I had going to where it was temporarily with a smaller, say 10' run (of RG-59 so still room for improvement) and my SNR at the HVR-1600, even after all of the splitters is now "015c" or 34.8 dB. I'm still going to go replacing all of that RG-59 with shorter, custom made lengths of RG6 cables. I can't go "too short" when making those can I or would even a 6-12 inch cable be perfectly fine? I'm thinking of the runs between that last 4 way splitter and the tuners in the Myth backend. b. Brian, There is no minimum cable length for RF. Although for practical reasons i rarely go below 30 cm (1 '). It should be possible for you to buy "drop cables" which have a length of 1m5 (about 5') and are commonly used in HE to connect equipment. screw-on F-connectors are another source of problems. Crimping F-connectors are best, but those need a fitting crimp tool. Cheers, Rudy -- 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 10/15] V4L: Add camera 3A lock control
Hi Sakari, On 04/24/2012 10:59 PM, Sakari Ailus wrote: > Dzien dobry Sylwester, > > (I hope it's not too wrong time of the day for that! ;)) No, it's never too wrong;) > Sylwester Nawrocki wrote: >> On 04/23/2012 07:47 AM, Sakari Ailus wrote: >>> Sylwester Nawrocki wrote: On 04/17/2012 06:09 PM, Sakari Ailus wrote: > On Tue, Apr 17, 2012 at 12:09:51PM +0200, Sylwester Nawrocki wrote: ... >>> I was thinking how does the situation really differ from disabling the >>> corresponding automatic algorithm. There may be subtle differences in >>> practice albeit in principle the two are no different. And if some of >>> the >>> sensors implement it as lock, then I guess it gives us few options >>> for the >>> user space interface. >> >> Can you anticipate any any possible issues such diversity might bring to >> applications ? I imagine such control can be quite useful for snapshot, >> and with current control API design and the drivers' behaviour >> applications >> cannot be sure what settings a device ends up with after switching from >> "auto" to "manual" - last auto settings or the manual values. Usually its >> just the previous manual values. > > On software controlled digital cameras, depending on what the manual > configuration actually means, you either get the same than by locking > the automatic control or the previous manual configuration. If the means > for manual configuration are the same than what the automatic algorithm > uses then it's the first case. However, I have a feeling that such low > level controls might often not work the best for manual control: for > white balance users seldom wish to fiddle with SRGB matrix or gamma > tables directly. Colour balance might just do mostly the same and be > more convenient, with the automatic algorithm still doing some work to > configure the underlying low-level configuration. I was thinking more along the lines of currently available V4L2 controls. And with cameras with I2C control interface there is often a device specific user interface of which characteristics are slightly different that what we would have implemented directly in software in user space. > Perhaps it would make sense to suggest that the control algorithm locks > should be implemented even in cases where the lock would mean exactly > the same than just disabling the algorithm. What do you think? IMHO a driver should expose the control only if hardware supports locking. Once we have clear semantics for the lock control, it should be up to the driver's author to decide whether such functionality should be exposed or not. I believe the locking and disabling/enabling automatic settings will differ in most cases, there may be different latencies involved, etc. Also I think the lock control should have top priority, .e.g when the user sets V4L2_CID_AUTO_FOCUS_START control nothing should happen, unless V4L2_LOCK_FOCUS is set to 0. What do you think ? BTW, I've changed names of the individual lock bits an removed "_3A" prefix from them. It seems to look better that way: #define V4L2_CID_3A_LOCK(V4L2_CID_CAMERA_CLASS_BASE+27) #define V4L2_LOCK_EXPOSURE (1 << 0) #define V4L2_LOCK_WHITE_BALANCE (1 << 1) #define V4L2_LOCK_FOCUS (1 << 2) Or must we abide the rule that individual option names are created by only removing the "_CID" part from the control name and appending the option's name ? > Shouldn't the lock be related to contiguous focus only btw.? Regular > autofocus is typically a one-time operation, the lens ending up on a > position where the AF algorithm left it. Perhaps, it would make sense mostly for continuous auto focus. However, I'm inclined to not limit it to continuous auto focus only. I think regardless of the focus mode the lock should freeze focusing, so still image capture is not disturbed. -- 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
[GIT PULL FOR v3.5] cpia2: major overhaul to get it in a working state again.
Hi all, I managed to get hold of a cpia2-based Hanse USB microscope almost a year ago after reports from Andrea that the driver didn't work properly. I finally had time to take a really good look at the driver and it was broken in many respects. This patch brings the driver up to speed with respect to the v4l2 framework and it now works as expected, including disconnect handling and suspend/resume handling. The only thing left to do some day is to convert it to the videobuf2 framework. Regards, Hans The following changes since commit bcb2cf6e0bf033d79821c89e5ccb328bfbd44907: [media] ngene: remove an unneeded condition (2012-04-26 15:29:23 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git cpia2 for you to fetch changes up to 14a3d232eb5d25c768f40fbd6a87db48a249a391: cpia2: major overhaul to get it in a working state again. (2012-04-29 13:44:44 +0200) Hans Verkuil (1): cpia2: major overhaul to get it in a working state again. drivers/media/video/cpia2/cpia2.h | 34 ++- drivers/media/video/cpia2/cpia2_core.c | 142 +++- drivers/media/video/cpia2/cpia2_usb.c | 78 +-- drivers/media/video/cpia2/cpia2_v4l.c | 846 + drivers/media/video/cpia2/cpia2dev.h | 50 - 5 files changed, 363 insertions(+), 787 deletions(-) delete mode 100644 drivers/media/video/cpia2/cpia2dev.h -- 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: can't rmmod au0828; modprobe au0828 and have a working device
On 12-04-19 08:08 AM, Brian J. Murrell wrote: > I have an HVR-950Q: > > [44847.234403] tveeprom 0-0050: Hauppauge model 72001, rev B3F0, serial# > *** > [44847.294643] tveeprom 0-0050: MAC address is **:**:**:**:**:** > [44847.343417] tveeprom 0-0050: tuner model is Xceive XC5000 (idx 150, type > 76) > [44847.402873] tveeprom 0-0050: TV standards NTSC(M) ATSC/DVB Digital (eeprom > 0x88) > [44847.465471] tveeprom 0-0050: audio processor is AU8522 (idx 44) > [44847.515481] tveeprom 0-0050: decoder processor is AU8522 (idx 42) > [44847.567162] tveeprom 0-0050: has no radio, has IR receiver, has no IR > transmitter > [44847.630272] hauppauge_eeprom: hauppauge eeprom: model=72001 > > I cannot seem to get it to work after removing the au0828 xc5000 au8522 > modules and then modprobing the au0828 module. To follow-up on this, it seems that if I remove and insert the au0828 enough times, it will be functional again. Race perhaps? It seems when it's inserted and doesn't work, the following is logged in the kernel message buffer: xc5000: Device not found at addr 0x61 (0x) and when it's inserted successfully: xc5000: Successfully identified at address 0x61 xc5000: Firmware has not been loaded previously It also seems that the module will become non-functional just as if I had removed and inserted it, all on it's own without any removal insertion. b. signature.asc Description: OpenPGP digital signature
Re: HVR-1600 QAM recordings with slight glitches in them
On 12-04-29 03:02 AM, Rudy Zijlstra wrote: > Brian, Hi Rudy, > There is no minimum cable length for RF. Although for practical reasons > i rarely go below 30 cm (1 '). Indeed. Especially with RG6 they can get a bit stiff. > It should be possible for you to buy "drop cables" which have a length > of 1m5 (about 5') and are commonly used in HE to connect equipment. Yeah, I'm trying to reduce the cable "nest" and I only have about 6" between the back of the equipment and the wall, where the splitter will be so 1' cables will probably do the trick. > screw-on F-connectors are another source of problems. Yes. > Crimping > F-connectors are best, but those need a fitting crimp tool. Indeed, and I have one. :-) Thanks for the info! b. signature.asc Description: OpenPGP digital signature
[PATCH 0/3] omap3isp: Implement selection API support
Hi everybody, There 3 patches implement support for the selection API in the OMAP3 ISP driver. Support for the legacy subdev crop API is removed from the driver, but still available through the translation layer in the subdev core. Laurent Pinchart (3): omap3isp: ccdc: Add selection support on output formatter source pad omap3isp: preview: Replace the crop API by the selection API omap3isp: resizer: Replace the crop API by the selection API drivers/media/video/omap3isp/ispccdc.c| 180 ++-- drivers/media/video/omap3isp/ispccdc.h|2 + drivers/media/video/omap3isp/isppreview.c | 74 +--- drivers/media/video/omap3isp/ispresizer.c | 138 ++ 4 files changed, 311 insertions(+), 83 deletions(-) -- 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 2/3] omap3isp: preview: Replace the crop API by the selection API
Support for the legacy crop API is emulated in the subdev core. Signed-off-by: Laurent Pinchart --- drivers/media/video/omap3isp/isppreview.c | 74 + 1 files changed, 54 insertions(+), 20 deletions(-) diff --git a/drivers/media/video/omap3isp/isppreview.c b/drivers/media/video/omap3isp/isppreview.c index 420b131..cbc8870 100644 --- a/drivers/media/video/omap3isp/isppreview.c +++ b/drivers/media/video/omap3isp/isppreview.c @@ -1929,55 +1929,89 @@ static int preview_enum_frame_size(struct v4l2_subdev *sd, } /* - * preview_get_crop - Retrieve the crop rectangle on a pad + * preview_get_selection - Retrieve a selection rectangle on a pad * @sd: ISP preview V4L2 subdevice * @fh: V4L2 subdev file handle - * @crop: crop rectangle + * @sel: Selection rectangle + * + * The only supported rectangles are the crop rectangles on the sink pad. * * Return 0 on success or a negative error code otherwise. */ -static int preview_get_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, - struct v4l2_subdev_crop *crop) +static int preview_get_selection(struct v4l2_subdev *sd, +struct v4l2_subdev_fh *fh, +struct v4l2_subdev_selection *sel) { struct isp_prev_device *prev = v4l2_get_subdevdata(sd); + struct v4l2_mbus_framefmt *format; + + if (sel->pad != PREV_PAD_SINK) + return -EINVAL; + + switch (sel->target) { + case V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS: + sel->r.left = 0; + sel->r.top = 0; + sel->r.width = INT_MAX; + sel->r.height = INT_MAX; + + format = __preview_get_format(prev, fh, PREV_PAD_SINK, + sel->which); + preview_try_crop(prev, format, &sel->r); + break; + + case V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL: + sel->r = *__preview_get_crop(prev, fh, sel->which); + break; - /* Cropping is only supported on the sink pad. */ - if (crop->pad != PREV_PAD_SINK) + default: return -EINVAL; + } - crop->rect = *__preview_get_crop(prev, fh, crop->which); return 0; } /* - * preview_set_crop - Retrieve the crop rectangle on a pad + * preview_set_selection - Set a selection rectangle on a pad * @sd: ISP preview V4L2 subdevice * @fh: V4L2 subdev file handle - * @crop: crop rectangle + * @sel: Selection rectangle + * + * The only supported rectangle is the actual crop rectangle on the sink pad. * * Return 0 on success or a negative error code otherwise. */ -static int preview_set_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, - struct v4l2_subdev_crop *crop) +static int preview_set_selection(struct v4l2_subdev *sd, +struct v4l2_subdev_fh *fh, +struct v4l2_subdev_selection *sel) { struct isp_prev_device *prev = v4l2_get_subdevdata(sd); struct v4l2_mbus_framefmt *format; - /* Cropping is only supported on the sink pad. */ - if (crop->pad != PREV_PAD_SINK) + if (sel->target != V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL || + sel->pad != PREV_PAD_SINK) return -EINVAL; /* The crop rectangle can't be changed while streaming. */ if (prev->state != ISP_PIPELINE_STREAM_STOPPED) return -EBUSY; - format = __preview_get_format(prev, fh, PREV_PAD_SINK, crop->which); - preview_try_crop(prev, format, &crop->rect); - *__preview_get_crop(prev, fh, crop->which) = crop->rect; + /* Modifying the crop rectangle always changes the format on the source +* pad. If the KEEP_CONFIG flag is set, just return the current crop +* rectangle. +*/ + if (sel->flags & V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG) { + sel->r = *__preview_get_crop(prev, fh, sel->which); + return 0; + } + + format = __preview_get_format(prev, fh, PREV_PAD_SINK, sel->which); + preview_try_crop(prev, format, &sel->r); + *__preview_get_crop(prev, fh, sel->which) = sel->r; /* Update the source format. */ - format = __preview_get_format(prev, fh, PREV_PAD_SOURCE, crop->which); - preview_try_format(prev, fh, PREV_PAD_SOURCE, format, crop->which); + format = __preview_get_format(prev, fh, PREV_PAD_SOURCE, sel->which); + preview_try_format(prev, fh, PREV_PAD_SOURCE, format, sel->which); return 0; } @@ -2086,8 +2120,8 @@ static const struct v4l2_subdev_pad_ops preview_v4l2_pad_ops = { .enum_frame_size = preview_enum_frame_size, .get_fmt = preview_get_format, .set_fmt = preview_set_format, - .get_crop = preview_get_crop, - .set_crop = preview_set_crop, + .get_selection = preview_get_selection, + .set_selection = pre
[PATCH 3/3] omap3isp: resizer: Replace the crop API by the selection API
Support for the legacy crop API is emulated in the subdev core. Signed-off-by: Laurent Pinchart --- drivers/media/video/omap3isp/ispresizer.c | 138 ++--- 1 files changed, 88 insertions(+), 50 deletions(-) diff --git a/drivers/media/video/omap3isp/ispresizer.c b/drivers/media/video/omap3isp/ispresizer.c index 6958a9e..d7341ab 100644 --- a/drivers/media/video/omap3isp/ispresizer.c +++ b/drivers/media/video/omap3isp/ispresizer.c @@ -1188,32 +1188,6 @@ static int resizer_set_stream(struct v4l2_subdev *sd, int enable) } /* - * resizer_g_crop - handle get crop subdev operation - * @sd : pointer to v4l2 subdev structure - * @pad : subdev pad - * @crop : pointer to crop structure - * @which : active or try format - * return zero - */ -static int resizer_g_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, - struct v4l2_subdev_crop *crop) -{ - struct isp_res_device *res = v4l2_get_subdevdata(sd); - struct v4l2_mbus_framefmt *format; - struct resizer_ratio ratio; - - /* Only sink pad has crop capability */ - if (crop->pad != RESZ_PAD_SINK) - return -EINVAL; - - format = __resizer_get_format(res, fh, RESZ_PAD_SOURCE, crop->which); - crop->rect = *__resizer_get_crop(res, fh, crop->which); - resizer_calc_ratios(res, &crop->rect, format, &ratio); - - return 0; -} - -/* * resizer_try_crop - mangles crop parameters. */ static void resizer_try_crop(const struct v4l2_mbus_framefmt *sink, @@ -1223,7 +1197,7 @@ static void resizer_try_crop(const struct v4l2_mbus_framefmt *sink, const unsigned int spv = DEFAULT_PHASE; const unsigned int sph = DEFAULT_PHASE; - /* Crop rectangle is constrained to the output size so that zoom ratio + /* Crop rectangle is constrained by the output size so that zoom ratio * cannot exceed +/-4.0. */ unsigned int min_width = @@ -1248,51 +1222,115 @@ static void resizer_try_crop(const struct v4l2_mbus_framefmt *sink, } /* - * resizer_s_crop - handle set crop subdev operation - * @sd : pointer to v4l2 subdev structure - * @pad : subdev pad - * @crop : pointer to crop structure - * @which : active or try format - * return -EINVAL or zero when succeed + * resizer_get_selection - Retrieve a selection rectangle on a pad + * @sd: ISP resizer V4L2 subdevice + * @fh: V4L2 subdev file handle + * @sel: Selection rectangle + * + * The only supported rectangles are the crop rectangles on the sink pad. + * + * Return 0 on success or a negative error code otherwise. */ -static int resizer_s_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, - struct v4l2_subdev_crop *crop) +static int resizer_get_selection(struct v4l2_subdev *sd, +struct v4l2_subdev_fh *fh, +struct v4l2_subdev_selection *sel) +{ + struct isp_res_device *res = v4l2_get_subdevdata(sd); + struct v4l2_mbus_framefmt *format_source; + struct v4l2_mbus_framefmt *format_sink; + struct resizer_ratio ratio; + + if (sel->pad != RESZ_PAD_SINK) + return -EINVAL; + + format_sink = __resizer_get_format(res, fh, RESZ_PAD_SINK, + sel->which); + format_source = __resizer_get_format(res, fh, RESZ_PAD_SOURCE, +sel->which); + + switch (sel->target) { + case V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS: + sel->r.left = 0; + sel->r.top = 0; + sel->r.width = INT_MAX; + sel->r.height = INT_MAX; + + resizer_try_crop(format_sink, format_source, &sel->r); + resizer_calc_ratios(res, &sel->r, format_source, &ratio); + break; + + case V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL: + sel->r = *__resizer_get_crop(res, fh, sel->which); + resizer_calc_ratios(res, &sel->r, format_source, &ratio); + break; + + default: + return -EINVAL; + } + + return 0; +} + +/* + * resizer_set_selection - Set a selection rectangle on a pad + * @sd: ISP resizer V4L2 subdevice + * @fh: V4L2 subdev file handle + * @sel: Selection rectangle + * + * The only supported rectangle is the actual crop rectangle on the sink pad. + * + * FIXME: This function currently behaves as if the KEEP_CONFIG selection flag + * was always set. + * + * Return 0 on success or a negative error code otherwise. + */ +static int resizer_set_selection(struct v4l2_subdev *sd, +struct v4l2_subdev_fh *fh, +struct v4l2_subdev_selection *sel) { struct isp_res_device *res = v4l2_get_subdevdata(sd); struct isp_device *isp = to_isp_device(res); struct v4l2_mbus_framefmt *format_sink, *format_source; struct resizer_ratio ratio; - /* Only sink pad h
[PATCH 1/3] omap3isp: ccdc: Add selection support on output formatter source pad
Signed-off-by: Laurent Pinchart --- drivers/media/video/omap3isp/ispccdc.c | 180 +--- drivers/media/video/omap3isp/ispccdc.h |2 + 2 files changed, 169 insertions(+), 13 deletions(-) diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c index 8d8d6f3..8db8f3e 100644 --- a/drivers/media/video/omap3isp/ispccdc.c +++ b/drivers/media/video/omap3isp/ispccdc.c @@ -38,6 +38,9 @@ #include "ispreg.h" #include "ispccdc.h" +#define CCDC_MIN_WIDTH 32 +#define CCDC_MIN_HEIGHT32 + static struct v4l2_mbus_framefmt * __ccdc_get_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, unsigned int pad, enum v4l2_subdev_format_whence which); @@ -1118,6 +1121,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) struct isp_parallel_platform_data *pdata = NULL; struct v4l2_subdev *sensor; struct v4l2_mbus_framefmt *format; + const struct v4l2_rect *crop; const struct isp_format_info *fmt_info; struct v4l2_subdev_format fmt_src; unsigned int depth_out; @@ -1211,14 +1215,14 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT); /* CCDC_PAD_SOURCE_OF */ - format = &ccdc->formats[CCDC_PAD_SOURCE_OF]; + crop = &ccdc->crop; - isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) | - ((format->width - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT), + isp_reg_writel(isp, (crop->left << ISPCCDC_HORZ_INFO_SPH_SHIFT) | + ((crop->width - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT), OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO); - isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV0_SHIFT, + isp_reg_writel(isp, crop->top << ISPCCDC_VERT_START_SLV0_SHIFT, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START); - isp_reg_writel(isp, (format->height - 1) + isp_reg_writel(isp, (crop->height - 1) << ISPCCDC_VERT_LINES_NLV_SHIFT, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES); @@ -1793,6 +1797,16 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, return &ccdc->formats[pad]; } +static struct v4l2_rect * +__ccdc_get_crop(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, + enum v4l2_subdev_format_whence which) +{ + if (which == V4L2_SUBDEV_FORMAT_TRY) + return v4l2_subdev_get_try_crop(fh, CCDC_PAD_SOURCE_OF); + else + return &ccdc->crop; +} + /* * ccdc_try_format - Try video format on a pad * @ccdc: ISP CCDC device @@ -1809,6 +1823,7 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, const struct isp_format_info *info; unsigned int width = fmt->width; unsigned int height = fmt->height; + struct v4l2_rect *crop; unsigned int i; switch (pad) { @@ -1834,14 +1849,10 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SINK, which); memcpy(fmt, format, sizeof(*fmt)); - /* The data formatter truncates the number of horizontal output -* pixels to a multiple of 16. To avoid clipping data, allow -* callers to request an output size bigger than the input size -* up to the nearest multiple of 16. -*/ - fmt->width = clamp_t(u32, width, 32, fmt->width + 15); - fmt->width &= ~15; - fmt->height = clamp_t(u32, height, 32, fmt->height); + /* Hardcode the output size to the crop rectangle size. */ + crop = __ccdc_get_crop(ccdc, fh, which); + fmt->width = crop->width; + fmt->height = crop->height; break; case CCDC_PAD_SOURCE_VP: @@ -1869,6 +1880,49 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, } /* + * ccdc_try_crop - Validate a crop rectangle + * @ccdc: ISP CCDC device + * @sink: format on the sink pad + * @crop: crop rectangle to be validated + */ +static void ccdc_try_crop(struct isp_ccdc_device *ccdc, + const struct v4l2_mbus_framefmt *sink, + struct v4l2_rect *crop) +{ + const struct isp_format_info *info; + unsigned int max_width; + + /* For Bayer formats, restrict left/top and width/height to even values +* to keep the Bayer pattern. +*/ + info = omap3isp_video_format_info(sink->code); + if (info->flavor != V4L2_MBUS_FMT_Y8_1X8) { + crop->left &= ~1; + crop->top &= ~1; + } + + crop->left = clamp_t(u32, crop->left, 0, sink->width - CCDC_MIN_WIDTH); + crop->top = clamp_t(u32, crop->top, 0, sink->height -
Re: ATSC-MH driver support for the Hauppauge WinTV Aero-m
New pull request follows, API patch attached (see below) On Thu, Apr 19, 2012 at 10:40 AM, Michael Krufky wrote: > On Thu, Apr 19, 2012 at 9:36 AM, Mauro Carvalho Chehab > wrote: >> Em 10-04-2012 00:49, Michael Krufky escreveu: >>> These patches have been around and tested for quite some time. Every >>> few weeks I have to regenerate them in order to stay in sync with the >>> media tree. I think it's time for some review and possibly merge into >>> the master development repository. This complies with what was >>> discussed in at the media developer kernel summit in Prague, Oct 2011. >>> Once merged, I'll have time to work on some userspace utilities. For >>> now, I have created a very basic ATSC-MH scanning application that >>> demonstrates the API additions. The app can be found here: >>> http://linuxtv.org/hg/~mkrufky/mhscan >>> [snip] >> >> This patch is incomplete: >> - It doesn't increment the version number; >> - Docbook is untouched. >> >> Also, I didn't see any post of those patches at the ML. Please post the >> patches at the ML for review before sending a pull request, especially >> when API changes are there. > > Mauro, > > Thanks for the feedback. I'll make the Docbook changes, then I'll > patchbomb the mailing list (it's just a handful of patches for the API > change) and follow it up with another pull request. > > Cheers, > > Mike Mauro, I've made the changes that you've requested, and I've reduced the pull request to only include the API changes. Attached please find the patch. The following changes since commit 296da3cd14db9eb5606924962b2956c9c656dbb0: [media] pwc: poll(): Check that the device has not beem claimed for streaming already (2012-03-27 11:42:04 -0300) are available in the git repository at: git://git.linuxtv.org/mkrufky/mxl111sf atscmh_for_v3.5 Michael Krufky (3): linux-dvb v5 API support for ATSC-MH DocBook: document new DTV Properties for ATSC-MH delivery system increment DVB API to version 5.6 for ATSC-MH frontend control Documentation/DocBook/media/dvb/dvbproperty.xml | 178 +++ drivers/media/dvb/dvb-core/dvb_frontend.c | 92 - drivers/media/dvb/dvb-core/dvb_frontend.h | 22 +++ include/linux/dvb/frontend.h| 54 +++- include/linux/dvb/version.h |2 +- 5 files changed, 345 insertions(+), 3 deletions(-) Cheers, Mike atsc-mh-dvb-api-5-6.patch Description: Binary data
Re: [PATCH] v4l: v4l2-ctrls: Add forward declaration of struct file
On Monday 23 April 2012 13:29:11 Laurent Pinchart wrote: > This fixes the following warning: > > In file included from drivers/media/video/v4l2-subdev.c:29: > include/media/v4l2-ctrls.h:501: warning: 'struct file' declared inside > parameter list > include/media/v4l2-ctrls.h:501: warning: its scope is only this > definition or declaration, which is probably not what you want > include/media/v4l2-ctrls.h:509: warning: 'struct file' declared inside > parameter list Ping ? Should I include this in my next pull request ? > Signed-off-by: Laurent Pinchart > --- > include/media/v4l2-ctrls.h |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > index 33907a9..9022e1c 100644 > --- a/include/media/v4l2-ctrls.h > +++ b/include/media/v4l2-ctrls.h > @@ -25,6 +25,7 @@ > #include > > /* forward references */ > +struct file; > struct v4l2_ctrl_handler; > struct v4l2_ctrl_helper; > struct v4l2_ctrl; -- 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: ATSC-MH driver support for the Hauppauge WinTV Aero-m
>From 78d56f9888c1c768b43cc75e0e02d0e60848bcc4 Mon Sep 17 00:00:00 2001 From: Michael Krufky Date: Sun, 29 Jan 2012 13:44:58 -0500 Subject: [PATCH 1/3] linux-dvb v5 API support for ATSC-MH Signed-off-by: Michael Krufky --- drivers/media/dvb/dvb-core/dvb_frontend.c | 92 - drivers/media/dvb/dvb-core/dvb_frontend.h | 22 +++ include/linux/dvb/frontend.h | 54 +- 3 files changed, 166 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c index 4555baa..067f10a 100644 --- a/drivers/media/dvb/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c @@ -180,13 +180,13 @@ static enum dvbv3_emulation_type dvbv3_type(u32 delivery_system) case SYS_DMBTH: return DVBV3_OFDM; case SYS_ATSC: + case SYS_ATSCMH: case SYS_DVBC_ANNEX_B: return DVBV3_ATSC; case SYS_UNDEFINED: case SYS_ISDBC: case SYS_DVBH: case SYS_DAB: - case SYS_ATSCMH: default: /* * Doesn't know how to emulate those types and/or @@ -1027,6 +1027,28 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = { _DTV_CMD(DTV_HIERARCHY, 0, 0), _DTV_CMD(DTV_ENUM_DELSYS, 0, 0), + + _DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0), + _DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0), + + _DTV_CMD(DTV_ATSCMH_FIC_VER, 0, 0), + _DTV_CMD(DTV_ATSCMH_PARADE_ID, 0, 0), + _DTV_CMD(DTV_ATSCMH_NOG, 0, 0), + _DTV_CMD(DTV_ATSCMH_TNOG, 0, 0), + _DTV_CMD(DTV_ATSCMH_SGN, 0, 0), + _DTV_CMD(DTV_ATSCMH_PRC, 0, 0), + _DTV_CMD(DTV_ATSCMH_RS_FRAME_MODE, 0, 0), + _DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 0, 0), + _DTV_CMD(DTV_ATSCMH_RS_CODE_MODE_PRI, 0, 0), + _DTV_CMD(DTV_ATSCMH_RS_CODE_MODE_SEC, 0, 0), + _DTV_CMD(DTV_ATSCMH_SCCC_BLOCK_MODE, 0, 0), + _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_A, 0, 0), + _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0), + _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0), + _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0), + _DTV_CMD(DTV_ATSCMH_FIC_ERR, 0, 0), + _DTV_CMD(DTV_ATSCMH_CRC_ERR, 0, 0), + _DTV_CMD(DTV_ATSCMH_RS_ERR, 0, 0), }; static void dtv_property_dump(struct dtv_property *tvp) @@ -1118,6 +1140,8 @@ static int dtv_property_cache_sync(struct dvb_frontend *fe, case DVBV3_ATSC: dprintk("%s() Preparing ATSC req\n", __func__); c->modulation = p->u.vsb.modulation; + if (c->delivery_system == SYS_ATSCMH) + break; if ((c->modulation == VSB_8) || (c->modulation == VSB_16)) c->delivery_system = SYS_ATSC; else @@ -1364,6 +1388,63 @@ static int dtv_property_process_get(struct dvb_frontend *fe, case DTV_DVBT2_PLP_ID: tvp->u.data = c->dvbt2_plp_id; break; + + /* ATSC-MH */ + case DTV_ATSCMH_FIC_VER: + tvp->u.data = fe->dtv_property_cache.atscmh_fic_ver; + break; + case DTV_ATSCMH_PARADE_ID: + tvp->u.data = fe->dtv_property_cache.atscmh_parade_id; + break; + case DTV_ATSCMH_NOG: + tvp->u.data = fe->dtv_property_cache.atscmh_nog; + break; + case DTV_ATSCMH_TNOG: + tvp->u.data = fe->dtv_property_cache.atscmh_tnog; + break; + case DTV_ATSCMH_SGN: + tvp->u.data = fe->dtv_property_cache.atscmh_sgn; + break; + case DTV_ATSCMH_PRC: + tvp->u.data = fe->dtv_property_cache.atscmh_prc; + break; + case DTV_ATSCMH_RS_FRAME_MODE: + tvp->u.data = fe->dtv_property_cache.atscmh_rs_frame_mode; + break; + case DTV_ATSCMH_RS_FRAME_ENSEMBLE: + tvp->u.data = fe->dtv_property_cache.atscmh_rs_frame_ensemble; + break; + case DTV_ATSCMH_RS_CODE_MODE_PRI: + tvp->u.data = fe->dtv_property_cache.atscmh_rs_code_mode_pri; + break; + case DTV_ATSCMH_RS_CODE_MODE_SEC: + tvp->u.data = fe->dtv_property_cache.atscmh_rs_code_mode_sec; + break; + case DTV_ATSCMH_SCCC_BLOCK_MODE: + tvp->u.data = fe->dtv_property_cache.atscmh_sccc_block_mode; + break; + case DTV_ATSCMH_SCCC_CODE_MODE_A: + tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_a; + break; + case DTV_ATSCMH_SCCC_CODE_MODE_B: + tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_b; + break; + case DTV_ATSCMH_SCCC_CODE_MODE_C: + tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_c; + break; + case DTV_ATSCMH_SCCC_CODE_MODE_D: + tvp->u.data = fe->dtv_property
[PATCH 0/3] MT9P031 sensor driver fixes
Hi everybody, Here are 3 small patches for the MT9P031 sensor driver. I plan to push them for v3.5. Laurent Pinchart (3): mt9p031: Identify color/mono models using I2C device name mt9p031: Replace the reset board callback by a GPIO number mt9p031: Implement black level compensation control drivers/media/video/mt9p031.c | 161 + include/media/mt9p031.h | 19 +++--- 2 files changed, 155 insertions(+), 25 deletions(-) -- 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 1/3] mt9p031: Identify color/mono models using I2C device name
Instead of passing a color/monochrome flag through platform data, rely on the I2C device name to identify the chip model. Signed-off-by: Laurent Pinchart --- drivers/media/video/mt9p031.c | 14 +++--- include/media/mt9p031.h |6 -- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c index c81eaf4..5b8a396 100644 --- a/drivers/media/video/mt9p031.c +++ b/drivers/media/video/mt9p031.c @@ -99,6 +99,11 @@ #define MT9P031_TEST_PATTERN_RED 0xa2 #define MT9P031_TEST_PATTERN_BLUE 0xa3 +enum mt9p031_model { + MT9P031_MODEL_COLOR, + MT9P031_MODEL_MONOCHROME, +}; + struct mt9p031 { struct v4l2_subdev subdev; struct media_pad pad; @@ -109,6 +114,7 @@ struct mt9p031 { struct mutex power_lock; /* lock to protect power_count */ int power_count; + enum mt9p031_model model; struct aptina_pll pll; /* Registers cache */ @@ -764,7 +770,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) format = v4l2_subdev_get_try_format(fh, 0); - if (mt9p031->pdata->version == MT9P031_MONOCHROME_VERSION) + if (mt9p031->model == MT9P031_MODEL_MONOCHROME) format->code = V4L2_MBUS_FMT_Y12_1X12; else format->code = V4L2_MBUS_FMT_SGRBG12_1X12; @@ -842,6 +848,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->pdata = pdata; mt9p031->output_control = MT9P031_OUTPUT_CONTROL_DEF; mt9p031->mode2 = MT9P031_READ_MODE_2_ROW_BLC; + mt9p031->model = did->driver_data; v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 4); @@ -882,7 +889,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->crop.left = MT9P031_COLUMN_START_DEF; mt9p031->crop.top = MT9P031_ROW_START_DEF; - if (mt9p031->pdata->version == MT9P031_MONOCHROME_VERSION) + if (mt9p031->model == MT9P031_MODEL_MONOCHROME) mt9p031->format.code = V4L2_MBUS_FMT_Y12_1X12; else mt9p031->format.code = V4L2_MBUS_FMT_SGRBG12_1X12; @@ -918,7 +925,8 @@ static int mt9p031_remove(struct i2c_client *client) } static const struct i2c_device_id mt9p031_id[] = { - { "mt9p031", 0 }, + { "mt9p031", MT9P031_MODEL_COLOR }, + { "mt9p031m", MT9P031_MODEL_MONOCHROME }, { } }; MODULE_DEVICE_TABLE(i2c, mt9p031_id); diff --git a/include/media/mt9p031.h b/include/media/mt9p031.h index 96448c7..5b5090f 100644 --- a/include/media/mt9p031.h +++ b/include/media/mt9p031.h @@ -3,17 +3,11 @@ struct v4l2_subdev; -enum { - MT9P031_COLOR_VERSION, - MT9P031_MONOCHROME_VERSION, -}; - struct mt9p031_platform_data { int (*set_xclk)(struct v4l2_subdev *subdev, int hz); int (*reset)(struct v4l2_subdev *subdev, int active); int ext_freq; /* input frequency to the mt9p031 for PLL dividers */ int target_freq; /* frequency target for the PLL */ - int version; /* MT9P031_COLOR_VERSION or MT9P031_MONOCHROME_VERSION */ }; #endif -- 1.7.3.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
[PATCH 3/3] mt9p031: Implement black level compensation control
Add four new controls to configure black level compensation (BLC): - V4L2_CID_BLC_AUTO selects between manual and auto BLC - V4L2_CID_BLC_TARGET_LEVEL sets the target level for auto BLC - V4L2_CID_BLC_ANALOG_OFFSET sets the analog offset for manual BLC - V4L2_CID_BLC_DIGITAL_OFFSET sets the digital offset for manual BLC Signed-off-by: Laurent Pinchart --- drivers/media/video/mt9p031.c | 118 ++--- 1 files changed, 111 insertions(+), 7 deletions(-) diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c index 3a93631..8f061d9 100644 --- a/drivers/media/video/mt9p031.c +++ b/drivers/media/video/mt9p031.c @@ -91,7 +91,14 @@ #defineMT9P031_GLOBAL_GAIN_MAX 1024 #defineMT9P031_GLOBAL_GAIN_DEF 8 #defineMT9P031_GLOBAL_GAIN_MULT(1 << 6) +#define MT9P031_ROW_BLACK_TARGET 0x49 #define MT9P031_ROW_BLACK_DEF_OFFSET 0x4b +#define MT9P031_GREEN1_OFFSET 0x60 +#define MT9P031_GREEN2_OFFSET 0x61 +#define MT9P031_BLACK_LEVEL_CALIBRATION0x62 +#defineMT9P031_BLC_MANUAL_BLC (1 << 0) +#define MT9P031_RED_OFFSET 0x63 +#define MT9P031_BLUE_OFFSET0x64 #define MT9P031_TEST_PATTERN 0xa0 #defineMT9P031_TEST_PATTERN_SHIFT 3 #defineMT9P031_TEST_PATTERN_ENABLE (1 << 0) @@ -110,7 +117,6 @@ struct mt9p031 { struct media_pad pad; struct v4l2_rect crop; /* Sensor window */ struct v4l2_mbus_framefmt format; - struct v4l2_ctrl_handler ctrls; struct mt9p031_platform_data *pdata; struct mutex power_lock; /* lock to protect power_count */ int power_count; @@ -119,6 +125,10 @@ struct mt9p031 { struct aptina_pll pll; int reset; + struct v4l2_ctrl_handler ctrls; + struct v4l2_ctrl *blc_auto; + struct v4l2_ctrl *blc_offset; + /* Registers cache */ u16 output_control; u16 mode2; @@ -565,6 +575,10 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev, */ #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) static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) { @@ -629,11 +643,17 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_TEST_PATTERN: if (!ctrl->val) { - ret = mt9p031_set_mode2(mt9p031, - 0, MT9P031_READ_MODE_2_ROW_BLC); - if (ret < 0) - return ret; - + /* Restore the black level compensation settings. */ + if (mt9p031->blc_auto->cur.val != 0) { + ret = mt9p031_s_ctrl(mt9p031->blc_auto); + if (ret < 0) + return ret; + } + if (mt9p031->blc_offset->cur.val != 0) { + ret = mt9p031_s_ctrl(mt9p031->blc_offset); + if (ret < 0) + return ret; + } return mt9p031_write(client, MT9P031_TEST_PATTERN, MT9P031_TEST_PATTERN_DISABLE); } @@ -648,10 +668,14 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) if (ret < 0) return ret; + /* Disable digital black level compensation when using a test +* pattern. +*/ ret = mt9p031_set_mode2(mt9p031, MT9P031_READ_MODE_2_ROW_BLC, 0); if (ret < 0) return ret; + ret = mt9p031_write(client, MT9P031_ROW_BLACK_DEF_OFFSET, 0); if (ret < 0) return ret; @@ -659,7 +683,40 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) return mt9p031_write(client, MT9P031_TEST_PATTERN, ((ctrl->val - 1) << MT9P031_TEST_PATTERN_SHIFT) | MT9P031_TEST_PATTERN_ENABLE); + + case V4L2_CID_BLC_AUTO: + ret = mt9p031_set_mode2(mt9p031, + ctrl->val ? 0 : MT9P031_READ_MODE_2_ROW_BLC, + ctrl->val ? MT9P031_READ_MODE_2_ROW_BLC : 0); + if (ret < 0) +
[PATCH 2/3] mt9p031: Replace the reset board callback by a GPIO number
Use the GPIO from the sensor driver instead of calling back to board code. Signed-off-by: Laurent Pinchart --- drivers/media/video/mt9p031.c | 29 +++-- include/media/mt9p031.h | 13 ++--- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c index 5b8a396..3a93631 100644 --- a/drivers/media/video/mt9p031.c +++ b/drivers/media/video/mt9p031.c @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -116,6 +117,7 @@ struct mt9p031 { enum mt9p031_model model; struct aptina_pll pll; + int reset; /* Registers cache */ u16 output_control; @@ -247,8 +249,8 @@ static inline int mt9p031_pll_disable(struct mt9p031 *mt9p031) static int mt9p031_power_on(struct mt9p031 *mt9p031) { /* Ensure RESET_BAR is low */ - if (mt9p031->pdata->reset) { - mt9p031->pdata->reset(&mt9p031->subdev, 1); + if (mt9p031->reset != -1) { + gpio_set_value(mt9p031->reset, 0); usleep_range(1000, 2000); } @@ -258,8 +260,8 @@ static int mt9p031_power_on(struct mt9p031 *mt9p031) mt9p031->pdata->ext_freq); /* Now RESET_BAR must be high */ - if (mt9p031->pdata->reset) { - mt9p031->pdata->reset(&mt9p031->subdev, 0); + if (mt9p031->reset != -1) { + gpio_set_value(mt9p031->reset, 1); usleep_range(1000, 2000); } @@ -268,8 +270,8 @@ static int mt9p031_power_on(struct mt9p031 *mt9p031) static void mt9p031_power_off(struct mt9p031 *mt9p031) { - if (mt9p031->pdata->reset) { - mt9p031->pdata->reset(&mt9p031->subdev, 1); + if (mt9p031->reset != -1) { + gpio_set_value(mt9p031->reset, 0); usleep_range(1000, 2000); } @@ -849,6 +851,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->output_control = MT9P031_OUTPUT_CONTROL_DEF; mt9p031->mode2 = MT9P031_READ_MODE_2_ROW_BLC; mt9p031->model = did->driver_data; + mt9p031->reset = -1; v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 4); @@ -899,10 +902,22 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->format.field = V4L2_FIELD_NONE; mt9p031->format.colorspace = V4L2_COLORSPACE_SRGB; + if (pdata->reset != -1) { + ret = gpio_request_one(pdata->reset, GPIOF_OUT_INIT_LOW, + "mt9p031_rst"); + if (ret < 0) + goto done; + + mt9p031->reset = pdata->reset; + } + ret = mt9p031_pll_setup(mt9p031); done: if (ret < 0) { + if (mt9p031->reset != -1) + gpio_free(mt9p031->reset); + v4l2_ctrl_handler_free(&mt9p031->ctrls); media_entity_cleanup(&mt9p031->subdev.entity); kfree(mt9p031); @@ -919,6 +934,8 @@ static int mt9p031_remove(struct i2c_client *client) v4l2_ctrl_handler_free(&mt9p031->ctrls); v4l2_device_unregister_subdev(subdev); media_entity_cleanup(&subdev->entity); + if (mt9p031->reset != -1) + gpio_free(mt9p031->reset); kfree(mt9p031); return 0; diff --git a/include/media/mt9p031.h b/include/media/mt9p031.h index 5b5090f..0c97b19 100644 --- a/include/media/mt9p031.h +++ b/include/media/mt9p031.h @@ -3,11 +3,18 @@ struct v4l2_subdev; +/* + * struct mt9p031_platform_data - MT9P031 platform data + * @set_xclk: Clock frequency set callback + * @reset: Chip reset GPIO (set to -1 if not used) + * @ext_freq: Input clock frequency + * @target_freq: Pixel clock frequency + */ struct mt9p031_platform_data { int (*set_xclk)(struct v4l2_subdev *subdev, int hz); - int (*reset)(struct v4l2_subdev *subdev, int active); - int ext_freq; /* input frequency to the mt9p031 for PLL dividers */ - int target_freq; /* frequency target for the PLL */ + int reset; + int ext_freq; + int target_freq; }; #endif -- 1.7.3.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: [media-ctl PATCH] Compare entity name length aswell
Hi Sergio, On Saturday 28 April 2012 10:04:01 Aguirre, Sergio wrote: > On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre wrote: > > Otherwise, some false positives might arise when > > having 2 subdevices with similar names, like: > > > > "OMAP4 ISS ISP IPIPEIF" > > "OMAP4 ISS ISP IPIPE" > > > > Before this patch, trying to find "OMAP4 ISS ISP IPIPE", resulted > > in a false entity match, retrieving "OMAP4 ISS ISP IPIPEIF" > > information instead. > > > > Checking length should ensure such cases are handled well. > > Any feedback about this? Thanks for the patch, and sorry for the delay. > > Signed-off-by: Sergio Aguirre > > --- > > src/mediactl.c |3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/src/mediactl.c b/src/mediactl.c > > index 5b8c587..451a386 100644 > > --- a/src/mediactl.c > > +++ b/src/mediactl.c > > @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct > > media_device *media, for (i = 0; i < media->entities_count; ++i) { > >struct media_entity *entity = &media->entities[i]; > > > > - if (strncmp(entity->info.name, name, length) == 0) > > + if ((strncmp(entity->info.name, name, length) == 0) && > > + (strlen(entity->info.name) == length)) > >return entity; > >} Instead of calling strlen() which has a O(n) complexity, what about just checking that the entity name has a '\0' in the length'th position ? Something like the following patch: >From 46bec667b675573cf1ce698c68112e3dbd31930e Mon Sep 17 00:00:00 2001 From: Sergio Aguirre Date: Wed, 25 Apr 2012 08:57:13 -0500 Subject: [PATCH] Compare name length to avoid false positives in media_get_entity_by_name If two subdevice have names that only differ by a suffix (such as "OMAP4 ISS ISP IPIPE" and "OMAP4 ISS ISP IPIPEIF") the media_get_entity_by_name function might return a pointer to the entity with the longest name when called with the shortest name. Fix this by verifying that the candidate entity name length is equal to the requested name length. Signed-off-by: Sergio Aguirre Signed-off-by: Laurent Pinchart --- src/mediactl.c |9 - src/tools.h|1 + 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/mediactl.c b/src/mediactl.c index 5b8c587..bc6a713 100644 --- a/src/mediactl.c +++ b/src/mediactl.c @@ -63,10 +63,17 @@ struct media_entity *media_get_entity_by_name(struct media_device *media, { unsigned int i; + /* A match is impossible if the entity name is longer than the maximum +* size we can get from the kernel. +*/ + if (length >= FIELD_SIZEOF(struct media_entity_desc, name)) + return NULL; + for (i = 0; i < media->entities_count; ++i) { struct media_entity *entity = &media->entities[i]; - if (strncmp(entity->info.name, name, length) == 0) + if (strncmp(entity->info.name, name, length) == 0 && + entity->info.name[length] == '\0') return entity; } diff --git a/src/tools.h b/src/tools.h index e56edb2..de06cb3 100644 --- a/src/tools.h +++ b/src/tools.h @@ -23,6 +23,7 @@ #define __TOOLS_H__ #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) +#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) #endif /* __TOOLS_H__ */ -- 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
cron job: media_tree daily build: WARNINGS
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:Sun Apr 29 19:00:15 CEST 2012 git hash:bcb2cf6e0bf033d79821c89e5ccb328bfbd44907 gcc version: i686-linux-gcc (GCC) 4.6.3 host hardware:x86_64 host os: 3.1-2.slh.1-amd64 linux-git-arm-eabi-exynos: WARNINGS linux-git-arm-eabi-omap: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-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: OK linux-3.1-i686: OK linux-3.2.1-i686: OK linux-3.3-i686: 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 apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.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
[GIT PULL for v3.5] Control events support for uvcvideo
Hi Mauro, The following changes since commit bcb2cf6e0bf033d79821c89e5ccb328bfbd44907: [media] ngene: remove an unneeded condition (2012-04-26 15:29:23 -0300) are available in the git repository at: git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-events Hans de Goede (10): media/radio: use v4l2_ctrl_subscribe_event where possible v4l2-event: Add v4l2_subscribed_event_ops v4l2-ctrls: Use v4l2_subscribed_event_ops uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning uvcvideo: Refactor uvc_ctrl_get and query uvcvideo: Move __uvc_ctrl_get() up uvcvideo: Add support for control events uvcvideo: Properly report the inactive flag for inactive controls uvcvideo: Send control change events for slave ctrls when the master changes uvcvideo: Drop unused ctrl member from struct uvc_control_mapping Documentation/video4linux/v4l2-framework.txt | 28 ++- drivers/media/radio/radio-isa.c | 10 +- drivers/media/radio/radio-keene.c| 14 +- drivers/media/video/ivtv/ivtv-ioctl.c|3 +- drivers/media/video/omap3isp/ispccdc.c |2 +- drivers/media/video/omap3isp/ispstat.c |2 +- drivers/media/video/uvc/uvc_ctrl.c | 320 + drivers/media/video/uvc/uvc_v4l2.c | 46 +++- drivers/media/video/uvc/uvcvideo.h | 26 ++- drivers/media/video/v4l2-ctrls.c | 58 - drivers/media/video/v4l2-event.c | 71 +++--- drivers/usb/gadget/uvc_v4l2.c|2 +- include/media/v4l2-ctrls.h |7 +- include/media/v4l2-event.h | 24 ++- 14 files changed, 456 insertions(+), 157 deletions(-) -- 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: video capture driver interlacing question (easycap)
Hi, > > i.e., no, you should not merge fields in the driver, IIRC, you just hand > them over to the user in separate buffers. > Thanks a lot for your answer. However, in that case I'm a little confused by the fact that em28xx driver merges both fields by copying one line from each into a buffer before handing it back to user. I'd love if someone could clarify this issue. Thanks a lot again, Ezequiel. -- 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-ctl PATCH] Compare entity name length aswell
Hi Laurent, On Sun, Apr 29, 2012 at 12:40 PM, Laurent Pinchart wrote: > Hi Sergio, > > On Saturday 28 April 2012 10:04:01 Aguirre, Sergio wrote: >> On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre wrote: >> > Otherwise, some false positives might arise when >> > having 2 subdevices with similar names, like: >> > >> > "OMAP4 ISS ISP IPIPEIF" >> > "OMAP4 ISS ISP IPIPE" >> > >> > Before this patch, trying to find "OMAP4 ISS ISP IPIPE", resulted >> > in a false entity match, retrieving "OMAP4 ISS ISP IPIPEIF" >> > information instead. >> > >> > Checking length should ensure such cases are handled well. >> >> Any feedback about this? > > Thanks for the patch, and sorry for the delay. No problem. :) > >> > Signed-off-by: Sergio Aguirre >> > --- >> > src/mediactl.c | 3 ++- >> > 1 files changed, 2 insertions(+), 1 deletions(-) >> > >> > diff --git a/src/mediactl.c b/src/mediactl.c >> > index 5b8c587..451a386 100644 >> > --- a/src/mediactl.c >> > +++ b/src/mediactl.c >> > @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct >> > media_device *media, for (i = 0; i < media->entities_count; ++i) { >> > struct media_entity *entity = &media->entities[i]; >> > >> > - if (strncmp(entity->info.name, name, length) == 0) >> > + if ((strncmp(entity->info.name, name, length) == 0) && >> > + (strlen(entity->info.name) == length)) >> > return entity; >> > } > > Instead of calling strlen() which has a O(n) complexity, what about just > checking that the entity name has a '\0' in the length'th position ? Something > like the following patch: > > From 46bec667b675573cf1ce698c68112e3dbd31930e Mon Sep 17 00:00:00 2001 > From: Sergio Aguirre > Date: Wed, 25 Apr 2012 08:57:13 -0500 > Subject: [PATCH] Compare name length to avoid false positives in > media_get_entity_by_name > > If two subdevice have names that only differ by a suffix (such as "OMAP4 > ISS ISP IPIPE" and "OMAP4 ISS ISP IPIPEIF") the media_get_entity_by_name > function might return a pointer to the entity with the longest name when > called with the shortest name. Fix this by verifying that the candidate > entity name length is equal to the requested name length. > > Signed-off-by: Sergio Aguirre > Signed-off-by: Laurent Pinchart > --- > src/mediactl.c | 9 - > src/tools.h | 1 + > 2 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/src/mediactl.c b/src/mediactl.c > index 5b8c587..bc6a713 100644 > --- a/src/mediactl.c > +++ b/src/mediactl.c > @@ -63,10 +63,17 @@ struct media_entity *media_get_entity_by_name(struct > media_device *media, > { > unsigned int i; > > + /* A match is impossible if the entity name is longer than the maximum > + * size we can get from the kernel. > + */ > + if (length >= FIELD_SIZEOF(struct media_entity_desc, name)) > + return NULL; > + Good idea. > for (i = 0; i < media->entities_count; ++i) { > struct media_entity *entity = &media->entities[i]; > > - if (strncmp(entity->info.name, name, length) == 0) > + if (strncmp(entity->info.name, name, length) == 0 && > + entity->info.name[length] == '\0') ACK. Your patch is definitely better. IMHO, I think it'll be more fair if you put yourself as the author of this new patch, and me as: "Reported-by:". Regards, Sergio > return entity; > } > > diff --git a/src/tools.h b/src/tools.h > index e56edb2..de06cb3 100644 > --- a/src/tools.h > +++ b/src/tools.h > @@ -23,6 +23,7 @@ > #define __TOOLS_H__ > > #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) > +#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) > > #endif /* __TOOLS_H__ */ > > -- > 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 PATCH 5/8] v4l: fix compiler warnings.
On Mon, 2012-04-23 at 13:38 +0200, Hans Verkuil wrote: > media_build/v4l/au0828-video.c: In function 'au0828_irq_callback': > media_build/v4l/au0828-video.c:123:6: warning: variable 'rc' set but not used > [-Wunused-but-set-variable] > media_build/v4l/cx23888-ir.c: In function 'pulse_clocks_to_clock_divider': > media_build/v4l/cx23888-ir.c:334:6: warning: variable 'rem' set but not used > [-Wunused-but-set-variable] > media_build/v4l/cx25840-ir.c: In function 'pulse_clocks_to_clock_divider': > media_build/v4l/cx25840-ir.c:319:6: warning: variable 'rem' set but not used > [-Wunused-but-set-variable] > media_build/v4l/cx25840-ir.c: In function 'cx25840_ir_tx_write': > media_build/v4l/cx25840-ir.c:863:21: warning: variable 'c' set but not used > [-Wunused-but-set-variable] > media_build/v4l/em28xx-audio.c: In function 'snd_em28xx_hw_capture_params': > media_build/v4l/em28xx-audio.c:346:31: warning: variable 'format' set but not > used [-Wunused-but-set-variable] > media_build/v4l/em28xx-audio.c:346:25: warning: variable 'rate' set but not > used [-Wunused-but-set-variable] > media_build/v4l/em28xx-audio.c:346:15: warning: variable 'channels' set but > not used [-Wunused-but-set-variable] > media_build/v4l/et61x251_core.c: In function 'et61x251_urb_complete': > media_build/v4l/et61x251_core.c:370:16: warning: variable 'len' set but not > used [-Wunused-but-set-variable] > media_build/v4l/et61x251_core.c: In function 'et61x251_stream_interrupt': > media_build/v4l/et61x251_core.c:581:7: warning: variable 'timeout' set but > not used [-Wunused-but-set-variable] > media_build/v4l/hdpvr-control.c: In function 'get_input_lines_info': > media_build/v4l/hdpvr-control.c:98:6: warning: variable 'ret' set but not > used [-Wunused-but-set-variable] > media_build/v4l/hdpvr-video.c: In function 'hdpvr_try_ctrl': > media_build/v4l/hdpvr-video.c:955:6: warning: variable 'ret' set but not used > [-Wunused-but-set-variable] > media_build/v4l/saa7134-video.c: In function 'saa7134_s_tuner': > media_build/v4l/saa7134-video.c:2030:6: warning: variable 'rx' set but not > used [-Wunused-but-set-variable] > media_build/v4l/sn9c102_core.c: In function 'sn9c102_stream_interrupt': > media_build/v4l/sn9c102_core.c:998:7: warning: variable 'timeout' set but not > used [-Wunused-but-set-variable] > > Signed-off-by: Hans Verkuil cx23888-ir.c and cx25840-ir.c changes look good to me. Reviewed-by: Andy Walls -Andy > --- > drivers/media/video/au0828/au0828-video.c|4 ++-- > drivers/media/video/cx23885/cx23888-ir.c |4 +--- > drivers/media/video/cx25840/cx25840-ir.c |6 +- > drivers/media/video/em28xx/em28xx-audio.c|9 + > drivers/media/video/et61x251/et61x251_core.c | 11 --- > drivers/media/video/hdpvr/hdpvr-control.c|2 ++ > drivers/media/video/hdpvr/hdpvr-video.c |2 +- > drivers/media/video/saa7134/saa7134-video.c |2 +- > drivers/media/video/sn9c102/sn9c102_core.c |4 +--- > 9 files changed, 18 insertions(+), 26 deletions(-) -- 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: [ 3960.758784] 1 lock held by motion/7776: [ 3960.758788] #0: (&queue->mutex){......}, at: [] uvc_queue_enable+0x32/0xc0
Hi Sander, On Saturday 28 April 2012 22:02:46 Sander Eikelenboom wrote: > Hi, > > I'm using a webcam (logitech) supported by the uvcvideo module to capture > video. Previously once in a while I would get the "uvcvideo: Failed to > resubmit video URB (-27).", but the grabbing continued without a problem. > Now the video grabbing program (motion) seems to lock due to some nested > lock if i interpret it right. The uvcvideo driver doesn't use nested locks, this is "just" a normal locking issue. The mutex_lock_nested() call in the backtrace comes from lock debugging support in your kernel. A quick look at the code doesn't reveal any obvious locking issue. Do you have multiple threads accessing the device at the same time ? > Additional problem is that i don't really know what kernel version was still > OK, so can't help with that info. > > This is on a vanilla 3.4 RC kernel latest commit: > c629eaf8392b676b4f83c3dc344e66402bfeec92 > > [ 3696.753490] ehci_hcd :09:00.1: request 880016091400 would > overflow (3923+31 >= 3936) [ 3696.753494] uvcvideo: Failed to resubmit > video URB (-27). > [ 3696.753563] ehci_hcd :09:00.1: request 880016091800 would > overflow (3922+31 >= 3936) [ 3696.753566] uvcvideo: Failed to resubmit > video URB (-27). > [ 3696.753609] ehci_hcd :09:00.1: request 880016090800 would > overflow (3922+31 >= 3936) [ 3696.753611] uvcvideo: Failed to resubmit > video URB (-27). > [ 3696.753645] ehci_hcd :09:00.1: request 880016090c00 would > overflow (3922+31 >= 3936) [ 3696.753647] uvcvideo: Failed to resubmit > video URB (-27). > [ 3696.753656] ehci_hcd :09:00.1: request 880016091000 would > overflow (3922+31 >= 3936) [ 3696.753657] uvcvideo: Failed to resubmit > video URB (-27). > [ 3960.758154] INFO: task motion:7776 blocked for more than 120 seconds. > [ 3960.758168] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. [ 3960.758174] motion D 0201 0 7776 > 1 0x [ 3960.758183] 8800239d9b68 0216 > eaa50018 810a451b [ 3960.758192] 88002392cf60 > 00012600 8800239d9fd8 8800239d8010 [ 3960.758200] > 00012600 00012600 8800239d9fd8 00012600 [ > 3960.758209] Call Trace: > [ 3960.758219] [] ? __lock_acquire+0x5b/0xc00 > [ 3960.758226] [] ? hub_suspend+0xf8/0x130 > [ 3960.758232] [] ? usbdev_do_ioctl+0x194/0x1000 > [ 3960.758238] [] ? __lock_acquire+0x5b/0xc00 > [ 3960.758244] [] ? get_parent_ip+0x11/0x50 > [ 3960.758250] [] ? sub_preempt_count+0x9d/0xd0 > [ 3960.758257] [] ? hdpvr_probe+0x6c2/0xa30 The backtrace isn't valid, making this a bit harder to debug. Making compiling the kernel with debug symbols would help here. Could you also please enable lockdep (CONFIG_LOCKDEP=y) if not done already ? > [ 3960.758264] [] schedule+0x24/0x70 > [ 3960.758269] [] schedule_preempt_disabled+0x13/0x20 > [ 3960.758275] [] mutex_lock_nested+0x1a7/0x420 > [ 3960.758281] [] ? uvc_queue_enable+0x32/0xc0 > [ 3960.758287] [] uvc_queue_enable+0x32/0xc0 > [ 3960.758292] [] uvc_video_enable+0x12f/0x180 > [ 3960.758298] [] uvc_v4l2_do_ioctl+0x555/0x1190 > [ 3960.758304] [] ? sock_update_classid+0xa8/0x120 > [ 3960.758310] [] ? sock_sendmsg+0xee/0x120 > [ 3960.758316] [] video_usercopy+0x186/0x4c0 > [ 3960.758322] [] ? uvc_v4l2_set_streamparm+0x190/0x190 > [ 3960.758327] [] ? __lock_acquire+0x5b/0xc00 > [ 3960.758333] [] ? lock_release+0xff/0x210 > [ 3960.758338] [] ? get_parent_ip+0x11/0x50 > [ 3960.758344] [] uvc_v4l2_ioctl+0x24/0x70 > [ 3960.758349] [] ? __lock_acquire+0x5b/0xc00 > [ 3960.758740] [] ? fsnotify+0x84/0x360 > [ 3960.758745] [] v4l2_ioctl+0xb0/0x180 > [ 3960.758751] [] do_vfs_ioctl+0x93/0x500 > [ 3960.758756] [] ? lock_release+0xff/0x210 > [ 3960.758762] [] ? fget_light+0xd7/0x140 > [ 3960.758768] [] ? fget_light+0x3b/0x140 > [ 3960.758773] [] sys_ioctl+0x4a/0x80 > [ 3960.758778] [] system_call_fastpath+0x16/0x1b > [ 3960.758784] 1 lock held by motion/7776: > [ 3960.758788] #0: (&queue->mutex){..}, at: [] It looks like the driver tries to take the lock queue->mutex lock a second time. I don't see any code path that could lead to that. -- 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 PATCH 3/8] ivtv/cx18: fix compiler warnings
On Mon, 2012-04-23 at 13:38 +0200, Hans Verkuil wrote: > media_build/v4l/cx18-alsa-main.c: In function 'cx18_alsa_exit': > media_build/v4l/cx18-alsa-main.c:282:6: warning: variable 'ret' set but not > used [-Wunused-but-set-variable] > media_build/v4l/cx18-mailbox.c: In function 'cx18_api_call': > media_build/v4l/cx18-mailbox.c:598:6: warning: variable 'state' set but not > used [-Wunused-but-set-variable] > media_build/v4l/cx18-alsa-pcm.c: In function 'snd_cx18_pcm_capture_open': > media_build/v4l/cx18-alsa-pcm.c:156:6: warning: variable 'ret' set but not > used [-Wunused-but-set-variable] > media_build/v4l/cx18-alsa-pcm.c: In function 'snd_cx18_pcm_capture_close': > media_build/v4l/cx18-alsa-pcm.c:202:6: warning: variable 'ret' set but not > used [-Wunused-but-set-variable] > media_build/v4l/cx18-alsa-pcm.c: In function 'snd_cx18_pcm_hw_params': > media_build/v4l/cx18-alsa-pcm.c:255:6: warning: variable 'ret' set but not > used [-Wunused-but-set-variable] > media_build/v4l/cx18-streams.c: In function 'cx18_stop_v4l2_encode_stream': > media_build/v4l/cx18-streams.c:983:16: warning: variable 'then' set but not > used [-Wunused-but-set-variable] > media_build/v4l/ivtv-ioctl.c: In function 'ivtv_set_speed': > media_build/v4l/ivtv-ioctl.c:138:22: warning: variable 's' set but not used > [-Wunused-but-set-variable] > media_build/v4l/ivtvfb.c: In function 'ivtvfb_init': > media_build/v4l/ivtvfb.c:1286:6: warning: variable 'err' set but not used > [-Wunused-but-set-variable] > media_build/v4l/ivtvfb.c: In function 'ivtvfb_cleanup': > media_build/v4l/ivtvfb.c:1306:6: warning: variable 'err' set but not used > [-Wunused-but-set-variable] > > Signed-off-by: Hans Verkuil Looks ok. Reviewed-by: Andy Walls Some comments of no consequence are below... -Andy > --- > drivers/media/video/cx18/cx18-alsa-main.c |1 + > drivers/media/video/cx18/cx18-alsa-pcm.c | 10 +++--- > drivers/media/video/cx18/cx18-mailbox.c |6 +- > drivers/media/video/cx18/cx18-streams.c |3 --- > drivers/media/video/ivtv/ivtv-ioctl.c |3 --- > drivers/media/video/ivtv/ivtvfb.c |2 ++ > 6 files changed, 7 insertions(+), 18 deletions(-) > > diff --git a/drivers/media/video/cx18/cx18-alsa-main.c > b/drivers/media/video/cx18/cx18-alsa-main.c > index e118361..6d2a982 100644 > --- a/drivers/media/video/cx18/cx18-alsa-main.c > +++ b/drivers/media/video/cx18/cx18-alsa-main.c > @@ -285,6 +285,7 @@ static void __exit cx18_alsa_exit(void) > > drv = driver_find("cx18", &pci_bus_type); > ret = driver_for_each_device(drv, NULL, NULL, cx18_alsa_exit_callback); > + (void)ret; /* suppress compiler warning */ > Why not just remove 'ret', which is what the rest of the changeset does in similar situations? No big deal though. It is fine as is. > cx18_ext_init = NULL; > printk(KERN_INFO "cx18-alsa: module unload complete\n"); > diff --git a/drivers/media/video/cx18/cx18-alsa-pcm.c > b/drivers/media/video/cx18/cx18-alsa-pcm.c > index 82d195b..7a5b84a 100644 > --- a/drivers/media/video/cx18/cx18-alsa-pcm.c > +++ b/drivers/media/video/cx18/cx18-alsa-pcm.c > @@ -190,7 +190,7 @@ static int snd_cx18_pcm_capture_open(struct > snd_pcm_substream *substream) > ret = cx18_start_v4l2_encode_stream(s); > snd_cx18_unlock(cxsc); > > - return 0; > + return ret; > } Ugh. I need to change this code to set the streaming flag based on the success of getting the stream started. Your change is fine though. > > static int snd_cx18_pcm_capture_close(struct snd_pcm_substream *substream) > @@ -199,12 +199,11 @@ static int snd_cx18_pcm_capture_close(struct > snd_pcm_substream *substream) > struct v4l2_device *v4l2_dev = cxsc->v4l2_dev; > struct cx18 *cx = to_cx18(v4l2_dev); > struct cx18_stream *s; > - int ret; > > /* Instruct the cx18 to stop sending packets */ > snd_cx18_lock(cxsc); > s = &cx->streams[CX18_ENC_STREAM_TYPE_PCM]; > - ret = cx18_stop_v4l2_encode_stream(s, 0); > + cx18_stop_v4l2_encode_stream(s, 0); > clear_bit(CX18_F_S_STREAMING, &s->s_flags); > > cx18_release_stream(s); > @@ -252,13 +251,10 @@ static int snd_pcm_alloc_vmalloc_buffer(struct > snd_pcm_substream *subs, > static int snd_cx18_pcm_hw_params(struct snd_pcm_substream *substream, >struct snd_pcm_hw_params *params) > { > - int ret; > - > dprintk("%s called\n", __func__); > > - ret = snd_pcm_alloc_vmalloc_buffer(substream, > + return snd_pcm_alloc_vmalloc_buffer(substream, > params_buffer_bytes(params)); > - return 0; > } > > static int snd_cx18_pcm_hw_free(struct snd_pcm_substream *substream) > diff --git a/drivers/media/video/cx18/cx18-mailbox.c > b/drivers/media/video/cx18/cx18-mailbox.c > index 0c7796e..ed81183 100644 > --- a/drivers/media/video/cx18/cx18-mailbox.c > +++ b/drivers/media/video/cx18/cx18-mailbox.c > @@ -595,9
Re: [media-ctl PATCH] Compare entity name length aswell
Hi Sergio, On Sunday 29 April 2012 17:36:43 Aguirre, Sergio wrote: > On Sun, Apr 29, 2012 at 12:40 PM, Laurent Pinchart wrote: > > On Saturday 28 April 2012 10:04:01 Aguirre, Sergio wrote: > >> On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre wrote: > >> > Otherwise, some false positives might arise when > >> > having 2 subdevices with similar names, like: > >> > > >> > "OMAP4 ISS ISP IPIPEIF" > >> > "OMAP4 ISS ISP IPIPE" > >> > > >> > Before this patch, trying to find "OMAP4 ISS ISP IPIPE", resulted > >> > in a false entity match, retrieving "OMAP4 ISS ISP IPIPEIF" > >> > information instead. > >> > > >> > Checking length should ensure such cases are handled well. > >> > >> Any feedback about this? > > > > Thanks for the patch, and sorry for the delay. > > No problem. :) > > >> > Signed-off-by: Sergio Aguirre > >> > --- > >> > src/mediactl.c |3 ++- > >> > 1 files changed, 2 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/src/mediactl.c b/src/mediactl.c > >> > index 5b8c587..451a386 100644 > >> > --- a/src/mediactl.c > >> > +++ b/src/mediactl.c > >> > @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct > >> > media_device *media, for (i = 0; i < media->entities_count; ++i) { > >> >struct media_entity *entity = &media->entities[i]; > >> > > >> > - if (strncmp(entity->info.name, name, length) == 0) > >> > + if ((strncmp(entity->info.name, name, length) == 0) && > >> > + (strlen(entity->info.name) == length)) > >> >return entity; > >> >} > > > > Instead of calling strlen() which has a O(n) complexity, what about just > > checking that the entity name has a '\0' in the length'th position ? > > Something like the following patch: > > > > From 46bec667b675573cf1ce698c68112e3dbd31930e Mon Sep 17 00:00:00 2001 > > From: Sergio Aguirre > > Date: Wed, 25 Apr 2012 08:57:13 -0500 > > Subject: [PATCH] Compare name length to avoid false positives in > > media_get_entity_by_name > > > > If two subdevice have names that only differ by a suffix (such as "OMAP4 > > ISS ISP IPIPE" and "OMAP4 ISS ISP IPIPEIF") the media_get_entity_by_name > > function might return a pointer to the entity with the longest name when > > called with the shortest name. Fix this by verifying that the candidate > > entity name length is equal to the requested name length. > > > > Signed-off-by: Sergio Aguirre > > Signed-off-by: Laurent Pinchart > > --- > > src/mediactl.c |9 - > > src/tools.h|1 + > > 2 files changed, 9 insertions(+), 1 deletions(-) > > > > diff --git a/src/mediactl.c b/src/mediactl.c > > index 5b8c587..bc6a713 100644 > > --- a/src/mediactl.c > > +++ b/src/mediactl.c > > @@ -63,10 +63,17 @@ struct media_entity *media_get_entity_by_name(struct > > media_device *media, > > { > >unsigned int i; > > > > + /* A match is impossible if the entity name is longer than the > > maximum +* size we can get from the kernel. > > +*/ > > + if (length >= FIELD_SIZEOF(struct media_entity_desc, name)) > > + return NULL; > > + > > Good idea. > > >for (i = 0; i < media->entities_count; ++i) { > >struct media_entity *entity = &media->entities[i]; > > > > - if (strncmp(entity->info.name, name, length) == 0) > > + if (strncmp(entity->info.name, name, length) == 0 && > > + entity->info.name[length] == '\0') > > ACK. > > Your patch is definitely better. > > IMHO, I think it'll be more fair if you put yourself as the author of this > new patch, and me as: "Reported-by:". It's such a small patch, it's fine :-) Thank you for the proposal nonetheless. I've pushed the patch to the repository. > >return entity; > >} > > > > diff --git a/src/tools.h b/src/tools.h > > index e56edb2..de06cb3 100644 > > --- a/src/tools.h > > +++ b/src/tools.h > > @@ -23,6 +23,7 @@ > > #define __TOOLS_H__ > > > > #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) > > +#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) > > > > #endif /* __TOOLS_H__ */ > > -- 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: [media-ctl PATCH] Compare entity name length aswell
Hi Laurent, On Sun, Apr 29, 2012 at 6:13 PM, Laurent Pinchart wrote: > Hi Sergio, > > On Sunday 29 April 2012 17:36:43 Aguirre, Sergio wrote: >> On Sun, Apr 29, 2012 at 12:40 PM, Laurent Pinchart wrote: >> > On Saturday 28 April 2012 10:04:01 Aguirre, Sergio wrote: >> >> On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre wrote: >> >> > Otherwise, some false positives might arise when >> >> > having 2 subdevices with similar names, like: >> >> > >> >> > "OMAP4 ISS ISP IPIPEIF" >> >> > "OMAP4 ISS ISP IPIPE" >> >> > >> >> > Before this patch, trying to find "OMAP4 ISS ISP IPIPE", resulted >> >> > in a false entity match, retrieving "OMAP4 ISS ISP IPIPEIF" >> >> > information instead. >> >> > >> >> > Checking length should ensure such cases are handled well. >> >> >> >> Any feedback about this? >> > >> > Thanks for the patch, and sorry for the delay. >> >> No problem. :) >> >> >> > Signed-off-by: Sergio Aguirre >> >> > --- >> >> > src/mediactl.c | 3 ++- >> >> > 1 files changed, 2 insertions(+), 1 deletions(-) >> >> > >> >> > diff --git a/src/mediactl.c b/src/mediactl.c >> >> > index 5b8c587..451a386 100644 >> >> > --- a/src/mediactl.c >> >> > +++ b/src/mediactl.c >> >> > @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct >> >> > media_device *media, for (i = 0; i < media->entities_count; ++i) { >> >> > struct media_entity *entity = &media->entities[i]; >> >> > >> >> > - if (strncmp(entity->info.name, name, length) == 0) >> >> > + if ((strncmp(entity->info.name, name, length) == 0) && >> >> > + (strlen(entity->info.name) == length)) >> >> > return entity; >> >> > } >> > >> > Instead of calling strlen() which has a O(n) complexity, what about just >> > checking that the entity name has a '\0' in the length'th position ? >> > Something like the following patch: >> > >> > From 46bec667b675573cf1ce698c68112e3dbd31930e Mon Sep 17 00:00:00 2001 >> > From: Sergio Aguirre >> > Date: Wed, 25 Apr 2012 08:57:13 -0500 >> > Subject: [PATCH] Compare name length to avoid false positives in >> > media_get_entity_by_name >> > >> > If two subdevice have names that only differ by a suffix (such as "OMAP4 >> > ISS ISP IPIPE" and "OMAP4 ISS ISP IPIPEIF") the media_get_entity_by_name >> > function might return a pointer to the entity with the longest name when >> > called with the shortest name. Fix this by verifying that the candidate >> > entity name length is equal to the requested name length. >> > >> > Signed-off-by: Sergio Aguirre >> > Signed-off-by: Laurent Pinchart >> > --- >> > src/mediactl.c | 9 - >> > src/tools.h | 1 + >> > 2 files changed, 9 insertions(+), 1 deletions(-) >> > >> > diff --git a/src/mediactl.c b/src/mediactl.c >> > index 5b8c587..bc6a713 100644 >> > --- a/src/mediactl.c >> > +++ b/src/mediactl.c >> > @@ -63,10 +63,17 @@ struct media_entity *media_get_entity_by_name(struct >> > media_device *media, >> > { >> > unsigned int i; >> > >> > + /* A match is impossible if the entity name is longer than the >> > maximum + * size we can get from the kernel. >> > + */ >> > + if (length >= FIELD_SIZEOF(struct media_entity_desc, name)) >> > + return NULL; >> > + >> >> Good idea. >> >> > for (i = 0; i < media->entities_count; ++i) { >> > struct media_entity *entity = &media->entities[i]; >> > >> > - if (strncmp(entity->info.name, name, length) == 0) >> > + if (strncmp(entity->info.name, name, length) == 0 && >> > + entity->info.name[length] == '\0') >> >> ACK. >> >> Your patch is definitely better. >> >> IMHO, I think it'll be more fair if you put yourself as the author of this >> new patch, and me as: "Reported-by:". > > It's such a small patch, it's fine :-) Thank you for the proposal nonetheless. > > I've pushed the patch to the repository. Excellent! Thanks for that :) Regards, Sergio > >> > return entity; >> > } >> > >> > diff --git a/src/tools.h b/src/tools.h >> > index e56edb2..de06cb3 100644 >> > --- a/src/tools.h >> > +++ b/src/tools.h >> > @@ -23,6 +23,7 @@ >> > #define __TOOLS_H__ >> > >> > #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) >> > +#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) >> > >> > #endif /* __TOOLS_H__ */ >> > > > -- > 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: [GIT PULL FOR v3.5] An ivtv fix and support suspend/resume in radio-keene
On Fri, 2012-04-27 at 13:56 +0200, Hans Verkuil wrote: > Hi Mauro, > > One small trivial ivtv fix and a patch that adds support for suspend/resume > to the radio-keene driver. > > Regards, > > Hans > > The following changes since commit bcb2cf6e0bf033d79821c89e5ccb328bfbd44907: > > [media] ngene: remove an unneeded condition (2012-04-26 15:29:23 -0300) > > are available in the git repository at: > > git://linuxtv.org/hverkuil/media_tree.git fixes > > for you to fetch changes up to 71ea18d3e92d834926751f8460cf6893424b3852: > > radio-keene: support suspend/resume. (2012-04-27 09:57:02 +0200) > > > Hans Verkuil (2): > ivtv: set max/step to 0 for PTS and FRAME controls. > radio-keene: support suspend/resume. The ivtv change looks OK to me. Even though v4l2_ctrl_fill() fixes up those errors anyway. Reviewed-by: Andy Walls Regards, Andy > > drivers/media/radio/radio-keene.c | 20 > drivers/media/video/ivtv/ivtv-driver.c |4 ++-- > 2 files changed, 22 insertions(+), 2 deletions(-) -- 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.5] cpia2: major overhaul to get it in a working state again.
On Sun, 2012-04-29 at 13:57 +0200, Hans Verkuil wrote: > Hi all, > > I managed to get hold of a cpia2-based Hanse USB microscope almost a year > ago after reports from Andrea that the driver didn't work properly. > > I finally had time to take a really good look at the driver and it was > broken in many respects. This patch brings the driver up to speed with > respect to the v4l2 framework and it now works as expected, including > disconnect handling and suspend/resume handling. > > The only thing left to do some day is to convert it to the videobuf2 > framework. Hans, cpia2_s_ctrl() doesn't seem to have a case for V4L2_CID_ILLUMINATORS_2. Does control cluster magic take care of that? Regards, Andy > Regards, > > Hans > > The following changes since commit bcb2cf6e0bf033d79821c89e5ccb328bfbd44907: > > [media] ngene: remove an unneeded condition (2012-04-26 15:29:23 -0300) > > are available in the git repository at: > > git://linuxtv.org/hverkuil/media_tree.git cpia2 > > for you to fetch changes up to 14a3d232eb5d25c768f40fbd6a87db48a249a391: > > cpia2: major overhaul to get it in a working state again. (2012-04-29 > 13:44:44 +0200) > > > Hans Verkuil (1): > cpia2: major overhaul to get it in a working state again. > > drivers/media/video/cpia2/cpia2.h | 34 ++- > drivers/media/video/cpia2/cpia2_core.c | 142 +++- > drivers/media/video/cpia2/cpia2_usb.c | 78 +-- > drivers/media/video/cpia2/cpia2_v4l.c | 846 > + > drivers/media/video/cpia2/cpia2dev.h | 50 - > 5 files changed, 363 insertions(+), 787 deletions(-) > delete mode 100644 drivers/media/video/cpia2/cpia2dev.h > -- > 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: HVR-1600 QAM recordings with slight glitches in them
On Sun, Apr 29, 2012 at 4:27 PM, James Courtier-Dutton wrote: > There are two measurements. > 1) SNR. > This is a measure of the quality of the signal. Bigger is better. 30dB is a > good value. Spliters and amplifiers should only slightly reduce the SNR > value. 30 dB for ClearQAM is actually a very marginal SNR. It caps out at 40 dB, and as Andy pointed out, it's a logarithmic scale. On a good cable plant, you should expect an SNR between 35 and 40. Anything below 32 and you're very likely to have significant error rates. Don't confuse it with Over-the-Air ATSC, which will typically cap out at 30.0 dB. I don't know why you're not seeing valid data on femon with the 950q. It should be printing out fine, and it's on the same 0.1 dB scale. Try running just azap and see if the SNR is reported there. This indeed feels like a marginal signal condition problem, and Andy's assertion is well founded that the mxl5005/s5h1409 isn't exactly the best combo compared to more modern tuners and demodulators (Hauppauge switched to the tda18271 and s5h1411 for the newer revision of the HVR-1600). The Linux driver support should be on-par with Windows though in terms of performance as I did a bunch of work some time back to analyze the differences which resulted in some fixes. Cheers, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.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
RE: Using UVC webcam gadget with a real v4l2 device
Hi Laurent, > -Original Message- > From: Bhupesh SHARMA > Sent: Thursday, April 26, 2012 10:54 AM > To: 'Laurent Pinchart' > Cc: 'linux-...@vger.kernel.org'; 'linux-media@vger.kernel.org'; > 'ba...@ti.com'; 'g.liakhovet...@gmx.de' > Subject: RE: Using UVC webcam gadget with a real v4l2 device > > Hi Laurent, > > Sorry to jump-in before your reply on my previous mail, > but as I was studying the USERPTR stuff in more detail, I have a few > more > queries which I believe you can include in your reply as well.. > > > -Original Message- > > From: Bhupesh SHARMA > > Sent: Wednesday, April 25, 2012 8:37 PM > > To: 'Laurent Pinchart' > > Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org; > > ba...@ti.com; g.liakhovet...@gmx.de > > Subject: RE: Using UVC webcam gadget with a real v4l2 device > > > > Hi Laurent, > > > > > -Original Message- > > > From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] > > > Sent: Tuesday, April 24, 2012 2:26 AM > > > To: Bhupesh SHARMA > > > Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org; > > > ba...@ti.com; g.liakhovet...@gmx.de > > > Subject: Re: Using UVC webcam gadget with a real v4l2 device > > > > > > Hi Bhupesh, > > > > > > On Tuesday 24 April 2012 02:46:22 Bhupesh SHARMA wrote: > > > > On Monday, April 23, 2012 7:47 PM Laurent Pinchart wrote: > > > > > On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote: > > > > > > Hi Laurent, > > > > > > > > > > > > I have been doing some experimentation with the UVC webcam > > gadget > > > along > > > > > > with the UVC user-space application which you have written. > > > > > > > > > > > > The UVC webcam gadget works fine with the user space > > application > > > > > > handling the CONTROL events and providing DATA events. Now, I > > > wish to > > > > > > interface a real v4l2 device, for e.g. VIVI or more > > particularly > > > a > > > > > > soc_camera based host and subdev pair. > > > > > > > > > > > > Now, I see that I can achieve this by opening the UVC and > V4L2 > > > devices > > > > > > and doing MMAP -> REQBUF -> QBUF -> DQBUF calls on both the > > > devices per > > > > > > the UVC control event received. But this will involve copying > > the > > > video > > > > > > buffer in the user-space application from v4l2 (_CAPTURE) to > > uvc > > > > > > (_OUTPUT) domains, which will significantly reduce the video > > > capture > > > > > > performance. > > > > > > > > > > > > Is there a better solution to this issue? Maybe doing > something > > > like a > > > > > > RNDIS gadget does with the help of u_ether.c like helper > > > routines. But > > > > > > if I remember well it also requires the BRCTL (Bridge Control > > > Utility) > > > > > > in userspace to route data arriving on usb0 to eth0 and vice- > > > versa. Not > > > > > > sure though, if it does copying of a skb buffer from ethernet > > to > > > usb > > > > > > domain and vice-versa. > > > > > > > > > > To avoid copying data between the two devices you should use > > > USERPTR > > > > > instead of MMAP on at least one of the two V4L2 devices. The > UVC > > > gadget > > > > > driver doesn't support USERPTR yet though. This shouldn't be > too > > > difficult > > > > > to fix, we need toreplace the custom buffers queue > implementation > > > with > > > > > videobuf2, as has been done in the uvcvideo driver. > > > > > > > > I was thinking of using the USERPTR method too, but I realized > that > > > > currently neither UVC webcam gadget nor soc-camera subsystem > > supports > > > this > > > > IO method. They support only MMAP IO as of now :( > > > > > > Both soc-camera and the UVC gadget driver should be ported to > > videobuf2 > > > to fix > > > the problem. > > I am now a bit confused on how the entire system will work now: > - Does USERPTR method needs to be supported both in UVC gadget > and soc-camera side, > or one can still support the MMAP method and the other can now > be changed to support USERPTR method > and we can achieve a ZERO buffer copy operation using this > method? > > - More specifically, I would like to keep the soc-camera still > using MMAP (and hence still using video-buf) > and make changes at the UVC gadget side to support USERPTR and > videobuf2. Will this work? > > - At the application side how should we design the flow in case > both support USERPTR, i.e. the buffer needs > to be protected from simultaneous access from the UVC gadget > driver and soc-camera driver (to ensure that > a single buffer can be shared across them). Also in case we > keep soc-camera still using MMAP and UVC gadget > side supporting USERPTR, how can we share a common buffer > across the UVC gadget and soc-camera driver. > > - In case of USERPTR method the camera capture hardware should be > able to DMA the received data to the user > space buffers. Are there any specific requirements on the DMA > capability of these use-space buffers >
Re: [Linaro-mm-sig] [PATCHv24 00/16] Contiguous Memory Allocator
2012/4/3 Marek Szyprowski : > Hi, > > This is (yet another) update of CMA patches. I've rebased them onto > recent v3.4-rc1 kernel tree and integrated some minor bugfixes. The > first issue has been pointed by Sandeep Patil - alloc_contig_range > reclaimed two times too many pages, second issue (possible mismatch > between pageblock size and MAX_ORDER pages) has been recently spotted > by Michal Nazarewicz. > > These patches are also available on my git repository: > git://git.linaro.org/people/mszyprowski/linux-dma-mapping.git 3.4-rc1-cma-v24 > > Best regards > Marek Szyprowski > Samsung Poland R&D Center > > > > Patches in this patchset: Marek, how about the patch "mm: cma: add a simple kernel module as the helper to test CMA", did you forget merging this? http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/088412.html > > Marek Szyprowski (6): > mm: extract reclaim code from __alloc_pages_direct_reclaim() > mm: trigger page reclaim in alloc_contig_range() to stabilise >watermarks > drivers: add Contiguous Memory Allocator > X86: integrate CMA with DMA-mapping subsystem > ARM: integrate CMA with DMA-mapping subsystem > ARM: Samsung: use CMA for 2 memory banks for s5p-mfc device > > Mel Gorman (1): > mm: Serialize access to min_free_kbytes > > Michal Nazarewicz (9): > mm: page_alloc: remove trailing whitespace > mm: compaction: introduce isolate_migratepages_range() > mm: compaction: introduce map_pages() > mm: compaction: introduce isolate_freepages_range() > mm: compaction: export some of the functions > mm: page_alloc: introduce alloc_contig_range() > mm: page_alloc: change fallbacks array handling > mm: mmzone: MIGRATE_CMA migration type added > mm: page_isolation: MIGRATE_CMA isolation functions added > -barry -- 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.5] cpia2: major overhaul to get it in a working state again.
On Monday, April 30, 2012 01:51:20 Andy Walls wrote: > On Sun, 2012-04-29 at 13:57 +0200, Hans Verkuil wrote: > > Hi all, > > > > I managed to get hold of a cpia2-based Hanse USB microscope almost a year > > ago after reports from Andrea that the driver didn't work properly. > > > > I finally had time to take a really good look at the driver and it was > > broken in many respects. This patch brings the driver up to speed with > > respect to the v4l2 framework and it now works as expected, including > > disconnect handling and suspend/resume handling. > > > > The only thing left to do some day is to convert it to the videobuf2 > > framework. > > Hans, > > cpia2_s_ctrl() doesn't seem to have a case for V4L2_CID_ILLUMINATORS_2. > Does control cluster magic take care of that? Correct, the two illuminator controls are clustered so only the 'master control' (ILLUMINATORS_1) shows up in the switch. Regards, Hans > > Regards, > Andy > > > Regards, > > > > Hans > > > > The following changes since commit bcb2cf6e0bf033d79821c89e5ccb328bfbd44907: > > > > [media] ngene: remove an unneeded condition (2012-04-26 15:29:23 -0300) > > > > are available in the git repository at: > > > > git://linuxtv.org/hverkuil/media_tree.git cpia2 > > > > for you to fetch changes up to 14a3d232eb5d25c768f40fbd6a87db48a249a391: > > > > cpia2: major overhaul to get it in a working state again. (2012-04-29 > > 13:44:44 +0200) > > > > > > Hans Verkuil (1): > > cpia2: major overhaul to get it in a working state again. > > > > drivers/media/video/cpia2/cpia2.h | 34 ++- > > drivers/media/video/cpia2/cpia2_core.c | 142 +++- > > drivers/media/video/cpia2/cpia2_usb.c | 78 +-- > > drivers/media/video/cpia2/cpia2_v4l.c | 846 > > + > > drivers/media/video/cpia2/cpia2dev.h | 50 - > > 5 files changed, 363 insertions(+), 787 deletions(-) > > delete mode 100644 drivers/media/video/cpia2/cpia2dev.h > > -- > > 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 > -- 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.5] Control events support for uvcvideo
Hi Laurent, I know I am very late with this, but I looked through the event/control core changes and I found a locking bug there. I didn't have a chance to review the patch series when HdG posted it earlier this month, so my apologies for coming up with this only now. The problem is in v4l2-ctrls.c: diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c index 7023e6d..2a44355 100644 --- a/drivers/media/video/v4l2-ctrls.c +++ b/drivers/media/video/v4l2-ctrls.c @@ -2381,10 +2381,22 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val) } EXPORT_SYMBOL(v4l2_ctrl_s_ctrl); -void v4l2_ctrl_add_event(struct v4l2_ctrl *ctrl, - struct v4l2_subscribed_event *sev) +static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev) { - v4l2_ctrl_lock(ctrl); This lock... + struct v4l2_ctrl_handler *hdl = sev->fh->ctrl_handler; + struct v4l2_ctrl_ref *ref; + struct v4l2_ctrl *ctrl; + int ret = 0; + + mutex_lock(&hdl->lock); ...and this lock are not necessarily the same. If the control was from a subdevice then they will be different. This doesn't happen in uvc, but it will in other drivers. + + ref = find_ref(hdl, sev->id); The right approach is to use v4l2_ctrl_find here and then call v4l2_ctrl_lock(ctrl), just as was done in the original code. + if (!ref) { + ret = -EINVAL; + goto leave; + } + ctrl = ref->ctrl; + list_add_tail(&sev->node, &ctrl->ev_subs); if (ctrl->type != V4L2_CTRL_TYPE_CTRL_CLASS && (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL)) { @@ -2396,18 +2408,42 @@ void v4l2_ctrl_add_event(struct v4l2_ctrl *ctrl, fill_event(&ev, ctrl, changes); v4l2_event_queue_fh(sev->fh, &ev); } - v4l2_ctrl_unlock(ctrl); +leave: + mutex_unlock(&hdl->lock); + return ret; } -EXPORT_SYMBOL(v4l2_ctrl_add_event); -void v4l2_ctrl_del_event(struct v4l2_ctrl *ctrl, - struct v4l2_subscribed_event *sev) +static void v4l2_ctrl_del_event(struct v4l2_subscribed_event *sev) { - v4l2_ctrl_lock(ctrl); + struct v4l2_ctrl_handler *hdl = sev->fh->ctrl_handler; + + mutex_lock(&hdl->lock); Same problem here. list_del(&sev->node); - v4l2_ctrl_unlock(ctrl); + mutex_unlock(&hdl->lock); } -EXPORT_SYMBOL(v4l2_ctrl_del_event); So the code should be: static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev) { struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id); if (ctrl == NULL) return -EINVAL; v4l2_ctrl_lock(ctrl); list_add_tail(&sev->node, &ctrl->ev_subs); if (ctrl->type != V4L2_CTRL_TYPE_CTRL_CLASS && (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL)) { struct v4l2_event ev; u32 changes = V4L2_EVENT_CTRL_CH_FLAGS; if (!(ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY)) changes |= V4L2_EVENT_CTRL_CH_VALUE; fill_event(&ev, ctrl, changes); v4l2_event_queue_fh(sev->fh, &ev); } v4l2_ctrl_unlock(ctrl); return 0; } and: static void v4l2_ctrl_del_event(struct v4l2_subscribed_event *sev) { struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id); v4l2_ctrl_lock(ctrl); list_del(&sev->node); v4l2_ctrl_unlock(ctrl); } Regards, Hans On Sunday, April 29, 2012 19:59:01 Laurent Pinchart wrote: > Hi Mauro, > > The following changes since commit bcb2cf6e0bf033d79821c89e5ccb328bfbd44907: > > [media] ngene: remove an unneeded condition (2012-04-26 15:29:23 -0300) > > are available in the git repository at: > git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-events > > Hans de Goede (10): > media/radio: use v4l2_ctrl_subscribe_event where possible > v4l2-event: Add v4l2_subscribed_event_ops > v4l2-ctrls: Use v4l2_subscribed_event_ops > uvcvideo: Fix a "ignoring return value of ‘__clear_user’" warning > uvcvideo: Refactor uvc_ctrl_get and query > uvcvideo: Move __uvc_ctrl_get() up > uvcvideo: Add support for control events > uvcvideo: Properly report the inactive flag for inactive controls > uvcvideo: Send control change events for slave ctrls when the master > changes > uvcvideo: Drop unused ctrl member from struct uvc_control_mapping > > Documentation/video4linux/v4l2-framework.txt | 28 ++- > drivers/media/radio/radio-isa.c | 10 +- > drivers/media/radio/radio-keene.c| 14 +- > drivers/media/video/ivtv/ivtv-ioctl.c|3 +- > drivers/media/video/omap3isp/ispccdc.c |2 +- > drivers/media/video/omap3isp/ispstat.c |2 +- > drivers/media/video/uvc/uvc_ctrl.c | 320 + > drive