[PATCH 0 of 4] ov534 patches

[PATCH 0 of 4] ov534 patches

[Jim Paris]

Hi,

Here are some ov534 patches I've been working on.

ov534: don't check status twice
ov534: initialization cleanup
ov534: Fix frame size so we don't miss the last pixel
ov534: frame transfer improvements


-jim

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

[PATCH 1 of 4] ov534: don't check status twice

[Jim Paris]

# HG changeset patch
# User jim@jtan.com
# Date 1228334704 18000
# Node ID fa4fcbdb237a065c4033bd51be28fa9da9016cef
# Parent  ee27d949bfa5e050f858b5bae4cc9a050ff5b119
ov534: don't check status twice

sccb_reg_write already calls sccb_check_status, no need to do it
again.

Signed-off-by: Jim Paris <jim@jtan.com>

diff -r ee27d949bfa5 -r fa4fcbdb237a linux/drivers/media/video/gspca/ov534.c
--- a/linux/drivers/media/video/gspca/ov534.c	Tue Dec 02 19:00:57 2008 +0100
+++ b/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:05:04 2008 -0500
@@ -369,31 +369,23 @@
 	switch (sd->frame_rate) {
 	case 50:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x01);
-		sccb_check_status(gspca_dev->dev);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0x41);
-		sccb_check_status(gspca_dev->dev);
 		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x02);
 		break;
 	case 40:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x02);
-		sccb_check_status(gspca_dev->dev);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0xc1);
-		sccb_check_status(gspca_dev->dev);
 		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x04);
 		break;
 	case 30:
 	default:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x04);
-		sccb_check_status(gspca_dev->dev);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0x81);
-		sccb_check_status(gspca_dev->dev);
 		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x02);
 		break;
 	case 15:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x03);
-		sccb_check_status(gspca_dev->dev);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0x41);
-		sccb_check_status(gspca_dev->dev);
 		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x04);
 		break;
 	};

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

[PATCH 2 of 4] ov534: initialization cleanup

[Jim Paris]

# HG changeset patch
# User jim@jtan.com
# Date 1228334814 18000
# Node ID 893107a5df87228bb6766f26226fcef8069e3fc8
# Parent  fa4fcbdb237a065c4033bd51be28fa9da9016cef
ov534: initialization cleanup

Clean up bridge and sensor chip initialization by putting initial
register values in arrays rather than open-coding calls to
*_reg_write.

Also remove ov534_reg_verify_write, as the verify step doesn't appear
necessary.

The order of some writes was rearranged and some duplicate writes were
removed, but the camera behavior is unchanged.

Signed-off-by: Jim Paris <jim@jtan.com>

diff -r fa4fcbdb237a -r 893107a5df87 linux/drivers/media/video/gspca/ov534.c
--- a/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:05:04 2008 -0500
+++ b/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:06:54 2008 -0500
@@ -94,19 +94,6 @@
 	return data;
 }
 
-static void ov534_reg_verify_write(struct usb_device *udev, u16 reg, u16 val)
-{
-	u16 data;
-
-	ov534_reg_write(udev, reg, val);
-	data = ov534_reg_read(udev, reg);
-	if (data != val) {
-		PDEBUG(D_ERR | D_USBO,
-		       "unexpected result from read: 0x%04x != 0x%04x", val,
-		       data);
-	}
-}
-
 /* Two bits control LED: 0x21 bit 7 and 0x23 bit 7.
  * (direction and output)? */
 static void ov534_set_led(struct usb_device *udev, int status)
@@ -162,174 +149,168 @@
 		PDEBUG(D_ERR, "sccb_reg_write failed");
 }
 
