Skip to content

media: i2c: imx477: Correct minimum exposure lines #5050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

davidplowman
Copy link
Contributor

The minimum number of exposure lines value (IMX477_EXPOSURE_MIN) was
previously 20 but this is not correct. The datasheet is not completely
explicit, however the new value of 4 has been tested with all the
sensor modes supported by this driver, and matches the lowest exposure
value of 114us that could be achieved wtih Raspberry Pi's legacy
firmware driver.

Signed-off-by: David Plowman [email protected]

The minimum number of exposure lines value (IMX477_EXPOSURE_MIN) was
previously 20 but this is not correct. The datasheet is not completely
explicit, however the new value of 4 has been tested with all the
sensor modes supported by this driver, and matches the lowest exposure
value of 114us that could be achieved wtih Raspberry Pi's legacy
firmware driver.

Signed-off-by: David Plowman <[email protected]>
@davidplowman
Copy link
Contributor Author

@naushir @6by9

@naushir
Copy link
Contributor

naushir commented May 30, 2022

LGTM

@pelwell pelwell merged commit 999f4a1 into raspberrypi:rpi-5.15.y Jun 6, 2022
@@ -61,7 +61,7 @@ MODULE_PARM_DESC(trigger_mode, "Set vsync trigger mode: 1=source, 2=sink");
/* Exposure control */
#define IMX477_REG_EXPOSURE 0x0202
#define IMX477_EXPOSURE_OFFSET 22
#define IMX477_EXPOSURE_MIN 20
#define IMX477_EXPOSURE_MIN 4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table 5-24 Integration time setting (In case of FRM_LENGHT_CTL = 0h) appears to be fairly explicit that a COARSE_INTEG_TIME of > FRM_LENGTH_LINES - 20 is prohibited.

Also description of register 0350h FRM_LENGTH_CTL also states that it controls whether the frame length is changed when FRM_LENGTH_LINES < COARSE_INTEG_TIME + alpha, where alpha = 20 for this sensor.

Or am I getting confused here with the value that should be set in EXPOSURE_OFFSET for max exposure time? That appears that it should be 20 instead of 22.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree things are not explained in the clearest way and there is some interpretation involved!

I took these various statements to be a restriction on FRM_LENGTH_LINES, but not actually implying anything about the min value of COARSE_INTEG_TIME. Indeed, in our old firmware driver it goes lower than 20 by some way (giving 114us min exposure rather than 571us).

I thought EXPOSURE_OFFSET being 22 rather than 20 was maybe a question of tests including the case of equality or not, but honestly I don't know!

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Jun 6, 2022
See: raspberrypi/linux#5041

kernel: media: i2c: imx477: Correct minimum exposure lines
See: raspberrypi/linux#5050

kernel: arm64: Initialize jump labels before setup_machine_fdt()
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Jun 6, 2022
See: raspberrypi/linux#5041

kernel: media: i2c: imx477: Correct minimum exposure lines
See: raspberrypi/linux#5050

kernel: arm64: Initialize jump labels before setup_machine_fdt()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants