| Message ID | 20230627131830.54601-22-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov2680: Bugfixes + ACPI + selection(crop-tgt) API support | expand |
Hi Hans On 27/06/2023 15:18, Hans de Goede wrote: > Instead of using a long fixed register settings list for each resolution, > calculate the register settings based on the requested width + height. > > This is based on atomisp-ov2680 commit 0611888592df ("media: atomisp: > ov2680: Make setting the modes algorithm based"). > > This will allow future enhancements like adding hblank and vblank controls > and adding selection support. > > This also adds properly prgramming the ISP window and setting s/prgramming/programming This looks mostly good but I have one query down below. > the manual ISP window control bit in register 0x5708, this is > necessary for the hflip and vflip conrols to work properly. > > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/ov2680.c | 315 +++++++++++++++++-------------------- > 1 file changed, 143 insertions(+), 172 deletions(-) > > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > index 79789b9efadf..15e0b0ef5e8f 100644 > --- a/drivers/media/i2c/ov2680.c > +++ b/drivers/media/i2c/ov2680.c > @@ -38,28 +38,44 @@ > #define OV2680_REG_EXPOSURE_PK CCI_REG24(0x3500) > #define OV2680_REG_R_MANUAL CCI_REG8(0x3503) > #define OV2680_REG_GAIN_PK CCI_REG16(0x350a) > + > +#define OV2680_REG_SENSOR_CTRL_0A CCI_REG8(0x370a) > + > +#define OV2680_REG_HORIZONTAL_START CCI_REG16(0x3800) > +#define OV2680_REG_VERTICAL_START CCI_REG16(0x3802) > +#define OV2680_REG_HORIZONTAL_END CCI_REG16(0x3804) > +#define OV2680_REG_VERTICAL_END CCI_REG16(0x3806) > +#define OV2680_REG_HORIZONTAL_OUTPUT_SIZE CCI_REG16(0x3808) > +#define OV2680_REG_VERTICAL_OUTPUT_SIZE CCI_REG16(0x380a) > #define OV2680_REG_TIMING_HTS CCI_REG16(0x380c) > #define OV2680_REG_TIMING_VTS CCI_REG16(0x380e) > +#define OV2680_REG_ISP_X_WIN CCI_REG16(0x3810) > +#define OV2680_REG_ISP_Y_WIN CCI_REG16(0x3812) > +#define OV2680_REG_X_INC CCI_REG8(0x3814) > +#define OV2680_REG_Y_INC CCI_REG8(0x3815) > #define OV2680_REG_FORMAT1 CCI_REG8(0x3820) > #define OV2680_REG_FORMAT2 CCI_REG8(0x3821) > > #define OV2680_REG_ISP_CTRL00 CCI_REG8(0x5080) > > +#define OV2680_REG_X_WIN CCI_REG16(0x5704) > +#define OV2680_REG_Y_WIN CCI_REG16(0x5706) > + > #define OV2680_FRAME_RATE 30 > > -#define OV2680_WIDTH_MAX 1600 > -#define OV2680_HEIGHT_MAX 1200 > +#define OV2680_NATIVE_WIDTH 1616 > +#define OV2680_NATIVE_HEIGHT 1216 > + > +/* 66MHz pixel clock: 66MHz / 1704 * 1294 = 30fps */ > +#define OV2680_PIXELS_PER_LINE 1704 > +#define OV2680_LINES_PER_FRAME 1294 > + > +/* If possible send 16 extra rows / lines to the ISP as padding */ > +#define OV2680_END_MARGIN 16 > > #define OV2680_DEFAULT_WIDTH 800 > #define OV2680_DEFAULT_HEIGHT 600 > > -enum ov2680_mode_id { > - OV2680_MODE_QUXGA_800_600, > - OV2680_MODE_720P_1280_720, > - OV2680_MODE_UXGA_1600_1200, > - OV2680_MODE_MAX, > -}; > - > static const char * const ov2680_supply_name[] = { > "DOVDD", > "DVDD", > @@ -83,15 +99,6 @@ static const u8 ov2680_pll_multipliers[] = { > [OV2680_24_MHZ] = 55, > }; > > -struct ov2680_mode_info { > - const char *name; > - enum ov2680_mode_id id; > - u32 width; > - u32 height; > - const struct reg_sequence *reg_data; > - u32 reg_data_size; > -}; > - > struct ov2680_ctrls { > struct v4l2_ctrl_handler handler; > struct v4l2_ctrl *exposure; > @@ -104,6 +111,15 @@ struct ov2680_ctrls { > struct ov2680_mode { > struct v4l2_mbus_framefmt fmt; > struct v4l2_fract frame_interval; > + bool binning; > + u16 h_start; > + u16 v_start; > + u16 h_end; > + u16 v_end; > + u16 h_output_size; > + u16 v_output_size; > + u16 hts; > + u16 vts; > }; > > struct ov2680_dev { > @@ -125,8 +141,6 @@ struct ov2680_dev { > > struct ov2680_ctrls ctrls; > struct ov2680_mode mode; > - > - const struct ov2680_mode_info *current_mode; > }; > > static const char * const test_pattern_menu[] = { > @@ -144,136 +158,19 @@ static const int ov2680_hv_flip_bayer_order[] = { > MEDIA_BUS_FMT_SRGGB10_1X10, > }; > > -static const struct reg_sequence ov2680_setting_30fps_QUXGA_800_600[] = { > - /* Set PLL SP DIV to 1 for binning mode */ > - {0x3086, 0x01}, > - > - /* Sensor control register 0x0a to 0x23 for binning mode */ > - {0x370a, 0x23}, > - > - /* Set X and Y output size to 800x600 */ > - {0x3808, 0x03}, > - {0x3809, 0x20}, > - {0x380a, 0x02}, > - {0x380b, 0x58}, > - > - /* Set HTS + VTS to 1708x644 */ > - {0x380c, 0x06}, > - {0x380d, 0xac}, > - {0x380e, 0x02}, > - {0x380f, 0x84}, > - > - /* Set ISP WIN X and Y start to 4x4 */ > - {0x3811, 0x04}, > - {0x3813, 0x04}, > - > - /* Set X INC and Y INC for binning */ > - {0x3814, 0x31}, > - {0x3815, 0x31}, > - > - /* Initialize FORMAT1 to default/reset value (vflip disabled) */ > - {0x3820, 0xc0}, > +static const struct reg_sequence ov2680_global_setting[] = { > + /* R MANUAL set exposure and gain to manual (hw does not do auto) */ > + {0x3503, 0x03}, > > /* Set black level compensation range to 0 - 3 (default 0 - 11) */ > {0x4008, 0x00}, > {0x4009, 0x03}, > > - /* Set MIPI pclk period to 0x1e (default/reset is 0x18) */ > - {0x4837, 0x1e}, > - > - /* Initialize exposure to 0x4ee (overridden by the ctrl, drop this */ > - {0x3501, 0x4e}, > - {0x3502, 0xe0}, > - > - /* R MANUAL set exposure and gain to manual (hw does not do auto) */ > - {0x3503, 0x03}, > -}; > - > -static const struct reg_sequence ov2680_setting_30fps_720P_1280_720[] = { > - /* Set PLL SP DIV to 0 for not binning mode */ > - {0x3086, 0x00}, > - > - /* Set X and Y output size to 1280x720 */ > - {0x3808, 0x05}, > - {0x3809, 0x00}, > - {0x380a, 0x02}, > - {0x380b, 0xd0}, > - > - /* Set HTS + VTS to 1704x1294 */ > - {0x380c, 0x06}, > - {0x380d, 0xa8}, > - {0x380e, 0x05}, > - {0x380f, 0x0e}, > - > - /* Set ISP WIN X and Y start to 8x6 */ > - {0x3811, 0x08}, > - {0x3813, 0x06}, > - > - /* Set X INC and Y INC for non binning */ > - {0x3814, 0x11}, > - {0x3815, 0x11}, > - > - /* Initialize FORMAT1 to default/reset value (vflip disabled) */ > - {0x3820, 0xc0}, > - > - /* Set backlight compensation range start to 0 */ > - {0x4008, 0x00}, > -}; > - > -static const struct reg_sequence ov2680_setting_30fps_UXGA_1600_1200[] = { > - /* Set PLL SP DIV to 0 for not binning mode */ > - {0x3086, 0x00}, > - > - /* Initialize exposure to 0x4ee (overridden by the ctrl, drop this */ > - {0x3501, 0x4e}, > - {0x3502, 0xe0}, > - > - /* Set X and Y output size to 1600x1200 */ > - {0x3808, 0x06}, > - {0x3809, 0x40}, > - {0x380a, 0x04}, > - {0x380b, 0xb0}, > - > - /* Set HTS + VTS to 1704x1294 */ > - {0x380c, 0x06}, > - {0x380d, 0xa8}, > - {0x380e, 0x05}, > - {0x380f, 0x0e}, > - > - /* Set ISP WIN X and Y start to 0x0 */ > - {0x3811, 0x00}, > - {0x3813, 0x00}, > - > - /* Set X INC and Y INC for non binning */ > - {0x3814, 0x11}, > - {0x3815, 0x11}, > - > - /* Initialize FORMAT1 to default/reset value (vflip disabled) */ > - {0x3820, 0xc0}, > - > - /* Set backlight compensation range start to 0 */ > - {0x4008, 0x00}, > - > - /* Set MIPI pclk period to default/reset value of 0x18 */ > - {0x4837, 0x18} > -}; > - > -static const struct ov2680_mode_info ov2680_mode_init_data = { > - "mode_quxga_800_600", OV2680_MODE_QUXGA_800_600, 800, 600, > - ov2680_setting_30fps_QUXGA_800_600, > - ARRAY_SIZE(ov2680_setting_30fps_QUXGA_800_600), > -}; > - > -static const struct ov2680_mode_info ov2680_mode_data[OV2680_MODE_MAX] = { > - {"mode_quxga_800_600", OV2680_MODE_QUXGA_800_600, > - 800, 600, ov2680_setting_30fps_QUXGA_800_600, > - ARRAY_SIZE(ov2680_setting_30fps_QUXGA_800_600)}, > - {"mode_720p_1280_720", OV2680_MODE_720P_1280_720, > - 1280, 720, ov2680_setting_30fps_720P_1280_720, > - ARRAY_SIZE(ov2680_setting_30fps_720P_1280_720)}, > - {"mode_uxga_1600_1200", OV2680_MODE_UXGA_1600_1200, > - 1600, 1200, ov2680_setting_30fps_UXGA_1600_1200, > - ARRAY_SIZE(ov2680_setting_30fps_UXGA_1600_1200)}, > + /* > + * Window CONTROL 0x00 -> 0x01, enable manual window control, > + * this is necessary for full size flip and mirror support. > + */ > + {0x5708, 0x01}, > }; > > static struct ov2680_dev *to_ov2680_dev(struct v4l2_subdev *sd) > @@ -331,6 +228,85 @@ static void ov2680_fill_format(struct ov2680_dev *sensor, > ov2680_set_bayer_order(sensor, fmt); > } > > +static void ov2680_calc_mode(struct ov2680_dev *sensor) > +{ > + int width = sensor->mode.fmt.width; > + int height = sensor->mode.fmt.height; > + int orig_width = width; > + int orig_height = height; > + > + if (width <= (OV2680_NATIVE_WIDTH / 2) && > + height <= (OV2680_NATIVE_HEIGHT / 2)) { > + sensor->mode.binning = true; > + width *= 2; > + height *= 2; > + } else { > + sensor->mode.binning = false; > + } > + > + sensor->mode.h_start = ((OV2680_NATIVE_WIDTH - width) / 2) & ~1; > + sensor->mode.v_start = ((OV2680_NATIVE_HEIGHT - height) / 2) & ~1; > + sensor->mode.h_end = > + min(sensor->mode.h_start + width + OV2680_END_MARGIN - 1, > + OV2680_NATIVE_WIDTH - 1); > + sensor->mode.v_end = > + min(sensor->mode.v_start + height + OV2680_END_MARGIN - 1, > + OV2680_NATIVE_HEIGHT - 1); > + sensor->mode.h_output_size = orig_width; > + sensor->mode.v_output_size = orig_height; > + sensor->mode.hts = OV2680_PIXELS_PER_LINE; > + sensor->mode.vts = OV2680_LINES_PER_FRAME; > +} > + > +static int ov2680_set_mode(struct ov2680_dev *sensor) > +{ > + u8 sensor_ctrl_0a, inc, fmt1, fmt2; > + int ret = 0; > + > + if (sensor->mode.binning) { > + sensor_ctrl_0a = 0x23; > + inc = 0x31; > + fmt1 = 0xc2; > + fmt2 = 0x01; > + } else { > + sensor_ctrl_0a = 0x21; > + inc = 0x11; > + fmt1 = 0xc0; > + fmt2 = 0x00; > + } > + > + cci_write(sensor->regmap, OV2680_REG_SENSOR_CTRL_0A, > + sensor_ctrl_0a, &ret); > + cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_START, > + sensor->mode.h_start, &ret); > + cci_write(sensor->regmap, OV2680_REG_VERTICAL_START, > + sensor->mode.v_start, &ret); > + cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_END, > + sensor->mode.h_end, &ret); > + cci_write(sensor->regmap, OV2680_REG_VERTICAL_END, > + sensor->mode.v_end, &ret); > + cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_OUTPUT_SIZE, > + sensor->mode.h_output_size, &ret); > + cci_write(sensor->regmap, OV2680_REG_VERTICAL_OUTPUT_SIZE, > + sensor->mode.v_output_size, &ret); > + cci_write(sensor->regmap, OV2680_REG_TIMING_HTS, > + sensor->mode.hts, &ret); > + cci_write(sensor->regmap, OV2680_REG_TIMING_VTS, > + sensor->mode.vts, &ret); > + cci_write(sensor->regmap, OV2680_REG_ISP_X_WIN, 0, &ret); > + cci_write(sensor->regmap, OV2680_REG_ISP_Y_WIN, 0, &ret); > + cci_write(sensor->regmap, OV2680_REG_X_INC, inc, &ret); > + cci_write(sensor->regmap, OV2680_REG_Y_INC, inc, &ret); > + cci_write(sensor->regmap, OV2680_REG_X_WIN, > + sensor->mode.h_output_size, &ret); > + cci_write(sensor->regmap, OV2680_REG_Y_WIN, > + sensor->mode.v_output_size, &ret); > + cci_write(sensor->regmap, OV2680_REG_FORMAT1, fmt1, &ret); > + cci_write(sensor->regmap, OV2680_REG_FORMAT2, fmt2, &ret); > + > + return ret; > +} > + > static int ov2680_set_vflip(struct ov2680_dev *sensor, s32 val) > { > int ret; > @@ -400,14 +376,12 @@ static int ov2680_stream_enable(struct ov2680_dev *sensor) > return ret; > > ret = regmap_multi_reg_write(sensor->regmap, > - ov2680_mode_init_data.reg_data, > - ov2680_mode_init_data.reg_data_size); > + ov2680_global_setting, > + ARRAY_SIZE(ov2680_global_setting)); > if (ret < 0) > return ret; > > - ret = regmap_multi_reg_write(sensor->regmap, > - sensor->current_mode->reg_data, > - sensor->current_mode->reg_data_size); > + ret = ov2680_set_mode(sensor); > if (ret < 0) > return ret; > > @@ -557,21 +531,18 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, > { > struct ov2680_dev *sensor = to_ov2680_dev(sd); > struct v4l2_mbus_framefmt *try_fmt; > - const struct ov2680_mode_info *mode; > + unsigned int width, height; > int ret = 0; > > if (format->pad != 0) > return -EINVAL; > > - mode = v4l2_find_nearest_size(ov2680_mode_data, > - ARRAY_SIZE(ov2680_mode_data), > - width, height, > - format->format.width, > - format->format.height); > - if (!mode) > - return -EINVAL; > + width = min_t(unsigned int, ALIGN(format->format.width, 2), > + OV2680_NATIVE_WIDTH); > + height = min_t(unsigned int, ALIGN(format->format.height, 2), > + OV2680_NATIVE_HEIGHT); > > - ov2680_fill_format(sensor, &format->format, mode->width, mode->height); > + ov2680_fill_format(sensor, &format->format, width, height); > > if (format->which == V4L2_SUBDEV_FORMAT_TRY) { > try_fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); > @@ -586,8 +557,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, > goto unlock; > } > > - sensor->current_mode = mode; > sensor->mode.fmt = format->format; > + ov2680_calc_mode(sensor); > > unlock: > mutex_unlock(&sensor->lock); > @@ -609,15 +580,20 @@ static int ov2680_enum_frame_size(struct v4l2_subdev *sd, > struct v4l2_subdev_state *sd_state, > struct v4l2_subdev_frame_size_enum *fse) > { > - int index = fse->index; > + static const struct v4l2_frmsize_discrete ov2680_frame_sizes[] = { > + { 1600, 1200 }, > + { 1280, 720 }, > + { 800, 600 }, > + }; > + u32 index = fse->index; > > - if (index >= OV2680_MODE_MAX || index < 0) > + if (index >= ARRAY_SIZE(ov2680_frame_sizes)) > return -EINVAL; > > - fse->min_width = ov2680_mode_data[index].width; > - fse->min_height = ov2680_mode_data[index].height; > - fse->max_width = ov2680_mode_data[index].width; > - fse->max_height = ov2680_mode_data[index].height; > + fse->min_width = ov2680_frame_sizes[index].width; > + fse->min_height = ov2680_frame_sizes[index].height; > + fse->max_width = ov2680_frame_sizes[index].width; > + fse->max_height = ov2680_frame_sizes[index].height; > > return 0; > } Unless I'm missing something, .set_fmt() will let you set any arbitrary frame size you like within the bounds of the ov2680's pixel array. That's good, but why then should this callback only report the three discrete sizes? > @@ -700,19 +676,14 @@ static const struct v4l2_subdev_ops ov2680_subdev_ops = { > > static int ov2680_mode_init(struct ov2680_dev *sensor) > { > - const struct ov2680_mode_info *init_mode; > - > /* set initial mode */ > ov2680_fill_format(sensor, &sensor->mode.fmt, > OV2680_DEFAULT_WIDTH, OV2680_DEFAULT_HEIGHT); > + ov2680_calc_mode(sensor); > > sensor->mode.frame_interval.denominator = OV2680_FRAME_RATE; > sensor->mode.frame_interval.numerator = 1; > > - init_mode = &ov2680_mode_init_data; > - > - sensor->current_mode = init_mode; > - > return 0; > } >
Hi, On 7/3/23 10:50, Dan Scally wrote: > Hi Hans > > On 27/06/2023 15:18, Hans de Goede wrote: >> Instead of using a long fixed register settings list for each resolution, >> calculate the register settings based on the requested width + height. >> >> This is based on atomisp-ov2680 commit 0611888592df ("media: atomisp: >> ov2680: Make setting the modes algorithm based"). >> >> This will allow future enhancements like adding hblank and vblank controls >> and adding selection support. >> >> This also adds properly prgramming the ISP window and setting > > > s/prgramming/programming > > > This looks mostly good but I have one query down below. <snip> >> @@ -609,15 +580,20 @@ static int ov2680_enum_frame_size(struct v4l2_subdev *sd, >> struct v4l2_subdev_state *sd_state, >> struct v4l2_subdev_frame_size_enum *fse) >> { >> - int index = fse->index; >> + static const struct v4l2_frmsize_discrete ov2680_frame_sizes[] = { >> + { 1600, 1200 }, >> + { 1280, 720 }, >> + { 800, 600 }, >> + }; >> + u32 index = fse->index; >> - if (index >= OV2680_MODE_MAX || index < 0) >> + if (index >= ARRAY_SIZE(ov2680_frame_sizes)) >> return -EINVAL; >> - fse->min_width = ov2680_mode_data[index].width; >> - fse->min_height = ov2680_mode_data[index].height; >> - fse->max_width = ov2680_mode_data[index].width; >> - fse->max_height = ov2680_mode_data[index].height; >> + fse->min_width = ov2680_frame_sizes[index].width; >> + fse->min_height = ov2680_frame_sizes[index].height; >> + fse->max_width = ov2680_frame_sizes[index].width; >> + fse->max_height = ov2680_frame_sizes[index].height; >> return 0; >> } > > > Unless I'm missing something, .set_fmt() will let you set any arbitrary frame size you like within the bounds of the ov2680's pixel array. That's good, but why then should this callback only report the three discrete sizes? The later added selection (crop-target) support makes it possible for consumers of this driver to find out the real resolution + explicitly set a crop, rather then doing this through s_fmt. Just like how the ov5693 driver is doing. The ov5693 driver just lists the full + binned size in ov2680_enum_frame_size(). I was a bit reluctant to do this here to break any potential existing users of the drivers, but : 1. Given the poor state of the driver before I doubt that there are any actual users 2. Jacopo has asked me to change the gain control CID which is also an API break I think we might just as well go ahead here and only list the full + binned sizes like the ov5693 code does. So I'll go an update that for v4. Regards, Hans
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c index 79789b9efadf..15e0b0ef5e8f 100644 --- a/drivers/media/i2c/ov2680.c +++ b/drivers/media/i2c/ov2680.c @@ -38,28 +38,44 @@ #define OV2680_REG_EXPOSURE_PK CCI_REG24(0x3500) #define OV2680_REG_R_MANUAL CCI_REG8(0x3503) #define OV2680_REG_GAIN_PK CCI_REG16(0x350a) + +#define OV2680_REG_SENSOR_CTRL_0A CCI_REG8(0x370a) + +#define OV2680_REG_HORIZONTAL_START CCI_REG16(0x3800) +#define OV2680_REG_VERTICAL_START CCI_REG16(0x3802) +#define OV2680_REG_HORIZONTAL_END CCI_REG16(0x3804) +#define OV2680_REG_VERTICAL_END CCI_REG16(0x3806) +#define OV2680_REG_HORIZONTAL_OUTPUT_SIZE CCI_REG16(0x3808) +#define OV2680_REG_VERTICAL_OUTPUT_SIZE CCI_REG16(0x380a) #define OV2680_REG_TIMING_HTS CCI_REG16(0x380c) #define OV2680_REG_TIMING_VTS CCI_REG16(0x380e) +#define OV2680_REG_ISP_X_WIN CCI_REG16(0x3810) +#define OV2680_REG_ISP_Y_WIN CCI_REG16(0x3812) +#define OV2680_REG_X_INC CCI_REG8(0x3814) +#define OV2680_REG_Y_INC CCI_REG8(0x3815) #define OV2680_REG_FORMAT1 CCI_REG8(0x3820) #define OV2680_REG_FORMAT2 CCI_REG8(0x3821) #define OV2680_REG_ISP_CTRL00 CCI_REG8(0x5080) +#define OV2680_REG_X_WIN CCI_REG16(0x5704) +#define OV2680_REG_Y_WIN CCI_REG16(0x5706) + #define OV2680_FRAME_RATE 30 -#define OV2680_WIDTH_MAX 1600 -#define OV2680_HEIGHT_MAX 1200 +#define OV2680_NATIVE_WIDTH 1616 +#define OV2680_NATIVE_HEIGHT 1216 + +/* 66MHz pixel clock: 66MHz / 1704 * 1294 = 30fps */ +#define OV2680_PIXELS_PER_LINE 1704 +#define OV2680_LINES_PER_FRAME 1294 + +/* If possible send 16 extra rows / lines to the ISP as padding */ +#define OV2680_END_MARGIN 16 #define OV2680_DEFAULT_WIDTH 800 #define OV2680_DEFAULT_HEIGHT 600 -enum ov2680_mode_id { - OV2680_MODE_QUXGA_800_600, - OV2680_MODE_720P_1280_720, - OV2680_MODE_UXGA_1600_1200, - OV2680_MODE_MAX, -}; - static const char * const ov2680_supply_name[] = { "DOVDD", "DVDD", @@ -83,15 +99,6 @@ static const u8 ov2680_pll_multipliers[] = { [OV2680_24_MHZ] = 55, }; -struct ov2680_mode_info { - const char *name; - enum ov2680_mode_id id; - u32 width; - u32 height; - const struct reg_sequence *reg_data; - u32 reg_data_size; -}; - struct ov2680_ctrls { struct v4l2_ctrl_handler handler; struct v4l2_ctrl *exposure; @@ -104,6 +111,15 @@ struct ov2680_ctrls { struct ov2680_mode { struct v4l2_mbus_framefmt fmt; struct v4l2_fract frame_interval; + bool binning; + u16 h_start; + u16 v_start; + u16 h_end; + u16 v_end; + u16 h_output_size; + u16 v_output_size; + u16 hts; + u16 vts; }; struct ov2680_dev { @@ -125,8 +141,6 @@ struct ov2680_dev { struct ov2680_ctrls ctrls; struct ov2680_mode mode; - - const struct ov2680_mode_info *current_mode; }; static const char * const test_pattern_menu[] = { @@ -144,136 +158,19 @@ static const int ov2680_hv_flip_bayer_order[] = { MEDIA_BUS_FMT_SRGGB10_1X10, }; -static const struct reg_sequence ov2680_setting_30fps_QUXGA_800_600[] = { - /* Set PLL SP DIV to 1 for binning mode */ - {0x3086, 0x01}, - - /* Sensor control register 0x0a to 0x23 for binning mode */ - {0x370a, 0x23}, - - /* Set X and Y output size to 800x600 */ - {0x3808, 0x03}, - {0x3809, 0x20}, - {0x380a, 0x02}, - {0x380b, 0x58}, - - /* Set HTS + VTS to 1708x644 */ - {0x380c, 0x06}, - {0x380d, 0xac}, - {0x380e, 0x02}, - {0x380f, 0x84}, - - /* Set ISP WIN X and Y start to 4x4 */ - {0x3811, 0x04}, - {0x3813, 0x04}, - - /* Set X INC and Y INC for binning */ - {0x3814, 0x31}, - {0x3815, 0x31}, - - /* Initialize FORMAT1 to default/reset value (vflip disabled) */ - {0x3820, 0xc0}, +static const struct reg_sequence ov2680_global_setting[] = { + /* R MANUAL set exposure and gain to manual (hw does not do auto) */ + {0x3503, 0x03}, /* Set black level compensation range to 0 - 3 (default 0 - 11) */ {0x4008, 0x00}, {0x4009, 0x03}, - /* Set MIPI pclk period to 0x1e (default/reset is 0x18) */ - {0x4837, 0x1e}, - - /* Initialize exposure to 0x4ee (overridden by the ctrl, drop this */ - {0x3501, 0x4e}, - {0x3502, 0xe0}, - - /* R MANUAL set exposure and gain to manual (hw does not do auto) */ - {0x3503, 0x03}, -}; - -static const struct reg_sequence ov2680_setting_30fps_720P_1280_720[] = { - /* Set PLL SP DIV to 0 for not binning mode */ - {0x3086, 0x00}, - - /* Set X and Y output size to 1280x720 */ - {0x3808, 0x05}, - {0x3809, 0x00}, - {0x380a, 0x02}, - {0x380b, 0xd0}, - - /* Set HTS + VTS to 1704x1294 */ - {0x380c, 0x06}, - {0x380d, 0xa8}, - {0x380e, 0x05}, - {0x380f, 0x0e}, - - /* Set ISP WIN X and Y start to 8x6 */ - {0x3811, 0x08}, - {0x3813, 0x06}, - - /* Set X INC and Y INC for non binning */ - {0x3814, 0x11}, - {0x3815, 0x11}, - - /* Initialize FORMAT1 to default/reset value (vflip disabled) */ - {0x3820, 0xc0}, - - /* Set backlight compensation range start to 0 */ - {0x4008, 0x00}, -}; - -static const struct reg_sequence ov2680_setting_30fps_UXGA_1600_1200[] = { - /* Set PLL SP DIV to 0 for not binning mode */ - {0x3086, 0x00}, - - /* Initialize exposure to 0x4ee (overridden by the ctrl, drop this */ - {0x3501, 0x4e}, - {0x3502, 0xe0}, - - /* Set X and Y output size to 1600x1200 */ - {0x3808, 0x06}, - {0x3809, 0x40}, - {0x380a, 0x04}, - {0x380b, 0xb0}, - - /* Set HTS + VTS to 1704x1294 */ - {0x380c, 0x06}, - {0x380d, 0xa8}, - {0x380e, 0x05}, - {0x380f, 0x0e}, - - /* Set ISP WIN X and Y start to 0x0 */ - {0x3811, 0x00}, - {0x3813, 0x00}, - - /* Set X INC and Y INC for non binning */ - {0x3814, 0x11}, - {0x3815, 0x11}, - - /* Initialize FORMAT1 to default/reset value (vflip disabled) */ - {0x3820, 0xc0}, - - /* Set backlight compensation range start to 0 */ - {0x4008, 0x00}, - - /* Set MIPI pclk period to default/reset value of 0x18 */ - {0x4837, 0x18} -}; - -static const struct ov2680_mode_info ov2680_mode_init_data = { - "mode_quxga_800_600", OV2680_MODE_QUXGA_800_600, 800, 600, - ov2680_setting_30fps_QUXGA_800_600, - ARRAY_SIZE(ov2680_setting_30fps_QUXGA_800_600), -}; - -static const struct ov2680_mode_info ov2680_mode_data[OV2680_MODE_MAX] = { - {"mode_quxga_800_600", OV2680_MODE_QUXGA_800_600, - 800, 600, ov2680_setting_30fps_QUXGA_800_600, - ARRAY_SIZE(ov2680_setting_30fps_QUXGA_800_600)}, - {"mode_720p_1280_720", OV2680_MODE_720P_1280_720, - 1280, 720, ov2680_setting_30fps_720P_1280_720, - ARRAY_SIZE(ov2680_setting_30fps_720P_1280_720)}, - {"mode_uxga_1600_1200", OV2680_MODE_UXGA_1600_1200, - 1600, 1200, ov2680_setting_30fps_UXGA_1600_1200, - ARRAY_SIZE(ov2680_setting_30fps_UXGA_1600_1200)}, + /* + * Window CONTROL 0x00 -> 0x01, enable manual window control, + * this is necessary for full size flip and mirror support. + */ + {0x5708, 0x01}, }; static struct ov2680_dev *to_ov2680_dev(struct v4l2_subdev *sd) @@ -331,6 +228,85 @@ static void ov2680_fill_format(struct ov2680_dev *sensor, ov2680_set_bayer_order(sensor, fmt); } +static void ov2680_calc_mode(struct ov2680_dev *sensor) +{ + int width = sensor->mode.fmt.width; + int height = sensor->mode.fmt.height; + int orig_width = width; + int orig_height = height; + + if (width <= (OV2680_NATIVE_WIDTH / 2) && + height <= (OV2680_NATIVE_HEIGHT / 2)) { + sensor->mode.binning = true; + width *= 2; + height *= 2; + } else { + sensor->mode.binning = false; + } + + sensor->mode.h_start = ((OV2680_NATIVE_WIDTH - width) / 2) & ~1; + sensor->mode.v_start = ((OV2680_NATIVE_HEIGHT - height) / 2) & ~1; + sensor->mode.h_end = + min(sensor->mode.h_start + width + OV2680_END_MARGIN - 1, + OV2680_NATIVE_WIDTH - 1); + sensor->mode.v_end = + min(sensor->mode.v_start + height + OV2680_END_MARGIN - 1, + OV2680_NATIVE_HEIGHT - 1); + sensor->mode.h_output_size = orig_width; + sensor->mode.v_output_size = orig_height; + sensor->mode.hts = OV2680_PIXELS_PER_LINE; + sensor->mode.vts = OV2680_LINES_PER_FRAME; +} + +static int ov2680_set_mode(struct ov2680_dev *sensor) +{ + u8 sensor_ctrl_0a, inc, fmt1, fmt2; + int ret = 0; + + if (sensor->mode.binning) { + sensor_ctrl_0a = 0x23; + inc = 0x31; + fmt1 = 0xc2; + fmt2 = 0x01; + } else { + sensor_ctrl_0a = 0x21; + inc = 0x11; + fmt1 = 0xc0; + fmt2 = 0x00; + } + + cci_write(sensor->regmap, OV2680_REG_SENSOR_CTRL_0A, + sensor_ctrl_0a, &ret); + cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_START, + sensor->mode.h_start, &ret); + cci_write(sensor->regmap, OV2680_REG_VERTICAL_START, + sensor->mode.v_start, &ret); + cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_END, + sensor->mode.h_end, &ret); + cci_write(sensor->regmap, OV2680_REG_VERTICAL_END, + sensor->mode.v_end, &ret); + cci_write(sensor->regmap, OV2680_REG_HORIZONTAL_OUTPUT_SIZE, + sensor->mode.h_output_size, &ret); + cci_write(sensor->regmap, OV2680_REG_VERTICAL_OUTPUT_SIZE, + sensor->mode.v_output_size, &ret); + cci_write(sensor->regmap, OV2680_REG_TIMING_HTS, + sensor->mode.hts, &ret); + cci_write(sensor->regmap, OV2680_REG_TIMING_VTS, + sensor->mode.vts, &ret); + cci_write(sensor->regmap, OV2680_REG_ISP_X_WIN, 0, &ret); + cci_write(sensor->regmap, OV2680_REG_ISP_Y_WIN, 0, &ret); + cci_write(sensor->regmap, OV2680_REG_X_INC, inc, &ret); + cci_write(sensor->regmap, OV2680_REG_Y_INC, inc, &ret); + cci_write(sensor->regmap, OV2680_REG_X_WIN, + sensor->mode.h_output_size, &ret); + cci_write(sensor->regmap, OV2680_REG_Y_WIN, + sensor->mode.v_output_size, &ret); + cci_write(sensor->regmap, OV2680_REG_FORMAT1, fmt1, &ret); + cci_write(sensor->regmap, OV2680_REG_FORMAT2, fmt2, &ret); + + return ret; +} + static int ov2680_set_vflip(struct ov2680_dev *sensor, s32 val) { int ret; @@ -400,14 +376,12 @@ static int ov2680_stream_enable(struct ov2680_dev *sensor) return ret; ret = regmap_multi_reg_write(sensor->regmap, - ov2680_mode_init_data.reg_data, - ov2680_mode_init_data.reg_data_size); + ov2680_global_setting, + ARRAY_SIZE(ov2680_global_setting)); if (ret < 0) return ret; - ret = regmap_multi_reg_write(sensor->regmap, - sensor->current_mode->reg_data, - sensor->current_mode->reg_data_size); + ret = ov2680_set_mode(sensor); if (ret < 0) return ret; @@ -557,21 +531,18 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, { struct ov2680_dev *sensor = to_ov2680_dev(sd); struct v4l2_mbus_framefmt *try_fmt; - const struct ov2680_mode_info *mode; + unsigned int width, height; int ret = 0; if (format->pad != 0) return -EINVAL; - mode = v4l2_find_nearest_size(ov2680_mode_data, - ARRAY_SIZE(ov2680_mode_data), - width, height, - format->format.width, - format->format.height); - if (!mode) - return -EINVAL; + width = min_t(unsigned int, ALIGN(format->format.width, 2), + OV2680_NATIVE_WIDTH); + height = min_t(unsigned int, ALIGN(format->format.height, 2), + OV2680_NATIVE_HEIGHT); - ov2680_fill_format(sensor, &format->format, mode->width, mode->height); + ov2680_fill_format(sensor, &format->format, width, height); if (format->which == V4L2_SUBDEV_FORMAT_TRY) { try_fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); @@ -586,8 +557,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, goto unlock; } - sensor->current_mode = mode; sensor->mode.fmt = format->format; + ov2680_calc_mode(sensor); unlock: mutex_unlock(&sensor->lock); @@ -609,15 +580,20 @@ static int ov2680_enum_frame_size(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_frame_size_enum *fse) { - int index = fse->index; + static const struct v4l2_frmsize_discrete ov2680_frame_sizes[] = { + { 1600, 1200 }, + { 1280, 720 }, + { 800, 600 }, + }; + u32 index = fse->index; - if (index >= OV2680_MODE_MAX || index < 0) + if (index >= ARRAY_SIZE(ov2680_frame_sizes)) return -EINVAL; - fse->min_width = ov2680_mode_data[index].width; - fse->min_height = ov2680_mode_data[index].height; - fse->max_width = ov2680_mode_data[index].width; - fse->max_height = ov2680_mode_data[index].height; + fse->min_width = ov2680_frame_sizes[index].width; + fse->min_height = ov2680_frame_sizes[index].height; + fse->max_width = ov2680_frame_sizes[index].width; + fse->max_height = ov2680_frame_sizes[index].height; return 0; } @@ -700,19 +676,14 @@ static const struct v4l2_subdev_ops ov2680_subdev_ops = { static int ov2680_mode_init(struct ov2680_dev *sensor) { - const struct ov2680_mode_info *init_mode; - /* set initial mode */ ov2680_fill_format(sensor, &sensor->mode.fmt, OV2680_DEFAULT_WIDTH, OV2680_DEFAULT_HEIGHT); + ov2680_calc_mode(sensor); sensor->mode.frame_interval.denominator = OV2680_FRAME_RATE; sensor->mode.frame_interval.numerator = 1; - init_mode = &ov2680_mode_init_data; - - sensor->current_mode = init_mode; - return 0; }
Instead of using a long fixed register settings list for each resolution, calculate the register settings based on the requested width + height. This is based on atomisp-ov2680 commit 0611888592df ("media: atomisp: ov2680: Make setting the modes algorithm based"). This will allow future enhancements like adding hblank and vblank controls and adding selection support. This also adds properly prgramming the ISP window and setting the manual ISP window control bit in register 0x5708, this is necessary for the hflip and vflip conrols to work properly. Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/ov2680.c | 315 +++++++++++++++++-------------------- 1 file changed, 143 insertions(+), 172 deletions(-)