+static const __u8 ov534_reg_initdata[][2] = {
+	{ 0xe7, 0x3a },
+
+	{ OV534_REG_ADDRESS, 0x42 }, /* select OV772x sensor */
+
+	{ 0xc2, 0x0c },
+	{ 0x88, 0xf8 },
+	{ 0xc3, 0x69 },
+	{ 0x89, 0xff },
+	{ 0x76, 0x03 },
+	{ 0x92, 0x01 },
+	{ 0x93, 0x18 },
+	{ 0x94, 0x10 },
+	{ 0x95, 0x10 },
+	{ 0xe2, 0x00 },
+	{ 0xe7, 0x3e },
+
+	{ 0x1c, 0x0a },
+	{ 0x1d, 0x22 },
+	{ 0x1d, 0x06 },
+
+	{ 0x96, 0x00 },
+
+	{ 0x97, 0x20 },
+	{ 0x97, 0x20 },
+	{ 0x97, 0x20 },
+	{ 0x97, 0x0a },
+	{ 0x97, 0x3f },
+	{ 0x97, 0x4a },
+	{ 0x97, 0x20 },
+	{ 0x97, 0x15 },
+	{ 0x97, 0x0b },
+
+	{ 0x8e, 0x40 },
+	{ 0x1f, 0x81 },
+	{ 0x34, 0x05 },
+	{ 0xe3, 0x04 },
+	{ 0x88, 0x00 },
+	{ 0x89, 0x00 },
+	{ 0x76, 0x00 },
+	{ 0xe7, 0x2e },
+	{ 0x31, 0xf9 },
+	{ 0x25, 0x42 },
+	{ 0x21, 0xf0 },
+
+	{ 0x1c, 0x00 },
+	{ 0x1d, 0x40 },
+	{ 0x1d, 0x02 },
+	{ 0x1d, 0x00 },
+	{ 0x1d, 0x02 },
+	{ 0x1d, 0x57 },
+	{ 0x1d, 0xff },
+
+	{ 0x8d, 0x1c },
+	{ 0x8e, 0x80 },
+	{ 0xe5, 0x04 },
+
+	{ 0xc0, 0x50 },
+	{ 0xc1, 0x3c },
+	{ 0xc2, 0x0c },
+};
+
+static const __u8 ov772x_reg_initdata[][2] = {
+	{ 0x12, 0x80 },
+	{ 0x11, 0x01 },
+
+	{ 0x3d, 0x03 },
+	{ 0x17, 0x26 },
+	{ 0x18, 0xa0 },
+	{ 0x19, 0x07 },
+	{ 0x1a, 0xf0 },
+	{ 0x32, 0x00 },
+	{ 0x29, 0xa0 },
+	{ 0x2c, 0xf0 },
+	{ 0x65, 0x20 },
+	{ 0x11, 0x01 },
+	{ 0x42, 0x7f },
+	{ 0x63, 0xe0 },
+	{ 0x64, 0xff },
+	{ 0x66, 0x00 },
+	{ 0x13, 0xf0 },
+	{ 0x0d, 0x41 },
+	{ 0x0f, 0xc5 },
+	{ 0x14, 0x11 },
+
+	{ 0x22, 0x7f },
+	{ 0x23, 0x03 },
+	{ 0x24, 0x40 },
+	{ 0x25, 0x30 },
+	{ 0x26, 0xa1 },
+	{ 0x2a, 0x00 },
+	{ 0x2b, 0x00 },
+	{ 0x6b, 0xaa },
+	{ 0x13, 0xff },
+
+	{ 0x90, 0x05 },
+	{ 0x91, 0x01 },
+	{ 0x92, 0x03 },
+	{ 0x93, 0x00 },
+	{ 0x94, 0x60 },
+	{ 0x95, 0x3c },
+	{ 0x96, 0x24 },
+	{ 0x97, 0x1e },
+	{ 0x98, 0x62 },
+	{ 0x99, 0x80 },
+	{ 0x9a, 0x1e },
+	{ 0x9b, 0x08 },
+	{ 0x9c, 0x20 },
+	{ 0x9e, 0x81 },
+
+	{ 0xa6, 0x04 },
+	{ 0x7e, 0x0c },
+	{ 0x7f, 0x16 },
+	{ 0x80, 0x2a },
+	{ 0x81, 0x4e },
+	{ 0x82, 0x61 },
+	{ 0x83, 0x6f },
+	{ 0x84, 0x7b },
+	{ 0x85, 0x86 },
+	{ 0x86, 0x8e },
+	{ 0x87, 0x97 },
+	{ 0x88, 0xa4 },
+	{ 0x89, 0xaf },
+	{ 0x8a, 0xc5 },
+	{ 0x8b, 0xd7 },
+	{ 0x8c, 0xe8 },
+	{ 0x8d, 0x20 },
+
+	{ 0x0c, 0x90 },
+
+	{ 0x2b, 0x00 },
+	{ 0x22, 0x7f },
+	{ 0x23, 0x03 },
+	{ 0x11, 0x01 },
+	{ 0x0c, 0xd0 },
+	{ 0x64, 0xff },
+	{ 0x0d, 0x41 },
+
+	{ 0x14, 0x41 },
+	{ 0x0e, 0xcd },
+	{ 0xac, 0xbf },
+	{ 0x8e, 0x00 },
+	{ 0x0c, 0xd0 }
+};
+
+
 /* setup method */
 static void ov534_setup(struct usb_device *udev)
 {
-	ov534_reg_verify_write(udev, 0xe7, 0x3a);
+	int i;
 
-	ov534_reg_write(udev, OV534_REG_ADDRESS, 0x60);
-	ov534_reg_write(udev, OV534_REG_ADDRESS, 0x60);
-	ov534_reg_write(udev, OV534_REG_ADDRESS, 0x60);
-	ov534_reg_write(udev, OV534_REG_ADDRESS, 0x42);
-
-	ov534_reg_verify_write(udev, 0xc2, 0x0c);
-	ov534_reg_verify_write(udev, 0x88, 0xf8);
-	ov534_reg_verify_write(udev, 0xc3, 0x69);
-	ov534_reg_verify_write(udev, 0x89, 0xff);
-	ov534_reg_verify_write(udev, 0x76, 0x03);
-	ov534_reg_verify_write(udev, 0x92, 0x01);
-	ov534_reg_verify_write(udev, 0x93, 0x18);
-	ov534_reg_verify_write(udev, 0x94, 0x10);
-	ov534_reg_verify_write(udev, 0x95, 0x10);
-	ov534_reg_verify_write(udev, 0xe2, 0x00);
-	ov534_reg_verify_write(udev, 0xe7, 0x3e);
-
-	ov534_reg_write(udev, 0x1c, 0x0a);
-	ov534_reg_write(udev, 0x1d, 0x22);
-	ov534_reg_write(udev, 0x1d, 0x06);
-
-	ov534_reg_verify_write(udev, 0x96, 0x00);
-
-	ov534_reg_write(udev, 0x97, 0x20);
-	ov534_reg_write(udev, 0x97, 0x20);
-	ov534_reg_write(udev, 0x97, 0x20);
-	ov534_reg_write(udev, 0x97, 0x0a);
-	ov534_reg_write(udev, 0x97, 0x3f);
-	ov534_reg_write(udev, 0x97, 0x4a);
-	ov534_reg_write(udev, 0x97, 0x20);
-	ov534_reg_write(udev, 0x97, 0x15);
-	ov534_reg_write(udev, 0x97, 0x0b);
-
-	ov534_reg_verify_write(udev, 0x8e, 0x40);
-	ov534_reg_verify_write(udev, 0x1f, 0x81);
-	ov534_reg_verify_write(udev, 0x34, 0x05);
-	ov534_reg_verify_write(udev, 0xe3, 0x04);
-	ov534_reg_verify_write(udev, 0x88, 0x00);
-	ov534_reg_verify_write(udev, 0x89, 0x00);
-	ov534_reg_verify_write(udev, 0x76, 0x00);
-	ov534_reg_verify_write(udev, 0xe7, 0x2e);
-	ov534_reg_verify_write(udev, 0x31, 0xf9);
-	ov534_reg_verify_write(udev, 0x25, 0x42);
-	ov534_reg_verify_write(udev, 0x21, 0xf0);
-
-	ov534_reg_write(udev, 0x1c, 0x00);
-	ov534_reg_write(udev, 0x1d, 0x40);
-	ov534_reg_write(udev, 0x1d, 0x02);
-	ov534_reg_write(udev, 0x1d, 0x00);
-	ov534_reg_write(udev, 0x1d, 0x02);
-	ov534_reg_write(udev, 0x1d, 0x57);
-	ov534_reg_write(udev, 0x1d, 0xff);
-
-	ov534_reg_verify_write(udev, 0x8d, 0x1c);
-	ov534_reg_verify_write(udev, 0x8e, 0x80);
-	ov534_reg_verify_write(udev, 0xe5, 0x04);
+	/* Initialize bridge chip */
+	for (i = 0; i < ARRAY_SIZE(ov534_reg_initdata); i++)
+		ov534_reg_write(udev, ov534_reg_initdata[i][0],
+				ov534_reg_initdata[i][1]);
 
 	ov534_set_led(udev, 1);
 
-	sccb_reg_write(udev, 0x12, 0x80);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x11, 0x01);
-
-	ov534_set_led(udev, 0);
-
-	sccb_reg_write(udev, 0x3d, 0x03);
-	sccb_reg_write(udev, 0x17, 0x26);
-	sccb_reg_write(udev, 0x18, 0xa0);
-	sccb_reg_write(udev, 0x19, 0x07);
-	sccb_reg_write(udev, 0x1a, 0xf0);
-	sccb_reg_write(udev, 0x32, 0x00);
-	sccb_reg_write(udev, 0x29, 0xa0);
-	sccb_reg_write(udev, 0x2c, 0xf0);
-	sccb_reg_write(udev, 0x65, 0x20);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x42, 0x7f);
-	sccb_reg_write(udev, 0x63, 0xe0);
-	sccb_reg_write(udev, 0x64, 0xff);
-	sccb_reg_write(udev, 0x66, 0x00);
-	sccb_reg_write(udev, 0x13, 0xf0);
-	sccb_reg_write(udev, 0x0d, 0x41);
-	sccb_reg_write(udev, 0x0f, 0xc5);
-	sccb_reg_write(udev, 0x14, 0x11);
-
-	ov534_set_led(udev, 1);
-
-	sccb_reg_write(udev, 0x22, 0x7f);
-	sccb_reg_write(udev, 0x23, 0x03);
-	sccb_reg_write(udev, 0x24, 0x40);
-	sccb_reg_write(udev, 0x25, 0x30);
-	sccb_reg_write(udev, 0x26, 0xa1);
-	sccb_reg_write(udev, 0x2a, 0x00);
-	sccb_reg_write(udev, 0x2b, 0x00);
-	sccb_reg_write(udev, 0x6b, 0xaa);
-	sccb_reg_write(udev, 0x13, 0xff);
-
-	ov534_set_led(udev, 0);
-
-	sccb_reg_write(udev, 0x90, 0x05);
-	sccb_reg_write(udev, 0x91, 0x01);
-	sccb_reg_write(udev, 0x92, 0x03);
-	sccb_reg_write(udev, 0x93, 0x00);
-	sccb_reg_write(udev, 0x94, 0x60);
-	sccb_reg_write(udev, 0x95, 0x3c);
-	sccb_reg_write(udev, 0x96, 0x24);
-	sccb_reg_write(udev, 0x97, 0x1e);
-	sccb_reg_write(udev, 0x98, 0x62);
-	sccb_reg_write(udev, 0x99, 0x80);
-	sccb_reg_write(udev, 0x9a, 0x1e);
-	sccb_reg_write(udev, 0x9b, 0x08);
-	sccb_reg_write(udev, 0x9c, 0x20);
-	sccb_reg_write(udev, 0x9e, 0x81);
-
-	ov534_set_led(udev, 1);
-
-	sccb_reg_write(udev, 0xa6, 0x04);
-	sccb_reg_write(udev, 0x7e, 0x0c);
-	sccb_reg_write(udev, 0x7f, 0x16);
-	sccb_reg_write(udev, 0x80, 0x2a);
-	sccb_reg_write(udev, 0x81, 0x4e);
-	sccb_reg_write(udev, 0x82, 0x61);
-	sccb_reg_write(udev, 0x83, 0x6f);
-	sccb_reg_write(udev, 0x84, 0x7b);
-	sccb_reg_write(udev, 0x85, 0x86);
-	sccb_reg_write(udev, 0x86, 0x8e);
-	sccb_reg_write(udev, 0x87, 0x97);
-	sccb_reg_write(udev, 0x88, 0xa4);
-	sccb_reg_write(udev, 0x89, 0xaf);
-	sccb_reg_write(udev, 0x8a, 0xc5);
-	sccb_reg_write(udev, 0x8b, 0xd7);
-	sccb_reg_write(udev, 0x8c, 0xe8);
-	sccb_reg_write(udev, 0x8d, 0x20);
-
-	sccb_reg_write(udev, 0x0c, 0x90);
-
-	ov534_reg_verify_write(udev, 0xc0, 0x50);
-	ov534_reg_verify_write(udev, 0xc1, 0x3c);
-	ov534_reg_verify_write(udev, 0xc2, 0x0c);
-
-	ov534_set_led(udev, 1);
-
-	sccb_reg_write(udev, 0x2b, 0x00);
-	sccb_reg_write(udev, 0x22, 0x7f);
-	sccb_reg_write(udev, 0x23, 0x03);
-	sccb_reg_write(udev, 0x11, 0x01);
-	sccb_reg_write(udev, 0x0c, 0xd0);
-	sccb_reg_write(udev, 0x64, 0xff);
-	sccb_reg_write(udev, 0x0d, 0x41);
-
-	sccb_reg_write(udev, 0x14, 0x41);
-	sccb_reg_write(udev, 0x0e, 0xcd);
-	sccb_reg_write(udev, 0xac, 0xbf);
-	sccb_reg_write(udev, 0x8e, 0x00);
-	sccb_reg_write(udev, 0x0c, 0xd0);
+	/* Initialize sensor */
+	for (i = 0; i < ARRAY_SIZE(ov772x_reg_initdata); i++)
+		sccb_reg_write(udev, ov772x_reg_initdata[i][0],
+			       ov772x_reg_initdata[i][1]);
 
 	ov534_reg_write(udev, 0xe0, 0x09);
 	ov534_set_led(udev, 0);
