Hi Laurent,
thanks for putting these series together.
Am Mittwoch, 15. Februar 2023, 23:29:58 CET schrieb Laurent Pinchart:
> From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
>
> The driver exposed V4L2_CID_VBLANK as a read only control to allow
> for exposure calculations and determination of the frame rate.
>
> Convert to a read/write control so that the frame rate can be
> controlled.
> V4L2_CID_VBLANK also sets the limits for the exposure control,
> therefore exposure ranges have to be updated when vblank changes
> (either via s_ctrl, or via changing mode).
>
> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/imx290.c | 58 +++++++++++++++++++++++++++++++-------
> 1 file changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index f82fb05b6695..1ae01cd43efb 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -46,6 +46,7 @@
> #define IMX290_BLKLEVEL
IMX290_REG_16BIT(0x300a)
> #define IMX290_GAIN
IMX290_REG_8BIT(0x3014)
> #define IMX290_VMAX
IMX290_REG_24BIT(0x3018)
> +#define IMX290_VMAX_MAX 0x3ffff
> #define IMX290_HMAX
IMX290_REG_16BIT(0x301c)
> #define IMX290_HMAX_MAX 0xffff
> #define IMX290_SHS1
IMX290_REG_24BIT(0x3020)
> @@ -106,6 +107,9 @@
> #define IMX290_PGCTRL_THRU BIT(1)
> #define IMX290_PGCTRL_MODE(n) ((n) <<
4)
>
> +/* Number of lines by which exposure must be less than VMAX) */
I guess the ')' is a leftover. Despite that:
Acked-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
Best regards,
Alexander
> +#define IMX290_EXPOSURE_OFFSET 2
> +
> #define IMX290_VMAX_DEFAULT 1125
>
> #define IMX290_PIXEL_RATE 148500000
> @@ -222,6 +226,7 @@ struct imx290 {
> struct v4l2_ctrl *link_freq;
> struct v4l2_ctrl *hblank;
> struct v4l2_ctrl *vblank;
> + struct v4l2_ctrl *exposure;
> };
>
> static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
> @@ -235,7 +240,6 @@ static inline struct imx290 *to_imx290(struct
> v4l2_subdev *_sd)
>
> static const struct imx290_regval imx290_global_init_settings[] = {
> { IMX290_CTRL_07, IMX290_WINMODE_1080P },
> - { IMX290_VMAX, IMX290_VMAX_DEFAULT },
> { IMX290_EXTCK_FREQ, 0x2520 },
> { IMX290_WINWV_OB, 12 },
> { IMX290_WINPH, 0 },
> @@ -659,6 +663,16 @@ static int imx290_setup_format(struct imx290 *imx290,
> /*
> ---------------------------------------------------------------------------
> - * Controls
> */
> +static void imx290_exposure_update(struct imx290 *imx290,
> + const struct imx290_mode *mode)
> +{
> + unsigned int exposure_max;
> +
> + exposure_max = imx290->vblank->val + mode->height -
> + IMX290_EXPOSURE_OFFSET;
> + __v4l2_ctrl_modify_range(imx290->exposure, 1, exposure_max, 1,
> + exposure_max);
> +}
>
> static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> {
> @@ -666,7 +680,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> struct imx290, ctrls);
> const struct v4l2_mbus_framefmt *format;
> struct v4l2_subdev_state *state;
> - int ret = 0;
> + int ret = 0, vmax;
>
> /*
> * Return immediately for controls that don't need to be applied to
the
> @@ -675,6 +689,11 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
> return 0;
>
> + if (ctrl->id == V4L2_CID_VBLANK) {
> + /* Changing vblank changes the allowed range for exposure.
*/
> + imx290_exposure_update(imx290, imx290->current_mode);
> + }
> +
> /* V4L2 controls values will be applied only when power is already
up */
> if (!pm_runtime_get_if_in_use(imx290->dev))
> return 0;
> @@ -687,9 +706,23 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
> ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL);
> break;
>
> + case V4L2_CID_VBLANK:
> + ret = imx290_write(imx290, IMX290_VMAX,
> + ctrl->val + imx290->current_mode-
>height,
> + NULL);
> + /*
> + * Due to the way that exposure is programmed in this
sensor in
> + * relation to VMAX, we have to reprogramme it whenever
VMAX is
> + * changed.
> + * Update ctrl so that the V4L2_CID_EXPOSURE case can
refer to
> + * it.
> + */
> + ctrl = imx290->exposure;
> + fallthrough;
> case V4L2_CID_EXPOSURE:
> + vmax = imx290->vblank->val + imx290->current_mode->height;
> ret = imx290_write(imx290, IMX290_SHS1,
> - IMX290_VMAX_DEFAULT - ctrl->val -
1, NULL);
> + vmax - ctrl->val - 1, NULL);
> break;
>
> case V4L2_CID_TEST_PATTERN:
> @@ -746,13 +779,15 @@ static void imx290_ctrl_update(struct imx290 *imx290,
> {
> unsigned int hblank_min = mode->hmax_min - mode->width;
> unsigned int hblank_max = IMX290_HMAX_MAX - mode->width;
> - unsigned int vblank = IMX290_VMAX_DEFAULT - mode->height;
> + unsigned int vblank_min = IMX290_VMAX_DEFAULT - mode->height;
> + unsigned int vblank_max = IMX290_VMAX_MAX - mode->height;
>
> __v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
>
> __v4l2_ctrl_modify_range(imx290->hblank, hblank_min, hblank_max, 1,
> hblank_min);
> - __v4l2_ctrl_modify_range(imx290->vblank, vblank, vblank, 1, vblank);
> + __v4l2_ctrl_modify_range(imx290->vblank, vblank_min, vblank_max, 1,
> + vblank_min);
> }
>
> static int imx290_ctrl_init(struct imx290 *imx290)
> @@ -782,9 +817,13 @@ static int imx290_ctrl_init(struct imx290 *imx290)
> v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> V4L2_CID_ANALOGUE_GAIN, 0, 100, 1, 0);
>
> - v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> - V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2,
1,
> - IMX290_VMAX_DEFAULT - 2);
> + /*
> + * Correct range will be determined through imx290_ctrl_update
setting
> + * V4L2_CID_VBLANK.
> + */
> + imx290->exposure = v4l2_ctrl_new_std(&imx290->ctrls,
&imx290_ctrl_ops,
> + V4L2_CID_EXPOSURE, 1,
65535, 1,
> + 65535);
>
> /*
> * Set the link frequency, pixel rate, horizontal blanking and
vertical
> @@ -816,8 +855,6 @@ static int imx290_ctrl_init(struct imx290 *imx290)
>
> imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> V4L2_CID_VBLANK, 1, 1, 1,
1);
> - if (imx290->vblank)
> - imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops,
> &props);
> @@ -1003,6 +1040,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
> imx290->current_mode = mode;
>
> imx290_ctrl_update(imx290, &fmt->format, mode);
> + imx290_exposure_update(imx290, mode);
> }
>
> *format = fmt->format;
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
