[PATCH 00/03] video: nv1x/nvx1 support for the sh_mobile_ceu driver V2

[PATCH 00/03] video: nv1x/nvx1 support for the sh_mobile_ceu driver V2

[Magnus Damm]

NV12/NV21/NV16/NV61 support V2:

[PATCH 01/03] sh_mobile_ceu: use new pixel format translation code
[PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support
[PATCH 03/03] sh_mobile_ceu: add NV16 and NV61 support

This patchset is the second version of the nv12/nv21 patch named:
"video: nv12/nv21 support for the sh_mobile_ceu driver". This time
YUV support is added by using the new pixel format translation code.
The bulk of the work is done by the NV12 and NV21 patch and it only
requires the pixel format translation patches. NV16 and NV61 support
also requires the NV16/NV61 fourcc patch recently posted.

Tested with ov772x using soc_camera_platform and the ov772x
driver on a Migo-R QVGA board.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/media/video/sh_mobile_ceu_camera.c |  219 ++++++++++++++++++++++------
 1 file changed, 176 insertions(+), 43 deletions(-)

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 01/03] sh_mobile_ceu: use new pixel format translation code

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

This patch converts the sh_mobile_ceu driver to make use
of the new pixel format translation code. Only pass-though
mode at this point.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/media/video/sh_mobile_ceu_camera.c |   83 +++++++++++++++++++---------
 1 file changed, 57 insertions(+), 26 deletions(-)

--- 0008/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c	2008-12-09 22:56:02.000000000 +0900
@@ -434,8 +434,7 @@ static int sh_mobile_ceu_set_bus_param(s
 	return 0;
 }
 
-static int sh_mobile_ceu_try_bus_param(struct soc_camera_device *icd,
-				       __u32 pixfmt)
+static int sh_mobile_ceu_try_bus_param(struct soc_camera_device *icd)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
@@ -450,25 +449,60 @@ static int sh_mobile_ceu_try_bus_param(s
 	return 0;
 }
 