@@ -370,23 +351,23 @@
 	case 50:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x01);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0x41);
-		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x02);
+		ov534_reg_write(gspca_dev->dev, 0xe5, 0x02);
 		break;
 	case 40:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x02);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0xc1);
-		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x04);
+		ov534_reg_write(gspca_dev->dev, 0xe5, 0x04);
 		break;
 	case 30:
 	default:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x04);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0x81);
-		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x02);
+		ov534_reg_write(gspca_dev->dev, 0xe5, 0x02);
 		break;
 	case 15:
 		sccb_reg_write(gspca_dev->dev, 0x11, 0x03);
 		sccb_reg_write(gspca_dev->dev, 0x0d, 0x41);
-		ov534_reg_verify_write(gspca_dev->dev, 0xe5, 0x04);
+		ov534_reg_write(gspca_dev->dev, 0xe5, 0x04);
 		break;
 	};
 

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

[PATCH 3 of 4] ov534: Fix frame size so we don't miss the last pixel

[Jim Paris]

# HG changeset patch
# User jim@jtan.com
# Date 1228334850 18000
# Node ID 61c8d2959dd6ad1b45e0d8bf12b3d5748fc6a568
# Parent  893107a5df87228bb6766f26226fcef8069e3fc8
ov534: Fix frame size so we don't miss the last pixel

The frame size is too small, so we lose the last YUYV pixel.
Fix the setup and remove the last_pixel hack.

Signed-off-by: Jim Paris <jim@jtan.com>

diff -r 893107a5df87 -r 61c8d2959dd6 linux/drivers/media/video/gspca/ov534.c
--- a/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:06:54 2008 -0500
+++ b/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:07:30 2008 -0500
@@ -198,9 +198,9 @@
 	{ 0x1d, 0x40 },
 	{ 0x1d, 0x02 },
 	{ 0x1d, 0x00 },
-	{ 0x1d, 0x02 },
-	{ 0x1d, 0x57 },
-	{ 0x1d, 0xff },
+	{ 0x1d, 0x02 }, /* frame size 0x025800 * 4 = 614400 */
+	{ 0x1d, 0x58 }, /* frame size */
+	{ 0x1d, 0x00 }, /* frame size */
 
 	{ 0x8d, 0x1c },
 	{ 0x8e, 0x80 },
@@ -398,19 +398,12 @@
 static void sd_pkt_scan(struct gspca_dev *gspca_dev, struct gspca_frame *frame,
 			__u8 *data, int len)
 {
-	/*
-	 * The current camera setup doesn't stream the last pixel, so we set it
-	 * to a dummy value
-	 */
-	__u8 last_pixel[4] = { 0, 0, 0, 0 };
 	int framesize = gspca_dev->cam.bulk_size;
 
-	if (len == framesize - 4) {
-		frame =
-		    gspca_frame_add(gspca_dev, FIRST_PACKET, frame, data, len);
-		frame =
-		    gspca_frame_add(gspca_dev, LAST_PACKET, frame, last_pixel,
-				    4);
+	if (len == framesize) {
+		frame = gspca_frame_add(gspca_dev, FIRST_PACKET, frame,
+					data, len);
+		frame = gspca_frame_add(gspca_dev, LAST_PACKET, frame, data, 0);
 	} else
 		PDEBUG(D_PACK, "packet len = %d, framesize = %d", len,
 		       framesize);

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

[PATCH 4 of 4] ov534: frame transfer improvements

[Jim Paris]

# HG changeset patch
# User jim@jtan.com
# Date 1228335450 18000
# Node ID ede2dd835a2086131584558f5ccb1e2170135b07
# Parent  61c8d2959dd6ad1b45e0d8bf12b3d5748fc6a568
ov534: frame transfer improvements

The indirect registers at 0x1c/0x1d control frame settings.  If we
leave the values at 0x0a and 0x0b at their reset-time defaults, frame
data from the camera matches the UVC payload format.  This lets us
better reassemble the data into frames and know when data was lost.

This also lets us relax the bulk_size requirement from 600K to 2K,
which should help systems on with limited RAM (like the PS3).

Signed-off-by: Jim Paris <jim@jtan.com>

diff -r 61c8d2959dd6 -r ede2dd835a20 linux/drivers/media/video/gspca/ov534.c
--- a/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:07:30 2008 -0500
+++ b/linux/drivers/media/video/gspca/ov534.c	Wed Dec 03 15:17:30 2008 -0500
@@ -166,10 +166,6 @@
 	{ 0xe2, 0x00 },
 	{ 0xe7, 0x3e },
 
-	{ 0x1c, 0x0a },
-	{ 0x1d, 0x22 },
-	{ 0x1d, 0x06 },
-
 	{ 0x96, 0x00 },
 
 	{ 0x97, 0x20 },
@@ -328,10 +324,8 @@
 	cam->cam_mode = vga_mode;
 	cam->nmodes = ARRAY_SIZE(vga_mode);
 
-	cam->bulk_size = vga_mode[0].sizeimage;
+	cam->bulk_size = 2048;
 	cam->bulk_nurbs = 2;
-
-	PDEBUG(D_PROBE, "bulk_size = %d", cam->bulk_size);
 
 	return 0;
 }
@@ -379,8 +373,6 @@
 	PDEBUG(D_PROBE, "width = %d, height = %d",
 	       gspca_dev->width, gspca_dev->height);
 
-	gspca_dev->cam.bulk_size = gspca_dev->width * gspca_dev->height * 2;
-
 	/* start streaming data */
 	ov534_set_led(gspca_dev->dev, 1);
 	ov534_reg_write(gspca_dev->dev, 0xe0, 0x00);
@@ -395,18 +387,80 @@
 	ov534_set_led(gspca_dev->dev, 0);
 }
 
