diff mbox series

[V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions

Message ID 20241216043926.445001-1-shravan.chippa@microchip.com
State New
Headers show
Series [V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions | expand

Commit Message

shravan kumar Dec. 16, 2024, 4:39 a.m. UTC
From: Shravan Chippa <shravan.chippa@microchip.com>

Add support for 1280x720@30 and 640x480@30 resolutions
optimized the resolution arrays by added common register
array

Updated 1920x1080@30 resolution register array with
common register array

Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
---

changelog:

v2 -> v3
Removied blank lines reported by media CI robot 

v1 -> v2
Optimized the resolution arrays by added common register array

---
 drivers/media/i2c/imx334.c | 119 ++++++++++++++++++++++++++++++-------
 1 file changed, 99 insertions(+), 20 deletions(-)

Comments

shravan kumar Jan. 6, 2025, 10:18 a.m. UTC | #1
Gentle ping ...


> -----Original Message-----
> From: shravan kumar <shravan.chippa@microchip.com>
> Sent: Monday, December 16, 2024 10:09 AM
> To: sakari.ailus@linux.intel.com; mchehab@kernel.org
> Cc: kieran.bingham@ideasonboard.com; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; Conor Dooley - M52691
> <Conor.Dooley@microchip.com>; Valentina Fernandez Alanis - M63239
> <Valentina.FernandezAlanis@microchip.com>; Praveen Kumar - I30718
> <Praveen.Kumar@microchip.com>; shravan Chippa - I35088
> <Shravan.Chippa@microchip.com>
> Subject: [PATCH V3] media: i2c: imx334: Add support for 1280x720 &
> 640x480 resolutions
> 
> From: Shravan Chippa <shravan.chippa@microchip.com>
> 
> Add support for 1280x720@30 and 640x480@30 resolutions optimized the
> resolution arrays by added common register array
> 
> Updated 1920x1080@30 resolution register array with common register array
> 
> Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> ---
> 
> changelog:
> 
> v2 -> v3
> Removied blank lines reported by media CI robot
> 
> v1 -> v2
> Optimized the resolution arrays by added common register array
> 
> ---
>  drivers/media/i2c/imx334.c | 119 ++++++++++++++++++++++++++++++----
> ---
>  1 file changed, 99 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c index
> a544fc3df39c..58ad67dbb331 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -167,8 +167,8 @@ static const s64 link_freq[] = {
>  	IMX334_LINK_FREQ_445M,
>  };
> 
> -/* Sensor mode registers for 1920x1080@30fps */ -static const struct
> imx334_reg mode_1920x1080_regs[] = {
> +/* Sensor common mode registers for 445M link frequency */ static const
> +struct imx334_reg common_mode_regs_445m[] = {
>  	{0x3000, 0x01},
>  	{0x3018, 0x04},
>  	{0x3030, 0xca},
> @@ -176,26 +176,10 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
>  	{0x3032, 0x00},
>  	{0x3034, 0x4c},
>  	{0x3035, 0x04},
> -	{0x302c, 0xf0},
> -	{0x302d, 0x03},
> -	{0x302e, 0x80},
> -	{0x302f, 0x07},
> -	{0x3074, 0xcc},
> -	{0x3075, 0x02},
> -	{0x308e, 0xcd},
> -	{0x308f, 0x02},
> -	{0x3076, 0x38},
> -	{0x3077, 0x04},
> -	{0x3090, 0x38},
> -	{0x3091, 0x04},
> -	{0x3308, 0x38},
> -	{0x3309, 0x04},
> -	{0x30C6, 0x00},
> +	{0x30c6, 0x00},
>  	{0x30c7, 0x00},
>  	{0x30ce, 0x00},
>  	{0x30cf, 0x00},
> -	{0x30d8, 0x18},
> -	{0x30d9, 0x0a},
>  	{0x304c, 0x00},
>  	{0x304e, 0x00},
>  	{0x304f, 0x00},
> @@ -210,7 +194,7 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
>  	{0x300d, 0x29},
>  	{0x314c, 0x29},
>  	{0x314d, 0x01},
> -	{0x315a, 0x06},
> +	{0x315a, 0x0a},
>  	{0x3168, 0xa0},
>  	{0x316a, 0x7e},
>  	{0x319e, 0x02},
> @@ -330,6 +314,66 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
>  	{0x3002, 0x00},
>  };
> 
> +/* Sensor mode registers for 640x480@30fps */ static const struct
> +imx334_reg mode_640x480_regs[] = {
> +	{0x302c, 0x70},
> +	{0x302d, 0x06},
> +	{0x302e, 0x80},
> +	{0x302f, 0x02},
> +	{0x3074, 0x48},
> +	{0x3075, 0x07},
> +	{0x308e, 0x49},
> +	{0x308f, 0x07},
> +	{0x3076, 0xe0},
> +	{0x3077, 0x01},
> +	{0x3090, 0xe0},
> +	{0x3091, 0x01},
> +	{0x3308, 0xe0},
> +	{0x3309, 0x01},
> +	{0x30d8, 0x30},
> +	{0x30d9, 0x0b},
> +};
> +
> +/* Sensor mode registers for 1280x720@30fps */ static const struct
> +imx334_reg mode_1280x720_regs[] = {
> +	{0x302c, 0x30},
> +	{0x302d, 0x05},
> +	{0x302e, 0x00},
> +	{0x302f, 0x05},
> +	{0x3074, 0x84},
> +	{0x3075, 0x03},
> +	{0x308e, 0x85},
> +	{0x308f, 0x03},
> +	{0x3076, 0xd0},
> +	{0x3077, 0x02},
> +	{0x3090, 0xd0},
> +	{0x3091, 0x02},
> +	{0x3308, 0xd0},
> +	{0x3309, 0x02},
> +	{0x30d8, 0x30},
> +	{0x30d9, 0x0b},
> +};
> +
> +/* Sensor mode registers for 1920x1080@30fps */ static const struct
> +imx334_reg mode_1920x1080_regs[] = {
> +	{0x302c, 0xf0},
> +	{0x302d, 0x03},
> +	{0x302e, 0x80},
> +	{0x302f, 0x07},
> +	{0x3074, 0xcc},
> +	{0x3075, 0x02},
> +	{0x308e, 0xcd},
> +	{0x308f, 0x02},
> +	{0x3076, 0x38},
> +	{0x3077, 0x04},
> +	{0x3090, 0x38},
> +	{0x3091, 0x04},
> +	{0x3308, 0x38},
> +	{0x3309, 0x04},
> +	{0x30d8, 0x18},
> +	{0x30d9, 0x0a},
> +};
> +
>  /* Sensor mode registers for 3840x2160@30fps */  static const struct
> imx334_reg mode_3840x2160_regs[] = {
>  	{0x3000, 0x01},
> @@ -505,6 +549,32 @@ static const struct imx334_mode
> supported_modes[] = {
>  			.num_of_regs =
> ARRAY_SIZE(mode_1920x1080_regs),
>  			.regs = mode_1920x1080_regs,
>  		},
> +	}, {
> +		.width = 1280,
> +		.height = 720,
> +		.hblank = 2480,
> +		.vblank = 1170,
> +		.vblank_min = 45,
> +		.vblank_max = 132840,
> +		.pclk = 297000000,
> +		.link_freq_idx = 1,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
> +			.regs = mode_1280x720_regs,
> +		},
> +	}, {
> +		.width = 640,
> +		.height = 480,
> +		.hblank = 2480,
> +		.vblank = 1170,
> +		.vblank_min = 45,
> +		.vblank_max = 132840,
> +		.pclk = 297000000,
> +		.link_freq_idx = 1,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_640x480_regs),
> +			.regs = mode_640x480_regs,
> +		},
>  	},
>  };
> 
> @@ -989,6 +1059,15 @@ static int imx334_start_streaming(struct imx334
> *imx334)
>  	const struct imx334_reg_list *reg_list;
>  	int ret;
> 
> +	if (link_freq[imx334->cur_mode->link_freq_idx] ==
> IMX334_LINK_FREQ_445M) {
> +		ret = imx334_write_regs(imx334,
> common_mode_regs_445m,
> +
> 	ARRAY_SIZE(common_mode_regs_445m));
> +		if (ret) {
> +			dev_err(imx334->dev, "fail to write common
> registers");
> +			return ret;
> +		}
> +	}
> +
>  	/* Write sensor mode registers */
>  	reg_list = &imx334->cur_mode->reg_list;
>  	ret = imx334_write_regs(imx334, reg_list->regs,
> --
> 2.34.1
Sakari Ailus Feb. 12, 2025, 12:45 p.m. UTC | #2
Hi Shravan,

Apologies for the late review.

On Mon, Dec 16, 2024 at 10:09:26AM +0530, shravan kumar wrote:
> From: Shravan Chippa <shravan.chippa@microchip.com>
> 
> Add support for 1280x720@30 and 640x480@30 resolutions
> optimized the resolution arrays by added common register
> array
> 
> Updated 1920x1080@30 resolution register array with
> common register array
> 
> Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> ---
> 
> changelog:
> 
> v2 -> v3
> Removied blank lines reported by media CI robot 
> 
> v1 -> v2
> Optimized the resolution arrays by added common register array
> 
> ---
>  drivers/media/i2c/imx334.c | 119 ++++++++++++++++++++++++++++++-------
>  1 file changed, 99 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index a544fc3df39c..58ad67dbb331 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -167,8 +167,8 @@ static const s64 link_freq[] = {
>  	IMX334_LINK_FREQ_445M,
>  };
>  
> -/* Sensor mode registers for 1920x1080@30fps */
> -static const struct imx334_reg mode_1920x1080_regs[] = {
> +/* Sensor common mode registers for 445M link frequency */
> +static const struct imx334_reg common_mode_regs_445m[] = {
>  	{0x3000, 0x01},
>  	{0x3018, 0x04},
>  	{0x3030, 0xca},
> @@ -176,26 +176,10 @@ static const struct imx334_reg mode_1920x1080_regs[] = {
>  	{0x3032, 0x00},
>  	{0x3034, 0x4c},
>  	{0x3035, 0x04},
> -	{0x302c, 0xf0},
> -	{0x302d, 0x03},
> -	{0x302e, 0x80},
> -	{0x302f, 0x07},
> -	{0x3074, 0xcc},
> -	{0x3075, 0x02},
> -	{0x308e, 0xcd},
> -	{0x308f, 0x02},
> -	{0x3076, 0x38},
> -	{0x3077, 0x04},
> -	{0x3090, 0x38},
> -	{0x3091, 0x04},
> -	{0x3308, 0x38},
> -	{0x3309, 0x04},
> -	{0x30C6, 0x00},
> +	{0x30c6, 0x00},
>  	{0x30c7, 0x00},
>  	{0x30ce, 0x00},
>  	{0x30cf, 0x00},
> -	{0x30d8, 0x18},
> -	{0x30d9, 0x0a},
>  	{0x304c, 0x00},
>  	{0x304e, 0x00},
>  	{0x304f, 0x00},
> @@ -210,7 +194,7 @@ static const struct imx334_reg mode_1920x1080_regs[] = {
>  	{0x300d, 0x29},
>  	{0x314c, 0x29},
>  	{0x314d, 0x01},
> -	{0x315a, 0x06},
> +	{0x315a, 0x0a},
>  	{0x3168, 0xa0},
>  	{0x316a, 0x7e},
>  	{0x319e, 0x02},
> @@ -330,6 +314,66 @@ static const struct imx334_reg mode_1920x1080_regs[] = {
>  	{0x3002, 0x00},
>  };
>  
> +/* Sensor mode registers for 640x480@30fps */
> +static const struct imx334_reg mode_640x480_regs[] = {
> +	{0x302c, 0x70},
> +	{0x302d, 0x06},
> +	{0x302e, 0x80},
> +	{0x302f, 0x02},
> +	{0x3074, 0x48},
> +	{0x3075, 0x07},
> +	{0x308e, 0x49},
> +	{0x308f, 0x07},
> +	{0x3076, 0xe0},
> +	{0x3077, 0x01},
> +	{0x3090, 0xe0},
> +	{0x3091, 0x01},
> +	{0x3308, 0xe0},
> +	{0x3309, 0x01},
> +	{0x30d8, 0x30},
> +	{0x30d9, 0x0b},
> +};
> +
> +/* Sensor mode registers for 1280x720@30fps */
> +static const struct imx334_reg mode_1280x720_regs[] = {
> +	{0x302c, 0x30},
> +	{0x302d, 0x05},
> +	{0x302e, 0x00},
> +	{0x302f, 0x05},
> +	{0x3074, 0x84},
> +	{0x3075, 0x03},
> +	{0x308e, 0x85},
> +	{0x308f, 0x03},
> +	{0x3076, 0xd0},
> +	{0x3077, 0x02},
> +	{0x3090, 0xd0},
> +	{0x3091, 0x02},
> +	{0x3308, 0xd0},
> +	{0x3309, 0x02},
> +	{0x30d8, 0x30},
> +	{0x30d9, 0x0b},
> +};
> +
> +/* Sensor mode registers for 1920x1080@30fps */
> +static const struct imx334_reg mode_1920x1080_regs[] = {
> +	{0x302c, 0xf0},
> +	{0x302d, 0x03},
> +	{0x302e, 0x80},
> +	{0x302f, 0x07},
> +	{0x3074, 0xcc},
> +	{0x3075, 0x02},
> +	{0x308e, 0xcd},
> +	{0x308f, 0x02},
> +	{0x3076, 0x38},
> +	{0x3077, 0x04},
> +	{0x3090, 0x38},
> +	{0x3091, 0x04},
> +	{0x3308, 0x38},
> +	{0x3309, 0x04},
> +	{0x30d8, 0x18},
> +	{0x30d9, 0x0a},
> +};
> +
>  /* Sensor mode registers for 3840x2160@30fps */
>  static const struct imx334_reg mode_3840x2160_regs[] = {
>  	{0x3000, 0x01},
> @@ -505,6 +549,32 @@ static const struct imx334_mode supported_modes[] = {
>  			.num_of_regs = ARRAY_SIZE(mode_1920x1080_regs),
>  			.regs = mode_1920x1080_regs,
>  		},
> +	}, {
> +		.width = 1280,
> +		.height = 720,
> +		.hblank = 2480,
> +		.vblank = 1170,
> +		.vblank_min = 45,
> +		.vblank_max = 132840,
> +		.pclk = 297000000,
> +		.link_freq_idx = 1,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
> +			.regs = mode_1280x720_regs,
> +		},
> +	}, {
> +		.width = 640,
> +		.height = 480,
> +		.hblank = 2480,
> +		.vblank = 1170,
> +		.vblank_min = 45,
> +		.vblank_max = 132840,
> +		.pclk = 297000000,
> +		.link_freq_idx = 1,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_640x480_regs),
> +			.regs = mode_640x480_regs,
> +		},
>  	},
>  };
>  
> @@ -989,6 +1059,15 @@ static int imx334_start_streaming(struct imx334 *imx334)
>  	const struct imx334_reg_list *reg_list;
>  	int ret;
>  
> +	if (link_freq[imx334->cur_mode->link_freq_idx] == IMX334_LINK_FREQ_445M) {

Could you add a struct field for the common registers and the length of the
list, instead of referring to the link frequency index?

It'd be nice to have the same split done to the other frequency as well.

> +		ret = imx334_write_regs(imx334, common_mode_regs_445m,
> +					ARRAY_SIZE(common_mode_regs_445m));
> +		if (ret) {
> +			dev_err(imx334->dev, "fail to write common registers");
> +			return ret;
> +		}
> +	}
> +
>  	/* Write sensor mode registers */
>  	reg_list = &imx334->cur_mode->reg_list;
>  	ret = imx334_write_regs(imx334, reg_list->regs,
shravan kumar Feb. 13, 2025, 3:51 a.m. UTC | #3
Hi Sakari,


> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> Sent: Wednesday, February 12, 2025 6:15 PM
> To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>
> Cc: mchehab@kernel.org; kieran.bingham@ideasonboard.com; linux-
> media@vger.kernel.org; linux-kernel@vger.kernel.org; Conor Dooley -
> M52691 <Conor.Dooley@microchip.com>; Valentina Fernandez Alanis -
> M63239 <Valentina.FernandezAlanis@microchip.com>; Praveen Kumar -
> I30718 <Praveen.Kumar@microchip.com>
> Subject: Re: [PATCH V3] media: i2c: imx334: Add support for 1280x720 &
> 640x480 resolutions
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Hi Shravan,
> 
> Apologies for the late review.
> 
> On Mon, Dec 16, 2024 at 10:09:26AM +0530, shravan kumar wrote:
> > From: Shravan Chippa <shravan.chippa@microchip.com>
> >
> > Add support for 1280x720@30 and 640x480@30 resolutions optimized the
> > resolution arrays by added common register array
> >
> > Updated 1920x1080@30 resolution register array with common register
> > array
> >
> > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> > ---
> >
> > changelog:
> >
> > v2 -> v3
> > Removied blank lines reported by media CI robot
> >
> > v1 -> v2
> > Optimized the resolution arrays by added common register array
> >
> > ---
> >  drivers/media/i2c/imx334.c | 119
> > ++++++++++++++++++++++++++++++-------
> >  1 file changed, 99 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> > index a544fc3df39c..58ad67dbb331 100644
> > --- a/drivers/media/i2c/imx334.c
> > +++ b/drivers/media/i2c/imx334.c
> > @@ -167,8 +167,8 @@ static const s64 link_freq[] = {
> >       IMX334_LINK_FREQ_445M,
> >  };
> >
> > -/* Sensor mode registers for 1920x1080@30fps */ -static const struct
> > imx334_reg mode_1920x1080_regs[] = {
> > +/* Sensor common mode registers for 445M link frequency */ static
> > +const struct imx334_reg common_mode_regs_445m[] = {
> >       {0x3000, 0x01},
> >       {0x3018, 0x04},
> >       {0x3030, 0xca},
> > @@ -176,26 +176,10 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
> >       {0x3032, 0x00},
> >       {0x3034, 0x4c},
> >       {0x3035, 0x04},
> > -     {0x302c, 0xf0},
> > -     {0x302d, 0x03},
> > -     {0x302e, 0x80},
> > -     {0x302f, 0x07},
> > -     {0x3074, 0xcc},
> > -     {0x3075, 0x02},
> > -     {0x308e, 0xcd},
> > -     {0x308f, 0x02},
> > -     {0x3076, 0x38},
> > -     {0x3077, 0x04},
> > -     {0x3090, 0x38},
> > -     {0x3091, 0x04},
> > -     {0x3308, 0x38},
> > -     {0x3309, 0x04},
> > -     {0x30C6, 0x00},
> > +     {0x30c6, 0x00},
> >       {0x30c7, 0x00},
> >       {0x30ce, 0x00},
> >       {0x30cf, 0x00},
> > -     {0x30d8, 0x18},
> > -     {0x30d9, 0x0a},
> >       {0x304c, 0x00},
> >       {0x304e, 0x00},
> >       {0x304f, 0x00},
> > @@ -210,7 +194,7 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
> >       {0x300d, 0x29},
> >       {0x314c, 0x29},
> >       {0x314d, 0x01},
> > -     {0x315a, 0x06},
> > +     {0x315a, 0x0a},
> >       {0x3168, 0xa0},
> >       {0x316a, 0x7e},
> >       {0x319e, 0x02},
> > @@ -330,6 +314,66 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
> >       {0x3002, 0x00},
> >  };
> >
> > +/* Sensor mode registers for 640x480@30fps */ static const struct
> > +imx334_reg mode_640x480_regs[] = {
> > +     {0x302c, 0x70},
> > +     {0x302d, 0x06},
> > +     {0x302e, 0x80},
> > +     {0x302f, 0x02},
> > +     {0x3074, 0x48},
> > +     {0x3075, 0x07},
> > +     {0x308e, 0x49},
> > +     {0x308f, 0x07},
> > +     {0x3076, 0xe0},
> > +     {0x3077, 0x01},
> > +     {0x3090, 0xe0},
> > +     {0x3091, 0x01},
> > +     {0x3308, 0xe0},
> > +     {0x3309, 0x01},
> > +     {0x30d8, 0x30},
> > +     {0x30d9, 0x0b},
> > +};
> > +
> > +/* Sensor mode registers for 1280x720@30fps */ static const struct
> > +imx334_reg mode_1280x720_regs[] = {
> > +     {0x302c, 0x30},
> > +     {0x302d, 0x05},
> > +     {0x302e, 0x00},
> > +     {0x302f, 0x05},
> > +     {0x3074, 0x84},
> > +     {0x3075, 0x03},
> > +     {0x308e, 0x85},
> > +     {0x308f, 0x03},
> > +     {0x3076, 0xd0},
> > +     {0x3077, 0x02},
> > +     {0x3090, 0xd0},
> > +     {0x3091, 0x02},
> > +     {0x3308, 0xd0},
> > +     {0x3309, 0x02},
> > +     {0x30d8, 0x30},
> > +     {0x30d9, 0x0b},
> > +};
> > +
> > +/* Sensor mode registers for 1920x1080@30fps */ static const struct
> > +imx334_reg mode_1920x1080_regs[] = {
> > +     {0x302c, 0xf0},
> > +     {0x302d, 0x03},
> > +     {0x302e, 0x80},
> > +     {0x302f, 0x07},
> > +     {0x3074, 0xcc},
> > +     {0x3075, 0x02},
> > +     {0x308e, 0xcd},
> > +     {0x308f, 0x02},
> > +     {0x3076, 0x38},
> > +     {0x3077, 0x04},
> > +     {0x3090, 0x38},
> > +     {0x3091, 0x04},
> > +     {0x3308, 0x38},
> > +     {0x3309, 0x04},
> > +     {0x30d8, 0x18},
> > +     {0x30d9, 0x0a},
> > +};
> > +
> >  /* Sensor mode registers for 3840x2160@30fps */  static const struct
> > imx334_reg mode_3840x2160_regs[] = {
> >       {0x3000, 0x01},
> > @@ -505,6 +549,32 @@ static const struct imx334_mode
> supported_modes[] = {
> >                       .num_of_regs = ARRAY_SIZE(mode_1920x1080_regs),
> >                       .regs = mode_1920x1080_regs,
> >               },
> > +     }, {
> > +             .width = 1280,
> > +             .height = 720,
> > +             .hblank = 2480,
> > +             .vblank = 1170,
> > +             .vblank_min = 45,
> > +             .vblank_max = 132840,
> > +             .pclk = 297000000,
> > +             .link_freq_idx = 1,
> > +             .reg_list = {
> > +                     .num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
> > +                     .regs = mode_1280x720_regs,
> > +             },
> > +     }, {
> > +             .width = 640,
> > +             .height = 480,
> > +             .hblank = 2480,
> > +             .vblank = 1170,
> > +             .vblank_min = 45,
> > +             .vblank_max = 132840,
> > +             .pclk = 297000000,
> > +             .link_freq_idx = 1,
> > +             .reg_list = {
> > +                     .num_of_regs = ARRAY_SIZE(mode_640x480_regs),
> > +                     .regs = mode_640x480_regs,
> > +             },
> >       },
> >  };
> >
> > @@ -989,6 +1059,15 @@ static int imx334_start_streaming(struct imx334
> *imx334)
> >       const struct imx334_reg_list *reg_list;
> >       int ret;
> >
> > +     if (link_freq[imx334->cur_mode->link_freq_idx] ==
> > + IMX334_LINK_FREQ_445M) {
> 
> Could you add a struct field for the common registers and the length of the
> list, instead of referring to the link frequency index?
> 
> It'd be nice to have the same split done to the other frequency as well.

All value validated with respect to one link frequency (LINK_FREQ_445M)
In this case i will try to get common register values for both link frequencies (819M and 445M). 

Thanks,
Shravan

> 
> > +             ret = imx334_write_regs(imx334, common_mode_regs_445m,
> > +                                     ARRAY_SIZE(common_mode_regs_445m));
> > +             if (ret) {
> > +                     dev_err(imx334->dev, "fail to write common registers");
> > +                     return ret;
> > +             }
> > +     }
> > +
> >       /* Write sensor mode registers */
> >       reg_list = &imx334->cur_mode->reg_list;
> >       ret = imx334_write_regs(imx334, reg_list->regs,
> 
> --
> Kind regards,
> 
> Sakari Ailus
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index a544fc3df39c..58ad67dbb331 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -167,8 +167,8 @@  static const s64 link_freq[] = {
 	IMX334_LINK_FREQ_445M,
 };
 
-/* Sensor mode registers for 1920x1080@30fps */
-static const struct imx334_reg mode_1920x1080_regs[] = {
+/* Sensor common mode registers for 445M link frequency */
+static const struct imx334_reg common_mode_regs_445m[] = {
 	{0x3000, 0x01},
 	{0x3018, 0x04},
 	{0x3030, 0xca},
@@ -176,26 +176,10 @@  static const struct imx334_reg mode_1920x1080_regs[] = {
 	{0x3032, 0x00},
 	{0x3034, 0x4c},
 	{0x3035, 0x04},
-	{0x302c, 0xf0},
-	{0x302d, 0x03},
-	{0x302e, 0x80},
-	{0x302f, 0x07},
-	{0x3074, 0xcc},
-	{0x3075, 0x02},
-	{0x308e, 0xcd},
-	{0x308f, 0x02},
-	{0x3076, 0x38},
-	{0x3077, 0x04},
-	{0x3090, 0x38},
-	{0x3091, 0x04},
-	{0x3308, 0x38},
-	{0x3309, 0x04},
-	{0x30C6, 0x00},
+	{0x30c6, 0x00},
 	{0x30c7, 0x00},
 	{0x30ce, 0x00},
 	{0x30cf, 0x00},
-	{0x30d8, 0x18},
-	{0x30d9, 0x0a},
 	{0x304c, 0x00},
 	{0x304e, 0x00},
 	{0x304f, 0x00},
@@ -210,7 +194,7 @@  static const struct imx334_reg mode_1920x1080_regs[] = {
 	{0x300d, 0x29},
 	{0x314c, 0x29},
 	{0x314d, 0x01},
-	{0x315a, 0x06},
+	{0x315a, 0x0a},
 	{0x3168, 0xa0},
 	{0x316a, 0x7e},
 	{0x319e, 0x02},
@@ -330,6 +314,66 @@  static const struct imx334_reg mode_1920x1080_regs[] = {
 	{0x3002, 0x00},
 };
 
+/* Sensor mode registers for 640x480@30fps */
+static const struct imx334_reg mode_640x480_regs[] = {
+	{0x302c, 0x70},
+	{0x302d, 0x06},
+	{0x302e, 0x80},
+	{0x302f, 0x02},
+	{0x3074, 0x48},
+	{0x3075, 0x07},
+	{0x308e, 0x49},
+	{0x308f, 0x07},
+	{0x3076, 0xe0},
+	{0x3077, 0x01},
+	{0x3090, 0xe0},
+	{0x3091, 0x01},
+	{0x3308, 0xe0},
+	{0x3309, 0x01},
+	{0x30d8, 0x30},
+	{0x30d9, 0x0b},
+};
+
+/* Sensor mode registers for 1280x720@30fps */
+static const struct imx334_reg mode_1280x720_regs[] = {
+	{0x302c, 0x30},
+	{0x302d, 0x05},
+	{0x302e, 0x00},
+	{0x302f, 0x05},
+	{0x3074, 0x84},
+	{0x3075, 0x03},
+	{0x308e, 0x85},
+	{0x308f, 0x03},
+	{0x3076, 0xd0},
+	{0x3077, 0x02},
+	{0x3090, 0xd0},
+	{0x3091, 0x02},
+	{0x3308, 0xd0},
+	{0x3309, 0x02},
+	{0x30d8, 0x30},
+	{0x30d9, 0x0b},
+};
+
+/* Sensor mode registers for 1920x1080@30fps */
+static const struct imx334_reg mode_1920x1080_regs[] = {
+	{0x302c, 0xf0},
+	{0x302d, 0x03},
+	{0x302e, 0x80},
+	{0x302f, 0x07},
+	{0x3074, 0xcc},
+	{0x3075, 0x02},
+	{0x308e, 0xcd},
+	{0x308f, 0x02},
+	{0x3076, 0x38},
+	{0x3077, 0x04},
+	{0x3090, 0x38},
+	{0x3091, 0x04},
+	{0x3308, 0x38},
+	{0x3309, 0x04},
+	{0x30d8, 0x18},
+	{0x30d9, 0x0a},
+};
+
 /* Sensor mode registers for 3840x2160@30fps */
 static const struct imx334_reg mode_3840x2160_regs[] = {
 	{0x3000, 0x01},
@@ -505,6 +549,32 @@  static const struct imx334_mode supported_modes[] = {
 			.num_of_regs = ARRAY_SIZE(mode_1920x1080_regs),
 			.regs = mode_1920x1080_regs,
 		},
+	}, {
+		.width = 1280,
+		.height = 720,
+		.hblank = 2480,
+		.vblank = 1170,
+		.vblank_min = 45,
+		.vblank_max = 132840,
+		.pclk = 297000000,
+		.link_freq_idx = 1,
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
+			.regs = mode_1280x720_regs,
+		},
+	}, {
+		.width = 640,
+		.height = 480,
+		.hblank = 2480,
+		.vblank = 1170,
+		.vblank_min = 45,
+		.vblank_max = 132840,
+		.pclk = 297000000,
+		.link_freq_idx = 1,
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_640x480_regs),
+			.regs = mode_640x480_regs,
+		},
 	},
 };
 
@@ -989,6 +1059,15 @@  static int imx334_start_streaming(struct imx334 *imx334)
 	const struct imx334_reg_list *reg_list;
 	int ret;
 
+	if (link_freq[imx334->cur_mode->link_freq_idx] == IMX334_LINK_FREQ_445M) {
+		ret = imx334_write_regs(imx334, common_mode_regs_445m,
+					ARRAY_SIZE(common_mode_regs_445m));
+		if (ret) {
+			dev_err(imx334->dev, "fail to write common registers");
+			return ret;
+		}
+	}
+
 	/* Write sensor mode registers */
 	reg_list = &imx334->cur_mode->reg_list;
 	ret = imx334_write_regs(imx334, reg_list->regs,