* [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
@ 2024-12-16 4:39 shravan kumar
2025-01-06 10:18 ` Shravan.Chippa
2025-02-12 12:45 ` Sakari Ailus
0 siblings, 2 replies; 4+ messages in thread
From: shravan kumar @ 2024-12-16 4:39 UTC (permalink / raw)
To: sakari.ailus, mchehab
Cc: kieran.bingham, linux-media, linux-kernel, conor.dooley,
valentina.fernandezalanis, praveen.kumar, shravan.chippa
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
2024-12-16 4:39 [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
@ 2025-01-06 10:18 ` Shravan.Chippa
2025-02-12 12:45 ` Sakari Ailus
1 sibling, 0 replies; 4+ messages in thread
From: Shravan.Chippa @ 2025-01-06 10:18 UTC (permalink / raw)
To: Shravan.Chippa, sakari.ailus, mchehab
Cc: kieran.bingham, linux-media, linux-kernel, Conor.Dooley,
Valentina.FernandezAlanis, Praveen.Kumar
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
2024-12-16 4:39 [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
2025-01-06 10:18 ` Shravan.Chippa
@ 2025-02-12 12:45 ` Sakari Ailus
2025-02-13 3:51 ` Shravan.Chippa
1 sibling, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2025-02-12 12:45 UTC (permalink / raw)
To: shravan kumar
Cc: mchehab, kieran.bingham, linux-media, linux-kernel, conor.dooley,
valentina.fernandezalanis, praveen.kumar
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,
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
2025-02-12 12:45 ` Sakari Ailus
@ 2025-02-13 3:51 ` Shravan.Chippa
0 siblings, 0 replies; 4+ messages in thread
From: Shravan.Chippa @ 2025-02-13 3:51 UTC (permalink / raw)
To: sakari.ailus
Cc: mchehab, kieran.bingham, linux-media, linux-kernel, Conor.Dooley,
Valentina.FernandezAlanis, Praveen.Kumar
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-02-13 3:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-16 4:39 [PATCH V3] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
2025-01-06 10:18 ` Shravan.Chippa
2025-02-12 12:45 ` Sakari Ailus
2025-02-13 3:51 ` Shravan.Chippa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®