Hi, Reviewing and testing this one was, erm, interesting. See comments inline. On 04/28/2012 05:09 PM, Hans Verkuil wrote:
From: Hans Verkuil<hans.verkuil@xxxxxxxxx> Signed-off-by: Hans Verkuil<hans.verkuil@xxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 451 +++++++++++++++---------------------- 1 file changed, 182 insertions(+), 269 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index 7d9a4f1..e7b7599 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c
<snip various chunks, no comments on these>
+static int sd_setautogain(struct gspca_dev *gspca_dev, s32 val) +{ + if (!gspca_dev->streaming) + return 0; + setautogain(gspca_dev, val); + + return gspca_dev->usb_err; +} +
zcxx_s_ctrl can just as well call setautogain directly as it does for the others. It should gspca_dev->streaming for all controls anyways (more on that below).
+static int sd_setquality(struct gspca_dev *gspca_dev, s32 val) +{ + struct sd *sd = (struct sd *) gspca_dev; + int i; + + for (i = 0; i< ARRAY_SIZE(jpeg_qual) - 1; i++) { + if (val<= jpeg_qual[i]) + break; + } + sd->reg08 = i; + if (!gspca_dev->streaming) + return 0; + jpeg_set_qual(sd->jpeg_hdr, val); + return gspca_dev->usb_err; +} +
This for 99% duplicates the special handling you already have for this in zcxx_s_ctrl, which is also the only caller. More over this gets its wrong as under certain conditions it will end up with a different value of i then the code in zcxx_s_ctrl, and zcxx_s_ctrl bases the value it returns to the caller as the "achieved" value on its own calculation of i. So I thought lets refactor this a bit to get rid of this duplication and that turned out to be a long journey rather then a quick patch :) While testing my changes I found that there are various (pre-existing) problems with how zc3xx handles JPEG quality so I've fixed those first. The result is a 6 patch patchset (attached), which besides many fixes to the JPEG quality handling, also includes a new version of this patch removing the duplication.
+static int sd_set_jcomp(struct gspca_dev *gspca_dev, + struct v4l2_jpegcompression *jcomp) +{ + struct sd *sd = (struct sd *) gspca_dev; + + if (sd->jpegqual) { + v4l2_ctrl_s_ctrl(sd->jpegqual, jcomp->quality); + jcomp->quality = v4l2_ctrl_g_ctrl(sd->jpegqual); + return gspca_dev->usb_err; + } + jcomp->quality = jpeg_qual[sd->reg08]; + return 0; +} + +static int sd_get_jcomp(struct gspca_dev *gspca_dev, + struct v4l2_jpegcompression *jcomp) +{ + struct sd *sd = (struct sd *) gspca_dev; + + memset(jcomp, 0, sizeof *jcomp); + jcomp->quality = jpeg_qual[sd->reg08]; + jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT + | V4L2_JPEG_MARKER_DQT; + return 0; +} +
No need to move these upwards in the source file, leaving them where they are makes it easier to see what is actually changed.
+static int zcxx_g_volatile_ctrl(struct v4l2_ctrl *ctrl) +{ + struct sd *sd = container_of(ctrl->handler, struct sd, ctrl_handler); + + switch (ctrl->id) { + case V4L2_CID_AUTOGAIN: + if (ctrl->val&& sd->exposure&& sd->gspca_dev.streaming) + sd->exposure->val = getexposure(&sd->gspca_dev); + break; + } + return 0; +}
The call to getexposure needs to be surrounded by locking / unlocking usb_lock, and gspca_de->usb_err should be checked after the call. In general all calls to the actual hardware need to be made with the usb_lock hold, so that they cannot race with for example hardware accesses done from sd_start. This is important since although the usb-core will ensure that we never can do more then one control request at a time sometimes several control requests need to be done in order without interference (for example for accessing the i2c bus between the bridge and the sensor).
+ +static int zcxx_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct sd *sd = container_of(ctrl->handler, struct sd, ctrl_handler); + int ret; + int i; + + switch (ctrl->id) { + /* gamma/brightness/contrast cluster */ + case V4L2_CID_GAMMA: + setcontrast(&sd->gspca_dev, sd->gamma->val, + sd->brightness->val, sd->contrast->val); + return 0; + /* autogain/exposure cluster */ + case V4L2_CID_AUTOGAIN: + ret = sd_setautogain(&sd->gspca_dev, ctrl->val); + if (!ret&& !ctrl->val&& sd->exposure) + setexposure(&sd->gspca_dev, sd->exposure->val); + return ret; + case V4L2_CID_POWER_LINE_FREQUENCY: + setlightfreq(&sd->gspca_dev, ctrl->val); + return 0; + case V4L2_CID_SHARPNESS: + setsharpness(&sd->gspca_dev, ctrl->val); + return 0; + case V4L2_CID_JPEG_COMPRESSION_QUALITY: + for (i = 0; i< ARRAY_SIZE(jpeg_qual) - 1; i++) { + if (ctrl->val<= jpeg_qual[i]) + break; + } + if (i> 0&& i == sd->reg08&& ctrl->val< jpeg_qual[sd->reg08]) + i--; + ctrl->val = jpeg_qual[i]; + return sd_setquality(&sd->gspca_dev, ctrl->val); + } + return -EINVAL; +} +
Like zcxx_g_volatile_ctrl this function to should take the usb_lock before calling setfoo. Also after acquiring the lock it should check if the device is streaming and if it is not streaming it should not call any setfoo functions, as those will be done on sd_start then. Note that setfoo may work when calling them on this bridge while not streaming, but there are no guarantees the same holds for other bridges. In general the control paradigm in gspca is to not send any control values to the camera until streaming starts. Last this function to should check gspca_dev->usb_err (which works a bit like hdl->error accumulating io errors made by setfoo calls). So the order for any s_ctrl in gspca should normally be: 1) take usb_lock 2) clear gspca_dev->usb_err 3) check gspca_dev->streaming if not streaming normally goto unlock and return success immediately But there may be special cases where the passed in value should first be rounded to the nearest supported value (ie the jpeg quality in zc3xx.c) 4) call setfoo functions 5) store gspca_dev->usb_err in a local "ret" variable 6) unlock 7) return ret
+static const struct v4l2_ctrl_ops zcxx_ctrl_ops = { + .g_volatile_ctrl = zcxx_g_volatile_ctrl, + .s_ctrl = zcxx_s_ctrl, +}; + /* this function is called at probe and resume time */ static int sd_init(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; + struct v4l2_ctrl_handler *hdl =&sd->ctrl_handler; struct cam *cam; int sensor; static const u8 gamma[SENSOR_MAX] = { @@ -6688,7 +6673,6 @@ static int sd_init(struct gspca_dev *gspca_dev) case 0x2030: PDEBUG(D_PROBE, "Find Sensor PO2030"); sd->sensor = SENSOR_PO2030; - sd->ctrls[SHARPNESS].def = 0; /* from win traces */ break; case 0x7620: PDEBUG(D_PROBE, "Find Sensor OV7620"); @@ -6730,30 +6714,40 @@ static int sd_init(struct gspca_dev *gspca_dev) break; } - sd->ctrls[GAMMA].def = gamma[sd->sensor]; - sd->reg08 = reg08_tb[sd->sensor]; - sd->ctrls[QUALITY].def = jpeg_qual[sd->reg08]; - sd->ctrls[QUALITY].min = jpeg_qual[0]; - sd->ctrls[QUALITY].max = jpeg_qual[ARRAY_SIZE(jpeg_qual) - 1]; - - switch (sd->sensor) { - case SENSOR_HV7131R: - gspca_dev->ctrl_dis = (1<< QUALITY); - break; - case SENSOR_OV7630C: - gspca_dev->ctrl_dis = (1<< LIGHTFREQ) | (1<< EXPOSURE); - break; - case SENSOR_PAS202B: - gspca_dev->ctrl_dis = (1<< QUALITY) | (1<< EXPOSURE); - break; - default: - gspca_dev->ctrl_dis = (1<< EXPOSURE); - break; + gspca_dev->vdev.ctrl_handler = hdl; + v4l2_ctrl_handler_init(hdl, 8); + sd->brightness = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_BRIGHTNESS, 0, 255, 1, 128); + sd->contrast = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_CONTRAST, 0, 255, 1, 128); + sd->gamma = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_GAMMA, 1, 6, 1, gamma[sd->sensor]); + if (sd->sensor == SENSOR_HV7131R) + sd->exposure = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_EXPOSURE, 0x30d, 0x493e, 1, 0x927); + sd->autogain = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_AUTOGAIN, 0, 1, 1, 1); + if (sd->sensor != SENSOR_OV7630C&& sd->sensor != SENSOR_PAS202B) + sd->plfreq = v4l2_ctrl_new_std_menu(hdl,&zcxx_ctrl_ops, + V4L2_CID_POWER_LINE_FREQUENCY, + V4L2_CID_POWER_LINE_FREQUENCY_60HZ, 0, + V4L2_CID_POWER_LINE_FREQUENCY_DISABLED);
This is wrong the PAS202B should have a powerlinefreq control.
+ sd->sharpness = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_SHARPNESS, 0, 3, 1, + sd->sensor == SENSOR_PO2030 ? 0 : 2); + if (sd->sensor != SENSOR_HV7131R&& sd->sensor != SENSOR_PAS202B) + sd->jpegqual = v4l2_ctrl_new_std(hdl,&zcxx_ctrl_ops, + V4L2_CID_JPEG_COMPRESSION_QUALITY, + jpeg_qual[0], jpeg_qual[ARRAY_SIZE(jpeg_qual) - 1], 1, + jpeg_qual[sd->reg08]); + if (hdl->error) { + pr_err("Could not initialize controls\n"); + return hdl->error; } -#if AUTOGAIN_DEF - if (sd->ctrls[AUTOGAIN].val) - gspca_dev->ctrl_inac = (1<< EXPOSURE); -#endif + v4l2_ctrl_cluster(3,&sd->gamma); + if (sd->sensor == SENSOR_HV7131R) + v4l2_ctrl_auto_cluster(2,&sd->autogain, 0, true); + sd->reg08 = reg08_tb[sd->sensor]; /* switch off the led */ reg_w(gspca_dev, 0x01, 0x0000); @@ -6864,7 +6858,7 @@ static int sd_start(struct gspca_dev *gspca_dev) reg_w(gspca_dev, 0x03, 0x0008); break; } - setsharpness(gspca_dev); + setsharpness(gspca_dev, v4l2_ctrl_g_ctrl(sd->sharpness)); /* set the gamma tables when not set */ switch (sd->sensor) { @@ -6873,7 +6867,9 @@ static int sd_start(struct gspca_dev *gspca_dev) case SENSOR_OV7630C: break; default: - setcontrast(gspca_dev); + setcontrast(&sd->gspca_dev, v4l2_ctrl_g_ctrl(sd->gamma), + v4l2_ctrl_g_ctrl(sd->brightness), + v4l2_ctrl_g_ctrl(sd->contrast)); break; } setmatrix(gspca_dev); /* one more time? */ @@ -6886,7 +6882,7 @@ static int sd_start(struct gspca_dev *gspca_dev) } setquality(gspca_dev); jpeg_set_qual(sd->jpeg_hdr, jpeg_qual[sd->reg08]); - setlightfreq(gspca_dev); + setlightfreq(gspca_dev, v4l2_ctrl_g_ctrl(sd->plfreq));
sd->plfreq can be null, so this needs an if (sd->plfreq)
switch (sd->sensor) { case SENSOR_ADCM2700:
<snip> Moving on to the next patch in the series now, hopefully that one will go quicker :) Regards, Hans
>From 78fd672b2b87177ceff710ca26d9ae15420e66e7 Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@xxxxxxxxxx> Date: Sat, 5 May 2012 12:31:48 +0200 Subject: [PATCH 1/6] gspca_zc3xx: Fix setting of jpeg quality while streaming When the user changes the JPEG quality while the camera is streaming, the driver should not only change the JPEG headers send to userspace, but also actually tell the camera to use a different quantization table. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index 7d9a4f1..c7c9d11 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c @@ -5923,6 +5923,8 @@ static void setquality(struct gspca_dev *gspca_dev) struct sd *sd = (struct sd *) gspca_dev; s8 reg07; + jpeg_set_qual(sd->jpeg_hdr, jpeg_qual[sd->reg08]); + reg07 = 0; switch (sd->sensor) { case SENSOR_OV7620: @@ -6885,7 +6887,6 @@ static int sd_start(struct gspca_dev *gspca_dev) break; } setquality(gspca_dev); - jpeg_set_qual(sd->jpeg_hdr, jpeg_qual[sd->reg08]); setlightfreq(gspca_dev); switch (sd->sensor) { @@ -7041,7 +7042,7 @@ static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val) sd->reg08 = i; sd->ctrls[QUALITY].val = jpeg_qual[i]; if (gspca_dev->streaming) - jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val); + setquality(gspca_dev); return gspca_dev->usb_err; } -- 1.7.10
>From 38f3eb79db0350a05ac29473f0ce4db6efaa8960 Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@xxxxxxxxxx> Date: Sat, 5 May 2012 11:53:36 +0200 Subject: [PATCH 2/6] gspca_zc3xx: Fix JPEG quality setting code The current code is using bits 0-1 of register 8 of the zc3xx controller to set the JPEG quality, but the correct bits are bits 1-2. Bit 0 selects between truncation or rounding in the quantization phase of the compression, since rounding generally gives better results it should thus always be 1. This patch also corrects the quality percentages which belong to the 4 different settings. Lst this patch removes the different reg 8 defaults depending on the sensor type. Some of them where going for a default quality setting of 50%, which generally is not necessary in any way and results in poor image quality. 75% is a good default to use for all scenarios. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 64 +++++++++++++------------------------ 1 file changed, 22 insertions(+), 42 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index c7c9d11..f770676 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c @@ -32,7 +32,7 @@ MODULE_LICENSE("GPL"); static int force_sensor = -1; -#define REG08_DEF 3 /* default JPEG compression (70%) */ +#define REG08_DEF 3 /* default JPEG compression (75%) */ #include "zc3xx-reg.h" /* controls */ @@ -193,10 +193,10 @@ static const struct ctrl sd_ctrls[NCTRLS] = { .id = V4L2_CID_JPEG_COMPRESSION_QUALITY, .type = V4L2_CTRL_TYPE_INTEGER, .name = "Compression Quality", - .minimum = 40, - .maximum = 70, + .minimum = 50, + .maximum = 94, .step = 1, - .default_value = 70 /* updated in sd_init() */ + .default_value = 75, }, .set = sd_setquality }, @@ -241,8 +241,8 @@ static const struct v4l2_pix_format sif_mode[] = { .priv = 0}, }; -/* bridge reg08 -> JPEG quality conversion table */ -static u8 jpeg_qual[] = {40, 50, 60, 70, /*80*/}; +/* bridge reg08 bits 1-2 -> JPEG quality conversion table */ +static u8 jpeg_qual[] = {50, 75, 87, 94}; /* usb exchanges */ struct usb_action { @@ -5923,7 +5923,7 @@ static void setquality(struct gspca_dev *gspca_dev) struct sd *sd = (struct sd *) gspca_dev; s8 reg07; - jpeg_set_qual(sd->jpeg_hdr, jpeg_qual[sd->reg08]); + jpeg_set_qual(sd->jpeg_hdr, jpeg_qual[sd->reg08 >> 1]); reg07 = 0; switch (sd->sensor) { @@ -6079,11 +6079,12 @@ static void transfer_update(struct work_struct *work) struct sd *sd = container_of(work, struct sd, work); struct gspca_dev *gspca_dev = &sd->gspca_dev; int change, good; - u8 reg07, reg11; + u8 reg07, qual, reg11; /* synchronize with the main driver and initialize the registers */ mutex_lock(&gspca_dev->usb_lock); reg07 = 0; /* max */ + qual = sd->reg08 >> 1; reg_w(gspca_dev, reg07, 0x0007); reg_w(gspca_dev, sd->reg08, ZC3XX_R008_CLOCKSETTING); mutex_unlock(&gspca_dev->usb_lock); @@ -6108,9 +6109,9 @@ static void transfer_update(struct work_struct *work) case 0: /* max */ reg07 = sd->sensor == SENSOR_HV7131R ? 0x30 : 0x32; - if (sd->reg08 != 0) { + if (qual != 0) { change = 3; - sd->reg08--; + qual--; } break; case 0x32: @@ -6143,10 +6144,10 @@ static void transfer_update(struct work_struct *work) } } } else { /* reg07 max */ - if (sd->reg08 < sizeof jpeg_qual - 1) { + if (qual < sizeof jpeg_qual - 1) { good++; if (good > 10) { - sd->reg08++; + qual++; change = 2; } } @@ -6161,15 +6162,16 @@ static void transfer_update(struct work_struct *work) goto err; } if (change & 2) { + sd->reg08 = (qual << 1) | 1; reg_w(gspca_dev, sd->reg08, ZC3XX_R008_CLOCKSETTING); if (gspca_dev->usb_err < 0 || !gspca_dev->present || !gspca_dev->streaming) goto err; - sd->ctrls[QUALITY].val = jpeg_qual[sd->reg08]; + sd->ctrls[QUALITY].val = jpeg_qual[qual]; jpeg_set_qual(sd->jpeg_hdr, - jpeg_qual[sd->reg08]); + jpeg_qual[qual]); } } mutex_unlock(&gspca_dev->usb_lock); @@ -6561,27 +6563,6 @@ static int sd_init(struct gspca_dev *gspca_dev) [SENSOR_PO2030] = 1, [SENSOR_TAS5130C] = 1, }; - static const u8 reg08_tb[SENSOR_MAX] = { - [SENSOR_ADCM2700] = 1, - [SENSOR_CS2102] = 3, - [SENSOR_CS2102K] = 3, - [SENSOR_GC0303] = 2, - [SENSOR_GC0305] = 3, - [SENSOR_HDCS2020] = 1, - [SENSOR_HV7131B] = 3, - [SENSOR_HV7131R] = 3, - [SENSOR_ICM105A] = 3, - [SENSOR_MC501CB] = 3, - [SENSOR_MT9V111_1] = 3, - [SENSOR_MT9V111_3] = 3, - [SENSOR_OV7620] = 1, - [SENSOR_OV7630C] = 3, - [SENSOR_PAS106] = 3, - [SENSOR_PAS202B] = 3, - [SENSOR_PB0330] = 3, - [SENSOR_PO2030] = 2, - [SENSOR_TAS5130C] = 3, - }; sensor = zcxx_probeSensor(gspca_dev); if (sensor >= 0) @@ -6733,8 +6714,7 @@ static int sd_init(struct gspca_dev *gspca_dev) } sd->ctrls[GAMMA].def = gamma[sd->sensor]; - sd->reg08 = reg08_tb[sd->sensor]; - sd->ctrls[QUALITY].def = jpeg_qual[sd->reg08]; + sd->ctrls[QUALITY].def = jpeg_qual[sd->reg08 >> 1]; sd->ctrls[QUALITY].min = jpeg_qual[0]; sd->ctrls[QUALITY].max = jpeg_qual[ARRAY_SIZE(jpeg_qual) - 1]; @@ -7029,17 +7009,17 @@ static int sd_querymenu(struct gspca_dev *gspca_dev, static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val) { struct sd *sd = (struct sd *) gspca_dev; - int i; + int i, qual = sd->reg08 >> 1; - for (i = 0; i < ARRAY_SIZE(jpeg_qual) - 1; i++) { + for (i = 0; i < ARRAY_SIZE(jpeg_qual); i++) { if (val <= jpeg_qual[i]) break; } if (i > 0 - && i == sd->reg08 - && val < jpeg_qual[sd->reg08]) + && i == qual + && val < jpeg_qual[i]) i--; - sd->reg08 = i; + sd->reg08 = (i << 1) | 1; sd->ctrls[QUALITY].val = jpeg_qual[i]; if (gspca_dev->streaming) setquality(gspca_dev); -- 1.7.10
>From 00aef65c5f72326442f4aaf0a8ad49e67e1dc10e Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@xxxxxxxxxx> Date: Sat, 5 May 2012 14:26:33 +0200 Subject: [PATCH 3/6] gscpa_zc3xx: Always automatically adjust BRC as needed Always automatically adjust the Bit Rate Control setting as needed, independent of the sensor type. BRC is needed to not run out of bandwidth with higher quality settings independent of the sensor. Also only automatically adjust BRC, and don't adjust the JPEG quality control automatically, as that is not needed and leads to ugly flashes when it is changed. Note that before this patch-set the quality was never changed either due to the bugs in the quality handling fixed in previous patches in this set. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 159 +++++++++++++------------------------ 1 file changed, 53 insertions(+), 106 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index f770676..18ef68d 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c @@ -5921,22 +5921,8 @@ static void setexposure(struct gspca_dev *gspca_dev) static void setquality(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; - s8 reg07; - jpeg_set_qual(sd->jpeg_hdr, jpeg_qual[sd->reg08 >> 1]); - - reg07 = 0; - switch (sd->sensor) { - case SENSOR_OV7620: - reg07 = 0x30; - break; - case SENSOR_HV7131R: - case SENSOR_PAS202B: - return; /* done by work queue */ - } reg_w(gspca_dev, sd->reg08, ZC3XX_R008_CLOCKSETTING); - if (reg07 != 0) - reg_w(gspca_dev, reg07, 0x0007); } /* Matches the sensor's internal frame rate to the lighting frequency. @@ -6070,109 +6056,62 @@ static void setautogain(struct gspca_dev *gspca_dev) reg_w(gspca_dev, autoval, 0x0180); } -/* update the transfer parameters */ -/* This function is executed from a work queue. */ -/* The exact use of the bridge registers 07 and 08 is not known. - * The following algorithm has been adapted from ms-win traces */ +/* + * Update the transfer parameters. + * This function is executed from a work queue. + */ static void transfer_update(struct work_struct *work) { struct sd *sd = container_of(work, struct sd, work); struct gspca_dev *gspca_dev = &sd->gspca_dev; int change, good; - u8 reg07, qual, reg11; + u8 reg07, reg11; - /* synchronize with the main driver and initialize the registers */ - mutex_lock(&gspca_dev->usb_lock); - reg07 = 0; /* max */ - qual = sd->reg08 >> 1; - reg_w(gspca_dev, reg07, 0x0007); - reg_w(gspca_dev, sd->reg08, ZC3XX_R008_CLOCKSETTING); - mutex_unlock(&gspca_dev->usb_lock); + /* reg07 gets set to 0 by sd_start before starting us */ + reg07 = 0; good = 0; for (;;) { msleep(100); - /* get the transfer status */ - /* the bit 0 of the bridge register 11 indicates overflow */ mutex_lock(&gspca_dev->usb_lock); if (!gspca_dev->present || !gspca_dev->streaming) goto err; + + /* Bit 0 of register 11 indicates FIFO overflow */ + gspca_dev->usb_err = 0; reg11 = reg_r(gspca_dev, 0x0011); - if (gspca_dev->usb_err < 0 - || !gspca_dev->present || !gspca_dev->streaming) + if (gspca_dev->usb_err) goto err; change = reg11 & 0x01; if (change) { /* overflow */ - switch (reg07) { - case 0: /* max */ - reg07 = sd->sensor == SENSOR_HV7131R - ? 0x30 : 0x32; - if (qual != 0) { - change = 3; - qual--; - } - break; - case 0x32: - reg07 -= 4; - break; - default: - reg07 -= 2; - break; - case 2: - change = 0; /* already min */ - break; - } good = 0; + + if (reg07 == 0) /* Bit Rate Control not enabled? */ + reg07 = 0x32; /* Allow 98 bytes / unit */ + else if (reg07 > 2) + reg07 -= 2; /* Decrease allowed bytes / unit */ + else + change = 0; } else { /* no overflow */ - if (reg07 != 0) { /* if not max */ - good++; - if (good >= 10) { - good = 0; + good++; + if (good >= 10) { + good = 0; + if (reg07) { /* BRC enabled? */ change = 1; - reg07 += 2; - switch (reg07) { - case 0x30: - if (sd->sensor == SENSOR_PAS202B) - reg07 += 2; - break; - case 0x32: - case 0x34: + if (reg07 < 0x32) + reg07 += 2; + else reg07 = 0; - break; - } - } - } else { /* reg07 max */ - if (qual < sizeof jpeg_qual - 1) { - good++; - if (good > 10) { - qual++; - change = 2; - } } } } if (change) { - if (change & 1) { - reg_w(gspca_dev, reg07, 0x0007); - if (gspca_dev->usb_err < 0 - || !gspca_dev->present - || !gspca_dev->streaming) - goto err; - } - if (change & 2) { - sd->reg08 = (qual << 1) | 1; - reg_w(gspca_dev, sd->reg08, - ZC3XX_R008_CLOCKSETTING); - if (gspca_dev->usb_err < 0 - || !gspca_dev->present - || !gspca_dev->streaming) - goto err; - sd->ctrls[QUALITY].val = jpeg_qual[qual]; - jpeg_set_qual(sd->jpeg_hdr, - jpeg_qual[qual]); - } + gspca_dev->usb_err = 0; + reg_w(gspca_dev, reg07, 0x0007); + if (gspca_dev->usb_err) + goto err; } mutex_unlock(&gspca_dev->usb_lock); } @@ -6720,14 +6659,10 @@ static int sd_init(struct gspca_dev *gspca_dev) switch (sd->sensor) { case SENSOR_HV7131R: - gspca_dev->ctrl_dis = (1 << QUALITY); break; case SENSOR_OV7630C: gspca_dev->ctrl_dis = (1 << LIGHTFREQ) | (1 << EXPOSURE); break; - case SENSOR_PAS202B: - gspca_dev->ctrl_dis = (1 << QUALITY) | (1 << EXPOSURE); - break; default: gspca_dev->ctrl_dis = (1 << EXPOSURE); break; @@ -6742,6 +6677,13 @@ static int sd_init(struct gspca_dev *gspca_dev) return gspca_dev->usb_err; } +static int sd_pre_start(struct gspca_dev *gspca_dev) +{ + struct sd *sd = (struct sd *) gspca_dev; + gspca_dev->cam.needs_full_bandwidth = (sd->reg08 >= 4) ? 1 : 0; + return 0; +} + static int sd_start(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; @@ -6867,6 +6809,8 @@ static int sd_start(struct gspca_dev *gspca_dev) break; } setquality(gspca_dev); + /* Start with BRC disabled, transfer_update will enable it if needed */ + reg_w(gspca_dev, 0x00, 0x0007); setlightfreq(gspca_dev); switch (sd->sensor) { @@ -6904,19 +6848,14 @@ static int sd_start(struct gspca_dev *gspca_dev) setautogain(gspca_dev); - /* start the transfer update thread if needed */ - if (gspca_dev->usb_err >= 0) { - switch (sd->sensor) { - case SENSOR_HV7131R: - case SENSOR_PAS202B: - sd->work_thread = - create_singlethread_workqueue(KBUILD_MODNAME); - queue_work(sd->work_thread, &sd->work); - break; - } - } + if (gspca_dev->usb_err < 0) + return gspca_dev->usb_err; - return gspca_dev->usb_err; + /* Start the transfer parameters update thread */ + sd->work_thread = create_singlethread_workqueue(KBUILD_MODNAME); + queue_work(sd->work_thread, &sd->work); + + return 0; } /* called on streamoff with alt 0 and on disconnect */ @@ -7019,8 +6958,15 @@ static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val) && i == qual && val < jpeg_qual[i]) i--; + + /* With high quality settings we need max bandwidth */ + if (i >= 2 && gspca_dev->streaming && + !gspca_dev->cam.needs_full_bandwidth) + return -EBUSY; + sd->reg08 = (i << 1) | 1; sd->ctrls[QUALITY].val = jpeg_qual[i]; + if (gspca_dev->streaming) setquality(gspca_dev); return gspca_dev->usb_err; @@ -7070,6 +7016,7 @@ static const struct sd_desc sd_desc = { .nctrls = ARRAY_SIZE(sd_ctrls), .config = sd_config, .init = sd_init, + .isoc_init = sd_pre_start, .start = sd_start, .stop0 = sd_stop0, .pkt_scan = sd_pkt_scan, -- 1.7.10
>From 1c55e5072b567646b0e90773720ecb8030d90c99 Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@xxxxxxxxxx> Date: Sat, 5 May 2012 14:33:23 +0200 Subject: [PATCH 4/6] gscpa_zc3xx: Disable the highest quality setting as it is not usable Even with BRC the highest quality setting is not usable, BRC strips so much data from each MCU that the quality becomes worse then using a lower quality setting to begin with. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index 18ef68d..a8282b8 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c @@ -194,7 +194,7 @@ static const struct ctrl sd_ctrls[NCTRLS] = { .type = V4L2_CTRL_TYPE_INTEGER, .name = "Compression Quality", .minimum = 50, - .maximum = 94, + .maximum = 87, .step = 1, .default_value = 75, }, @@ -241,8 +241,11 @@ static const struct v4l2_pix_format sif_mode[] = { .priv = 0}, }; -/* bridge reg08 bits 1-2 -> JPEG quality conversion table */ -static u8 jpeg_qual[] = {50, 75, 87, 94}; +/* + * Bridge reg08 bits 1-2 -> JPEG quality conversion table. Note the highest + * quality setting is not usable as USB 1 does not have enough bandwidth. + */ +static u8 jpeg_qual[] = {50, 75, 87, /* 94 */}; /* usb exchanges */ struct usb_action { -- 1.7.10
>From c2a220d6cdd33615cd8dbff21c2e877a4b03c28e Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@xxxxxxxxxx> Date: Sat, 5 May 2012 14:54:17 +0200 Subject: [PATCH 5/6] gspca_zc3xx: Use get_jcomp to set the return values from set_jcomp This way not only quality gets set to the achieved value, but the other fields of the v4l2_jpegcompression struct also get properly set. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index a8282b8..5e00a7f 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c @@ -6975,16 +6975,6 @@ static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val) return gspca_dev->usb_err; } -static int sd_set_jcomp(struct gspca_dev *gspca_dev, - struct v4l2_jpegcompression *jcomp) -{ - struct sd *sd = (struct sd *) gspca_dev; - - sd_setquality(gspca_dev, jcomp->quality); - jcomp->quality = sd->ctrls[QUALITY].val; - return gspca_dev->usb_err; -} - static int sd_get_jcomp(struct gspca_dev *gspca_dev, struct v4l2_jpegcompression *jcomp) { @@ -6997,6 +6987,14 @@ static int sd_get_jcomp(struct gspca_dev *gspca_dev, return 0; } +static int sd_set_jcomp(struct gspca_dev *gspca_dev, + struct v4l2_jpegcompression *jcomp) +{ + sd_setquality(gspca_dev, jcomp->quality); + sd_get_jcomp(gspca_dev, jcomp); + return gspca_dev->usb_err; +} + #if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE) static int sd_int_pkt_scan(struct gspca_dev *gspca_dev, u8 *data, /* interrupt packet data */ -- 1.7.10
>From 6da72b85edbbcf8882d06ac1d4b9d0d991ab0142 Mon Sep 17 00:00:00 2001 From: Hans Verkuil <hans.verkuil@xxxxxxxxx> Date: Sat, 28 Apr 2012 17:09:51 +0200 Subject: [PATCH 6/6] gspca_zc3xx: convert to the control framework + various bug-fixes to the conversion by Hans de Goede Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/media/video/gspca/zc3xx.c | 427 +++++++++++++++---------------------- 1 file changed, 167 insertions(+), 260 deletions(-) diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index 5e00a7f..633bb4c 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c @@ -22,6 +22,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/input.h> +#include <media/v4l2-ctrls.h> #include "gspca.h" #include "jpeg.h" @@ -35,26 +36,23 @@ static int force_sensor = -1; #define REG08_DEF 3 /* default JPEG compression (75%) */ #include "zc3xx-reg.h" -/* controls */ -enum e_ctrl { - BRIGHTNESS, - CONTRAST, - EXPOSURE, - GAMMA, - AUTOGAIN, - LIGHTFREQ, - SHARPNESS, - QUALITY, - NCTRLS /* number of controls */ -}; - -#define AUTOGAIN_DEF 1 - /* specific webcam descriptor */ struct sd { struct gspca_dev gspca_dev; /* !! must be the first item */ - struct gspca_ctrl ctrls[NCTRLS]; + struct v4l2_ctrl_handler ctrl_handler; + struct { /* gamma/brightness/contrast control cluster */ + struct v4l2_ctrl *gamma; + struct v4l2_ctrl *brightness; + struct v4l2_ctrl *contrast; + }; + struct { /* autogain/exposure control cluster */ + struct v4l2_ctrl *autogain; + struct v4l2_ctrl *exposure; + }; + struct v4l2_ctrl *plfreq; + struct v4l2_ctrl *sharpness; + struct v4l2_ctrl *jpegqual; struct work_struct work; struct workqueue_struct *work_thread; @@ -94,114 +92,6 @@ enum sensors { SENSOR_MAX }; -/* V4L2 controls supported by the driver */ -static void setcontrast(struct gspca_dev *gspca_dev); -static void setexposure(struct gspca_dev *gspca_dev); -static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val); -static void setlightfreq(struct gspca_dev *gspca_dev); -static void setsharpness(struct gspca_dev *gspca_dev); -static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val); - -static const struct ctrl sd_ctrls[NCTRLS] = { -[BRIGHTNESS] = { - { - .id = V4L2_CID_BRIGHTNESS, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Brightness", - .minimum = 0, - .maximum = 255, - .step = 1, - .default_value = 128, - }, - .set_control = setcontrast - }, -[CONTRAST] = { - { - .id = V4L2_CID_CONTRAST, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Contrast", - .minimum = 0, - .maximum = 255, - .step = 1, - .default_value = 128, - }, - .set_control = setcontrast - }, -[EXPOSURE] = { - { - .id = V4L2_CID_EXPOSURE, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Exposure", - .minimum = 0x30d, - .maximum = 0x493e, - .step = 1, - .default_value = 0x927 - }, - .set_control = setexposure - }, -[GAMMA] = { - { - .id = V4L2_CID_GAMMA, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Gamma", - .minimum = 1, - .maximum = 6, - .step = 1, - .default_value = 4, - }, - .set_control = setcontrast - }, -[AUTOGAIN] = { - { - .id = V4L2_CID_AUTOGAIN, - .type = V4L2_CTRL_TYPE_BOOLEAN, - .name = "Auto Gain", - .minimum = 0, - .maximum = 1, - .step = 1, - .default_value = AUTOGAIN_DEF, - .flags = V4L2_CTRL_FLAG_UPDATE - }, - .set = sd_setautogain - }, -[LIGHTFREQ] = { - { - .id = V4L2_CID_POWER_LINE_FREQUENCY, - .type = V4L2_CTRL_TYPE_MENU, - .name = "Light frequency filter", - .minimum = 0, - .maximum = 2, /* 0: 0, 1: 50Hz, 2:60Hz */ - .step = 1, - .default_value = 0, - }, - .set_control = setlightfreq - }, -[SHARPNESS] = { - { - .id = V4L2_CID_SHARPNESS, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Sharpness", - .minimum = 0, - .maximum = 3, - .step = 1, - .default_value = 2, - }, - .set_control = setsharpness - }, -[QUALITY] = { - { - .id = V4L2_CID_JPEG_COMPRESSION_QUALITY, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Compression Quality", - .minimum = 50, - .maximum = 87, - .step = 1, - .default_value = 75, - }, - .set = sd_setquality - }, -}; - static const struct v4l2_pix_format vga_mode[] = { {320, 240, V4L2_PIX_FMT_JPEG, V4L2_FIELD_NONE, .bytesperline = 320, @@ -5821,10 +5711,8 @@ static void setmatrix(struct gspca_dev *gspca_dev) reg_w(gspca_dev, matrix[i], 0x010a + i); } -static void setsharpness(struct gspca_dev *gspca_dev) +static void setsharpness(struct gspca_dev *gspca_dev, s32 val) { - struct sd *sd = (struct sd *) gspca_dev; - int sharpness; static const u8 sharpness_tb[][2] = { {0x02, 0x03}, {0x04, 0x07}, @@ -5832,19 +5720,18 @@ static void setsharpness(struct gspca_dev *gspca_dev) {0x10, 0x1e} }; - sharpness = sd->ctrls[SHARPNESS].val; - reg_w(gspca_dev, sharpness_tb[sharpness][0], 0x01c6); + reg_w(gspca_dev, sharpness_tb[val][0], 0x01c6); reg_r(gspca_dev, 0x01c8); reg_r(gspca_dev, 0x01c9); reg_r(gspca_dev, 0x01ca); - reg_w(gspca_dev, sharpness_tb[sharpness][1], 0x01cb); + reg_w(gspca_dev, sharpness_tb[val][1], 0x01cb); } -static void setcontrast(struct gspca_dev *gspca_dev) +static void setcontrast(struct gspca_dev *gspca_dev, + s32 gamma, s32 brightness, s32 contrast) { - struct sd *sd = (struct sd *) gspca_dev; const u8 *Tgamma; - int g, i, brightness, contrast, adj, gp1, gp2; + int g, i, adj, gp1, gp2; u8 gr[16]; static const u8 delta_b[16] = /* delta for brightness */ {0x50, 0x38, 0x2d, 0x28, 0x24, 0x21, 0x1e, 0x1d, @@ -5867,10 +5754,10 @@ static void setcontrast(struct gspca_dev *gspca_dev) 0xe0, 0xeb, 0xf4, 0xff, 0xff, 0xff, 0xff, 0xff}, }; - Tgamma = gamma_tb[sd->ctrls[GAMMA].val - 1]; + Tgamma = gamma_tb[gamma - 1]; - contrast = ((int) sd->ctrls[CONTRAST].val - 128); /* -128 / 127 */ - brightness = ((int) sd->ctrls[BRIGHTNESS].val - 128); /* -128 / 92 */ + contrast -= 128; /* -128 / 127 */ + brightness -= 128; /* -128 / 92 */ adj = 0; gp1 = gp2 = 0; for (i = 0; i < 16; i++) { @@ -5897,25 +5784,15 @@ static void setcontrast(struct gspca_dev *gspca_dev) reg_w(gspca_dev, gr[i], 0x0130 + i); /* gradient */ } -static void getexposure(struct gspca_dev *gspca_dev) +static s32 getexposure(struct gspca_dev *gspca_dev) { - struct sd *sd = (struct sd *) gspca_dev; - - if (sd->sensor != SENSOR_HV7131R) - return; - sd->ctrls[EXPOSURE].val = (i2c_read(gspca_dev, 0x25) << 9) + return (i2c_read(gspca_dev, 0x25) << 9) | (i2c_read(gspca_dev, 0x26) << 1) | (i2c_read(gspca_dev, 0x27) >> 7); } -static void setexposure(struct gspca_dev *gspca_dev) +static void setexposure(struct gspca_dev *gspca_dev, s32 val) { - struct sd *sd = (struct sd *) gspca_dev; - int val; - - if (sd->sensor != SENSOR_HV7131R) - return; - val = sd->ctrls[EXPOSURE].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); @@ -5934,7 +5811,7 @@ static void setquality(struct gspca_dev *gspca_dev) * 60Hz, for American lighting * 0 = No Fliker (for outdoore usage) */ -static void setlightfreq(struct gspca_dev *gspca_dev) +static void setlightfreq(struct gspca_dev *gspca_dev, s32 val) { struct sd *sd = (struct sd *) gspca_dev; int i, mode; @@ -6018,7 +5895,7 @@ static void setlightfreq(struct gspca_dev *gspca_dev) tas5130c_60HZ, tas5130c_60HZScale}, }; - i = sd->ctrls[LIGHTFREQ].val * 2; + i = val * 2; mode = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv; if (mode) i++; /* 320x240 */ @@ -6028,14 +5905,14 @@ static void setlightfreq(struct gspca_dev *gspca_dev) usb_exchange(gspca_dev, zc3_freq); switch (sd->sensor) { case SENSOR_GC0305: - if (mode /* if 320x240 */ - && sd->ctrls[LIGHTFREQ].val == 1) /* and 50Hz */ + if (mode /* if 320x240 */ + && val == 1) /* and 50Hz */ reg_w(gspca_dev, 0x85, 0x018d); /* win: 0x80, 0x018d */ break; case SENSOR_OV7620: - if (!mode) { /* if 640x480 */ - if (sd->ctrls[LIGHTFREQ].val != 0) /* and filter */ + if (!mode) { /* if 640x480 */ + if (val != 0) /* and filter */ reg_w(gspca_dev, 0x40, 0x0002); else reg_w(gspca_dev, 0x44, 0x0002); @@ -6047,16 +5924,9 @@ static void setlightfreq(struct gspca_dev *gspca_dev) } } -static void setautogain(struct gspca_dev *gspca_dev) +static void setautogain(struct gspca_dev *gspca_dev, s32 val) { - struct sd *sd = (struct sd *) gspca_dev; - u8 autoval; - - if (sd->ctrls[AUTOGAIN].val) - autoval = 0x42; - else - autoval = 0x02; - reg_w(gspca_dev, autoval, 0x0180); + reg_w(gspca_dev, val ? 0x42 : 0x02, 0x0180); } /* @@ -6449,7 +6319,6 @@ static int sd_config(struct gspca_dev *gspca_dev, /* define some sensors from the vendor/product */ sd->sensor = id->driver_info; - gspca_dev->cam.ctrls = sd->ctrls; sd->reg08 = REG08_DEF; INIT_WORK(&sd->work, transfer_update); @@ -6457,10 +6326,97 @@ static int sd_config(struct gspca_dev *gspca_dev, return 0; } +static int zcxx_g_volatile_ctrl(struct v4l2_ctrl *ctrl) +{ + struct sd *sd = container_of(ctrl->handler, struct sd, ctrl_handler); + struct gspca_dev *gspca_dev = &sd->gspca_dev; + int ret = 0; + + switch (ctrl->id) { + case V4L2_CID_AUTOGAIN: + mutex_lock(&gspca_dev->usb_lock); + gspca_dev->usb_err = 0; + if (ctrl->val && sd->exposure && gspca_dev->streaming) + sd->exposure->val = getexposure(gspca_dev); + ret = gspca_dev->usb_err; + mutex_unlock(&gspca_dev->usb_lock); + break; + } + return ret; +} + +static int zcxx_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct sd *sd = container_of(ctrl->handler, struct sd, ctrl_handler); + struct gspca_dev *gspca_dev = &sd->gspca_dev; + int i, qual, ret = 0; + + mutex_lock(&gspca_dev->usb_lock); + gspca_dev->usb_err = 0; + + if (!gspca_dev->streaming && + ctrl->id != V4L2_CID_JPEG_COMPRESSION_QUALITY) + goto leave; + + switch (ctrl->id) { + /* gamma/brightness/contrast cluster */ + case V4L2_CID_GAMMA: + setcontrast(gspca_dev, sd->gamma->val, + sd->brightness->val, sd->contrast->val); + break; + /* autogain/exposure cluster */ + case V4L2_CID_AUTOGAIN: + setautogain(gspca_dev, ctrl->val); + if (!gspca_dev->usb_err && !ctrl->val && sd->exposure) + setexposure(gspca_dev, sd->exposure->val); + break; + case V4L2_CID_POWER_LINE_FREQUENCY: + setlightfreq(gspca_dev, ctrl->val); + break; + case V4L2_CID_SHARPNESS: + setsharpness(gspca_dev, ctrl->val); + break; + case V4L2_CID_JPEG_COMPRESSION_QUALITY: + qual = sd->reg08 >> 1; + + for (i = 0; i < ARRAY_SIZE(jpeg_qual); i++) { + if (ctrl->val <= jpeg_qual[i]) + break; + } + if (i > 0 && i == qual && ctrl->val < jpeg_qual[i]) + i--; + + /* With high quality settings we need max bandwidth */ + if (i >= 2 && gspca_dev->streaming && + !gspca_dev->cam.needs_full_bandwidth) { + ret = -EBUSY; + goto leave; + } + + sd->reg08 = (i << 1) | 1; + ctrl->val = jpeg_qual[i]; + + if (gspca_dev->streaming) + setquality(gspca_dev); + + break; + } + ret = gspca_dev->usb_err; +leave: + mutex_unlock(&gspca_dev->usb_lock); + return ret; +} + +static const struct v4l2_ctrl_ops zcxx_ctrl_ops = { + .g_volatile_ctrl = zcxx_g_volatile_ctrl, + .s_ctrl = zcxx_s_ctrl, +}; + /* this function is called at probe and resume time */ static int sd_init(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; + struct v4l2_ctrl_handler *hdl = &sd->ctrl_handler; struct cam *cam; int sensor; static const u8 gamma[SENSOR_MAX] = { @@ -6613,7 +6569,6 @@ static int sd_init(struct gspca_dev *gspca_dev) case 0x2030: PDEBUG(D_PROBE, "Find Sensor PO2030"); sd->sensor = SENSOR_PO2030; - sd->ctrls[SHARPNESS].def = 0; /* from win traces */ break; case 0x7620: PDEBUG(D_PROBE, "Find Sensor OV7620"); @@ -6655,25 +6610,38 @@ static int sd_init(struct gspca_dev *gspca_dev) break; } - sd->ctrls[GAMMA].def = gamma[sd->sensor]; - sd->ctrls[QUALITY].def = jpeg_qual[sd->reg08 >> 1]; - sd->ctrls[QUALITY].min = jpeg_qual[0]; - sd->ctrls[QUALITY].max = jpeg_qual[ARRAY_SIZE(jpeg_qual) - 1]; - - switch (sd->sensor) { - case SENSOR_HV7131R: - break; - case SENSOR_OV7630C: - gspca_dev->ctrl_dis = (1 << LIGHTFREQ) | (1 << EXPOSURE); - break; - default: - gspca_dev->ctrl_dis = (1 << EXPOSURE); - break; + gspca_dev->vdev.ctrl_handler = hdl; + v4l2_ctrl_handler_init(hdl, 8); + sd->brightness = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_BRIGHTNESS, 0, 255, 1, 128); + sd->contrast = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_CONTRAST, 0, 255, 1, 128); + sd->gamma = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_GAMMA, 1, 6, 1, gamma[sd->sensor]); + if (sd->sensor == SENSOR_HV7131R) + sd->exposure = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_EXPOSURE, 0x30d, 0x493e, 1, 0x927); + sd->autogain = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_AUTOGAIN, 0, 1, 1, 1); + if (sd->sensor != SENSOR_OV7630C) + sd->plfreq = v4l2_ctrl_new_std_menu(hdl, &zcxx_ctrl_ops, + V4L2_CID_POWER_LINE_FREQUENCY, + V4L2_CID_POWER_LINE_FREQUENCY_60HZ, 0, + V4L2_CID_POWER_LINE_FREQUENCY_DISABLED); + sd->sharpness = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_SHARPNESS, 0, 3, 1, + sd->sensor == SENSOR_PO2030 ? 0 : 2); + sd->jpegqual = v4l2_ctrl_new_std(hdl, &zcxx_ctrl_ops, + V4L2_CID_JPEG_COMPRESSION_QUALITY, + jpeg_qual[0], jpeg_qual[ARRAY_SIZE(jpeg_qual) - 1], 1, + jpeg_qual[REG08_DEF >> 1]); + if (hdl->error) { + pr_err("Could not initialize controls\n"); + return hdl->error; } -#if AUTOGAIN_DEF - if (sd->ctrls[AUTOGAIN].val) - gspca_dev->ctrl_inac = (1 << EXPOSURE); -#endif + v4l2_ctrl_cluster(3, &sd->gamma); + if (sd->sensor == SENSOR_HV7131R) + v4l2_ctrl_auto_cluster(2, &sd->autogain, 0, true); /* switch off the led */ reg_w(gspca_dev, 0x01, 0x0000); @@ -6791,7 +6759,7 @@ static int sd_start(struct gspca_dev *gspca_dev) reg_w(gspca_dev, 0x03, 0x0008); break; } - setsharpness(gspca_dev); + setsharpness(gspca_dev, v4l2_ctrl_g_ctrl(sd->sharpness)); /* set the gamma tables when not set */ switch (sd->sensor) { @@ -6800,7 +6768,9 @@ static int sd_start(struct gspca_dev *gspca_dev) case SENSOR_OV7630C: break; default: - setcontrast(gspca_dev); + setcontrast(gspca_dev, v4l2_ctrl_g_ctrl(sd->gamma), + v4l2_ctrl_g_ctrl(sd->brightness), + v4l2_ctrl_g_ctrl(sd->contrast)); break; } setmatrix(gspca_dev); /* one more time? */ @@ -6814,7 +6784,8 @@ static int sd_start(struct gspca_dev *gspca_dev) setquality(gspca_dev); /* Start with BRC disabled, transfer_update will enable it if needed */ reg_w(gspca_dev, 0x00, 0x0007); - setlightfreq(gspca_dev); + if (sd->plfreq) + setlightfreq(gspca_dev, v4l2_ctrl_g_ctrl(sd->plfreq)); switch (sd->sensor) { case SENSOR_ADCM2700: @@ -6825,7 +6796,7 @@ static int sd_start(struct gspca_dev *gspca_dev) reg_w(gspca_dev, 0x40, 0x0117); break; case SENSOR_HV7131R: - setexposure(gspca_dev); + setexposure(gspca_dev, v4l2_ctrl_g_ctrl(sd->exposure)); reg_w(gspca_dev, 0x00, ZC3XX_R1A7_CALCGLOBALMEAN); break; case SENSOR_GC0305: @@ -6849,7 +6820,7 @@ static int sd_start(struct gspca_dev *gspca_dev) break; } - setautogain(gspca_dev); + setautogain(gspca_dev, v4l2_ctrl_g_ctrl(sd->autogain)); if (gspca_dev->usb_err < 0) return gspca_dev->usb_err; @@ -6910,78 +6881,13 @@ static void sd_pkt_scan(struct gspca_dev *gspca_dev, gspca_frame_add(gspca_dev, INTER_PACKET, data, len); } -static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val) -{ - struct sd *sd = (struct sd *) gspca_dev; - - sd->ctrls[AUTOGAIN].val = val; - if (val) { - gspca_dev->ctrl_inac |= (1 << EXPOSURE); - } else { - gspca_dev->ctrl_inac &= ~(1 << EXPOSURE); - if (gspca_dev->streaming) - getexposure(gspca_dev); - } - if (gspca_dev->streaming) - setautogain(gspca_dev); - return gspca_dev->usb_err; -} - -static int sd_querymenu(struct gspca_dev *gspca_dev, - struct v4l2_querymenu *menu) -{ - switch (menu->id) { - case V4L2_CID_POWER_LINE_FREQUENCY: - switch (menu->index) { - case 0: /* V4L2_CID_POWER_LINE_FREQUENCY_DISABLED */ - strcpy((char *) menu->name, "NoFliker"); - return 0; - case 1: /* V4L2_CID_POWER_LINE_FREQUENCY_50HZ */ - strcpy((char *) menu->name, "50 Hz"); - return 0; - case 2: /* V4L2_CID_POWER_LINE_FREQUENCY_60HZ */ - strcpy((char *) menu->name, "60 Hz"); - return 0; - } - break; - } - return -EINVAL; -} - -static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val) -{ - struct sd *sd = (struct sd *) gspca_dev; - int i, qual = sd->reg08 >> 1; - - for (i = 0; i < ARRAY_SIZE(jpeg_qual); i++) { - if (val <= jpeg_qual[i]) - break; - } - if (i > 0 - && i == qual - && val < jpeg_qual[i]) - i--; - - /* With high quality settings we need max bandwidth */ - if (i >= 2 && gspca_dev->streaming && - !gspca_dev->cam.needs_full_bandwidth) - return -EBUSY; - - sd->reg08 = (i << 1) | 1; - sd->ctrls[QUALITY].val = jpeg_qual[i]; - - if (gspca_dev->streaming) - setquality(gspca_dev); - return gspca_dev->usb_err; -} - static int sd_get_jcomp(struct gspca_dev *gspca_dev, struct v4l2_jpegcompression *jcomp) { struct sd *sd = (struct sd *) gspca_dev; memset(jcomp, 0, sizeof *jcomp); - jcomp->quality = sd->ctrls[QUALITY].val; + jcomp->quality = v4l2_ctrl_g_ctrl(sd->jpegqual); jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT | V4L2_JPEG_MARKER_DQT; return 0; @@ -6990,9 +6896,13 @@ static int sd_get_jcomp(struct gspca_dev *gspca_dev, static int sd_set_jcomp(struct gspca_dev *gspca_dev, struct v4l2_jpegcompression *jcomp) { - sd_setquality(gspca_dev, jcomp->quality); + struct sd *sd = (struct sd *) gspca_dev; + int ret; + + ret = v4l2_ctrl_s_ctrl(sd->jpegqual, jcomp->quality); sd_get_jcomp(gspca_dev, jcomp); - return gspca_dev->usb_err; + + return ret; } #if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE) @@ -7013,15 +6923,12 @@ static int sd_int_pkt_scan(struct gspca_dev *gspca_dev, static const struct sd_desc sd_desc = { .name = KBUILD_MODNAME, - .ctrls = sd_ctrls, - .nctrls = ARRAY_SIZE(sd_ctrls), .config = sd_config, .init = sd_init, .isoc_init = sd_pre_start, .start = sd_start, .stop0 = sd_stop0, .pkt_scan = sd_pkt_scan, - .querymenu = sd_querymenu, .get_jcomp = sd_get_jcomp, .set_jcomp = sd_set_jcomp, #if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE) -- 1.7.10