[PATCH 0/8] soc-camera: Add support for configurable line stride

[PATCH 0/8] soc-camera: Add support for configurable line stride

[Laurent Pinchart]

Hi Guennadi,

This patch set adds support for configurable line stride to the soc-camera
framework and the sh_mobile_ceu_camera driver. I've successfully tested it on
a Mackerel board with the on-board VGA sensor.

Laurent Pinchart (8):
  soc_camera: Use soc_camera_device::sizeimage to compute buffer sizes
  soc_camera: Use soc_camera_device::bytesperline to compute line sizes
  soc-camera: Add plane layout information to struct soc_mbus_pixelfmt
  soc-camera: Fix bytes per line computation for planar formats
  soc-camera: Add soc_mbus_image_size
  soc-camera: Honor user-requested bytesperline and sizeimage
  soc-camera: Support user-configurable line stride
  sh_mobile_ceu_camera: Support user-configurable line stride

 drivers/media/video/atmel-isi.c            |   18 ++------
 drivers/media/video/mx1_camera.c           |   14 +-----
 drivers/media/video/mx2_camera.c           |   16 ++-----
 drivers/media/video/mx3_camera.c           |   41 +++++++++-------
 drivers/media/video/omap1_camera.c         |   22 ++++-----
 drivers/media/video/pxa_camera.c           |   15 +-----
 drivers/media/video/sh_mobile_ceu_camera.c |   68 ++++++++++++++++-----------
 drivers/media/video/soc_camera.c           |   35 ++++++++-------
 drivers/media/video/soc_mediabus.c         |   54 ++++++++++++++++++++++
 include/media/soc_camera.h                 |    4 ++
 include/media/soc_mediabus.h               |   21 +++++++++
 11 files changed, 184 insertions(+), 124 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/8] soc_camera: Use soc_camera_device::sizeimage to compute buffer sizes

[Laurent Pinchart]

Instead of computing the buffer size manually in the videobuf queue
setup and buffer prepare callbacks, use the previously negotiated
soc_camera_device::sizeimage value.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/atmel-isi.c            |   17 +++--------------
 drivers/media/video/mx1_camera.c           |   14 ++------------
 drivers/media/video/mx2_camera.c           |   14 ++------------
 drivers/media/video/mx3_camera.c           |   20 +++++++++-----------
 drivers/media/video/omap1_camera.c         |   14 ++------------
 drivers/media/video/pxa_camera.c           |   14 ++------------
 drivers/media/video/sh_mobile_ceu_camera.c |   25 +++++++++----------------
 7 files changed, 29 insertions(+), 89 deletions(-)

diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 8c775c5..73f8d05 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -257,7 +257,7 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct atmel_isi *isi = ici->priv;
 	unsigned long size;
-	int ret, bytes_per_line;
+	int ret;
 
 	/* Reset ISI */
 	ret = atmel_isi_wait_status(isi, WAIT_ISI_RESET);
@@ -268,13 +268,7 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
 	/* Disable all interrupts */
 	isi_writel(isi, ISI_INTDIS, ~0UL);
 
-	bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
-	size = bytes_per_line * icd->user_height;
+	size = icd->sizeimage;
 
 	if (!*nbuffers || *nbuffers > MAX_BUFFER_NUM)
 		*nbuffers = MAX_BUFFER_NUM;
@@ -313,13 +307,8 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	struct atmel_isi *isi = ici->priv;
 	unsigned long size;
 	struct isi_dma_desc *desc;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
 
-	size = bytes_per_line * icd->user_height;
+	size = icd->sizeimage;
 
 	if (vb2_plane_size(vb, 0) < size) {
 		dev_err(icd->parent, "%s data will not fit into plane (%lu < %lu)\n",
diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
index 18e94c7..6f4d5eb 100644
--- a/drivers/media/video/mx1_camera.c
+++ b/drivers/media/video/mx1_camera.c
@@ -126,13 +126,8 @@ static int mx1_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
 			      unsigned int *size)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
 
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
-	*size = bytes_per_line * icd->user_height;
+	*size = icd->sizeimage;
 
 	if (!*count)
 		*count = 32;
@@ -171,11 +166,6 @@ static int mx1_videobuf_prepare(struct videobuf_queue *vq,
 	struct soc_camera_device *icd = vq->priv_data;
 	struct mx1_buffer *buf = container_of(vb, struct mx1_buffer, vb);
 	int ret;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
 
 	dev_dbg(icd->parent, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
 		vb, vb->baddr, vb->bsize);
@@ -202,7 +192,7 @@ static int mx1_videobuf_prepare(struct videobuf_queue *vq,
 		vb->state	= VIDEOBUF_NEEDS_INIT;
 	}
 
-	vb->size = bytes_per_line * vb->height;
+	vb->size = icd->sizeimage;
 	if (0 != vb->baddr && vb->bsize < vb->size) {
 		ret = -EINVAL;
 		goto out;
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index a803d9e..e9b228d 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -433,15 +433,10 @@ static int mx2_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
 			      unsigned int *size)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-			icd->current_fmt->host_fmt);
 
 	dev_dbg(icd->parent, "count=%d, size=%d\n", *count, *size);
 
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
-	*size = bytes_per_line * icd->user_height;
+	*size = icd->sizeimage;
 
 	if (0 == *count)
 		*count = 32;
@@ -476,16 +471,11 @@ static int mx2_videobuf_prepare(struct videobuf_queue *vq,
 {
 	struct soc_camera_device *icd = vq->priv_data;
 	struct mx2_buffer *buf = container_of(vb, struct mx2_buffer, vb);
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-			icd->current_fmt->host_fmt);
 	int ret = 0;
 
 	dev_dbg(icd->parent, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
 		vb, vb->baddr, vb->bsize);
 
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
 #ifdef DEBUG
 	/*
 	 * This can be useful if you want to see if we actually fill
@@ -505,7 +495,7 @@ static int mx2_videobuf_prepare(struct videobuf_queue *vq,
 		vb->state	= VIDEOBUF_NEEDS_INIT;
 	}
 
-	vb->size = bytes_per_line * vb->height;
+	vb->size = icd->sizeimage;
 	if (vb->baddr && vb->bsize < vb->size) {
 		ret = -EINVAL;
 		goto out;
diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index f96f92f..da45a89 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -199,8 +199,6 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
-	int bytes_per_line;
-	unsigned int height;
 
 	if (!mx3_cam->idmac_channel[0])
 		return -EINVAL;
@@ -208,21 +206,21 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
 	if (fmt) {
 		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
 								fmt->fmt.pix.pixelformat);
+		int bytes_per_line;
+
 		if (!xlate)
 			return -EINVAL;
+
 		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
 							 xlate->host_fmt);
-		height = fmt->fmt.pix.height;
+		if (bytes_per_line < 0)
+			return bytes_per_line;
+
+		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
 	} else {
 		/* Called from VIDIOC_REQBUFS or in compatibility mode */
-		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-		height = icd->user_height;
+		sizes[0] = icd->sizeimage;
 	}
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
-	sizes[0] = bytes_per_line * height;
 
 	alloc_ctxs[0] = mx3_cam->alloc_ctx;
 
@@ -274,7 +272,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
 
 	BUG_ON(bytes_per_line <= 0);
 
-	new_size = bytes_per_line * icd->user_height;
+	new_size = icd->sizeimage;
 
 	if (vb2_plane_size(vb, 0) < new_size) {
 		dev_err(icd->parent, "Buffer #%d too small (%lu < %zu)\n",
diff --git a/drivers/media/video/omap1_camera.c b/drivers/media/video/omap1_camera.c
index 6a6cf38..cebe4bf 100644
--- a/drivers/media/video/omap1_camera.c
+++ b/drivers/media/video/omap1_camera.c
@@ -206,15 +206,10 @@ static int omap1_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
 		unsigned int *size)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-			icd->current_fmt->host_fmt);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct omap1_cam_dev *pcdev = ici->priv;
 
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
-	*size = bytes_per_line * icd->user_height;
+	*size = icd->sizeimage;
 
 	if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT(pcdev->vb_mode))
 		*count = OMAP1_CAMERA_MIN_BUF_COUNT(pcdev->vb_mode);
