* [PATCH V5 0/2] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
@ 2025-02-25 6:26 shravan kumar
2025-02-25 6:26 ` [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays shravan kumar
2025-02-25 6:26 ` [PATCH V5 2/2] media: i2c: imx334: add modes for 720p and 480p resolutions shravan kumar
0 siblings, 2 replies; 6+ messages in thread
From: shravan kumar @ 2025-02-25 6:26 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>
Changelog:
v4 -> v5:
Divided the patch into two separate patches:
- One for common register array values with existing resolutions
- Another for new modes supporting 640x480 and 1280x720 resolutions
v3 -> v4:
In response to the review request, the dependency of the common register value array
on the 445M link frequency has been eliminated. Although I do not have a test setup for
4k resolution at an 819M link frequency, I have made adjustments based on the IMX334
data sheet, the latest 4k resolution, and the common register value array.
Additionally, the 4k resolution has been switched to master mode to ensure consistency
with other resolutions that also use master mode.
v2 -> v3:
removied blank lines reported by media CI robot
v1 -> v2:
Optimized the resolution arrays by added common register
array
---
Shravan Chippa (2):
media: i2c: imx334: Optimized 4k and 2k mode register arrays
media: i2c: imx334: add modes for 720p and 480p resolutions
drivers/media/i2c/imx334.c | 214 +++++++++++++++++++------------------
1 file changed, 109 insertions(+), 105 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays
2025-02-25 6:26 [PATCH V5 0/2] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
@ 2025-02-25 6:26 ` shravan kumar
2025-02-26 14:28 ` Sakari Ailus
2025-02-25 6:26 ` [PATCH V5 2/2] media: i2c: imx334: add modes for 720p and 480p resolutions shravan kumar
1 sibling, 1 reply; 6+ messages in thread
From: shravan kumar @ 2025-02-25 6:26 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>
Optimized the resolution arrays by integrating a common register array.
Adjusted the register array values for 1920x1080@30 and 3840x2160@30
resolutions to align with the common register array values.
Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
---
drivers/media/i2c/imx334.c | 148 +++++++++++--------------------------
1 file changed, 43 insertions(+), 105 deletions(-)
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index a544fc3df39c..b2ad19abaca8 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 values */
+static const struct imx334_reg common_mode_regs[] = {
{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,116 +314,63 @@ static const struct imx334_reg mode_1920x1080_regs[] = {
{0x3002, 0x00},
};
+/* 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},
- {0x3002, 0x00},
- {0x3018, 0x04},
- {0x37b0, 0x36},
- {0x304c, 0x00},
- {0x300c, 0x3b},
- {0x300d, 0x2a},
- {0x3034, 0x26},
- {0x3035, 0x02},
- {0x314c, 0x29},
- {0x314d, 0x01},
- {0x315a, 0x02},
- {0x3168, 0xa0},
- {0x316a, 0x7e},
- {0x3288, 0x21},
- {0x328a, 0x02},
{0x302c, 0x3c},
{0x302d, 0x00},
{0x302e, 0x00},
{0x302f, 0x0f},
+ {0x3074, 0xb0},
+ {0x3075, 0x00},
+ {0x308e, 0xb1},
+ {0x308f, 0x00},
{0x3076, 0x70},
{0x3077, 0x08},
{0x3090, 0x70},
{0x3091, 0x08},
- {0x30d8, 0x20},
- {0x30d9, 0x12},
{0x3308, 0x70},
{0x3309, 0x08},
- {0x3414, 0x05},
- {0x3416, 0x18},
- {0x35ac, 0x0e},
- {0x3648, 0x01},
- {0x364a, 0x04},
- {0x364c, 0x04},
- {0x3678, 0x01},
- {0x367c, 0x31},
- {0x367e, 0x31},
- {0x3708, 0x02},
- {0x3714, 0x01},
- {0x3715, 0x02},
- {0x3716, 0x02},
- {0x3717, 0x02},
- {0x371c, 0x3d},
- {0x371d, 0x3f},
- {0x372c, 0x00},
- {0x372d, 0x00},
- {0x372e, 0x46},
- {0x372f, 0x00},
- {0x3730, 0x89},
- {0x3731, 0x00},
- {0x3732, 0x08},
- {0x3733, 0x01},
- {0x3734, 0xfe},
- {0x3735, 0x05},
- {0x375d, 0x00},
- {0x375e, 0x00},
- {0x375f, 0x61},
- {0x3760, 0x06},
- {0x3768, 0x1b},
- {0x3769, 0x1b},
- {0x376a, 0x1a},
- {0x376b, 0x19},
- {0x376c, 0x18},
- {0x376d, 0x14},
- {0x376e, 0x0f},
- {0x3776, 0x00},
- {0x3777, 0x00},
- {0x3778, 0x46},
- {0x3779, 0x00},
- {0x377a, 0x08},
- {0x377b, 0x01},
- {0x377c, 0x45},
- {0x377d, 0x01},
- {0x377e, 0x23},
- {0x377f, 0x02},
- {0x3780, 0xd9},
- {0x3781, 0x03},
- {0x3782, 0xf5},
- {0x3783, 0x06},
- {0x3784, 0xa5},
- {0x3788, 0x0f},
- {0x378a, 0xd9},
- {0x378b, 0x03},
- {0x378c, 0xeb},
- {0x378d, 0x05},
- {0x378e, 0x87},
- {0x378f, 0x06},
- {0x3790, 0xf5},
- {0x3792, 0x43},
- {0x3794, 0x7a},
- {0x3796, 0xa1},
- {0x3e04, 0x0e},
+ {0x30d8, 0x20},
+ {0x30d9, 0x12},
{0x319e, 0x00},
{0x3a00, 0x01},
{0x3a18, 0xbf},
- {0x3a19, 0x00},
{0x3a1a, 0x67},
- {0x3a1b, 0x00},
{0x3a1c, 0x6f},
- {0x3a1d, 0x00},
{0x3a1e, 0xd7},
{0x3a1f, 0x01},
+ {0x300d, 0x2a},
+ {0x3034, 0x26},
+ {0x3035, 0x02},
+ {0x315a, 0x02},
{0x3a20, 0x6f},
{0x3a21, 0x00},
{0x3a22, 0xcf},
{0x3a23, 0x00},
{0x3a24, 0x6f},
{0x3a25, 0x00},
+ {0x3a24, 0x6f},
+ {0x3a25, 0x00},
{0x3a26, 0xb7},
{0x3a27, 0x00},
{0x3a28, 0x5f},
@@ -989,6 +920,13 @@ static int imx334_start_streaming(struct imx334 *imx334)
const struct imx334_reg_list *reg_list;
int ret;
+ ret = imx334_write_regs(imx334, common_mode_regs,
+ ARRAY_SIZE(common_mode_regs));
+ 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] 6+ messages in thread
* [PATCH V5 2/2] media: i2c: imx334: add modes for 720p and 480p resolutions
2025-02-25 6:26 [PATCH V5 0/2] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
2025-02-25 6:26 ` [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays shravan kumar
@ 2025-02-25 6:26 ` shravan kumar
1 sibling, 0 replies; 6+ messages in thread
From: shravan kumar @ 2025-02-25 6:26 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>
Added support for 1280x720@30 and 640x480@30 resolutions
Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
---
drivers/media/i2c/imx334.c | 66 ++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index b2ad19abaca8..0172406780df 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -314,6 +314,46 @@ static const struct imx334_reg common_mode_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},
@@ -436,6 +476,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,
+ },
},
};
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays
2025-02-25 6:26 ` [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays shravan kumar
@ 2025-02-26 14:28 ` Sakari Ailus
2025-02-27 5:04 ` Shravan.Chippa
0 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2025-02-26 14:28 UTC (permalink / raw)
To: shravan kumar
Cc: mchehab, kieran.bingham, linux-media, linux-kernel, conor.dooley,
valentina.fernandezalanis, praveen.kumar
Hi Shravan,
On Tue, Feb 25, 2025 at 11:56:34AM +0530, shravan kumar wrote:
> From: Shravan Chippa <shravan.chippa@microchip.com>
>
> Optimized the resolution arrays by integrating a common register array.
>
> Adjusted the register array values for 1920x1080@30 and 3840x2160@30
> resolutions to align with the common register array values.
>
> Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> ---
> drivers/media/i2c/imx334.c | 148 +++++++++++--------------------------
> 1 file changed, 43 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index a544fc3df39c..b2ad19abaca8 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 values */
> +static const struct imx334_reg common_mode_regs[] = {
> {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,116 +314,63 @@ static const struct imx334_reg mode_1920x1080_regs[] = {
> {0x3002, 0x00},
> };
>
> +/* 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},
> - {0x3002, 0x00},
> - {0x3018, 0x04},
> - {0x37b0, 0x36},
> - {0x304c, 0x00},
> - {0x300c, 0x3b},
> - {0x300d, 0x2a},
> - {0x3034, 0x26},
> - {0x3035, 0x02},
> - {0x314c, 0x29},
> - {0x314d, 0x01},
> - {0x315a, 0x02},
> - {0x3168, 0xa0},
> - {0x316a, 0x7e},
> - {0x3288, 0x21},
> - {0x328a, 0x02},
> {0x302c, 0x3c},
> {0x302d, 0x00},
> {0x302e, 0x00},
> {0x302f, 0x0f},
> + {0x3074, 0xb0},
> + {0x3075, 0x00},
> + {0x308e, 0xb1},
This register wasn't part of the original register list for the mode. It
seems to have been written to in the other (binned?) mode only. It looks
like a possible bugfix. Should it be in a separate patch? This patch is
only meant to reorganise register settings to a base set and modes, not
change the registers written in any way.
> + {0x308f, 0x00},
> {0x3076, 0x70},
> {0x3077, 0x08},
> {0x3090, 0x70},
> {0x3091, 0x08},
> - {0x30d8, 0x20},
> - {0x30d9, 0x12},
> {0x3308, 0x70},
> {0x3309, 0x08},
> - {0x3414, 0x05},
> - {0x3416, 0x18},
> - {0x35ac, 0x0e},
> - {0x3648, 0x01},
> - {0x364a, 0x04},
> - {0x364c, 0x04},
> - {0x3678, 0x01},
> - {0x367c, 0x31},
> - {0x367e, 0x31},
> - {0x3708, 0x02},
> - {0x3714, 0x01},
> - {0x3715, 0x02},
> - {0x3716, 0x02},
> - {0x3717, 0x02},
> - {0x371c, 0x3d},
> - {0x371d, 0x3f},
> - {0x372c, 0x00},
> - {0x372d, 0x00},
> - {0x372e, 0x46},
> - {0x372f, 0x00},
> - {0x3730, 0x89},
> - {0x3731, 0x00},
> - {0x3732, 0x08},
> - {0x3733, 0x01},
> - {0x3734, 0xfe},
> - {0x3735, 0x05},
> - {0x375d, 0x00},
> - {0x375e, 0x00},
> - {0x375f, 0x61},
> - {0x3760, 0x06},
> - {0x3768, 0x1b},
> - {0x3769, 0x1b},
> - {0x376a, 0x1a},
> - {0x376b, 0x19},
> - {0x376c, 0x18},
> - {0x376d, 0x14},
> - {0x376e, 0x0f},
> - {0x3776, 0x00},
> - {0x3777, 0x00},
> - {0x3778, 0x46},
> - {0x3779, 0x00},
> - {0x377a, 0x08},
> - {0x377b, 0x01},
> - {0x377c, 0x45},
> - {0x377d, 0x01},
> - {0x377e, 0x23},
> - {0x377f, 0x02},
> - {0x3780, 0xd9},
> - {0x3781, 0x03},
> - {0x3782, 0xf5},
> - {0x3783, 0x06},
> - {0x3784, 0xa5},
> - {0x3788, 0x0f},
> - {0x378a, 0xd9},
> - {0x378b, 0x03},
> - {0x378c, 0xeb},
> - {0x378d, 0x05},
> - {0x378e, 0x87},
> - {0x378f, 0x06},
> - {0x3790, 0xf5},
> - {0x3792, 0x43},
> - {0x3794, 0x7a},
> - {0x3796, 0xa1},
> - {0x3e04, 0x0e},
> + {0x30d8, 0x20},
> + {0x30d9, 0x12},
> {0x319e, 0x00},
> {0x3a00, 0x01},
> {0x3a18, 0xbf},
> - {0x3a19, 0x00},
> {0x3a1a, 0x67},
> - {0x3a1b, 0x00},
> {0x3a1c, 0x6f},
> - {0x3a1d, 0x00},
> {0x3a1e, 0xd7},
> {0x3a1f, 0x01},
> + {0x300d, 0x2a},
> + {0x3034, 0x26},
> + {0x3035, 0x02},
> + {0x315a, 0x02},
> {0x3a20, 0x6f},
> {0x3a21, 0x00},
> {0x3a22, 0xcf},
> {0x3a23, 0x00},
> {0x3a24, 0x6f},
> {0x3a25, 0x00},
> + {0x3a24, 0x6f},
> + {0x3a25, 0x00},
> {0x3a26, 0xb7},
> {0x3a27, 0x00},
> {0x3a28, 0x5f},
> @@ -989,6 +920,13 @@ static int imx334_start_streaming(struct imx334 *imx334)
> const struct imx334_reg_list *reg_list;
> int ret;
>
> + ret = imx334_write_regs(imx334, common_mode_regs,
> + ARRAY_SIZE(common_mode_regs));
> + 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] 6+ messages in thread
* RE: [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays
2025-02-26 14:28 ` Sakari Ailus
@ 2025-02-27 5:04 ` Shravan.Chippa
2025-02-27 9:21 ` Sakari Ailus
0 siblings, 1 reply; 6+ messages in thread
From: Shravan.Chippa @ 2025-02-27 5:04 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 26, 2025 7:59 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 V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode
> register arrays
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Hi Shravan,
>
> On Tue, Feb 25, 2025 at 11:56:34AM +0530, shravan kumar wrote:
> > From: Shravan Chippa <shravan.chippa@microchip.com>
> >
> > Optimized the resolution arrays by integrating a common register array.
> >
> > Adjusted the register array values for 1920x1080@30 and 3840x2160@30
> > resolutions to align with the common register array values.
> >
> > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> > ---
> > drivers/media/i2c/imx334.c | 148
> > +++++++++++--------------------------
> > 1 file changed, 43 insertions(+), 105 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> > index a544fc3df39c..b2ad19abaca8 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 values */ static const struct
> > +imx334_reg common_mode_regs[] = {
> > {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,116 +314,63 @@ static const struct imx334_reg
> mode_1920x1080_regs[] = {
> > {0x3002, 0x00},
> > };
> >
> > +/* 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},
> > - {0x3002, 0x00},
> > - {0x3018, 0x04},
> > - {0x37b0, 0x36},
> > - {0x304c, 0x00},
> > - {0x300c, 0x3b},
> > - {0x300d, 0x2a},
> > - {0x3034, 0x26},
> > - {0x3035, 0x02},
> > - {0x314c, 0x29},
> > - {0x314d, 0x01},
> > - {0x315a, 0x02},
> > - {0x3168, 0xa0},
> > - {0x316a, 0x7e},
> > - {0x3288, 0x21},
> > - {0x328a, 0x02},
> > {0x302c, 0x3c},
> > {0x302d, 0x00},
> > {0x302e, 0x00},
> > {0x302f, 0x0f},
> > + {0x3074, 0xb0},
> > + {0x3075, 0x00},
> > + {0x308e, 0xb1},
>
> This register wasn't part of the original register list for the mode. It seems to
> have been written to in the other (binned?) mode only. It looks like a possible
> bugfix. Should it be in a separate patch? This patch is only meant to reorganise
> register settings to a base set and modes, not change the registers written in
> any way.
Yes, it is correct that these values are not part of the original 3840x2160 register list. However, these registers have been modified in the other three mode registers array. The 3840x2160 mode operates with the reset value. If we switch to the other three modes and then return to the 3840x2160 mode, it should function correctly. Therefore, I am restoring the reset values.
Thanks,
Shravan
>
> > + {0x308f, 0x00},
> > {0x3076, 0x70},
> > {0x3077, 0x08},
> > {0x3090, 0x70},
> > {0x3091, 0x08},
> > - {0x30d8, 0x20},
> > - {0x30d9, 0x12},
> > {0x3308, 0x70},
> > {0x3309, 0x08},
> > - {0x3414, 0x05},
> > - {0x3416, 0x18},
> > - {0x35ac, 0x0e},
> > - {0x3648, 0x01},
> > - {0x364a, 0x04},
> > - {0x364c, 0x04},
> > - {0x3678, 0x01},
> > - {0x367c, 0x31},
> > - {0x367e, 0x31},
> > - {0x3708, 0x02},
> > - {0x3714, 0x01},
> > - {0x3715, 0x02},
> > - {0x3716, 0x02},
> > - {0x3717, 0x02},
> > - {0x371c, 0x3d},
> > - {0x371d, 0x3f},
> > - {0x372c, 0x00},
> > - {0x372d, 0x00},
> > - {0x372e, 0x46},
> > - {0x372f, 0x00},
> > - {0x3730, 0x89},
> > - {0x3731, 0x00},
> > - {0x3732, 0x08},
> > - {0x3733, 0x01},
> > - {0x3734, 0xfe},
> > - {0x3735, 0x05},
> > - {0x375d, 0x00},
> > - {0x375e, 0x00},
> > - {0x375f, 0x61},
> > - {0x3760, 0x06},
> > - {0x3768, 0x1b},
> > - {0x3769, 0x1b},
> > - {0x376a, 0x1a},
> > - {0x376b, 0x19},
> > - {0x376c, 0x18},
> > - {0x376d, 0x14},
> > - {0x376e, 0x0f},
> > - {0x3776, 0x00},
> > - {0x3777, 0x00},
> > - {0x3778, 0x46},
> > - {0x3779, 0x00},
> > - {0x377a, 0x08},
> > - {0x377b, 0x01},
> > - {0x377c, 0x45},
> > - {0x377d, 0x01},
> > - {0x377e, 0x23},
> > - {0x377f, 0x02},
> > - {0x3780, 0xd9},
> > - {0x3781, 0x03},
> > - {0x3782, 0xf5},
> > - {0x3783, 0x06},
> > - {0x3784, 0xa5},
> > - {0x3788, 0x0f},
> > - {0x378a, 0xd9},
> > - {0x378b, 0x03},
> > - {0x378c, 0xeb},
> > - {0x378d, 0x05},
> > - {0x378e, 0x87},
> > - {0x378f, 0x06},
> > - {0x3790, 0xf5},
> > - {0x3792, 0x43},
> > - {0x3794, 0x7a},
> > - {0x3796, 0xa1},
> > - {0x3e04, 0x0e},
> > + {0x30d8, 0x20},
> > + {0x30d9, 0x12},
> > {0x319e, 0x00},
> > {0x3a00, 0x01},
> > {0x3a18, 0xbf},
> > - {0x3a19, 0x00},
> > {0x3a1a, 0x67},
> > - {0x3a1b, 0x00},
> > {0x3a1c, 0x6f},
> > - {0x3a1d, 0x00},
> > {0x3a1e, 0xd7},
> > {0x3a1f, 0x01},
> > + {0x300d, 0x2a},
> > + {0x3034, 0x26},
> > + {0x3035, 0x02},
> > + {0x315a, 0x02},
> > {0x3a20, 0x6f},
> > {0x3a21, 0x00},
> > {0x3a22, 0xcf},
> > {0x3a23, 0x00},
> > {0x3a24, 0x6f},
> > {0x3a25, 0x00},
> > + {0x3a24, 0x6f},
> > + {0x3a25, 0x00},
> > {0x3a26, 0xb7},
> > {0x3a27, 0x00},
> > {0x3a28, 0x5f},
> > @@ -989,6 +920,13 @@ static int imx334_start_streaming(struct imx334
> *imx334)
> > const struct imx334_reg_list *reg_list;
> > int ret;
> >
> > + ret = imx334_write_regs(imx334, common_mode_regs,
> > + ARRAY_SIZE(common_mode_regs));
> > + 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] 6+ messages in thread
* Re: [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays
2025-02-27 5:04 ` Shravan.Chippa
@ 2025-02-27 9:21 ` Sakari Ailus
0 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2025-02-27 9:21 UTC (permalink / raw)
To: Shravan.Chippa
Cc: mchehab, kieran.bingham, linux-media, linux-kernel, Conor.Dooley,
Valentina.FernandezAlanis, Praveen.Kumar
Hi Shravan,
On Thu, Feb 27, 2025 at 05:04:11AM +0000, Shravan.Chippa@microchip.com wrote:
> Hi Sakari,
>
> > -----Original Message-----
> > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Sent: Wednesday, February 26, 2025 7:59 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 V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode
> > register arrays
> >
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > Hi Shravan,
> >
> > On Tue, Feb 25, 2025 at 11:56:34AM +0530, shravan kumar wrote:
> > > From: Shravan Chippa <shravan.chippa@microchip.com>
> > >
> > > Optimized the resolution arrays by integrating a common register array.
> > >
> > > Adjusted the register array values for 1920x1080@30 and 3840x2160@30
> > > resolutions to align with the common register array values.
> > >
> > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> > > ---
> > > drivers/media/i2c/imx334.c | 148
> > > +++++++++++--------------------------
> > > 1 file changed, 43 insertions(+), 105 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> > > index a544fc3df39c..b2ad19abaca8 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 values */ static const struct
> > > +imx334_reg common_mode_regs[] = {
> > > {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,116 +314,63 @@ static const struct imx334_reg
> > mode_1920x1080_regs[] = {
> > > {0x3002, 0x00},
> > > };
> > >
> > > +/* 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},
> > > - {0x3002, 0x00},
> > > - {0x3018, 0x04},
> > > - {0x37b0, 0x36},
> > > - {0x304c, 0x00},
> > > - {0x300c, 0x3b},
> > > - {0x300d, 0x2a},
> > > - {0x3034, 0x26},
> > > - {0x3035, 0x02},
> > > - {0x314c, 0x29},
> > > - {0x314d, 0x01},
> > > - {0x315a, 0x02},
> > > - {0x3168, 0xa0},
> > > - {0x316a, 0x7e},
> > > - {0x3288, 0x21},
> > > - {0x328a, 0x02},
> > > {0x302c, 0x3c},
> > > {0x302d, 0x00},
> > > {0x302e, 0x00},
> > > {0x302f, 0x0f},
> > > + {0x3074, 0xb0},
> > > + {0x3075, 0x00},
> > > + {0x308e, 0xb1},
> >
> > This register wasn't part of the original register list for the mode. It seems to
> > have been written to in the other (binned?) mode only. It looks like a possible
> > bugfix. Should it be in a separate patch? This patch is only meant to reorganise
> > register settings to a base set and modes, not change the registers written in
> > any way.
>
> Yes, it is correct that these values are not part of the original
> 3840x2160 register list. However, these registers have been modified in
> the other three mode registers array. The 3840x2160 mode operates with
> the reset value. If we switch to the other three modes and then return to
> the 3840x2160 mode, it should function correctly. Therefore, I am
> restoring the reset values.
Please do wrap at 75 characters when replying.
Such changes belong to separate patches. Please add further patches in such
cases.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-27 9:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-25 6:26 [PATCH V5 0/2] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
2025-02-25 6:26 ` [PATCH V5 1/2] media: i2c: imx334: Optimized 4k and 2k mode register arrays shravan kumar
2025-02-26 14:28 ` Sakari Ailus
2025-02-27 5:04 ` Shravan.Chippa
2025-02-27 9:21 ` Sakari Ailus
2025-02-25 6:26 ` [PATCH V5 2/2] media: i2c: imx334: add modes for 720p and 480p resolutions shravan kumar
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®