From 24e8a7d7181be45e0d67335f05bf0773fe6ad261 Mon Sep 17 00:00:00 2001
From: Vinay Varma <varmavinaym@gmail.com>
Date: Fri, 22 Sep 2023 18:17:42 +0100
Subject: [PATCH 0502/1085] media: i2c: imx219: fix binning and rate_factor for
 480p and 1232p

At a high FPS with RAW10, there is frame corruption for 480p because the
rate_factor of 2 is used with the normal 2x2 bining [1]. This commit
ties the rate_factor to the selected binning mode. For the 480p mode,
analog 2x2 binning mode with a rate_factor of 2 is always used. For the
1232p mode the normal 2x2 binning mode is used for RAW10 while analog
2x2 binning mode is used for RAW8.

[1] raspberrypi#5493

Signed-off-by: Vinay Varma <varmavinaym@gmail.com>
Reworked due to upstream changes
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/imx219.c | 144 +++++++++++++++++++++++++++----------
 1 file changed, 105 insertions(+), 39 deletions(-)

--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -104,8 +104,8 @@
 /* Binning  Mode */
 #define IMX219_REG_BINNING_MODE		CCI_REG16(0x0174)
 #define IMX219_BINNING_NONE		0x0000
-#define IMX219_BINNING_2X2		0x0101
-#define IMX219_BINNING_2X2_ANALOG	0x0303
+#define IMX219_BINNING_2X2_NORMAL	0x0101
+#define IMX219_BINNING_2X2_SPECIAL	0x0303
 
 #define IMX219_REG_CSI_DATA_FORMAT_A	CCI_REG16(0x018c)
 
@@ -171,6 +171,18 @@ enum pad_types {
 	NUM_PADS
 };
 
+enum binning_mode {
+	BINNING_NONE,
+	BINNING_NORMAL_2x2,
+	BINNING_SPECIAL_2x2,
+};
+
+enum binning_bit_depths {
+	BINNING_IDX_8_BIT,
+	BINNING_IDX_10_BIT,
+	BINNING_IDX_MAX
+};
+
 struct imx219_reg {
 	u16 address;
 	u8 val;
@@ -197,11 +209,8 @@ struct imx219_mode {
 	/* Default register values */
 	struct imx219_reg_list reg_list;
 
-	/* 2x2 binning is used */
-	bool binning;
-
-	/* Relative pixel clock rate factor for the mode. */
-	unsigned int rate_factor;
+	/* binning mode based on format code */
+	enum binning_mode binning[BINNING_IDX_MAX];
 };
 
 static const struct cci_reg_sequence imx219_common_regs[] = {
@@ -406,8 +415,10 @@ static const struct imx219_mode supporte
 			.num_of_regs = ARRAY_SIZE(mode_3280x2464_regs),
 			.regs = mode_3280x2464_regs,
 		},
-		.binning = false,
-		.rate_factor = 1,
+		.binning = {
+			[BINNING_IDX_8_BIT] = BINNING_NONE,
+			[BINNING_IDX_10_BIT] = BINNING_NONE,
+		},
 	},
 	{
 		/* 1080P 30fps cropped */
@@ -424,8 +435,10 @@ static const struct imx219_mode supporte
 			.num_of_regs = ARRAY_SIZE(mode_1920_1080_regs),
 			.regs = mode_1920_1080_regs,
 		},
-		.binning = false,
-		.rate_factor = 1,
+		.binning = {
+			[BINNING_IDX_8_BIT] = BINNING_NONE,
+			[BINNING_IDX_10_BIT] = BINNING_NONE,
+		},
 	},
 	{
 		/* 2x2 binned 30fps mode */
@@ -442,8 +455,10 @@ static const struct imx219_mode supporte
 			.num_of_regs = ARRAY_SIZE(mode_1640_1232_regs),
 			.regs = mode_1640_1232_regs,
 		},
