Thanks for the review, Bingbu and Sakari.
Because I have also ran the `sensor entry criteria` to make sure the driver behavior.
So it takes a delay for feedback.
I modified most ideas by previous comments.
However, we cannot take this part away.
> > > + /* frame length line need to be aligned */
> > > + vblank = (vblank & 0xfffc) + OV08X40_VTS_ALIGNMENT;
> >
> > I think there should be an existing macro can help you do that, then
> > you can remove `OV08X40_VTS_ALIGNMENT`.
>I'd change vblank limits so you don't need this check anymore.
About the alignment, it needs to be added or the flick will appear.
-----Original Message-----
From: Chen, Jason Z <jason.z.chen@xxxxxxxxx>
Sent: Thursday, September 14, 2023 2:01 PM
To: linux-media@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx; bingbu.cao@xxxxxxxxxxxxxxx
Cc: Yeh, Andy <andy.yeh@xxxxxxxxx>; Zhang, Qingwu <qingwu.zhang@xxxxxxxxx>; Chen, Jason Z <jason.z.chen@xxxxxxxxx>
Subject: [PATCH] media: ov08x40: Modify the tline calculation in 1C mode
From: Jason Chen <jason.z.chen@xxxxxxxxx>
The tline value need to be doubled in 1C mode.
To follow the vendor's update for 1C mode:
Tline time = 2 * HTS / SCLK
Exposure unit :
1 * HTS = 0.5 Tline
Signed-off-by: Jason Chen <jason.z.chen@xxxxxxxxx>
---
drivers/media/i2c/ov08x40.c | 55 ++++++++++++++++++++++++++++---------
1 file changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c index 637da4df690..fc7880f60f0 100644
--- a/drivers/media/i2c/ov08x40.c
+++ b/drivers/media/i2c/ov08x40.c
@@ -34,7 +34,7 @@
/* V_TIMING internal */
#define OV08X40_REG_VTS 0x380e
-#define OV08X40_VTS_30FPS 0x1388
+#define OV08X40_VTS_30FPS 0x09c4 /* the VTS need to be half in 1c */
#define OV08X40_VTS_BIN_30FPS 0x115c
#define OV08X40_VTS_MAX 0x7fff
@@ -44,7 +44,8 @@
/* Exposure control */
#define OV08X40_REG_EXPOSURE 0x3500
-#define OV08X40_EXPOSURE_MAX_MARGIN 31
+#define OV08X40_EXPOSURE_MAX_MARGIN 8
+#define OV08X40_EXPOSURE_BIN_MAX_MARGIN 2
#define OV08X40_EXPOSURE_MIN 1
#define OV08X40_EXPOSURE_STEP 1
#define OV08X40_EXPOSURE_DEFAULT 0x40
@@ -126,13 +127,17 @@ struct ov08x40_mode {
u32 vts_def;
u32 vts_min;
- /* HTS */
- u32 hts;
+ /* Line Length Pixels */
+ u32 llp;
/* Index of Link frequency config to be used */
u32 link_freq_index;
/* Default register values */
struct ov08x40_reg_list reg_list;
+
+ /* Exposure calculation */
+ int exposure_margin;
+ int exposure_shift;
};
static const struct ov08x40_reg mipi_data_rate_800mbps[] = { @@ -2354,7 +2359,7 @@ static const char * const ov08x40_test_pattern_menu[] = {
/* Configurations for supported link frequencies */
#define OV08X40_LINK_FREQ_400MHZ 400000000ULL
-
+#define OV08X40_SCLK_96MHZ 96000000ULL
#define OV08X40_EXT_CLK 19200000
#define OV08X40_DATA_LANES 4
@@ -2392,26 +2397,30 @@ static const struct ov08x40_mode supported_modes[] = {
.height = 2416,
.vts_def = OV08X40_VTS_30FPS,
.vts_min = OV08X40_VTS_30FPS,
- .hts = 640,
+ .llp = 0x10aa, /* in 1c mode, tline time = 2 * HTS / SCLK */
.lanes = 4,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_3856x2416_regs),
.regs = mode_3856x2416_regs,
},
.link_freq_index = OV08X40_LINK_FREQ_400MHZ_INDEX,
+ .exposure_shift = 1,
+ .exposure_margin = OV08X40_EXPOSURE_MAX_MARGIN,
},
{
.width = 1928,
.height = 1208,
.vts_def = OV08X40_VTS_BIN_30FPS,
.vts_min = OV08X40_VTS_BIN_30FPS,
- .hts = 720,
+ .llp = 0x960,
.lanes = 4,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_1928x1208_regs),
.regs = mode_1928x1208_regs,
},
.link_freq_index = OV08X40_LINK_FREQ_400MHZ_INDEX,
+ .exposure_shift = 0,
+ .exposure_margin = OV08X40_EXPOSURE_BIN_MAX_MARGIN,
},
};
@@ -2667,13 +2676,21 @@ static int ov08x40_set_ctrl(struct v4l2_ctrl *ctrl)
struct ov08x40, ctrl_handler);
struct i2c_client *client = v4l2_get_subdevdata(&ov08x->sd);
s64 max;
+ int exp, vblank;
int ret = 0;
/* Propagate change of current control to all related controls */
switch (ctrl->id) {
case V4L2_CID_VBLANK:
/* Update max exposure while meeting expected vblanking */
- max = ov08x->cur_mode->height + ctrl->val - OV08X40_EXPOSURE_MAX_MARGIN;
+ /* because in 1c mode, 1 HTS = 0.5 tline
+ * fps = sclk / hts / vts
+ * so the vts value needs to be double
+ */
+ max = ((ov08x->cur_mode->height + ctrl->val) <<
+ ov08x->cur_mode->exposure_shift) -
+ ov08x->cur_mode->exposure_margin;
+
__v4l2_ctrl_modify_range(ov08x->exposure,
ov08x->exposure->minimum,
max, ov08x->exposure->step, max); @@ -2697,15 +2714,23 @@ static int ov08x40_set_ctrl(struct v4l2_ctrl *ctrl)
ret = ov08x40_update_digital_gain(ov08x, ctrl->val);
break;
case V4L2_CID_EXPOSURE:
+ exp = (ctrl->val << ov08x->cur_mode->exposure_shift) -
+ ov08x->cur_mode->exposure_margin;
+
ret = ov08x40_write_reg(ov08x, OV08X40_REG_EXPOSURE,
OV08X40_REG_VALUE_24BIT,
- ctrl->val);
+ exp);
break;
case V4L2_CID_VBLANK:
+ vblank = ((ov08x->cur_mode->height + ctrl->val) <<
+ ov08x->cur_mode->exposure_shift);
+ /* frame length line should be aligned arccording
+ * to the value provided by the sensor vendor
+ */
+ vblank = (vblank & 0xfffc) + 4;
ret = ov08x40_write_reg(ov08x, OV08X40_REG_VTS,
OV08X40_REG_VALUE_16BIT,
- ov08x->cur_mode->height
- + ctrl->val);
+ vblank);
break;
case V4L2_CID_TEST_PATTERN:
ret = ov08x40_enable_test_pattern(ov08x, ctrl->val); @@ -2849,7 +2874,9 @@ ov08x40_set_pad_format(struct v4l2_subdev *sd,
1,
vblank_def);
__v4l2_ctrl_s_ctrl(ov08x->vblank, vblank_def);
- h_blank = ov08x->cur_mode->hts;
+
+ h_blank = ov08x->cur_mode->llp - ov08x->cur_mode->width;
+
__v4l2_ctrl_modify_range(ov08x->hblank, h_blank,
h_blank, 1, h_blank);
}
@@ -3075,7 +3102,8 @@ static int ov08x40_init_controls(struct ov08x40 *ov08x)
OV08X40_VTS_MAX - mode->height, 1,
vblank_def);
- hblank = ov08x->cur_mode->hts;
+ hblank = ov08x->cur_mode->llp - ov08x->cur_mode->width;
+
ov08x->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov08x40_ctrl_ops,
V4L2_CID_HBLANK,
hblank, hblank, 1, hblank);
@@ -3320,6 +3348,7 @@ static struct i2c_driver ov08x40_i2c_driver = { module_i2c_driver(ov08x40_i2c_driver);
MODULE_AUTHOR("Jason Chen <jason.z.chen@xxxxxxxxx>");
+MODULE_AUTHOR("Qingwu Zhang <qingwu.zhang@xxxxxxxxx>");
MODULE_AUTHOR("Shawn Tu");
MODULE_DESCRIPTION("OmniVision OV08X40 sensor driver"); MODULE_LICENSE("GPL");
--
2.34.1
