From bf21a5259c435d41b9b098cfc8775b043fa5beeb Mon Sep 17 00:00:00 2001 From: Naushir Patuck Date: Wed, 24 Sep 2025 13:11:19 +0100 Subject: [PATCH 1/2] drivers: media: imx219: Remove enum binning_mode The enum binning_mode is redundant, it only uses values from the earlier defined binning modes. Remove it. Signed-off-by: Naushir Patuck --- drivers/media/i2c/imx219.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 83de9d59b5c5c1..f9e440e27cd55d 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -148,12 +148,6 @@ #define IMX219_PIXEL_ARRAY_WIDTH 3280U #define IMX219_PIXEL_ARRAY_HEIGHT 2464U -enum binning_mode { - BINNING_NONE = IMX219_BINNING_NONE, - BINNING_X2 = IMX219_BINNING_X2, - BINNING_ANALOG_X2 = IMX219_BINNING_X2_ANALOG, -}; - /* Mode : resolution and related config&values */ struct imx219_mode { /* Frame width */ @@ -409,8 +403,8 @@ static u32 imx219_get_format_bpp(const struct v4l2_mbus_framefmt *format) } } -static enum binning_mode imx219_get_binning(struct imx219 *imx219, u8 *bin_h, - u8 *bin_v) +static unsigned int imx219_get_binning(struct imx219 *imx219, u8 *bin_h, + u8 *bin_v) { struct v4l2_subdev_state *state = v4l2_subdev_get_locked_active_state(&imx219->sd); @@ -422,23 +416,23 @@ static enum binning_mode imx219_get_binning(struct imx219 *imx219, u8 *bin_h, *bin_v = crop->height / format->height; if (*bin_h == 2 && *bin_v == 2) - return BINNING_ANALOG_X2; + return IMX219_BINNING_X2_ANALOG; else if (*bin_h == 2 || *bin_v == 2) /* * Don't use analog binning if only one dimension * is binned, as it crops the other dimension */ - return BINNING_X2; + return IMX219_BINNING_X2; else - return BINNING_NONE; + return IMX219_BINNING_NONE; } static inline u32 imx219_get_rate_factor(struct imx219 *imx219) { u8 bin_h, bin_v; - enum binning_mode binning = imx219_get_binning(imx219, &bin_h, &bin_v); + unsigned int binning = imx219_get_binning(imx219, &bin_h, &bin_v); - if (binning == BINNING_ANALOG_X2) + if (binning == IMX219_BINNING_X2_ANALOG) return 2; return 1; @@ -674,7 +668,7 @@ static int imx219_set_framefmt(struct imx219 *imx219, { const struct v4l2_mbus_framefmt *format; const struct v4l2_rect *crop; - enum binning_mode binning; + unsigned int binning; u8 bin_h, bin_v; u32 bpp; int ret = 0; @@ -694,9 +688,9 @@ static int imx219_set_framefmt(struct imx219 *imx219, binning = imx219_get_binning(imx219, &bin_h, &bin_v); cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H, - (bin_h == 2) ? binning : BINNING_NONE, &ret); + (bin_h == 2) ? binning : IMX219_BINNING_NONE, &ret); cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_V, - (bin_v == 2) ? binning : BINNING_NONE, &ret); + (bin_v == 2) ? binning : IMX219_BINNING_NONE, &ret); cci_write(imx219->regmap, IMX219_REG_X_OUTPUT_SIZE, format->width, &ret); From df2d768ca7ac8d1d6272a766049b73b4254dd7f5 Mon Sep 17 00:00:00 2001 From: Vinay Varma Date: Fri, 22 Sep 2023 18:17:42 +0100 Subject: [PATCH 2/2] 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 Signed-off-by: Dave Stevenson Signed-off-by: Naushir Patuck Reworked due to upstream changes --- drivers/media/i2c/imx219.c | 48 +++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index f9e440e27cd55d..8e74181d4eb222 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -148,6 +148,12 @@ #define IMX219_PIXEL_ARRAY_WIDTH 3280U #define IMX219_PIXEL_ARRAY_HEIGHT 2464U +enum binning_bit_depths { + BINNING_IDX_8_BIT, + BINNING_IDX_10_BIT, + BINNING_IDX_MAX +}; + /* Mode : resolution and related config&values */ struct imx219_mode { /* Frame width */ @@ -157,6 +163,9 @@ struct imx219_mode { /* V-timing */ unsigned int vts_def; + + /* binning mode based on format code */ + unsigned int binning[BINNING_IDX_MAX]; }; static const struct cci_reg_sequence imx219_common_regs[] = { @@ -316,24 +325,40 @@ static const struct imx219_mode supported_modes[] = { .width = 3280, .height = 2464, .vts_def = 3526, + .binning = { + [BINNING_IDX_8_BIT] = IMX219_BINNING_NONE, + [BINNING_IDX_10_BIT] = IMX219_BINNING_NONE, + }, }, { /* 1080P 30fps cropped */ .width = 1920, .height = 1080, .vts_def = 1763, + .binning = { + [BINNING_IDX_8_BIT] = IMX219_BINNING_NONE, + [BINNING_IDX_10_BIT] = IMX219_BINNING_NONE, + }, }, { /* 2x2 binned 60fps mode */ .width = 1640, .height = 1232, .vts_def = 1763, + .binning = { + [BINNING_IDX_8_BIT] = IMX219_BINNING_X2_ANALOG, + [BINNING_IDX_10_BIT] = IMX219_BINNING_X2, + }, }, { /* 640x480 60fps mode */ .width = 640, .height = 480, .vts_def = 1763, + .binning = { + [BINNING_IDX_8_BIT] = IMX219_BINNING_X2_ANALOG, + [BINNING_IDX_10_BIT] = IMX219_BINNING_X2_ANALOG, + }, }, }; @@ -411,12 +436,33 @@ static unsigned int imx219_get_binning(struct imx219 *imx219, u8 *bin_h, const struct v4l2_mbus_framefmt *format = v4l2_subdev_state_get_format(state, 0); const struct v4l2_rect *crop = v4l2_subdev_state_get_crop(state, 0); + unsigned int bin_mode = IMX219_BINNING_NONE; + const struct imx219_mode *mode = + v4l2_find_nearest_size(supported_modes, + ARRAY_SIZE(supported_modes), + width, height, + format->width, format->height); + 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: + bin_mode = mode->binning[BINNING_IDX_8_BIT]; + break; + + case MEDIA_BUS_FMT_SRGGB10_1X10: + case MEDIA_BUS_FMT_SGRBG10_1X10: + case MEDIA_BUS_FMT_SGBRG10_1X10: + case MEDIA_BUS_FMT_SBGGR10_1X10: + bin_mode = mode->binning[BINNING_IDX_10_BIT]; + break; + } *bin_h = crop->width / format->width; *bin_v = crop->height / format->height; if (*bin_h == 2 && *bin_v == 2) - return IMX219_BINNING_X2_ANALOG; + return bin_mode; else if (*bin_h == 2 || *bin_v == 2) /* * Don't use analog binning if only one dimension