Hi Dave
On Tue, May 30, 2023 at 06:29:53PM +0100, Dave Stevenson wrote:
> The sensor has a register CIT_LSHIFT which extends the exposure
> and frame times by the specified power of 2 for longer
> exposure times.
>
> Add support for this by configuring this register via V4L2_CID_VBLANK
> and extending the V4L2_CID_EXPOSURE range accordingly.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/imx258.c | 38 ++++++++++++++++++++++++++++++++------
> 1 file changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index f5199e3243e8..1e424058fcb9 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -69,6 +69,10 @@
> #define IMX258_HDR_RATIO_STEP 1
> #define IMX258_HDR_RATIO_DEFAULT 0x0
>
> +/* Long exposure multiplier */
> +#define IMX258_LONG_EXP_SHIFT_MAX 7
> +#define IMX258_LONG_EXP_SHIFT_REG 0x3002
> +
> /* Test Pattern Control */
> #define IMX258_REG_TEST_PATTERN 0x0600
>
> @@ -629,6 +633,8 @@ struct imx258 {
> struct v4l2_ctrl *vblank;
> struct v4l2_ctrl *hblank;
> struct v4l2_ctrl *exposure;
> + /* Current long exposure factor in use. Set through V4L2_CID_VBLANK */
> + unsigned int long_exp_shift;
>
> /* Current mode */
> const struct imx258_mode *cur_mode;
> @@ -793,6 +799,26 @@ static void imx258_adjust_exposure_range(struct imx258 *imx258)
> exposure_def);
> }
>
> +static int imx258_set_frame_length(struct imx258 *imx258, unsigned int val)
> +{
> + int ret;
> +
> + imx258->long_exp_shift = 0;
> +
> + while (val > IMX258_VTS_MAX) {
> + imx258->long_exp_shift++;
> + val >>= 1;
> + }
> +
> + ret = imx258_write_reg(imx258, IMX258_REG_VTS,
> + IMX258_REG_VALUE_16BIT, val);
> + if (ret)
> + return ret;
> +
> + return imx258_write_reg(imx258, IMX258_LONG_EXP_SHIFT_REG,
> + IMX258_REG_VALUE_08BIT, imx258->long_exp_shift);
> +}
> +
> static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct imx258 *imx258 =
> @@ -823,7 +849,7 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_EXPOSURE:
> ret = imx258_write_reg(imx258, IMX258_REG_EXPOSURE,
> IMX258_REG_VALUE_16BIT,
> - ctrl->val);
> + ctrl->val >> imx258->long_exp_shift);
Shouldn't this be done only if vblank > VTS_MAX ?
> break;
> case V4L2_CID_DIGITAL_GAIN:
> ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
> @@ -855,9 +881,8 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> }
> break;
> case V4L2_CID_VBLANK:
> - ret = imx258_write_reg(imx258, IMX258_REG_VTS,
> - IMX258_REG_VALUE_16BIT,
> - imx258->cur_mode->height + ctrl->val);
> + ret = imx258_set_frame_length(imx258,
> + imx258->cur_mode->height + ctrl->val);
> break;
> default:
> dev_info(&client->dev,
> @@ -983,8 +1008,9 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
> imx258->cur_mode->height;
> __v4l2_ctrl_modify_range(
> imx258->vblank, vblank_min,
> - IMX258_VTS_MAX - imx258->cur_mode->height, 1,
> - vblank_def);
> + ((1 << IMX258_LONG_EXP_SHIFT_MAX) * IMX258_VTS_MAX) -
> + imx258->cur_mode->height,
> + 1, vblank_def);
> __v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
> h_blank =
> imx258->link_freq_configs[mode->link_freq_index].pixels_per_line
> --
> 2.25.1
>
