Skip to contentSkip to conversation

gspca_ov534 raw bayer (SGRBG8) support

PATCH In review Philipp Zabel 2 participants 11 messages Dec 14, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover gspca_ov534 raw bayer (SGRBG8) support

Hi,

this series adds raw bayer (V4L2_PIX_FMT_SGRBG8) support to the gspca ov534-ov772x driver used for the PlayStation Eye camera for VGA and QVGA modes. Selecting the SGRBG8 format bypasses image processing (brightness, contrast, saturation, and hue controls).

regards
Philipp

Philipp Zabel (8):
media: gspca: ov534: replace msleep(10) with usleep_range
media: gspca: support multiple pixel formats in ENUM_FRAMEINTERVALS
media: gspca: support multiple pixel formats in TRY_FMT
media: gspca: ov543-ov772x: move video format specific registers into
bridge_start
media: gspca: ov534-ov772x: add SGBRG8 bayer mode support
media: gspca: ov534-ov722x: remove mode specific video data registers
from bridge_init
media: gspca: ov534-ov722x: remove camera clock setup from bridge_init
media: gspca: ov534-ov772x: remove unnecessary COM3 initialization

 drivers/media/usb/gspca/gspca.c |  18 ++--
 drivers/media/usb/gspca/ov534.c | 153 +++++++++++++++++++++++---------
 2 files changed, 123 insertions(+), 48 deletions(-)
Signature · 1 line
2.20.0
#1 lore raw

PATCH 1/8 media: gspca: ov534: replace msleep(10) with usleep_range

For short waits, usleep_range should be used instead of msleep, see Documentation/timers/timers-howto.txt.

---
 drivers/media/usb/gspca/ov534.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/usb/gspca/ov534.c +2 -2
index d06dc0755b9a..44f06a58bb67 100644
@@ -679,7 +679,7 @@ static int sccb_check_status(struct gspca_dev *gspca_dev)
 	int i;
 
 	for (i = 0; i < 5; i++) {
-		msleep(10);
+		usleep_range(10000, 20000);
 		data = ov534_reg_read(gspca_dev, OV534_REG_STATUS);
 
 		switch (data) {
@@ -1277,7 +1277,7 @@ static int sd_init(struct gspca_dev *gspca_dev)
 
 	/* reset sensor */
 	sccb_reg_write(gspca_dev, 0x12, 0x80);
-	msleep(10);
+	usleep_range(10000, 20000);
 
 	/* probe the sensor */
 	sccb_reg_read(gspca_dev, 0x0a);
Signature · 1 line
2.20.0
#2 lore raw

PATCH 2/8 media: gspca: support multiple pixel formats in ENUM_FRAMEINTERVALS

In reply to Philipp Zabel

If a driver supports multiple pixel formats with the same frame size, ENUM_FRAMEINTERVALS will currently only work for the first pixel format. Fix this by adding pixelformat support to wxh_to_mode().

---
 drivers/media/usb/gspca/gspca.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/media/usb/gspca/gspca.c +5 -3
index 3137f5d89d80..13361cfa6903 100644
@@ -912,13 +912,14 @@ static void gspca_set_default_mode(struct gspca_dev *gspca_dev)
 }
 
 static int wxh_to_mode(struct gspca_dev *gspca_dev,
-			int width, int height)
+			int width, int height, u32 pixelformat)
 {
 	int i;
 
 	for (i = 0; i < gspca_dev->cam.nmodes; i++) {
 		if (width == gspca_dev->cam.cam_mode[i].width
-		    && height == gspca_dev->cam.cam_mode[i].height)
+		    && height == gspca_dev->cam.cam_mode[i].height
+		    && pixelformat == gspca_dev->cam.cam_mode[i].pixelformat)
 			return i;
 	}
 	return -EINVAL;
