RE: [PATCH] media: ov08x40: Modify the tline calculation in 1C mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux