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>

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 00f8da6..43ca6ea 100644 (file)
--- 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 supported_modes[] = {
                        .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 supported_modes[] = {
                        .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 supported_modes[] = {
                        .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 supported_modes[] = {
                        .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 imx219 *imx219, u32 code)
        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_ctrl *ctrl)
        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_ctrl *ctrl)
                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_ctrl *ctrl)
                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 v4l2_subdev *sd,
        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_subdev *sd,
                        __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 imx219 *imx219,
 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 imx219 *imx219)
                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,