+/* Values for bmHeaderInfo (Video and Still Image Payload Headers, 2.4.3.3) */
+#define UVC_STREAM_EOH	(1 << 7)
+#define UVC_STREAM_ERR	(1 << 6)
+#define UVC_STREAM_STI	(1 << 5)
+#define UVC_STREAM_RES	(1 << 4)
+#define UVC_STREAM_SCR	(1 << 3)
+#define UVC_STREAM_PTS	(1 << 2)
+#define UVC_STREAM_EOF	(1 << 1)
+#define UVC_STREAM_FID	(1 << 0)
+
 static void sd_pkt_scan(struct gspca_dev *gspca_dev, struct gspca_frame *frame,
 			__u8 *data, int len)
 {
-	int framesize = gspca_dev->cam.bulk_size;
+	static __u32 last_pts;
+	__u32 this_pts;
+	static int last_fid;
+	int this_fid;
 
-	if (len == framesize) {
-		frame = gspca_frame_add(gspca_dev, FIRST_PACKET, frame,
-					data, len);
-		frame = gspca_frame_add(gspca_dev, LAST_PACKET, frame, data, 0);
-	} else
-		PDEBUG(D_PACK, "packet len = %d, framesize = %d", len,
-		       framesize);
+	/* Payloads are prefixed with a the UVC-style header.  We
+	   consider a frame to start when the FID toggles, or the PTS
+	   changes.  A frame ends when EOF is set, and we've received
+	   the correct number of bytes. */
+
+	/* Verify UVC header.  Header length is always 12 */
+	if (data[0] != 12 || len < 12) {
+		PDEBUG(D_PACK, "bad header");
+		goto discard;
+	}
+
+	/* Check errors */
+	if (data[1] & UVC_STREAM_ERR) {
+		PDEBUG(D_PACK, "payload error");
+		goto discard;
+	}
+
+	/* Extract PTS and FID */
+	if (!(data[1] & UVC_STREAM_PTS)) {
+		PDEBUG(D_PACK, "PTS not present");
+		goto discard;
+	}
+	this_pts = (data[5] << 24) | (data[4] << 16) | (data[3] << 8) | data[2];
+	this_fid = (data[1] & UVC_STREAM_FID) ? 1 : 0;
+
+	/* If PTS or FID has changed, start a new frame. */
+	if (this_pts != last_pts || this_fid != last_fid) {
+		frame = gspca_frame_add(gspca_dev, FIRST_PACKET, frame, NULL, 0);
+		last_pts = this_pts;
+		last_fid = this_fid;
+	}
+
+	/* Add the data from this payload */
+	frame = gspca_frame_add(gspca_dev, INTER_PACKET, frame,
+				data + 12, len - 12);
+
+	/* If this packet is marked as EOF, end the frame */
+	if (data[1] & UVC_STREAM_EOF) {
+		last_pts = 0;
+
+		if ((frame->data_end - frame->data) !=
+		    (gspca_dev->width * gspca_dev->height * 2)) {
+			PDEBUG(D_PACK, "short frame");
+			goto discard;
+		}
+
+		gspca_frame_add(gspca_dev, LAST_PACKET, frame, NULL, 0);
+	}
+
+	/* Done */
+	return;
+
+discard:
+	/* Discard data until a new frame starts. */
+	gspca_frame_add(gspca_dev, DISCARD_PACKET, frame, NULL, 0);
+	return;
 }
 
 /* sub-driver description */

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

