Skip to contentSkip to conversation

gspca_zc3xx: Implement proper autogain and exposure control for OV7648

PATCH In review Ondrej Zary 2 participants 4 messages May 24, 2018 – May 25, 2018

Subsystems (2)
Media (V4L/DVB) Gspca USB webcam driver
lore
Share & tools
lore

Ondrej Zary

author
#0 lore raw

PATCH 1/3 gspca_zc3xx: Implement proper autogain and exposure control for OV7648

The ZS0211 internal autogain causes pumping and flickering with OV7648 sensor on 0ac8:307b webcam.
Implement OV7648 autogain and exposure control and use that instead.

Signed-off-by Ondrej Zary <[email protected]>
---
 drivers/media/usb/gspca/zc3xx.c | 42 +++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)
drivers/media/usb/gspca/zc3xx.c +34 -8
index 25b4dbe8e049..992918b3ad0c 100644
@@ -5778,16 +5778,34 @@ static void setcontrast(struct gspca_dev *gspca_dev,
 
 static s32 getexposure(struct gspca_dev *gspca_dev)
 {
-	return (i2c_read(gspca_dev, 0x25) << 9)
-		| (i2c_read(gspca_dev, 0x26) << 1)
-		| (i2c_read(gspca_dev, 0x27) >> 7);
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	switch (sd->sensor) {
+	case SENSOR_HV7131R:
+		return (i2c_read(gspca_dev, 0x25) << 9)
+			| (i2c_read(gspca_dev, 0x26) << 1)
+			| (i2c_read(gspca_dev, 0x27) >> 7);
+	case SENSOR_OV7620:
+		return i2c_read(gspca_dev, 0x10);
+	default:
+		return -1;
+	}
 }
 
 static void setexposure(struct gspca_dev *gspca_dev, s32 val)
 {
-	i2c_write(gspca_dev, 0x25, val >> 9, 0x00);
-	i2c_write(gspca_dev, 0x26, val >> 1, 0x00);
-	i2c_write(gspca_dev, 0x27, val << 7, 0x00);
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	switch (sd->sensor) {
+	case SENSOR_HV7131R:
+		i2c_write(gspca_dev, 0x25, val >> 9, 0x00);
+		i2c_write(gspca_dev, 0x26, val >> 1, 0x00);
+		i2c_write(gspca_dev, 0x27, val << 7, 0x00);
+		break;
+	case SENSOR_OV7620:
+		i2c_write(gspca_dev, 0x10, val, 0x00);
+		break;
+	}
 }
 
 static void setquality(struct gspca_dev *gspca_dev)

@@ -5918,7 +5936,12 @@ static void setlightfreq(struct gspca_dev *gspca_dev, s32 val)
 
 static void setautogain(struct gspca_dev *gspca_dev, s32 val)
 {
-	reg_w(gspca_dev, val ? 0x42 : 0x02, 0x0180);
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	if (sd->sensor == SENSOR_OV7620)
+		i2c_write(gspca_dev, 0x13, val ? 0xa3 : 0x80, 0x00);
+	else
+		reg_w(gspca_dev, val ? 0x42 : 0x02, 0x0180);
 }
 
 /*

@@ -6439,6 +6462,9 @@ static int sd_init_controls(struct gspca_dev *gspca_dev)
 	if (sd->sensor == SENSOR_HV7131R)
 		sd->exposure = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops,
 			V4L2_CID_EXPOSURE, 0x30d, 0x493e, 1, 0x927);
+	else if (sd->sensor == SENSOR_OV7620)
+		sd->exposure = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops,
+			V4L2_CID_EXPOSURE, 0, 255, 1, 0x41);
 	sd->autogain = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops,
 			V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
 	if (sd->sensor != SENSOR_OV7630C)

@@ -6458,7 +6484,7 @@ static int sd_init_controls(struct gspca_dev *gspca_dev)
 		return hdl->error;
 	}
 	v4l2_ctrl_cluster(3, &sd->gamma);
-	if (sd->sensor == SENSOR_HV7131R)
+	if (sd->sensor == SENSOR_HV7131R || sd->sensor == SENSOR_OV7620)
 		v4l2_ctrl_auto_cluster(2, &sd->autogain, 0, true);
 	return 0;
 }
Signature · 1 line
Ondrej Zary
author
#1 lore raw

PATCH 2/3 gspca_zc3xx: Fix power line frequency settings for OV7648

Power line frequency settings for OV7648 sensor contain autogain and exposure commands, affecting unrelated controls. Remove them.

Signed-off-by Ondrej Zary <[email protected]>
---
 drivers/media/usb/gspca/zc3xx.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/media/usb/gspca/zc3xx.c +10 -7
index 992918b3ad0c..9a78420e8ad8 100644
@@ -3184,7 +3184,8 @@ static const struct usb_action ov7620_InitialScale[] = {	/* 320x240 */
 	{}
 };
 static const struct usb_action ov7620_50HZ[] = {
-	{0xaa, 0x13, 0x00a3},	/* 00,13,a3,aa */
+/*	{0xaa, 0x13, 0x00a3},	 * 00,13,a3,aa
+				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},	/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0096},	/* 00,2b,96,aa */
 	{0xaa, 0x75, 0x008a},	/* 00,75,8a,aa */

@@ -3195,15 +3196,16 @@ static const struct usb_action ov7620_50HZ[] = {
 	{0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH},	/* 01,95,00,cc */
 	{0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID},	/* 01,96,00,cc */
 	{0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW},	/* 01,97,83,cc */
-	{0xaa, 0x10, 0x0082},				/* 00,10,82,aa */
+/*	{0xaa, 0x10, 0x0082},	 * 00,10,82,aa
+				 * don't change exposure */
 	{0xaa, 0x76, 0x0003},				/* 00,76,03,aa */
 /*	{0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},		 * 00,02,40,cc
 							 * if mode0 (640x480) */
 	{}
 };
 static const struct usb_action ov7620_60HZ[] = {
-	{0xaa, 0x13, 0x00a3},			/* 00,13,a3,aa */
-						/* (bug in zs211.inf) */
+/*	{0xaa, 0x13, 0x00a3},	 * 00,13,a3,aa
+				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},			/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0000},			/* 00,2b,00,aa */
 	{0xaa, 0x75, 0x008a},			/* 00,75,8a,aa */

@@ -3214,7 +3216,8 @@ static const struct usb_action ov7620_60HZ[] = {
 	{0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH}, /* 01,95,00,cc */
 	{0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID}, /* 01,96,00,cc */
 	{0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW}, /* 01,97,83,cc */
-	{0xaa, 0x10, 0x0020},			/* 00,10,20,aa */
+/*	{0xaa, 0x10, 0x0020},	 * 00,10,20,aa
+				 * don't change exposure */
 	{0xaa, 0x76, 0x0003},			/* 00,76,03,aa */
 /*	{0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},	 * 00,02,40,cc
 						 * if mode0 (640x480) */

@@ -3224,8 +3227,8 @@ static const struct usb_action ov7620_60HZ[] = {
 	{}
 };
 static const struct usb_action ov7620_NoFliker[] = {
-	{0xaa, 0x13, 0x00a3},			/* 00,13,a3,aa */
-						/* (bug in zs211.inf) */
+/*	{0xaa, 0x13, 0x00a3},	 * 00,13,a3,aa
+				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},			/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0000},			/* 00,2b,00,aa */
 	{0xaa, 0x75, 0x008e},			/* 00,75,8e,aa */
Signature · 1 line
Ondrej Zary
#2 lore raw

Re: [PATCH 2/3] gspca_zc3xx: Fix power line frequency settings for OV7648

Quoted 13 lines · Ondrej Zary

Power line frequency settings for OV7648 sensor contain autogain and exposure commands, affecting unrelated controls. Remove them.

Signed-off-by Ondrej Zary <[email protected]>
---
 drivers/media/usb/gspca/zc3xx.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/media/usb/gspca/zc3xx.c +0 -0
index 992918b3ad0c..9a78420e8ad8 100644

drivers/media/usb/gspca/zc3xx.c static const struct usb_action ov7620_InitialScale[] = { /* 320x240 */ · line 3191

Quoted 9 lines
drivers/media/usb/gspca/zc3xx.c +2 -1
@@ -3184,7 +3184,8 @@ static const struct usb_action ov7620_InitialScale[] = {	/* 320x240 */
 	{}
 };
 static const struct usb_action ov7620_50HZ[] = {
-	{0xaa, 0x13, 0x00a3},	/* 00,13,a3,aa */
+/*	{0xaa, 0x13, 0x00a3},	 * 00,13,a3,aa
+				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},	/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0096},	/* 00,2b,96,aa */
 	{0xaa, 0x75, 0x008a},	/* 00,75,8a,aa */

Just remove these lines altogether. There are still present in the git history if they are ever needed again. Same for the next patch.

Regards,

Hans

Quoted 42 lines
drivers/media/usb/gspca/zc3xx.c +8 -6
@@ -3195,15 +3196,16 @@ static const struct usb_action ov7620_50HZ[] = {
 	{0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH},	/* 01,95,00,cc */
 	{0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID},	/* 01,96,00,cc */
 	{0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW},	/* 01,97,83,cc */
-	{0xaa, 0x10, 0x0082},				/* 00,10,82,aa */
+/*	{0xaa, 0x10, 0x0082},	 * 00,10,82,aa
+				 * don't change exposure */
 	{0xaa, 0x76, 0x0003},				/* 00,76,03,aa */
 /*	{0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},		 * 00,02,40,cc
 							 * if mode0 (640x480) */
 	{}
 };
 static const struct usb_action ov7620_60HZ[] = {
-	{0xaa, 0x13, 0x00a3},			/* 00,13,a3,aa */
-						/* (bug in zs211.inf) */
+/*	{0xaa, 0x13, 0x00a3},	 * 00,13,a3,aa
+				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},			/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0000},			/* 00,2b,00,aa */
 	{0xaa, 0x75, 0x008a},			/* 00,75,8a,aa */

@@ -3214,7 +3216,8 @@ static const struct usb_action ov7620_60HZ[] = {
 	{0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH}, /* 01,95,00,cc */
 	{0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID}, /* 01,96,00,cc */
 	{0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW}, /* 01,97,83,cc */
-	{0xaa, 0x10, 0x0020},			/* 00,10,20,aa */
+/*	{0xaa, 0x10, 0x0020},	 * 00,10,20,aa
+				 * don't change exposure */
 	{0xaa, 0x76, 0x0003},			/* 00,76,03,aa */
 /*	{0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},	 * 00,02,40,cc
 						 * if mode0 (640x480) */

@@ -3224,8 +3227,8 @@ static const struct usb_action ov7620_60HZ[] = {
 	{}
 };
 static const struct usb_action ov7620_NoFliker[] = {
-	{0xaa, 0x13, 0x00a3},			/* 00,13,a3,aa */
-						/* (bug in zs211.inf) */
+/*	{0xaa, 0x13, 0x00a3},	 * 00,13,a3,aa
+				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},			/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0000},			/* 00,2b,00,aa */
 	{0xaa, 0x75, 0x008e},			/* 00,75,8e,aa */
author
#3 lore raw

PATCH 3/3 gspca_zc3xx: Fix exposure with power line frequency for OV7648

In reply to Ondrej Zary

The 50Hz and 60Hz power line frequency settings disable short (1/120s and 1/100s) exposure times for banding filter, causing overexposed image near lamps. No flicker setting enables them (when banding filter is disabled and they're not used).

Seems that the logic is just the wrong way around. Fix it. (This bug came from the Windows driver.)

Signed-off-by Ondrej Zary <[email protected]>
---
 drivers/media/usb/gspca/zc3xx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/media/usb/gspca/zc3xx.c +9 -3
index 9a78420e8ad8..299ea70bfb67 100644
@@ -3188,7 +3188,9 @@ static const struct usb_action ov7620_50HZ[] = {
 				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},	/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0096},	/* 00,2b,96,aa */
-	{0xaa, 0x75, 0x008a},	/* 00,75,8a,aa */
+/*	{0xaa, 0x75, 0x008a},	 * 00,75,8a,aa */
+	/* enable 1/120s & 1/100s exposures for banding filter */
+	{0xaa, 0x75, 0x008e},
 	{0xaa, 0x2d, 0x0005},	/* 00,2d,05,aa */
 	{0xa0, 0x00, ZC3XX_R190_EXPOSURELIMITHIGH},	/* 01,90,00,cc */
 	{0xa0, 0x04, ZC3XX_R191_EXPOSURELIMITMID},	/* 01,91,04,cc */

@@ -3208,7 +3210,9 @@ static const struct usb_action ov7620_60HZ[] = {
 				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},			/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0000},			/* 00,2b,00,aa */
-	{0xaa, 0x75, 0x008a},			/* 00,75,8a,aa */
+/*	{0xaa, 0x75, 0x008a},			 * 00,75,8a,aa */
+	/* enable 1/120s & 1/100s exposures for banding filter */
+	{0xaa, 0x75, 0x008e},
 	{0xaa, 0x2d, 0x0005},			/* 00,2d,05,aa */
 	{0xa0, 0x00, ZC3XX_R190_EXPOSURELIMITHIGH}, /* 01,90,00,cc */
 	{0xa0, 0x04, ZC3XX_R191_EXPOSURELIMITMID}, /* 01,91,04,cc */

@@ -3231,7 +3235,9 @@ static const struct usb_action ov7620_NoFliker[] = {
 				 * don't change autoexposure */
 	{0xdd, 0x00, 0x0100},			/* 00,01,00,dd */
 	{0xaa, 0x2b, 0x0000},			/* 00,2b,00,aa */
-	{0xaa, 0x75, 0x008e},			/* 00,75,8e,aa */
+/*	{0xaa, 0x75, 0x008e},			 * 00,75,8e,aa */
+	/* disable 1/120s & 1/100s exposures for banding filter */
+	{0xaa, 0x75, 0x008a},
 	{0xaa, 0x2d, 0x0001},			/* 00,2d,01,aa */
 	{0xa0, 0x00, ZC3XX_R190_EXPOSURELIMITHIGH}, /* 01,90,00,cc */
 	{0xa0, 0x04, ZC3XX_R191_EXPOSURELIMITMID}, /* 01,91,04,cc */
Signature · 1 line
Ondrej Zary