| Message ID | 20250228103332.3647098-1-shravan.chippa@microchip.com |
|---|---|
| Headers | show |
| Series | media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions | expand |
Hi Sakari, > -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: Friday, February 28, 2025 5:19 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 V6 1/3] 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 Fri, Feb 28, 2025 at 04:03:30PM +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 | 132 > > +++++++++---------------------------- > > 1 file changed, 31 insertions(+), 101 deletions(-) > > > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c > > index a544fc3df39c..a800f2203592 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}, > > We still have this change in the patch that's just supposed to move register > address/value pairs around. :-( Please check the changes yourself before > posting v7. Do I need to split the patch or drop this change ? in v7 Thanks, Shravan > > > {0x3168, 0xa0}, > > {0x316a, 0x7e}, > > {0x319e, 0x02}, > > @@ -330,24 +314,32 @@ 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}, > > @@ -356,82 +348,13 @@ static const struct imx334_reg > mode_3840x2160_regs[] = { > > {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}, > > {0x319e, 0x00}, > > {0x3a00, 0x01}, > > {0x3a18, 0xbf}, > > - {0x3a19, 0x00}, > > {0x3a1a, 0x67}, > > - {0x3a1b, 0x00}, > > {0x3a1c, 0x6f}, > > - {0x3a1d, 0x00}, > > {0x3a1e, 0xd7}, > > {0x3a1f, 0x01}, > > {0x3a20, 0x6f}, > > @@ -989,6 +912,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, > > -- > Regards, > > Sakari Ailus
From: Shravan Chippa <shravan.chippa@microchip.com> Changelog: v5 -> v6: split the patches from two to three 1) Optimized the resolution arrays by integrating a common register array 2) 3840x2160 mode restored with reset values of imx334 3) 1280x720@30 and 640x480@30 resolutions 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 (3): media: i2c: imx334: Optimized 4k and 2k mode register arrays media: i2c: imx334: update mode_3840x2160_regs array media: i2c: imx334: add modes for 720p and 480p resolutions drivers/media/i2c/imx334.c | 204 +++++++++++++++++++------------------ 1 file changed, 103 insertions(+), 101 deletions(-)