On Mon, Jan 23, 2023 at 01:51:44PM +0100, Hans de Goede wrote:
> The fps, lines-per-frame and skip-frames values are the same for all
> resolutions, use defines for these.
>
> The ov2680_res_preview[] incorrectly sets fps to 60 for some low-res
> modes, this is incorrect with the current fixed (resolution independent)
> lines-per-frame value.
>
> Note this not drop the now no longer used fps, lines-per-frame and
> skip-frames struct ov2680_resolution members. The entire struct is going
> away in the next patches so that would just cause unnecessary changes.
Reviewed-by: Andy Shevchenko <andy@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> .../staging/media/atomisp/i2c/atomisp-ov2680.c | 16 ++++------------
> drivers/staging/media/atomisp/i2c/ov2680.h | 2 ++
> 2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index 6693f042f4f2..dee6eb3d8c63 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -685,11 +685,8 @@ static int ov2680_s_config(struct v4l2_subdev *sd,
> static int ov2680_g_frame_interval(struct v4l2_subdev *sd,
> struct v4l2_subdev_frame_interval *interval)
> {
> - struct ov2680_device *dev = to_ov2680_sensor(sd);
> -
> interval->interval.numerator = 1;
> - interval->interval.denominator = dev->res->fps;
> -
> + interval->interval.denominator = OV2680_FPS;
> return 0;
> }
>
> @@ -733,8 +730,8 @@ static int ov2680_enum_frame_interval(struct v4l2_subdev *sd,
> fie->which > V4L2_SUBDEV_FORMAT_ACTIVE)
> return -EINVAL;
>
> - fract.denominator = ov2680_res_preview[fie->index].fps;
> fract.numerator = 1;
> + fract.denominator = OV2680_FPS;
>
> fie->interval = fract;
>
> @@ -743,12 +740,7 @@ static int ov2680_enum_frame_interval(struct v4l2_subdev *sd,
>
> static int ov2680_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
> {
> - struct ov2680_device *dev = to_ov2680_sensor(sd);
> -
> - mutex_lock(&dev->input_lock);
> - *frames = dev->res->skip_frames;
> - mutex_unlock(&dev->input_lock);
> -
> + *frames = OV2680_SKIP_FRAMES;
> return 0;
> }
>
> @@ -792,7 +784,7 @@ static int ov2680_init_controls(struct ov2680_device *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 = sensor->res->lines_per_frame - OV2680_INTEGRATION_TIME_MARGIN;
> + int exp_max = OV2680_LINES_PER_FRAME - OV2680_INTEGRATION_TIME_MARGIN;
>
> v4l2_ctrl_handler_init(hdl, 4);
>
> diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
> index 9bbb34dd95a5..4c1d20f514b2 100644
> --- a/drivers/staging/media/atomisp/i2c/ov2680.h
> +++ b/drivers/staging/media/atomisp/i2c/ov2680.h
> @@ -38,6 +38,8 @@
> /* 1704 * 1294 * 30fps = 66MHz pixel clock */
> #define OV2680_PIXELS_PER_LINE 1704
> #define OV2680_LINES_PER_FRAME 1294
> +#define OV2680_FPS 30
> +#define OV2680_SKIP_FRAMES 3
>
> /* If possible send 16 extra rows / lines to the ISP as padding */
> #define OV2680_END_MARGIN 16
> --
> 2.39.0
>
--
With Best Regards,
Andy Shevchenko
