* [PATCH 0/4] Miscellaneous OMAP3 ISP fixes and enhancements @ 2012-04-23 11:29 Laurent Pinchart 2012-04-23 11:29 ` [PATCH 1/4] omap3isp: preview: Rename last occurences of *_rgb_to_ycbcr to *_csc Laurent Pinchart ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Laurent Pinchart @ 2012-04-23 11:29 UTC (permalink / raw) To: linux-media; +Cc: sakari.ailus Hi everybody, Here are 4 miscellaneous patches for the OMAP3 ISP driver. Laurent Pinchart (4): omap3isp: preview: Rename last occurences of *_rgb_to_ycbcr to *_csc omap3isp: preview: Add support for greyscale input omap3isp: ccdc: Add crop support on output formatter source pad omap3isp: Mark probe and cleanup functions with __devinit and __devexit drivers/media/video/omap3isp/isp.c | 6 +- drivers/media/video/omap3isp/ispccdc.c | 147 ++++++++++++++++++++++++++--- drivers/media/video/omap3isp/ispccdc.h | 2 + drivers/media/video/omap3isp/isppreview.c | 58 +++++++----- 4 files changed, 172 insertions(+), 41 deletions(-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] omap3isp: preview: Rename last occurences of *_rgb_to_ycbcr to *_csc 2012-04-23 11:29 [PATCH 0/4] Miscellaneous OMAP3 ISP fixes and enhancements Laurent Pinchart @ 2012-04-23 11:29 ` Laurent Pinchart 2012-04-23 11:29 ` [PATCH 2/4] omap3isp: preview: Add support for greyscale input Laurent Pinchart ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Laurent Pinchart @ 2012-04-23 11:29 UTC (permalink / raw) To: linux-media; +Cc: sakari.ailus For consistency reasons, rename the last remaining occurences of rgb_to_ycbcr to csc (color space conversion). Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/video/omap3isp/isppreview.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/video/omap3isp/isppreview.c b/drivers/media/video/omap3isp/isppreview.c index 5bdf9e7..f839cf8 100644 --- a/drivers/media/video/omap3isp/isppreview.c +++ b/drivers/media/video/omap3isp/isppreview.c @@ -608,12 +608,12 @@ preview_config_rgb_blending(struct isp_prev_device *prev, const void *rgb2rgb) } /* - * Configures the RGB-YCbYCr conversion matrix + * Configures the color space conversion (RGB toYCbYCr) matrix * @prev_csc: Structure containing the RGB to YCbYCr matrix and the * YCbCr offset. */ static void -preview_config_rgb_to_ycbcr(struct isp_prev_device *prev, const void *prev_csc) +preview_config_csc(struct isp_prev_device *prev, const void *prev_csc) { struct isp_device *isp = to_isp_device(prev); const struct omap3isp_prev_csc *csc = prev_csc; @@ -860,7 +860,7 @@ static const struct preview_update update_attrs[] = { FIELD_SIZEOF(struct prev_params, rgb2rgb), offsetof(struct omap3isp_prev_update_config, rgb2rgb), }, /* OMAP3ISP_PREV_COLOR_CONV */ { - preview_config_rgb_to_ycbcr, + preview_config_csc, NULL, offsetof(struct prev_params, csc), FIELD_SIZEOF(struct prev_params, csc), -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] omap3isp: preview: Add support for greyscale input 2012-04-23 11:29 [PATCH 0/4] Miscellaneous OMAP3 ISP fixes and enhancements Laurent Pinchart 2012-04-23 11:29 ` [PATCH 1/4] omap3isp: preview: Rename last occurences of *_rgb_to_ycbcr to *_csc Laurent Pinchart @ 2012-04-23 11:29 ` Laurent Pinchart 2012-04-24 23:19 ` Sakari Ailus 2012-04-23 11:29 ` [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad Laurent Pinchart 2012-04-23 11:29 ` [PATCH 4/4] omap3isp: Mark probe and cleanup functions with __devinit and __devexit Laurent Pinchart 3 siblings, 1 reply; 12+ messages in thread From: Laurent Pinchart @ 2012-04-23 11:29 UTC (permalink / raw) To: linux-media; +Cc: sakari.ailus Configure CFA interpolation automatically based on the input format. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/video/omap3isp/isppreview.c | 52 ++++++++++++++++------------ 1 files changed, 30 insertions(+), 22 deletions(-) diff --git a/drivers/media/video/omap3isp/isppreview.c b/drivers/media/video/omap3isp/isppreview.c index f839cf8..420b131 100644 --- a/drivers/media/video/omap3isp/isppreview.c +++ b/drivers/media/video/omap3isp/isppreview.c @@ -441,23 +441,6 @@ preview_enable_dcor(struct isp_prev_device *prev, u8 enable) } /* - * preview_enable_cfa - Enable/Disable the CFA Interpolation. - * @enable: 1 - Enables the CFA. - */ -static void -preview_enable_cfa(struct isp_prev_device *prev, u8 enable) -{ - struct isp_device *isp = to_isp_device(prev); - - if (enable) - isp_reg_set(isp, OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR, - ISPPRV_PCR_CFAEN); - else - isp_reg_clr(isp, OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR, - ISPPRV_PCR_CFAEN); -} - -/* * preview_enable_gammabypass - Enables/Disables the GammaByPass * @enable: 1 - Bypasses Gamma - 10bit input is cropped to 8MSB. * 0 - Goes through Gamma Correction. input and output is 10bit. @@ -831,7 +814,7 @@ static const struct preview_update update_attrs[] = { offsetof(struct omap3isp_prev_update_config, hmed), }, /* OMAP3ISP_PREV_CFA */ { preview_config_cfa, - preview_enable_cfa, + NULL, offsetof(struct prev_params, cfa), FIELD_SIZEOF(struct prev_params, cfa), offsetof(struct omap3isp_prev_update_config, cfa), @@ -1078,6 +1061,27 @@ static void preview_config_averager(struct isp_prev_device *prev, u8 average) } /* + * preview_config_input_format - Configure the input format + * @prev: The preview engine + * @format: Format on the preview engine sink pad + * + * Enable CFA interpolation for Bayer formats and disable it for greyscale + * formats. + */ +static void preview_config_input_format(struct isp_prev_device *prev, + const struct v4l2_mbus_framefmt *format) +{ + struct isp_device *isp = to_isp_device(prev); + + if (format->code != V4L2_MBUS_FMT_Y10_1X10) + isp_reg_set(isp, OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR, + ISPPRV_PCR_CFAEN); + else + isp_reg_clr(isp, OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR, + ISPPRV_PCR_CFAEN); +} + +/* * preview_config_input_size - Configure the input frame size * * The preview engine crops several rows and columns internally depending on @@ -1090,6 +1094,7 @@ static void preview_config_averager(struct isp_prev_device *prev, u8 average) */ static void preview_config_input_size(struct isp_prev_device *prev, u32 active) { + const struct v4l2_mbus_framefmt *format = &prev->formats[PREV_PAD_SINK]; struct isp_device *isp = to_isp_device(prev); unsigned int sph = prev->crop.left; unsigned int eph = prev->crop.left + prev->crop.width - 1; @@ -1097,15 +1102,16 @@ static void preview_config_input_size(struct isp_prev_device *prev, u32 active) unsigned int elv = prev->crop.top + prev->crop.height - 1; u32 features; - features = (prev->params.params[0].features & active) - | (prev->params.params[1].features & ~active); - - if (features & OMAP3ISP_PREV_CFA) { + if (format->code == V4L2_MBUS_FMT_Y10_1X10) { sph -= 2; eph += 2; slv -= 2; elv += 2; } + + features = (prev->params.params[0].features & active) + | (prev->params.params[1].features & ~active); + if (features & (OMAP3ISP_PREV_DEFECT_COR | OMAP3ISP_PREV_NF)) { sph -= 2; eph += 2; @@ -1436,6 +1442,7 @@ static void preview_configure(struct isp_prev_device *prev) preview_adjust_bandwidth(prev); + preview_config_input_format(prev, format); preview_config_input_size(prev, active); if (prev->input == PREVIEW_INPUT_CCDC) @@ -1723,6 +1730,7 @@ __preview_get_crop(struct isp_prev_device *prev, struct v4l2_subdev_fh *fh, /* previewer format descriptions */ static const unsigned int preview_input_fmts[] = { + V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_MBUS_FMT_SRGGB10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10, -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] omap3isp: preview: Add support for greyscale input 2012-04-23 11:29 ` [PATCH 2/4] omap3isp: preview: Add support for greyscale input Laurent Pinchart @ 2012-04-24 23:19 ` Sakari Ailus 0 siblings, 0 replies; 12+ messages in thread From: Sakari Ailus @ 2012-04-24 23:19 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media Hi Laurent, Thanks for the patch! On Mon, Apr 23, 2012 at 01:29:53PM +0200, Laurent Pinchart wrote: > Configure CFA interpolation automatically based on the input format. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/video/omap3isp/isppreview.c | 52 ++++++++++++++++------------ > 1 files changed, 30 insertions(+), 22 deletions(-) Acked-by: <sakari.ailus@iki.fi> (Same for 1/4, too!) It looks like the OMAP 3 ISP will be functionally complete soon! 8-) Regards, -- Sakari Ailus e-mail: sakari.ailus@iki.fi jabber/XMPP/Gmail: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad 2012-04-23 11:29 [PATCH 0/4] Miscellaneous OMAP3 ISP fixes and enhancements Laurent Pinchart 2012-04-23 11:29 ` [PATCH 1/4] omap3isp: preview: Rename last occurences of *_rgb_to_ycbcr to *_csc Laurent Pinchart 2012-04-23 11:29 ` [PATCH 2/4] omap3isp: preview: Add support for greyscale input Laurent Pinchart @ 2012-04-23 11:29 ` Laurent Pinchart 2012-04-23 22:23 ` Sakari Ailus 2012-04-23 11:29 ` [PATCH 4/4] omap3isp: Mark probe and cleanup functions with __devinit and __devexit Laurent Pinchart 3 siblings, 1 reply; 12+ messages in thread From: Laurent Pinchart @ 2012-04-23 11:29 UTC (permalink / raw) To: linux-media; +Cc: sakari.ailus Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/video/omap3isp/ispccdc.c | 147 +++++++++++++++++++++++++++++--- drivers/media/video/omap3isp/ispccdc.h | 2 + 2 files changed, 136 insertions(+), 13 deletions(-) diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c index 8d8d6f3..f7851fa 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_HEIGHT 32 + 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 - CCDC_MIN_HEIGHT); + + /* 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. + */ + max_width = (sink->width - crop->left + 15) & ~15; + crop->width = clamp_t(u32, crop->width, CCDC_MIN_WIDTH, max_width) + & ~15; + crop->height = clamp_t(u32, crop->height, CCDC_MIN_HEIGHT, + sink->height - crop->top); + + /* Odd width/height values don't make sense for Bayer formats. */ + if (info->flavor != V4L2_MBUS_FMT_Y8_1X8) { + crop->width &= ~1; + crop->height &= ~1; + } +} + +/* * ccdc_enum_mbus_code - Handle pixel format enumeration * @sd : pointer to v4l2 subdev structure * @fh : V4L2 subdev file handle @@ -1940,6 +1994,60 @@ static int ccdc_enum_frame_size(struct v4l2_subdev *sd, } /* + * ccdc_get_crop - Retrieve the crop rectangle on a pad + * @sd: ISP CCDC V4L2 subdevice + * @fh: V4L2 subdev file handle + * @crop: crop rectangle + * + * Return 0 on success or a negative error code otherwise. + */ +static int ccdc_get_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_crop *crop) +{ + struct isp_ccdc_device *ccdc = v4l2_get_subdevdata(sd); + + /* Cropping is only supported on the output formatter source pad. */ + if (crop->pad != CCDC_PAD_SOURCE_OF) + return -EINVAL; + + crop->rect = *__ccdc_get_crop(ccdc, fh, crop->which); + return 0; +} + +/* + * ccdc_set_crop - Retrieve the crop rectangle on a pad + * @sd: ISP CCDC V4L2 subdevice + * @fh: V4L2 subdev file handle + * @crop: crop rectangle + * + * Return 0 on success or a negative error code otherwise. + */ +static int ccdc_set_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_crop *crop) +{ + struct isp_ccdc_device *ccdc = v4l2_get_subdevdata(sd); + struct v4l2_mbus_framefmt *format; + + /* Cropping is only supported on the output formatter source pad. */ + if (crop->pad != CCDC_PAD_SOURCE_OF) + return -EINVAL; + + /* The crop rectangle can't be changed while streaming. */ + if (ccdc->state != ISP_PIPELINE_STREAM_STOPPED) + return -EBUSY; + + format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SINK, crop->which); + ccdc_try_crop(ccdc, format, &crop->rect); + *__ccdc_get_crop(ccdc, fh, crop->which) = crop->rect; + + /* Update the source format. */ + format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SOURCE_OF, crop->which); + ccdc_try_format(ccdc, fh, CCDC_PAD_SOURCE_OF, format, crop->which); + + return 0; +} + +/* * ccdc_get_format - Retrieve the video format on a pad * @sd : ISP CCDC V4L2 subdevice * @fh : V4L2 subdev file handle @@ -1976,6 +2084,7 @@ static int ccdc_set_format(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, { struct isp_ccdc_device *ccdc = v4l2_get_subdevdata(sd); struct v4l2_mbus_framefmt *format; + struct v4l2_rect *crop; format = __ccdc_get_format(ccdc, fh, fmt->pad, fmt->which); if (format == NULL) @@ -1986,6 +2095,16 @@ static int ccdc_set_format(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, /* Propagate the format from sink to source */ if (fmt->pad == CCDC_PAD_SINK) { + /* Reset the crop rectangle. */ + crop = __ccdc_get_crop(ccdc, fh, fmt->which); + crop->left = 0; + crop->top = 0; + crop->width = fmt->format.width; + crop->height = fmt->format.height; + + ccdc_try_crop(ccdc, &fmt->format, crop); + + /* Update the source formats. */ format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SOURCE_OF, fmt->which); *format = fmt->format; @@ -2044,6 +2163,8 @@ static const struct v4l2_subdev_pad_ops ccdc_v4l2_pad_ops = { .enum_frame_size = ccdc_enum_frame_size, .get_fmt = ccdc_get_format, .set_fmt = ccdc_set_format, + .get_crop = ccdc_get_crop, + .set_crop = ccdc_set_crop, }; /* V4L2 subdev operations */ diff --git a/drivers/media/video/omap3isp/ispccdc.h b/drivers/media/video/omap3isp/ispccdc.h index 6d0264b..966bbf8 100644 --- a/drivers/media/video/omap3isp/ispccdc.h +++ b/drivers/media/video/omap3isp/ispccdc.h @@ -147,6 +147,7 @@ struct ispccdc_lsc { * @subdev: V4L2 subdevice * @pads: Sink and source media entity pads * @formats: Active video formats + * @crop: Active crop rectangle on the OF source pad * @input: Active input * @output: Active outputs * @video_out: Output video node @@ -173,6 +174,7 @@ struct isp_ccdc_device { struct v4l2_subdev subdev; struct media_pad pads[CCDC_PADS_NUM]; struct v4l2_mbus_framefmt formats[CCDC_PADS_NUM]; + struct v4l2_rect crop; enum ccdc_input_entity input; unsigned int output; -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad 2012-04-23 11:29 ` [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad Laurent Pinchart @ 2012-04-23 22:23 ` Sakari Ailus 2012-04-24 9:08 ` Laurent Pinchart 0 siblings, 1 reply; 12+ messages in thread From: Sakari Ailus @ 2012-04-23 22:23 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media Hi Laurent, The patch looks good as such on the first glance, but I have another question: why are you not using the selections API instead? It's in Mauro's tree already. Also, the old S_CROP IOCTL only has been defined for sink pads, not source. Cheers, -- Sakari Ailus sakari.ailus@iki.fi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad 2012-04-23 22:23 ` Sakari Ailus @ 2012-04-24 9:08 ` Laurent Pinchart 2012-04-24 15:14 ` Sakari Ailus 0 siblings, 1 reply; 12+ messages in thread From: Laurent Pinchart @ 2012-04-24 9:08 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media Hi Sakari, On Tuesday 24 April 2012 01:23:06 Sakari Ailus wrote: > Hi Laurent, > > The patch looks good as such on the first glance, but I have another > question: why are you not using the selections API instead? It's in > Mauro's tree already. You're totally right, we need to convert the selection API. The reason why I've implemented crop support at the CCDC output was simply that I needed it for a project and didn't have time to implement the selection API. As the code works, I considered it would be good to have it upstream until we switch to the selection API. > Also, the old S_CROP IOCTL only has been defined for sink pads, not source. We're already using crop on source pads on sensors ;-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad 2012-04-24 9:08 ` Laurent Pinchart @ 2012-04-24 15:14 ` Sakari Ailus 2012-04-24 20:10 ` Laurent Pinchart 0 siblings, 1 reply; 12+ messages in thread From: Sakari Ailus @ 2012-04-24 15:14 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media Hi Laurent, On Tue, Apr 24, 2012 at 11:08:12AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 24 April 2012 01:23:06 Sakari Ailus wrote: > > Hi Laurent, > > > > The patch looks good as such on the first glance, but I have another > > question: why are you not using the selections API instead? It's in > > Mauro's tree already. > > You're totally right, we need to convert the selection API. The reason why > I've implemented crop support at the CCDC output was simply that I needed it > for a project and didn't have time to implement the selection API. As the code > works, I considered it would be good to have it upstream until we switch to > the selection API. "Until we switch to the selection API"? The subdev selection API is in Mauro's tree already so I see no reason not to use it. Implementing new functionality in a driver using API we've just marked obsolete is... not pretty. The compatibility code for the old crop ioctls exist, too, so you get exactly the same functionality as well. > > Also, the old S_CROP IOCTL only has been defined for sink pads, not source. > > We're already using crop on source pads on sensors ;-) Is that supposed to work? At the very least least it does not follow the spec. Kind regards, -- Sakari Ailus e-mail: sakari.ailus@iki.fi jabber/XMPP/Gmail: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad 2012-04-24 15:14 ` Sakari Ailus @ 2012-04-24 20:10 ` Laurent Pinchart 2012-04-24 20:46 ` Sakari Ailus 0 siblings, 1 reply; 12+ messages in thread From: Laurent Pinchart @ 2012-04-24 20:10 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media Hi Sakari, On Tuesday 24 April 2012 18:14:20 Sakari Ailus wrote: > On Tue, Apr 24, 2012 at 11:08:12AM +0200, Laurent Pinchart wrote: > > On Tuesday 24 April 2012 01:23:06 Sakari Ailus wrote: > > > Hi Laurent, > > > > > > The patch looks good as such on the first glance, but I have another > > > question: why are you not using the selections API instead? It's in > > > Mauro's tree already. > > > > You're totally right, we need to convert the selection API. The reason why > > I've implemented crop support at the CCDC output was simply that I needed > > it for a project and didn't have time to implement the selection API. As > > the code works, I considered it would be good to have it upstream until > > we switch to the selection API. > > "Until we switch to the selection API"? The subdev selection API is in > Mauro's tree already so I see no reason not to use it. Implementing new > functionality in a driver using API we've just marked obsolete is... not > pretty. You're of course totally right. I've pushed back on enough attemps similar to this one to know that I will have to give up here and implement selection support :-) > The compatibility code for the old crop ioctls exist, too, so you get > exactly the same functionality as well. > > > > Also, the old S_CROP IOCTL only has been defined for sink pads, not > > > source. > > > > We're already using crop on source pads on sensors ;-) > > Is that supposed to work? At the very least least it does not follow the > spec. It doesn't follow the spec. But it works :-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad 2012-04-24 20:10 ` Laurent Pinchart @ 2012-04-24 20:46 ` Sakari Ailus 0 siblings, 0 replies; 12+ messages in thread From: Sakari Ailus @ 2012-04-24 20:46 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 24 April 2012 18:14:20 Sakari Ailus wrote: >> On Tue, Apr 24, 2012 at 11:08:12AM +0200, Laurent Pinchart wrote: >>> On Tuesday 24 April 2012 01:23:06 Sakari Ailus wrote: >>>> Hi Laurent, >>>> >>>> The patch looks good as such on the first glance, but I have another >>>> question: why are you not using the selections API instead? It's in >>>> Mauro's tree already. >>> >>> You're totally right, we need to convert the selection API. The reason why >>> I've implemented crop support at the CCDC output was simply that I needed >>> it for a project and didn't have time to implement the selection API. As >>> the code works, I considered it would be good to have it upstream until >>> we switch to the selection API. >> >> "Until we switch to the selection API"? The subdev selection API is in >> Mauro's tree already so I see no reason not to use it. Implementing new >> functionality in a driver using API we've just marked obsolete is... not >> pretty. > > You're of course totally right. I've pushed back on enough attemps similar to > this one to know that I will have to give up here and implement selection > support :-) Thanks! :-) You should not need to make many changes to your existing patch, I believe. The additional functionality you should support is the bounds rectangle; other than that, it's primarily just renaming things. >> The compatibility code for the old crop ioctls exist, too, so you get >> exactly the same functionality as well. >> >>>> Also, the old S_CROP IOCTL only has been defined for sink pads, not >>>> source. >>> >>> We're already using crop on source pads on sensors ;-) >> >> Is that supposed to work? At the very least least it does not follow the >> spec. > > It doesn't follow the spec. But it works :-) One of the reasons we implemented selections API was to get proper support for source pad crop. Well, at least it wouldn't work _with_ scaling in the same subdev. Cheers, -- Sakari Ailus sakari.ailus@iki.fi ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] omap3isp: Mark probe and cleanup functions with __devinit and __devexit 2012-04-23 11:29 [PATCH 0/4] Miscellaneous OMAP3 ISP fixes and enhancements Laurent Pinchart ` (2 preceding siblings ...) 2012-04-23 11:29 ` [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad Laurent Pinchart @ 2012-04-23 11:29 ` Laurent Pinchart 2012-04-24 23:05 ` Sakari Ailus 3 siblings, 1 reply; 12+ messages in thread From: Laurent Pinchart @ 2012-04-23 11:29 UTC (permalink / raw) To: linux-media; +Cc: sakari.ailus Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/video/omap3isp/isp.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c index df6416c..0307ac3 100644 --- a/drivers/media/video/omap3isp/isp.c +++ b/drivers/media/video/omap3isp/isp.c @@ -1978,7 +1978,7 @@ error_csiphy: * * Always returns 0. */ -static int isp_remove(struct platform_device *pdev) +static int __devexit isp_remove(struct platform_device *pdev) { struct isp_device *isp = platform_get_drvdata(pdev); int i; @@ -2059,7 +2059,7 @@ static int isp_map_mem_resource(struct platform_device *pdev, * -EINVAL if couldn't install ISR, * or clk_get return error value. */ -static int isp_probe(struct platform_device *pdev) +static int __devinit isp_probe(struct platform_device *pdev) { struct isp_platform_data *pdata = pdev->dev.platform_data; struct isp_device *isp; @@ -2227,7 +2227,7 @@ MODULE_DEVICE_TABLE(platform, omap3isp_id_table); static struct platform_driver omap3isp_driver = { .probe = isp_probe, - .remove = isp_remove, + .remove = __devexit_p(isp_remove), .id_table = omap3isp_id_table, .driver = { .owner = THIS_MODULE, -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] omap3isp: Mark probe and cleanup functions with __devinit and __devexit 2012-04-23 11:29 ` [PATCH 4/4] omap3isp: Mark probe and cleanup functions with __devinit and __devexit Laurent Pinchart @ 2012-04-24 23:05 ` Sakari Ailus 0 siblings, 0 replies; 12+ messages in thread From: Sakari Ailus @ 2012-04-24 23:05 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media On Mon, Apr 23, 2012 at 01:29:55PM +0200, Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/video/omap3isp/isp.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) Acked-by: Sakari Ailus <sakari.ailus@iki.fi> -- Sakari Ailus e-mail: sakari.ailus@iki.fi jabber/XMPP/Gmail: sailus@retiisi.org.uk ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-04-24 23:19 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-23 11:29 [PATCH 0/4] Miscellaneous OMAP3 ISP fixes and enhancements Laurent Pinchart 2012-04-23 11:29 ` [PATCH 1/4] omap3isp: preview: Rename last occurences of *_rgb_to_ycbcr to *_csc Laurent Pinchart 2012-04-23 11:29 ` [PATCH 2/4] omap3isp: preview: Add support for greyscale input Laurent Pinchart 2012-04-24 23:19 ` Sakari Ailus 2012-04-23 11:29 ` [PATCH 3/4] omap3isp: ccdc: Add crop support on output formatter source pad Laurent Pinchart 2012-04-23 22:23 ` Sakari Ailus 2012-04-24 9:08 ` Laurent Pinchart 2012-04-24 15:14 ` Sakari Ailus 2012-04-24 20:10 ` Laurent Pinchart 2012-04-24 20:46 ` Sakari Ailus 2012-04-23 11:29 ` [PATCH 4/4] omap3isp: Mark probe and cleanup functions with __devinit and __devexit Laurent Pinchart 2012-04-24 23:05 ` Sakari Ailus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.