+static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, int idx,
+				     struct soc_camera_format_xlate *xlate)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	int ret;
+	int formats = 0;
+
+	ret = sh_mobile_ceu_try_bus_param(icd);
+	if (ret < 0)
+		return 0;
+
+	switch (icd->formats[idx].fourcc) {
+	default:
+		/* Generic pass-through */
+		formats++;
+		if (xlate) {
+			xlate->host_fmt = icd->formats + idx;
+			xlate->cam_fmt = icd->formats + idx;
+			xlate->buswidth = icd->formats[idx].depth;
+			xlate++;
+			dev_dbg(&ici->dev,
+				"Providing format %s in pass-through mode\n",
+				icd->formats[idx].name);
+		}
+	}
+
+	return formats;
+}
+
 static int sh_mobile_ceu_set_fmt(struct soc_camera_device *icd,
 				 __u32 pixfmt, struct v4l2_rect *rect)
 {
-	const struct soc_camera_data_format *cam_fmt;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	const struct soc_camera_format_xlate *xlate;
 	int ret;
 
-	/*
-	 * TODO: find a suitable supported by the SoC output format, check
-	 * whether the sensor supports one of acceptable input formats.
-	 */
-	if (pixfmt) {
-		cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
-		if (!cam_fmt)
-			return -EINVAL;
+	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
+	if (!xlate) {
+		dev_warn(&ici->dev, "Format %x not found\n", pixfmt);
+		return -EINVAL;
+	}
+
+	switch (pixfmt) {
+	case 0:				/* Only geometry change */
+		ret = icd->ops->set_fmt(icd, pixfmt, rect);
+		break;
+	default:
+		ret = icd->ops->set_fmt(icd, xlate->cam_fmt->fourcc, rect);
 	}
 
-	ret = icd->ops->set_fmt(icd, pixfmt, rect);
-	if (pixfmt && !ret)
-		icd->current_fmt = cam_fmt;
+	if (pixfmt && !ret) {
+		icd->buswidth = xlate->buswidth;
+		icd->current_fmt = xlate->host_fmt;
+	}
 
 	return ret;
 }
@@ -476,19 +510,15 @@ static int sh_mobile_ceu_set_fmt(struct 
 static int sh_mobile_ceu_try_fmt(struct soc_camera_device *icd,
 				 struct v4l2_format *f)
 {
-	const struct soc_camera_data_format *cam_fmt;
-	int ret = sh_mobile_ceu_try_bus_param(icd, f->fmt.pix.pixelformat);
-
-	if (ret < 0)
-		return ret;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	const struct soc_camera_format_xlate *xlate;
+	__u32 pixfmt = f->fmt.pix.pixelformat;
 
-	/*
-	 * TODO: find a suitable supported by the SoC output format, check
-	 * whether the sensor supports one of acceptable input formats.
-	 */
-	cam_fmt = soc_camera_format_by_fourcc(icd, f->fmt.pix.pixelformat);
-	if (!cam_fmt)
+	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
+	if (!xlate) {
+		dev_warn(&ici->dev, "Format %x not found\n", pixfmt);
 		return -EINVAL;
+	}
 
 	/* FIXME: calculate using depth and bus width */
 
@@ -504,7 +534,7 @@ static int sh_mobile_ceu_try_fmt(struct 
 	f->fmt.pix.height &= ~0x03;
 
 	f->fmt.pix.bytesperline = f->fmt.pix.width *
-		DIV_ROUND_UP(cam_fmt->depth, 8);
+		DIV_ROUND_UP(xlate->host_fmt->depth, 8);
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
 
 	/* limit to sensor capabilities */
@@ -576,6 +606,7 @@ static struct soc_camera_host_ops sh_mob
 	.owner		= THIS_MODULE,
 	.add		= sh_mobile_ceu_add_device,
 	.remove		= sh_mobile_ceu_remove_device,
+	.get_formats	= sh_mobile_ceu_get_formats,
 	.set_fmt	= sh_mobile_ceu_set_fmt,
 	.try_fmt	= sh_mobile_ceu_try_fmt,
 	.reqbufs	= sh_mobile_ceu_reqbufs,

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

This patch adds NV12/NV21 support to the sh_mobile_ceu driver.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
 1 file changed, 99 insertions(+), 15 deletions(-)

--- 0031/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c	2008-12-10 13:09:43.000000000 +0900
@@ -99,6 +99,8 @@ struct sh_mobile_ceu_dev {
 	struct videobuf_buffer *active;
 
 	struct sh_mobile_ceu_info *pdata;
+
+	const struct soc_camera_data_format *camera_fmt;
 };
 
 static void ceu_write(struct sh_mobile_ceu_dev *priv,
@@ -158,6 +160,9 @@ static void free_buffer(struct videobuf_
 
 static void sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
 {
+	struct soc_camera_device *icd = pcdev->icd;
+	unsigned long phys_addr;
+
 	ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) & ~1);
 	ceu_write(pcdev, CETCR, ~ceu_read(pcdev, CETCR) & 0x0317f313);
 	ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) | 1);
@@ -166,11 +171,21 @@ static void sh_mobile_ceu_capture(struct
 
 	ceu_write(pcdev, CETCR, 0x0317f313 ^ 0x10);
 
-	if (pcdev->active) {
-		pcdev->active->state = VIDEOBUF_ACTIVE;
-		ceu_write(pcdev, CDAYR, videobuf_to_dma_contig(pcdev->active));
-		ceu_write(pcdev, CAPSR, 0x1); /* start capture */
+	if (!pcdev->active)
+		return;
+
+	phys_addr = videobuf_to_dma_contig(pcdev->active);
+	ceu_write(pcdev, CDAYR, phys_addr);
+
+	switch (icd->current_fmt->fourcc) {
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		phys_addr += (icd->width * icd->height);
+		ceu_write(pcdev, CDACR, phys_addr);
 	}
+
+	pcdev->active->state = VIDEOBUF_ACTIVE;
+	ceu_write(pcdev, CAPSR, 0x1); /* start capture */
 }
 
 static int sh_mobile_ceu_videobuf_prepare(struct videobuf_queue *vq,
@@ -364,6 +379,7 @@ static int sh_mobile_ceu_set_bus_param(s
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
 	int ret, buswidth, width, cfszr_width, cdwdr_width;
 	unsigned long camera_flags, common_flags, value;
+	int yuv_mode, yuv_lineskip;
 
 	camera_flags = icd->ops->query_bus_param(icd);
 	common_flags = soc_camera_bus_param_compatible(camera_flags,
@@ -389,7 +405,35 @@ static int sh_mobile_ceu_set_bus_param(s
 	ceu_write(pcdev, CRCNTR, 0);
 	ceu_write(pcdev, CRCMPR, 0);
 
-	value = 0x00000010;
+	value = 0x00000010; /* data fetch by default */
+	yuv_mode = yuv_lineskip = 0;
+
+	switch (icd->current_fmt->fourcc) {
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		yuv_lineskip = 1; /* skip for NV12/21, no skip for NV16/61 */
+		yuv_mode = 1;
+		switch (pcdev->camera_fmt->fourcc) {
+		case V4L2_PIX_FMT_UYVY:
+			value = 0x00000000; /* Cb0, Y0, Cr0, Y1 */
+			break;
+		case V4L2_PIX_FMT_VYUY:
+			value = 0x00000100; /* Cr0, Y0, Cb0, Y1 */
+			break;
+		case V4L2_PIX_FMT_YUYV:
+			value = 0x00000200; /* Y0, Cb0, Y1, Cr0 */
+			break;
+		case V4L2_PIX_FMT_YVYU:
+			value = 0x00000300; /* Y0, Cr0, Y1, Cb0 */
+			break;
+		default:
+			BUG();
+		}
+	}
+
+	if (icd->current_fmt->fourcc == V4L2_PIX_FMT_NV21)
+		value ^= 0x00000100; /* swap U, V to change from NV12->NV21 */
+
 	value |= (common_flags & SOCAM_VSYNC_ACTIVE_LOW) ? (1 << 1) : 0;
 	value |= (common_flags & SOCAM_HSYNC_ACTIVE_LOW) ? (1 << 0) : 0;
 	value |= (buswidth == 16) ? (1 << 12) : 0;
@@ -400,16 +444,22 @@ static int sh_mobile_ceu_set_bus_param(s
 
 	mdelay(1);
 
-	width = icd->width * (icd->current_fmt->depth / 8);
-	width = (buswidth == 16) ? width / 2 : width;
-	cfszr_width = (buswidth == 8) ? width / 2 : width;
-	cdwdr_width = (buswidth == 16) ? width * 2 : width;
+	if (yuv_mode) {
+		width = icd->width * 2;
+		width = (buswidth == 16) ? width / 2 : width;
+		cfszr_width = cdwdr_width = icd->width;
+	} else {
+		width = icd->width * ((icd->current_fmt->depth + 7) >> 3);
+		width = (buswidth == 16) ? width / 2 : width;
+		cfszr_width = (buswidth == 8) ? width / 2 : width;
+		cdwdr_width = (buswidth == 16) ? width * 2 : width;
+	}
 
 	ceu_write(pcdev, CAMOR, 0);
 	ceu_write(pcdev, CAPWR, (icd->height << 16) | width);
-	ceu_write(pcdev, CFLCR, 0); /* data fetch mode - no scaling */
+	ceu_write(pcdev, CFLCR, 0); /* no scaling */
 	ceu_write(pcdev, CFSZR, (icd->height << 16) | cfszr_width);
-	ceu_write(pcdev, CLFCR, 0); /* data fetch mode - no lowpass filter */
+	ceu_write(pcdev, CLFCR, 0); /* no lowpass filter */
 
 	/* A few words about byte order (observed in Big Endian mode)
 	 *
@@ -423,14 +473,16 @@ static int sh_mobile_ceu_set_bus_param(s
 	 * using 7 we swap the data bytes to match the incoming order:
 	 * D0, D1, D2, D3, D4, D5, D6, D7
 	 */
-	ceu_write(pcdev, CDOCR, 0x00000017);
+	value = 0x00000017;
+	if (yuv_lineskip)
+		value &= ~0x00000010; /* convert 4:2:2 -> 4:2:0 */
+
+	ceu_write(pcdev, CDOCR, value);
 
 	ceu_write(pcdev, CDWDR, cdwdr_width);
 	ceu_write(pcdev, CFWCR, 0); /* keep "datafetch firewall" disabled */
 
 	/* not in bundle mode: skip CBDSR, CDAYR2, CDACR2, CDBYR2, CDBCR2 */
-	/* in data fetch mode: no need for CDACR, CDBYR, CDBCR */
-
 	return 0;
 }
 
@@ -449,11 +501,26 @@ static int sh_mobile_ceu_try_bus_param(s
 	return 0;
 }
 
+static const struct soc_camera_data_format sh_mobile_ceu_formats[] = {
+	{
+		.name		= "NV12",
+		.depth		= 12,
+		.fourcc		= V4L2_PIX_FMT_NV12,
+		.colorspace	= V4L2_COLORSPACE_JPEG,
+	},
+	{
+		.name		= "NV21",
+		.depth		= 12,
+		.fourcc		= V4L2_PIX_FMT_NV21,
+		.colorspace	= V4L2_COLORSPACE_JPEG,
+	},
+};
+
 static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, int idx,
 				     struct soc_camera_format_xlate *xlate)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
-	int ret;
+	int ret, k, n;
 	int formats = 0;
 
 	ret = sh_mobile_ceu_try_bus_param(icd);
@@ -461,6 +528,21 @@ static int sh_mobile_ceu_get_formats(str
 		return 0;
 
 	switch (icd->formats[idx].fourcc) {
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_VYUY:
+	case V4L2_PIX_FMT_YUYV:
+	case V4L2_PIX_FMT_YVYU:
+		n = ARRAY_SIZE(sh_mobile_ceu_formats);
+		formats += n;
+		for (k = 0; xlate && k < n; k++) {
+			xlate->host_fmt = &sh_mobile_ceu_formats[k];
+			xlate->cam_fmt = icd->formats + idx;
+			xlate->buswidth = icd->formats[idx].depth;
+			xlate++;
+			dev_dbg(&ici->dev, "Providing format %s using %s\n",
+				sh_mobile_ceu_formats[k].name,
+				icd->formats[idx].name);
+		}
 	default:
 		/* Generic pass-through */
 		formats++;
@@ -482,6 +564,7 @@ static int sh_mobile_ceu_set_fmt(struct 
 				 __u32 pixfmt, struct v4l2_rect *rect)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct sh_mobile_ceu_dev *pcdev = ici->priv;
 	const struct soc_camera_format_xlate *xlate;
 	int ret;
 
@@ -502,6 +585,7 @@ static int sh_mobile_ceu_set_fmt(struct 
 	if (pixfmt && !ret) {
 		icd->buswidth = xlate->buswidth;
 		icd->current_fmt = xlate->host_fmt;
+		pcdev->camera_fmt = xlate->cam_fmt;
 	}
 
 	return ret;

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 03/03] sh_mobile_ceu: add NV16 and NV61 support

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

This patch adds NV16/NV61 support to the sh_mobile_ceu driver.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/media/video/sh_mobile_ceu_camera.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

--- 0021/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c	2008-12-10 00:12:02.000000000 +0900
@@ -180,6 +180,8 @@ static void sh_mobile_ceu_capture(struct
 	switch (icd->current_fmt->fourcc) {
 	case V4L2_PIX_FMT_NV12:
 	case V4L2_PIX_FMT_NV21:
+	case V4L2_PIX_FMT_NV16:
+	case V4L2_PIX_FMT_NV61:
 		phys_addr += (icd->width * icd->height);
 		ceu_write(pcdev, CDACR, phys_addr);
 	}
@@ -412,6 +414,9 @@ static int sh_mobile_ceu_set_bus_param(s
 	case V4L2_PIX_FMT_NV12:
 	case V4L2_PIX_FMT_NV21:
 		yuv_lineskip = 1; /* skip for NV12/21, no skip for NV16/61 */
+		/* fall-through */
+	case V4L2_PIX_FMT_NV16:
+	case V4L2_PIX_FMT_NV61:
 		yuv_mode = 1;
 		switch (pcdev->camera_fmt->fourcc) {
 		case V4L2_PIX_FMT_UYVY:
@@ -431,8 +436,9 @@ static int sh_mobile_ceu_set_bus_param(s
 		}
 	}
 
-	if (icd->current_fmt->fourcc == V4L2_PIX_FMT_NV21)
-		value ^= 0x00000100; /* swap U, V to change from NV12->NV21 */
+	if ((icd->current_fmt->fourcc == V4L2_PIX_FMT_NV21) ||
+	    (icd->current_fmt->fourcc == V4L2_PIX_FMT_NV61))
+		value ^= 0x00000100; /* swap U, V to change from NV1x->NVx1 */
 
 	value |= (common_flags & SOCAM_VSYNC_ACTIVE_LOW) ? (1 << 1) : 0;
 	value |= (common_flags & SOCAM_HSYNC_ACTIVE_LOW) ? (1 << 0) : 0;
@@ -514,6 +520,18 @@ static const struct soc_camera_data_form
 		.fourcc		= V4L2_PIX_FMT_NV21,
 		.colorspace	= V4L2_COLORSPACE_JPEG,
 	},
+	{
+		.name		= "NV16",
+		.depth		= 16,
+		.fourcc		= V4L2_PIX_FMT_NV16,
+		.colorspace	= V4L2_COLORSPACE_JPEG,
+	},
+	{
+		.name		= "NV61",
+		.depth		= 16,
+		.fourcc		= V4L2_PIX_FMT_NV61,
+		.colorspace	= V4L2_COLORSPACE_JPEG,
+	},
 };
 
 static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, int idx,

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 00/03] video: nv1x/nvx1 support for the sh_mobile_ceu driver V2

[morimoto.kuninori]

Hi Magnus, Guennadi

> [PATCH 01/03] sh_mobile_ceu: use new pixel format translation code
> [PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support

It works well on my environment !!
I can not test NV16/61
Thanks

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

Best regards
--
Kuninori Morimoto

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 00/03] video: nv1x/nvx1 support for the sh_mobile_ceu driver V2

[Magnus Damm]

Hi Morimoto-san,

On Fri, Dec 12, 2008 at 5:37 PM,  <morimoto.kuninori@renesas.com> wrote:
>
> Hi Magnus, Guennadi
>
>> [PATCH 01/03] sh_mobile_ceu: use new pixel format translation code
>> [PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support
>
> It works well on my environment !!
> I can not test NV16/61
> Thanks
>
> Acked-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>

Thanks for testing. Regarding NV16 and NV61, there is very little
support in open source tools for these formats. I've heard a positive
NV16 status report from other people using an internal NV16-extended
version of the my old NV12 patch, so I'm fairly confident in my NV16
implementation. Especially since the only logic changes from NV12 are
12bpp->16bpp and the no-lineskip setting. =)

Cheers,

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support

[Guennadi Liakhovetski]

On Wed, 10 Dec 2008, Magnus Damm wrote:

> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch adds NV12/NV21 support to the sh_mobile_ceu driver.
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
> 
>  drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
>  1 file changed, 99 insertions(+), 15 deletions(-)
> 
> --- 0031/drivers/media/video/sh_mobile_ceu_camera.c
> +++ work/drivers/media/video/sh_mobile_ceu_camera.c	2008-12-10 13:09:43.000000000 +0900
> @@ -99,6 +99,8 @@ struct sh_mobile_ceu_dev {
>  	struct videobuf_buffer *active;
>  
>  	struct sh_mobile_ceu_info *pdata;
> +
> +	const struct soc_camera_data_format *camera_fmt;
>  };
>  
>  static void ceu_write(struct sh_mobile_ceu_dev *priv,
> @@ -158,6 +160,9 @@ static void free_buffer(struct videobuf_
>  
>  static void sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
>  {
> +	struct soc_camera_device *icd = pcdev->icd;
> +	unsigned long phys_addr;

dma_addr_t

> +
>  	ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) & ~1);
>  	ceu_write(pcdev, CETCR, ~ceu_read(pcdev, CETCR) & 0x0317f313);
>  	ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) | 1);
> @@ -166,11 +171,21 @@ static void sh_mobile_ceu_capture(struct
>  
>  	ceu_write(pcdev, CETCR, 0x0317f313 ^ 0x10);
>  
> -	if (pcdev->active) {
> -		pcdev->active->state = VIDEOBUF_ACTIVE;
> -		ceu_write(pcdev, CDAYR, videobuf_to_dma_contig(pcdev->active));
> -		ceu_write(pcdev, CAPSR, 0x1); /* start capture */
> +	if (!pcdev->active)
> +		return;
> +
> +	phys_addr = videobuf_to_dma_contig(pcdev->active);
> +	ceu_write(pcdev, CDAYR, phys_addr);

Hm, looks like someone could have reviewed this driver a bit better on 
submission:-) I think, your ceu_write() should really take a u32 as an 
argument, not unsigned long, respectively, ceu_read() should return u32. 
So there is even less motivation for "unsigned long phys_addr" above. A 
patch to change these functions would be welcome:-)

> +
> +	switch (icd->current_fmt->fourcc) {
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +		phys_addr += (icd->width * icd->height);

superfluous parenthesis

> +		ceu_write(pcdev, CDACR, phys_addr);
>  	}
> +
> +	pcdev->active->state = VIDEOBUF_ACTIVE;
> +	ceu_write(pcdev, CAPSR, 0x1); /* start capture */
>  }
>  
>  static int sh_mobile_ceu_videobuf_prepare(struct videobuf_queue *vq,
> @@ -364,6 +379,7 @@ static int sh_mobile_ceu_set_bus_param(s
>  	struct sh_mobile_ceu_dev *pcdev = ici->priv;
>  	int ret, buswidth, width, cfszr_width, cdwdr_width;
>  	unsigned long camera_flags, common_flags, value;
> +	int yuv_mode, yuv_lineskip;
>  
>  	camera_flags = icd->ops->query_bus_param(icd);
>  	common_flags = soc_camera_bus_param_compatible(camera_flags,
> @@ -389,7 +405,35 @@ static int sh_mobile_ceu_set_bus_param(s
>  	ceu_write(pcdev, CRCNTR, 0);
>  	ceu_write(pcdev, CRCMPR, 0);
>  
> -	value = 0x00000010;
> +	value = 0x00000010; /* data fetch by default */
> +	yuv_mode = yuv_lineskip = 0;
> +
> +	switch (icd->current_fmt->fourcc) {
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_NV21:
> +		yuv_lineskip = 1; /* skip for NV12/21, no skip for NV16/61 */
> +		yuv_mode = 1;
> +		switch (pcdev->camera_fmt->fourcc) {
> +		case V4L2_PIX_FMT_UYVY:
> +			value = 0x00000000; /* Cb0, Y0, Cr0, Y1 */
> +			break;
> +		case V4L2_PIX_FMT_VYUY:
> +			value = 0x00000100; /* Cr0, Y0, Cb0, Y1 */
> +			break;
> +		case V4L2_PIX_FMT_YUYV:
> +			value = 0x00000200; /* Y0, Cb0, Y1, Cr0 */
> +			break;
> +		case V4L2_PIX_FMT_YVYU:
> +			value = 0x00000300; /* Y0, Cr0, Y1, Cb0 */
> +			break;
> +		default:
> +			BUG();
> +		}
> +	}
> +
> +	if (icd->current_fmt->fourcc == V4L2_PIX_FMT_NV21)
> +		value ^= 0x00000100; /* swap U, V to change from NV12->NV21 */
> +
>  	value |= (common_flags & SOCAM_VSYNC_ACTIVE_LOW) ? (1 << 1) : 0;
>  	value |= (common_flags & SOCAM_HSYNC_ACTIVE_LOW) ? (1 << 0) : 0;
>  	value |= (buswidth == 16) ? (1 << 12) : 0;

superfluous parenthesis before and after "?" - this and other occurrences 
would make a nice cleanup patch:-)

> @@ -400,16 +444,22 @@ static int sh_mobile_ceu_set_bus_param(s
>  
>  	mdelay(1);
>  
> -	width = icd->width * (icd->current_fmt->depth / 8);
> -	width = (buswidth == 16) ? width / 2 : width;
> -	cfszr_width = (buswidth == 8) ? width / 2 : width;
> -	cdwdr_width = (buswidth == 16) ? width * 2 : width;
> +	if (yuv_mode) {
> +		width = icd->width * 2;
> +		width = (buswidth == 16) ? width / 2 : width;

superfluous parenthesis

> +		cfszr_width = cdwdr_width = icd->width;
> +	} else {
> +		width = icd->width * ((icd->current_fmt->depth + 7) >> 3);
> +		width = (buswidth == 16) ? width / 2 : width;
> +		cfszr_width = (buswidth == 8) ? width / 2 : width;
> +		cdwdr_width = (buswidth == 16) ? width * 2 : width;

ditto - in last 3 lines

> +	}
>  
>  	ceu_write(pcdev, CAMOR, 0);
>  	ceu_write(pcdev, CAPWR, (icd->height << 16) | width);
> -	ceu_write(pcdev, CFLCR, 0); /* data fetch mode - no scaling */
> +	ceu_write(pcdev, CFLCR, 0); /* no scaling */
>  	ceu_write(pcdev, CFSZR, (icd->height << 16) | cfszr_width);
> -	ceu_write(pcdev, CLFCR, 0); /* data fetch mode - no lowpass filter */
> +	ceu_write(pcdev, CLFCR, 0); /* no lowpass filter */
>  
>  	/* A few words about byte order (observed in Big Endian mode)
>  	 *
> @@ -423,14 +473,16 @@ static int sh_mobile_ceu_set_bus_param(s
>  	 * using 7 we swap the data bytes to match the incoming order:
>  	 * D0, D1, D2, D3, D4, D5, D6, D7
>  	 */
> -	ceu_write(pcdev, CDOCR, 0x00000017);
> +	value = 0x00000017;
> +	if (yuv_lineskip)
> +		value &= ~0x00000010; /* convert 4:2:2 -> 4:2:0 */
> +
> +	ceu_write(pcdev, CDOCR, value);
>  
>  	ceu_write(pcdev, CDWDR, cdwdr_width);
>  	ceu_write(pcdev, CFWCR, 0); /* keep "datafetch firewall" disabled */
>  
>  	/* not in bundle mode: skip CBDSR, CDAYR2, CDACR2, CDBYR2, CDBCR2 */
> -	/* in data fetch mode: no need for CDACR, CDBYR, CDBCR */
> -
>  	return 0;
>  }
>  
> @@ -449,11 +501,26 @@ static int sh_mobile_ceu_try_bus_param(s
>  	return 0;
>  }
>  
> +static const struct soc_camera_data_format sh_mobile_ceu_formats[] = {
> +	{
> +		.name		= "NV12",
> +		.depth		= 12,
> +		.fourcc		= V4L2_PIX_FMT_NV12,
> +		.colorspace	= V4L2_COLORSPACE_JPEG,
> +	},
> +	{
> +		.name		= "NV21",
> +		.depth		= 12,
> +		.fourcc		= V4L2_PIX_FMT_NV21,
> +		.colorspace	= V4L2_COLORSPACE_JPEG,
> +	},
> +};
> +
>  static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, int idx,
>  				     struct soc_camera_format_xlate *xlate)
>  {
>  	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> -	int ret;
> +	int ret, k, n;
>  	int formats = 0;
>  
>  	ret = sh_mobile_ceu_try_bus_param(icd);
> @@ -461,6 +528,21 @@ static int sh_mobile_ceu_get_formats(str
>  		return 0;
>  
>  	switch (icd->formats[idx].fourcc) {
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_VYUY:
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_YVYU:
> +		n = ARRAY_SIZE(sh_mobile_ceu_formats);
> +		formats += n;
> +		for (k = 0; xlate && k < n; k++) {
> +			xlate->host_fmt = &sh_mobile_ceu_formats[k];
> +			xlate->cam_fmt = icd->formats + idx;
> +			xlate->buswidth = icd->formats[idx].depth;
> +			xlate++;
> +			dev_dbg(&ici->dev, "Providing format %s using %s\n",
> +				sh_mobile_ceu_formats[k].name,
> +				icd->formats[idx].name);
> +		}
>  	default:
>  		/* Generic pass-through */
>  		formats++;
> @@ -482,6 +564,7 @@ static int sh_mobile_ceu_set_fmt(struct 
>  				 __u32 pixfmt, struct v4l2_rect *rect)
>  {
>  	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +	struct sh_mobile_ceu_dev *pcdev = ici->priv;
>  	const struct soc_camera_format_xlate *xlate;
>  	int ret;
>  
> @@ -502,6 +585,7 @@ static int sh_mobile_ceu_set_fmt(struct 
>  	if (pixfmt && !ret) {
>  		icd->buswidth = xlate->buswidth;
>  		icd->current_fmt = xlate->host_fmt;
> +		pcdev->camera_fmt = xlate->cam_fmt;
>  	}
>  
>  	return ret;
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 03/03] sh_mobile_ceu: add NV16 and NV61 support

[Guennadi Liakhovetski]

On Wed, 10 Dec 2008, Magnus Damm wrote:

> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch adds NV16/NV61 support to the sh_mobile_ceu driver.

I guess I cannot apply / push this patch befor your NV16 / NV61 is 
applied, or shall I pull that patch too, Mauro?

Thanks
Guennadi

> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
> 
>  drivers/media/video/sh_mobile_ceu_camera.c |   22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> --- 0021/drivers/media/video/sh_mobile_ceu_camera.c
> +++ work/drivers/media/video/sh_mobile_ceu_camera.c	2008-12-10 00:12:02.000000000 +0900
> @@ -180,6 +180,8 @@ static void sh_mobile_ceu_capture(struct
>  	switch (icd->current_fmt->fourcc) {
>  	case V4L2_PIX_FMT_NV12:
>  	case V4L2_PIX_FMT_NV21:
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
>  		phys_addr += (icd->width * icd->height);
>  		ceu_write(pcdev, CDACR, phys_addr);
>  	}
> @@ -412,6 +414,9 @@ static int sh_mobile_ceu_set_bus_param(s
>  	case V4L2_PIX_FMT_NV12:
>  	case V4L2_PIX_FMT_NV21:
>  		yuv_lineskip = 1; /* skip for NV12/21, no skip for NV16/61 */
> +		/* fall-through */
> +	case V4L2_PIX_FMT_NV16:
> +	case V4L2_PIX_FMT_NV61:
>  		yuv_mode = 1;
>  		switch (pcdev->camera_fmt->fourcc) {
>  		case V4L2_PIX_FMT_UYVY:
> @@ -431,8 +436,9 @@ static int sh_mobile_ceu_set_bus_param(s
>  		}
>  	}
>  
> -	if (icd->current_fmt->fourcc == V4L2_PIX_FMT_NV21)
> -		value ^= 0x00000100; /* swap U, V to change from NV12->NV21 */
> +	if ((icd->current_fmt->fourcc == V4L2_PIX_FMT_NV21) ||
> +	    (icd->current_fmt->fourcc == V4L2_PIX_FMT_NV61))
> +		value ^= 0x00000100; /* swap U, V to change from NV1x->NVx1 */
>  
>  	value |= (common_flags & SOCAM_VSYNC_ACTIVE_LOW) ? (1 << 1) : 0;
>  	value |= (common_flags & SOCAM_HSYNC_ACTIVE_LOW) ? (1 << 0) : 0;
> @@ -514,6 +520,18 @@ static const struct soc_camera_data_form
>  		.fourcc		= V4L2_PIX_FMT_NV21,
>  		.colorspace	= V4L2_COLORSPACE_JPEG,
>  	},
> +	{
> +		.name		= "NV16",
> +		.depth		= 16,
> +		.fourcc		= V4L2_PIX_FMT_NV16,
> +		.colorspace	= V4L2_COLORSPACE_JPEG,
> +	},
> +	{
> +		.name		= "NV61",
> +		.depth		= 16,
> +		.fourcc		= V4L2_PIX_FMT_NV61,
> +		.colorspace	= V4L2_COLORSPACE_JPEG,
> +	},
>  };
>  
>  static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, int idx,
> 
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 03/03] sh_mobile_ceu: add NV16 and NV61 support

[Mauro Carvalho Chehab]

On Sat, 13 Dec 2008 22:56:37 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Wed, 10 Dec 2008, Magnus Damm wrote:
> 
> > From: Magnus Damm <damm@igel.co.jp>
> > 
> > This patch adds NV16/NV61 support to the sh_mobile_ceu driver.
> 
> I guess I cannot apply / push this patch befor your NV16 / NV61 is 
> applied, or shall I pull that patch too, Mauro?
> 
Guennadi,

You can apply it on your tree. I'll then pull from you on your next pull request.


Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support

[Magnus Damm]

On Sun, Dec 14, 2008 at 6:52 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Wed, 10 Dec 2008, Magnus Damm wrote:
>> This patch adds NV12/NV21 support to the sh_mobile_ceu driver.
>>
>> Signed-off-by: Magnus Damm <damm@igel.co.jp>
>> ---
>>
>>  drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
>>  1 file changed, 99 insertions(+), 15 deletions(-)
>>
>> --- 0031/drivers/media/video/sh_mobile_ceu_camera.c
>> +++ work/drivers/media/video/sh_mobile_ceu_camera.c   2008-12-10 13:09:43.000000000 +0900
>> @@ -158,6 +160,9 @@ static void free_buffer(struct videobuf_
>>
>>  static void sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
>>  {
>> +     struct soc_camera_device *icd = pcdev->icd;
>> +     unsigned long phys_addr;
>
> dma_addr_t

Yeah, good plan.

>> +
>>       ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) & ~1);
>>       ceu_write(pcdev, CETCR, ~ceu_read(pcdev, CETCR) & 0x0317f313);
>>       ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) | 1);
>> @@ -166,11 +171,21 @@ static void sh_mobile_ceu_capture(struct
>>
>>       ceu_write(pcdev, CETCR, 0x0317f313 ^ 0x10);
>>
>> -     if (pcdev->active) {
>> -             pcdev->active->state = VIDEOBUF_ACTIVE;
>> -             ceu_write(pcdev, CDAYR, videobuf_to_dma_contig(pcdev->active));
>> -             ceu_write(pcdev, CAPSR, 0x1); /* start capture */
>> +     if (!pcdev->active)
>> +             return;
>> +
>> +     phys_addr = videobuf_to_dma_contig(pcdev->active);
>> +     ceu_write(pcdev, CDAYR, phys_addr);
>
> Hm, looks like someone could have reviewed this driver a bit better on
> submission:-) I think, your ceu_write() should really take a u32 as an
> argument, not unsigned long, respectively, ceu_read() should return u32.

Well, to fit nicely together with ioread32() and iowrite32() i suppose
u32 is a better fit. Otoh, both u32 and unsigned longs are 32-bit on
regular 32-bit SuperH unless I'm mistaken.

> So there is even less motivation for "unsigned long phys_addr" above. A
> patch to change these functions would be welcome:-)

Usually physically addresses are kept as unsigned longs in the kernel.
That's the reason behind the unsigned longs. The best would of course
be to use an unsigned long to keep a pfn, but I'd say having an
unsigned long to keep the physical address as-is is close enough since
the hardware is using 32-bit registers for destination addresses
anyway. dma_addr_t sounds like a good plan though.

I agree about the parenthesis issues - I'll suspect I used them
because of compiler warnings, but I'll have a look.

I will post a cleanup patch that you can apply on top of this patch
and the interlace patch. This to avoid changing the interlace patch.

Thanks for your help!

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 02/03] sh_mobile_ceu: add NV12 and NV21 support

[Guennadi Liakhovetski]

On Mon, 15 Dec 2008, Magnus Damm wrote:

> On Sun, Dec 14, 2008 at 6:52 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > On Wed, 10 Dec 2008, Magnus Damm wrote:
> >> This patch adds NV12/NV21 support to the sh_mobile_ceu driver.
> >>
> >> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> >> ---
> >>
> >>  drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
> >>  1 file changed, 99 insertions(+), 15 deletions(-)
> >>
> >> --- 0031/drivers/media/video/sh_mobile_ceu_camera.c
> >> +++ work/drivers/media/video/sh_mobile_ceu_camera.c   2008-12-10 13:09:43.000000000 +0900
> >> @@ -158,6 +160,9 @@ static void free_buffer(struct videobuf_
> >>
> >>  static void sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
> >>  {
> >> +     struct soc_camera_device *icd = pcdev->icd;
> >> +     unsigned long phys_addr;
> >
> > dma_addr_t
> 
> Yeah, good plan.
> 
> >> +
> >>       ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) & ~1);
> >>       ceu_write(pcdev, CETCR, ~ceu_read(pcdev, CETCR) & 0x0317f313);
> >>       ceu_write(pcdev, CEIER, ceu_read(pcdev, CEIER) | 1);
> >> @@ -166,11 +171,21 @@ static void sh_mobile_ceu_capture(struct
> >>
> >>       ceu_write(pcdev, CETCR, 0x0317f313 ^ 0x10);
> >>
> >> -     if (pcdev->active) {
> >> -             pcdev->active->state = VIDEOBUF_ACTIVE;
> >> -             ceu_write(pcdev, CDAYR, videobuf_to_dma_contig(pcdev->active));
> >> -             ceu_write(pcdev, CAPSR, 0x1); /* start capture */
> >> +     if (!pcdev->active)
> >> +             return;
> >> +
> >> +     phys_addr = videobuf_to_dma_contig(pcdev->active);
> >> +     ceu_write(pcdev, CDAYR, phys_addr);
> >
> > Hm, looks like someone could have reviewed this driver a bit better on
> > submission:-) I think, your ceu_write() should really take a u32 as an
> > argument, not unsigned long, respectively, ceu_read() should return u32.
> 
> Well, to fit nicely together with ioread32() and iowrite32() i suppose
> u32 is a better fit. Otoh, both u32 and unsigned longs are 32-bit on
> regular 32-bit SuperH unless I'm mistaken.

Yes, they would be, still, it is cleaner to use the same type, not just 
one that happens to mean the same:-)

> > So there is even less motivation for "unsigned long phys_addr" above. A
> > patch to change these functions would be welcome:-)
> 
> Usually physically addresses are kept as unsigned longs in the kernel.
> That's the reason behind the unsigned longs. The best would of course
> be to use an unsigned long to keep a pfn, but I'd say having an
> unsigned long to keep the physical address as-is is close enough since
> the hardware is using 32-bit registers for destination addresses
> anyway. dma_addr_t sounds like a good plan though.

I usually try to select variable types to match their use - either to 
store return values from functions, or to match argument types if they are 
used as arguments to functions. phys_addr is used first to store return 
from videobuf_to_dma_contig(), which is dma_addr_t, then as an argument to 
ceu_write(), which after your clean-up will be u32. And it is also used in 
calculations:

> +		phys_addr += (icd->width * icd->height);

I think, dma_addr_t is never smaller than 32-bit, so, it sort of makes 
sense to first use it to make sure all calculations fit, and then 
(implicitly) truncate it to u32 when passing as an argument to 
ceu_write().

> I agree about the parenthesis issues - I'll suspect I used them
> because of compiler warnings, but I'll have a look.

Definitly not all of them.

> I will post a cleanup patch that you can apply on top of this patch
> and the interlace patch. This to avoid changing the interlace patch.

Hm, yeah... Ok, let's do that, but then, please, also fix extra 
parenthesis that that patch introduces, ok?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list