@@ -256,15 +251,10 @@ static int omap1_videobuf_prepare(struct videobuf_queue *vq,
 {
 	struct soc_camera_device *icd = vq->priv_data;
 	struct omap1_cam_buf *buf = container_of(vb, struct omap1_cam_buf, vb);
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-			icd->current_fmt->host_fmt);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct omap1_cam_dev *pcdev = ici->priv;
 	int ret;
 
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
 	WARN_ON(!list_empty(&vb->queue));
 
 	BUG_ON(NULL == icd->current_fmt);
@@ -281,7 +271,7 @@ static int omap1_videobuf_prepare(struct videobuf_queue *vq,
 		vb->state  = VIDEOBUF_NEEDS_INIT;
 	}
 
-	vb->size = bytes_per_line * vb->height;
+	vb->size = icd->sizeimage;
 
 	if (vb->baddr && vb->bsize < vb->size) {
 		ret = -EINVAL;
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 79fb22c..e7da832 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -241,15 +241,10 @@ static int pxa_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
 			      unsigned int *size)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
 
 	dev_dbg(icd->parent, "count=%d, size=%d\n", *count, *size);
 
-	*size = bytes_per_line * icd->user_height;
+	*size = icd->sizeimage;
 
 	if (0 == *count)
 		*count = 32;
@@ -435,11 +430,6 @@ static int pxa_videobuf_prepare(struct videobuf_queue *vq,
 	struct pxa_buffer *buf = container_of(vb, struct pxa_buffer, vb);
 	int ret;
 	int size_y, size_u = 0, size_v = 0;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
 
 	dev_dbg(dev, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
 		vb, vb->baddr, vb->bsize);
@@ -474,7 +464,7 @@ static int pxa_videobuf_prepare(struct videobuf_queue *vq,
 		vb->state	= VIDEOBUF_NEEDS_INIT;
 	}
 
-	vb->size = bytes_per_line * vb->height;
+	vb->size = icd->sizeimage;
 	if (0 != vb->baddr && vb->bsize < vb->size) {
 		ret = -EINVAL;
 		goto out;
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index c51decf..f4eb9e1 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -206,27 +206,25 @@ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq,
 	struct soc_camera_device *icd = container_of(vq, struct soc_camera_device, vb2_vidq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
-	int bytes_per_line;
-	unsigned int height;
 
 	if (fmt) {
 		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
 								fmt->fmt.pix.pixelformat);
+		int bytes_per_line;
+
 		if (!xlate)
 			return -EINVAL;
+
 		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
 							 xlate->host_fmt);
-		height = fmt->fmt.pix.height;
+		if (bytes_per_line < 0)
+			return bytes_per_line;
+
+		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
 	} else {
 		/* Called from VIDIOC_REQBUFS or in compatibility mode */
-		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-		height = icd->user_height;
+		sizes[0] = icd->sizeimage;
 	}
-	if (bytes_per_line < 0)
-		return bytes_per_line;
-
-	sizes[0] = bytes_per_line * height;
 
 	alloc_ctxs[0] = pcdev->alloc_ctx;
 
@@ -373,13 +371,8 @@ static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb)
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
 	struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb);
 	unsigned long size;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		goto error;
 
-	size = icd->user_height * bytes_per_line;
+	size = icd->sizeimage;
 
 	if (vb2_plane_size(vb, 0) < size) {
 		dev_err(icd->parent, "Buffer #%d too small (%lu < %lu)\n",
-- 
1.7.3.4

Re: [PATCH 1/8] soc_camera: Use soc_camera_device::sizeimage to compute buffer sizes

[Guennadi Liakhovetski]

Hi Laurent

Thanks for the patches. This one looks good mostly, a couple of questions 
though:

On Wed, 25 Jan 2012, Laurent Pinchart wrote:

> Instead of computing the buffer size manually in the videobuf queue
> setup and buffer prepare callbacks, use the previously negotiated
> soc_camera_device::sizeimage value.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/atmel-isi.c            |   17 +++--------------
>  drivers/media/video/mx1_camera.c           |   14 ++------------
>  drivers/media/video/mx2_camera.c           |   14 ++------------
>  drivers/media/video/mx3_camera.c           |   20 +++++++++-----------
>  drivers/media/video/omap1_camera.c         |   14 ++------------
>  drivers/media/video/pxa_camera.c           |   14 ++------------
>  drivers/media/video/sh_mobile_ceu_camera.c |   25 +++++++++----------------
>  7 files changed, 29 insertions(+), 89 deletions(-)

[snip]

> diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> index a803d9e..e9b228d 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -433,15 +433,10 @@ static int mx2_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
>  			      unsigned int *size)
>  {
>  	struct soc_camera_device *icd = vq->priv_data;
> -	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> -			icd->current_fmt->host_fmt);
>  
>  	dev_dbg(icd->parent, "count=%d, size=%d\n", *count, *size);
>  
> -	if (bytes_per_line < 0)
> -		return bytes_per_line;
> -
> -	*size = bytes_per_line * icd->user_height;
> +	*size = icd->sizeimage;
>  
>  	if (0 == *count)
>  		*count = 32;

I think, there is a bug in mx2_camera_try_fmt(), which also would affect 
these your calculations. On i.MX25 they restrict the image size based on 
maximum supported number of bytes. In such a case they recalculate
.bytesperline, but they fail to update .sizeimage. The fix seems to be 
trivial, please add it, then your calculations should be fine.

> @@ -476,16 +471,11 @@ static int mx2_videobuf_prepare(struct videobuf_queue *vq,
>  {
>  	struct soc_camera_device *icd = vq->priv_data;
>  	struct mx2_buffer *buf = container_of(vb, struct mx2_buffer, vb);
> -	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> -			icd->current_fmt->host_fmt);
>  	int ret = 0;
>  
>  	dev_dbg(icd->parent, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
>  		vb, vb->baddr, vb->bsize);
>  
> -	if (bytes_per_line < 0)
> -		return bytes_per_line;
> -
>  #ifdef DEBUG
>  	/*
>  	 * This can be useful if you want to see if we actually fill
> @@ -505,7 +495,7 @@ static int mx2_videobuf_prepare(struct videobuf_queue *vq,
>  		vb->state	= VIDEOBUF_NEEDS_INIT;
>  	}
>  
> -	vb->size = bytes_per_line * vb->height;
> +	vb->size = icd->sizeimage;
>  	if (vb->baddr && vb->bsize < vb->size) {
>  		ret = -EINVAL;
>  		goto out;
> diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
> index f96f92f..da45a89 100644
> --- a/drivers/media/video/mx3_camera.c
> +++ b/drivers/media/video/mx3_camera.c
> @@ -199,8 +199,6 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
>  	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
>  	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>  	struct mx3_camera_dev *mx3_cam = ici->priv;
> -	int bytes_per_line;
> -	unsigned int height;
>  
>  	if (!mx3_cam->idmac_channel[0])
>  		return -EINVAL;
> @@ -208,21 +206,21 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
>  	if (fmt) {
>  		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
>  								fmt->fmt.pix.pixelformat);
> +		int bytes_per_line;
> +
>  		if (!xlate)
>  			return -EINVAL;
> +
>  		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
>  							 xlate->host_fmt);
> -		height = fmt->fmt.pix.height;
> +		if (bytes_per_line < 0)
> +			return bytes_per_line;
> +
> +		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
>  	} else {
>  		/* Called from VIDIOC_REQBUFS or in compatibility mode */
> -		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> -						icd->current_fmt->host_fmt);
> -		height = icd->user_height;
> +		sizes[0] = icd->sizeimage;
>  	}
> -	if (bytes_per_line < 0)
> -		return bytes_per_line;
> -
> -	sizes[0] = bytes_per_line * height;
>  
>  	alloc_ctxs[0] = mx3_cam->alloc_ctx;
>  
> @@ -274,7 +272,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
>  
>  	BUG_ON(bytes_per_line <= 0);
>  
> -	new_size = bytes_per_line * icd->user_height;
> +	new_size = icd->sizeimage;

Don't you think, you could eliminate bytes_per_line too and just use 
icd->bytesperline here too?

>  
>  	if (vb2_plane_size(vb, 0) < new_size) {
>  		dev_err(icd->parent, "Buffer #%d too small (%lu < %zu)\n",

[snip]

> diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
> index c51decf..f4eb9e1 100644
> --- a/drivers/media/video/sh_mobile_ceu_camera.c
> +++ b/drivers/media/video/sh_mobile_ceu_camera.c
> @@ -206,27 +206,25 @@ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq,
>  	struct soc_camera_device *icd = container_of(vq, struct soc_camera_device, vb2_vidq);
>  	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>  	struct sh_mobile_ceu_dev *pcdev = ici->priv;
> -	int bytes_per_line;
> -	unsigned int height;
>  
>  	if (fmt) {
>  		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
>  								fmt->fmt.pix.pixelformat);
> +		int bytes_per_line;
> +
>  		if (!xlate)
>  			return -EINVAL;
> +
>  		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
>  							 xlate->host_fmt);
> -		height = fmt->fmt.pix.height;
> +		if (bytes_per_line < 0)
> +			return bytes_per_line;
> +
> +		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
>  	} else {
>  		/* Called from VIDIOC_REQBUFS or in compatibility mode */
> -		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> -						icd->current_fmt->host_fmt);
> -		height = icd->user_height;
> +		sizes[0] = icd->sizeimage;
>  	}
> -	if (bytes_per_line < 0)
> -		return bytes_per_line;
> -
> -	sizes[0] = bytes_per_line * height;
>  
>  	alloc_ctxs[0] = pcdev->alloc_ctx;
>  
> @@ -373,13 +371,8 @@ static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb)
>  	struct sh_mobile_ceu_dev *pcdev = ici->priv;
>  	struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb);
>  	unsigned long size;
> -	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> -						icd->current_fmt->host_fmt);
> -
> -	if (bytes_per_line < 0)
> -		goto error;
>  
> -	size = icd->user_height * bytes_per_line;
> +	size = icd->sizeimage;
>  
>  	if (vb2_plane_size(vb, 0) < size) {
>  		dev_err(icd->parent, "Buffer #%d too small (%lu < %lu)\n",

Looks like sh_mobile_ceu_set_rect() can also be simplified, since there 
bytes_per_line is calculated for data-fetch mode, for which the 
->bytesperline can also be used?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/8] soc_camera: Use soc_camera_device::sizeimage to compute buffer sizes

[Laurent Pinchart]

Hi Guennadi,

On Thursday 26 January 2012 16:21:00 Guennadi Liakhovetski wrote:
> Hi Laurent
> 
> Thanks for the patches. This one looks good mostly, a couple of questions
> though:
> 
> On Wed, 25 Jan 2012, Laurent Pinchart wrote:
> > Instead of computing the buffer size manually in the videobuf queue
> > setup and buffer prepare callbacks, use the previously negotiated
> > soc_camera_device::sizeimage value.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/video/atmel-isi.c            |   17 +++--------------
> >  drivers/media/video/mx1_camera.c           |   14 ++------------
> >  drivers/media/video/mx2_camera.c           |   14 ++------------
> >  drivers/media/video/mx3_camera.c           |   20 +++++++++-----------
> >  drivers/media/video/omap1_camera.c         |   14 ++------------
> >  drivers/media/video/pxa_camera.c           |   14 ++------------
> >  drivers/media/video/sh_mobile_ceu_camera.c |   25
> >  +++++++++---------------- 7 files changed, 29 insertions(+), 89
> >  deletions(-)
> 
> [snip]
> 
> > diff --git a/drivers/media/video/mx2_camera.c
> > b/drivers/media/video/mx2_camera.c index a803d9e..e9b228d 100644
> > --- a/drivers/media/video/mx2_camera.c
> > +++ b/drivers/media/video/mx2_camera.c
> > @@ -433,15 +433,10 @@ static int mx2_videobuf_setup(struct videobuf_queue
> > *vq, unsigned int *count,
> > 
> >  			      unsigned int *size)
> >  
> >  {
> >  
> >  	struct soc_camera_device *icd = vq->priv_data;
> > 
> > -	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> > -			icd->current_fmt->host_fmt);
> > 
> >  	dev_dbg(icd->parent, "count=%d, size=%d\n", *count, *size);
> > 
> > -	if (bytes_per_line < 0)
> > -		return bytes_per_line;
> > -
> > -	*size = bytes_per_line * icd->user_height;
> > +	*size = icd->sizeimage;
> > 
> >  	if (0 == *count)
> >  	
> >  		*count = 32;
> 
> I think, there is a bug in mx2_camera_try_fmt(), which also would affect
> these your calculations. On i.MX25 they restrict the image size based on
> maximum supported number of bytes. In such a case they recalculate
> .bytesperline, but they fail to update .sizeimage. The fix seems to be
> trivial, please add it, then your calculations should be fine.

Indeed. I'll add a patch for that, and I'll also modify mx2_camera_try_fmt() 
to use the new soc_mbus_image_size() function.

> > @@ -476,16 +471,11 @@ static int mx2_videobuf_prepare(struct
> > videobuf_queue *vq,
> > 
> >  {
> >  
> >  	struct soc_camera_device *icd = vq->priv_data;
> >  	struct mx2_buffer *buf = container_of(vb, struct mx2_buffer, vb);
> > 
> > -	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> > -			icd->current_fmt->host_fmt);
> > 
> >  	int ret = 0;
> >  	
> >  	dev_dbg(icd->parent, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
> >  	
> >  		vb, vb->baddr, vb->bsize);
> > 
> > -	if (bytes_per_line < 0)
> > -		return bytes_per_line;
> > -
> > 
> >  #ifdef DEBUG
> >  
> >  	/*
> >  	
> >  	 * This can be useful if you want to see if we actually fill
> > 
> > @@ -505,7 +495,7 @@ static int mx2_videobuf_prepare(struct videobuf_queue
> > *vq,
> > 
> >  		vb->state	= VIDEOBUF_NEEDS_INIT;
> >  	
> >  	}
> > 
> > -	vb->size = bytes_per_line * vb->height;
> > +	vb->size = icd->sizeimage;
> > 
> >  	if (vb->baddr && vb->bsize < vb->size) {
> >  	
> >  		ret = -EINVAL;
> >  		goto out;
> > 
> > diff --git a/drivers/media/video/mx3_camera.c
> > b/drivers/media/video/mx3_camera.c index f96f92f..da45a89 100644
> > --- a/drivers/media/video/mx3_camera.c
> > +++ b/drivers/media/video/mx3_camera.c
> > @@ -199,8 +199,6 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
> > 
> >  	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> >  	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> >  	struct mx3_camera_dev *mx3_cam = ici->priv;
> > 
> > -	int bytes_per_line;
> > -	unsigned int height;
> > 
> >  	if (!mx3_cam->idmac_channel[0])
> >  	
> >  		return -EINVAL;
> > 
> > @@ -208,21 +206,21 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
> > 
> >  	if (fmt) {
> >  	
> >  		const struct soc_camera_format_xlate *xlate =
> >  		soc_camera_xlate_by_fourcc(icd,
> >  		
> >  								fmt->fmt.pix.pixelformat);
> > 
> > +		int bytes_per_line;
> > +
> > 
> >  		if (!xlate)
> >  		
> >  			return -EINVAL;
> > 
> > +
> > 
> >  		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
> >  		
> >  							 xlate->host_fmt);
> > 
> > -		height = fmt->fmt.pix.height;
> > +		if (bytes_per_line < 0)
> > +			return bytes_per_line;
> > +
> > +		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
> > 
> >  	} else {
> >  	
> >  		/* Called from VIDIOC_REQBUFS or in compatibility mode */
> > 
> > -		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> > -						icd->current_fmt->host_fmt);
> > -		height = icd->user_height;
> > +		sizes[0] = icd->sizeimage;
> > 
> >  	}
> > 
> > -	if (bytes_per_line < 0)
> > -		return bytes_per_line;
> > -
> > -	sizes[0] = bytes_per_line * height;
> > 
> >  	alloc_ctxs[0] = mx3_cam->alloc_ctx;
> > 
> > @@ -274,7 +272,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
> > 
> >  	BUG_ON(bytes_per_line <= 0);
> > 
> > -	new_size = bytes_per_line * icd->user_height;
> > +	new_size = icd->sizeimage;
> 
> Don't you think, you could eliminate bytes_per_line too and just use
> icd->bytesperline here too?

