[PATCH 0/2] media: imx335: Support vertical flip

[PATCH 0/2] media: imx335: Support vertical flip

[Umang Jain]

Hi all,

This work intends to supprt vertical flipping for IMX335 driver.
1/2 contains a small drive by fix, to rename the mode struct name
2/2 introduces the support for vertical flip for the mode.

Umang Jain (2):
  media: imx335: Rectify name of mode struct
  media: imx335: Support vertical flip

 drivers/media/i2c/imx335.c | 77 +++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 5 deletions(-)

-- 
2.45.0

[PATCH 1/2] media: imx335: Rectify name of mode struct

[Umang Jain]

In commit 81495a59baeb ("media: imx335: Fix active area height discrepency")
the height for the mode struct was rectified to '1944'. However, the
name of mode struct is still reflecting to '1940'. Update it.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/media/i2c/imx335.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
index 990d74214cc2..6c1e61b6696b 100644
--- a/drivers/media/i2c/imx335.c
+++ b/drivers/media/i2c/imx335.c
@@ -252,7 +252,7 @@ static const int imx335_tpg_val[] = {
 };
 
 /* Sensor mode registers */
-static const struct cci_reg_sequence mode_2592x1940_regs[] = {
+static const struct cci_reg_sequence mode_2592x1944_regs[] = {
 	{ IMX335_REG_MODE_SELECT, IMX335_MODE_STANDBY },
 	{ IMX335_REG_MASTER_MODE, 0x00 },
 	{ IMX335_REG_WINMODE, 0x04 },
@@ -416,8 +416,8 @@ static const struct imx335_mode supported_mode = {
 	.vblank_max = 133060,
 	.pclk = 396000000,
 	.reg_list = {
-		.num_of_regs = ARRAY_SIZE(mode_2592x1940_regs),
-		.regs = mode_2592x1940_regs,
+		.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
+		.regs = mode_2592x1944_regs,
 	},
 };
 
-- 
2.45.0

Re: [PATCH 1/2] media: imx335: Rectify name of mode struct

[Tommaso Merciai]

Hi Umang,

On Wed, Jul 10, 2024 at 10:16:31AM +0530, Umang Jain wrote:
> In commit 81495a59baeb ("media: imx335: Fix active area height discrepency")
> the height for the mode struct was rectified to '1944'. However, the
> name of mode struct is still reflecting to '1940'. Update it.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/media/i2c/imx335.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> index 990d74214cc2..6c1e61b6696b 100644
> --- a/drivers/media/i2c/imx335.c
> +++ b/drivers/media/i2c/imx335.c
> @@ -252,7 +252,7 @@ static const int imx335_tpg_val[] = {
>  };
>  
>  /* Sensor mode registers */
> -static const struct cci_reg_sequence mode_2592x1940_regs[] = {
> +static const struct cci_reg_sequence mode_2592x1944_regs[] = {
>  	{ IMX335_REG_MODE_SELECT, IMX335_MODE_STANDBY },
>  	{ IMX335_REG_MASTER_MODE, 0x00 },
>  	{ IMX335_REG_WINMODE, 0x04 },
> @@ -416,8 +416,8 @@ static const struct imx335_mode supported_mode = {
>  	.vblank_max = 133060,
>  	.pclk = 396000000,
>  	.reg_list = {
> -		.num_of_regs = ARRAY_SIZE(mode_2592x1940_regs),
> -		.regs = mode_2592x1940_regs,
> +		.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
> +		.regs = mode_2592x1944_regs,
>  	},
>  };
>  
> -- 
> 2.45.0
> 
> 

Looks good to me.
Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com>

Thanks & Regards,
Tommaso

Re: [PATCH 1/2] media: imx335: Rectify name of mode struct

[Kieran Bingham]

Quoting Umang Jain (2024-07-10 05:46:31)
> In commit 81495a59baeb ("media: imx335: Fix active area height discrepency")
> the height for the mode struct was rectified to '1944'. However, the
> name of mode struct is still reflecting to '1940'. Update it.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/media/i2c/imx335.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> index 990d74214cc2..6c1e61b6696b 100644
> --- a/drivers/media/i2c/imx335.c
> +++ b/drivers/media/i2c/imx335.c
> @@ -252,7 +252,7 @@ static const int imx335_tpg_val[] = {
>  };
>  
>  /* Sensor mode registers */
> -static const struct cci_reg_sequence mode_2592x1940_regs[] = {
> +static const struct cci_reg_sequence mode_2592x1944_regs[] = {
>         { IMX335_REG_MODE_SELECT, IMX335_MODE_STANDBY },
>         { IMX335_REG_MASTER_MODE, 0x00 },
>         { IMX335_REG_WINMODE, 0x04 },
> @@ -416,8 +416,8 @@ static const struct imx335_mode supported_mode = {
>         .vblank_max = 133060,
>         .pclk = 396000000,
>         .reg_list = {
> -               .num_of_regs = ARRAY_SIZE(mode_2592x1940_regs),
> -               .regs = mode_2592x1940_regs,
> +               .num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
> +               .regs = mode_2592x1944_regs,
>         },
>  };
>  
> -- 
> 2.45.0
>

[PATCH 2/2] media: imx335: Support vertical flip

[Umang Jain]

Support vertical flip by setting REG_VREVERSE.
Additional registers also needs to be set per mode, according
to the readout direction (normal/inverted) as mentioned in the
data sheet.

Since the register IMX335_REG_AREA3_ST_ADR_1 is based on the
flip (and is set via vflip related registers), it has been
moved out of the 2592x1944 mode regs.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/media/i2c/imx335.c | 71 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
index 6c1e61b6696b..cd150606a8a9 100644
--- a/drivers/media/i2c/imx335.c
+++ b/drivers/media/i2c/imx335.c
@@ -56,6 +56,9 @@
 #define IMX335_AGAIN_STEP		1
 #define IMX335_AGAIN_DEFAULT		0
 
+/* Vertical flip */
+#define IMX335_REG_VREVERSE		CCI_REG8(0x304f)
+
 #define IMX335_REG_TPG_TESTCLKEN	CCI_REG8(0x3148)
 
 #define IMX335_REG_INCLKSEL1		CCI_REG16_LE(0x314c)
@@ -155,6 +158,8 @@ static const char * const imx335_supply_name[] = {
  * @vblank_max: Maximum vertical blanking in lines
  * @pclk: Sensor pixel clock
  * @reg_list: Register list for sensor mode
+ * @vflip_normal: Register list vflip (normal readout)
+ * @vflip_inverted: Register list vflip (inverted readout)
  */
 struct imx335_mode {
 	u32 width;
@@ -166,6 +171,8 @@ struct imx335_mode {
 	u32 vblank_max;
 	u64 pclk;
 	struct imx335_reg_list reg_list;
+	struct imx335_reg_list vflip_normal;
+	struct imx335_reg_list vflip_inverted;
 };
 
 /**
@@ -183,6 +190,7 @@ struct imx335_mode {
  * @pclk_ctrl: Pointer to pixel clock control
  * @hblank_ctrl: Pointer to horizontal blanking control
  * @vblank_ctrl: Pointer to vertical blanking control
+ * @vflip: Pointer to vertical flip control
  * @exp_ctrl: Pointer to exposure control
  * @again_ctrl: Pointer to analog gain control
  * @vblank: Vertical blanking in lines
@@ -207,6 +215,7 @@ struct imx335 {
 	struct v4l2_ctrl *pclk_ctrl;
 	struct v4l2_ctrl *hblank_ctrl;
 	struct v4l2_ctrl *vblank_ctrl;
+	struct v4l2_ctrl *vflip;
 	struct {
 		struct v4l2_ctrl *exp_ctrl;
 		struct v4l2_ctrl *again_ctrl;
@@ -259,7 +268,6 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = {
 	{ IMX335_REG_HTRIMMING_START, 48 },
 	{ IMX335_REG_HNUM, 2592 },
 	{ IMX335_REG_Y_OUT_SIZE, 1944 },
-	{ IMX335_REG_AREA3_ST_ADR_1, 176 },
 	{ IMX335_REG_AREA3_WIDTH_1, 3928 },
 	{ IMX335_REG_OPB_SIZE_V, 0 },
 	{ IMX335_REG_XVS_XHS_DRV, 0x00 },
@@ -333,6 +341,26 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = {
 	{ CCI_REG8(0x3a00), 0x00 },
 };
 
+static const struct cci_reg_sequence mode_2592x1944_vflip_normal[] = {
+	{ IMX335_REG_AREA3_ST_ADR_1, 176 },
+
+	/* Undocumented V-Flip related registers on Page 55 of datasheet. */
+	{ CCI_REG8(0x3081), 0x02, },
+	{ CCI_REG8(0x3083), 0x02, },
+	{ CCI_REG16_LE(0x30b6), 0x00 },
+	{ CCI_REG16_LE(0x3116), 0x08 },
+};
+
+static const struct cci_reg_sequence mode_2592x1944_vflip_inverted[] = {
+	{ IMX335_REG_AREA3_ST_ADR_1, 4112 },
+
+	/* Undocumented V-Flip related registers on Page 55 of datasheet. */
+	{ CCI_REG8(0x3081), 0xfe, },
+	{ CCI_REG8(0x3083), 0xfe, },
+	{ CCI_REG16_LE(0x30b6), 0x1fa },
+	{ CCI_REG16_LE(0x3116), 0x002 },
+};
+
 static const struct cci_reg_sequence raw10_framefmt_regs[] = {
 	{ IMX335_REG_ADBIT, 0x00 },
 	{ IMX335_REG_MDBIT, 0x00 },
@@ -419,6 +447,14 @@ static const struct imx335_mode supported_mode = {
 		.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
 		.regs = mode_2592x1944_regs,
 	},
+	.vflip_normal = {
+		.num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_normal),
+		.regs = mode_2592x1944_vflip_normal,
+	},
+	.vflip_inverted = {
+		.num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_inverted),
+		.regs = mode_2592x1944_vflip_inverted,
+	},
 };
 
 /**
@@ -492,6 +528,26 @@ static int imx335_update_exp_gain(struct imx335 *imx335, u32 exposure, u32 gain)
 	return ret;
 }
 
+static int imx335_update_vertical_flip(struct imx335 *imx335, u32 vflip)
+{
+	int ret = 0;
+
+	if (vflip)
+		cci_multi_reg_write(imx335->cci,
+				    imx335->cur_mode->vflip_inverted.regs,
+				    imx335->cur_mode->vflip_inverted.num_of_regs,
+				    &ret);
+	else
+		cci_multi_reg_write(imx335->cci,
+				    imx335->cur_mode->vflip_normal.regs,
+				    imx335->cur_mode->vflip_normal.num_of_regs,
+				    &ret);
+	if (ret)
+		return ret;
+
+	return cci_write(imx335->cci, IMX335_REG_VREVERSE, vflip, NULL);
+}
+
 static int imx335_update_test_pattern(struct imx335 *imx335, u32 pattern_index)
 {
 	int ret = 0;
@@ -584,6 +640,10 @@ static int imx335_set_ctrl(struct v4l2_ctrl *ctrl)
 
 		ret = imx335_update_exp_gain(imx335, exposure, analog_gain);
 
+		break;
+	case V4L2_CID_VFLIP:
+		ret = imx335_update_vertical_flip(imx335, ctrl->val);
+
 		break;
 	case V4L2_CID_TEST_PATTERN:
 		ret = imx335_update_test_pattern(imx335, ctrl->val);
@@ -1167,7 +1227,7 @@ static int imx335_init_controls(struct imx335 *imx335)
 		return ret;
 
 	/* v4l2_fwnode_device_properties can add two more controls */
-	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
+	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
 	if (ret)
 		return ret;
 
@@ -1202,6 +1262,13 @@ static int imx335_init_controls(struct imx335 *imx335)
 
 	v4l2_ctrl_cluster(2, &imx335->exp_ctrl);
 
+	imx335->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
+					  &imx335_ctrl_ops,
+					  V4L2_CID_VFLIP,
+					  0, 1, 1, 0);
+	if (imx335->vflip)
+		imx335->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
 	imx335->vblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
 						&imx335_ctrl_ops,
 						V4L2_CID_VBLANK,
-- 
2.45.0

Re: [PATCH 2/2] media: imx335: Support vertical flip

[Tommaso Merciai]

Hi Umang,

On Wed, Jul 10, 2024 at 10:16:32AM +0530, Umang Jain wrote:
> Support vertical flip by setting REG_VREVERSE.
> Additional registers also needs to be set per mode, according
> to the readout direction (normal/inverted) as mentioned in the
> data sheet.
> 
> Since the register IMX335_REG_AREA3_ST_ADR_1 is based on the
> flip (and is set via vflip related registers), it has been
> moved out of the 2592x1944 mode regs.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/media/i2c/imx335.c | 71 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> index 6c1e61b6696b..cd150606a8a9 100644
> --- a/drivers/media/i2c/imx335.c
> +++ b/drivers/media/i2c/imx335.c
> @@ -56,6 +56,9 @@
>  #define IMX335_AGAIN_STEP		1
>  #define IMX335_AGAIN_DEFAULT		0
>  
> +/* Vertical flip */
> +#define IMX335_REG_VREVERSE		CCI_REG8(0x304f)
> +
>  #define IMX335_REG_TPG_TESTCLKEN	CCI_REG8(0x3148)
>  
>  #define IMX335_REG_INCLKSEL1		CCI_REG16_LE(0x314c)
> @@ -155,6 +158,8 @@ static const char * const imx335_supply_name[] = {
>   * @vblank_max: Maximum vertical blanking in lines
>   * @pclk: Sensor pixel clock
>   * @reg_list: Register list for sensor mode
> + * @vflip_normal: Register list vflip (normal readout)
> + * @vflip_inverted: Register list vflip (inverted readout)
>   */
>  struct imx335_mode {
>  	u32 width;
> @@ -166,6 +171,8 @@ struct imx335_mode {
>  	u32 vblank_max;
>  	u64 pclk;
>  	struct imx335_reg_list reg_list;
> +	struct imx335_reg_list vflip_normal;
> +	struct imx335_reg_list vflip_inverted;
>  };
>  
>  /**
> @@ -183,6 +190,7 @@ struct imx335_mode {
>   * @pclk_ctrl: Pointer to pixel clock control
>   * @hblank_ctrl: Pointer to horizontal blanking control
>   * @vblank_ctrl: Pointer to vertical blanking control
> + * @vflip: Pointer to vertical flip control
>   * @exp_ctrl: Pointer to exposure control
>   * @again_ctrl: Pointer to analog gain control
>   * @vblank: Vertical blanking in lines
> @@ -207,6 +215,7 @@ struct imx335 {
>  	struct v4l2_ctrl *pclk_ctrl;
>  	struct v4l2_ctrl *hblank_ctrl;
>  	struct v4l2_ctrl *vblank_ctrl;
> +	struct v4l2_ctrl *vflip;
>  	struct {
>  		struct v4l2_ctrl *exp_ctrl;
>  		struct v4l2_ctrl *again_ctrl;
> @@ -259,7 +268,6 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = {
>  	{ IMX335_REG_HTRIMMING_START, 48 },
>  	{ IMX335_REG_HNUM, 2592 },
>  	{ IMX335_REG_Y_OUT_SIZE, 1944 },
> -	{ IMX335_REG_AREA3_ST_ADR_1, 176 },
>  	{ IMX335_REG_AREA3_WIDTH_1, 3928 },
>  	{ IMX335_REG_OPB_SIZE_V, 0 },
>  	{ IMX335_REG_XVS_XHS_DRV, 0x00 },
> @@ -333,6 +341,26 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = {
>  	{ CCI_REG8(0x3a00), 0x00 },
>  };
>  
> +static const struct cci_reg_sequence mode_2592x1944_vflip_normal[] = {
> +	{ IMX335_REG_AREA3_ST_ADR_1, 176 },
> +
> +	/* Undocumented V-Flip related registers on Page 55 of datasheet. */
> +	{ CCI_REG8(0x3081), 0x02, },
> +	{ CCI_REG8(0x3083), 0x02, },
> +	{ CCI_REG16_LE(0x30b6), 0x00 },
> +	{ CCI_REG16_LE(0x3116), 0x08 },
> +};
> +
> +static const struct cci_reg_sequence mode_2592x1944_vflip_inverted[] = {
> +	{ IMX335_REG_AREA3_ST_ADR_1, 4112 },
> +
> +	/* Undocumented V-Flip related registers on Page 55 of datasheet. */
> +	{ CCI_REG8(0x3081), 0xfe, },
> +	{ CCI_REG8(0x3083), 0xfe, },
> +	{ CCI_REG16_LE(0x30b6), 0x1fa },
> +	{ CCI_REG16_LE(0x3116), 0x002 },
> +};
> +
>  static const struct cci_reg_sequence raw10_framefmt_regs[] = {
>  	{ IMX335_REG_ADBIT, 0x00 },
>  	{ IMX335_REG_MDBIT, 0x00 },
> @@ -419,6 +447,14 @@ static const struct imx335_mode supported_mode = {
>  		.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
>  		.regs = mode_2592x1944_regs,
>  	},
> +	.vflip_normal = {
> +		.num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_normal),
> +		.regs = mode_2592x1944_vflip_normal,
> +	},
> +	.vflip_inverted = {
> +		.num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_inverted),
> +		.regs = mode_2592x1944_vflip_inverted,
> +	},
>  };
>  
>  /**
> @@ -492,6 +528,26 @@ static int imx335_update_exp_gain(struct imx335 *imx335, u32 exposure, u32 gain)
>  	return ret;
>  }
>  
> +static int imx335_update_vertical_flip(struct imx335 *imx335, u32 vflip)
> +{
> +	int ret = 0;
> +
> +	if (vflip)
> +		cci_multi_reg_write(imx335->cci,
> +				    imx335->cur_mode->vflip_inverted.regs,
> +				    imx335->cur_mode->vflip_inverted.num_of_regs,
> +				    &ret);
> +	else
> +		cci_multi_reg_write(imx335->cci,
> +				    imx335->cur_mode->vflip_normal.regs,
> +				    imx335->cur_mode->vflip_normal.num_of_regs,
> +				    &ret);
> +	if (ret)
> +		return ret;
> +
> +	return cci_write(imx335->cci, IMX335_REG_VREVERSE, vflip, NULL);
> +}
> +
>  static int imx335_update_test_pattern(struct imx335 *imx335, u32 pattern_index)
>  {
>  	int ret = 0;
> @@ -584,6 +640,10 @@ static int imx335_set_ctrl(struct v4l2_ctrl *ctrl)
>  
>  		ret = imx335_update_exp_gain(imx335, exposure, analog_gain);
>  
> +		break;
> +	case V4L2_CID_VFLIP:
> +		ret = imx335_update_vertical_flip(imx335, ctrl->val);
> +
>  		break;
>  	case V4L2_CID_TEST_PATTERN:
>  		ret = imx335_update_test_pattern(imx335, ctrl->val);
> @@ -1167,7 +1227,7 @@ static int imx335_init_controls(struct imx335 *imx335)
>  		return ret;
>  
>  	/* v4l2_fwnode_device_properties can add two more controls */
> -	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> +	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
>  	if (ret)
>  		return ret;
>  
> @@ -1202,6 +1262,13 @@ static int imx335_init_controls(struct imx335 *imx335)
>  
>  	v4l2_ctrl_cluster(2, &imx335->exp_ctrl);
>  
> +	imx335->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> +					  &imx335_ctrl_ops,
> +					  V4L2_CID_VFLIP,
> +					  0, 1, 1, 0);
> +	if (imx335->vflip)
> +		imx335->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> +
>  	imx335->vblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
>  						&imx335_ctrl_ops,
>  						V4L2_CID_VBLANK,
> -- 
> 2.45.0
> 

This patch looks good to me.
Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com>

Thanks & Regards,
Tommaso

Re: [PATCH 2/2] media: imx335: Support vertical flip

[Kieran Bingham]

Quoting Umang Jain (2024-07-10 05:46:32)
> Support vertical flip by setting REG_VREVERSE.
> Additional registers also needs to be set per mode, according
> to the readout direction (normal/inverted) as mentioned in the
> data sheet.
> 
> Since the register IMX335_REG_AREA3_ST_ADR_1 is based on the
> flip (and is set via vflip related registers), it has been
> moved out of the 2592x1944 mode regs.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/media/i2c/imx335.c | 71 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> index 6c1e61b6696b..cd150606a8a9 100644
> --- a/drivers/media/i2c/imx335.c
> +++ b/drivers/media/i2c/imx335.c
> @@ -56,6 +56,9 @@
>  #define IMX335_AGAIN_STEP              1
>  #define IMX335_AGAIN_DEFAULT           0
>  
> +/* Vertical flip */
> +#define IMX335_REG_VREVERSE            CCI_REG8(0x304f)
> +
>  #define IMX335_REG_TPG_TESTCLKEN       CCI_REG8(0x3148)
>  
>  #define IMX335_REG_INCLKSEL1           CCI_REG16_LE(0x314c)
> @@ -155,6 +158,8 @@ static const char * const imx335_supply_name[] = {
>   * @vblank_max: Maximum vertical blanking in lines
>   * @pclk: Sensor pixel clock
>   * @reg_list: Register list for sensor mode
> + * @vflip_normal: Register list vflip (normal readout)
> + * @vflip_inverted: Register list vflip (inverted readout)
>   */
>  struct imx335_mode {
>         u32 width;
> @@ -166,6 +171,8 @@ struct imx335_mode {
>         u32 vblank_max;
>         u64 pclk;
>         struct imx335_reg_list reg_list;
> +       struct imx335_reg_list vflip_normal;
> +       struct imx335_reg_list vflip_inverted;
>  };
>  
>  /**
> @@ -183,6 +190,7 @@ struct imx335_mode {
>   * @pclk_ctrl: Pointer to pixel clock control
>   * @hblank_ctrl: Pointer to horizontal blanking control
>   * @vblank_ctrl: Pointer to vertical blanking control
> + * @vflip: Pointer to vertical flip control
>   * @exp_ctrl: Pointer to exposure control
>   * @again_ctrl: Pointer to analog gain control
>   * @vblank: Vertical blanking in lines
> @@ -207,6 +215,7 @@ struct imx335 {
>         struct v4l2_ctrl *pclk_ctrl;
>         struct v4l2_ctrl *hblank_ctrl;
>         struct v4l2_ctrl *vblank_ctrl;
> +       struct v4l2_ctrl *vflip;
>         struct {
>                 struct v4l2_ctrl *exp_ctrl;
>                 struct v4l2_ctrl *again_ctrl;
> @@ -259,7 +268,6 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = {
>         { IMX335_REG_HTRIMMING_START, 48 },
>         { IMX335_REG_HNUM, 2592 },
>         { IMX335_REG_Y_OUT_SIZE, 1944 },
> -       { IMX335_REG_AREA3_ST_ADR_1, 176 },
>         { IMX335_REG_AREA3_WIDTH_1, 3928 },
>         { IMX335_REG_OPB_SIZE_V, 0 },
>         { IMX335_REG_XVS_XHS_DRV, 0x00 },
> @@ -333,6 +341,26 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = {
>         { CCI_REG8(0x3a00), 0x00 },
>  };
>  
> +static const struct cci_reg_sequence mode_2592x1944_vflip_normal[] = {
> +       { IMX335_REG_AREA3_ST_ADR_1, 176 },
> +
> +       /* Undocumented V-Flip related registers on Page 55 of datasheet. */
> +       { CCI_REG8(0x3081), 0x02, },
> +       { CCI_REG8(0x3083), 0x02, },
> +       { CCI_REG16_LE(0x30b6), 0x00 },
> +       { CCI_REG16_LE(0x3116), 0x08 },
> +};
> +
> +static const struct cci_reg_sequence mode_2592x1944_vflip_inverted[] = {
> +       { IMX335_REG_AREA3_ST_ADR_1, 4112 },
> +
> +       /* Undocumented V-Flip related registers on Page 55 of datasheet. */
> +       { CCI_REG8(0x3081), 0xfe, },
> +       { CCI_REG8(0x3083), 0xfe, },
> +       { CCI_REG16_LE(0x30b6), 0x1fa },
> +       { CCI_REG16_LE(0x3116), 0x002 },

A little more awkward than the usual flip controls, but I think we do
need to track what the datasheet gives us for now unless we can get more
information from Sony or do some reverse engineering here which isn't
really worth the effort at the moment.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +};
> +
>  static const struct cci_reg_sequence raw10_framefmt_regs[] = {
>         { IMX335_REG_ADBIT, 0x00 },
>         { IMX335_REG_MDBIT, 0x00 },
> @@ -419,6 +447,14 @@ static const struct imx335_mode supported_mode = {
>                 .num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
>                 .regs = mode_2592x1944_regs,
>         },
> +       .vflip_normal = {
> +               .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_normal),
> +               .regs = mode_2592x1944_vflip_normal,
> +       },
> +       .vflip_inverted = {
> +               .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_inverted),
> +               .regs = mode_2592x1944_vflip_inverted,
> +       },
>  };
>  
>  /**
> @@ -492,6 +528,26 @@ static int imx335_update_exp_gain(struct imx335 *imx335, u32 exposure, u32 gain)
>         return ret;
>  }
>  
> +static int imx335_update_vertical_flip(struct imx335 *imx335, u32 vflip)
> +{
> +       int ret = 0;
> +
> +       if (vflip)
> +               cci_multi_reg_write(imx335->cci,
> +                                   imx335->cur_mode->vflip_inverted.regs,
> +                                   imx335->cur_mode->vflip_inverted.num_of_regs,
> +                                   &ret);
> +       else
> +               cci_multi_reg_write(imx335->cci,
> +                                   imx335->cur_mode->vflip_normal.regs,
> +                                   imx335->cur_mode->vflip_normal.num_of_regs,
> +                                   &ret);
> +       if (ret)
> +               return ret;
> +
> +       return cci_write(imx335->cci, IMX335_REG_VREVERSE, vflip, NULL);
> +}
> +
>  static int imx335_update_test_pattern(struct imx335 *imx335, u32 pattern_index)
>  {
>         int ret = 0;
> @@ -584,6 +640,10 @@ static int imx335_set_ctrl(struct v4l2_ctrl *ctrl)
>  
>                 ret = imx335_update_exp_gain(imx335, exposure, analog_gain);
>  
> +               break;
> +       case V4L2_CID_VFLIP:
> +               ret = imx335_update_vertical_flip(imx335, ctrl->val);
> +
>                 break;
>         case V4L2_CID_TEST_PATTERN:
>                 ret = imx335_update_test_pattern(imx335, ctrl->val);
> @@ -1167,7 +1227,7 @@ static int imx335_init_controls(struct imx335 *imx335)
>                 return ret;
>  
>         /* v4l2_fwnode_device_properties can add two more controls */
> -       ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> +       ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
>         if (ret)
>                 return ret;
>  
> @@ -1202,6 +1262,13 @@ static int imx335_init_controls(struct imx335 *imx335)
>  
>         v4l2_ctrl_cluster(2, &imx335->exp_ctrl);
>  
> +       imx335->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> +                                         &imx335_ctrl_ops,
> +                                         V4L2_CID_VFLIP,
> +                                         0, 1, 1, 0);
> +       if (imx335->vflip)
> +               imx335->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> +
>         imx335->vblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr,
>                                                 &imx335_ctrl_ops,
>                                                 V4L2_CID_VBLANK,
> -- 
> 2.45.0
>

Re: [PATCH 0/2] media: imx335: Support vertical flip

[Umang Jain]

Hello

Can this be collected please ?

On 10/07/24 10:16 am, Umang Jain wrote:
> Hi all,
>
> This work intends to supprt vertical flipping for IMX335 driver.
> 1/2 contains a small drive by fix, to rename the mode struct name
> 2/2 introduces the support for vertical flip for the mode.
>
> Umang Jain (2):
>    media: imx335: Rectify name of mode struct
>    media: imx335: Support vertical flip
>
>   drivers/media/i2c/imx335.c | 77 +++++++++++++++++++++++++++++++++++---
>   1 file changed, 72 insertions(+), 5 deletions(-)
>