Re: [PATCH 0 of 4] ov534 patches

[Jean-Francois Moine]

On Wed, 2008-12-03 at 15:46 -0500, Jim Paris wrote:
> Hi,

Hi Jim,

> Here are some ov534 patches I've been working on.
> 
> ov534: don't check status twice
> ov534: initialization cleanup
> ov534: Fix frame size so we don't miss the last pixel
> ov534: frame transfer improvements

Thank you for these patchs. Some changes were already done in my
repository. I merged them and pushed. May you check if everything is
correct?

Also, I moved the last fid and pts to the sd structure. This allows many
webcams to work simultaneously. I was wondering about the reset of these
variables: last_fid is never reset and last_pts is reset on
UVC_STREAM_EOF. Shouldn't they also be reset on streaming start?

Cheers.

-- 
Ken ar c'hentañ |             ** Breizh ha Linux atav! **
Jef             |               http://moinejf.free.fr/


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

Re: [PATCH 0 of 4] ov534 patches

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 1115 bytes --]

On Thu, 04 Dec 2008 09:14:02 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Wed, 2008-12-03 at 15:46 -0500, Jim Paris wrote:
> > Hi,
>

Thanks Jim.

> Hi Jim,
> 
> > Here are some ov534 patches I've been working on.
> > 
> > ov534: don't check status twice
> > ov534: initialization cleanup
> > ov534: Fix frame size so we don't miss the last pixel
> > ov534: frame transfer improvements
> 
> Thank you for these patchs. Some changes were already done in my
> repository. I merged them and pushed. May you check if everything is
> correct?
> 