Sure. That's why the next patch does just that :-)

> >  	if (vb2_plane_size(vb, 0) < new_size) {
> >  	
> >  		dev_err(icd->parent, "Buffer #%d too small (%lu < %zu)\n",
> 
> [snip]
> 
> > diff --git a/drivers/media/video/sh_mobile_ceu_camera.c
> > b/drivers/media/video/sh_mobile_ceu_camera.c index c51decf..f4eb9e1
> > 100644
> > --- a/drivers/media/video/sh_mobile_ceu_camera.c
> > +++ b/drivers/media/video/sh_mobile_ceu_camera.c
> > @@ -206,27 +206,25 @@ static int sh_mobile_ceu_videobuf_setup(struct
> > vb2_queue *vq,
> > 
> >  	struct soc_camera_device *icd = container_of(vq, struct
> >  	soc_camera_device, vb2_vidq); struct soc_camera_host *ici =
> >  	to_soc_camera_host(icd->parent); struct sh_mobile_ceu_dev *pcdev =
> >  	ici->priv;
> > 
> > -	int bytes_per_line;
> > -	unsigned int height;
> > 
> >  	if (fmt) {
> >  	
> >  		const struct soc_camera_format_xlate *xlate =
> >  		soc_camera_xlate_by_fourcc(icd,
> >  		
> >  								fmt->fmt.pix.pixelformat);
> > 
> > +		int bytes_per_line;
> > +
> > 
> >  		if (!xlate)
> >  		
> >  			return -EINVAL;
> > 
> > +
> > 
> >  		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
> >  		
> >  							 xlate->host_fmt);
> > 
> > -		height = fmt->fmt.pix.height;
> > +		if (bytes_per_line < 0)
> > +			return bytes_per_line;
> > +
> > +		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
> > 
> >  	} else {
> >  	
> >  		/* Called from VIDIOC_REQBUFS or in compatibility mode */
> > 
> > -		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> > -						icd->current_fmt->host_fmt);
> > -		height = icd->user_height;
> > +		sizes[0] = icd->sizeimage;
> > 
> >  	}
> > 
> > -	if (bytes_per_line < 0)
> > -		return bytes_per_line;
> > -
> > -	sizes[0] = bytes_per_line * height;
> > 
> >  	alloc_ctxs[0] = pcdev->alloc_ctx;
> > 
> > @@ -373,13 +371,8 @@ static void sh_mobile_ceu_videobuf_queue(struct
> > vb2_buffer *vb)
> > 
> >  	struct sh_mobile_ceu_dev *pcdev = ici->priv;
> >  	struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb);
> >  	unsigned long size;
> > 
> > -	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> > -						icd->current_fmt->host_fmt);
> > -
> > -	if (bytes_per_line < 0)
> > -		goto error;
> > 
> > -	size = icd->user_height * bytes_per_line;
> > +	size = icd->sizeimage;
> > 
> >  	if (vb2_plane_size(vb, 0) < size) {
> >  	
> >  		dev_err(icd->parent, "Buffer #%d too small (%lu < %lu)\n",
> 
> Looks like sh_mobile_ceu_set_rect() can also be simplified, since there
> bytes_per_line is calculated for data-fetch mode, for which the
> ->bytesperline can also be used?

Is sh_mobile_ceu_set_rect() guaranteed to be called after try_fmt(), with the 
->bytesperline value set to the correct value for the current format ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/8] soc_camera: Use soc_camera_device::sizeimage to compute buffer sizes

[Guennadi Liakhovetski]

Hi Laurent

Sorry for a slightly delayed reply;-)

On Thu, 26 Jan 2012, Laurent Pinchart wrote:

[snip]

> > > diff --git a/drivers/media/video/sh_mobile_ceu_camera.c
> > > b/drivers/media/video/sh_mobile_ceu_camera.c index c51decf..f4eb9e1
> > > 100644
> > > --- a/drivers/media/video/sh_mobile_ceu_camera.c
> > > +++ b/drivers/media/video/sh_mobile_ceu_camera.c

[snip]

> > Looks like sh_mobile_ceu_set_rect() can also be simplified, since there
> > bytes_per_line is calculated for data-fetch mode, for which the
> > ->bytesperline can also be used?
> 
> Is sh_mobile_ceu_set_rect() guaranteed to be called after try_fmt(), with the 
> ->bytesperline value set to the correct value for the current format ?

I think it is, yes. soc_camera.c always configures the pipeline upon the 
first .open() call by calling soc_camera_set_fmt(), at which point 
->bytesperline is set too. Also, just to avoid confusion - above you meant 
set_fmt(), not try_fmt(), right? *try* are not supposed to set anything.

So, if you agree, either you can do a patch 11/9 or I can do it myself. Or 
you could do a v3 of just one patch 3/9.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/8] soc_camera: Use soc_camera_device::sizeimage to compute buffer sizes

[Laurent Pinchart]

Hi Guennadi,

On Wednesday 25 April 2012 00:06:03 Guennadi Liakhovetski wrote:
> Hi Laurent
> 
> Sorry for a slightly delayed reply;-)

So slightly :-)

> On Thu, 26 Jan 2012, Laurent Pinchart wrote:
> 
> [snip]
> 
> > > > diff --git a/drivers/media/video/sh_mobile_ceu_camera.c
> > > > b/drivers/media/video/sh_mobile_ceu_camera.c index c51decf..f4eb9e1
> > > > 100644
> > > > --- a/drivers/media/video/sh_mobile_ceu_camera.c
> > > > +++ b/drivers/media/video/sh_mobile_ceu_camera.c
> 
> [snip]
> 
> > > Looks like sh_mobile_ceu_set_rect() can also be simplified, since there
> > > bytes_per_line is calculated for data-fetch mode, for which the
> > > ->bytesperline can also be used?
> > 
> > Is sh_mobile_ceu_set_rect() guaranteed to be called after try_fmt(), with
> > the ->bytesperline value set to the correct value for the current format
> > ?
>
> I think it is, yes. soc_camera.c always configures the pipeline upon the
> first .open() call by calling soc_camera_set_fmt(), at which point
> ->bytesperline is set too. Also, just to avoid confusion - above you meant
> set_fmt(), not try_fmt(), right? *try* are not supposed to set anything.

Yes, I meant set_fmt(), sorry.

I've just checked the code paths in which sh_mobile_ceu_set_rect() is called, 
and I don't see any issue there. We can thus simplify the data-fetch mode code 
in sh_mobile_ceu_set_rect().

> So, if you agree, either you can do a patch 11/9 or I can do it myself. Or
> you could do a v3 of just one patch 3/9.

I'd rather avoid a v3 if possible :-) If you could add a 11/9 patch that would 
be great.

-- 
Regards,

Laurent Pinchart

[PATCH 2/8] soc_camera: Use soc_camera_device::bytesperline to compute line sizes

[Laurent Pinchart]

Instead of computing the line sizes, use the previously negotiated
soc_camera_device::bytesperline value.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/mx3_camera.c           |    7 ++-----
 drivers/media/video/sh_mobile_ceu_camera.c |    4 +---
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index da45a89..c68f07e 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -265,13 +265,10 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
 	struct idmac_channel *ichan = mx3_cam->idmac_channel[0];
 	struct idmac_video_param *video = &ichan->params.video;
 	const struct soc_mbus_pixelfmt *host_fmt = icd->current_fmt->host_fmt;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, host_fmt);
 	unsigned long flags;
 	dma_cookie_t cookie;
 	size_t new_size;
 