-		.binning = true,
-		.rate_factor = 1,
+		.binning = {
+			[BINNING_IDX_8_BIT] = BINNING_SPECIAL_2x2,
+			[BINNING_IDX_10_BIT] = BINNING_NORMAL_2x2,
+		},
 	},
 	{
 		/* 640x480 30fps mode */
@@ -460,12 +475,10 @@ static const struct imx219_mode supporte
 			.num_of_regs = ARRAY_SIZE(mode_640_480_regs),
 			.regs = mode_640_480_regs,
 		},
-		.binning = true,
-		/*
-		 * This mode uses a special 2x2 binning that doubles the
-		 * internal pixel clock rate.
-		 */
-		.rate_factor = 2,
+		.binning = {
+			[BINNING_IDX_8_BIT] = BINNING_SPECIAL_2x2,
+			[BINNING_IDX_10_BIT] = BINNING_SPECIAL_2x2,
+		},
 	},
 };
 
@@ -523,12 +536,64 @@ static u32 imx219_get_format_code(struct
 	return imx219_mbus_formats[i];
 }
 
+static int imx219_resolve_binning(struct imx219 *imx219,
+				  const struct v4l2_mbus_framefmt *format,
+				  enum binning_mode *binning)
+{
+	u32 fmt;
+
+	if (format)
+		fmt = format->code;
+	else
+		fmt = MEDIA_BUS_FMT_SRGGB10_1X10;
+
+	switch (fmt) {
+	case MEDIA_BUS_FMT_SRGGB8_1X8:
+	case MEDIA_BUS_FMT_SGRBG8_1X8:
+	case MEDIA_BUS_FMT_SGBRG8_1X8:
+	case MEDIA_BUS_FMT_SBGGR8_1X8:
+		*binning = imx219->mode->binning[BINNING_IDX_8_BIT];
+		return 0;
+
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+	case MEDIA_BUS_FMT_SGRBG10_1X10:
+	case MEDIA_BUS_FMT_SGBRG10_1X10:
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+		*binning = imx219->mode->binning[BINNING_IDX_10_BIT];
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static int imx219_get_rate_factor(struct imx219 *imx219,
+				  const struct v4l2_mbus_framefmt *format)
+{
+	enum binning_mode binning = BINNING_NONE;
+	int ret;
+
+	ret = imx219_resolve_binning(imx219, format, &binning);
+	if (ret < 0)
+		return ret;
+
+	switch (binning) {
+	case BINNING_NONE:
+	case BINNING_NORMAL_2x2:
+		return 1;
+	case BINNING_SPECIAL_2x2:
+		return 2;
+	}
+	return -EINVAL;
+}
+
 static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct imx219 *imx219 =
 		container_of(ctrl->handler, struct imx219, ctrl_handler);
 	struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
 	int ret = 0;
+	const struct v4l2_mbus_framefmt *format;
+	struct v4l2_subdev_state *state;
+	int rate_factor;
 
 	if (ctrl->id == V4L2_CID_VBLANK) {
 		int exposure_max, exposure_def;
@@ -550,6 +615,10 @@ static int imx219_set_ctrl(struct v4l2_c
 	if (pm_runtime_get_if_in_use(&client->dev) == 0)
 		return 0;
 
+	state = v4l2_subdev_get_locked_active_state(&imx219->sd);
+	format = v4l2_subdev_get_pad_format(&imx219->sd, state, 0);
+	rate_factor = imx219_get_rate_factor(imx219, format);
+
 	switch (ctrl->id) {
 	case V4L2_CID_ANALOGUE_GAIN:
 		cci_write(imx219->regmap, IMX219_REG_ANALOG_GAIN,
@@ -557,7 +626,7 @@ static int imx219_set_ctrl(struct v4l2_c
 		break;
 	case V4L2_CID_EXPOSURE:
 		cci_write(imx219->regmap, IMX219_REG_EXPOSURE,
-			  ctrl->val / imx219->mode->rate_factor, &ret);
+			  ctrl->val / rate_factor, &ret);
 		break;
 	case V4L2_CID_DIGITAL_GAIN:
 		cci_write(imx219->regmap, IMX219_REG_DIGITAL_GAIN,
@@ -574,7 +643,7 @@ static int imx219_set_ctrl(struct v4l2_c
 		break;
 	case V4L2_CID_VBLANK:
 		cci_write(imx219->regmap, IMX219_REG_VTS,
-			  (imx219->mode->height + ctrl->val) / imx219->mode->rate_factor, &ret);
+			  (imx219->mode->height + ctrl->val) / rate_factor, &ret);
 		break;
 	case V4L2_CID_HBLANK:
 		cci_write(imx219->regmap, IMX219_REG_HTS,
@@ -715,10 +784,11 @@ static int imx219_enum_frame_size(struct
 	return 0;
 }
 
-static unsigned long imx219_get_pixel_rate(struct imx219 *imx219)
+static unsigned long imx219_get_pixel_rate(struct imx219 *imx219,
+					   const struct v4l2_mbus_framefmt *format)
 {
 	return ((imx219->lanes == 2) ? IMX219_PIXEL_RATE :
-		IMX219_PIXEL_RATE_4LANE) * imx219->mode->rate_factor;
+		IMX219_PIXEL_RATE_4LANE) * imx219_get_rate_factor(imx219, format);
 }
 
 static int imx219_set_pad_format(struct v4l2_subdev *sd,
@@ -781,7 +851,7 @@ static int imx219_set_pad_format(struct
 			__v4l2_ctrl_s_ctrl(imx219->hblank, hblank);
 
 			/* Scale the pixel rate based on the mode specific factor */
-			pixel_rate = imx219_get_pixel_rate(imx219);
+			pixel_rate = imx219_get_pixel_rate(imx219, &fmt->format);
 			__v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate,
 						 pixel_rate, 1, pixel_rate);
 		}
@@ -819,24 +889,20 @@ static int imx219_set_framefmt(struct im
 static int imx219_set_binning(struct imx219 *imx219,
 			      const struct v4l2_mbus_framefmt *format)
 {
-	if (!imx219->mode->binning)
+	enum binning_mode binning = BINNING_NONE;
+
+	imx219_resolve_binning(imx219, format, &binning);
+
+	switch (binning) {
+	case BINNING_NONE:
 		return cci_write(imx219->regmap, IMX219_REG_BINNING_MODE,
 				 IMX219_BINNING_NONE, NULL);
-
-	switch (format->code) {
-	case MEDIA_BUS_FMT_SRGGB8_1X8:
-	case MEDIA_BUS_FMT_SGRBG8_1X8:
-	case MEDIA_BUS_FMT_SGBRG8_1X8:
-	case MEDIA_BUS_FMT_SBGGR8_1X8:
+	case BINNING_NORMAL_2x2:
 		return cci_write(imx219->regmap, IMX219_REG_BINNING_MODE,
-				 IMX219_BINNING_2X2_ANALOG, NULL);
-
-	case MEDIA_BUS_FMT_SRGGB10_1X10:
-	case MEDIA_BUS_FMT_SGRBG10_1X10:
-	case MEDIA_BUS_FMT_SGBRG10_1X10:
-	case MEDIA_BUS_FMT_SBGGR10_1X10:
+				 IMX219_BINNING_2X2_NORMAL, NULL);
+	case BINNING_SPECIAL_2x2:
 		return cci_write(imx219->regmap, IMX219_REG_BINNING_MODE,
-				 IMX219_BINNING_2X2, NULL);
+				 IMX219_BINNING_2X2_SPECIAL, NULL);
 	}
 
 	return -EINVAL;
@@ -1158,7 +1224,7 @@ static int imx219_init_controls(struct i
 		return ret;
 
 	/* By default, PIXEL_RATE is read only */
-	pixel_rate = imx219_get_pixel_rate(imx219);
+	pixel_rate = imx219_get_pixel_rate(imx219, NULL);
 	imx219->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
 					       V4L2_CID_PIXEL_RATE,
 					       pixel_rate, pixel_rate, 1,