@@ -1152,7 +1153,8 @@ static int vidioc_enum_frameintervals(struct file *filp, void *priv,
 	int mode;
 	__u32 i;
 
-	mode = wxh_to_mode(gspca_dev, fival->width, fival->height);
+	mode = wxh_to_mode(gspca_dev, fival->width, fival->height,
+			   fival->pixel_format);
 	if (mode < 0)
 		return -EINVAL;
 
Signature · 1 line
2.20.0
#3 lore raw

PATCH 3/8 media: gspca: support multiple pixel formats in TRY_FMT

In reply to Philipp Zabel

If a driver supports multiple pixel formats with the same frame size, TRY_FMT will currently always return the first pixel format. Fix this by adding pixelformat support to wxh_to_nearest_mode().

---
 drivers/media/usb/gspca/gspca.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/media/usb/gspca/gspca.c +8 -2
index 13361cfa6903..ac70b36d67b7 100644
@@ -926,10 +926,16 @@ static int wxh_to_mode(struct gspca_dev *gspca_dev,
 }
 
 static int wxh_to_nearest_mode(struct gspca_dev *gspca_dev,
-			int width, int height)
+			int width, int height, u32 pixelformat)
 {
 	int i;
 
+	for (i = gspca_dev->cam.nmodes; --i > 0; ) {
+		if (width >= gspca_dev->cam.cam_mode[i].width
+		    && height >= gspca_dev->cam.cam_mode[i].height
+		    && pixelformat == gspca_dev->cam.cam_mode[i].pixelformat)
+			return i;
+	}
 	for (i = gspca_dev->cam.nmodes; --i > 0; ) {
 		if (width >= gspca_dev->cam.cam_mode[i].width
 		    && height >= gspca_dev->cam.cam_mode[i].height)
@@ -1059,7 +1065,7 @@ static int try_fmt_vid_cap(struct gspca_dev *gspca_dev,
 		    fmt->fmt.pix.pixelformat, w, h);
 
 	/* search the nearest mode for width and height */
-	mode = wxh_to_nearest_mode(gspca_dev, w, h);
+	mode = wxh_to_nearest_mode(gspca_dev, w, h, fmt->fmt.pix.pixelformat);
 
 	/* OK if right palette */
 	if (gspca_dev->cam.cam_mode[mode].pixelformat
Signature · 1 line
2.20.0
#4 lore raw

PATCH 4/8 media: gspca: ov543-ov772x: move video format specific registers into bridge_start

In reply to Philipp Zabel

In preparation for adding SGBRG8 as a second video format besides YUYV, move video format specific register settings from the bridge_init array into the bridge_start arrays.

---
 drivers/media/usb/gspca/ov534.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
drivers/media/usb/gspca/ov534.c +10 -9
index 44f06a58bb67..077c49a74709 100644
@@ -411,9 +411,7 @@ static const u8 sensor_start_qvga_767x[][2] = {
 };
 
 static const u8 bridge_init_772x[][2] = {
-	{ 0xc2, 0x0c },
 	{ 0x88, 0xf8 },
-	{ 0xc3, 0x69 },
 	{ 0x89, 0xff },
 	{ 0x76, 0x03 },
 	{ 0x92, 0x01 },
@@ -439,7 +437,6 @@ static const u8 bridge_init_772x[][2] = {
 	{ 0x1f, 0x81 },
 	{ 0x34, 0x05 },
 	{ 0xe3, 0x04 },
-	{ 0x88, 0x00 },
 	{ 0x89, 0x00 },
 	{ 0x76, 0x00 },
 	{ 0xe7, 0x2e },
@@ -460,13 +457,7 @@ static const u8 bridge_init_772x[][2] = {
 	{ 0x1d, 0x08 }, /* turn on UVC header */
 	{ 0x1d, 0x0e }, /* .. */
 
-	{ 0x8d, 0x1c },
-	{ 0x8e, 0x80 },
 	{ 0xe5, 0x04 },
-
-	{ 0xc0, 0x50 },
-	{ 0xc1, 0x3c },
-	{ 0xc2, 0x0c },
 };
 static const u8 sensor_init_772x[][2] = {
 	{ 0x12, 0x80 },
@@ -562,6 +553,7 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x0c, 0xd0 }
 };
 static const u8 bridge_start_vga_772x[][2] = {
+	{0x88, 0x00},
 	{0x1c, 0x00},
 	{0x1d, 0x40},
 	{0x1d, 0x02},
@@ -569,8 +561,12 @@ static const u8 bridge_start_vga_772x[][2] = {
 	{0x1d, 0x02},
 	{0x1d, 0x58},
 	{0x1d, 0x00},
+	{0x8d, 0x1c},
+	{0x8e, 0x80},
 	{0xc0, 0x50},
 	{0xc1, 0x3c},
+	{0xc2, 0x0c},
+	{0xc3, 0x69},
 };
 static const u8 sensor_start_vga_772x[][2] = {
 	{0x12, 0x00},
@@ -583,6 +579,7 @@ static const u8 sensor_start_vga_772x[][2] = {
 	{0x65, 0x20},
 };
 static const u8 bridge_start_qvga_772x[][2] = {
+	{0x88, 0x00},
 	{0x1c, 0x00},
 	{0x1d, 0x40},
 	{0x1d, 0x02},
@@ -590,8 +587,12 @@ static const u8 bridge_start_qvga_772x[][2] = {
 	{0x1d, 0x01},
 	{0x1d, 0x4b},
 	{0x1d, 0x00},
+	{0x8d, 0x1c},
+	{0x8e, 0x80},
 	{0xc0, 0x28},
 	{0xc1, 0x1e},
+	{0xc2, 0x0c},
+	{0xc3, 0x69},
 };
 static const u8 sensor_start_qvga_772x[][2] = {
 	{0x12, 0x40},
Signature · 1 line
2.20.0
#5 lore raw

PATCH 5/8 media: gspca: ov534-ov772x: add SGBRG8 bayer mode support

In reply to Philipp Zabel

Add support to pass through the sensor's native SGBRG8 bayer pattern, allowing to cut the required USB bandwidth in half.

---
 drivers/media/usb/gspca/ov534.c | 115 +++++++++++++++++++++++++++-----
 1 file changed, 98 insertions(+), 17 deletions(-)
drivers/media/usb/gspca/ov534.c +98 -17
index 077c49a74709..5b73f7f58ae6 100644
@@ -103,6 +103,16 @@ static const struct v4l2_pix_format ov772x_mode[] = {
 	 .sizeimage = 640 * 480 * 2,
 	 .colorspace = V4L2_COLORSPACE_SRGB,
 	 .priv = 0},
+	{320, 240, V4L2_PIX_FMT_SGRBG8, V4L2_FIELD_NONE,
+	 .bytesperline = 320,
+	 .sizeimage = 320 * 240,
+	 .colorspace = V4L2_COLORSPACE_SRGB,
+	 .priv = 1},
+	{640, 480, V4L2_PIX_FMT_SGRBG8, V4L2_FIELD_NONE,
+	 .bytesperline = 640,
+	 .sizeimage = 640 * 480,
+	 .colorspace = V4L2_COLORSPACE_SRGB,
+	 .priv = 0},
 };
 static const struct v4l2_pix_format ov767x_mode[] = {
 	{320, 240, V4L2_PIX_FMT_JPEG, V4L2_FIELD_NONE,
@@ -127,6 +137,14 @@ static const struct framerates ov772x_framerates[] = {
 		.rates = vga_rates,
 		.nrates = ARRAY_SIZE(vga_rates),
 	},
+	{ /* 320x240 SGBRG8 */
+		.rates = qvga_rates,
+		.nrates = ARRAY_SIZE(qvga_rates),
+	},
+	{ /* 640x480 SGBRG8 */
+		.rates = vga_rates,
+		.nrates = ARRAY_SIZE(vga_rates),
+	},
 };
 
 struct reg_array {
@@ -552,7 +570,7 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x8e, 0x00 },		/* De-noise threshold */
 	{ 0x0c, 0xd0 }
 };
-static const u8 bridge_start_vga_772x[][2] = {
+static const u8 bridge_start_vga_yuyv_772x[][2] = {
 	{0x88, 0x00},
 	{0x1c, 0x00},
 	{0x1d, 0x40},
@@ -568,7 +586,7 @@ static const u8 bridge_start_vga_772x[][2] = {
 	{0xc2, 0x0c},
 	{0xc3, 0x69},
 };
-static const u8 sensor_start_vga_772x[][2] = {
+static const u8 sensor_start_vga_yuyv_772x[][2] = {
 	{0x12, 0x00},
 	{0x17, 0x26},
 	{0x18, 0xa0},
@@ -577,8 +595,9 @@ static const u8 sensor_start_vga_772x[][2] = {
 	{0x29, 0xa0},
 	{0x2c, 0xf0},
 	{0x65, 0x20},
+	{0x67, 0x00},
 };
-static const u8 bridge_start_qvga_772x[][2] = {
+static const u8 bridge_start_qvga_yuyv_772x[][2] = {
 	{0x88, 0x00},
 	{0x1c, 0x00},
 	{0x1d, 0x40},
@@ -594,7 +613,7 @@ static const u8 bridge_start_qvga_772x[][2] = {
 	{0xc2, 0x0c},
 	{0xc3, 0x69},
 };
-static const u8 sensor_start_qvga_772x[][2] = {
+static const u8 sensor_start_qvga_yuyv_772x[][2] = {
 	{0x12, 0x40},
 	{0x17, 0x3f},
 	{0x18, 0x50},
@@ -603,6 +622,61 @@ static const u8 sensor_start_qvga_772x[][2] = {
 	{0x29, 0x50},
 	{0x2c, 0x78},
 	{0x65, 0x2f},
+	{0x67, 0x00},
+};
+static const u8 bridge_start_vga_gbrg_772x[][2] = {
+	{0x88, 0x08},
+	{0x1c, 0x00},
+	{0x1d, 0x00},
+	{0x1d, 0x02},
+	{0x1d, 0x00},
+	{0x1d, 0x01},
+	{0x1d, 0x2c},
+	{0x1d, 0x00},
+	{0x8d, 0x00},
+	{0x8e, 0x00},
+	{0xc0, 0x50},
+	{0xc1, 0x3c},
+	{0xc2, 0x01},
+	{0xc3, 0x01},
+};
+static const u8 sensor_start_vga_gbrg_772x[][2] = {
+	{0x12, 0x01},
+	{0x17, 0x26},
+	{0x18, 0xa0},
+	{0x19, 0x07},
+	{0x1a, 0xf0},
+	{0x29, 0xa0},
+	{0x2c, 0xf0},
+	{0x65, 0x20},
+	{0x67, 0x02},
+};
+static const u8 bridge_start_qvga_gbrg_772x[][2] = {
+	{0x88, 0x08},
+	{0x1c, 0x00},
+	{0x1d, 0x00},
+	{0x1d, 0x02},
+	{0x1d, 0x00},
+	{0x1d, 0x00},
+	{0x1d, 0x4b},
+	{0x1d, 0x00},
+	{0x8d, 0x00},
+	{0x8e, 0x00},
+	{0xc0, 0x28},
+	{0xc1, 0x1e},
+	{0xc2, 0x01},
+	{0xc3, 0x01},
+};
+static const u8 sensor_start_qvga_gbrg_772x[][2] = {
+	{0x12, 0x41},
+	{0x17, 0x3f},
+	{0x18, 0x50},
+	{0x19, 0x03},
+	{0x1a, 0x78},
+	{0x29, 0x50},
+	{0x2c, 0x78},
+	{0x65, 0x2f},
+	{0x67, 0x02},
 };
 
 static void ov534_reg_write(struct gspca_dev *gspca_dev, u16 reg, u8 val)
@@ -1316,25 +1390,33 @@ static int sd_start(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 	int mode;
-	static const struct reg_array bridge_start[NSENSORS][2] = {
+	static const struct reg_array bridge_start[NSENSORS][4] = {
 	[SENSOR_OV767x] = {{bridge_start_qvga_767x,
 					ARRAY_SIZE(bridge_start_qvga_767x)},
 			{bridge_start_vga_767x,
 					ARRAY_SIZE(bridge_start_vga_767x)}},
-	[SENSOR_OV772x] = {{bridge_start_qvga_772x,
-					ARRAY_SIZE(bridge_start_qvga_772x)},
-			{bridge_start_vga_772x,
-					ARRAY_SIZE(bridge_start_vga_772x)}},
+	[SENSOR_OV772x] = {{bridge_start_qvga_yuyv_772x,
+				ARRAY_SIZE(bridge_start_qvga_yuyv_772x)},
+			{bridge_start_vga_yuyv_772x,
+				ARRAY_SIZE(bridge_start_vga_yuyv_772x)},
+			{bridge_start_qvga_gbrg_772x,
+				ARRAY_SIZE(bridge_start_qvga_gbrg_772x)},
+			{bridge_start_vga_gbrg_772x,
+				ARRAY_SIZE(bridge_start_vga_gbrg_772x)} },
 	};
-	static const struct reg_array sensor_start[NSENSORS][2] = {
+	static const struct reg_array sensor_start[NSENSORS][4] = {
 	[SENSOR_OV767x] = {{sensor_start_qvga_767x,
 					ARRAY_SIZE(sensor_start_qvga_767x)},
 			{sensor_start_vga_767x,
 					ARRAY_SIZE(sensor_start_vga_767x)}},
-	[SENSOR_OV772x] = {{sensor_start_qvga_772x,
-					ARRAY_SIZE(sensor_start_qvga_772x)},
-			{sensor_start_vga_772x,
-					ARRAY_SIZE(sensor_start_vga_772x)}},
+	[SENSOR_OV772x] = {{sensor_start_qvga_yuyv_772x,
+				ARRAY_SIZE(sensor_start_qvga_yuyv_772x)},
+			{sensor_start_vga_yuyv_772x,
+				ARRAY_SIZE(sensor_start_vga_yuyv_772x)},
+			{sensor_start_qvga_gbrg_772x,
+				ARRAY_SIZE(sensor_start_qvga_gbrg_772x)},
+			{sensor_start_vga_gbrg_772x,
+				ARRAY_SIZE(sensor_start_vga_gbrg_772x)} },
 	};
 
 	/* (from ms-win trace) */
@@ -1440,10 +1522,9 @@ static void sd_pkt_scan(struct gspca_dev *gspca_dev,
 		/* If this packet is marked as EOF, end the frame */
 		} else if (data[1] & UVC_STREAM_EOF) {
 			sd->last_pts = 0;
-			if (gspca_dev->pixfmt.pixelformat == V4L2_PIX_FMT_YUYV
+			if (gspca_dev->pixfmt.pixelformat != V4L2_PIX_FMT_JPEG
 			 && gspca_dev->image_len + len - 12 !=
-				   gspca_dev->pixfmt.width *
-					gspca_dev->pixfmt.height * 2) {
+			    gspca_dev->pixfmt.sizeimage) {
 				gspca_dbg(gspca_dev, D_PACK, "wrong sized frame\n");
 				goto discard;
 			}
Signature · 1 line
2.20.0
#6 lore raw

PATCH 6/8 media: gspca: ov534-ov722x: remove mode specific video data registers from bridge_init

In reply to Philipp Zabel

The video format, payload size, and frame size setup is video format and frame size specific. Those registers are overwritten during bridge_start anyway.

---
 drivers/media/usb/gspca/ov534.c | 9 ---------
 1 file changed, 9 deletions(-)
drivers/media/usb/gspca/ov534.c +0 -9
index 5b73f7f58ae6..bc9d2eb5db30 100644
@@ -462,15 +462,6 @@ static const u8 bridge_init_772x[][2] = {
 	{ 0x25, 0x42 },
 	{ 0x21, 0xf0 },
 
-	{ 0x1c, 0x00 },
-	{ 0x1d, 0x40 },
-	{ 0x1d, 0x02 }, /* payload size 0x0200 * 4 = 2048 bytes */
-	{ 0x1d, 0x00 }, /* payload size */
-
-	{ 0x1d, 0x02 }, /* frame size 0x025800 * 4 = 614400 */
-	{ 0x1d, 0x58 }, /* frame size */
-	{ 0x1d, 0x00 }, /* frame size */
-
 	{ 0x1c, 0x0a },
 	{ 0x1d, 0x08 }, /* turn on UVC header */
 	{ 0x1d, 0x0e }, /* .. */
Signature · 1 line
2.20.0
#7 lore raw

PATCH 7/8 media: gspca: ov534-ov722x: remove camera clock setup from bridge_init

In reply to Philipp Zabel

This register is later overwritten by set_frame_rate anyway.

---
 drivers/media/usb/gspca/ov534.c | 2 --
 1 file changed, 2 deletions(-)
drivers/media/usb/gspca/ov534.c +0 -2
index bc9d2eb5db30..23deeedd3279 100644
@@ -465,8 +465,6 @@ static const u8 bridge_init_772x[][2] = {
 	{ 0x1c, 0x0a },
 	{ 0x1d, 0x08 }, /* turn on UVC header */
 	{ 0x1d, 0x0e }, /* .. */
-
-	{ 0xe5, 0x04 },
 };
 static const u8 sensor_init_772x[][2] = {
 	{ 0x12, 0x80 },
Signature · 1 line
2.20.0
#8 lore raw

PATCH 8/8 media: gspca: ov534-ov772x: remove unnecessary COM3 initialization

In reply to Philipp Zabel

The COM3 register at address 0x0c already defaults to 0x10, the two bits COM3[7:6] are set according to V4L2 controls by sethvflip later. There is no need to set it multiple times during bridge initialization.

---
 drivers/media/usb/gspca/ov534.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/media/usb/gspca/ov534.c +0 -4
index 23deeedd3279..02c90ad96b76 100644
@@ -543,13 +543,10 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x8c, 0xe8 },
 	{ 0x8d, 0x20 },
 
-	{ 0x0c, 0x90 },
-
 	{ 0x2b, 0x00 },
 	{ 0x22, 0x7f },
 	{ 0x23, 0x03 },
 	{ 0x11, 0x01 },
-	{ 0x0c, 0xd0 },
 	{ 0x64, 0xff },
 	{ 0x0d, 0x41 },
 
@@ -557,7 +554,6 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x0e, 0xcd },
 	{ 0xac, 0xbf },
 	{ 0x8e, 0x00 },		/* De-noise threshold */
-	{ 0x0c, 0xd0 }
 };
 static const u8 bridge_start_vga_yuyv_772x[][2] = {
 	{0x88, 0x00},
Signature · 1 line
2.20.0
#9 lore raw
In reply to Philipp Zabel
Quoted 6 lines · Philipp Zabel

Hi,

this series adds raw bayer (V4L2_PIX_FMT_SGRBG8) support to the gspca ov534-ov772x driver used for the PlayStation Eye camera for VGA and QVGA modes. Selecting the SGRBG8 format bypasses image processing (brightness, contrast, saturation, and hue controls).

I'm curious since I don't see many patches for gspca: what are you planning to use this for?

Regards,

Hans

Quoted 20 lines

regards
Philipp

Philipp Zabel (8):
media: gspca: ov534: replace msleep(10) with usleep_range
media: gspca: support multiple pixel formats in ENUM_FRAMEINTERVALS
media: gspca: support multiple pixel formats in TRY_FMT
media: gspca: ov543-ov772x: move video format specific registers into
bridge_start
media: gspca: ov534-ov772x: add SGBRG8 bayer mode support
media: gspca: ov534-ov722x: remove mode specific video data registers
from bridge_init
media: gspca: ov534-ov722x: remove camera clock setup from bridge_init
media: gspca: ov534-ov772x: remove unnecessary COM3 initialization

 drivers/media/usb/gspca/gspca.c |  18 ++--
 drivers/media/usb/gspca/ov534.c | 153 +++++++++++++++++++++++---------
 2 files changed, 123 insertions(+), 48 deletions(-)
#10 lore raw
Quoted 11 lines · Hans Verkuil
Quoted 6 lines · Philipp Zabel

Hi,

this series adds raw bayer (V4L2_PIX_FMT_SGRBG8) support to the gspca ov534-ov772x driver used for the PlayStation Eye camera for VGA and QVGA modes. Selecting the SGRBG8 format bypasses image processing (brightness, contrast, saturation, and hue controls).

I'm curious since I don't see many patches for gspca: what are you planning to use this for?

I'd like to track the 3D motion of a PlayStation VR headset and Move controllers with a few of these.

regards
Philipp