-	BUG_ON(bytes_per_line <= 0);
-
 	new_size = icd->sizeimage;
 
 	if (vb2_plane_size(vb, 0) < new_size) {
@@ -312,9 +309,9 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
 		 * horizontal parameters in this case are expressed in bytes,
 		 * not in pixels.
 		 */
-		video->out_width	= bytes_per_line;
+		video->out_width	= icd->bytesperline;
 		video->out_height	= icd->user_height;
-		video->out_stride	= bytes_per_line;
+		video->out_stride	= icd->bytesperline;
 	} else {
 		/*
 		 * For IPU known formats the pixel unit will be managed
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index f4eb9e1..fcf96b3 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -333,9 +333,7 @@ static int sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
 		if (planar)
 			phys_addr_bottom = phys_addr_top + icd->user_width;
 		else
-			phys_addr_bottom = phys_addr_top +
-				soc_mbus_bytes_per_line(icd->user_width,
-							icd->current_fmt->host_fmt);
+			phys_addr_bottom = phys_addr_top + icd->bytesperline;
 		ceu_write(pcdev, bottom1, phys_addr_bottom);
 	}
 
-- 
1.7.3.4

Re: [PATCH 2/8] soc_camera: Use soc_camera_device::bytesperline to compute line sizes

[Guennadi Liakhovetski]

On Wed, 25 Jan 2012, Laurent Pinchart wrote:

> Instead of computing the line sizes, use the previously negotiated
> soc_camera_device::bytesperline value.

Ok, some of my comments to the previous patch should actually be applied 
to this one instead, but you'll figure it out ;-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 3/8] soc-camera: Add plane layout information to struct soc_mbus_pixelfmt

[Laurent Pinchart]

To compute the number of bytes per line according to the V4L2
specification, we need information about planes layout for planar
formats. The new enum soc_mbus_layout convey that information.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/atmel-isi.c            |    1 +
 drivers/media/video/mx3_camera.c           |    2 +
 drivers/media/video/omap1_camera.c         |    8 ++++++
 drivers/media/video/pxa_camera.c           |    1 +
 drivers/media/video/sh_mobile_ceu_camera.c |    4 +++
 drivers/media/video/soc_mediabus.c         |   33 ++++++++++++++++++++++++++++
 include/media/soc_mediabus.h               |   19 ++++++++++++++++
 7 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 73f8d05..e104b19 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -624,6 +624,7 @@ static const struct soc_mbus_pixelfmt isi_camera_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 };
 
diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index c68f07e..813323c 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -637,12 +637,14 @@ static const struct soc_mbus_pixelfmt mx3_camera_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_NONE,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	}, {
 		.fourcc			= V4L2_PIX_FMT_GREY,
 		.name			= "Monochrome 8 bit",
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_NONE,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 };
 
diff --git a/drivers/media/video/omap1_camera.c b/drivers/media/video/omap1_camera.c
index cebe4bf..76752e5 100644
--- a/drivers/media/video/omap1_camera.c
+++ b/drivers/media/video/omap1_camera.c
@@ -989,6 +989,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_VYUY8_2X8,
@@ -998,6 +999,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YUYV8_2X8,
@@ -1007,6 +1009,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YVYU8_2X8,
@@ -1016,6 +1019,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
@@ -1025,6 +1029,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
@@ -1034,6 +1039,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB565_2X8_BE,
@@ -1043,6 +1049,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB565_2X8_LE,
@@ -1052,6 +1059,7 @@ static const struct soc_mbus_lookup omap1_cam_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 },
 };
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index e7da832..e618dc4 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -1233,6 +1233,7 @@ static const struct soc_mbus_pixelfmt pxa_camera_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PLANAR_Y_U_V,
 	},
 };
 
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index fcf96b3..bd6ae79 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -957,24 +957,28 @@ static const struct soc_mbus_pixelfmt sh_mobile_ceu_formats[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_1_5X8,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PLANAR_2Y_C,
 	}, {
 		.fourcc			= V4L2_PIX_FMT_NV21,
 		.name			= "NV21",
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_1_5X8,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PLANAR_2Y_C,
 	}, {
 		.fourcc			= V4L2_PIX_FMT_NV16,
 		.name			= "NV16",
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PLANAR_Y_C,
 	}, {
 		.fourcc			= V4L2_PIX_FMT_NV61,
 		.name			= "NV61",
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PLANAR_Y_C,
 	},
 };
 
diff --git a/drivers/media/video/soc_mediabus.c b/drivers/media/video/soc_mediabus.c
index cf7f219..44dba6c 100644
--- a/drivers/media/video/soc_mediabus.c
+++ b/drivers/media/video/soc_mediabus.c
@@ -24,6 +24,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YVYU8_2X8,
@@ -33,6 +34,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_UYVY8_2X8,
@@ -42,6 +44,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_VYUY8_2X8,
@@ -51,6 +54,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
@@ -60,6 +64,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
@@ -69,6 +74,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB565_2X8_LE,
@@ -78,6 +84,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB565_2X8_BE,
@@ -87,6 +94,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR8_1X8,
@@ -96,6 +104,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_NONE,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR10_1X10,
@@ -105,6 +114,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 10,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_Y8_1X8,
@@ -114,6 +124,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_NONE,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_Y10_1X10,
@@ -123,6 +134,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 10,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE,
@@ -132,6 +144,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE,
@@ -141,6 +154,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADLO,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE,
@@ -150,6 +164,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE,
@@ -159,6 +174,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADLO,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_JPEG_1X8,
@@ -168,6 +184,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample        = 8,
 		.packing                = SOC_MBUS_PACKING_VARIABLE,
 		.order                  = SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE,
@@ -177,6 +194,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_2X8_PADHI,
 		.order			= SOC_MBUS_ORDER_BE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YUYV8_1_5X8,
@@ -186,6 +204,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_1_5X8,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YVYU8_1_5X8,
@@ -195,6 +214,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_1_5X8,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_UYVY8_1X16,
@@ -204,6 +224,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 16,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_VYUY8_1X16,
@@ -213,6 +234,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 16,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YUYV8_1X16,
@@ -222,6 +244,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 16,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_YVYU8_1X16,
@@ -231,6 +254,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 16,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SGRBG8_1X8,
@@ -240,6 +264,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_NONE,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
@@ -249,6 +274,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 8,
 		.packing		= SOC_MBUS_PACKING_NONE,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SGBRG10_1X10,
@@ -258,6 +284,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 10,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SGRBG10_1X10,
@@ -267,6 +294,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 10,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SRGGB10_1X10,
@@ -276,6 +304,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 10,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SBGGR12_1X12,
@@ -285,6 +314,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 12,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SGBRG12_1X12,
@@ -294,6 +324,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 12,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SGRBG12_1X12,
@@ -303,6 +334,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 12,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 }, {
 	.code = V4L2_MBUS_FMT_SRGGB12_1X12,
@@ -312,6 +344,7 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
 		.bits_per_sample	= 12,
 		.packing		= SOC_MBUS_PACKING_EXTEND16,
 		.order			= SOC_MBUS_ORDER_LE,
+		.layout			= SOC_MBUS_LAYOUT_PACKED,
 	},
 },
 };
diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
index 73f1e7e..18b0864 100644
--- a/include/media/soc_mediabus.h
+++ b/include/media/soc_mediabus.h
@@ -47,6 +47,24 @@ enum soc_mbus_order {
 };
 
 /**
+ * enum soc_mbus_layout - planes layout in memory
+ * @SOC_MBUS_LAYOUT_PACKED:		color components packed
+ * @SOC_MBUS_LAYOUT_PLANAR_Y_U_V:	YUV components stored in 3 planes
+ * @SOC_MBUS_LAYOUT_PLANAR_2Y_C:	YUV components stored in a luma and a
+ *					chroma plane (C plane is half the size
+ *					of Y plane)
+ * @SOC_MBUS_LAYOUT_PLANAR_Y_C:		YUV components stored in a luma and a
+ *					chroma plane (C plane is the same size
+ *					as Y plane)
+ */
+enum soc_mbus_layout {
+	SOC_MBUS_LAYOUT_PACKED = 0,
+	SOC_MBUS_LAYOUT_PLANAR_Y_U_V,
+	SOC_MBUS_LAYOUT_PLANAR_2Y_C,
+	SOC_MBUS_LAYOUT_PLANAR_Y_C,
+};
+
+/**
  * struct soc_mbus_pixelfmt - Data format on the media bus
  * @name:		Name of the format
  * @fourcc:		Fourcc code, that will be obtained if the data is
@@ -60,6 +78,7 @@ struct soc_mbus_pixelfmt {
 	u32			fourcc;
 	enum soc_mbus_packing	packing;
 	enum soc_mbus_order	order;
+	enum soc_mbus_layout	layout;
 	u8			bits_per_sample;
 };
 
-- 
1.7.3.4

Re: [PATCH 3/8] soc-camera: Add plane layout information to struct soc_mbus_pixelfmt

[Guennadi Liakhovetski]

On Wed, 25 Jan 2012, Laurent Pinchart wrote:

> To compute the number of bytes per line according to the V4L2
> specification, we need information about planes layout for planar
> formats. The new enum soc_mbus_layout convey that information.

Maybe it is better to call that value not "the number of bytes per line 
according to the V4L2 specification," but rather "the value of the 
.bytesperline field?" Also, "conveys" seems a better fit to me:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 3/8] soc-camera: Add plane layout information to struct soc_mbus_pixelfmt

[Laurent Pinchart]

Hi Guennadi,

On Thursday 26 January 2012 16:38:31 Guennadi Liakhovetski wrote:
> On Wed, 25 Jan 2012, Laurent Pinchart wrote:
> > To compute the number of bytes per line according to the V4L2
> > specification, we need information about planes layout for planar
> > formats. The new enum soc_mbus_layout convey that information.
> 
> Maybe it is better to call that value not "the number of bytes per line
> according to the V4L2 specification," but rather "the value of the
> .bytesperline field?" Also, "conveys" seems a better fit to me:-)

OK. I'll change that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/8] soc-camera: Add plane layout information to struct soc_mbus_pixelfmt

[Guennadi Liakhovetski]

One more question:

On Wed, 25 Jan 2012, Laurent Pinchart wrote:

> To compute the number of bytes per line according to the V4L2
> specification, we need information about planes layout for planar
> formats. The new enum soc_mbus_layout convey that information.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/atmel-isi.c            |    1 +
>  drivers/media/video/mx3_camera.c           |    2 +
>  drivers/media/video/omap1_camera.c         |    8 ++++++
>  drivers/media/video/pxa_camera.c           |    1 +
>  drivers/media/video/sh_mobile_ceu_camera.c |    4 +++
>  drivers/media/video/soc_mediabus.c         |   33 ++++++++++++++++++++++++++++
>  include/media/soc_mediabus.h               |   19 ++++++++++++++++
>  7 files changed, 68 insertions(+), 0 deletions(-)

[snip]

> diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
> index 73f1e7e..18b0864 100644
> --- a/include/media/soc_mediabus.h
> +++ b/include/media/soc_mediabus.h
> @@ -47,6 +47,24 @@ enum soc_mbus_order {
>  };
>  
>  /**
> + * enum soc_mbus_layout - planes layout in memory
> + * @SOC_MBUS_LAYOUT_PACKED:		color components packed
> + * @SOC_MBUS_LAYOUT_PLANAR_Y_U_V:	YUV components stored in 3 planes
> + * @SOC_MBUS_LAYOUT_PLANAR_2Y_C:	YUV components stored in a luma and a
> + *					chroma plane (C plane is half the size
> + *					of Y plane)
> + * @SOC_MBUS_LAYOUT_PLANAR_Y_C:		YUV components stored in a luma and a
> + *					chroma plane (C plane is the same size
> + *					as Y plane)
> + */
> +enum soc_mbus_layout {
> +	SOC_MBUS_LAYOUT_PACKED = 0,
> +	SOC_MBUS_LAYOUT_PLANAR_Y_U_V,

Shouldn't we call this SOC_MBUS_LAYOUT_PLANAR_2Y_U_V?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 3/8] soc-camera: Add plane layout information to struct soc_mbus_pixelfmt

[Laurent Pinchart]

Hi Guennadi,

On Thursday 26 January 2012 17:01:15 Guennadi Liakhovetski wrote:
> One more question:
> 
> On Wed, 25 Jan 2012, Laurent Pinchart wrote:
> > To compute the number of bytes per line according to the V4L2
> > specification, we need information about planes layout for planar
> > formats. The new enum soc_mbus_layout convey that information.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/video/atmel-isi.c            |    1 +
> >  drivers/media/video/mx3_camera.c           |    2 +
> >  drivers/media/video/omap1_camera.c         |    8 ++++++
> >  drivers/media/video/pxa_camera.c           |    1 +
> >  drivers/media/video/sh_mobile_ceu_camera.c |    4 +++
> >  drivers/media/video/soc_mediabus.c         |   33
> >  ++++++++++++++++++++++++++++ include/media/soc_mediabus.h              
> >  |   19 ++++++++++++++++ 7 files changed, 68 insertions(+), 0
> >  deletions(-)
> 
> [snip]
> 
> > diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
> > index 73f1e7e..18b0864 100644
> > --- a/include/media/soc_mediabus.h
> > +++ b/include/media/soc_mediabus.h
> > @@ -47,6 +47,24 @@ enum soc_mbus_order {
> > 
> >  };
> >  
> >  /**
> > 
> > + * enum soc_mbus_layout - planes layout in memory
> > + * @SOC_MBUS_LAYOUT_PACKED:		color components packed
> > + * @SOC_MBUS_LAYOUT_PLANAR_Y_U_V:	YUV components stored in 3 planes
> > + * @SOC_MBUS_LAYOUT_PLANAR_2Y_C:	YUV components stored in a luma and a
> > + *					chroma plane (C plane is half the size
> > + *					of Y plane)
> > + * @SOC_MBUS_LAYOUT_PLANAR_Y_C:		YUV components stored in a luma 
and a
> > + *					chroma plane (C plane is the same size
> > + *					as Y plane)
> > + */
> > +enum soc_mbus_layout {
> > +	SOC_MBUS_LAYOUT_PACKED = 0,
> > +	SOC_MBUS_LAYOUT_PLANAR_Y_U_V,
> 
> Shouldn't we call this SOC_MBUS_LAYOUT_PLANAR_2Y_U_V?

I'll change that.

-- 
Regards,

Laurent Pinchart

[PATCH 4/8] soc-camera: Fix bytes per line computation for planar formats

[Laurent Pinchart]

The V4L2 specification defines bytesperline for planar formats as the
number of bytes per line for the largest plane. Modify
soc_mbus_bytes_per_line() accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/soc_mediabus.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/soc_mediabus.c b/drivers/media/video/soc_mediabus.c
index 44dba6c..a707314 100644
--- a/drivers/media/video/soc_mediabus.c
+++ b/drivers/media/video/soc_mediabus.c
@@ -378,6 +378,9 @@ EXPORT_SYMBOL(soc_mbus_samples_per_pixel);
 
 s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf)
 {
+	if (mf->layout != SOC_MBUS_LAYOUT_PACKED)
+		return width * mf->bits_per_sample / 8;
+
 	switch (mf->packing) {
 	case SOC_MBUS_PACKING_NONE:
 		return width * mf->bits_per_sample / 8;
-- 
1.7.3.4

[PATCH 5/8] soc-camera: Add soc_mbus_image_size

[Laurent Pinchart]

The function returns the minimum size of an image for a given number of
bytes per line (as per the V4L2 specification), width and format.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/soc_mediabus.c |   18 ++++++++++++++++++
 include/media/soc_mediabus.h       |    2 ++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/soc_mediabus.c b/drivers/media/video/soc_mediabus.c
index a707314..3f47774 100644
--- a/drivers/media/video/soc_mediabus.c
+++ b/drivers/media/video/soc_mediabus.c
@@ -397,6 +397,24 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf)
 }
 EXPORT_SYMBOL(soc_mbus_bytes_per_line);
 
+s32 soc_mbus_image_size(u32 bytes_per_line, u32 height,
+			const struct soc_mbus_pixelfmt *mf)
+{
+	if (mf->layout == SOC_MBUS_LAYOUT_PACKED)
+		return bytes_per_line * height;
+
+	switch (mf->packing) {
+	case SOC_MBUS_PACKING_2X8_PADHI:
+	case SOC_MBUS_PACKING_2X8_PADLO:
+		return bytes_per_line * height * 2;
+	case SOC_MBUS_PACKING_1_5X8:
+		return bytes_per_line * height * 3 / 2;
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL(soc_mbus_image_size);
+
 const struct soc_mbus_pixelfmt *soc_mbus_find_fmtdesc(
 	enum v4l2_mbus_pixelcode code,
 	const struct soc_mbus_lookup *lookup,
diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
index 18b0864..eeac234 100644
--- a/include/media/soc_mediabus.h
+++ b/include/media/soc_mediabus.h
@@ -99,6 +99,8 @@ const struct soc_mbus_pixelfmt *soc_mbus_find_fmtdesc(
 const struct soc_mbus_pixelfmt *soc_mbus_get_fmtdesc(
 	enum v4l2_mbus_pixelcode code);
 s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf);
+s32 soc_mbus_image_size(u32 bytes_per_line, u32 height,
+			const struct soc_mbus_pixelfmt *mf);
 int soc_mbus_samples_per_pixel(const struct soc_mbus_pixelfmt *mf,
 			unsigned int *numerator, unsigned int *denominator);
 unsigned int soc_mbus_config_compatible(const struct v4l2_mbus_config *cfg,
-- 
1.7.3.4

Re: [PATCH 5/8] soc-camera: Add soc_mbus_image_size

[Guennadi Liakhovetski]

On Wed, 25 Jan 2012, Laurent Pinchart wrote:

> The function returns the minimum size of an image for a given number of
> bytes per line (as per the V4L2 specification), width and format.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/soc_mediabus.c |   18 ++++++++++++++++++
>  include/media/soc_mediabus.h       |    2 ++
>  2 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/soc_mediabus.c b/drivers/media/video/soc_mediabus.c
> index a707314..3f47774 100644
> --- a/drivers/media/video/soc_mediabus.c
> +++ b/drivers/media/video/soc_mediabus.c
> @@ -397,6 +397,24 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf)
>  }
>  EXPORT_SYMBOL(soc_mbus_bytes_per_line);
>  
> +s32 soc_mbus_image_size(u32 bytes_per_line, u32 height,
> +			const struct soc_mbus_pixelfmt *mf)

What do you think about making mf the first parameter? :-)

> +{
> +	if (mf->layout == SOC_MBUS_LAYOUT_PACKED)
> +		return bytes_per_line * height;
> +
> +	switch (mf->packing) {
> +	case SOC_MBUS_PACKING_2X8_PADHI:
> +	case SOC_MBUS_PACKING_2X8_PADLO:
> +		return bytes_per_line * height * 2;
> +	case SOC_MBUS_PACKING_1_5X8:
> +		return bytes_per_line * height * 3 / 2;

Hm, confused. Why have you decided to calculate the size based on packing 
and not on layout?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 5/8] soc-camera: Add soc_mbus_image_size

[Laurent Pinchart]

Hi Guennadi,

On Thursday 26 January 2012 16:59:23 Guennadi Liakhovetski wrote:
> On Wed, 25 Jan 2012, Laurent Pinchart wrote:
> > The function returns the minimum size of an image for a given number of
> > bytes per line (as per the V4L2 specification), width and format.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/video/soc_mediabus.c |   18 ++++++++++++++++++
> >  include/media/soc_mediabus.h       |    2 ++
> >  2 files changed, 20 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/video/soc_mediabus.c
> > b/drivers/media/video/soc_mediabus.c index a707314..3f47774 100644
> > --- a/drivers/media/video/soc_mediabus.c
> > +++ b/drivers/media/video/soc_mediabus.c
> > @@ -397,6 +397,24 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct
> > soc_mbus_pixelfmt *mf)
> > 
> >  }
> >  EXPORT_SYMBOL(soc_mbus_bytes_per_line);
> > 
> > +s32 soc_mbus_image_size(u32 bytes_per_line, u32 height,
> > +			const struct soc_mbus_pixelfmt *mf)
> 
> What do you think about making mf the first parameter? :-)

I copied the parameters order from soc_mbus_bytes_per_line(). I like having 
the format first, so I'll change that for soc_mbus_image_size().

> > +{
> > +	if (mf->layout == SOC_MBUS_LAYOUT_PACKED)
> > +		return bytes_per_line * height;
> > +
> > +	switch (mf->packing) {
> > +	case SOC_MBUS_PACKING_2X8_PADHI:
> > +	case SOC_MBUS_PACKING_2X8_PADLO:
> > +		return bytes_per_line * height * 2;
> > +	case SOC_MBUS_PACKING_1_5X8:
> > +		return bytes_per_line * height * 3 / 2;
> 
> Hm, confused. Why have you decided to calculate the size based on packing
> and not on layout?

Because planar YUV 4:2:0, 4:2:2 and 4:4:4 formats would all use 
SOC_MBUS_LAYOUT_Y_U_V. I could create SOC_MBUS_LAYOUT_2Y_U_V and 
SOC_MBUS_LAYOUT_4Y_U_V instead. As existing planar formats all have a 
bits_per_sample value of 8, mf->packing was already used by 
soc_mbus_bytes_per_line() (before my patches) to compute the total line size 
in bytes, so I thought it made sense to reuse it in soc_mbus_image_size().

-- 
Regards,

Laurent Pinchart

[PATCH 6/8] soc-camera: Honor user-requested bytesperline and sizeimage

[Laurent Pinchart]

Compute the bytesperline and sizeimage values when trying/setting
formats or when allocating buffers by taking the user-requested values
into account.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/mx3_camera.c           |   20 +++++++++++++-----
 drivers/media/video/sh_mobile_ceu_camera.c |   20 +++++++++++++-----
 drivers/media/video/soc_camera.c           |   29 ++++++++++++++-------------
 3 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index 813323c..4fd62d1 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -206,17 +206,25 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
 	if (fmt) {
 		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
 								fmt->fmt.pix.pixelformat);
-		int bytes_per_line;
+		unsigned int bytes_per_line;
+		int ret;
 
 		if (!xlate)
 			return -EINVAL;
 
-		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
-							 xlate->host_fmt);
-		if (bytes_per_line < 0)
-			return bytes_per_line;
+		ret = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
+					      xlate->host_fmt);
+		if (ret < 0)
+			return ret;
+
+		bytes_per_line = max_t(u32, fmt->fmt.pix.bytesperline, ret);
+
+		ret = soc_mbus_image_size(bytes_per_line, fmt->fmt.pix.height,
+					  xlate->host_fmt);
+		if (ret < 0)
+			return ret;
 
-		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
+		sizes[0] = max_t(u32, fmt->fmt.pix.sizeimage, ret);
 	} else {
 		/* Called from VIDIOC_REQBUFS or in compatibility mode */
 		sizes[0] = icd->sizeimage;
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index bd6ae79..47b336b 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -210,17 +210,25 @@ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq,
 	if (fmt) {
 		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
 								fmt->fmt.pix.pixelformat);
-		int bytes_per_line;
+		unsigned int bytes_per_line;
+		int ret;
 
 		if (!xlate)
 			return -EINVAL;
 
-		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
-							 xlate->host_fmt);
-		if (bytes_per_line < 0)
-			return bytes_per_line;
+		ret = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
+					      xlate->host_fmt);
+		if (ret < 0)
+			return ret;
+
+		bytes_per_line = max_t(u32, fmt->fmt.pix.bytesperline, ret);
+
+		ret = soc_mbus_image_size(bytes_per_line, fmt->fmt.pix.height,
+					  xlate->host_fmt);
+		if (ret < 0)
+			return ret;
 
