* [PATCH v2 01/25] media: i2c: imx283: Report correct V4L2_SEL_TGT_CROP
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:39 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 02/25] media: i2c: imx283: Fix handling of unsupported mbus codes Kieran Bingham
` (23 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham,
Stefan Klug, stable
From: Stefan Klug <stefan.klug@ideasonboard.com>
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.
Cc: stable@vger.kernel.org # v6.10-rc1-70-gccb4eb4496fa
Fixes: ccb4eb4496fa ("media: i2c: Add imx283 camera sensor driver")
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
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 8ab63ad8f385..e5c04d259625 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;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 01/25] media: i2c: imx283: Report correct V4L2_SEL_TGT_CROP
2026-02-13 14:01 ` [PATCH v2 01/25] media: i2c: imx283: Report correct V4L2_SEL_TGT_CROP Kieran Bingham
@ 2026-02-16 14:39 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:39 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham, Stefan Klug, stable
Quoting Kieran Bingham (2026-02-13 19:31:40)
> From: Stefan Klug <stefan.klug@ideasonboard.com>
>
> 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.
>
> Cc: stable@vger.kernel.org # v6.10-rc1-70-gccb4eb4496fa
> Fixes: ccb4eb4496fa ("media: i2c: Add imx283 camera sensor driver")
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 8ab63ad8f385..e5c04d259625 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;
> }
>
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 02/25] media: i2c: imx283: Fix handling of unsupported mbus codes
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 01/25] media: i2c: imx283: Report correct V4L2_SEL_TGT_CROP Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:41 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 03/25] media: i2c: imx283: Move imx283_mode structure definition Kieran Bingham
` (22 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham,
Stefan Klug, stable
From: Stefan Klug <stefan.klug@ideasonboard.com>
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.
Cc: stable@vger.kernel.org # v6.10-rc1-70-gccb4eb4496fa
Fixes: ccb4eb4496fa ("media: i2c: Add imx283 camera sensor driver")
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
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 e5c04d259625..9a47cd0b181a 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,
@@ -1361,8 +1371,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,
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 02/25] media: i2c: imx283: Fix handling of unsupported mbus codes
2026-02-13 14:01 ` [PATCH v2 02/25] media: i2c: imx283: Fix handling of unsupported mbus codes Kieran Bingham
@ 2026-02-16 14:41 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:41 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham, Stefan Klug, stable
Quoting Kieran Bingham (2026-02-13 19:31:41)
> From: Stefan Klug <stefan.klug@ideasonboard.com>
>
> 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.
>
> Cc: stable@vger.kernel.org # v6.10-rc1-70-gccb4eb4496fa
> Fixes: ccb4eb4496fa ("media: i2c: Add imx283 camera sensor driver")
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 e5c04d259625..9a47cd0b181a 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,
> @@ -1361,8 +1371,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,
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 03/25] media: i2c: imx283: Move imx283_mode structure definition
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 01/25] media: i2c: imx283: Report correct V4L2_SEL_TGT_CROP Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 02/25] media: i2c: imx283: Fix handling of unsupported mbus codes Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:41 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 04/25] media: i2c: imx283: Move scan out data to single data structure Kieran Bingham
` (21 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 9a47cd0b181a..d53cea49baae 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 */
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 03/25] media: i2c: imx283: Move imx283_mode structure definition
2026-02-13 14:01 ` [PATCH v2 03/25] media: i2c: imx283: Move imx283_mode structure definition Kieran Bingham
@ 2026-02-16 14:41 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:41 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:42)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 9a47cd0b181a..d53cea49baae 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 */
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 04/25] media: i2c: imx283: Move scan out data to single data structure
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (2 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 03/25] media: i2c: imx283: Move imx283_mode structure definition Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:42 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 05/25] media: i2c: imx283: Remove horizontal_ob Kieran Bingham
` (20 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 d53cea49baae..3e97ad38f716 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);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 04/25] media: i2c: imx283: Move scan out data to single data structure
2026-02-13 14:01 ` [PATCH v2 04/25] media: i2c: imx283: Move scan out data to single data structure Kieran Bingham
@ 2026-02-16 14:42 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:42 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:43)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 d53cea49baae..3e97ad38f716 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);
> }
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 05/25] media: i2c: imx283: Remove horizontal_ob
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (3 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 04/25] media: i2c: imx283: Move scan out data to single data structure Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 06/25] media: i2c: imx283: Move vertical_ob to scan modes Kieran Bingham
` (19 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 3e97ad38f716..fe63700da872 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,
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 06/25] media: i2c: imx283: Move vertical_ob to scan modes
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (4 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 05/25] media: i2c: imx283: Remove horizontal_ob Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 07/25] media: i2c: imx283: Factor out vertical cropping parameters Kieran Bingham
` (18 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 fe63700da872..164e7c6125ae 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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 07/25] media: i2c: imx283: Factor out vertical cropping parameters
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (5 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 06/25] media: i2c: imx283: Move vertical_ob to scan modes Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:52 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 08/25] media: i2c: imx283: Vertical offset corrections Kieran Bingham
` (17 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 164e7c6125ae..0abfeeb89425 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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 07/25] media: i2c: imx283: Factor out vertical cropping parameters
2026-02-13 14:01 ` [PATCH v2 07/25] media: i2c: imx283: Factor out vertical cropping parameters Kieran Bingham
@ 2026-02-16 14:52 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:52 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:46)
> 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.
On Pi5 I see that 10bit mode is broken (black output) both before and after
this patch.
Testing 10bit output after applying the whole series, I at least don't get
a fully black frame, but I still get a frame with top 80% is bright but
garbled, and bottom 20% is black, so most probably an early line end signal
somewhere.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
In any case, for this patch:
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 164e7c6125ae..0abfeeb89425 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);
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 08/25] media: i2c: imx283: Vertical offset corrections
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (6 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 07/25] media: i2c: imx283: Factor out vertical cropping parameters Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 09/25] media: i2c: imx283: Define recommended area Kieran Bingham
` (16 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 0abfeeb89425..95f93ee0747f 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,
},
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 09/25] media: i2c: imx283: Define recommended area
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (7 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 08/25] media: i2c: imx283: Vertical offset corrections Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-14 10:17 ` kernel test robot
2026-02-13 14:01 ` [PATCH v2 10/25] media: i2c: imx283: Move Horizontal configuration block Kieran Bingham
` (15 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 95f93ee0747f..32b8070756f0 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,
},
};
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 09/25] media: i2c: imx283: Define recommended area
2026-02-13 14:01 ` [PATCH v2 09/25] media: i2c: imx283: Define recommended area Kieran Bingham
@ 2026-02-14 10:17 ` kernel test robot
0 siblings, 0 replies; 42+ messages in thread
From: kernel test robot @ 2026-02-14 10:17 UTC (permalink / raw)
To: Kieran Bingham, Umang Jain, Sakari Ailus, Mauro Carvalho Chehab,
Hans Verkuil
Cc: llvm, oe-kbuild-all, linux-media, Jai Luthra, linux-kernel,
Kieran Bingham
Hi Kieran,
kernel test robot noticed the following build errors:
[auto build test ERROR on c824345288d11e269ce41b36c105715bc2286050]
url: https://github.com/intel-lab-lkp/linux/commits/Kieran-Bingham/media-i2c-imx283-Report-correct-V4L2_SEL_TGT_CROP/20260213-221320
base: c824345288d11e269ce41b36c105715bc2286050
patch link: https://lore.kernel.org/r/20260213-mainline-imx283-v2-v2-9-be40a3770ebf%40ideasonboard.com
patch subject: [PATCH v2 09/25] media: i2c: imx283: Define recommended area
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20260214/202602141812.smD7Mebb-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260214/202602141812.smD7Mebb-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602141812.smD7Mebb-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/media/i2c/imx283.c:510:11: error: initializer element is not a compile-time constant
.crop = imx283_recommended_area,
^~~~~~~~~~~~~~~~~~~~~~~
drivers/media/i2c/imx283.c:571:11: error: initializer element is not a compile-time constant
.crop = imx283_recommended_area,
^~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
vim +510 drivers/media/i2c/imx283.c
489
490 /* Mode configs */
491 static const struct imx283_mode supported_modes_12bit[] = {
492 {
493 /* 20MPix 21.40 fps readout mode 0 */
494 .scan = &imx283_scan_modes[IMX283_MODE_0],
495
496 .width = 5472,
497 .height = 3648,
498 .min_hmax = 5914, /* 887 @ 480MHz/72MHz */
499 .min_vmax = 3793, /* Lines */
500
501 .hbin_ratio = 1,
502 .vbin_ratio = 1,
503
504 /* 20.00 FPS */
505 .default_hmax = 6000, /* 900 @ 480MHz/72MHz */
506 .default_vmax = 4000,
507
508 .min_shr = 11,
509
> 510 .crop = imx283_recommended_area,
511 },
512 {
513 /*
514 * Readout mode 2 : 2/2 binned mode (2736x1824)
515 */
516 .scan = &imx283_scan_modes[IMX283_MODE_2],
517 .width = 2736,
518 .height = 1824,
519 .min_hmax = 2414, /* Pixels (362 * 480MHz/72MHz + padding) */
520 .min_vmax = 3840, /* Lines */
521
522 /* 50.00 FPS */
523 .default_hmax = 2500, /* 375 @ 480MHz/72Mhz */
524 .default_vmax = 3840,
525
526 .hbin_ratio = 2,
527 .vbin_ratio = 2,
528
529 .min_shr = 12,
530
531 .crop = imx283_recommended_area,
532 },
533 {
534 /*
535 * Readout mode 3 : 3/3 binned mode (1824x1216)
536 */
537 .scan = &imx283_scan_modes[IMX283_MODE_3],
538 .width = 1824,
539 .height = 1216,
540 .min_hmax = 1894, /* Pixels (284 * 480MHz/72MHz + padding) */
541 .min_vmax = 4200, /* Lines */
542
543 /* 60.00 fps */
544 .default_hmax = 1900, /* 285 @ 480MHz/72Mhz */
545 .default_vmax = 4200,
546
547 .hbin_ratio = 3,
548 .vbin_ratio = 3,
549
550 .min_shr = 16,
551
552 .crop = imx283_recommended_area,
553 },
554 };
555
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 10/25] media: i2c: imx283: Move Horizontal configuration block
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (8 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 09/25] media: i2c: imx283: Define recommended area Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:53 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 11/25] media: i2c: imx283: Constrain scope of vertical calculations Kieran Bingham
` (14 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 32b8070756f0..6d551a26cfa6 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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 10/25] media: i2c: imx283: Move Horizontal configuration block
2026-02-13 14:01 ` [PATCH v2 10/25] media: i2c: imx283: Move Horizontal configuration block Kieran Bingham
@ 2026-02-16 14:53 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:53 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:49)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 32b8070756f0..6d551a26cfa6 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);
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 11/25] media: i2c: imx283: Constrain scope of vertical calculations
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (9 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 10/25] media: i2c: imx283: Move Horizontal configuration block Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:54 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 12/25] media: i2c: imx283: Simplify v_pos determination Kieran Bingham
` (13 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 6d551a26cfa6..f115a6df7b31 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 */
{
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 11/25] media: i2c: imx283: Constrain scope of vertical calculations
2026-02-13 14:01 ` [PATCH v2 11/25] media: i2c: imx283: Constrain scope of vertical calculations Kieran Bingham
@ 2026-02-16 14:54 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:54 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:50)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 6d551a26cfa6..f115a6df7b31 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 */
> {
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 12/25] media: i2c: imx283: Simplify v_pos determination
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (10 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 11/25] media: i2c: imx283: Constrain scope of vertical calculations Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 14:57 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 13/25] media: i2c: imx283: Move binning to scan modes Kieran Bingham
` (12 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 f115a6df7b31..315c050c4fd0 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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 12/25] media: i2c: imx283: Simplify v_pos determination
2026-02-13 14:01 ` [PATCH v2 12/25] media: i2c: imx283: Simplify v_pos determination Kieran Bingham
@ 2026-02-16 14:57 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:57 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:51)
> 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 <kieran.bingham@ideasonboard.com>
> ---
> 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 f115a6df7b31..315c050c4fd0 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;
nit: I find `((top / mode->vbin_ratio) / 2)` easier to read
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> 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);
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 13/25] media: i2c: imx283: Move binning to scan modes
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (11 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 12/25] media: i2c: imx283: Simplify v_pos determination Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 15:00 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 14/25] media: i2c: imx283: Move minimum exposure handling " Kieran Bingham
` (11 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 315c050c4fd0..d333be4e66d7 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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 13/25] media: i2c: imx283: Move binning to scan modes
2026-02-13 14:01 ` [PATCH v2 13/25] media: i2c: imx283: Move binning to scan modes Kieran Bingham
@ 2026-02-16 15:00 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 15:00 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:52)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 315c050c4fd0..d333be4e66d7 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);
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 14/25] media: i2c: imx283: Move minimum exposure handling to scan modes
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (12 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 13/25] media: i2c: imx283: Move binning to scan modes Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 15:01 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 15/25] media: i2c: imx283: Simplify and clamp widcut calculation Kieran Bingham
` (10 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 d333be4e66d7..6c2be9195cba 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;
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 14/25] media: i2c: imx283: Move minimum exposure handling to scan modes
2026-02-13 14:01 ` [PATCH v2 14/25] media: i2c: imx283: Move minimum exposure handling " Kieran Bingham
@ 2026-02-16 15:01 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 15:01 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:53)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 d333be4e66d7..6c2be9195cba 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;
>
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 15/25] media: i2c: imx283: Simplify and clamp widcut calculation
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (13 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 14/25] media: i2c: imx283: Move minimum exposure handling " Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 16/25] media: i2c: imx283: Account for clamp region coordinates Kieran Bingham
` (9 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 6c2be9195cba..f9cf2bb0e10d 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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 16/25] media: i2c: imx283: Account for clamp region coordinates
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (14 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 15/25] media: i2c: imx283: Simplify and clamp widcut calculation Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 17/25] media: i2c: imx283: Crop leading lines with user clamp Kieran Bingham
` (8 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 f9cf2bb0e10d..c3a44d2c6508 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 */
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 17/25] media: i2c: imx283: Crop leading lines with user clamp
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (15 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 16/25] media: i2c: imx283: Account for clamp region coordinates Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 18/25] media: i2c: imx283: Reduce vertical cutting Kieran Bingham
` (7 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 c3a44d2c6508..07958cd1889e 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;
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 18/25] media: i2c: imx283: Reduce vertical cutting
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (16 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 17/25] media: i2c: imx283: Crop leading lines with user clamp Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-13 14:01 ` [PATCH v2 19/25] media: i2c: imx283: Provide Native pixel array capture mode Kieran Bingham
` (6 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 07958cd1889e..df48793835fd 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;
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 19/25] media: i2c: imx283: Provide Native pixel array capture mode
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (17 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 18/25] media: i2c: imx283: Reduce vertical cutting Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 15:06 ` Jai Luthra
2026-02-13 14:01 ` [PATCH v2 20/25] media: i2c: imx283: Provide a full active pixels mode Kieran Bingham
` (5 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
Provide a mode that outputs all pixels from the full native array.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
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 df48793835fd..4976c08c6832 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],
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 19/25] media: i2c: imx283: Provide Native pixel array capture mode
2026-02-13 14:01 ` [PATCH v2 19/25] media: i2c: imx283: Provide Native pixel array capture mode Kieran Bingham
@ 2026-02-16 15:06 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 15:06 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:58)
> Provide a mode that outputs all pixels from the full native array.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 df48793835fd..4976c08c6832 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],
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 20/25] media: i2c: imx283: Provide a full active pixels mode
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (18 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 19/25] media: i2c: imx283: Provide Native pixel array capture mode Kieran Bingham
@ 2026-02-13 14:01 ` Kieran Bingham
2026-02-16 15:08 ` Jai Luthra
2026-02-13 14:02 ` [PATCH v2 21/25] media: i2c: imx283: Provide an effective pixel array mode Kieran Bingham
` (4 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:01 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
Add a mode to support output of the full illuminated area of the pixel
array.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
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 4976c08c6832..d001bfb5e241 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],
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 20/25] media: i2c: imx283: Provide a full active pixels mode
2026-02-13 14:01 ` [PATCH v2 20/25] media: i2c: imx283: Provide a full active pixels mode Kieran Bingham
@ 2026-02-16 15:08 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 15:08 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:31:59)
> Add a mode to support output of the full illuminated area of the pixel
> array.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 4976c08c6832..d001bfb5e241 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],
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 21/25] media: i2c: imx283: Provide an effective pixel array mode
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (19 preceding siblings ...)
2026-02-13 14:01 ` [PATCH v2 20/25] media: i2c: imx283: Provide a full active pixels mode Kieran Bingham
@ 2026-02-13 14:02 ` Kieran Bingham
2026-02-16 15:09 ` Jai Luthra
2026-02-13 14:02 ` [PATCH v2 22/25] media: i2c: imx283: Recalculate SHR on blanking changes Kieran Bingham
` (3 subsequent siblings)
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:02 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 d001bfb5e241..75a35d9db05d 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],
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 21/25] media: i2c: imx283: Provide an effective pixel array mode
2026-02-13 14:02 ` [PATCH v2 21/25] media: i2c: imx283: Provide an effective pixel array mode Kieran Bingham
@ 2026-02-16 15:09 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 15:09 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Quoting Kieran Bingham (2026-02-13 19:32:00)
> 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 <kieran.bingham@ideasonboard.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
> 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 d001bfb5e241..75a35d9db05d 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],
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 22/25] media: i2c: imx283: Recalculate SHR on blanking changes
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (20 preceding siblings ...)
2026-02-13 14:02 ` [PATCH v2 21/25] media: i2c: imx283: Provide an effective pixel array mode Kieran Bingham
@ 2026-02-13 14:02 ` Kieran Bingham
2026-02-13 14:02 ` [PATCH v2 23/25] media: i2c: imx283: Fix binned mode blanking timings Kieran Bingham
` (2 subsequent siblings)
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:02 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 75a35d9db05d..7fb654512c20 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:
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 23/25] media: i2c: imx283: Fix binned mode blanking timings
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (21 preceding siblings ...)
2026-02-13 14:02 ` [PATCH v2 22/25] media: i2c: imx283: Recalculate SHR on blanking changes Kieran Bingham
@ 2026-02-13 14:02 ` Kieran Bingham
2026-02-16 14:27 ` Jai Luthra
2026-02-13 14:02 ` [PATCH v2 24/25] media: i2c: imx283: Update exposure range on blanking changes Kieran Bingham
2026-02-13 14:02 ` [PATCH v2 25/25] media: i2c: imx283: Simplify VFLIP control setting Kieran Bingham
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:02 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 7fb654512c20..25e669370751 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,
@@ -1509,13 +1512,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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 23/25] media: i2c: imx283: Fix binned mode blanking timings
2026-02-13 14:02 ` [PATCH v2 23/25] media: i2c: imx283: Fix binned mode blanking timings Kieran Bingham
@ 2026-02-16 14:27 ` Jai Luthra
0 siblings, 0 replies; 42+ messages in thread
From: Jai Luthra @ 2026-02-16 14:27 UTC (permalink / raw)
To: Hans Verkuil, Kieran Bingham, Mauro Carvalho Chehab, Sakari Ailus,
Umang Jain
Cc: linux-media, linux-kernel, Kieran Bingham
Hi Kieran,
Thanks for the patch!
Quoting Kieran Bingham (2026-02-13 19:32:02)
> 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 <kieran.bingham@ideasonboard.com>
> ---
> 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 7fb654512c20..25e669370751 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);
Shouldn't this be `- crop->height` too?
> - __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,
> @@ -1509,13 +1512,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);
>
> --
> 2.52.0
>
Testing this series on Pi 5, I notice that this patch causes a regression.
Before this patch I could get ~45 and ~60fps for the 2x and 3x binned
modes, while now I only get ~20fps.
----- thinking out loud -------
The sensor expects HMAX to be programmed in units of sensor's internal
readout clock @ 72MHz.
This is the supported minimum for HMAX for various readout modes:
No binning: 887 units per line
2x2 binning: 362 ..
3x3 binning: 284 ..
Scaling these up from "72 Mhz" units to the pixel rate with 720Mhz link
frequency on Pi 5 (that is 480Mpixels/sec), I get:
No binning: 5914 pixels per line
2x2 binning: 2414 .. (*)
3x3 binning: 1894 ..
Which matches the min_hmax described in the mode table.
(*) 2414 < 2736 (width of the 2x2 mode) explains why I can only capture
max 45fps instead of intended 50fps. The sensor might be able to read a
line faster than what the link rate of 720Mhz supports.
------------------------------
Looking at the patch again
> - min_hblank = mode->min_hmax - mode->width;
> + min_hblank = mode->min_hmax - crop->width;
hblank = min_hmax (1894 for 3x binning) - crop.width (5472) is negative
> - imx283->hmax = imx283_internal_clock(pixel_rate, mode->width + ctrl->val);
> + imx283->hmax = imx283_internal_clock(pixel_rate, mode->crop.width + ctrl->val);
Passing "pixels" in crop dimensions to the internal_clock function seems
wrong to me.
What exactly do we get by representing HMAX/VMAX in "full resolution" pixel
units?
TBH both this new approach and the older one make it too confusing. The
sensor's HMAX is in the unit of "time" w.r.t the 72Mhz internal clock,
regardless of the pixel rate/link frequency.
IMHO the mode table should store that information directly, instead of in
units of pre or post-binned pixels, that too for an arbitrary link
frequency.
Thanks,
Jai
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 24/25] media: i2c: imx283: Update exposure range on blanking changes
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (22 preceding siblings ...)
2026-02-13 14:02 ` [PATCH v2 23/25] media: i2c: imx283: Fix binned mode blanking timings Kieran Bingham
@ 2026-02-13 14:02 ` Kieran Bingham
2026-02-13 14:02 ` [PATCH v2 25/25] media: i2c: imx283: Simplify VFLIP control setting Kieran Bingham
24 siblings, 0 replies; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:02 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 25e669370751..f0ede67921d9 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) {
+ 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->vmax = mode->crop.height + ctrl->val;
-
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);
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v2 25/25] media: i2c: imx283: Simplify VFLIP control setting
2026-02-13 14:01 [PATCH v2 00/25] drivers: media: imx283 improvements Kieran Bingham
` (23 preceding siblings ...)
2026-02-13 14:02 ` [PATCH v2 24/25] media: i2c: imx283: Update exposure range on blanking changes Kieran Bingham
@ 2026-02-13 14:02 ` Kieran Bingham
2026-02-13 23:20 ` kernel test robot
24 siblings, 1 reply; 42+ messages in thread
From: Kieran Bingham @ 2026-02-13 14:02 UTC (permalink / raw)
To: Umang Jain, Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil
Cc: Jai Luthra, linux-media, linux-kernel, Kieran Bingham
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 <kieran.bingham@ideasonboard.com>
---
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 f0ede67921d9..1698754ea059 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:
--
2.52.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v2 25/25] media: i2c: imx283: Simplify VFLIP control setting
2026-02-13 14:02 ` [PATCH v2 25/25] media: i2c: imx283: Simplify VFLIP control setting Kieran Bingham
@ 2026-02-13 23:20 ` kernel test robot
0 siblings, 0 replies; 42+ messages in thread
From: kernel test robot @ 2026-02-13 23:20 UTC (permalink / raw)
To: Kieran Bingham, Umang Jain, Sakari Ailus, Mauro Carvalho Chehab,
Hans Verkuil
Cc: oe-kbuild-all, linux-media, Jai Luthra, linux-kernel,
Kieran Bingham
Hi Kieran,
kernel test robot noticed the following build errors:
[auto build test ERROR on c824345288d11e269ce41b36c105715bc2286050]
url: https://github.com/intel-lab-lkp/linux/commits/Kieran-Bingham/media-i2c-imx283-Report-correct-V4L2_SEL_TGT_CROP/20260213-221320
base: c824345288d11e269ce41b36c105715bc2286050
patch link: https://lore.kernel.org/r/20260213-mainline-imx283-v2-v2-25-be40a3770ebf%40ideasonboard.com
patch subject: [PATCH v2 25/25] media: i2c: imx283: Simplify VFLIP control setting
config: openrisc-randconfig-r071-20260214 (https://download.01.org/0day-ci/archive/20260214/202602140716.ayhsqB6c-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 9.5.0
smatch version: v0.5.0-8994-gd50c5a4c
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260214/202602140716.ayhsqB6c-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602140716.ayhsqB6c-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/media/i2c/imx283.c: In function 'imx283_set_ctrl':
>> drivers/media/i2c/imx283.c:951:3: error: a label can only be part of a statement and a declaration is not a statement
951 | u8 trim = IMX283_HTRIMMING_EN;
| ^~
vim +951 drivers/media/i2c/imx283.c
850
851 static int imx283_set_ctrl(struct v4l2_ctrl *ctrl)
852 {
853 struct imx283 *imx283 = container_of(ctrl->handler, struct imx283,
854 ctrl_handler);
855 const struct imx283_mode *mode;
856 struct v4l2_mbus_framefmt *fmt;
857 const struct imx283_mode *mode_list;
858 struct v4l2_subdev_state *state;
859 unsigned int num_modes;
860 u64 shr;
861 int ret = 0;
862
863 state = v4l2_subdev_get_locked_active_state(&imx283->sd);
864 fmt = v4l2_subdev_state_get_format(state, 0);
865
866 get_mode_table(fmt->code, &mode_list, &num_modes);
867 mode = v4l2_find_nearest_size(mode_list, num_modes, width, height,
868 fmt->width, fmt->height);
869
870 /*
871 * The VBLANK/HBLANK controls change the limits of usable exposure,
872 * so check and adjust if necessary.
873 */
874 if (ctrl->id == V4L2_CID_HBLANK) {
875 u64 pixel_rate = imx283_pixel_rate(imx283, mode);
876 u32 max_width = mode->crop.width + ctrl->val;
877
878 imx283->hmax = imx283_internal_clock(pixel_rate, max_width);
879 }
880
881 if (ctrl->id == V4L2_CID_VBLANK)
882 imx283->vmax = mode->crop.height + ctrl->val;
883
884 if (ctrl->id == V4L2_CID_HBLANK ||
885 ctrl->id == V4L2_CID_VBLANK) {
886 /* Honour the VBLANK limits when setting exposure. */
887 s64 current_exposure, max_exposure, min_exposure;
888
889 imx283_exposure_limits(imx283, mode,
890 &min_exposure, &max_exposure);
891
892 current_exposure = imx283->exposure->val;
893 current_exposure = clamp(current_exposure, min_exposure,
894 max_exposure);
895
896 __v4l2_ctrl_modify_range(imx283->exposure, min_exposure,
897 max_exposure, 1, current_exposure);
898 }
899
900 /*
901 * Applying V4L2 control value only happens
902 * when power is up for streaming
903 */
904 if (!pm_runtime_get_if_active(imx283->dev))
905 return 0;
906
907 switch (ctrl->id) {
908 case V4L2_CID_EXPOSURE:
909 shr = imx283_shr(imx283, mode, ctrl->val);
910 dev_dbg(imx283->dev, "V4L2_CID_EXPOSURE : %d - SHR: %lld\n",
911 ctrl->val, shr);
912 ret = cci_write(imx283->cci, IMX283_REG_SHR, shr, NULL);
913 break;
914
915 case V4L2_CID_HBLANK:
916 dev_dbg(imx283->dev, "V4L2_CID_HBLANK : %d HMAX : %u\n",
917 ctrl->val, imx283->hmax);
918 ret = cci_write(imx283->cci, IMX283_REG_HMAX, imx283->hmax, NULL);
919
920 /* Recompute the SHR based on the new timings */
921 shr = imx283_shr(imx283, mode, imx283->exposure->val);
922 cci_write(imx283->cci, IMX283_REG_SHR, shr, &ret);
923
924 break;
925
926 case V4L2_CID_VBLANK:
927 imx283->vmax = mode->crop.height + ctrl->val;
928 dev_dbg(imx283->dev, "V4L2_CID_VBLANK : %d VMAX : %u\n",
929 ctrl->val, imx283->vmax);
930 ret = cci_write(imx283->cci, IMX283_REG_VMAX, imx283->vmax, NULL);
931
932 /* Recompute the SHR based on the new timings */
933 shr = imx283_shr(imx283, mode, imx283->exposure->val);
934 cci_write(imx283->cci, IMX283_REG_SHR, shr, &ret);
935
936 break;
937
938 case V4L2_CID_ANALOGUE_GAIN:
939 ret = cci_write(imx283->cci, IMX283_REG_ANALOG_GAIN, ctrl->val, NULL);
940 break;
941
942 case V4L2_CID_DIGITAL_GAIN:
943 ret = cci_write(imx283->cci, IMX283_REG_DIGITAL_GAIN, ctrl->val, NULL);
944 break;
945
946 case V4L2_CID_VFLIP:
947 /*
948 * VFLIP is managed by BIT(0) of IMX283_REG_HTRIMMING address, hence
949 * both need to be set simultaneously.
950 */
> 951 u8 trim = IMX283_HTRIMMING_EN;
952
953 trim |= ctrl->val ? IMX283_MDVREV : 0;
954 cci_write(imx283->cci, IMX283_REG_HTRIMMING, trim, &ret);
955
956 break;
957
958 case V4L2_CID_TEST_PATTERN:
959 ret = imx283_update_test_pattern(imx283, ctrl->val);
960 break;
961
962 default:
963 dev_err(imx283->dev, "ctrl(id:0x%x, val:0x%x) is not handled\n",
964 ctrl->id, ctrl->val);
965 break;
966 }
967
968 pm_runtime_put(imx283->dev);
969
970 return ret;
971 }
972
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 42+ messages in thread