* [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
* 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
* [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
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®