Tested the latest version, I am getting "payload error"s setting
frame_rate=50, loosing about 50% of frames. I tried raising bulk_size
but then I get "frame overflow" errors from gspca, I'll investigate
further.

Regards,
   Antonio

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

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

Re: [PATCH 0 of 4] ov534 patches

[Jim Paris]

Antonio Ospite wrote:
> Tested the latest version, I am getting "payload error"s setting
> frame_rate=50, loosing about 50% of frames. I tried raising bulk_size
> but then I get "frame overflow" errors from gspca, I'll investigate
> further.

If bulk_size is bigger, the headers appear every 2048 bytes, not at
the beginning of each bulk packet.  Try the attached patch?

(The layout of sd_pkt_scan gets a bit strange, but the differences are
more clear this way; I can rewrite it a little more clearly)

-jim

--
ov534: improve payload handling

Frame data in bulk transfers is separated into 2048-byte payloads.
Each payload has its own header.

Signed-off-by: Jim Paris <jim@jtan.com>

diff -r ffeb9d2be572 linux/drivers/media/video/gspca/ov534.c
--- a/linux/drivers/media/video/gspca/ov534.c	Mon Dec 08 10:41:04 2008 +0100
+++ b/linux/drivers/media/video/gspca/ov534.c	Tue Dec 09 16:06:07 2008 -0500
@@ -1,6 +1,7 @@
 /*
  * ov534/ov772x gspca driver
  * Copyright (C) 2008 Antonio Ospite <ospite@studenti.unina.it>
+ * Copyright (C) 2008 Jim Paris <jim@jtan.com>
  *
  * Based on a prototype written by Mark Ferrell <majortrips@gmail.com>
  * USB protocol reverse engineered by Jim Paris <jim@jtan.com>
@@ -193,8 +194,8 @@
 
 	{ 0x1c, 0x00 },
 	{ 0x1d, 0x40 },
-	{ 0x1d, 0x02 },
-	{ 0x1d, 0x00 },
+	{ 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 */
@@ -325,7 +326,7 @@
 	cam->cam_mode = vga_mode;
 	cam->nmodes = ARRAY_SIZE(vga_mode);
 
-	cam->bulk_size = 2048;
+	cam->bulk_size = 16384;
 	cam->bulk_nurbs = 2;
 
 	return 0;
@@ -402,6 +403,17 @@
 	struct sd *sd = (struct sd *) gspca_dev;
 	__u32 this_pts;
 	int this_fid;
+	int remaining_len = len;
+	__u8 *next_data = data;
+
+scan_next:
+	if (remaining_len <= 0)
+		return;
+
+	data = next_data;
+	len = min(remaining_len, 2048);
+	remaining_len -= len;
+	next_data += len;
 
 	/* Payloads are prefixed with a the UVC-style header.  We
 	   consider a frame to start when the FID toggles, or the PTS
@@ -452,12 +464,13 @@
 		gspca_frame_add(gspca_dev, LAST_PACKET, frame, NULL, 0);
 	}
 
-	/* Done */
-	return;
+	/* Done this payload */
+	goto scan_next;
 
 discard:
 	/* Discard data until a new frame starts. */
 	gspca_frame_add(gspca_dev, DISCARD_PACKET, frame, NULL, 0);
+	goto scan_next;
 }
 
 /* sub-driver description */

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

Re: [PATCH 0 of 4] ov534 patches

[Jim Paris]

Hi Jean, Antonio,

Jean-Francois Moine wrote:
> Thank you for these patchs. Some changes were already done in my
> repository. I merged them and pushed. May you check if everything is
> correct?

I took a brief look and the merge looks OK, thanks.

> Also, I moved the last fid and pts to the sd structure. This allows many
> webcams to work simultaneously. I was wondering about the reset of these
> variables: last_fid is never reset and last_pts is reset on
> UVC_STREAM_EOF. Shouldn't they also be reset on streaming start?

I didn't consider multiple cameras.  Moving them of course makes sense
for that.  I did think of the reset case and thought it was OK, but
given Antonio's report below there might need to be some fixes in here
anyway.

Antonio Ospite wrote:
> Tested the latest version, I am getting "payload error"s setting
> frame_rate=50, loosing about 50% of frames. I tried raising
> bulk_size but then I get "frame overflow" errors from gspca, I'll
> investigate further.

