[PATCH] media: i2c: imx219: Fix binning for RAW8 capture

[PATCH] media: i2c: imx219: Fix binning for RAW8 capture

[Jai Luthra]

2x2 binning worked fine for RAW10 capture, but for RAW8 capture it would
lead to corrupted frames [1][2] when using 1232p for capture.

Using the special 2x2 analog binning mode fixes RAW8 1232p capture, but
breaks RAW10 1232p capture. So now we choose the binning mode depending
upon the frame format selected.

As 480p RAW8/RAW10 capture works fine with both binning modes, it can
share the same code path as 1232p for selecting binning mode.

[1] https://forums.raspberrypi.com/viewtopic.php?t=332103
[2] https://github.com/raspberrypi/libcamera-apps/issues/281

Signed-off-by: Jai Luthra <j-luthra@ti.com>
---
 drivers/media/i2c/imx219.c | 58 ++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 77bd79a5954e..a7afd914214e 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -89,6 +89,12 @@
 
 #define IMX219_REG_ORIENTATION		0x0172
 
+/* Binning  Mode */
+#define IMX219_REG_BINNING_MODE		0x0174
+#define IMX219_BINNING_NONE		0x0000
+#define IMX219_BINNING_2X2		0x0101
+#define IMX219_BINNING_2X2_ANALOG	0x0303
+
 /* Test Pattern Control */
 #define IMX219_REG_TEST_PATTERN		0x0600
 #define IMX219_TEST_PATTERN_DISABLE	0
@@ -143,6 +149,9 @@ struct imx219_mode {
 
 	/* Default register values */
 	struct imx219_reg_list reg_list;
+
+	/* 2x2 binning is used */
+	bool binning;
 };
 
 /*
@@ -176,8 +185,6 @@ static const struct imx219_reg mode_3280x2464_regs[] = {
 	{0x016f, 0xa0},
 	{0x0170, 0x01},
 	{0x0171, 0x01},
-	{0x0174, 0x00},
-	{0x0175, 0x00},
 	{0x0301, 0x05},
 	{0x0303, 0x01},
 	{0x0304, 0x03},
@@ -235,8 +242,6 @@ static const struct imx219_reg mode_1920_1080_regs[] = {
 	{0x016f, 0x38},
 	{0x0170, 0x01},
 	{0x0171, 0x01},
-	{0x0174, 0x00},
-	{0x0175, 0x00},
 	{0x0301, 0x05},
 	{0x0303, 0x01},
 	{0x0304, 0x03},
@@ -290,8 +295,6 @@ static const struct imx219_reg mode_1640_1232_regs[] = {
 	{0x016f, 0xd0},
 	{0x0170, 0x01},
 	{0x0171, 0x01},
-	{0x0174, 0x01},
-	{0x0175, 0x01},
 	{0x0301, 0x05},
 	{0x0303, 0x01},
 	{0x0304, 0x03},
@@ -349,8 +352,6 @@ static const struct imx219_reg mode_640_480_regs[] = {
 	{0x016f, 0xe0},
 	{0x0170, 0x01},
 	{0x0171, 0x01},
-	{0x0174, 0x03},
-	{0x0175, 0x03},
 	{0x0301, 0x05},
 	{0x0303, 0x01},
 	{0x0304, 0x03},
@@ -485,6 +486,7 @@ static const struct imx219_mode supported_modes[] = {
 			.num_of_regs = ARRAY_SIZE(mode_3280x2464_regs),
 			.regs = mode_3280x2464_regs,
 		},
+		.binning = false,
 	},
 	{
 		/* 1080P 30fps cropped */
@@ -501,6 +503,7 @@ static const struct imx219_mode supported_modes[] = {
 			.num_of_regs = ARRAY_SIZE(mode_1920_1080_regs),
 			.regs = mode_1920_1080_regs,
 		},
+		.binning = false,
 	},
 	{
 		/* 2x2 binned 30fps mode */
@@ -517,6 +520,7 @@ static const struct imx219_mode supported_modes[] = {
 			.num_of_regs = ARRAY_SIZE(mode_1640_1232_regs),
 			.regs = mode_1640_1232_regs,
 		},
+		.binning = true,
 	},
 	{
 		/* 640x480 30fps mode */
@@ -533,6 +537,7 @@ static const struct imx219_mode supported_modes[] = {
 			.num_of_regs = ARRAY_SIZE(mode_640_480_regs),
 			.regs = mode_640_480_regs,
 		},
+		.binning = true,
 	},
 };
 
@@ -979,6 +984,35 @@ static int imx219_set_framefmt(struct imx219 *imx219)
 	return -EINVAL;
 }
 
+static int imx219_set_binning(struct imx219 *imx219)
+{
+	if (!imx219->mode->binning) {
+		return imx219_write_reg(imx219, IMX219_REG_BINNING_MODE,
+					IMX219_REG_VALUE_16BIT,
+					IMX219_BINNING_NONE);
+	}
+
+	switch (imx219->fmt.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:
+		return imx219_write_reg(imx219, IMX219_REG_BINNING_MODE,
+					IMX219_REG_VALUE_16BIT,
+					IMX219_BINNING_2X2_ANALOG);
+
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+	case MEDIA_BUS_FMT_SGRBG10_1X10:
+	case MEDIA_BUS_FMT_SGBRG10_1X10:
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+		return imx219_write_reg(imx219, IMX219_REG_BINNING_MODE,
+					IMX219_REG_VALUE_16BIT,
+					IMX219_BINNING_2X2);
+	}
+
+	return -EINVAL;
+}
+
 static const struct v4l2_rect *
 __imx219_get_pad_crop(struct imx219 *imx219,
 		      struct v4l2_subdev_state *sd_state,
@@ -1056,6 +1090,14 @@ static int imx219_start_streaming(struct imx219 *imx219)
 		goto err_rpm_put;
 	}
 
+	ret = imx219_set_binning(imx219);
+	if (ret) {
+		dev_err(&client->dev, "%s failed to set binning: %d\n",
+			__func__, ret);
+		goto err_rpm_put;
+	}
+
+
 	/* Apply customized values from user */
 	ret =  __v4l2_ctrl_handler_setup(imx219->sd.ctrl_handler);
 	if (ret)
-- 
2.17.1