Skip to contentSkip to conversation

video: nv1x/nvx1 support for the sh_mobile_ceu driver V2

PATCH In review Magnus Damm 4 participants 11 messages Dec 10, 2008 – Dec 15, 2008 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

cover video: nv1x/nvx1 support for the sh_mobile_ceu driver V2

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.

---

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

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#1 lore raw

PATCH 1/3 sh_mobile_ceu: use new pixel format translation code

From: Magnus Damm <[email protected]>

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.

---

 drivers/media/video/sh_mobile_ceu_camera.c |   83 +++++++++++++++++++---------
 1 file changed, 57 insertions(+), 26 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c +57 -26
@@ -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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#2 lore raw

PATCH 2/3 sh_mobile_ceu: add NV12 and NV21 support

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

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

---

 drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
 1 file changed, 99 insertions(+), 15 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c +99 -15
@@ -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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#3 lore raw

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

Quoted 22 lines · Magnus Damm

From: Magnus Damm <[email protected]>

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

---

 drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
 1 file changed, 99 insertions(+), 15 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c +2 -0
@@ -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,

drivers/media/video/sh_mobile_ceu_camera.c static void free_buffer(struct videobuf_ · line 164

Quoted 5 lines
drivers/media/video/sh_mobile_ceu_camera.c +2 -0
@@ -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

Quoted 7 lines
drivers/media/video/sh_mobile_ceu_camera.c +1 -0
+
 	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);

drivers/media/video/sh_mobile_ceu_camera.c static void sh_mobile_ceu_capture(struct · line 178

Quoted 10 lines
drivers/media/video/sh_mobile_ceu_camera.c +5 -4
 
-	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:-)

drivers/media/video/sh_mobile_ceu_camera.c static void sh_mobile_ceu_capture(struct · line 183

Quoted 5 lines
drivers/media/video/sh_mobile_ceu_camera.c +5 -0
+
+	switch (icd->current_fmt->fourcc) {
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		phys_addr += (icd->width * icd->height);

superfluous parenthesis

Quoted 43 lines
drivers/media/video/sh_mobile_ceu_camera.c +27 -1
+		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:

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_set_bus_param(s · line 439

Quoted 10 lines
drivers/media/video/sh_mobile_ceu_camera.c +7 -0
+			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:-)

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_set_bus_param(s · line 449

Quoted 11 lines
drivers/media/video/sh_mobile_ceu_camera.c +3 -4
@@ -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

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_set_bus_param(s · line 455

Quoted 6 lines
drivers/media/video/sh_mobile_ceu_camera.c +6 -0
+		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

Quoted 92 lines
drivers/media/video/sh_mobile_ceu_camera.c +40 -6
+	}
 
 	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;
 

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_set_fmt(struct · line 591

Quoted 7 lines
drivers/media/video/sh_mobile_ceu_camera.c +1 -0
@@ -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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#4 lore raw

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

Quoted 13 lines · Guennadi Liakhovetski
Quoted 12 lines · Magnus Damm

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

---

 drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
 1 file changed, 99 insertions(+), 15 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +0 -0
@@ -158,6 +160,9 @@ static void free_buffer(struct videobuf_
 
Quoted 6 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 static void free_buffer(struct videobuf_ · line 164

Quoted 4 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +2 -0
 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.

Quoted 8 lines
Quoted 8 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +1 -0
+
      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);
 
Quoted 13 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 static void sh_mobile_ceu_capture(struct · line 178

Quoted 9 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +5 -4
-     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.

Quoted 2 lines

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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#5 lore raw

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

Quoted 46 lines · Magnus Damm
Quoted 13 lines · Guennadi Liakhovetski
Quoted 12 lines · Magnus Damm

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

---

 drivers/media/video/sh_mobile_ceu_camera.c |  114 ++++++++++++++++++++++++----
 1 file changed, 99 insertions(+), 15 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +0 -0
@@ -158,6 +160,9 @@ static void free_buffer(struct videobuf_
 
Quoted 6 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 static void free_buffer(struct videobuf_ · line 164

Quoted 4 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +2 -0
 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.

Quoted 8 lines
Quoted 8 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +1 -0
+
      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);
 
Quoted 13 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 static void sh_mobile_ceu_capture(struct · line 178

Quoted 9 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-12-10 13:09:43.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +5 -4
-     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.

Quoted 3 lines

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:-)

Quoted 9 lines
Quoted 2 lines

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:

drivers/media/video/sh_mobile_ceu_camera.c static void sh_mobile_ceu_capture(struct · line 183

Quoted 1 line
drivers/media/video/sh_mobile_ceu_camera.c +1 -0
+		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().

Quoted 2 lines

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.

Quoted 2 lines

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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#6 lore raw

PATCH 3/3 sh_mobile_ceu: add NV16 and NV61 support

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

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

---

 drivers/media/video/sh_mobile_ceu_camera.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c +20 -2
@@ -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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#7 lore raw

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

Quoted 3 lines · Magnus Damm

From: Magnus Damm <[email protected]>

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

Quoted 50 lines

---

 drivers/media/video/sh_mobile_ceu_camera.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c +13 -2
@@ -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,

drivers/media/video/sh_mobile_ceu_camera.c static const struct soc_camera_data_form · line 537

Quoted 10 lines
drivers/media/video/sh_mobile_ceu_camera.c +7 -0
+	},
+	{
+		.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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#8 lore raw

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

Quoted 9 lines · On Sat, 13 Dec 2008 22:56:37 +0100 (CET) Guennadi Liakhovetski…
Quoted 3 lines · Magnus Damm

From: Magnus Damm <[email protected]>

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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Acked-by on the v1 series
#9 lore raw
In reply to Magnus Damm

Hi Magnus, Guennadi

Quoted 2 lines

[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

Best regards
--
Kuninori Morimoto

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#10 lore raw

Hi Morimoto-san,

Quoted 11 lines · On Fri, Dec 12, 2008 at 5:37 PM, <[email protected]> wrote:

Hi Magnus, Guennadi

Quoted 2 lines

[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

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:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list