Skip to contentSkip to conversation

video: nv12/nv21 support for the sh_mobile_ceu driver

PATCH In review Magnus Damm 2 participants 3 messages Nov 13, 2008 Media (V4L/DVB)

lore
Share & tools
lore

Magnus Damm

author
#0 lore raw

PATCH 1/1 video: nv12/nv21 support for the sh_mobile_ceu driver

From: Magnus Damm <[email protected]>

This patch adds nv12/nv21 mode support to the SuperH Mobile CEU driver. These modes are translated by the hardware, and added to the list of available modes if the connected camera can output one of the supported input modes. Other modes are just handled using data transfer as usual.

The hardware also supports nv16/nv61 which is trivial to add on top of this.

Signed-off-by Magnus Damm <[email protected]>

---

Depends on the VYUY fourcc.
I suspect that this change may conflict with at least 4 other patches. =)

 drivers/media/video/sh_mobile_ceu_camera.c |  165 +++++++++++++++++++++++++---
 1 file changed, 150 insertions(+), 15 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c +150 -15
@@ -97,6 +97,10 @@ struct sh_mobile_ceu_dev {
 	struct videobuf_buffer *active;
 
 	struct sh_mobile_ceu_info *pdata;
+
+	const struct soc_camera_data_format *camera_formats;
+	int camera_num_formats;
+	unsigned int camera_fourcc;
 };
 
 static void ceu_write(struct sh_mobile_ceu_dev *priv,

@@ -156,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);

@@ -164,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,

@@ -295,6 +312,9 @@ static int sh_mobile_ceu_add_device(stru
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
 	int ret = -EBUSY;
+	const struct soc_camera_data_format *format;
+	struct soc_camera_data_format *fmt;
+	int k, yuv_mode_possible;
 
 	mutex_lock(&camera_lock);
 

@@ -314,6 +334,60 @@ static int sh_mobile_ceu_add_device(stru
 		msleep(1);
 
 	pcdev->icd = icd;
+
+	/* check if we can enable NVxx modes and
+	 * remember the last supported camera fourcc mode
+	 */
+
+	format = NULL;
+	yuv_mode_possible = 0;
+	for (k = 0; k < icd->num_formats; k++) {
+		format = &icd->formats[k];
+
+		switch (format->fourcc) {
+		case V4L2_PIX_FMT_UYVY:
+		case V4L2_PIX_FMT_VYUY:
+		case V4L2_PIX_FMT_YUYV:
+		case V4L2_PIX_FMT_YVYU:
+			yuv_mode_possible = 1;
+			pcdev->camera_fourcc = format->fourcc;
+			break;
+		}
+	}
+
+	/* override list with translated yuv formats if possible */
+	if (yuv_mode_possible && format) {
+		k = icd->num_formats + 2; /* camera formats + NV12 + NV21 */
+
+		fmt = kzalloc(sizeof(*icd->formats) * k, GFP_KERNEL);
+		if (fmt) {
+			pcdev->camera_formats = icd->formats;
+			pcdev->camera_num_formats = icd->num_formats;
+
+			memcpy(fmt, icd->formats,
+			       icd->num_formats * sizeof(*fmt));
+
+			icd->formats = fmt;
+			icd->num_formats = k;
+			icd->current_fmt = &icd->formats[0];
+
+			fmt += pcdev->camera_num_formats;
+			fmt->fourcc = V4L2_PIX_FMT_NV12;
+			fmt->name = "NV12";
+			fmt->depth = 12;
+			fmt->colorspace = format->colorspace;
+
+			fmt++;
+			fmt->fourcc = V4L2_PIX_FMT_NV21;
+			fmt->name = "NV21";
+			fmt->depth = 12;
+			fmt->colorspace = format->colorspace;
+		} else {
+			dev_err(&icd->dev, "Unable to add NVxx formats.\n");
+			ici->ops->remove(icd);
+			ret = -ENOMEM;
+		}
+	}
 err:
 	mutex_unlock(&camera_lock);
 

@@ -342,6 +416,16 @@ static void sh_mobile_ceu_remove_device(
 	}
 	spin_unlock_irqrestore(&pcdev->lock, flags);
 
+	/* restore formats */
+	if (pcdev->camera_num_formats) {
+		icd->current_fmt = &pcdev->camera_formats[0];
+		kfree(icd->formats);
+		icd->formats = pcdev->camera_formats;
+		icd->num_formats = pcdev->camera_num_formats;
+		pcdev->camera_formats = NULL;
+		pcdev->camera_num_formats = 0;
+	}
+
 	icd->ops->release(icd);
 
 	dev_info(&icd->dev,

@@ -358,6 +442,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,

@@ -383,7 +468,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_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;

@@ -394,16 +507,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)
 	 *

@@ -417,14 +536,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;
 }
 

@@ -447,7 +568,21 @@ static int sh_mobile_ceu_try_bus_param(s
 static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
 				     __u32 pixfmt, struct v4l2_rect *rect)
 {
-	return icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct sh_mobile_ceu_dev *pcdev = ici->priv;
+	__u32 camera_fourcc;
+
+	/* for NVxx modes configure camera with fourcc determined earlier */
+	switch (pixfmt) {
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:
+		camera_fourcc = pcdev->camera_fourcc;
+		break;
+	default:
+		camera_fourcc = pixfmt;
+	}
+
+	return icd->ops->set_fmt_cap(icd, camera_fourcc, rect);
 }
 
 static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,

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

#1 lore raw
Quoted 8 lines · Magnus Damm

From: Magnus Damm <[email protected]>

This patch adds nv12/nv21 mode support to the SuperH Mobile CEU driver. These modes are translated by the hardware, and added to the list of available modes if the connected camera can output one of the supported input modes. Other modes are just handled using data transfer as usual.

The hardware also supports nv16/nv61 which is trivial to add on top of this.

Heh, you realise, that you're walking on burning coal, don't you?:-) So, you'll appreciate, if I postpone this patch until the API reasonably stabilises, and it is very likely you'll have to redo this?

