[PATCH 1/2] soc-camera: separate S_FMT and S_CROP operations

[PATCH 1/2] soc-camera: separate S_FMT and S_CROP operations

[Guennadi Liakhovetski]

As host and camera drivers become more complex, differences between S_FMT and
S_CROP functionality grow, this patch separates them.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mt9m001.c              |   19 +++-
 drivers/media/video/mt9m111.c              |   56 +++++++---
 drivers/media/video/mt9t031.c              |   84 ++++++++++------
 drivers/media/video/mt9v022.c              |   62 +++++++----
 drivers/media/video/mx3_camera.c           |  157 +++++++++++++++++-----------
 drivers/media/video/ov772x.c               |   31 +++++-
 drivers/media/video/pxa_camera.c           |   67 +++++++++---
 drivers/media/video/sh_mobile_ceu_camera.c |   17 ++-
 drivers/media/video/soc_camera.c           |   20 ++--
 drivers/media/video/soc_camera_platform.c  |    9 ++-
 drivers/media/video/tw9910.c               |   20 +++-
 include/media/soc_camera.h                 |    6 +-
 12 files changed, 369 insertions(+), 179 deletions(-)

diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
index 2d1034d..a6703d2 100644
--- a/drivers/media/video/mt9m001.c
+++ b/drivers/media/video/mt9m001.c
@@ -284,8 +284,8 @@ static unsigned long mt9m001_query_bus_param(struct soc_camera_device *icd)
 	return soc_camera_apply_sensor_flags(icl, flags);
 }
 
-static int mt9m001_set_fmt(struct soc_camera_device *icd,
-			   __u32 pixfmt, struct v4l2_rect *rect)
+static int mt9m001_set_crop(struct soc_camera_device *icd,
+			    struct v4l2_rect *rect)
 {
 	struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
 	int ret;
@@ -324,6 +324,20 @@ static int mt9m001_set_fmt(struct soc_camera_device *icd,
 	return ret;
 }
 
+static int mt9m001_set_fmt(struct soc_camera_device *icd,
+			   struct v4l2_format *f)
+{
+	struct v4l2_rect rect = {
+		.left	= icd->x_current,
+		.top	= icd->y_current,
+		.width	= f->fmt.pix.width,
+		.height	= f->fmt.pix.height,
+	};
+
+	/* No support for scaling so far, just crop. TODO: use skipping */
+	return mt9m001_set_crop(icd, &rect);
+}
+
 static int mt9m001_try_fmt(struct soc_camera_device *icd,
 			   struct v4l2_format *f)
 {
@@ -449,6 +463,7 @@ static struct soc_camera_ops mt9m001_ops = {
 	.release		= mt9m001_release,
 	.start_capture		= mt9m001_start_capture,
 	.stop_capture		= mt9m001_stop_capture,
+	.set_crop		= mt9m001_set_crop,
 	.set_fmt		= mt9m001_set_fmt,
 	.try_fmt		= mt9m001_try_fmt,
 	.set_bus_param		= mt9m001_set_bus_param,
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 7e6be36..cdd1ddb 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -152,7 +152,7 @@ struct mt9m111 {
 	struct soc_camera_device icd;
 	int model;	/* V4L2_IDENT_MT9M11x* codes from v4l2-chip-ident.h */
 	enum mt9m111_context context;
-	unsigned int left, top, width, height;
+	struct v4l2_rect rect;
 	u32 pixfmt;
 	unsigned char autoexposure;
 	unsigned char datawidth;
@@ -249,12 +249,13 @@ static int mt9m111_set_context(struct soc_camera_device *icd,
 		return reg_write(CONTEXT_CONTROL, valA);
 }
 
-static int mt9m111_setup_rect(struct soc_camera_device *icd)
+static int mt9m111_setup_rect(struct soc_camera_device *icd,
+			      struct v4l2_rect *rect)
 {
 	struct mt9m111 *mt9m111 = container_of(icd, struct mt9m111, icd);
 	int ret, is_raw_format;
-	int width = mt9m111->width;
-	int height = mt9m111->height;
+	int width = rect->width;
+	int height = rect->height;
 
 	if ((mt9m111->pixfmt == V4L2_PIX_FMT_SBGGR8)
 	    || (mt9m111->pixfmt == V4L2_PIX_FMT_SBGGR16))
@@ -262,9 +263,9 @@ static int mt9m111_setup_rect(struct soc_camera_device *icd)
 	else
 		is_raw_format = 0;
 
-	ret = reg_write(COLUMN_START, mt9m111->left);
+	ret = reg_write(COLUMN_START, rect->left);
 	if (!ret)
-		ret = reg_write(ROW_START, mt9m111->top);
+		ret = reg_write(ROW_START, rect->top);
 
 	if (is_raw_format) {
 		if (!ret)
@@ -436,6 +437,22 @@ static int mt9m111_set_bus_param(struct soc_camera_device *icd, unsigned long f)
 	return 0;
 }
 
+static int mt9m111_set_crop(struct soc_camera_device *icd,
+			    struct v4l2_rect *rect)
+{
+	struct mt9m111 *mt9m111 = container_of(icd, struct mt9m111, icd);
+	int ret;
+
+	dev_dbg(&icd->dev, "%s left=%d, top=%d, width=%d, height=%d\n",
+		__func__, rect->left, rect->top, rect->width,
+		rect->height);
+
+	ret = mt9m111_setup_rect(icd, rect);
+	if (!ret)
+		mt9m111->rect = *rect;
+	return ret;
+}
+
 static int mt9m111_set_pixfmt(struct soc_camera_device *icd, u32 pixfmt)
 {
 	struct mt9m111 *mt9m111 = container_of(icd, struct mt9m111, icd);
@@ -486,23 +503,27 @@ static int mt9m111_set_pixfmt(struct soc_camera_device *icd, u32 pixfmt)
 }
 
 static int mt9m111_set_fmt(struct soc_camera_device *icd,
-			   __u32 pixfmt, struct v4l2_rect *rect)
+			   struct v4l2_format *f)
 {
 	struct mt9m111 *mt9m111 = container_of(icd, struct mt9m111, icd);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	struct v4l2_rect rect = {
+		.left	= mt9m111->rect.left,
+		.top	= mt9m111->rect.top,
+		.width	= pix->width,
+		.height	= pix->height,
+	};
 	int ret;
 
-	mt9m111->left = rect->left;
-	mt9m111->top = rect->top;
-	mt9m111->width = rect->width;
-	mt9m111->height = rect->height;
-
 	dev_dbg(&icd->dev, "%s fmt=%x left=%d, top=%d, width=%d, height=%d\n",
-		__func__, pixfmt, mt9m111->left, mt9m111->top, mt9m111->width,
-		mt9m111->height);
+		__func__, pix->pixelformat, rect.left, rect.top, rect.width,
+		rect.height);
 
-	ret = mt9m111_setup_rect(icd);
+	ret = mt9m111_setup_rect(icd, &rect);
+	if (!ret)
+		ret = mt9m111_set_pixfmt(icd, pix->pixelformat);
 	if (!ret)
-		ret = mt9m111_set_pixfmt(icd, pixfmt);
+		mt9m111->rect = rect;
 	return ret;
 }
 
@@ -633,6 +654,7 @@ static struct soc_camera_ops mt9m111_ops = {
 	.release		= mt9m111_release,
 	.start_capture		= mt9m111_start_capture,
 	.stop_capture		= mt9m111_stop_capture,
+	.set_crop		= mt9m111_set_crop,
 	.set_fmt		= mt9m111_set_fmt,
 	.try_fmt		= mt9m111_try_fmt,
 	.query_bus_param	= mt9m111_query_bus_param,
@@ -817,7 +839,7 @@ static int mt9m111_restore_state(struct soc_camera_device *icd)
 
 	mt9m111_set_context(icd, mt9m111->context);
 	mt9m111_set_pixfmt(icd, mt9m111->pixfmt);
-	mt9m111_setup_rect(icd);
+	mt9m111_setup_rect(icd, &mt9m111->rect);
 	mt9m111_set_flip(icd, mt9m111->hflip, MT9M111_RMB_MIRROR_COLS);
 	mt9m111_set_flip(icd, mt9m111->vflip, MT9M111_RMB_MIRROR_ROWS);
 	mt9m111_set_global_gain(icd, icd->gain);
diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c
index acc1fa9..677be18 100644
--- a/drivers/media/video/mt9t031.c
+++ b/drivers/media/video/mt9t031.c
@@ -213,36 +213,14 @@ static void recalculate_limits(struct soc_camera_device *icd,
 	icd->height_max = MT9T031_MAX_HEIGHT / yskip;
 }
 
-static int mt9t031_set_fmt(struct soc_camera_device *icd,
-			   __u32 pixfmt, struct v4l2_rect *rect)
+static int mt9t031_set_params(struct soc_camera_device *icd,
+			      struct v4l2_rect *rect, u16 xskip, u16 yskip)
 {
 	struct mt9t031 *mt9t031 = container_of(icd, struct mt9t031, icd);
 	int ret;
+	u16 xbin, ybin, width, height, left, top;
 	const u16 hblank = MT9T031_HORIZONTAL_BLANK,
 		vblank = MT9T031_VERTICAL_BLANK;
-	u16 xbin, xskip, ybin, yskip, width, height, left, top;
-
-	if (pixfmt) {
-		/*
-		 * try_fmt has put rectangle within limits.
-		 * S_FMT - use binning and skipping for scaling, recalculate
-		 * limits, used for cropping
-		 */
-		/* Is this more optimal than just a division? */
-		for (xskip = 8; xskip > 1; xskip--)
-			if (rect->width * xskip <= MT9T031_MAX_WIDTH)
-				break;
-
-		for (yskip = 8; yskip > 1; yskip--)
-			if (rect->height * yskip <= MT9T031_MAX_HEIGHT)
-				break;
-
-		recalculate_limits(icd, xskip, yskip);
-	} else {
-		/* CROP - no change in scaling, or in limits */
-		xskip = mt9t031->xskip;
-		yskip = mt9t031->yskip;
-	}
 
 	/* Make sure we don't exceed sensor limits */
 	if (rect->left + rect->width > icd->width_max)
@@ -289,7 +267,7 @@ static int mt9t031_set_fmt(struct soc_camera_device *icd,
 	if (ret >= 0)
 		ret = reg_write(icd, MT9T031_VERTICAL_BLANKING, vblank);
 
-	if (pixfmt) {
+	if (yskip != mt9t031->yskip || xskip != mt9t031->xskip) {
 		/* Binning, skipping */
 		if (ret >= 0)
 			ret = reg_write(icd, MT9T031_COLUMN_ADDRESS_MODE,
@@ -325,15 +303,58 @@ static int mt9t031_set_fmt(struct soc_camera_device *icd,
 		}
 	}
 
-	if (!ret && pixfmt) {
+	/* Re-enable register update, commit all changes */
+	if (ret >= 0)
+		ret = reg_clear(icd, MT9T031_OUTPUT_CONTROL, 1);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int mt9t031_set_crop(struct soc_camera_device *icd,
+			    struct v4l2_rect *rect)
+{
+	struct mt9t031 *mt9t031 = container_of(icd, struct mt9t031, icd);
+
+	/* CROP - no change in scaling, or in limits */
+	return mt9t031_set_params(icd, rect, mt9t031->xskip, mt9t031->yskip);
+}
+
+static int mt9t031_set_fmt(struct soc_camera_device *icd,
+			   struct v4l2_format *f)
+{
+	struct mt9t031 *mt9t031 = container_of(icd, struct mt9t031, icd);
+	int ret;
+	u16 xskip, yskip;
+	struct v4l2_rect rect = {
+		.left	= icd->x_current,
+		.top	= icd->y_current,
+		.width	= f->fmt.pix.width,
+		.height	= f->fmt.pix.height,
+	};
+
+	/*
+	 * try_fmt has put rectangle within limits.
+	 * S_FMT - use binning and skipping for scaling, recalculate
+	 * limits, used for cropping
+	 */
+	/* Is this more optimal than just a division? */
+	for (xskip = 8; xskip > 1; xskip--)
+		if (rect.width * xskip <= MT9T031_MAX_WIDTH)
+			break;
+
+	for (yskip = 8; yskip > 1; yskip--)
+		if (rect.height * yskip <= MT9T031_MAX_HEIGHT)
+			break;
+
+	recalculate_limits(icd, xskip, yskip);
+
+	ret = mt9t031_set_params(icd, &rect, xskip, yskip);
+	if (!ret) {
 		mt9t031->xskip = xskip;
 		mt9t031->yskip = yskip;
 	}
 
-	/* Re-enable register update, commit all changes */
-	reg_clear(icd, MT9T031_OUTPUT_CONTROL, 1);
-
-	return ret < 0 ? ret : 0;
+	return ret;
 }
 
 static int mt9t031_try_fmt(struct soc_camera_device *icd,
@@ -470,6 +491,7 @@ static struct soc_camera_ops mt9t031_ops = {
 	.release		= mt9t031_release,
 	.start_capture		= mt9t031_start_capture,
 	.stop_capture		= mt9t031_stop_capture,
+	.set_crop		= mt9t031_set_crop,
 	.set_fmt		= mt9t031_set_fmt,
 	.try_fmt		= mt9t031_try_fmt,
 	.set_bus_param		= mt9t031_set_bus_param,
diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 6eb4b3a..3871d4a 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -340,32 +340,11 @@ static unsigned long mt9v022_query_bus_param(struct soc_camera_device *icd)
 		width_flag;
 }
 
-static int mt9v022_set_fmt(struct soc_camera_device *icd,
-			   __u32 pixfmt, struct v4l2_rect *rect)
+static int mt9v022_set_crop(struct soc_camera_device *icd,
+			    struct v4l2_rect *rect)
 {
-	struct mt9v022 *mt9v022 = container_of(icd, struct mt9v022, icd);
 	int ret;
 
-	/* The caller provides a supported format, as verified per call to
-	 * icd->try_fmt(), datawidth is from our supported format list */
-	switch (pixfmt) {
-	case V4L2_PIX_FMT_GREY:
-	case V4L2_PIX_FMT_Y16:
-		if (mt9v022->model != V4L2_IDENT_MT9V022IX7ATM)
-			return -EINVAL;
-		break;
-	case V4L2_PIX_FMT_SBGGR8:
-	case V4L2_PIX_FMT_SBGGR16:
-		if (mt9v022->model != V4L2_IDENT_MT9V022IX7ATC)
-			return -EINVAL;
-		break;
-	case 0:
-		/* No format change, only geometry */
-		break;
-	default:
-		return -EINVAL;
-	}
-
 	/* Like in example app. Contradicts the datasheet though */
 	ret = reg_read(icd, MT9V022_AEC_AGC_ENABLE);
 	if (ret >= 0) {
@@ -403,6 +382,42 @@ static int mt9v022_set_fmt(struct soc_camera_device *icd,
 	return 0;
 }
 
+static int mt9v022_set_fmt(struct soc_camera_device *icd,
+			   struct v4l2_format *f)
+{
+	struct mt9v022 *mt9v022 = container_of(icd, struct mt9v022, icd);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	struct v4l2_rect rect = {
+		.left	= icd->x_current,
+		.top	= icd->y_current,
+		.width	= pix->width,
+		.height	= pix->height,
+	};
+
+	/* The caller provides a supported format, as verified per call to
+	 * icd->try_fmt(), datawidth is from our supported format list */
+	switch (pix->pixelformat) {
+	case V4L2_PIX_FMT_GREY:
+	case V4L2_PIX_FMT_Y16:
+		if (mt9v022->model != V4L2_IDENT_MT9V022IX7ATM)
+			return -EINVAL;
+		break;
+	case V4L2_PIX_FMT_SBGGR8:
+	case V4L2_PIX_FMT_SBGGR16:
+		if (mt9v022->model != V4L2_IDENT_MT9V022IX7ATC)
+			return -EINVAL;
+		break;
+	case 0:
+		/* No format change, only geometry */
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* No support for scaling on this camera, just crop. */
+	return mt9v022_set_crop(icd, &rect);
+}
+
 static int mt9v022_try_fmt(struct soc_camera_device *icd,
 			   struct v4l2_format *f)
 {
@@ -544,6 +559,7 @@ static struct soc_camera_ops mt9v022_ops = {
 	.release		= mt9v022_release,
 	.start_capture		= mt9v022_start_capture,
 	.stop_capture		= mt9v022_stop_capture,
+	.set_crop		= mt9v022_set_crop,
 	.set_fmt		= mt9v022_set_fmt,
 	.try_fmt		= mt9v022_try_fmt,
 	.set_bus_param		= mt9v022_set_bus_param,
diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index a925d09..70629e1 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -544,16 +544,14 @@ static void mx3_camera_remove_device(struct soc_camera_device *icd)
 }
 
 static bool channel_change_requested(struct soc_camera_device *icd,
-				     const struct soc_camera_format_xlate *xlate,
-				     __u32 pixfmt, struct v4l2_rect *rect)
+				     struct v4l2_rect *rect)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
 	struct idmac_channel *ichan = mx3_cam->idmac_channel[0];
 
-	/* So far only one configuration is supported */
-	return pixfmt || (ichan && rect->width * rect->height > 
-			  icd->width * icd->height);
+	/* Do buffers have to be re-allocated or channel re-configured? */
+	return ichan && rect->width * rect->height > icd->width * icd->height;
 }
 
 static int test_platform_param(struct mx3_camera_dev *mx3_cam,
@@ -733,61 +731,10 @@ passthrough:
 	return formats;
 }
 
-static int mx3_camera_set_fmt(struct soc_camera_device *icd,
-			      __u32 pixfmt, struct v4l2_rect *rect)
+static void configure_geometry(struct mx3_camera_dev *mx3_cam,
+			       struct v4l2_rect *rect)
 {
-	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
-	struct mx3_camera_dev *mx3_cam = ici->priv;
-	const struct soc_camera_format_xlate *xlate;
 	u32 ctrl, width_field, height_field;
-	int ret;
-
-	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
-	if (pixfmt && !xlate) {
-		dev_warn(&ici->dev, "Format %x not found\n", pixfmt);
-		return -EINVAL;
-	}
-
-	/*
-	 * We now know pixel formats and can decide upon DMA-channel(s)
-	 * So far only direct camera-to-memory is supported
-	 */
-	if (channel_change_requested(icd, xlate, pixfmt, rect)) {
-		dma_cap_mask_t mask;
-		struct dma_chan *chan;
-		struct idmac_channel **ichan = &mx3_cam->idmac_channel[0];
-		/* We have to use IDMAC_IC_7 for Bayer / generic data */
-		struct dma_chan_request rq = {.mx3_cam = mx3_cam,
-					      .id = IDMAC_IC_7};
-
-		if (*ichan) {
-			struct videobuf_buffer *vb, *_vb;
-			dma_release_channel(&(*ichan)->dma_chan);
-			*ichan = NULL;
-			mx3_cam->active = NULL;
-			list_for_each_entry_safe(vb, _vb, &mx3_cam->capture, queue) {
-				list_del_init(&vb->queue);
-				vb->state = VIDEOBUF_ERROR;
-				wake_up(&vb->done);
-			}
-		}
-
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-		dma_cap_set(DMA_PRIVATE, mask);
-		chan = dma_request_channel(mask, chan_filter, &rq);
-		if (!chan)
-			return -EBUSY;
-
-		*ichan = to_idmac_chan(chan);
-		(*ichan)->client = mx3_cam;
-	}
-
-	/*
-	 * Might have to perform a complete interface initialisation like in
-	 * ipu_csi_init_interface() in mxc_v4l2_s_param(). Also consider
-	 * mxc_v4l2_s_fmt()
-	 */
 
 	/* Setup frame size - this cannot be changed on-the-fly... */
 	width_field = rect->width - 1;
@@ -808,9 +755,98 @@ static int mx3_camera_set_fmt(struct soc_camera_device *icd,
 	 * No need to free resources here if we fail, we'll see if we need to
 	 * do this next time we are called
 	 */
+}
+
+static int acquire_dma_channel(struct mx3_camera_dev *mx3_cam)
+{
+	dma_cap_mask_t mask;
+	struct dma_chan *chan;
+	struct idmac_channel **ichan = &mx3_cam->idmac_channel[0];
+	/* We have to use IDMAC_IC_7 for Bayer / generic data */
+	struct dma_chan_request rq = {.mx3_cam = mx3_cam,
+				      .id = IDMAC_IC_7};
+
+	if (*ichan) {
+		struct videobuf_buffer *vb, *_vb;
+		dma_release_channel(&(*ichan)->dma_chan);
+		*ichan = NULL;
+		mx3_cam->active = NULL;
+		list_for_each_entry_safe(vb, _vb, &mx3_cam->capture, queue) {
+			list_del_init(&vb->queue);
+			vb->state = VIDEOBUF_ERROR;
+			wake_up(&vb->done);
+		}
+	}
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	dma_cap_set(DMA_PRIVATE, mask);
+	chan = dma_request_channel(mask, chan_filter, &rq);
+	if (!chan)
+		return -EBUSY;
+
+	*ichan = to_idmac_chan(chan);
+	(*ichan)->client = mx3_cam;
+
+	return 0;
+}
+
+static int mx3_camera_set_crop(struct soc_camera_device *icd,
+			       struct v4l2_rect *rect)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct mx3_camera_dev *mx3_cam = ici->priv;
+
+	/*
+	 * We now know pixel formats and can decide upon DMA-channel(s)
+	 * So far only direct camera-to-memory is supported
+	 */
+	if (channel_change_requested(icd, rect)) {
+		int ret = acquire_dma_channel(mx3_cam);
+		if (ret < 0)
+			return ret;
+	}
+
+	configure_geometry(mx3_cam, rect);
+
+	return icd->ops->set_crop(icd, rect);
+}
+
+static int mx3_camera_set_fmt(struct soc_camera_device *icd,
+			      struct v4l2_format *f)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct mx3_camera_dev *mx3_cam = ici->priv;
+	const struct soc_camera_format_xlate *xlate;
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	struct v4l2_rect rect = {
+		.left	= icd->x_current,
+		.top	= icd->y_current,
+		.width	= pix->width,
+		.height	= pix->height,
+	};
+	int ret;
+
+	xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
+	if (!xlate) {
+		dev_warn(&ici->dev, "Format %x not found\n", pix->pixelformat);
+		return -EINVAL;
+	}
+
+	ret = acquire_dma_channel(mx3_cam);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Might have to perform a complete interface initialisation like in
+	 * ipu_csi_init_interface() in mxc_v4l2_s_param(). Also consider
+	 * mxc_v4l2_s_fmt()
+	 */
+
+	configure_geometry(mx3_cam, &rect);
 
-	ret = icd->ops->set_fmt(icd, pixfmt ? xlate->cam_fmt->fourcc : 0, rect);
-	if (pixfmt && !ret) {
+	ret = icd->ops->set_fmt(icd, f);
+	if (!ret) {
 		icd->buswidth = xlate->buswidth;
 		icd->current_fmt = xlate->host_fmt;
 	}
@@ -1031,6 +1067,7 @@ static struct soc_camera_host_ops mx3_soc_camera_host_ops = {
 	.suspend	= mx3_camera_suspend,
 	.resume		= mx3_camera_resume,
 #endif
+	.set_crop	= mx3_camera_set_crop,
 	.set_fmt	= mx3_camera_set_fmt,
 	.try_fmt	= mx3_camera_try_fmt,
 	.get_formats	= mx3_camera_get_formats,
diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 880e51f..8dd93b3 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -781,11 +781,9 @@ ov772x_select_win(u32 width, u32 height)
 	return win;
 }
 
-static int ov772x_set_fmt(struct soc_camera_device *icd,
-			  __u32                     pixfmt,
-			  struct v4l2_rect         *rect)
+static int ov772x_set_params(struct ov772x_priv *priv, u32 width, u32 height,
+			     u32 pixfmt)
 {
-	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
 	int ret = -EINVAL;
 	u8  val;
 	int i;
@@ -806,7 +804,7 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
 	/*
 	 * select win
 	 */
-	priv->win = ov772x_select_win(rect->width, rect->height);
+	priv->win = ov772x_select_win(width, height);
 
 	/*
 	 * reset hardware
@@ -870,6 +868,28 @@ ov772x_set_fmt_error:
 	return ret;
 }
 
+static int ov772x_set_crop(struct soc_camera_device *icd,
+			   struct v4l2_rect *rect)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+
+	if (!priv->fmt)
+		return -EINVAL;
+
+	return ov772x_set_params(priv, rect->width, rect->height,
+				 priv->fmt->fourcc);
+}
+
+static int ov772x_set_fmt(struct soc_camera_device *icd,
+			  struct v4l2_format *f)
+{
+	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+
+	return ov772x_set_params(priv, pix->width, pix->height,
+				 pix->pixelformat);
+}
+
 static int ov772x_try_fmt(struct soc_camera_device *icd,
 			  struct v4l2_format       *f)
 {
@@ -959,6 +979,7 @@ static struct soc_camera_ops ov772x_ops = {
 	.release		= ov772x_release,
 	.start_capture		= ov772x_start_capture,
 	.stop_capture		= ov772x_stop_capture,
+	.set_crop		= ov772x_set_crop,
 	.set_fmt		= ov772x_set_fmt,
 	.try_fmt		= ov772x_try_fmt,
 	.set_bus_param		= ov772x_set_bus_param,
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 7b71b9c..a30747f 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -1151,8 +1151,43 @@ static int pxa_camera_get_formats(struct soc_camera_device *icd, int idx,
 	return formats;
 }
 
+static int pxa_camera_set_crop(struct soc_camera_device *icd,
+			       struct v4l2_rect *rect)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct pxa_camera_dev *pcdev = ici->priv;
+	struct soc_camera_sense sense = {
+		.master_clock = pcdev->mclk,
+		.pixel_clock_max = pcdev->ciclk / 4,
+	};
+	int ret;
+
+	/* If PCLK is used to latch data from the sensor, check sense */
+	if (pcdev->platform_flags & PXA_CAMERA_PCLK_EN)
+		icd->sense = &sense;
+
+	ret = icd->ops->set_crop(icd, rect);
+
+	icd->sense = NULL;
+
+	if (ret < 0) {
+		dev_warn(&ici->dev, "Failed to crop to %ux%u@%u:%u\n",
+			 rect->width, rect->height, rect->left, rect->top);
+	} else if (sense.flags & SOCAM_SENSE_PCLK_CHANGED) {
+		if (sense.pixel_clock > sense.pixel_clock_max) {
+			dev_err(&ici->dev,
+				"pixel clock %lu set by the camera too high!",
+				sense.pixel_clock);
+			return -EIO;
+		}
+		recalculate_fifo_timeout(pcdev, sense.pixel_clock);
+	}
+
+	return ret;
+}
+
 static int pxa_camera_set_fmt(struct soc_camera_device *icd,
-			      __u32 pixfmt, struct v4l2_rect *rect)
+			      struct v4l2_format *f)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
@@ -1162,35 +1197,30 @@ static int pxa_camera_set_fmt(struct soc_camera_device *icd,
 		.master_clock = pcdev->mclk,
 		.pixel_clock_max = pcdev->ciclk / 4,
 	};
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	struct v4l2_format cam_f = *f;
 	int ret;
 
-	if (pixfmt) {
-		xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
-		if (!xlate) {
-			dev_warn(&ici->dev, "Format %x not found\n", pixfmt);
-			return -EINVAL;
-		}
-
-		cam_fmt = xlate->cam_fmt;
+	xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
+	if (!xlate) {
+		dev_warn(&ici->dev, "Format %x not found\n", pix->pixelformat);
+		return -EINVAL;
 	}
 
+	cam_fmt = xlate->cam_fmt;
+
 	/* If PCLK is used to latch data from the sensor, check sense */
 	if (pcdev->platform_flags & PXA_CAMERA_PCLK_EN)
 		icd->sense = &sense;
 
-	switch (pixfmt) {
-	case 0:				/* Only geometry change */
-		ret = icd->ops->set_fmt(icd, pixfmt, rect);
-		break;
-	default:
-		ret = icd->ops->set_fmt(icd, cam_fmt->fourcc, rect);
-	}
+	cam_f.fmt.pix.pixelformat = cam_fmt->fourcc;
+	ret = icd->ops->set_fmt(icd, &cam_f);
 
 	icd->sense = NULL;
 
 	if (ret < 0) {
 		dev_warn(&ici->dev, "Failed to configure for format %x\n",
-			 pixfmt);
+			 pix->pixelformat);
 	} else if (sense.flags & SOCAM_SENSE_PCLK_CHANGED) {
 		if (sense.pixel_clock > sense.pixel_clock_max) {
 			dev_err(&ici->dev,
@@ -1201,7 +1231,7 @@ static int pxa_camera_set_fmt(struct soc_camera_device *icd,
 		recalculate_fifo_timeout(pcdev, sense.pixel_clock);
 	}
 
-	if (pixfmt && !ret) {
+	if (!ret) {
 		icd->buswidth = xlate->buswidth;
 		icd->current_fmt = xlate->host_fmt;
 	}
@@ -1365,6 +1395,7 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.remove		= pxa_camera_remove_device,
 	.suspend	= pxa_camera_suspend,
 	.resume		= pxa_camera_resume,
+	.set_crop	= pxa_camera_set_crop,
 	.get_formats	= pxa_camera_get_formats,
 	.set_fmt	= pxa_camera_set_fmt,
 	.try_fmt	= pxa_camera_try_fmt,
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index b8234c7..59bbefd 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -637,24 +637,30 @@ add_single_format:
 	return formats;
 }
 
+static int sh_mobile_ceu_set_crop(struct soc_camera_device *icd,
+				  struct v4l2_rect *rect)
+{
+	return icd->ops->set_crop(icd, rect);
+}
+
 static int sh_mobile_ceu_set_fmt(struct soc_camera_device *icd,
-				 __u32 pixfmt, struct v4l2_rect *rect)
+				 struct v4l2_format *f)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
+	__u32 pixfmt = f->fmt.pix.pixelformat;
 	const struct soc_camera_format_xlate *xlate;
+	struct v4l2_format cam_f = *f;
 	int ret;
 
-	if (!pixfmt)
-		return icd->ops->set_fmt(icd, pixfmt, rect);
-
 	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
 	if (!xlate) {
 		dev_warn(&ici->dev, "Format %x not found\n", pixfmt);
 		return -EINVAL;
 	}
 
-	ret = icd->ops->set_fmt(icd, xlate->cam_fmt->fourcc, rect);
+	cam_f.fmt.pix.pixelformat = xlate->cam_fmt->fourcc;
+	ret = icd->ops->set_fmt(icd, &cam_f);
 
 	if (!ret) {
 		icd->buswidth = xlate->buswidth;
@@ -786,6 +792,7 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.add		= sh_mobile_ceu_add_device,
 	.remove		= sh_mobile_ceu_remove_device,
 	.get_formats	= sh_mobile_ceu_get_formats,
+	.set_crop	= sh_mobile_ceu_set_crop,
 	.set_fmt	= sh_mobile_ceu_set_fmt,
 	.try_fmt	= sh_mobile_ceu_try_fmt,
 	.reqbufs	= sh_mobile_ceu_reqbufs,
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index fcb05f0..9939b04 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -417,9 +417,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 	struct soc_camera_device *icd = icf->icd;
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct v4l2_pix_format *pix = &f->fmt.pix;
-	__u32 pixfmt = pix->pixelformat;
 	int ret;
-	struct v4l2_rect rect;
 
 	WARN_ON(priv != file->private_data);
 
@@ -435,23 +433,19 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 		goto unlock;
 	}
 
-	rect.left	= icd->x_current;
-	rect.top	= icd->y_current;
-	rect.width	= pix->width;
-	rect.height	= pix->height;
-	ret = ici->ops->set_fmt(icd, pix->pixelformat, &rect);
+	ret = ici->ops->set_fmt(icd, f);
 	if (ret < 0) {
 		goto unlock;
 	} else if (!icd->current_fmt ||
-		   icd->current_fmt->fourcc != pixfmt) {
+		   icd->current_fmt->fourcc != pix->pixelformat) {
 		dev_err(&ici->dev,
 			"Host driver hasn't set up current format correctly!\n");
 		ret = -EINVAL;
 		goto unlock;
 	}
 
-	icd->width		= rect.width;
-	icd->height		= rect.height;
+	icd->width		= f->fmt.pix.width;
+	icd->height		= f->fmt.pix.height;
 	icf->vb_vidq.field	= pix->field;
 	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
@@ -461,7 +455,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 		icd->width, icd->height);
 
 	/* set physical bus parameters */
-	ret = ici->ops->set_bus_param(icd, pixfmt);
+	ret = ici->ops->set_bus_param(icd, pix->pixelformat);
 
 unlock:
 	mutex_unlock(&icf->vb_vidq.vb_lock);
@@ -685,7 +679,7 @@ static int soc_camera_s_crop(struct file *file, void *fh,
 	/* Cropping is allowed during a running capture, guard consistency */
 	mutex_lock(&icf->vb_vidq.vb_lock);
 
-	ret = ici->ops->set_fmt(icd, 0, &a->c);
+	ret = ici->ops->set_crop(icd, &a->c);
 	if (!ret) {
 		icd->width	= a->c.width;
 		icd->height	= a->c.height;
@@ -918,6 +912,7 @@ int soc_camera_host_register(struct soc_camera_host *ici)
 	if (!ici || !ici->ops ||
 	    !ici->ops->try_fmt ||
 	    !ici->ops->set_fmt ||
+	    !ici->ops->set_crop ||
 	    !ici->ops->set_bus_param ||
 	    !ici->ops->querycap ||
 	    !ici->ops->init_videobuf ||
@@ -998,6 +993,7 @@ int soc_camera_device_register(struct soc_camera_device *icd)
 	    !icd->ops->release ||
 	    !icd->ops->start_capture ||
 	    !icd->ops->stop_capture ||
+	    !icd->ops->set_crop ||
 	    !icd->ops->set_fmt ||
 	    !icd->ops->try_fmt ||
 	    !icd->ops->query_bus_param ||
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index 013ab06..c486763 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -79,8 +79,14 @@ soc_camera_platform_query_bus_param(struct soc_camera_device *icd)
 	return p->bus_param;
 }
 
+static int soc_camera_platform_set_crop(struct soc_camera_device *icd,
+					struct v4l2_rect *rect)
+{
+	return 0;
+}
+
 static int soc_camera_platform_set_fmt(struct soc_camera_device *icd,
-				       __u32 pixfmt, struct v4l2_rect *rect)
+				       struct v4l2_format *f)
 {
 	return 0;
 }
@@ -125,6 +131,7 @@ static struct soc_camera_ops soc_camera_platform_ops = {
 	.release		= soc_camera_platform_release,
 	.start_capture		= soc_camera_platform_start_capture,
 	.stop_capture		= soc_camera_platform_stop_capture,
+	.set_crop		= soc_camera_platform_set_crop,
 	.set_fmt		= soc_camera_platform_set_fmt,
 	.try_fmt		= soc_camera_platform_try_fmt,
 	.set_bus_param		= soc_camera_platform_set_bus_param,
diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c
index 17d8205..5e03d65 100644
--- a/drivers/media/video/tw9910.c
+++ b/drivers/media/video/tw9910.c
@@ -641,8 +641,8 @@ static int tw9910_set_register(struct soc_camera_device *icd,
 }
 #endif
 
-static int tw9910_set_fmt(struct soc_camera_device *icd, __u32 pixfmt,
-			      struct v4l2_rect *rect)
+static int tw9910_set_crop(struct soc_camera_device *icd,
+			   struct v4l2_rect *rect)
 {
 	struct tw9910_priv *priv = container_of(icd, struct tw9910_priv, icd);
 	int                 ret  = -EINVAL;
@@ -733,8 +733,21 @@ tw9910_set_fmt_error:
 	return ret;
 }
 
+static int tw9910_set_fmt(struct soc_camera_device *icd,
+			  struct v4l2_format *f)
+{
+	struct v4l2_rect rect = {
+		.left	= icd->x_current,
+		.top	= icd->y_current,
+		.width	= f->fmt.pix.width,
+		.height	= f->fmt.pix.height,
+	};
+
+	return tw9910_set_crop(icd, &rect);
+}
+
 static int tw9910_try_fmt(struct soc_camera_device *icd,
-			      struct v4l2_format *f)
+			  struct v4l2_format *f)
 {
 	struct v4l2_pix_format *pix = &f->fmt.pix;
 	const struct tw9910_scale_ctrl *scale;
@@ -822,6 +835,7 @@ static struct soc_camera_ops tw9910_ops = {
 	.release		= tw9910_release,
 	.start_capture		= tw9910_start_capture,
 	.stop_capture		= tw9910_stop_capture,
+	.set_crop		= tw9910_set_crop,
 	.set_fmt		= tw9910_set_fmt,
 	.try_fmt		= tw9910_try_fmt,
 	.set_bus_param		= tw9910_set_bus_param,
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index c63a340..e9eb607 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -74,7 +74,8 @@ struct soc_camera_host_ops {
 	int (*resume)(struct soc_camera_device *);
 	int (*get_formats)(struct soc_camera_device *, int,
 			   struct soc_camera_format_xlate *);
-	int (*set_fmt)(struct soc_camera_device *, __u32, struct v4l2_rect *);
+	int (*set_crop)(struct soc_camera_device *, struct v4l2_rect *);
+	int (*set_fmt)(struct soc_camera_device *, struct v4l2_format *);
 	int (*try_fmt)(struct soc_camera_device *, struct v4l2_format *);
 	void (*init_videobuf)(struct videobuf_queue *,
 			      struct soc_camera_device *);
@@ -159,7 +160,8 @@ struct soc_camera_ops {
 	int (*release)(struct soc_camera_device *);
 	int (*start_capture)(struct soc_camera_device *);
 	int (*stop_capture)(struct soc_camera_device *);
-	int (*set_fmt)(struct soc_camera_device *, __u32, struct v4l2_rect *);
+	int (*set_crop)(struct soc_camera_device *, struct v4l2_rect *);
+	int (*set_fmt)(struct soc_camera_device *, struct v4l2_format *);
 	int (*try_fmt)(struct soc_camera_device *, struct v4l2_format *);
 	unsigned long (*query_bus_param)(struct soc_camera_device *);
 	int (*set_bus_param)(struct soc_camera_device *, unsigned long);
-- 
1.5.4

[PATCH 2/2] soc-camera: configure drivers with a default format on open

[Guennadi Liakhovetski]

Currently soc-camera doesn't set up any image format without an explicit S_FMT.
It seems this should be supported, since, for example, capture-example.c from
v4l2-apps by default doesn't issue an S_FMT. This patch configures a default
image format on open().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Morimoto-san, please, have a look how far these two patches take you, I 
lost the track of the problems a bit:-) Does capture-example work for you 
now without the "-f"?

 drivers/media/video/soc_camera.c |   33 +++++++++++++++++++++++++++++----
 1 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 9939b04..4e88c7f 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -30,6 +30,10 @@
 #include <media/videobuf-core.h>
 #include <media/soc_camera.h>
 
+/* Default to VGA resolution */
+#define DEFAULT_WIDTH	640
+#define DEFAULT_HEIGHT	480
+
 static LIST_HEAD(hosts);
 static LIST_HEAD(devices);
 static DEFINE_MUTEX(list_lock);
@@ -297,6 +301,15 @@ static int soc_camera_open(struct file *file)
 
 	/* Now we really have to activate the camera */
 	if (icd->use_count == 1) {
+		struct v4l2_format f = {
+			.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+			.fmt.pix = {
+				.width		= DEFAULT_WIDTH,
+				.height		= DEFAULT_HEIGHT,
+				.field		= V4L2_FIELD_ANY,
+			},
+		};
+
 		ret = soc_camera_init_user_formats(icd);
 		if (ret < 0)
 			goto eiufmt;
@@ -305,6 +318,18 @@ static int soc_camera_open(struct file *file)
 			dev_err(&icd->dev, "Couldn't activate the camera: %d\n", ret);
 			goto eiciadd;
 		}
+
+		f.fmt.pix.pixelformat	= icd->current_fmt->fourcc;
+		f.fmt.pix.colorspace	= icd->current_fmt->colorspace;
+
+		/* Try to configure with default parameters */
+		ret = ici->ops->set_fmt(icd, &f);
+		if (!ret) {
+			icd->width		= f.fmt.pix.width;
+			icd->height		= f.fmt.pix.height;
+			icf->vb_vidq.field	= f.fmt.pix.field;
+			ici->ops->set_bus_param(icd, f.fmt.pix.pixelformat);
+		}
 	}
 
 	mutex_unlock(&icd->video_lock);
@@ -444,8 +469,8 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 		goto unlock;
 	}
 
-	icd->width		= f->fmt.pix.width;
-	icd->height		= f->fmt.pix.height;
+	icd->width		= pix->width;
+	icd->height		= pix->height;
 	icf->vb_vidq.field	= pix->field;
 	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
@@ -642,8 +667,8 @@ static int soc_camera_cropcap(struct file *file, void *fh,
 	a->bounds.height		= icd->height_max;
 	a->defrect.left			= icd->x_min;
 	a->defrect.top			= icd->y_min;
-	a->defrect.width		= 640;
-	a->defrect.height		= 480;
+	a->defrect.width		= DEFAULT_WIDTH;
+	a->defrect.height		= DEFAULT_HEIGHT;
 	a->pixelaspect.numerator	= 1;
 	a->pixelaspect.denominator	= 1;
 
-- 
1.5.4

Re: [PATCH 2/2] soc-camera: configure drivers with a default format on open

[morimoto.kuninori]

Dear Guennadi

> Currently soc-camera doesn't set up any image format without an explicit S_FMT.
> It seems this should be supported, since, for example, capture-example.c from
> v4l2-apps by default doesn't issue an S_FMT. This patch configures a default
> image format on open().
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> Morimoto-san, please, have a look how far these two patches take you, I 
> lost the track of the problems a bit:-) Does capture-example work for you 
> now without the "-f"?

Oooops. sorry ov772x and tw9910 doesn't works...
The reason is videobuf-core.c :: videobuf_next_field.

BUG_ON(V4L2_FIELD_ANY == filed);

sh_mobile_ceu use V4L2_FIELD_ANY now.
and ov772x and tw9910 change field in try_fmt.
But try_fmt isn't called...


Best regards
--
Kuninori Morimoto
 

[PATCH 2/2 v2] soc-camera: configure drivers with a default format on open

[Guennadi Liakhovetski]

Currently soc-camera doesn't set up any image format without an explicit S_FMT.
It seems this should be supported, since, for example, capture-example.c from
v4l2-apps by default doesn't issue an S_FMT. This patch configures a default
image format on open().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
On Fri, 20 Feb 2009, morimoto.kuninori@renesas.com wrote:

> > Morimoto-san, please, have a look how far these two patches take you, I 
> > lost the track of the problems a bit:-) Does capture-example work for you 
> > now without the "-f"?
> 
> Oooops. sorry ov772x and tw9910 doesn't works...
> The reason is videobuf-core.c :: videobuf_next_field.
> 
> BUG_ON(V4L2_FIELD_ANY == filed);
> 
> sh_mobile_ceu use V4L2_FIELD_ANY now.
> and ov772x and tw9910 change field in try_fmt.
> But try_fmt isn't called...

Ok, you're right. I do call it now. How about this version?

 drivers/media/video/soc_camera.c |  100 ++++++++++++++++++++++++++------------
 1 files changed, 68 insertions(+), 32 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 9939b04..fcd6b2c 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -30,6 +30,10 @@
 #include <media/videobuf-core.h>
 #include <media/soc_camera.h>
 
+/* Default to VGA resolution */
+#define DEFAULT_WIDTH	640
+#define DEFAULT_HEIGHT	480
+
 static LIST_HEAD(hosts);
 static LIST_HEAD(devices);
 static DEFINE_MUTEX(list_lock);
@@ -256,6 +260,44 @@ static void soc_camera_free_user_formats(struct soc_camera_device *icd)
 	vfree(icd->user_formats);
 }
 
+/* Called with .vb_lock held */
+static int soc_camera_set_fmt(struct soc_camera_file *icf,
+			      struct v4l2_format *f)
+{
+	struct soc_camera_device *icd = icf->icd;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	int ret;
+
+	/* We always call try_fmt() before set_fmt() or set_crop() */
+	ret = ici->ops->try_fmt(icd, f);
+	if (ret < 0)
+		return ret;
+
+	ret = ici->ops->set_fmt(icd, f);
+	if (ret < 0) {
+		return ret;
+	} else if (!icd->current_fmt ||
+		   icd->current_fmt->fourcc != pix->pixelformat) {
+		dev_err(&ici->dev,
+			"Host driver hasn't set up current format correctly!\n");
+		return -EINVAL;
+	}
+
+	icd->width		= pix->width;
+	icd->height		= pix->height;
+	icf->vb_vidq.field	= pix->field;
+	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
+			 f->type);
+
+	dev_dbg(&icd->dev, "set width: %d height: %d\n",
+		icd->width, icd->height);
+
+	/* set physical bus parameters */
+	return ici->ops->set_bus_param(icd, pix->pixelformat);
+}
+
 static int soc_camera_open(struct file *file)
 {
 	struct video_device *vdev;
@@ -297,6 +339,15 @@ static int soc_camera_open(struct file *file)
 
 	/* Now we really have to activate the camera */
 	if (icd->use_count == 1) {
+		struct v4l2_format f = {
+			.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+			.fmt.pix = {
+				.width		= DEFAULT_WIDTH,
+				.height		= DEFAULT_HEIGHT,
+				.field		= V4L2_FIELD_ANY,
+			},
+		};
+
 		ret = soc_camera_init_user_formats(icd);
 		if (ret < 0)
 			goto eiufmt;
@@ -305,6 +356,14 @@ static int soc_camera_open(struct file *file)
 			dev_err(&icd->dev, "Couldn't activate the camera: %d\n", ret);
 			goto eiciadd;
 		}
+
+		f.fmt.pix.pixelformat	= icd->current_fmt->fourcc;
+		f.fmt.pix.colorspace	= icd->current_fmt->colorspace;
+
+		/* Try to configure with default parameters */
+		ret = soc_camera_set_fmt(icf, &f);
+		if (ret < 0)
+			goto esfmt;
 	}
 
 	mutex_unlock(&icd->video_lock);
@@ -316,7 +375,12 @@ static int soc_camera_open(struct file *file)
 
 	return 0;
 
-	/* First two errors are entered with the .video_lock held */
+	/*
+	 * First three errors are entered with the .video_lock held
+	 * and use_count == 1
+	 */
+esfmt:
+	ici->ops->remove(icd);
 eiciadd:
 	soc_camera_free_user_formats(icd);
 eiufmt:
@@ -415,16 +479,10 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
-	struct v4l2_pix_format *pix = &f->fmt.pix;
 	int ret;
 
 	WARN_ON(priv != file->private_data);
 
-	ret = soc_camera_try_fmt_vid_cap(file, priv, f);
-	if (ret < 0)
-		return ret;
-
 	mutex_lock(&icf->vb_vidq.vb_lock);
 
 	if (videobuf_queue_is_busy(&icf->vb_vidq)) {
@@ -433,29 +491,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 		goto unlock;
 	}
 
-	ret = ici->ops->set_fmt(icd, f);
-	if (ret < 0) {
-		goto unlock;
-	} else if (!icd->current_fmt ||
-		   icd->current_fmt->fourcc != pix->pixelformat) {
-		dev_err(&ici->dev,
-			"Host driver hasn't set up current format correctly!\n");
-		ret = -EINVAL;
-		goto unlock;
-	}
-
-	icd->width		= f->fmt.pix.width;
-	icd->height		= f->fmt.pix.height;
-	icf->vb_vidq.field	= pix->field;
-	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
-			 f->type);
-
-	dev_dbg(&icd->dev, "set width: %d height: %d\n",
-		icd->width, icd->height);
-
-	/* set physical bus parameters */
-	ret = ici->ops->set_bus_param(icd, pix->pixelformat);
+	ret = soc_camera_set_fmt(icf, f);
 
 unlock:
 	mutex_unlock(&icf->vb_vidq.vb_lock);
@@ -642,8 +678,8 @@ static int soc_camera_cropcap(struct file *file, void *fh,
 	a->bounds.height		= icd->height_max;
 	a->defrect.left			= icd->x_min;
 	a->defrect.top			= icd->y_min;
-	a->defrect.width		= 640;
-	a->defrect.height		= 480;
+	a->defrect.width		= DEFAULT_WIDTH;
+	a->defrect.height		= DEFAULT_HEIGHT;
 	a->pixelaspect.numerator	= 1;
 	a->pixelaspect.denominator	= 1;
 
-- 
1.5.4

Re: [PATCH 2/2 v2] soc-camera: configure drivers with a default format on open

[morimoto.kuninori]

Dear Guennadi

> Currently soc-camera doesn't set up any image format without an explicit S_FMT.
> It seems this should be supported, since, for example, capture-example.c from
> v4l2-apps by default doesn't issue an S_FMT. This patch configures a default
> image format on open().
(snip)
> Ok, you're right. I do call it now. How about this version?

Thank you for hard work.
but... It doesn't work either in same reason.

I'm not sure for detail.
but meybe there is a problem in the call order.

ov772x_try_fmt is called when soc_camera_open is called.
but sh_mobile_ceu_init_videobuf that set filed to ANY
is called after soc_camera_open.
And ov772x_try_fmt should be called after sh_mobile_ceu_init_videobuf.

Best regards
--
Kuninori Morimoto
 

[PATCH] sh-mobile-ceu-camera: set field to the value, configured at open()

[Guennadi Liakhovetski]

For the case, that we have to capture with a default format, i.e., when the
user doesn't call S_FMT, we have to use the field value according to the
default, configured at open() time.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

On Mon, 23 Feb 2009, morimoto.kuninori@renesas.com wrote:

> ov772x_try_fmt is called when soc_camera_open is called.
> but sh_mobile_ceu_init_videobuf that set filed to ANY
> is called after soc_camera_open.
> And ov772x_try_fmt should be called after sh_mobile_ceu_init_videobuf.

Ok, you're right again. But that's not the problem with my previous patch 
this time, it's that one more patch was missing - this one. And this time 
I even actually tested it - at least with the capture-example. I would 
need an mplayer or gstreamer for migor... Testing video blindly is better 
than no testing at all, but worse than visually:-)

 drivers/media/video/sh_mobile_ceu_camera.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 3f71cb8..55a5eae 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -94,7 +94,7 @@ struct sh_mobile_ceu_dev {
 	spinlock_t lock;
 	struct list_head capture;
 	struct videobuf_buffer *active;
-	int is_interlace;
+	int is_interlaced;
 
 	struct sh_mobile_ceu_info *pdata;
 
@@ -205,7 +205,7 @@ static void sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
 
 	phys_addr_top = videobuf_to_dma_contig(pcdev->active);
 	ceu_write(pcdev, CDAYR, phys_addr_top);
-	if (pcdev->is_interlace) {
+	if (pcdev->is_interlaced) {
 		phys_addr_bottom = phys_addr_top + icd->width;
 		ceu_write(pcdev, CDBYR, phys_addr_bottom);
 	}
@@ -217,7 +217,7 @@ static void sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
 	case V4L2_PIX_FMT_NV61:
 		phys_addr_top += icd->width * icd->height;
 		ceu_write(pcdev, CDACR, phys_addr_top);
-		if (pcdev->is_interlace) {
+		if (pcdev->is_interlaced) {
 			phys_addr_bottom = phys_addr_top + icd->width;
 			ceu_write(pcdev, CDBCR, phys_addr_bottom);
 		}
@@ -481,7 +481,7 @@ static int sh_mobile_ceu_set_bus_param(struct soc_camera_device *icd,
 	ceu_write(pcdev, CAMCR, value);
 
 	ceu_write(pcdev, CAPCR, 0x00300000);
-	ceu_write(pcdev, CAIFR, (pcdev->is_interlace) ? 0x101 : 0);
+	ceu_write(pcdev, CAIFR, pcdev->is_interlaced ? 0x101 : 0);
 
 	mdelay(1);
 
@@ -497,7 +497,7 @@ static int sh_mobile_ceu_set_bus_param(struct soc_camera_device *icd,
 	}
 
 	height = icd->height;
-	if (pcdev->is_interlace) {
+	if (pcdev->is_interlaced) {
 		height /= 2;
 		cdwdr_width *= 2;
 	}
@@ -711,13 +711,13 @@ static int sh_mobile_ceu_try_fmt(struct soc_camera_device *icd,
 
 	switch (f->fmt.pix.field) {
 	case V4L2_FIELD_INTERLACED:
-		pcdev->is_interlace = 1;
+		pcdev->is_interlaced = 1;
 		break;
 	case V4L2_FIELD_ANY:
 		f->fmt.pix.field = V4L2_FIELD_NONE;
 		/* fall-through */
 	case V4L2_FIELD_NONE:
-		pcdev->is_interlace = 0;
+		pcdev->is_interlaced = 0;
 		break;
 	default:
 		ret = -EINVAL;
@@ -783,7 +783,8 @@ static void sh_mobile_ceu_init_videobuf(struct videobuf_queue *q,
 				       &sh_mobile_ceu_videobuf_ops,
 				       &ici->dev, &pcdev->lock,
 				       V4L2_BUF_TYPE_VIDEO_CAPTURE,
-				       V4L2_FIELD_ANY,
+				       pcdev->is_interlaced ?
+				       V4L2_FIELD_INTERLACED : V4L2_FIELD_NONE,
 				       sizeof(struct sh_mobile_ceu_buffer),
 				       icd);
 }
-- 
1.5.4

Re: [PATCH] sh-mobile-ceu-camera: set field to the value, configured at open()

[morimoto.kuninori]

Dear Guennadi

> Ok, you're right again. But that's not the problem with my previous patch 
> this time, it's that one more patch was missing - this one. And this time 
> I even actually tested it - at least with the capture-example. I would 
> need an mplayer or gstreamer for migor... Testing video blindly is better 
> than no testing at all, but worse than visually:-)

Thank you for your hard work.
Good !!. It works well on my local environment.

MigoR + ov772x + capture-example
MigoR + tw9910 + capture-example

Tested-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>

Best regards
--
Kuninori Morimoto
 

Re: [PATCH 2/2] soc-camera: configure drivers with a default format on open

[Magnus Damm]

On Fri, Feb 20, 2009 at 12:20 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Currently soc-camera doesn't set up any image format without an explicit S_FMT.
> It seems this should be supported, since, for example, capture-example.c from
> v4l2-apps by default doesn't issue an S_FMT. This patch configures a default
> image format on open().
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---

I like the idea behind this patch, but I wonder if it is compatible
with standard V4L2 behaviour. Please double check against the  open()
comment in section "4.1.3. Image Format Negotiation" below:

http://v4l2spec.bytesex.org/spec/c6488.htm#AEN6520

Cheers,

/ magnus

[PATCH 2/2 v3] soc-camera: configure drivers with a default format on open

[Guennadi Liakhovetski]

Currently soc-camera doesn't set up any image format without an explicit 
S_FMT. According to the API this should be supported, for example, 
capture-example.c from v4l2-apps by default doesn't issue an S_FMT. This 
patch moves negotiating of available host-camera format translations to 
probe() time, and restores the state from the last close() on the next 
open(). This is needed for some drivers, which power down or reset 
hardware after the last user closes the interface. This patch also has a 
nice side-effect of avoiding multiple allocation anf freeing of format 
translation tables.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
On Tue, 24 Feb 2009, Magnus Damm wrote:

> I like the idea behind this patch, but I wonder if it is compatible
> with standard V4L2 behaviour. Please double check against the  open()
> comment in section "4.1.3. Image Format Negotiation" below:
> 
> http://v4l2spec.bytesex.org/spec/c6488.htm#AEN6520

Hm, until now I was interpreting it in a way "as long as a video device is 
kept open, subsequent open() calls shouldn't change its state," but as 
soon as a device is released by all users, its state is lost. But now I 
see also here http://v4l2spec.bytesex.org/spec/r14090.htm:

<quote>
At the first open() call after loading the driver they will be reset to 
default values, drivers are never in an undefined state.
</quote>

With this patch I still issue a S_FMT on "every first open," i.e., every 
time the use count goes from 0 to 1, but I don't use default parameters 
any more, but those from the previous session, which should make us 
sufficiently compliant with the standard:-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index fcd6b2c..078d4b1 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -286,7 +286,9 @@ static int soc_camera_set_fmt(struct soc_camera_file *icf,
 
 	icd->width		= pix->width;
 	icd->height		= pix->height;
-	icf->vb_vidq.field	= pix->field;
+	icf->vb_vidq.field	=
+		icd->field	= pix->field;
+
 	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
 			 f->type);
@@ -339,26 +341,24 @@ static int soc_camera_open(struct file *file)
 
 	/* Now we really have to activate the camera */
 	if (icd->use_count == 1) {
+		/* Restore parameters before the last close() per V4L2 API */
 		struct v4l2_format f = {
 			.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
 			.fmt.pix = {
-				.width		= DEFAULT_WIDTH,
-				.height		= DEFAULT_HEIGHT,
-				.field		= V4L2_FIELD_ANY,
+				.width		= icd->width,
+				.height		= icd->height,
+				.field		= icd->field,
+				.pixelformat	= icd->current_fmt->fourcc,
+				.colorspace	= icd->current_fmt->colorspace,
 			},
 		};
 
-		ret = soc_camera_init_user_formats(icd);
-		if (ret < 0)
-			goto eiufmt;
 		ret = ici->ops->add(icd);
 		if (ret < 0) {
 			dev_err(&icd->dev, "Couldn't activate the camera: %d\n", ret);
 			goto eiciadd;
 		}
 
-		f.fmt.pix.pixelformat	= icd->current_fmt->fourcc;
-		f.fmt.pix.colorspace	= icd->current_fmt->colorspace;
 
 		/* Try to configure with default parameters */
 		ret = soc_camera_set_fmt(icf, &f);
@@ -382,8 +382,6 @@ static int soc_camera_open(struct file *file)
 esfmt:
 	ici->ops->remove(icd);
 eiciadd:
-	soc_camera_free_user_formats(icd);
-eiufmt:
 	icd->use_count--;
 	mutex_unlock(&icd->video_lock);
 	module_put(ici->ops->owner);
@@ -403,10 +401,9 @@ static int soc_camera_close(struct file *file)
 
 	mutex_lock(&icd->video_lock);
 	icd->use_count--;
-	if (!icd->use_count) {
+	if (!icd->use_count)
 		ici->ops->remove(icd);
-		soc_camera_free_user_formats(icd);
-	}
+
 	mutex_unlock(&icd->video_lock);
 
 	module_put(icd->ops->owner);
@@ -874,9 +871,18 @@ static int soc_camera_probe(struct device *dev)
 		qctrl = soc_camera_find_qctrl(icd->ops, V4L2_CID_EXPOSURE);
 		icd->exposure = qctrl ? qctrl->default_value :
 			(unsigned short)~0;
+
+		ret = soc_camera_init_user_formats(icd);
+		if (ret < 0)
+			goto eiufmt;
+
+		icd->height	= DEFAULT_HEIGHT;
+		icd->width	= DEFAULT_WIDTH;
+		icd->field	= V4L2_FIELD_ANY;
 	}
-	ici->ops->remove(icd);
 
+eiufmt:
+	ici->ops->remove(icd);
 eiadd:
 	mutex_unlock(&icd->video_lock);
 	module_put(ici->ops->owner);
@@ -895,6 +901,8 @@ static int soc_camera_remove(struct device *dev)
 	if (icd->ops->remove)
 		icd->ops->remove(icd);
 
+	soc_camera_free_user_formats(icd);
+
 	return 0;
 }
 
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index e9eb607..013c818 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -45,6 +45,7 @@ struct soc_camera_device {
 	int num_formats;
 	struct soc_camera_format_xlate *user_formats;
 	int num_user_formats;
+	enum v4l2_field field;		/* Preserve field over close() */
 	struct module *owner;
 	void *host_priv;		/* Per-device host private data */
 	/* soc_camera.c private count. Only accessed with .video_lock held */