From 011166dc482fbedac908149d6fe6ece90711212b Mon Sep 17 00:00:00 2001 From: Vinay Varma Date: Fri, 22 Sep 2023 18:17:42 +0100 Subject: [PATCH] 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 Reworked due to upstream changes Signed-off-by: Dave Stevenson --- drivers/media/i2c/imx219.c | 144 +++++++++++++++++++++++++++++++++------------ 1 file changed, 105 insertions(+), 39 deletions(-) diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 00f8da6..43ca6ea 100644 --- 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, -- 2.7.4