I don't think I see any payload errors even at 50fps.  For the bulk
size, I'm not sure exactly how the payloads work into that.  I suppose
that when bulk_size is larger than the camera's payload size (2048),
we get another payload header at data[2048] but don't pay attention to
it.  If this header had the EOF then we can send gspca too much data,
causing frame overflow.  (there's no overflow check in ov534 since
gspca handles it already).

With the current setup, we're essentially getting a UVC stream.  This
makes sense since the marketing for ov534 says it supports UVC.  So
some documentation for this would be
  http://www.usb.org/developers/devclass_docs/USB_Video_Class_1_1.zip

This header and payload format is handled by a couple drivers that I
found:

  linux/drivers/media/video/uvc/uvc_video.c
     uvc_video_decode_start
     uvc_video_decode_data
     uvc_video_decode_end
     
  (I thought there was another Linux driver that also handled this
   payloads itself, but now I can't find it again)

  http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/usb/uvideo.c?rev=1.99
     uvideo_vs_decode_stream_header

Some discussion I found on payload headers in bulk transfers is here

  http://osdir.com/ml/linux.drivers.uvc.devel/2007-05/msg00036.html

Maybe finding a way to switch to isoc would make things easier?

-jim

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

Re: [PATCH 0 of 4] ov534 patches

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 1600 bytes --]

On Thu, 4 Dec 2008 12:15:46 -0500
Jim Paris <jim@jtan.com> wrote:

> 
> Antonio Ospite wrote:
> > Tested the latest version, I am getting "payload error"s setting
> > frame_rate=50, loosing about 50% of frames. I tried raising
> > bulk_size but then I get "frame overflow" errors from gspca, I'll
> > investigate further.
> 
> I don't think I see any payload errors even at 50fps.  For the bulk
> size, I'm not sure exactly how the payloads work into that.  I suppose
> that when bulk_size is larger than the camera's payload size (2048),
> we get another payload header at data[2048] but don't pay attention to
> it.  If this header had the EOF then we can send gspca too much data,
> causing frame overflow.  (there's no overflow check in ov534 since
> gspca handles it already).
>
> With the current setup, we're essentially getting a UVC stream.  This
> makes sense since the marketing for ov534 says it supports UVC.  So
> some documentation for this would be
>   http://www.usb.org/developers/devclass_docs/USB_Video_Class_1_1.zip
>

I need to read something more about UVC.

Ah, and from a quick test on PS3 it looks like the header is not added
here, but I haven't had the chance to see what exactly happened.

Hope to post some news soon.

Regards,
   Antonio

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

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

Re: [PATCH 0 of 4] ov534 patches

[Jim Paris]

Antonio Ospite wrote:
> I need to read something more about UVC.

Me too.  This bridge chip supports it -- maybe we just need to tweak
the USB descriptors so that the existing UVC driver knows what to do?

> Ah, and from a quick test on PS3 it looks like the header is not added
> here, but I haven't had the chance to see what exactly happened.

I'm relying on power on defaults, but we can also enable the header
explicitly if that's more reliable.

-jim

--

ov534: explicitly initialize frame format

Set frame format registers 0x0a and 0x0b to explicit values
rather than relying on reset-time defaults

Signed-off-by: Jim Paris <jim@jtan.com>

diff -r bc3e6d69f66b linux/drivers/media/video/gspca/ov534.c
--- a/linux/drivers/media/video/gspca/ov534.c	Tue Dec 09 16:06:08 2008 -0500
+++ b/linux/drivers/media/video/gspca/ov534.c	Tue Dec 09 16:20:30 2008 -0500
@@ -199,6 +199,10 @@
 	{ 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 }, /* .. */
 
 	{ 0x8d, 0x1c },
 	{ 0x8e, 0x80 },

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

Re: [PATCH 0 of 4] ov534 patches

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 1485 bytes --]

On Tue, 9 Dec 2008 16:21:27 -0500
Jim Paris <jim@jtan.com> wrote:

> Antonio Ospite wrote:
> > I need to read something more about UVC.
> 
> Me too.  This bridge chip supports it -- maybe we just need to tweak
> the USB descriptors so that the existing UVC driver knows what to do?
> 
> > Ah, and from a quick test on PS3 it looks like the header is not added
> > here, but I haven't had the chance to see what exactly happened.
> 
> I'm relying on power on defaults, but we can also enable the header
> explicitly if that's more reliable.
> 
> -jim
> 
> --
> 
> ov534: explicitly initialize frame format
>

With this one, plus:
ov534: improve payload handling

the PlayStation eye works again on PS3 at 30fps.

I haven't tested on PC at high frame rate yet, my main PC is broken
right now. I'll also add QVGA resolution as soon as it is fixed, 320x240
is handy for skype and co.

Jim, do you think a _per_resolution_ frame rate table is acceptable for
now? We can switch to a formula in a second time if we figure out how to
do that. I guess that registers values controlling frame rate are a
function of the frame size, right?

Thanks,
   Antonio

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

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