-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Grégory Lardière wrote: | Hi, | | We also had problems when reading S5K4AA registers, maybe the same fix will work for S5K83A, take a look at revision 322. | Concerning exposure time, it seems controlled by a 16bits value (registers 0x17-0x18) on sensor S5K4AA, it might also be the case for S5K83A... | I also found (with help from Arnaud) that both sensors share the same page map register 0xec, so there is not only 256, but virtually 16*256 registers to try ;-) | | Greg | | Direct link to the changeset for that revision: http://m560x-driver.svn.sourceforge.net/viewvc/m560x-driver/m560x/branches/m5602/m5602_s5k4aa.c?r1=315&r2=322&pathrev=322 | | ----- Message d'origine ---- | De : Erik Andrén <[EMAIL PROTECTED]> | À : Lukáš Karas <[EMAIL PROTECTED]> | Cc : m560x-driver-devel <[email protected]> | Envoyé le : Dimanche, 7 Septembre 2008, 15h51mn 20s | Objet : Re: [M560x-driver-devel] m5602/s5k83a brightness, whiteness and gain control | | Hi, | and thanks for the patch! | | I've added some more comments below. | | ~ Lukáa Karas wrote: | | Ok, thanks for comments. | | New patch is in attachment (patch comparet to branche m5602, revision | 372). | | | | ______________________________________________________________ | |> Od: [EMAIL PROTECTED] | |> Komu: Lukáa Karas <[EMAIL PROTECTED]> | |> CC: m560x-driver-devel <[email protected]> | |> Datum: 06.09.2008 17:42 | |> PYedmt: Re: [M560x-driver-devel] m5602/s5k83a brightness,whiteness | and gain control | |> | | | | | | ~ 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. | | I hardly belive they've spread out the different bits in an illogical | way. There must be some systems. We need to figure out how to read the | correct registers and interpret the results in a deterministic and | consistent way, otherwise having these controls is pointless. | | | | |> 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. | | Thanks | | | | ~ 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. | | This is interesting, I wonder why? Could there be some kind of sync-signal? | | | | | + | | + 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 | | Expose all possible resolution, do not limit it in the driver. | | | | | + 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. | | What is the dependency between the registers? Could one be some kind of | multiplier? | | | | |> Can you see video with this? | | Don't understand what you mean here. | | | | | + 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 | | Again, nice improvements. | | Thanks, | Erik | | | | | | | | | | | | | | | | | | ------------------------------------------------------------------------- | | | 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 - ------------------------------------------------------------------------ - ------------------------------------------------------------------------- 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) iD8DBQFIw/mhN7qBt+4UG0ERAvGwAJ91erAawQGRngarkHSXdiDUxGccEACgoBSL U2SkGUKSaSDAosmiTPM5c+Y= =g15R -----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
