| Message ID | 20230530173000.3060865-3-dave.stevenson@raspberrypi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | imx258 improvements series | expand |
Hi Dave On Tue, May 30, 2023 at 06:29:41PM +0100, Dave Stevenson wrote: > The output image is defined as being 4208x3118 pixels in size. > Y_ADD_STA register was set to 0, and Y_ADD_END to 3118, giving > 3119 lines total. > > The datasheet lists a requirement for Y_ADD_STA to be a multiple > of a power of 2 depending on binning/scaling mode (2 for full pixel, > 4 for x2-bin/scale, 8 for (x2-bin)+(x2-subsample) or x4-bin, or 16 > for (x4-bin)+(x2-subsample)). > (Y_ADD_END – Y_ADD_STA + 1) also has to be a similar power of 2. > > The current configuration for the full res modes breaks that second > requirement, and we can't increase Y_ADD_STA to 1 to retain exactly > the same field of view as that then breaks the first requirement. > For the binned modes, they are worse off as 3118 is not a multiple of > 4. > > Increase the main mode to 4208x3120 so that it is the same FOV as the > binned modes, with Y_ADD_STA at 0. > Fix Y_ADD_STA and Y_ADD_END for the binned modes so that they meet the > sensor requirements. > > This does change the Bayer order as the default configuration is for > H&V flips to be enabled, so readout is from Y_STA_END to Y_ADD_STA, > and this patch has changed Y_STA_END. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> Good catch! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > drivers/media/i2c/imx258.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > index 6028579393b5..946b1a12971d 100644 > --- a/drivers/media/i2c/imx258.c > +++ b/drivers/media/i2c/imx258.c > @@ -111,7 +111,7 @@ struct imx258_mode { > struct imx258_reg_list reg_list; > }; > > -/* 4208x3118 needs 1267Mbps/lane, 4 lanes */ > +/* 4208x3120 needs 1267Mbps/lane, 4 lanes */ > static const struct imx258_reg mipi_data_rate_1267mbps[] = { > { 0x0301, 0x05 }, > { 0x0303, 0x02 }, > @@ -148,7 +148,7 @@ static const struct imx258_reg mipi_data_rate_640mbps[] = { > { 0x0823, 0x00 }, > }; > > -static const struct imx258_reg mode_4208x3118_regs[] = { > +static const struct imx258_reg mode_4208x3120_regs[] = { > { 0x0136, 0x13 }, > { 0x0137, 0x33 }, > { 0x3051, 0x00 }, > @@ -210,7 +210,7 @@ static const struct imx258_reg mode_4208x3118_regs[] = { > { 0x0348, 0x10 }, > { 0x0349, 0x6F }, > { 0x034A, 0x0C }, > - { 0x034B, 0x2E }, > + { 0x034B, 0x2F }, > { 0x0381, 0x01 }, > { 0x0383, 0x01 }, > { 0x0385, 0x01 }, > @@ -329,7 +329,7 @@ static const struct imx258_reg mode_2104_1560_regs[] = { > { 0x0348, 0x10 }, > { 0x0349, 0x6F }, > { 0x034A, 0x0C }, > - { 0x034B, 0x2E }, > + { 0x034B, 0x2F }, > { 0x0381, 0x01 }, > { 0x0383, 0x01 }, > { 0x0385, 0x01 }, > @@ -448,7 +448,7 @@ static const struct imx258_reg mode_1048_780_regs[] = { > { 0x0348, 0x10 }, > { 0x0349, 0x6F }, > { 0x034A, 0x0C }, > - { 0x034B, 0x2E }, > + { 0x034B, 0x2F }, > { 0x0381, 0x01 }, > { 0x0383, 0x01 }, > { 0x0385, 0x01 }, > @@ -562,12 +562,12 @@ static const struct imx258_link_freq_config link_freq_configs[] = { > static const struct imx258_mode supported_modes[] = { > { > .width = 4208, > - .height = 3118, > + .height = 3120, > .vts_def = IMX258_VTS_30FPS, > .vts_min = IMX258_VTS_30FPS, > .reg_list = { > - .num_of_regs = ARRAY_SIZE(mode_4208x3118_regs), > - .regs = mode_4208x3118_regs, > + .num_of_regs = ARRAY_SIZE(mode_4208x3120_regs), > + .regs = mode_4208x3120_regs, > }, > .link_freq_index = IMX258_LINK_FREQ_1267MBPS, > }, > @@ -710,7 +710,7 @@ static int imx258_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > /* Initialize try_fmt */ > try_fmt->width = supported_modes[0].width; > try_fmt->height = supported_modes[0].height; > - try_fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10; > + try_fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10; > try_fmt->field = V4L2_FIELD_NONE; > > return 0; > @@ -822,7 +822,7 @@ static int imx258_enum_mbus_code(struct v4l2_subdev *sd, > if (code->index > 0) > return -EINVAL; > > - code->code = MEDIA_BUS_FMT_SGRBG10_1X10; > + code->code = MEDIA_BUS_FMT_SBGGR10_1X10; > > return 0; > } > @@ -834,7 +834,7 @@ static int imx258_enum_frame_size(struct v4l2_subdev *sd, > if (fse->index >= ARRAY_SIZE(supported_modes)) > return -EINVAL; > > - if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10) > + if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10) > return -EINVAL; > > fse->min_width = supported_modes[fse->index].width; > @@ -850,7 +850,7 @@ static void imx258_update_pad_format(const struct imx258_mode *mode, > { > fmt->format.width = mode->width; > fmt->format.height = mode->height; > - fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10; > + fmt->format.code = MEDIA_BUS_FMT_SBGGR10_1X10; > fmt->format.field = V4L2_FIELD_NONE; > } > > @@ -898,7 +898,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd, > mutex_lock(&imx258->mutex); > > /* Only one raw bayer(GBRG) order is supported */ > - fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10; > + fmt->format.code = MEDIA_BUS_FMT_SBGGR10_1X10; > > mode = v4l2_find_nearest_size(supported_modes, > ARRAY_SIZE(supported_modes), width, height, > -- > 2.25.1 >
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c index 6028579393b5..946b1a12971d 100644 --- a/drivers/media/i2c/imx258.c +++ b/drivers/media/i2c/imx258.c @@ -111,7 +111,7 @@ struct imx258_mode { struct imx258_reg_list reg_list; }; -/* 4208x3118 needs 1267Mbps/lane, 4 lanes */ +/* 4208x3120 needs 1267Mbps/lane, 4 lanes */ static const struct imx258_reg mipi_data_rate_1267mbps[] = { { 0x0301, 0x05 }, { 0x0303, 0x02 }, @@ -148,7 +148,7 @@ static const struct imx258_reg mipi_data_rate_640mbps[] = { { 0x0823, 0x00 }, }; -static const struct imx258_reg mode_4208x3118_regs[] = { +static const struct imx258_reg mode_4208x3120_regs[] = { { 0x0136, 0x13 }, { 0x0137, 0x33 }, { 0x3051, 0x00 }, @@ -210,7 +210,7 @@ static const struct imx258_reg mode_4208x3118_regs[] = { { 0x0348, 0x10 }, { 0x0349, 0x6F }, { 0x034A, 0x0C }, - { 0x034B, 0x2E }, + { 0x034B, 0x2F }, { 0x0381, 0x01 }, { 0x0383, 0x01 }, { 0x0385, 0x01 }, @@ -329,7 +329,7 @@ static const struct imx258_reg mode_2104_1560_regs[] = { { 0x0348, 0x10 }, { 0x0349, 0x6F }, { 0x034A, 0x0C }, - { 0x034B, 0x2E }, + { 0x034B, 0x2F }, { 0x0381, 0x01 }, { 0x0383, 0x01 }, { 0x0385, 0x01 }, @@ -448,7 +448,7 @@ static const struct imx258_reg mode_1048_780_regs[] = { { 0x0348, 0x10 }, { 0x0349, 0x6F }, { 0x034A, 0x0C }, - { 0x034B, 0x2E }, + { 0x034B, 0x2F }, { 0x0381, 0x01 }, { 0x0383, 0x01 }, { 0x0385, 0x01 }, @@ -562,12 +562,12 @@ static const struct imx258_link_freq_config link_freq_configs[] = { static const struct imx258_mode supported_modes[] = { { .width = 4208, - .height = 3118, + .height = 3120, .vts_def = IMX258_VTS_30FPS, .vts_min = IMX258_VTS_30FPS, .reg_list = { - .num_of_regs = ARRAY_SIZE(mode_4208x3118_regs), - .regs = mode_4208x3118_regs, + .num_of_regs = ARRAY_SIZE(mode_4208x3120_regs), + .regs = mode_4208x3120_regs, }, .link_freq_index = IMX258_LINK_FREQ_1267MBPS, }, @@ -710,7 +710,7 @@ static int imx258_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) /* Initialize try_fmt */ try_fmt->width = supported_modes[0].width; try_fmt->height = supported_modes[0].height; - try_fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10; + try_fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10; try_fmt->field = V4L2_FIELD_NONE; return 0; @@ -822,7 +822,7 @@ static int imx258_enum_mbus_code(struct v4l2_subdev *sd, if (code->index > 0) return -EINVAL; - code->code = MEDIA_BUS_FMT_SGRBG10_1X10; + code->code = MEDIA_BUS_FMT_SBGGR10_1X10; return 0; } @@ -834,7 +834,7 @@ static int imx258_enum_frame_size(struct v4l2_subdev *sd, if (fse->index >= ARRAY_SIZE(supported_modes)) return -EINVAL; - if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10) + if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10) return -EINVAL; fse->min_width = supported_modes[fse->index].width; @@ -850,7 +850,7 @@ static void imx258_update_pad_format(const struct imx258_mode *mode, { fmt->format.width = mode->width; fmt->format.height = mode->height; - fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10; + fmt->format.code = MEDIA_BUS_FMT_SBGGR10_1X10; fmt->format.field = V4L2_FIELD_NONE; } @@ -898,7 +898,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd, mutex_lock(&imx258->mutex); /* Only one raw bayer(GBRG) order is supported */ - fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10; + fmt->format.code = MEDIA_BUS_FMT_SBGGR10_1X10; mode = v4l2_find_nearest_size(supported_modes, ARRAY_SIZE(supported_modes), width, height,
The output image is defined as being 4208x3118 pixels in size. Y_ADD_STA register was set to 0, and Y_ADD_END to 3118, giving 3119 lines total. The datasheet lists a requirement for Y_ADD_STA to be a multiple of a power of 2 depending on binning/scaling mode (2 for full pixel, 4 for x2-bin/scale, 8 for (x2-bin)+(x2-subsample) or x4-bin, or 16 for (x4-bin)+(x2-subsample)). (Y_ADD_END – Y_ADD_STA + 1) also has to be a similar power of 2. The current configuration for the full res modes breaks that second requirement, and we can't increase Y_ADD_STA to 1 to retain exactly the same field of view as that then breaks the first requirement. For the binned modes, they are worse off as 3118 is not a multiple of 4. Increase the main mode to 4208x3120 so that it is the same FOV as the binned modes, with Y_ADD_STA at 0. Fix Y_ADD_STA and Y_ADD_END for the binned modes so that they meet the sensor requirements. This does change the Bayer order as the default configuration is for H&V flips to be enabled, so readout is from Y_STA_END to Y_ADD_STA, and this patch has changed Y_STA_END. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- drivers/media/i2c/imx258.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)