Just a couple of points I came across while skipping over this patch:

Quoted 12 lines
Signed-off-by Magnus Damm <[email protected]>

---

Depends on the VYUY fourcc.
I suspect that this change may conflict with at least 4 other patches. =)

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

drivers/media/video/sh_mobile_ceu_camera.c struct sh_mobile_ceu_dev { · line 103

Quoted 7 lines
drivers/media/video/sh_mobile_ceu_camera.c +4 -0
@@ -97,6 +97,10 @@ struct sh_mobile_ceu_dev {
 	struct videobuf_buffer *active;
 
 	struct sh_mobile_ceu_info *pdata;
+
+	const struct soc_camera_data_format *camera_formats;
+	int camera_num_formats;
+	unsigned int camera_fourcc;

These formats are assigned dynamically and are camera-dependent, right? Even though SH camera host probably is not supposed to handle more than one camera at a time, I wouldn't do this. This is what I added a void *host_priv to struct soc_camera_device for - for host-private per-camera data (see patch 3/5 that still waits moderator's approval:-().

Quoted 75 lines
drivers/media/video/sh_mobile_ceu_camera.c +42 -4
 };
 
 static void ceu_write(struct sh_mobile_ceu_dev *priv,

@@ -156,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);

@@ -164,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,

@@ -295,6 +312,9 @@ static int sh_mobile_ceu_add_device(stru
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
 	int ret = -EBUSY;
+	const struct soc_camera_data_format *format;
+	struct soc_camera_data_format *fmt;
+	int k, yuv_mode_possible;
 
 	mutex_lock(&camera_lock);
 

@@ -314,6 +334,60 @@ static int sh_mobile_ceu_add_device(stru
 		msleep(1);
 
 	pcdev->icd = icd;
+
+	/* check if we can enable NVxx modes and
+	 * remember the last supported camera fourcc mode
+	 */
+
+	format = NULL;
+	yuv_mode_possible = 0;
+	for (k = 0; k < icd->num_formats; k++) {
+		format = &icd->formats[k];
+
+		switch (format->fourcc) {
+		case V4L2_PIX_FMT_UYVY:
+		case V4L2_PIX_FMT_VYUY:
+		case V4L2_PIX_FMT_YUYV:
+		case V4L2_PIX_FMT_YVYU:
+			yuv_mode_possible = 1;
+			pcdev->camera_fourcc = format->fourcc;
+			break;
+		}
+	}
+
+	/* override list with translated yuv formats if possible */

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_add_device(stru · line 368

Quoted 10 lines
drivers/media/video/sh_mobile_ceu_camera.c +10 -0
+	if (yuv_mode_possible && format) {
+		k = icd->num_formats + 2; /* camera formats + NV12 + NV21 */
+
+		fmt = kzalloc(sizeof(*icd->formats) * k, GFP_KERNEL);
+		if (fmt) {
+			pcdev->camera_formats = icd->formats;
+			pcdev->camera_num_formats = icd->num_formats;
+
+			memcpy(fmt, icd->formats,
+			       icd->num_formats * sizeof(*fmt));

No! Never write to icd->formats! And even less so in a host driver. It is for a reason it is marked "const".

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_add_device(stru · line 370

Quoted 2 lines
drivers/media/video/sh_mobile_ceu_camera.c +2 -0
+
+			icd->formats = fmt;

And never overload it.

Quoted 24 lines
drivers/media/video/sh_mobile_ceu_camera.c +20 -0
+			icd->num_formats = k;
+			icd->current_fmt = &icd->formats[0];
+
+			fmt += pcdev->camera_num_formats;
+			fmt->fourcc = V4L2_PIX_FMT_NV12;
+			fmt->name = "NV12";
+			fmt->depth = 12;
+			fmt->colorspace = format->colorspace;
+
+			fmt++;
+			fmt->fourcc = V4L2_PIX_FMT_NV21;
+			fmt->name = "NV21";
+			fmt->depth = 12;
+			fmt->colorspace = format->colorspace;
+		} else {
+			dev_err(&icd->dev, "Unable to add NVxx formats.\n");
+			ici->ops->remove(icd);
+			ret = -ENOMEM;
+		}
+	}
 err:
 	mutex_unlock(&camera_lock);

drivers/media/video/sh_mobile_ceu_camera.c static void sh_mobile_ceu_remove_device( · line 422

Quoted 7 lines
drivers/media/video/sh_mobile_ceu_camera.c +4 -0
@@ -342,6 +416,16 @@ static void sh_mobile_ceu_remove_device(
 	}
 	spin_unlock_irqrestore(&pcdev->lock, flags);
 
+	/* restore formats */
+	if (pcdev->camera_num_formats) {
+		icd->current_fmt = &pcdev->camera_formats[0];
+		kfree(icd->formats);

And never free it! Yes, I realise that you're freeing not the original ->formats data, but what you have assigned to it above.

All in all - please, wait a bit, it shall all become clearer shortly.

Quoted 117 lines
drivers/media/video/sh_mobile_ceu_camera.c +61 -11
+		icd->formats = pcdev->camera_formats;
+		icd->num_formats = pcdev->camera_num_formats;
+		pcdev->camera_formats = NULL;
+		pcdev->camera_num_formats = 0;
+	}
+
 	icd->ops->release(icd);
 
 	dev_info(&icd->dev,

@@ -358,6 +442,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,

@@ -383,7 +468,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_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;

@@ -394,16 +507,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)
 	 *

@@ -417,14 +536,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;
 }
 

@@ -447,7 +568,21 @@ static int sh_mobile_ceu_try_bus_param(s
 static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
 				     __u32 pixfmt, struct v4l2_rect *rect)
 {
-	return icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct sh_mobile_ceu_dev *pcdev = ici->priv;
+	__u32 camera_fourcc;
+
+	/* for NVxx modes configure camera with fourcc determined earlier */
+	switch (pixfmt) {
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_NV21:

drivers/media/video/sh_mobile_ceu_camera.c static int sh_mobile_ceu_try_bus_param(s · line 588

Quoted 10 lines
drivers/media/video/sh_mobile_ceu_camera.c +7 -0
+		camera_fourcc = pcdev->camera_fourcc;
+		break;
+	default:
+		camera_fourcc = pixfmt;
+	}
+
+	return icd->ops->set_fmt_cap(icd, camera_fourcc, rect);
 }
 
 static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,

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

author
#2 lore raw
Quoted 9 lines · Guennadi Liakhovetski
Quoted 7 lines · Magnus Damm

From: Magnus Damm <[email protected]>

This patch adds nv12/nv21 mode support to the SuperH Mobile CEU driver. These modes are translated by the hardware, and added to the list of available modes if the connected camera can output one of the supported input modes. Other modes are just handled using data transfer as usual.

Quoted 5 lines
Quoted 1 line

The hardware also supports nv16/nv61 which is trivial to add on top of this.

Heh, you realise, that you're walking on burning coal, don't you?:-) So, you'll appreciate, if I postpone this patch until the API reasonably stabilises, and it is very likely you'll have to redo this?

I realize that the ground is moving under me, but there's not so much I can do about it. I've worked on this patch on and off the last months, and I've been postponing the posting for too long time.

Quoted 1 line

Just a couple of points I came across while skipping over this patch:

Thanks for checking my code.

Quoted 14 lines
Quoted 14 lines
Signed-off-by Magnus Damm <[email protected]>

---

Depends on the VYUY fourcc.
I suspect that this change may conflict with at least 4 other patches. =)

 drivers/media/video/sh_mobile_ceu_camera.c |  165 +++++++++++++++++++++++++---
 1 file changed, 150 insertions(+), 15 deletions(-)
drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +0 -0
@@ -97,6 +97,10 @@ struct sh_mobile_ceu_dev {
      struct videobuf_buffer *active;
Quoted 7 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 struct sh_mobile_ceu_dev { · line 103

Quoted 5 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +4 -0
      struct sh_mobile_ceu_info *pdata;
+
+     const struct soc_camera_data_format *camera_formats;
+     int camera_num_formats;
+     unsigned int camera_fourcc;

These formats are assigned dynamically and are camera-dependent, right?

Yes.

Quoted 4 lines

Even though SH camera host probably is not supposed to handle more than one camera at a time, I wouldn't do this. This is what I added a void *host_priv to struct soc_camera_device for - for host-private per-camera data (see patch 3/5 that still waits moderator's approval:-().

Yeah, keeping it together with the rest of the camera data makes sense. I rather not do this at all though. =)

[snip]

Quoted 31 lines
Quoted 31 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +27 -0
@@ -314,6 +334,60 @@ static int sh_mobile_ceu_add_device(stru
              msleep(1);
 
      pcdev->icd = icd;
+
+     /* check if we can enable NVxx modes and
+      * remember the last supported camera fourcc mode
+      */
+
+     format = NULL;
+     yuv_mode_possible = 0;
+     for (k = 0; k < icd->num_formats; k++) {
+             format = &icd->formats[k];
+
+             switch (format->fourcc) {
+             case V4L2_PIX_FMT_UYVY:
+             case V4L2_PIX_FMT_VYUY:
+             case V4L2_PIX_FMT_YUYV:
+             case V4L2_PIX_FMT_YVYU:
+                     yuv_mode_possible = 1;
+                     pcdev->camera_fourcc = format->fourcc;
+                     break;
+             }
+     }
+
+     /* override list with translated yuv formats if possible */
+     if (yuv_mode_possible && format) {
+             k = icd->num_formats + 2; /* camera formats + NV12 + NV21 */
+
+             fmt = kzalloc(sizeof(*icd->formats) * k, GFP_KERNEL);
+             if (fmt) {
Quoted 8 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 static int sh_mobile_ceu_add_device(stru · line 368

Quoted 5 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +5 -0
+                     pcdev->camera_formats = icd->formats;
+                     pcdev->camera_num_formats = icd->num_formats;
+
+                     memcpy(fmt, icd->formats,
+                            icd->num_formats * sizeof(*fmt));

No! Never write to icd->formats! And even less so in a host driver. It is for a reason it is marked "const".

I agree it's dirty as hell, but it was initially written for pre-2.6.27. At that time I couldn't find any better way for a host to export formats to user space. You remember me talking about bitmaps some time ago - this ickiness is the reason why. =)

Quoted 4 lines

drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 static int sh_mobile_ceu_add_device(stru · line 370

Quoted 2 lines
drivers/media/video/sh_mobile_ceu_camera.c 2008-11-13 16:13:06.000000000 +0900 renamed from drivers/media/video/sh_mobile_ceu_camera.c +2 -0
+
+                     icd->formats = fmt;

And never overload it.

[snip]

Quoted 4 lines

And never free it! Yes, I realise that you're freeing not the original ->formats data, but what you have assigned to it above.

All in all - please, wait a bit, it shall all become clearer shortly.

Yes! I'm more than happy to wait a bit. I hope the patch at least shows what I want to do. =)

Cheers,

/ magnus

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