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
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion drivers/media/i2c/imx477.c
Original file line number Diff line number Diff line change
Expand Up @@ -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!

#define IMX477_EXPOSURE_STEP 1
#define IMX477_EXPOSURE_DEFAULT 0x640
#define IMX477_EXPOSURE_MAX (IMX477_FRAME_LENGTH_MAX - \
Expand Down