Hi Hans,
Thanks for re-posting this.
On Mon, Apr 15, 2024 at 11:37:02AM +0200, Hans de Goede wrote:
> Add vblank control to allow changing the framerate /
> higher exposure values.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/media/i2c/ov2680.c | 47 ++++++++++++++++++++++++++++++++------
> 1 file changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> index 6c3d7862b2aa..d44e1934b25a 100644
> --- a/drivers/media/i2c/ov2680.c
> +++ b/drivers/media/i2c/ov2680.c
> @@ -75,6 +75,8 @@
> #define OV2680_ACTIVE_START_TOP 8
> #define OV2680_MIN_CROP_WIDTH 2
> #define OV2680_MIN_CROP_HEIGHT 2
> +#define OV2680_MIN_VBLANK 4
> +#define OV2680_MAX_VBLANK 0xffff
>
> /* Fixed pre-div of 1/2 */
> #define OV2680_PLL_PREDIV0 2
> @@ -84,7 +86,7 @@
>
> /* 66MHz pixel clock: 66MHz / 1704 * 1294 = 30fps */
> #define OV2680_PIXELS_PER_LINE 1704
> -#define OV2680_LINES_PER_FRAME 1294
> +#define OV2680_LINES_PER_FRAME_30FPS 1294
>
> /* Max exposure time is VTS - 8 */
> #define OV2680_INTEGRATION_TIME_MARGIN 8
> @@ -127,6 +129,7 @@ struct ov2680_ctrls {
> struct v4l2_ctrl *test_pattern;
> struct v4l2_ctrl *link_freq;
> struct v4l2_ctrl *pixel_rate;
> + struct v4l2_ctrl *vblank;
> };
>
> struct ov2680_mode {
> @@ -394,8 +397,7 @@ static int ov2680_set_mode(struct ov2680_dev *sensor)
> sensor->mode.v_output_size, &ret);
> cci_write(sensor->regmap, OV2680_REG_TIMING_HTS,
> OV2680_PIXELS_PER_LINE, &ret);
> - cci_write(sensor->regmap, OV2680_REG_TIMING_VTS,
> - OV2680_LINES_PER_FRAME, &ret);
> + /* VTS gets set by the vblank ctrl */
> 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);
> @@ -469,6 +471,15 @@ static int ov2680_exposure_set(struct ov2680_dev *sensor, u32 exp)
> NULL);
> }
>
> +static void ov2680_exposure_update_range(struct ov2680_dev *sensor)
> +{
> + int exp_max = sensor->mode.fmt.height + sensor->ctrls.vblank->val -
> + OV2680_INTEGRATION_TIME_MARGIN;
> +
> + __v4l2_ctrl_modify_range(sensor->ctrls.exposure, 0, exp_max,
> + 1, exp_max);
__v4l2_ctrl_modify_range() may fail.
> +}
> +
> static int ov2680_stream_enable(struct ov2680_dev *sensor)
> {
> int ret;
> @@ -635,7 +646,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_mbus_framefmt *try_fmt;
> const struct v4l2_rect *crop;
> unsigned int width, height;
> - int ret = 0;
> + int def, max, ret = 0;
>
> crop = __ov2680_get_pad_crop(sensor, sd_state, format->pad,
> format->which);
> @@ -664,6 +675,15 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
> sensor->mode.fmt = format->format;
> ov2680_calc_mode(sensor);
>
> + /* vblank range is height dependent adjust and reset to default */
> + max = OV2680_MAX_VBLANK - height;
> + def = OV2680_LINES_PER_FRAME_30FPS - height;
> + __v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV2680_MIN_VBLANK, max,
> + 1, def);
> + __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, def);
And so may __v4l2_ctrl_s_ctrl().
> + /* exposure range depends on vts which may have changed */
> + ov2680_exposure_update_range(sensor);
> +
> unlock:
> mutex_unlock(&sensor->lock);
>
> @@ -833,6 +853,10 @@ static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
> struct ov2680_dev *sensor = to_ov2680_dev(sd);
> int ret;
>
> + /* Update exposure range on vblank changes */
> + if (ctrl->id == V4L2_CID_VBLANK)
> + ov2680_exposure_update_range(sensor);
> +
> /* Only apply changes to the controls if the device is powered up */
> if (!pm_runtime_get_if_in_use(sensor->sd.dev)) {
> ov2680_set_bayer_order(sensor, &sensor->mode.fmt);
> @@ -855,6 +879,10 @@ static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_TEST_PATTERN:
> ret = ov2680_test_pattern_set(sensor, ctrl->val);
> break;
> + case V4L2_CID_VBLANK:
> + ret = cci_write(sensor->regmap, OV2680_REG_TIMING_VTS,
> + sensor->mode.fmt.height + ctrl->val, NULL);
> + break;
> default:
> ret = -EINVAL;
> break;
> @@ -913,8 +941,7 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor)
> const struct v4l2_ctrl_ops *ops = &ov2680_ctrl_ops;
> struct ov2680_ctrls *ctrls = &sensor->ctrls;
> struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> - int exp_max = OV2680_LINES_PER_FRAME - OV2680_INTEGRATION_TIME_MARGIN;
> - int ret = 0;
> + int def, max, ret = 0;
>
> v4l2_i2c_subdev_init(&sensor->sd, client, &ov2680_subdev_ops);
> sensor->sd.internal_ops = &ov2680_internal_ops;
> @@ -939,8 +966,9 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor)
> ARRAY_SIZE(test_pattern_menu) - 1,
> 0, 0, test_pattern_menu);
>
> + max = OV2680_LINES_PER_FRAME_30FPS - OV2680_INTEGRATION_TIME_MARGIN;
> ctrls->exposure = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE,
> - 0, exp_max, 1, exp_max);
> + 0, max, 1, max);
>
> ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN,
> 0, 1023, 1, 250);
> @@ -951,6 +979,11 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor)
> 0, sensor->pixel_rate,
> 1, sensor->pixel_rate);
>
> + max = OV2680_MAX_VBLANK - OV2680_DEFAULT_HEIGHT;
> + def = OV2680_LINES_PER_FRAME_30FPS - OV2680_DEFAULT_HEIGHT;
> + ctrls->vblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK,
> + OV2680_MIN_VBLANK, max, 1, def);
> +
> if (hdl->error) {
> ret = hdl->error;
> goto cleanup_entity;
--
Kind regards,
Sakari Ailus
