diff mbox series

[1/3] media: i2c: imx415: Add read/write control of VBLANK

Message ID 20250109-media-imx415-v1-1-366ba6a234ab@raspberrypi.com (mailing list archive)
State New
Headers show
Series media: imx415: Improvements for framerate and link frequency control | expand

Commit Message

Dave Stevenson Jan. 9, 2025, 11:17 a.m. UTC 
This also requires that the ranges for the exposure control
are updated.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/imx415.c | 51 ++++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 18 deletions(-)

Comments

Dave Stevenson Jan. 9, 2025, 2:51 p.m. UTC | #1 
On Thu, 9 Jan 2025 at 11:17, Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> This also requires that the ranges for the exposure control
> are updated.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
>  drivers/media/i2c/imx415.c | 51 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
> index 3f7924aa1bd3..2c8c0905aa99 100644
> --- a/drivers/media/i2c/imx415.c
> +++ b/drivers/media/i2c/imx415.c
> @@ -26,6 +26,7 @@
>  #define IMX415_PIXEL_ARRAY_WIDTH  3864
>  #define IMX415_PIXEL_ARRAY_HEIGHT 2192
>  #define IMX415_PIXEL_ARRAY_VBLANK 58
> +#define IMX415_EXPOSURE_OFFSET   8
>
>  #define IMX415_NUM_CLK_PARAM_REGS 11
>
> @@ -51,6 +52,7 @@
>  #define IMX415_OUTSEL            CCI_REG8(0x30c0)
>  #define IMX415_DRV               CCI_REG8(0x30c1)
>  #define IMX415_VMAX              CCI_REG24_LE(0x3024)
> +#define IMX415_VMAX_MAX                  0xfffff
>  #define IMX415_HMAX              CCI_REG16_LE(0x3028)
>  #define IMX415_SHR0              CCI_REG24_LE(0x3050)
>  #define IMX415_GAIN_PCG_0        CCI_REG16_LE(0x3090)
> @@ -447,7 +449,6 @@ static const struct imx415_clk_params imx415_clk_params[] = {
>
>  /* all-pixel 2-lane 720 Mbps 15.74 Hz mode */
>  static const struct cci_reg_sequence imx415_mode_2_720[] = {
> -       { IMX415_VMAX, 0x08CA },
>         { IMX415_HMAX, 0x07F0 },
>         { IMX415_LANEMODE, IMX415_LANEMODE_2 },
>         { IMX415_TCLKPOST, 0x006F },
> @@ -463,7 +464,6 @@ static const struct cci_reg_sequence imx415_mode_2_720[] = {
>
>  /* all-pixel 2-lane 1440 Mbps 30.01 Hz mode */
>  static const struct cci_reg_sequence imx415_mode_2_1440[] = {
> -       { IMX415_VMAX, 0x08CA },
>         { IMX415_HMAX, 0x042A },
>         { IMX415_LANEMODE, IMX415_LANEMODE_2 },
>         { IMX415_TCLKPOST, 0x009F },
> @@ -479,7 +479,6 @@ static const struct cci_reg_sequence imx415_mode_2_1440[] = {
>
>  /* all-pixel 4-lane 891 Mbps 30 Hz mode */
>  static const struct cci_reg_sequence imx415_mode_4_891[] = {
> -       { IMX415_VMAX, 0x08CA },
>         { IMX415_HMAX, 0x044C },
>         { IMX415_LANEMODE, IMX415_LANEMODE_4 },
>         { IMX415_TCLKPOST, 0x007F },
> @@ -600,6 +599,7 @@ struct imx415 {
>         struct v4l2_ctrl *vblank;
>         struct v4l2_ctrl *hflip;
>         struct v4l2_ctrl *vflip;
> +       struct v4l2_ctrl *exposure;
>
>         unsigned int cur_mode;
>         unsigned int num_data_lanes;
> @@ -730,17 +730,37 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
>                                              ctrls);
>         const struct v4l2_mbus_framefmt *format;
>         struct v4l2_subdev_state *state;
> +       u32 exposure_max;
>         unsigned int vmax;
>         unsigned int flip;
>         int ret;
>
> -       if (!pm_runtime_get_if_in_use(sensor->dev))
> -               return 0;
> -
>         state = v4l2_subdev_get_locked_active_state(&sensor->subdev);
>         format = v4l2_subdev_state_get_format(state, 0);
>
> +       if (ctrl->id == V4L2_CID_VBLANK) {
> +               exposure_max = format->height + ctrl->val -
> +                              IMX415_EXPOSURE_OFFSET;
> +               __v4l2_ctrl_modify_range(sensor->exposure,
> +                                        sensor->exposure->minimum,
> +                                        exposure_max, sensor->exposure->step,
> +                                        sensor->exposure->default_value);
> +       }
> +
> +       if (!pm_runtime_get_if_in_use(sensor->dev))
> +               return 0;
> +
>         switch (ctrl->id) {
> +       case V4L2_CID_VBLANK:
> +               ret = cci_write(sensor->regmap, IMX415_VMAX,
> +                               format->height + ctrl->val, NULL);
> +               if (ret)
> +                       return ret;
> +               /*
> +                * Deliberately fall through as exposure is set based on VMAX
> +                * which has just changed.
> +                */
> +               fallthrough;

Checkpatch complains "WARNING: Prefer 'fallthrough;' over fallthrough comment"
Having a comment as to why we're wanting to fall through as well as
fallthrough; seems reasonable to me.

However what I have just noticed is that the V4L2_CID_EXPOSURE case
uses ctrl->val, which is going to be incorrect when we fall through.
I'll sort out an update in due course.

  Dave

>         case V4L2_CID_EXPOSURE:
>                 /* clamp the exposure value to VMAX. */
>                 vmax = format->height + sensor->vblank->cur.val;
> @@ -787,7 +807,8 @@ static int imx415_ctrls_init(struct imx415 *sensor)
>         u64 pixel_rate = supported_modes[sensor->cur_mode].pixel_rate;
>         u64 lane_rate = supported_modes[sensor->cur_mode].lane_rate;
>         u32 exposure_max = IMX415_PIXEL_ARRAY_HEIGHT +
> -                          IMX415_PIXEL_ARRAY_VBLANK - 8;
> +                          IMX415_PIXEL_ARRAY_VBLANK -
> +                          IMX415_EXPOSURE_OFFSET;
>         u32 hblank;
>         unsigned int i;
>         int ret;
> @@ -816,8 +837,9 @@ static int imx415_ctrls_init(struct imx415 *sensor)
>         if (ctrl)
>                 ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> -       v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops, V4L2_CID_EXPOSURE,
> -                         4, exposure_max, 1, exposure_max);
> +       sensor->exposure = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
> +                                            V4L2_CID_EXPOSURE, 4,
> +                                            exposure_max, 1, exposure_max);
>
>         v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
>                           V4L2_CID_ANALOGUE_GAIN, IMX415_AGAIN_MIN,
> @@ -834,16 +856,9 @@ static int imx415_ctrls_init(struct imx415 *sensor)
>         sensor->vblank = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
>                                            V4L2_CID_VBLANK,
>                                            IMX415_PIXEL_ARRAY_VBLANK,
> -                                          IMX415_PIXEL_ARRAY_VBLANK, 1,
> -                                          IMX415_PIXEL_ARRAY_VBLANK);
> -       if (sensor->vblank)
> -               sensor->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +                                          IMX415_VMAX_MAX - IMX415_PIXEL_ARRAY_HEIGHT,
> +                                          1, IMX415_PIXEL_ARRAY_VBLANK);
>
> -       /*
> -        * The pixel rate used here is a virtual value and can be used for
> -        * calculating the frame rate together with hblank. It may not
> -        * necessarily be the physically correct pixel clock.
> -        */
>         v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE, pixel_rate,
>                           pixel_rate, 1, pixel_rate);
>
>
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
index 3f7924aa1bd3..2c8c0905aa99 100644
--- a/drivers/media/i2c/imx415.c
+++ b/drivers/media/i2c/imx415.c
@@ -26,6 +26,7 @@ 
 #define IMX415_PIXEL_ARRAY_WIDTH  3864
 #define IMX415_PIXEL_ARRAY_HEIGHT 2192
 #define IMX415_PIXEL_ARRAY_VBLANK 58
+#define IMX415_EXPOSURE_OFFSET	  8
 
 #define IMX415_NUM_CLK_PARAM_REGS 11
 
@@ -51,6 +52,7 @@ 
 #define IMX415_OUTSEL		  CCI_REG8(0x30c0)
 #define IMX415_DRV		  CCI_REG8(0x30c1)
 #define IMX415_VMAX		  CCI_REG24_LE(0x3024)
+#define IMX415_VMAX_MAX		  0xfffff
 #define IMX415_HMAX		  CCI_REG16_LE(0x3028)
 #define IMX415_SHR0		  CCI_REG24_LE(0x3050)
 #define IMX415_GAIN_PCG_0	  CCI_REG16_LE(0x3090)
@@ -447,7 +449,6 @@  static const struct imx415_clk_params imx415_clk_params[] = {
 
 /* all-pixel 2-lane 720 Mbps 15.74 Hz mode */
 static const struct cci_reg_sequence imx415_mode_2_720[] = {
-	{ IMX415_VMAX, 0x08CA },
 	{ IMX415_HMAX, 0x07F0 },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
 	{ IMX415_TCLKPOST, 0x006F },
@@ -463,7 +464,6 @@  static const struct cci_reg_sequence imx415_mode_2_720[] = {
 
 /* all-pixel 2-lane 1440 Mbps 30.01 Hz mode */
 static const struct cci_reg_sequence imx415_mode_2_1440[] = {
-	{ IMX415_VMAX, 0x08CA },
 	{ IMX415_HMAX, 0x042A },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_2 },
 	{ IMX415_TCLKPOST, 0x009F },
@@ -479,7 +479,6 @@  static const struct cci_reg_sequence imx415_mode_2_1440[] = {
 
 /* all-pixel 4-lane 891 Mbps 30 Hz mode */
 static const struct cci_reg_sequence imx415_mode_4_891[] = {
-	{ IMX415_VMAX, 0x08CA },
 	{ IMX415_HMAX, 0x044C },
 	{ IMX415_LANEMODE, IMX415_LANEMODE_4 },
 	{ IMX415_TCLKPOST, 0x007F },
@@ -600,6 +599,7 @@  struct imx415 {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *hflip;
 	struct v4l2_ctrl *vflip;
+	struct v4l2_ctrl *exposure;
 
 	unsigned int cur_mode;
 	unsigned int num_data_lanes;
@@ -730,17 +730,37 @@  static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
 					     ctrls);
 	const struct v4l2_mbus_framefmt *format;
 	struct v4l2_subdev_state *state;
+	u32 exposure_max;
 	unsigned int vmax;
 	unsigned int flip;
 	int ret;
 
-	if (!pm_runtime_get_if_in_use(sensor->dev))
-		return 0;
-
 	state = v4l2_subdev_get_locked_active_state(&sensor->subdev);
 	format = v4l2_subdev_state_get_format(state, 0);
 
+	if (ctrl->id == V4L2_CID_VBLANK) {
+		exposure_max = format->height + ctrl->val -
+			       IMX415_EXPOSURE_OFFSET;
+		__v4l2_ctrl_modify_range(sensor->exposure,
+					 sensor->exposure->minimum,
+					 exposure_max, sensor->exposure->step,
+					 sensor->exposure->default_value);
+	}
+
+	if (!pm_runtime_get_if_in_use(sensor->dev))
+		return 0;
+
 	switch (ctrl->id) {
+	case V4L2_CID_VBLANK:
+		ret = cci_write(sensor->regmap, IMX415_VMAX,
+				format->height + ctrl->val, NULL);
+		if (ret)
+			return ret;
+		/*
+		 * Deliberately fall through as exposure is set based on VMAX
+		 * which has just changed.
+		 */
+		fallthrough;
 	case V4L2_CID_EXPOSURE:
 		/* clamp the exposure value to VMAX. */
 		vmax = format->height + sensor->vblank->cur.val;
@@ -787,7 +807,8 @@  static int imx415_ctrls_init(struct imx415 *sensor)
 	u64 pixel_rate = supported_modes[sensor->cur_mode].pixel_rate;
 	u64 lane_rate = supported_modes[sensor->cur_mode].lane_rate;
 	u32 exposure_max = IMX415_PIXEL_ARRAY_HEIGHT +
-			   IMX415_PIXEL_ARRAY_VBLANK - 8;
+			   IMX415_PIXEL_ARRAY_VBLANK -
+			   IMX415_EXPOSURE_OFFSET;
 	u32 hblank;
 	unsigned int i;
 	int ret;
@@ -816,8 +837,9 @@  static int imx415_ctrls_init(struct imx415 *sensor)
 	if (ctrl)
 		ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
-	v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops, V4L2_CID_EXPOSURE,
-			  4, exposure_max, 1, exposure_max);
+	sensor->exposure = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
+					     V4L2_CID_EXPOSURE, 4,
+					     exposure_max, 1, exposure_max);
 
 	v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
 			  V4L2_CID_ANALOGUE_GAIN, IMX415_AGAIN_MIN,
@@ -834,16 +856,9 @@  static int imx415_ctrls_init(struct imx415 *sensor)
 	sensor->vblank = v4l2_ctrl_new_std(&sensor->ctrls, &imx415_ctrl_ops,
 					   V4L2_CID_VBLANK,
 					   IMX415_PIXEL_ARRAY_VBLANK,
-					   IMX415_PIXEL_ARRAY_VBLANK, 1,
-					   IMX415_PIXEL_ARRAY_VBLANK);
-	if (sensor->vblank)
-		sensor->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+					   IMX415_VMAX_MAX - IMX415_PIXEL_ARRAY_HEIGHT,
+					   1, IMX415_PIXEL_ARRAY_VBLANK);
 
-	/*
-	 * The pixel rate used here is a virtual value and can be used for
-	 * calculating the frame rate together with hblank. It may not
-	 * necessarily be the physically correct pixel clock.
-	 */
 	v4l2_ctrl_new_std(&sensor->ctrls, NULL, V4L2_CID_PIXEL_RATE, pixel_rate,
 			  pixel_rate, 1, pixel_rate);