Are you sure that register 0x18 is gain? you mentioned an effect on FPS. Wouldn't this imply that it is instead exposure time?
2008/9/6 Lukáš Karas <[EMAIL PROTECTED]>: > Ok, thanks for comments. > New patch is in attachment (patch comparet to branche m5602, revision 372). > > ______________________________________________________________ >> Od: [EMAIL PROTECTED] >> Komu: Lukáš Karas <[EMAIL PROTECTED]> >> CC: m560x-driver-devel <[email protected]> >> Datum: 06.09.2008 17:42 >> Předmět: Re: [M560x-driver-devel] m5602/s5k83a brightness,whiteness and gain >> control >> >>-----BEGIN PGP SIGNED MESSAGE----- >>Hash: SHA1 >> >> >> >>~ Luká? Karas wrote: >>| Hi Eric. >>| >>| There is my today edit for s5k83a sensor: >>http://www.karry.wz.cz/download/m5602.20080906.zip >>| I was edited m5602 branch. Now is possible controll brightness, >>whiteness and gain. >>| >>| I identify sensor register (0x18), that drasticaly extend expose time. >>With maximal value is fps about 3! >>| In this case getting good images in dark. You can see at: >>http://www.karry.wz.cz/download/m5602-night_mode.png >>| I named this properties as GAIN. It is right? I dont find apposites >>properties on http://v4l2spec.bytesex.org/spec/x542.htm ... >>| >>| At end, I have small question. What you use as camera viewer? I find >>neither viewer, that support change all parameters >>| and work good with m5602... >>| >>| Regards Karry >> >> >>Hi again. >>This is a diff against the current driver. See my comments inline. >>Please address them and send me the updated driver as a patch. >> >> >>- --- >>/home/erik/Utveckling/m560x-driver/m560x/branches/m5602/m5602_s5k83a.c >>2008-09-06 16:59:02.000000000 +0200 >>+++ m5602_s5k83a.c 2008-09-06 13:07:53.000000000 +0200 >>@@ -18,6 +18,11 @@ >> >>~ #include "m5602_s5k83a.h" >> >>+static int brightness = S5K83A_DEFAULT_BRIGHTNESS; >>+static int whiteness = S5K83A_DEFAULT_WHITENESS; >>+static int gain = S5K83A_DEFAULT_GAIN; >>+ >> >>Do not use static integers in the code. It's not pretty and can cause >>race conditions. Also why shadow the registers in the sensor with local >>variables? >> > > Sorry, I dont know how make it better. I try read register values instead > shadows his, but read function return me different values compare with > written in some cases! > Especiali with whitness, what is write with sequence three registers. > It is hard when we have any datasheet. > > Maybe for pretty code create new structure with actual controls... > >>+ >>~ static int s5k83a_probe(struct m5602_camera *cam) >>~ { >>~ u8 prod_id = 0, ver_id = 0; >>@@ -159,6 +164,10 @@ >>~ return -EINVAL; >>~ } >>~ } >>+ >>+ s5k83a_set_brightness(cam,brightness); >>+ s5k83a_set_whiteness(cam, whiteness); >>+ s5k83a_set_gain(cam, gain); >> >>This is ok I guess, although I would prefer if you altered the init code >>in the m5602_s5k83a.h instead. > > Good idea. I do it. > >> >>~ if (dump_sensor) >>~ s5k83a_dump_registers(cam); >>@@ -173,13 +182,102 @@ >> >>~ static int s5k83a_get_ctrl(struct m5602_camera *cam, struct >>v4l2_control *ctrl) >>~ { >>- - return 0; >>+ switch (ctrl->id) { >>+ case V4L2_CID_BRIGHTNESS: >>+ ctrl->value = brightness; >>+ PDEBUG(DBG_V4L2, "get brightness (0x%x)", ctrl->value ); >>+ return 0; >> >>Probe the actual sensor registers instead of copying the shadowed variable. >> > > I explayin top, why I do this way. > >>+ case V4L2_CID_WHITENESS: >>+ ctrl->value = whiteness; >>+ PDEBUG(DBG_V4L2, "get whiteness (0x%x)", ctrl->value ); >> >>Same here. >> >>+ return 0; >>+ case V4L2_CID_GAIN: >>+ ctrl->value = gain; >>+ PDEBUG(DBG_V4L2, "get gain (0x%x)", ctrl->value ); >>+ return 0; >> >>And here >>+ default: >>+ PDEBUG(DBG_V4L2, "s5k83a_get_ctrl 0x%x", ctrl->id); >> >>This should warn that the selected ioctl isn't supported. >> > > OK > >> >>+ } >>+ return EINVAL; >>~ } >> >> >> >>~ static int s5k83a_set_ctrl(struct m5602_camera *cam, >>~ const struct v4l2_control *ctrl) >>~ { >>- - return 0; >>+ switch (ctrl->id) { >>+ case V4L2_CID_BRIGHTNESS: >>+ brightness = ctrl->value; >>+ s5k83a_set_brightness( cam, ctrl->value ); >>+ PDEBUG(DBG_V4L2, "set brightness to 0x%x", ctrl->value ); >>+ return 0; >> >>Please remove the spaces around the brackets and replace the return with >>a break. >> > > ok > >> >>+ case V4L2_CID_WHITENESS: >>+ whiteness = ctrl->value; >>+ s5k83a_set_whiteness(cam, ctrl->value); >>+ PDEBUG(DBG_V4L2, "set whiteness to 0x%x", ctrl->value ); >>+ return 0; >> >>Same here. >> >>+ case V4L2_CID_GAIN: >>+ gain = ctrl->value; >>+ s5k83a_set_gain(cam, ctrl->value); >>+ PDEBUG(DBG_V4L2, "set whiteness to 0x%x", ctrl->value ); >>+ return 0; >> >>Same here. >>+ default: >>+ PDEBUG(DBG_V4L2, "s5k83a_set_ctrl 0x%x to value 0x%x", >>ctrl->id, ctrl->value); >>+ } >>+ return EINVAL; >> >>Check for errors and set the return value properly. > > ok > >> >>+} >>+ >>+ >>+static void s5k83a_set_brightness(struct m5602_camera *cam, u16 value) >>+{ >>+ int err = 0; >>+ u8 data[2]; >>+ >>+ data[0] = 0x00; >>+ data[1] = 0x20; >>+ err = s5k83a_write_sensor(cam, 0x14, data, 2); >>Is this necessary? What does it do? >> > > Yes. It is necessary. I extract this sequence from snoop with > windows driver. Without write 0x14 and 0x0d registers > before 0x1b has strange results. > >>+ >>+ data[0] = 0x01; >>+ data[1] = (value & 0xff); // part of expose time (i dont know >>what is this...) >> >>If you don't know what this is why do you write to it? > > Value of second byte change fractionally brightness. > This change is little with compare 0x1b register. > I set this value to 0x00 > >> >>+ err = s5k83a_write_sensor(cam, 0x0d, data, 2); >>+ >>+ data[0] = (value ) >> 3; // brightness, high 5 bits >>+ data[1] = (value ) >> 2; // brightness, high 6 bits (range is >>7bits, but with the highest bit is unusable) >>This doesn't make any sense. Do you really need to write almost the same >>value to the two bytes? What happens if you only write to one byte? >> > > So. This values are partly experimented. Both bytes change brightness. > If use both, range is greater. > > Can you see video with this? > >>+ err = s5k83a_write_sensor(cam, 0x1b, data, 2); >>+ >>+} >>+ >>+static void s5k83a_set_whiteness(struct m5602_camera *cam, u8 value) >>+{ >>+ int tmp, err = 0; >>+ u8 data[2]; >>+ tmp = value; >>+ >>+ >>+ data[0] = value; >>+ data[1] = 0x00; >>+ err = s5k83a_write_sensor(cam, 0x0a, data, 1); >>+ >>If you only write the first byte, don't set the next at all. >> > > ok > >>+ >>+ /* >>+ // this code is for playing :-D >>+ info("set to value 0x%x", tmp); >>+ >>+ data[0] = tmp; >>+ data[1] = tmp; >>+ err = s5k83a_write_sensor(cam, 0xe8, data, 2); >>+ */ >>Remove this comment. >> > > ok > >>+} >>+ >>+static void s5k83a_set_gain(struct m5602_camera *cam, u8 value) >>Don't return void but the error value generated by the >>s5k83a_write_sensor() command. >> >>+{ >>+ // values near 0x3c is greath for nigh mode >>Remove this comment. >> > > ok > >>+ int err = 0; >>+ u8 data[2]; >>+ >>+ data[0] = 0; >>+ data[1] = value; >>+ err = s5k83a_write_sensor(cam, 0x18, data, 2); >> >>Is it only the high byte that alters the gain? >> > > Yes exactly. First byte change nothing. Second byte change "gain" only from > 0x00 to 0x3c... > >>+ >>~ } >> >>~ static void s5k83a_dump_registers(struct m5602_camera *cam) >> >>Thanks, >>Erik >> >> > > Please write me how solve problem with shadows registers/ controls > > Karry > > >> >>| >>| >>| >>| ------------------------------------------------------------------------- >>| This SF.Net email is sponsored by the Moblin Your Move Developer's >>challenge >>| Build the coolest Linux based applications with Moblin SDK & win great >>prizes >>| Grand prize is a trip for two to an Open Source event anywhere in the >>world >>| http://moblin-contest.org/redirect.php?banner_id=100&url=/ >>| _______________________________________________ >>| M560x-driver-devel mailing list >>| [email protected] >>| https://lists.sourceforge.net/lists/listinfo/m560x-driver-devel >>| >>-----BEGIN PGP SIGNATURE----- >>Version: GnuPG v1.4.6 (GNU/Linux) >> >>iD8DBQFIwqQcN7qBt+4UG0ERAgiAAJ9VeLNvfTyxm+oHSRNphSpItDOazwCdFXn0 >>h7BA8goqSrDa0FMyJfEPrNE= >>=OpcB >>-----END PGP SIGNATURE----- >> > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > M560x-driver-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/m560x-driver-devel > > ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ M560x-driver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/m560x-driver-devel
