lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zo5Lf6c6HKlPm6Rs@tom-HP-ZBook-Fury-15-G7-Mobile-Workstation>
Date: Wed, 10 Jul 2024 10:51:11 +0200
From: Tommaso Merciai <tomm.merciai@...il.com>
To: Umang Jain <umang.jain@...asonboard.com>
Cc: linux-media@...r.kernel.org,
	Alexander Shiyan <eagle.alexander923@...il.com>,
	Kieran Bingham <kieran.bingham@...asonboard.com>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] media: imx335: Support vertical flip

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@...asonboard.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@...il.com>

Thanks & Regards,
Tommaso


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