diff mbox series

[v7,4/5] media: i2c: imx219: Increase minimum LLP for binned modes

Message ID 20250217-imx219_fixes-v7-4-83c3c63b737e@ideasonboard.com (mailing list archive)
State New
Headers show
Series media: i2c: imx219: Fixes for blanking, pixel rate and binning | expand

Commit Message

Jai Luthra Feb. 17, 2025, 1:27 p.m. UTC 
The sensor's internal ADC supports a minimum line length of 3448 pixels,
which may be too small to use with analog binning, where ADC operates on
two lines together. Switch to a higher minimum line length of 3560
pixels to fix the blocky artefacts seen with analog binning.

To keep the same default framerate as before, lower the default fll value
for the binned modes to compensate for the increase in llp.

Link: https://github.com/raspberrypi/rpicam-apps/issues/281#issuecomment-1082894118
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
---
 drivers/media/i2c/imx219.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 8239e7ea8ec03849b339c1f314485266d4c4d8bd..418f88386659d494ff674d64ed69b8441d1ee2cd 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -75,6 +75,7 @@ 
 #define IMX219_VBLANK_MIN		32
 #define IMX219_REG_LINE_LENGTH_A	CCI_REG16(0x0162)
 #define IMX219_LLP_MIN			0x0d78
+#define IMX219_BINNED_LLP_MIN		0x0de8
 #define IMX219_LLP_MAX			0x7ff0
 
 #define IMX219_REG_X_ADD_STA_A		CCI_REG16(0x0164)
@@ -298,13 +299,13 @@  static const struct imx219_mode supported_modes[] = {
 		/* 2x2 binned 30fps mode */
 		.width = 1640,
 		.height = 1232,
-		.fll_def = 1763,
+		.fll_def = 1707,
 	},
 	{
 		/* 640x480 30fps mode */
 		.width = 640,
 		.height = 480,
-		.fll_def = 1763,
+		.fll_def = 1707,
 	},
 };
 
@@ -845,7 +846,7 @@  static int imx219_set_pad_format(struct v4l2_subdev *sd,
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
 		int exposure_max;
 		int exposure_def;
-		int hblank;
+		int hblank, llp_min;
 
 		/* Update limits and set FPS to default */
 		__v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
@@ -861,6 +862,17 @@  static int imx219_set_pad_format(struct v4l2_subdev *sd,
 					 imx219->exposure->minimum,
 					 exposure_max, imx219->exposure->step,
 					 exposure_def);
+
+		/*
+		 * With analog binning the default minimum line length of 3448
+		 * can cause artefacts because the ADC operates on two lines
+		 * together. Switch to higher minimum of 3560 if we are binning.
+		 */
+		llp_min = (bin_h || bin_v) ? IMX219_BINNED_LLP_MIN :
+					     IMX219_LLP_MIN;
+		__v4l2_ctrl_modify_range(imx219->hblank, llp_min - mode->width,
+					 IMX219_LLP_MAX - mode->width, 1,
+					 llp_min - mode->width);
 		/*
 		 * Retain PPL setting from previous mode so that the
 		 * line time does not change on a mode change.
@@ -869,10 +881,6 @@  static int imx219_set_pad_format(struct v4l2_subdev *sd,
 		 * mode width subtracted.
 		 */
 		hblank = prev_line_len - mode->width;
-		__v4l2_ctrl_modify_range(imx219->hblank,
-					 IMX219_LLP_MIN - mode->width,
-					 IMX219_LLP_MAX - mode->width, 1,
-					 IMX219_LLP_MIN - mode->width);
 		__v4l2_ctrl_s_ctrl(imx219->hblank, hblank);
 	}