-		sizes[0] = bytes_per_line * fmt->fmt.pix.height;
+		sizes[0] = max_t(u32, fmt->fmt.pix.sizeimage, ret);
 	} else {
 		/* Called from VIDIOC_REQBUFS or in compatibility mode */
 		sizes[0] = icd->sizeimage;
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 62e4312..b49ad27 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -164,6 +164,7 @@ static int soc_camera_try_fmt(struct soc_camera_device *icd,
 			      struct v4l2_format *f)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	const struct soc_camera_format_xlate *xlate;
 	struct v4l2_pix_format *pix = &f->fmt.pix;
 	int ret;
 
@@ -177,22 +178,22 @@ static int soc_camera_try_fmt(struct soc_camera_device *icd,
 	if (ret < 0)
 		return ret;
 
-	if (!pix->sizeimage) {
-		if (!pix->bytesperline) {
-			const struct soc_camera_format_xlate *xlate;
+	xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
+	if (!xlate)
+		return -EINVAL;
 
-			xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
-			if (!xlate)
-				return -EINVAL;
+	ret = soc_mbus_bytes_per_line(pix->width, xlate->host_fmt);
+	if (ret < 0)
+		return ret;
 
-			ret = soc_mbus_bytes_per_line(pix->width,
-						      xlate->host_fmt);
-			if (ret > 0)
-				pix->bytesperline = ret;
-		}
-		if (pix->bytesperline)
-			pix->sizeimage = pix->bytesperline * pix->height;
-	}
+	pix->bytesperline = max_t(u32, pix->bytesperline, ret);
+
+	ret = soc_mbus_image_size(pix->bytesperline, pix->height,
+				  xlate->host_fmt);
+	if (ret < 0)
+		return ret;
+
+	pix->sizeimage = max_t(u32, pix->sizeimage, ret);
 
 	return 0;
 }
-- 
1.7.3.4

[PATCH 7/8] soc-camera: Support user-configurable line stride

[Laurent Pinchart]

Add a capabilities field to the soc_camera_host structure to flag hosts
that support user-configurable line strides. soc_camera_try_fmt() then
passes the user-provided bytesperline and sizeimage format fields to
such hosts, and expects the host to check (and fix if needed) the
values.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/mx2_camera.c |    2 ++
 drivers/media/video/soc_camera.c |    6 ++++--
 include/media/soc_camera.h       |    4 ++++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index e9b228d..e8c8021 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1430,6 +1430,8 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
 	pcdev->soc_host.priv		= pcdev;
 	pcdev->soc_host.v4l2_dev.dev	= &pdev->dev;
 	pcdev->soc_host.nr		= pdev->id;
+	if (cpu_is_mx25()) {
+		pcdev->soc_host.capabilities = SOCAM_HOST_CAP_STRIDE;
 	err = soc_camera_host_register(&pcdev->soc_host);
 	if (err)
 		goto exit_free_emma;
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index b49ad27..84aabc3 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -171,8 +171,10 @@ static int soc_camera_try_fmt(struct soc_camera_device *icd,
 	dev_dbg(icd->pdev, "TRY_FMT(%c%c%c%c, %ux%u)\n",
 		pixfmtstr(pix->pixelformat), pix->width, pix->height);
 
-	pix->bytesperline = 0;
-	pix->sizeimage = 0;
+	if (!(ici->capabilities & SOCAM_HOST_CAP_STRIDE)) {
+		pix->bytesperline = 0;
+		pix->sizeimage = 0;
+	}
 
 	ret = ici->ops->try_fmt(icd, f);
 	if (ret < 0)
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 5fb2c3d..b7d1d9b 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -56,10 +56,14 @@ struct soc_camera_device {
 	};
 };
 
+/* Host supports programmable stride */
+#define SOCAM_HOST_CAP_STRIDE		(1 << 0)
+
 struct soc_camera_host {
 	struct v4l2_device v4l2_dev;
 	struct list_head list;
 	unsigned char nr;				/* Host number */
+	u32 capabilities;
 	void *priv;
 	const char *drv_name;
 	struct soc_camera_host_ops *ops;
-- 
1.7.3.4

[PATCH 8/8] sh_mobile_ceu_camera: Support user-configurable line stride

[Laurent Pinchart]

In image mode, the CEU allows configurable line strides up to 8188
pixels.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/sh_mobile_ceu_camera.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 47b336b..ac98f47 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -338,19 +338,15 @@ static int sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
 
 	ceu_write(pcdev, top1, phys_addr_top);
 	if (V4L2_FIELD_NONE != pcdev->field) {
-		if (planar)
-			phys_addr_bottom = phys_addr_top + icd->user_width;
-		else
-			phys_addr_bottom = phys_addr_top + icd->bytesperline;
+		phys_addr_bottom = phys_addr_top + icd->bytesperline;
 		ceu_write(pcdev, bottom1, phys_addr_bottom);
 	}
 
 	if (planar) {
-		phys_addr_top += icd->user_width *
-			icd->user_height;
+		phys_addr_top += icd->bytesperline * icd->user_height;
 		ceu_write(pcdev, top2, phys_addr_top);
 		if (V4L2_FIELD_NONE != pcdev->field) {
-			phys_addr_bottom = phys_addr_top + icd->user_width;
+			phys_addr_bottom = phys_addr_top + icd->bytesperline;
 			ceu_write(pcdev, bottom2, phys_addr_bottom);
 		}
 	}
@@ -677,7 +673,7 @@ static void sh_mobile_ceu_set_rect(struct soc_camera_device *icd)
 			in_width *= 2;
 			left_offset *= 2;
 		}
-		cdwdr_width = width;
+		cdwdr_width = icd->bytesperline;
 	} else {
 		int bytes_per_line = soc_mbus_bytes_per_line(width,
 						icd->current_fmt->host_fmt);
@@ -1840,6 +1836,8 @@ static int sh_mobile_ceu_set_fmt(struct soc_camera_device *icd,
 	return 0;
 }
 
+#define CEU_CHDW_MAX	8188U	/* Maximum line stride */
+
 static int sh_mobile_ceu_try_fmt(struct soc_camera_device *icd,
 				 struct v4l2_format *f)
 {
@@ -1916,10 +1914,20 @@ static int sh_mobile_ceu_try_fmt(struct soc_camera_device *icd,
 			pix->width = width;
 		if (mf.height > height)
 			pix->height = height;
+
+		pix->bytesperline = max(pix->bytesperline, pix->width);
+		pix->bytesperline = min(pix->bytesperline, CEU_CHDW_MAX);
+		pix->bytesperline &= ~3;
+		break;
+
+	default:
+		/* Configurable stride isn't supported in pass-through mode. */
+		pix->bytesperline  = 0;
 	}
 
 	pix->width	&= ~3;
 	pix->height	&= ~3;
+	pix->sizeimage	= 0;
 
 	dev_geo(icd->parent, "%s(): return %d, fmt 0x%x, %ux%u\n",
 		__func__, ret, pix->pixelformat, pix->width, pix->height);
@@ -2136,6 +2144,7 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
 	pcdev->ici.nr = pdev->id;
 	pcdev->ici.drv_name = dev_name(&pdev->dev);
 	pcdev->ici.ops = &sh_mobile_ceu_host_ops;
+	pcdev->ici.capabilities = SOCAM_HOST_CAP_STRIDE;
 
 	pcdev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
 	if (IS_ERR(pcdev->alloc_ctx)) {
-- 
1.7.3.4