From 921d99bac5610dbef5ee46e6b9c728feff33b970 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Wed, 30 Oct 2024 17:34:21 +0100 Subject: [PATCH 01/25] media: i2c: imx283: Report correct V4L2_SEL_TGT_CROP The target crop rectangle is initialized with the crop of the default sensor mode. This is incorrect when a different sensor mode gets selected. Fix that by updating the crop rectangle when changing the sensor mode. Signed-off-by: Stefan Klug --- drivers/media/i2c/imx283.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index da618c8cbadc3b..bcc6f0260ddf15 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -956,6 +956,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_format *fmt) { + struct v4l2_rect *crop; struct v4l2_mbus_framefmt *format; const struct imx283_mode *mode; struct imx283 *imx283 = to_imx283(sd); @@ -982,6 +983,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, *format = fmt->format; + crop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD); + *crop = mode->crop; + return 0; } From 70aaca0d91872de84040331767ba1d51d0ddd126 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Tue, 11 Feb 2025 15:13:06 +0100 Subject: [PATCH 02/25] media: i2c: imx283: Fix handling of unsupported mbus codes When the code requested by imx283_set_pad_format() is not supported, a kernel exception occurs due to dereferencing the mode variable which is null. Fix that by correcting the code to a valid value before getting the mode table. While at it, remove the cases for the other unsupported codes in get_mode_table. Signed-off-by: Stefan Klug --- drivers/media/i2c/imx283.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index bcc6f0260ddf15..2e79ae609a12cd 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -576,23 +576,31 @@ static inline struct imx283 *to_imx283(struct v4l2_subdev *sd) return container_of_const(sd, struct imx283, sd); } +static inline int get_format_code(unsigned int code) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(imx283_mbus_codes); i++) + if (imx283_mbus_codes[i] == code) + break; + + if (i >= ARRAY_SIZE(imx283_mbus_codes)) + i = 0; + + return imx283_mbus_codes[i]; +} + static inline void get_mode_table(unsigned int code, const struct imx283_mode **mode_list, unsigned int *num_modes) { switch (code) { case MEDIA_BUS_FMT_SRGGB12_1X12: - case MEDIA_BUS_FMT_SGRBG12_1X12: - case MEDIA_BUS_FMT_SGBRG12_1X12: - case MEDIA_BUS_FMT_SBGGR12_1X12: *mode_list = supported_modes_12bit; *num_modes = ARRAY_SIZE(supported_modes_12bit); 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: *mode_list = supported_modes_10bit; *num_modes = ARRAY_SIZE(supported_modes_10bit); break; @@ -963,6 +971,8 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, const struct imx283_mode *mode_list; unsigned int num_modes; + fmt->format.code = get_format_code(fmt->format.code); + get_mode_table(fmt->format.code, &mode_list, &num_modes); mode = v4l2_find_nearest_size(mode_list, num_modes, width, height, @@ -1363,8 +1373,6 @@ static int imx283_init_controls(struct imx283 *imx283) imx283->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx283_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); - if (imx283->vflip) - imx283->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx283_ctrl_ops, V4L2_CID_TEST_PATTERN, From 77924c8a5b2ffb1592cad841aace9524813df1b9 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Thu, 18 Sep 2025 17:30:43 +0100 Subject: [PATCH 03/25] media: i2c: imx283: Move imx283_mode structure definition Move the struct imx283_mode further down in the compilation unit so that it can make reference of the scan out mode structures which are presently defined after. No functional change intended in this commit. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 118 ++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 2e79ae609a12cd..0915e3b6ef8953 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -195,65 +195,6 @@ struct imx283_reg_list { const struct cci_reg_sequence *regs; }; -/* Mode : resolution and related config values */ -struct imx283_mode { - unsigned int mode; - - /* Bits per pixel */ - unsigned int bpp; - - /* Frame width */ - unsigned int width; - - /* Frame height */ - unsigned int height; - - /* - * Minimum horizontal timing in pixel-units - * - * Note that HMAX is written in 72MHz units, and the datasheet assumes a - * 720MHz link frequency. Convert datasheet values with the following: - * - * For 12 bpp modes (480Mbps) convert with: - * hmax = [hmax in 72MHz units] * 480 / 72 - * - * For 10 bpp modes (576Mbps) convert with: - * hmax = [hmax in 72MHz units] * 576 / 72 - */ - u32 min_hmax; - - /* minimum V-timing in lines */ - u32 min_vmax; - - /* default H-timing */ - u32 default_hmax; - - /* default V-timing */ - u32 default_vmax; - - /* minimum SHR */ - u32 min_shr; - - /* - * Per-mode vertical crop constants used to calculate values - * of IMX283REG_WIDCUT and IMX283_REG_VWINPOS. - */ - u32 veff; - u32 vst; - u32 vct; - - /* Horizontal and vertical binning ratio */ - u8 hbin_ratio; - u8 vbin_ratio; - - /* Optical Blanking */ - u32 horizontal_ob; - u32 vertical_ob; - - /* Analog crop rectangle. */ - struct v4l2_rect crop; -}; - struct imx283_input_frequency { unsigned int mhz; unsigned int reg_count; @@ -352,6 +293,65 @@ static const struct imx283_readout_mode imx283_readout_modes[] = { */ }; +/* Mode : resolution and related config values */ +struct imx283_mode { + unsigned int mode; + + /* Bits per pixel */ + unsigned int bpp; + + /* Frame width */ + unsigned int width; + + /* Frame height */ + unsigned int height; + + /* + * Minimum horizontal timing in pixel-units + * + * Note that HMAX is written in 72MHz units, and the datasheet assumes a + * 720MHz link frequency. Convert datasheet values with the following: + * + * For 12 bpp modes (480Mbps) convert with: + * hmax = [hmax in 72MHz units] * 480 / 72 + * + * For 10 bpp modes (576Mbps) convert with: + * hmax = [hmax in 72MHz units] * 576 / 72 + */ + u32 min_hmax; + + /* minimum V-timing in lines */ + u32 min_vmax; + + /* default H-timing */ + u32 default_hmax; + + /* default V-timing */ + u32 default_vmax; + + /* minimum SHR */ + u32 min_shr; + + /* + * Per-mode vertical crop constants used to calculate values + * of IMX283REG_WIDCUT and IMX283_REG_VWINPOS. + */ + u32 veff; + u32 vst; + u32 vct; + + /* Horizontal and vertical binning ratio */ + u8 hbin_ratio; + u8 vbin_ratio; + + /* Optical Blanking */ + u32 horizontal_ob; + u32 vertical_ob; + + /* Analog crop rectangle. */ + struct v4l2_rect crop; +}; + static const struct cci_reg_sequence mipi_data_rate_1440Mbps[] = { /* The default register settings provide the 1440Mbps rate */ { CCI_REG8(0x36c5), 0x00 }, /* Undocumented */ From cfd8960b6a6122964e9f1137b32c556ce4843167 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Thu, 18 Sep 2025 17:28:32 +0100 Subject: [PATCH 04/25] media: i2c: imx283: Move scan out data to single data structure Move the common data structures to a new scanout table and allow v4l2 output modes to reference their scanout. This removes duplication from the mode definitions. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 90 ++++++++++++++++++++++++++------------ 1 file changed, 62 insertions(+), 28 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 0915e3b6ef8953..0120b1e75b751e 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -264,28 +264,63 @@ struct imx283_readout_mode { u8 mdsel4; }; -static const struct imx283_readout_mode imx283_readout_modes[] = { +struct imx283_scanout { + u8 bpp; + struct imx283_readout_mode readout; +}; + +static const struct imx283_scanout imx283_scan_modes[] = { /* All pixel scan modes */ - [IMX283_MODE_0] = { 0x04, 0x03, 0x10, 0x00 }, /* 12 bit */ - [IMX283_MODE_1] = { 0x04, 0x01, 0x00, 0x00 }, /* 10 bit */ - [IMX283_MODE_1A] = { 0x04, 0x01, 0x20, 0x50 }, /* 10 bit */ - [IMX283_MODE_1S] = { 0x04, 0x41, 0x20, 0x50 }, /* 10 bit */ + [IMX283_MODE_0] = { + .bpp = 12, + .readout = { 0x04, 0x03, 0x10, 0x00 }, + }, + [IMX283_MODE_1] = { + .bpp = 10, + .readout = { 0x04, 0x01, 0x00, 0x00 }, + }, + [IMX283_MODE_1A] = { + .bpp = 10, + .readout = { 0x04, 0x01, 0x20, 0x50 }, + }, + [IMX283_MODE_1S] = { + .bpp = 10, + .readout = { 0x04, 0x41, 0x20, 0x50 }, + }, /* Horizontal / Vertical 2/2-line binning */ - [IMX283_MODE_2] = { 0x0d, 0x11, 0x50, 0x00 }, /* 12 bit */ - [IMX283_MODE_2A] = { 0x0d, 0x11, 0x70, 0x50 }, /* 12 bit */ + [IMX283_MODE_2] = { + .bpp = 12, + .readout = { 0x0d, 0x11, 0x50, 0x00 }, + }, + [IMX283_MODE_2A] = { + .bpp = 12, + .readout = { 0x0d, 0x11, 0x70, 0x50 }, + }, /* Horizontal / Vertical 3/3-line binning */ - [IMX283_MODE_3] = { 0x1e, 0x18, 0x10, 0x00 }, /* 12 bit */ + [IMX283_MODE_3] = { + .bpp = 12, + .readout = { 0x1e, 0x18, 0x10, 0x00 }, + }, /* Vertical 2/9 subsampling, horizontal 3 binning cropping */ - [IMX283_MODE_4] = { 0x29, 0x18, 0x30, 0x50 }, /* 12 bit */ + [IMX283_MODE_4] = { + .bpp = 12, + .readout = { 0x29, 0x18, 0x30, 0x50 }, + }, /* Vertical 2/19 subsampling binning, horizontal 3 binning */ - [IMX283_MODE_5] = { 0x2d, 0x18, 0x10, 0x00 }, /* 12 bit */ + [IMX283_MODE_5] = { + .bpp = 12, + .readout = { 0x2d, 0x18, 0x10, 0x00 }, + }, /* Vertical 2 binning horizontal 2/4, subsampling 16:9 cropping */ - [IMX283_MODE_6] = { 0x18, 0x21, 0x00, 0x09 }, /* 10 bit */ + [IMX283_MODE_6] = { + .bpp = 10, + .readout = { 0x18, 0x21, 0x00, 0x09 }, + }, /* * New modes should make sure the offset period is complied. @@ -293,12 +328,14 @@ static const struct imx283_readout_mode imx283_readout_modes[] = { */ }; +static bool scan_mode(const struct imx283_scanout *scan, enum imx283_modes mode) +{ + return scan == &imx283_scan_modes[mode]; +} + /* Mode : resolution and related config values */ struct imx283_mode { - unsigned int mode; - - /* Bits per pixel */ - unsigned int bpp; + const struct imx283_scanout *scan; /* Frame width */ unsigned int width; @@ -410,8 +447,8 @@ static const struct imx283_reg_list link_freq_reglist[] = { static const struct imx283_mode supported_modes_12bit[] = { { /* 20MPix 21.40 fps readout mode 0 */ - .mode = IMX283_MODE_0, - .bpp = 12, + .scan = &imx283_scan_modes[IMX283_MODE_0], + .width = 5472, .height = 3648, .min_hmax = 5914, /* 887 @ 480MHz/72MHz */ @@ -442,8 +479,7 @@ static const struct imx283_mode supported_modes_12bit[] = { /* * Readout mode 2 : 2/2 binned mode (2736x1824) */ - .mode = IMX283_MODE_2, - .bpp = 12, + .scan = &imx283_scan_modes[IMX283_MODE_2], .width = 2736, .height = 1824, .min_hmax = 2414, /* Pixels (362 * 480MHz/72MHz + padding) */ @@ -475,8 +511,7 @@ static const struct imx283_mode supported_modes_12bit[] = { /* * Readout mode 3 : 3/3 binned mode (1824x1216) */ - .mode = IMX283_MODE_3, - .bpp = 12, + .scan = &imx283_scan_modes[IMX283_MODE_3], .width = 1824, .height = 1216, .min_hmax = 1894, /* Pixels (284 * 480MHz/72MHz + padding) */ @@ -509,8 +544,7 @@ static const struct imx283_mode supported_modes_12bit[] = { static const struct imx283_mode supported_modes_10bit[] = { { /* 20MPix 25.48 fps readout mode 1 */ - .mode = IMX283_MODE_1, - .bpp = 10, + .scan = &imx283_scan_modes[IMX283_MODE_1], .width = 5472, .height = 3648, .min_hmax = 5960, /* 745 @ 576MHz / 72MHz */ @@ -616,7 +650,7 @@ static u64 imx283_pixel_rate(struct imx283 *imx283, const struct imx283_mode *mode) { u64 link_frequency = link_frequencies[__ffs(imx283->link_freq_bitmap)]; - unsigned int bpp = mode->bpp; + unsigned int bpp = mode->scan->bpp; const unsigned int ddr = 2; /* Double Data Rate */ const unsigned int lanes = 4; /* Only 4 lane support */ u64 numerator = link_frequency * ddr * lanes; @@ -673,7 +707,7 @@ static u32 imx283_exposure(struct imx283 *imx283, u64 numerator; /* Number of clocks per internal offset period */ - offset = mode->mode == IMX283_MODE_0 ? 209 : 157; + offset = scan_mode(mode->scan, IMX283_MODE_0) ? 209 : 157; numerator = (imx283->vmax * (svr + 1) - shr) * imx283->hmax + offset; do_div(numerator, imx283->hmax); @@ -708,7 +742,7 @@ static u32 imx283_shr(struct imx283 *imx283, const struct imx283_mode *mode, u64 temp; /* Number of clocks per internal offset period */ - offset = mode->mode == IMX283_MODE_0 ? 209 : 157; + offset = scan_mode(mode->scan, IMX283_MODE_0) ? 209 : 157; temp = ((u64)exposure * imx283->hmax - offset); do_div(temp, imx283->hmax); @@ -1073,7 +1107,7 @@ static int imx283_start_streaming(struct imx283 *imx283, * Set the readout mode registers. * MDSEL3 and MDSEL4 are updated to enable Arbitrary Vertical Cropping. */ - readout = &imx283_readout_modes[mode->mode]; + readout = &mode->scan->readout; cci_write(imx283->cci, IMX283_REG_MDSEL1, readout->mdsel1, &ret); cci_write(imx283->cci, IMX283_REG_MDSEL2, readout->mdsel2, &ret); cci_write(imx283->cci, IMX283_REG_MDSEL3, @@ -1082,7 +1116,7 @@ static int imx283_start_streaming(struct imx283 *imx283, readout->mdsel4 | IMX283_MDSEL4_VCROP_EN, &ret); /* Mode 1S specific entries from the Readout Drive Mode Tables */ - if (mode->mode == IMX283_MODE_1S) { + if (scan_mode(mode->scan, IMX283_MODE_1S)) { cci_write(imx283->cci, IMX283_REG_MDSEL7, 0x01, &ret); cci_write(imx283->cci, IMX283_REG_MDSEL18, 0x1098, &ret); } From 8deb66f724eb96b0023c648b0a529f201e50616d Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Fri, 19 Sep 2025 12:21:43 +0100 Subject: [PATCH 05/25] media: i2c: imx283: Remove horizontal_ob The horizontal optical black values are not used as we do not enable HOB output - and instead directly use HTRIMMING to request the desired horizontal cropping position. Remove the unused reference to simplify. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 0120b1e75b751e..92f55a8d70367f 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -382,7 +382,6 @@ struct imx283_mode { u8 vbin_ratio; /* Optical Blanking */ - u32 horizontal_ob; u32 vertical_ob; /* Analog crop rectangle. */ @@ -466,7 +465,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_vmax = 4000, .min_shr = 11, - .horizontal_ob = 96, .vertical_ob = 16, .crop = { .top = 40, @@ -497,7 +495,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .vbin_ratio = 2, .min_shr = 12, - .horizontal_ob = 48, .vertical_ob = 4, .crop = { @@ -529,7 +526,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .vbin_ratio = 3, .min_shr = 16, - .horizontal_ob = 32, .vertical_ob = 4, .crop = { @@ -555,7 +551,6 @@ static const struct imx283_mode supported_modes_10bit[] = { .default_vmax = 3840, .min_shr = 10, - .horizontal_ob = 96, .vertical_ob = 16, .crop = { .top = 40, From bcd21892703578144fdbeb42399d51eee06d5c0c Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Fri, 19 Sep 2025 12:33:14 +0100 Subject: [PATCH 06/25] media: i2c: imx283: Move vertical_ob to scan modes The Vertical Optical Black region is a property of the selected scan mode. Move the storage of this property to the scan mode table so it does not get duplicated when adding new output modes. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 92f55a8d70367f..8c452bf7f85be3 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -267,6 +267,9 @@ struct imx283_readout_mode { struct imx283_scanout { u8 bpp; struct imx283_readout_mode readout; + + /* Optical Blanking */ + u8 vertical_ob; }; static const struct imx283_scanout imx283_scan_modes[] = { @@ -274,52 +277,62 @@ static const struct imx283_scanout imx283_scan_modes[] = { [IMX283_MODE_0] = { .bpp = 12, .readout = { 0x04, 0x03, 0x10, 0x00 }, + .vertical_ob = 16, }, [IMX283_MODE_1] = { .bpp = 10, .readout = { 0x04, 0x01, 0x00, 0x00 }, + .vertical_ob = 16, }, [IMX283_MODE_1A] = { .bpp = 10, .readout = { 0x04, 0x01, 0x20, 0x50 }, + .vertical_ob = 16, }, [IMX283_MODE_1S] = { .bpp = 10, .readout = { 0x04, 0x41, 0x20, 0x50 }, + .vertical_ob = 16, }, /* Horizontal / Vertical 2/2-line binning */ [IMX283_MODE_2] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x50, 0x00 }, + .vertical_ob = 4, }, [IMX283_MODE_2A] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x70, 0x50 }, + .vertical_ob = 4, }, /* Horizontal / Vertical 3/3-line binning */ [IMX283_MODE_3] = { .bpp = 12, .readout = { 0x1e, 0x18, 0x10, 0x00 }, + .vertical_ob = 4, }, /* Vertical 2/9 subsampling, horizontal 3 binning cropping */ [IMX283_MODE_4] = { .bpp = 12, .readout = { 0x29, 0x18, 0x30, 0x50 }, + .vertical_ob = 4, }, /* Vertical 2/19 subsampling binning, horizontal 3 binning */ [IMX283_MODE_5] = { .bpp = 12, .readout = { 0x2d, 0x18, 0x10, 0x00 }, + .vertical_ob = 4, }, /* Vertical 2 binning horizontal 2/4, subsampling 16:9 cropping */ [IMX283_MODE_6] = { .bpp = 10, .readout = { 0x18, 0x21, 0x00, 0x09 }, + .vertical_ob = 4, }, /* @@ -381,9 +394,6 @@ struct imx283_mode { u8 hbin_ratio; u8 vbin_ratio; - /* Optical Blanking */ - u32 vertical_ob; - /* Analog crop rectangle. */ struct v4l2_rect crop; }; @@ -465,7 +475,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_vmax = 4000, .min_shr = 11, - .vertical_ob = 16, .crop = { .top = 40, .left = 108, @@ -495,7 +504,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .vbin_ratio = 2, .min_shr = 12, - .vertical_ob = 4, .crop = { .top = 40, @@ -526,7 +534,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .vbin_ratio = 3, .min_shr = 16, - .vertical_ob = 4, .crop = { .top = 40, @@ -551,7 +558,6 @@ static const struct imx283_mode supported_modes_10bit[] = { .default_vmax = 3840, .min_shr = 10, - .vertical_ob = 16, .crop = { .top = 40, .left = 108, @@ -1132,7 +1138,7 @@ static int imx283_start_streaming(struct imx283 *imx283, mode->crop.height); y_out_size = mode->crop.height / mode->vbin_ratio; - write_v_size = y_out_size + mode->vertical_ob; + write_v_size = y_out_size + mode->scan->vertical_ob; /* * cropping start position = (VWINPOS – Vst) × 2 * cropping width = Veff – (VWIDCUT – Vct) × 2 @@ -1147,7 +1153,7 @@ static int imx283_start_streaming(struct imx283 *imx283, cci_write(imx283->cci, IMX283_REG_VWIDCUT, v_widcut, &ret); cci_write(imx283->cci, IMX283_REG_VWINPOS, v_pos, &ret); - cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->vertical_ob, &ret); + cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->scan->vertical_ob, &ret); /* TODO: Validate mode->crop is fully contained within imx283_native_area */ cci_write(imx283->cci, IMX283_REG_HTRIMMING_START, mode->crop.left, &ret); From ce733ad625fda73f6913a5128893cdc1dc65b5f1 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Sun, 21 Sep 2025 12:30:43 +0100 Subject: [PATCH 07/25] media: i2c: imx283: Factor out vertical cropping parameters The vertical cropping parameters are specific to the readout mode selected and do not need to be duplicated on v4l2 mode choices. Move them to the imx283_scanout definitions. This also fixes the 10bit mode which had not yet defined the veff correctly and worked by chance. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 61 ++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 8c452bf7f85be3..2a5f6a49d32987 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -270,6 +270,11 @@ struct imx283_scanout { /* Optical Blanking */ u8 vertical_ob; + + /* Vertical Arbitrary Cropping Function */ + u16 vst; + u16 vct; + u16 veff; }; static const struct imx283_scanout imx283_scan_modes[] = { @@ -278,21 +283,33 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x04, 0x03, 0x10, 0x00 }, .vertical_ob = 16, + .vst = 0, + .vct = 0, + .veff = 3694, }, [IMX283_MODE_1] = { .bpp = 10, .readout = { 0x04, 0x01, 0x00, 0x00 }, .vertical_ob = 16, + .vst = 0, + .vct = 0, + .veff = 3694, }, [IMX283_MODE_1A] = { .bpp = 10, .readout = { 0x04, 0x01, 0x20, 0x50 }, .vertical_ob = 16, + .vst = 146, + .vct = 291, + .veff = 3112, }, [IMX283_MODE_1S] = { .bpp = 10, .readout = { 0x04, 0x41, 0x20, 0x50 }, .vertical_ob = 16, + .vst = 162, + .vct = 324, + .veff = 3046, }, /* Horizontal / Vertical 2/2-line binning */ @@ -300,11 +317,17 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x50, 0x00 }, .vertical_ob = 4, + .vst = 0, + .vct = 0, + .veff = 1824, }, [IMX283_MODE_2A] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x70, 0x50 }, .vertical_ob = 4, + .vst = 71, + .vct = 143, + .veff = 1556, }, /* Horizontal / Vertical 3/3-line binning */ @@ -312,6 +335,9 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x1e, 0x18, 0x10, 0x00 }, .vertical_ob = 4, + .vst = 0, + .vct = 0, + .veff = 1234, }, /* Vertical 2/9 subsampling, horizontal 3 binning cropping */ @@ -319,6 +345,9 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x29, 0x18, 0x30, 0x50 }, .vertical_ob = 4, + .vst = 9, + .vct = 17, + .veff = 378, }, /* Vertical 2/19 subsampling binning, horizontal 3 binning */ @@ -326,6 +355,9 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x2d, 0x18, 0x10, 0x00 }, .vertical_ob = 4, + .vst = 0, + .vct = 0, + .veff = 198, }, /* Vertical 2 binning horizontal 2/4, subsampling 16:9 cropping */ @@ -333,6 +365,9 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 10, .readout = { 0x18, 0x21, 0x00, 0x09 }, .vertical_ob = 4, + .vst = 0, + .vct = 0, + .veff = 1556, }, /* @@ -382,14 +417,6 @@ struct imx283_mode { /* minimum SHR */ u32 min_shr; - /* - * Per-mode vertical crop constants used to calculate values - * of IMX283REG_WIDCUT and IMX283_REG_VWINPOS. - */ - u32 veff; - u32 vst; - u32 vct; - /* Horizontal and vertical binning ratio */ u8 hbin_ratio; u8 vbin_ratio; @@ -463,10 +490,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .min_hmax = 5914, /* 887 @ 480MHz/72MHz */ .min_vmax = 3793, /* Lines */ - .veff = 3694, - .vst = 0, - .vct = 0, - .hbin_ratio = 1, .vbin_ratio = 1, @@ -496,10 +519,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 2500, /* 375 @ 480MHz/72Mhz */ .default_vmax = 3840, - .veff = 1824, - .vst = 0, - .vct = 0, - .hbin_ratio = 2, .vbin_ratio = 2, @@ -526,10 +545,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 1900, /* 285 @ 480MHz/72Mhz */ .default_vmax = 4200, - .veff = 1234, - .vst = 0, - .vct = 0, - .hbin_ratio = 3, .vbin_ratio = 3, @@ -1144,9 +1159,9 @@ static int imx283_start_streaming(struct imx283 *imx283, * cropping width = Veff – (VWIDCUT – Vct) × 2 */ v_pos = imx283->vflip->val ? - ((-mode->crop.top / mode->vbin_ratio) / 2) + mode->vst : - ((mode->crop.top / mode->vbin_ratio) / 2) + mode->vst; - v_widcut = ((mode->veff - y_out_size) / 2) + mode->vct; + ((-mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst : + ((mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst; + v_widcut = ((mode->scan->veff - y_out_size) / 2) + mode->scan->vct; cci_write(imx283->cci, IMX283_REG_Y_OUT_SIZE, y_out_size, &ret); cci_write(imx283->cci, IMX283_REG_WRITE_VSIZE, write_v_size, &ret); From b1988477ea25cc97df315f6cc1dffeedaacacda7 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Fri, 3 Oct 2025 14:39:28 +0100 Subject: [PATCH 08/25] media: i2c: imx283: Vertical offset corrections The IMX283 has different vertical offsets when applying binning modes. To provide consistent framing in each mode - ensure that the offsets measured are accounted for. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 2a5f6a49d32987..d1dd61fcc758e7 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -272,7 +272,7 @@ struct imx283_scanout { u8 vertical_ob; /* Vertical Arbitrary Cropping Function */ - u16 vst; + s16 vst; u16 vct; u16 veff; }; @@ -283,7 +283,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x04, 0x03, 0x10, 0x00 }, .vertical_ob = 16, - .vst = 0, + .vst = -1, /* Align to Mode 2/3 */ .vct = 0, .veff = 3694, }, @@ -291,7 +291,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 10, .readout = { 0x04, 0x01, 0x00, 0x00 }, .vertical_ob = 16, - .vst = 0, + .vst = -1, /* Align to Mode 2/3 */ .vct = 0, .veff = 3694, }, @@ -317,7 +317,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x50, 0x00 }, .vertical_ob = 4, - .vst = 0, + .vst = -2, /* Provides alignment to Mode 0/1 */ .vct = 0, .veff = 1824, }, @@ -335,7 +335,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x1e, 0x18, 0x10, 0x00 }, .vertical_ob = 4, - .vst = 0, + .vst = 1, /* Provides alignment to Mode 0/1 */ .vct = 0, .veff = 1234, }, From 3a26d4d46c37dc351905f8cad302c8151c550b83 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Sun, 21 Sep 2025 12:59:38 +0100 Subject: [PATCH 09/25] media: i2c: imx283: Define recommended area Provide a common reference for the recommended recording area to use in each of the binning modes. No functional change intended in this commit. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 44 +++++++++++++++----------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index d1dd61fcc758e7..260234f0c75921 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -175,7 +175,7 @@ #define IMX283_XCLR_MIN_DELAY_US (1 * USEC_PER_MSEC) #define IMX283_XCLR_DELAY_RANGE_US (1 * USEC_PER_MSEC) -/* IMX283 native and active pixel array size. */ +/* IMX283 crop regions and positions */ static const struct v4l2_rect imx283_native_area = { .top = 0, .left = 0, @@ -184,8 +184,16 @@ static const struct v4l2_rect imx283_native_area = { }; static const struct v4l2_rect imx283_active_area = { - .top = 40, - .left = 108, + .top = 16, + .left = 96, + .width = 5496, + .height = 3694, +}; + +/* Datasheet recommended recording pixels */ +static const struct v4l2_rect imx283_recommended_area = { + .top = 16 + 12 + 12, /* Clamp, Ignored area, Color margin */ + .left = 96 + 12, /* Horizontal black, Color margin */ .width = 5472, .height = 3648, }; @@ -498,12 +506,8 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_vmax = 4000, .min_shr = 11, - .crop = { - .top = 40, - .left = 108, - .width = 5472, - .height = 3648, - }, + + .crop = imx283_recommended_area, }, { /* @@ -524,12 +528,7 @@ static const struct imx283_mode supported_modes_12bit[] = { .min_shr = 12, - .crop = { - .top = 40, - .left = 108, - .width = 5472, - .height = 3648, - }, + .crop = imx283_recommended_area, }, { /* @@ -550,12 +549,7 @@ static const struct imx283_mode supported_modes_12bit[] = { .min_shr = 16, - .crop = { - .top = 40, - .left = 108, - .width = 5472, - .height = 3648, - }, + .crop = imx283_recommended_area, }, }; @@ -573,12 +567,8 @@ static const struct imx283_mode supported_modes_10bit[] = { .default_vmax = 3840, .min_shr = 10, - .crop = { - .top = 40, - .left = 108, - .width = 5472, - .height = 3648, - }, + + .crop = imx283_recommended_area, }, }; From c3b736f2535174cdee263e5019ed1bde82a50ed0 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Sun, 21 Sep 2025 12:59:38 +0100 Subject: [PATCH 10/25] media: i2c: imx283: Move Horizontal configuration block Adapt the Horiztonal configuration into its own scope to improve readability of these two associated register configurations. No functional change intended in this commit. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 260234f0c75921..4aa872642219c7 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1160,10 +1160,19 @@ static int imx283_start_streaming(struct imx283 *imx283, cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->scan->vertical_ob, &ret); - /* TODO: Validate mode->crop is fully contained within imx283_native_area */ - cci_write(imx283->cci, IMX283_REG_HTRIMMING_START, mode->crop.left, &ret); - cci_write(imx283->cci, IMX283_REG_HTRIMMING_END, - mode->crop.left + mode->crop.width, &ret); + /* Horizontal Configuration */ + { + /* + * While Vertical OB is excluded from the sensor positions, + * the Horizontal OB is included within the whole HTRIMMING + * calculation. + */ + u32 left = mode->crop.left; + u32 right = left + mode->crop.width; + + cci_write(imx283->cci, IMX283_REG_HTRIMMING_START, left, &ret); + cci_write(imx283->cci, IMX283_REG_HTRIMMING_END, right, &ret); + } /* Disable embedded data */ cci_write(imx283->cci, IMX283_REG_EBD_X_OUT_SIZE, 0, &ret); From 63eb6cd9aecd10acbb709f5282643e6efc7834c4 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 22 Sep 2025 15:21:33 +0100 Subject: [PATCH 11/25] media: i2c: imx283: Constrain scope of vertical calculations Reduce the scope of the 5 registers used for vertical positioning to make it easier to maintain and calculate the exact vertical configuration. No functional changes intended in this patch, which simplifies later development. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 41 ++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 4aa872642219c7..a9312ab1a06be3 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1092,10 +1092,7 @@ static int imx283_start_streaming(struct imx283 *imx283, const struct v4l2_mbus_framefmt *fmt; const struct imx283_mode *mode_list; unsigned int num_modes; - u32 v_widcut; - s32 v_pos; - u32 write_v_size; - u32 y_out_size; + int ret = 0; fmt = v4l2_subdev_state_get_format(state, 0); @@ -1142,23 +1139,29 @@ static int imx283_start_streaming(struct imx283 *imx283, mode->crop.width, mode->crop.height); - y_out_size = mode->crop.height / mode->vbin_ratio; - write_v_size = y_out_size + mode->scan->vertical_ob; - /* - * cropping start position = (VWINPOS – Vst) × 2 - * cropping width = Veff – (VWIDCUT – Vct) × 2 - */ - v_pos = imx283->vflip->val ? - ((-mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst : - ((mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst; - v_widcut = ((mode->scan->veff - y_out_size) / 2) + mode->scan->vct; + /* Vertical Configuration */ + { + u32 y_out_size = mode->crop.height / mode->vbin_ratio; + u32 write_v_size = y_out_size + mode->scan->vertical_ob; + u32 v_widcut; + s32 v_pos; - cci_write(imx283->cci, IMX283_REG_Y_OUT_SIZE, y_out_size, &ret); - cci_write(imx283->cci, IMX283_REG_WRITE_VSIZE, write_v_size, &ret); - cci_write(imx283->cci, IMX283_REG_VWIDCUT, v_widcut, &ret); - cci_write(imx283->cci, IMX283_REG_VWINPOS, v_pos, &ret); + /* + * cropping start position = (VWINPOS – Vst) × 2 + * cropping width = Veff – (VWIDCUT – Vct) × 2 + */ + v_pos = imx283->vflip->val ? + ((-mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst : + ((mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst; + v_widcut = ((mode->scan->veff - y_out_size) / 2) + mode->scan->vct; - cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->scan->vertical_ob, &ret); + cci_write(imx283->cci, IMX283_REG_Y_OUT_SIZE, y_out_size, &ret); + cci_write(imx283->cci, IMX283_REG_WRITE_VSIZE, write_v_size, &ret); + cci_write(imx283->cci, IMX283_REG_VWIDCUT, v_widcut, &ret); + cci_write(imx283->cci, IMX283_REG_VWINPOS, v_pos, &ret); + + cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->scan->vertical_ob, &ret); + } /* Horizontal Configuration */ { From 09b6977c168b52d9207432aa81f46d1f4bf87b5c Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 22 Sep 2025 15:28:31 +0100 Subject: [PATCH 12/25] media: i2c: imx283: Simplify v_pos determination Refactor the v_pos to separate out the vflip handling from the top coordinate. No functional change is intended in this commit. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index a9312ab1a06be3..d92dceb07ed6da 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1143,16 +1143,18 @@ static int imx283_start_streaming(struct imx283 *imx283, { u32 y_out_size = mode->crop.height / mode->vbin_ratio; u32 write_v_size = y_out_size + mode->scan->vertical_ob; + s16 top = mode->crop.top; u32 v_widcut; s32 v_pos; + if (imx283->vflip->val) + top = -top; + /* * cropping start position = (VWINPOS – Vst) × 2 * cropping width = Veff – (VWIDCUT – Vct) × 2 */ - v_pos = imx283->vflip->val ? - ((-mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst : - ((mode->crop.top / mode->vbin_ratio) / 2) + mode->scan->vst; + v_pos = (top / mode->vbin_ratio / 2) + mode->scan->vst; v_widcut = ((mode->scan->veff - y_out_size) / 2) + mode->scan->vct; cci_write(imx283->cci, IMX283_REG_Y_OUT_SIZE, y_out_size, &ret); From 559c06bc1a851a3f2903578dcf6d443ee8ec9188 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Tue, 7 Oct 2025 19:14:51 +0100 Subject: [PATCH 13/25] media: i2c: imx283: Move binning to scan modes The binning factors are determined by the chosen scan mode. Move the definition of the binning ratio to the scan mode strutures and remove from the v4l2 output mode definitions. The horizontal binning ratio is not used and therefore is dropped. This also fixes the 10-bit mode handling which previously had an undefined vbin_ratio for MODE1. V4L2 does not currently expose an API to support the differences between binning and skipping, so while the mode capabilities are kept for the skipping modes - there is no definition to use them yet. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index d92dceb07ed6da..25140ba66e0230 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -279,6 +279,9 @@ struct imx283_scanout { /* Optical Blanking */ u8 vertical_ob; + /* vertical binning ratio */ + u8 vbin_ratio; + /* Vertical Arbitrary Cropping Function */ s16 vst; u16 vct; @@ -291,6 +294,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x04, 0x03, 0x10, 0x00 }, .vertical_ob = 16, + .vbin_ratio = 1, .vst = -1, /* Align to Mode 2/3 */ .vct = 0, .veff = 3694, @@ -299,6 +303,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 10, .readout = { 0x04, 0x01, 0x00, 0x00 }, .vertical_ob = 16, + .vbin_ratio = 1, .vst = -1, /* Align to Mode 2/3 */ .vct = 0, .veff = 3694, @@ -307,6 +312,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 10, .readout = { 0x04, 0x01, 0x20, 0x50 }, .vertical_ob = 16, + .vbin_ratio = 1, .vst = 146, .vct = 291, .veff = 3112, @@ -315,6 +321,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 10, .readout = { 0x04, 0x41, 0x20, 0x50 }, .vertical_ob = 16, + .vbin_ratio = 1, .vst = 162, .vct = 324, .veff = 3046, @@ -325,6 +332,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x50, 0x00 }, .vertical_ob = 4, + .vbin_ratio = 2, .vst = -2, /* Provides alignment to Mode 0/1 */ .vct = 0, .veff = 1824, @@ -333,6 +341,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x0d, 0x11, 0x70, 0x50 }, .vertical_ob = 4, + .vbin_ratio = 2, .vst = 71, .vct = 143, .veff = 1556, @@ -343,6 +352,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x1e, 0x18, 0x10, 0x00 }, .vertical_ob = 4, + .vbin_ratio = 3, .vst = 1, /* Provides alignment to Mode 0/1 */ .vct = 0, .veff = 1234, @@ -353,6 +363,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x29, 0x18, 0x30, 0x50 }, .vertical_ob = 4, + .vbin_ratio = 1, /* SUBSAMPLING UNDEFINED */ .vst = 9, .vct = 17, .veff = 378, @@ -363,6 +374,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 12, .readout = { 0x2d, 0x18, 0x10, 0x00 }, .vertical_ob = 4, + .vbin_ratio = 1, /* SUBSAMPLING UNDEFINED */ .vst = 0, .vct = 0, .veff = 198, @@ -373,6 +385,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .bpp = 10, .readout = { 0x18, 0x21, 0x00, 0x09 }, .vertical_ob = 4, + .vbin_ratio = 2, /* SUBSAMPLING UNDEFINED */ .vst = 0, .vct = 0, .veff = 1556, @@ -425,10 +438,6 @@ struct imx283_mode { /* minimum SHR */ u32 min_shr; - /* Horizontal and vertical binning ratio */ - u8 hbin_ratio; - u8 vbin_ratio; - /* Analog crop rectangle. */ struct v4l2_rect crop; }; @@ -498,9 +507,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .min_hmax = 5914, /* 887 @ 480MHz/72MHz */ .min_vmax = 3793, /* Lines */ - .hbin_ratio = 1, - .vbin_ratio = 1, - /* 20.00 FPS */ .default_hmax = 6000, /* 900 @ 480MHz/72MHz */ .default_vmax = 4000, @@ -523,9 +529,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 2500, /* 375 @ 480MHz/72Mhz */ .default_vmax = 3840, - .hbin_ratio = 2, - .vbin_ratio = 2, - .min_shr = 12, .crop = imx283_recommended_area, @@ -544,9 +547,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 1900, /* 285 @ 480MHz/72Mhz */ .default_vmax = 4200, - .hbin_ratio = 3, - .vbin_ratio = 3, - .min_shr = 16, .crop = imx283_recommended_area, @@ -1141,7 +1141,7 @@ static int imx283_start_streaming(struct imx283 *imx283, /* Vertical Configuration */ { - u32 y_out_size = mode->crop.height / mode->vbin_ratio; + u32 y_out_size = mode->crop.height / mode->scan->vbin_ratio; u32 write_v_size = y_out_size + mode->scan->vertical_ob; s16 top = mode->crop.top; u32 v_widcut; @@ -1154,7 +1154,7 @@ static int imx283_start_streaming(struct imx283 *imx283, * cropping start position = (VWINPOS – Vst) × 2 * cropping width = Veff – (VWIDCUT – Vct) × 2 */ - v_pos = (top / mode->vbin_ratio / 2) + mode->scan->vst; + v_pos = (top / mode->scan->vbin_ratio / 2) + mode->scan->vst; v_widcut = ((mode->scan->veff - y_out_size) / 2) + mode->scan->vct; cci_write(imx283->cci, IMX283_REG_Y_OUT_SIZE, y_out_size, &ret); From 1b29354a0dc91a9aaa4bc44b6c5b8d6a43ea71d7 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Tue, 7 Oct 2025 20:01:29 +0100 Subject: [PATCH 14/25] media: i2c: imx283: Move minimum exposure handling to scan modes The minimum exposure is a factor of the scan mode. Move the definitions of the minimum exposure timeto the scan mode structures and clean up the duplication from the v4l2 output mode definitions. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 25140ba66e0230..bc288e695b7260 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -286,6 +286,9 @@ struct imx283_scanout { s16 vst; u16 vct; u16 veff; + + /* minimum SHR */ + u32 min_shr; }; static const struct imx283_scanout imx283_scan_modes[] = { @@ -298,6 +301,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = -1, /* Align to Mode 2/3 */ .vct = 0, .veff = 3694, + .min_shr = 11, }, [IMX283_MODE_1] = { .bpp = 10, @@ -307,6 +311,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = -1, /* Align to Mode 2/3 */ .vct = 0, .veff = 3694, + .min_shr = 10, }, [IMX283_MODE_1A] = { .bpp = 10, @@ -316,6 +321,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 146, .vct = 291, .veff = 3112, + .min_shr = 10, }, [IMX283_MODE_1S] = { .bpp = 10, @@ -325,6 +331,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 162, .vct = 324, .veff = 3046, + .min_shr = 10, }, /* Horizontal / Vertical 2/2-line binning */ @@ -336,6 +343,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = -2, /* Provides alignment to Mode 0/1 */ .vct = 0, .veff = 1824, + .min_shr = 12, }, [IMX283_MODE_2A] = { .bpp = 12, @@ -345,6 +353,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 71, .vct = 143, .veff = 1556, + .min_shr = 12, }, /* Horizontal / Vertical 3/3-line binning */ @@ -356,6 +365,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 1, /* Provides alignment to Mode 0/1 */ .vct = 0, .veff = 1234, + .min_shr = 16, }, /* Vertical 2/9 subsampling, horizontal 3 binning cropping */ @@ -367,6 +377,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 9, .vct = 17, .veff = 378, + .min_shr = 4, }, /* Vertical 2/19 subsampling binning, horizontal 3 binning */ @@ -378,6 +389,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 0, .vct = 0, .veff = 198, + .min_shr = 4, }, /* Vertical 2 binning horizontal 2/4, subsampling 16:9 cropping */ @@ -389,6 +401,7 @@ static const struct imx283_scanout imx283_scan_modes[] = { .vst = 0, .vct = 0, .veff = 1556, + .min_shr = 12, }, /* @@ -435,9 +448,6 @@ struct imx283_mode { /* default V-timing */ u32 default_vmax; - /* minimum SHR */ - u32 min_shr; - /* Analog crop rectangle. */ struct v4l2_rect crop; }; @@ -511,8 +521,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 6000, /* 900 @ 480MHz/72MHz */ .default_vmax = 4000, - .min_shr = 11, - .crop = imx283_recommended_area, }, { @@ -529,8 +537,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 2500, /* 375 @ 480MHz/72Mhz */ .default_vmax = 3840, - .min_shr = 12, - .crop = imx283_recommended_area, }, { @@ -547,8 +553,6 @@ static const struct imx283_mode supported_modes_12bit[] = { .default_hmax = 1900, /* 285 @ 480MHz/72Mhz */ .default_vmax = 4200, - .min_shr = 16, - .crop = imx283_recommended_area, }, }; @@ -566,8 +570,6 @@ static const struct imx283_mode supported_modes_10bit[] = { .default_hmax = 6000, /* 750 @ 576MHz / 72MHz */ .default_vmax = 3840, - .min_shr = 10, - .crop = imx283_recommended_area, }, }; @@ -726,7 +728,7 @@ static void imx283_exposure_limits(struct imx283 *imx283, s64 *min_exposure, s64 *max_exposure) { u32 svr = 0; /* SVR feature is not currently supported */ - u64 min_shr = mode->min_shr; + u64 min_shr = mode->scan->min_shr; /* Global Shutter is not supported */ u64 max_shr = (svr + 1) * imx283->vmax - 4; From f629053429279cf760af442caac8e9bf0a01f89d Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 22 Sep 2025 15:46:37 +0100 Subject: [PATCH 15/25] media: i2c: imx283: Simplify and clamp widcut calculation The vertical handling cuts unused lines to restrict the output to the desired height. It can be valid to set y_outsize larger than the veff to include optical black regions in the visible image. Ensure that the cut is clamped to the veff to prevent underflow. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index bc288e695b7260..daff15a6b84642 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1145,7 +1145,11 @@ static int imx283_start_streaming(struct imx283 *imx283, { u32 y_out_size = mode->crop.height / mode->scan->vbin_ratio; u32 write_v_size = y_out_size + mode->scan->vertical_ob; + s16 top = mode->crop.top; + u16 veff = mode->scan->veff; + u16 cut = veff - min(veff, y_out_size); + u32 v_widcut; s32 v_pos; @@ -1157,7 +1161,7 @@ static int imx283_start_streaming(struct imx283 *imx283, * cropping width = Veff – (VWIDCUT – Vct) × 2 */ v_pos = (top / mode->scan->vbin_ratio / 2) + mode->scan->vst; - v_widcut = ((mode->scan->veff - y_out_size) / 2) + mode->scan->vct; + v_widcut = (cut / 2) + mode->scan->vct; cci_write(imx283->cci, IMX283_REG_Y_OUT_SIZE, y_out_size, &ret); cci_write(imx283->cci, IMX283_REG_WRITE_VSIZE, write_v_size, &ret); From 9522ab802cfd82f31b8f58accc5a32ccbef8f968 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Wed, 24 Sep 2025 16:51:00 +0100 Subject: [PATCH 16/25] media: i2c: imx283: Account for clamp region coordinates The user clamp region is included in the pixel native area coordinates but is not included in the sensor vertical coordinates when configuring the VWINPOS. Remove the 16 line offset from the top position to account for this and in the event that a crop position requested vertical optical black, reduce the OB_SIZE_V register which causes those lines to be output in the main image data type instead. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index daff15a6b84642..01769424d20f90 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1144,15 +1144,31 @@ static int imx283_start_streaming(struct imx283 *imx283, /* Vertical Configuration */ { u32 y_out_size = mode->crop.height / mode->scan->vbin_ratio; - u32 write_v_size = y_out_size + mode->scan->vertical_ob; - s16 top = mode->crop.top; + /* + * The CLAMP region contains the Vertical Optical Black (VOB) + * lines, which are not included in the effective image height + * and 0 of the VWIDPOS corresponds to the first line after. + * + * This puts any request to view VOB as negative positions. + */ + s16 top = mode->crop.top - 16; u16 veff = mode->scan->veff; u16 cut = veff - min(veff, y_out_size); u32 v_widcut; s32 v_pos; + /* + * Reduce the v_ob when the requested crop position is below + * zero to output the VOB on image data. + */ + u8 v_ob = mode->scan->vertical_ob + min_t(s16, 0, top); + u32 write_v_size = y_out_size + v_ob; + + /* Clamp our top position now that VOB is handled */ + top = max_t(s16, 0, top); + if (imx283->vflip->val) top = -top; @@ -1168,7 +1184,7 @@ static int imx283_start_streaming(struct imx283 *imx283, cci_write(imx283->cci, IMX283_REG_VWIDCUT, v_widcut, &ret); cci_write(imx283->cci, IMX283_REG_VWINPOS, v_pos, &ret); - cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, mode->scan->vertical_ob, &ret); + cci_write(imx283->cci, IMX283_REG_OB_SIZE_V, v_ob, &ret); } /* Horizontal Configuration */ From 3dc2a59beeb23e280d6082011eb6668ce9cfd3a3 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 22 Sep 2025 17:26:29 +0100 Subject: [PATCH 17/25] media: i2c: imx283: Crop leading lines with user clamp The IMX283 introduces an extra line on the all-pixel scan out modes to prevent bayer re-ordering on flip handling. This is undesireable as it introduces the line in all crop configurations when the image is not flipped. The OB_SIZE_V register determines how many lines from the output will be directed into the custom data type for optical black region. To overcome the extra line which is forcefully added, utilise the vertical optical black region to redirect the extra line. An additional line also needs to be redirected to once again retain the bayer order. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 01769424d20f90..95ea4b11836da3 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1169,6 +1169,29 @@ static int imx283_start_streaming(struct imx283 *imx283, /* Clamp our top position now that VOB is handled */ top = max_t(s16, 0, top); + /* + * The all-pixel scan modes introduce an 'extra line' at the + * start of the image when not flipped. This is understood to be + * a mechanism to preserve bayer ordering regardless of vflip + * however it introduces an undesirable line of black pixels. + * + * Crop this by adding two extra lines and moving them into the + * OB data type without impacting the effective image height or + * positioning. + * + * It's ok for top to go negative here as the sensor does in + * fact have extra 'hidden' lines in this region and the sensor + * supports negative vertical cropping positions. (estimated + * about 48 extra lines) + */ + if ((scan_mode(mode->scan, IMX283_MODE_0) || + scan_mode(mode->scan, IMX283_MODE_1)) && + imx283->vflip->val == 0) { + top -= 2; + v_ob += 2; + write_v_size += 2; + } + if (imx283->vflip->val) top = -top; From 51a6e32f4eb265dc3d496e9c833c10a7a6f9c4d3 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 22 Sep 2025 18:46:49 +0100 Subject: [PATCH 18/25] media: i2c: imx283: Reduce vertical cutting The size should represent the amount of lines to cut vertically and exclude from the pixel array for the image data. Keeping this at the expected value causes corruption on the last line. Reduce by increasing the defined size of the image by a single line to ensure that the data is valid for the final output. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 95ea4b11836da3..0787fbc89c274f 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1154,7 +1154,12 @@ static int imx283_start_streaming(struct imx283 *imx283, */ s16 top = mode->crop.top - 16; u16 veff = mode->scan->veff; - u16 cut = veff - min(veff, y_out_size); + + /* + * Lots of empirical testing shows that we need to cut one less + * line than expected or we get corruption in the last line. + */ + u16 cut = veff - min(veff, y_out_size + 1); u32 v_widcut; s32 v_pos; From ef0d775bb10586a4cf16af2bd471770e5bb5f242 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Wed, 16 Oct 2024 12:26:59 +0530 Subject: [PATCH 19/25] media: i2c: imx283: Provide Native pixel array capture mode Provide a mode that outputs all pixels from the full native array. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 0787fbc89c274f..2048682ccbcf3d 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -508,6 +508,21 @@ static const struct imx283_reg_list link_freq_reglist[] = { /* Mode configs */ static const struct imx283_mode supported_modes_12bit[] = { + { + /* Full Native pixel array, including HOB/VOB. 5592x3710 */ + .scan = &imx283_scan_modes[IMX283_MODE_0], + + .width = 5592, + .height = 3710, /* 3694 + 16 additional lines for VOB */ + .min_hmax = 5914, /* 887 @ 480MHz/72MHz */ + .min_vmax = 3793, /* Lines */ + + /* 20.00 FPS */ + .default_hmax = 6000, /* 900 @ 480MHz/72MHz */ + .default_vmax = 4000, + + .crop = imx283_native_area, + }, { /* 20MPix 21.40 fps readout mode 0 */ .scan = &imx283_scan_modes[IMX283_MODE_0], From d521316697cc63dbe3ce9f24906bf872e2d83cb9 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 22 Sep 2025 10:52:56 +0100 Subject: [PATCH 20/25] media: i2c: imx283: Provide a full active pixels mode Add a mode to support output of the full illuminated area of the pixel array. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 2048682ccbcf3d..3cdb9ec41893c6 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -523,6 +523,21 @@ static const struct imx283_mode supported_modes_12bit[] = { .crop = imx283_native_area, }, + { + /* All illuminated pixels : 5496x3694 */ + .scan = &imx283_scan_modes[IMX283_MODE_0], + + .width = 5496, + .height = 3694, + .min_hmax = 5914, /* 887 @ 480MHz/72MHz */ + .min_vmax = 3793, /* Lines */ + + /* 20.00 FPS */ + .default_hmax = 6000, /* 900 @ 480MHz/72MHz */ + .default_vmax = 4000, + + .crop = imx283_active_area, + }, { /* 20MPix 21.40 fps readout mode 0 */ .scan = &imx283_scan_modes[IMX283_MODE_0], From 0a93b53a9c8acd51f406be152e90766c609e2a60 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Wed, 24 Sep 2025 19:54:50 +0100 Subject: [PATCH 21/25] media: i2c: imx283: Provide an effective pixel array mode Provide a mode that includes all effective pixels which includes a 12 pixel margin for colour processing on all edges. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 3cdb9ec41893c6..b031812e816bfb 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -183,6 +183,13 @@ static const struct v4l2_rect imx283_native_area = { .height = 3710, }; +static const struct v4l2_rect imx283_effective_area = { + .top = 16 + 12, /* Clamp + Ignored area*/ + .left = 96, + .width = 5496, + .height = 3672, +}; + static const struct v4l2_rect imx283_active_area = { .top = 16, .left = 96, @@ -538,6 +545,21 @@ static const struct imx283_mode supported_modes_12bit[] = { .crop = imx283_active_area, }, + { + /* Effective Pixel Mode : 5496x3672 */ + .scan = &imx283_scan_modes[IMX283_MODE_0], + + .width = 5496, + .height = 3672, + .min_hmax = 5914, /* 887 @ 480MHz/72MHz */ + .min_vmax = 3793, /* Lines */ + + /* 20.00 FPS */ + .default_hmax = 6000, /* 900 @ 480MHz/72MHz */ + .default_vmax = 4000, + + .crop = imx283_effective_area, + }, { /* 20MPix 21.40 fps readout mode 0 */ .scan = &imx283_scan_modes[IMX283_MODE_0], From 5f68734acda2f2bc174fc4954a1174def379f3fb Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Tue, 3 Dec 2024 15:50:50 +0000 Subject: [PATCH 22/25] media: i2c: imx283: Recalculate SHR on blanking changes The exposure control on the imx283 is handled through the SHR register. This value is calculated based upon the hmax and vmax registers as a property of the total line and frame length. Ensure that the SHR is updated whenever the blankings update and adjust the frame intervals to ensure the correct exposure is configured on the sensor. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index b031812e816bfb..a61614eb5f73ba 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -909,6 +909,11 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) dev_dbg(imx283->dev, "V4L2_CID_HBLANK : %d HMAX : %u\n", ctrl->val, imx283->hmax); ret = cci_write(imx283->cci, IMX283_REG_HMAX, imx283->hmax, NULL); + + /* Recompute the SHR based on the new timings */ + shr = imx283_shr(imx283, mode, imx283->exposure->val); + cci_write(imx283->cci, IMX283_REG_SHR, shr, &ret); + break; case V4L2_CID_VBLANK: @@ -916,6 +921,11 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) dev_dbg(imx283->dev, "V4L2_CID_VBLANK : %d VMAX : %u\n", ctrl->val, imx283->vmax); ret = cci_write(imx283->cci, IMX283_REG_VMAX, imx283->vmax, NULL); + + /* Recompute the SHR based on the new timings */ + shr = imx283_shr(imx283, mode, imx283->exposure->val); + cci_write(imx283->cci, IMX283_REG_SHR, shr, &ret); + break; case V4L2_CID_ANALOGUE_GAIN: From d8be84afe5793d72efbcc30110f6b7df83f08848 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Mon, 13 Oct 2025 17:38:32 +0100 Subject: [PATCH 23/25] media: i2c: imx283: Fix binned mode blanking timings The IMX283 supports binning modes which combine multiple measured pixels into a single output pixel. The minimum timings for this must account for the operations on all measured pixels, not the output pixel sizes. Determine and calculate all hmax and vmax values in respect of the HBLANK and VBLANK controls against the native resolution of the mode as specified in the mode crop rectangle as opposed to the output mode width and height. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index a61614eb5f73ba..5b000373291f75 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -875,7 +875,7 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) /* Honour the VBLANK limits when setting exposure. */ s64 current_exposure, max_exposure, min_exposure; - imx283->vmax = mode->height + ctrl->val; + imx283->vmax = mode->crop.height + ctrl->val; imx283_exposure_limits(imx283, mode, &min_exposure, &max_exposure); @@ -905,7 +905,7 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_HBLANK: pixel_rate = imx283_pixel_rate(imx283, mode); - imx283->hmax = imx283_internal_clock(pixel_rate, mode->width + ctrl->val); + imx283->hmax = imx283_internal_clock(pixel_rate, mode->crop.width + ctrl->val); dev_dbg(imx283->dev, "V4L2_CID_HBLANK : %d HMAX : %u\n", ctrl->val, imx283->hmax); ret = cci_write(imx283->cci, IMX283_REG_HMAX, imx283->hmax, NULL); @@ -917,7 +917,7 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) break; case V4L2_CID_VBLANK: - imx283->vmax = mode->height + ctrl->val; + imx283->vmax = mode->crop.height + ctrl->val; dev_dbg(imx283->dev, "V4L2_CID_VBLANK : %d VMAX : %u\n", ctrl->val, imx283->vmax); ret = cci_write(imx283->cci, IMX283_REG_VMAX, imx283->vmax, NULL); @@ -1042,6 +1042,9 @@ static void imx283_set_framing_limits(struct imx283 *imx283, u64 pixel_rate = imx283_pixel_rate(imx283, mode); u64 min_hblank, max_hblank, def_hblank; + /* Use crop for timings from native sensor units */ + const struct v4l2_rect *crop = &mode->crop; + /* Initialise hmax and vmax for exposure calculations */ imx283->hmax = imx283_internal_clock(pixel_rate, mode->default_hmax); imx283->vmax = mode->default_vmax; @@ -1050,18 +1053,18 @@ static void imx283_set_framing_limits(struct imx283 *imx283, * Horizontal Blanking * Convert the HMAX_MAX (72MHz) to Pixel rate values for HBLANK_MAX */ - min_hblank = mode->min_hmax - mode->width; - max_hblank = imx283_iclk_to_pix(pixel_rate, IMX283_HMAX_MAX) - mode->width; - def_hblank = mode->default_hmax - mode->width; + min_hblank = mode->min_hmax - crop->width; + max_hblank = imx283_iclk_to_pix(pixel_rate, IMX283_HMAX_MAX) - crop->width; + def_hblank = mode->default_hmax - crop->width; __v4l2_ctrl_modify_range(imx283->hblank, min_hblank, max_hblank, 1, def_hblank); __v4l2_ctrl_s_ctrl(imx283->hblank, def_hblank); /* Vertical Blanking */ - __v4l2_ctrl_modify_range(imx283->vblank, mode->min_vmax - mode->height, - IMX283_VMAX_MAX - mode->height, 1, + __v4l2_ctrl_modify_range(imx283->vblank, mode->min_vmax - crop->height, + IMX283_VMAX_MAX - crop->height, 1, mode->default_vmax - mode->height); - __v4l2_ctrl_s_ctrl(imx283->vblank, mode->default_vmax - mode->height); + __v4l2_ctrl_s_ctrl(imx283->vblank, mode->default_vmax - crop->height); } static int imx283_set_pad_format(struct v4l2_subdev *sd, @@ -1511,13 +1514,13 @@ static int imx283_init_controls(struct imx283 *imx283) /* Initialise vblank/hblank/exposure based on the current mode. */ imx283->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx283_ctrl_ops, V4L2_CID_VBLANK, - mode->min_vmax - mode->height, + mode->min_vmax - mode->crop.height, IMX283_VMAX_MAX, 1, - mode->default_vmax - mode->height); + mode->default_vmax - mode->crop.height); - min_hblank = mode->min_hmax - mode->width; - max_hblank = imx283_iclk_to_pix(pixel_rate, IMX283_HMAX_MAX) - mode->width; - def_hblank = mode->default_hmax - mode->width; + min_hblank = mode->min_hmax - mode->crop.width; + max_hblank = imx283_iclk_to_pix(pixel_rate, IMX283_HMAX_MAX) - mode->crop.width; + def_hblank = mode->default_hmax - mode->crop.width; imx283->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx283_ctrl_ops, V4L2_CID_HBLANK, min_hblank, max_hblank, 1, def_hblank); From 19479abdd2ad47b21bdba9d8d0c1585dab6b3763 Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Tue, 14 Oct 2025 16:05:11 +0100 Subject: [PATCH 24/25] media: i2c: imx283: Update exposure range on blanking changes The exposure ranges are updated currently whenever the VBLANK control is updated. However, the total exposure limits are a factor of both the HBLANK and the VBLANK durations. Determine the hmax value any time the control is set, including if the device is not yet streaming and update the limits accordingly. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 5b000373291f75..094410d5214044 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -857,7 +857,7 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) const struct imx283_mode *mode_list; struct v4l2_subdev_state *state; unsigned int num_modes; - u64 shr, pixel_rate; + u64 shr; int ret = 0; state = v4l2_subdev_get_locked_active_state(&imx283->sd); @@ -868,15 +868,24 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) fmt->width, fmt->height); /* - * The VBLANK control may change the limits of usable exposure, so check - * and adjust if necessary. + * The VBLANK/HBLANK controls change the limits of usable exposure, + * so check and adjust if necessary. */ - if (ctrl->id == V4L2_CID_VBLANK) { - /* Honour the VBLANK limits when setting exposure. */ - s64 current_exposure, max_exposure, min_exposure; + if (ctrl->id == V4L2_CID_HBLANK) { + u64 pixel_rate = imx283_pixel_rate(imx283, mode); + u32 max_width = mode->crop.width + ctrl->val; + + imx283->hmax = imx283_internal_clock(pixel_rate, max_width); + } + if (ctrl->id == V4L2_CID_VBLANK) imx283->vmax = mode->crop.height + ctrl->val; + if (ctrl->id == V4L2_CID_HBLANK || + ctrl->id == V4L2_CID_VBLANK) { + /* Honour the VBLANK limits when setting exposure. */ + s64 current_exposure, max_exposure, min_exposure; + imx283_exposure_limits(imx283, mode, &min_exposure, &max_exposure); @@ -904,8 +913,6 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) break; case V4L2_CID_HBLANK: - pixel_rate = imx283_pixel_rate(imx283, mode); - imx283->hmax = imx283_internal_clock(pixel_rate, mode->crop.width + ctrl->val); dev_dbg(imx283->dev, "V4L2_CID_HBLANK : %d HMAX : %u\n", ctrl->val, imx283->hmax); ret = cci_write(imx283->cci, IMX283_REG_HMAX, imx283->hmax, NULL); From 3d804a829df321546e92bf4db3731e049f8eb08f Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Tue, 14 Oct 2025 16:10:06 +0100 Subject: [PATCH 25/25] media: i2c: imx283: Simplify VFLIP control setting The VFLIP control is configured through the MDVREV bit of the HTRIMMING register. Simplify the conditional branch from 5 lines to 3 by determining the trim value which must always include setting the HTRIMMING_EN bit, and conditionally set the MDVREV before applying it to the hardware. Signed-off-by: Kieran Bingham --- drivers/media/i2c/imx283.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 094410d5214044..667da30bed2d90 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -948,13 +948,11 @@ static int imx283_set_ctrl(struct v4l2_ctrl *ctrl) * VFLIP is managed by BIT(0) of IMX283_REG_HTRIMMING address, hence * both need to be set simultaneously. */ - if (ctrl->val) { - cci_write(imx283->cci, IMX283_REG_HTRIMMING, - IMX283_HTRIMMING_EN | IMX283_MDVREV, &ret); - } else { - cci_write(imx283->cci, IMX283_REG_HTRIMMING, - IMX283_HTRIMMING_EN, &ret); - } + u8 trim = IMX283_HTRIMMING_EN; + + trim |= ctrl->val ? IMX283_MDVREV : 0; + cci_write(imx283->cci, IMX283_REG_HTRIMMING, trim, &ret); + break; case V4L2_CID_TEST_PATTERN: