mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
@ 2024-11-29 10:00 shravan kumar
  2024-11-29 11:58 ` Kieran Bingham
  0 siblings, 1 reply; 5+ messages in thread
From: shravan kumar @ 2024-11-29 10:00 UTC (permalink / raw)
  To: sakari.ailus, mchehab
  Cc: 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

Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
---
 drivers/media/i2c/imx334.c | 352 +++++++++++++++++++++++++++++++++++++
 1 file changed, 352 insertions(+)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index a544fc3df39c..b7cdebc2382e 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -167,6 +167,332 @@ static const s64 link_freq[] = {
 	IMX334_LINK_FREQ_445M,
 };
 
+/* Sensor mode registers for 640x480@30fps */
+static const struct imx334_reg mode_640x480_regs[] = {
+	{0x3000, 0x01},
+	{0x3018, 0x04},
+	{0x3030, 0xca},
+	{0x3031, 0x08},
+	{0x3032, 0x00},
+	{0x3034, 0x4c},
+	{0x3035, 0x04},
+	{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},
+	{0x30C6, 0x00},
+	{0x30c7, 0x00},
+	{0x30ce, 0x00},
+	{0x30cf, 0x00},
+	{0x304c, 0x00},
+	{0x304e, 0x00},
+	{0x304f, 0x00},
+	{0x3050, 0x00},
+	{0x30b6, 0x00},
+	{0x30b7, 0x00},
+	{0x3116, 0x08},
+	{0x3117, 0x00},
+	{0x31a0, 0x20},
+	{0x31a1, 0x0f},
+	{0x300c, 0x3b},
+	{0x300d, 0x29},
+	{0x314c, 0x29},
+	{0x314d, 0x01},
+	{0x315a, 0x0a},
+	{0x3168, 0xa0},
+	{0x316a, 0x7e},
+	{0x319e, 0x02},
+	{0x3199, 0x00},
+	{0x319d, 0x00},
+	{0x31dd, 0x03},
+	{0x3300, 0x00},
+	{0x341c, 0xff},
+	{0x341d, 0x01},
+	{0x3a01, 0x03},
+	{0x3a18, 0x7f},
+	{0x3a19, 0x00},
+	{0x3a1a, 0x37},
+	{0x3a1b, 0x00},
+	{0x3a1c, 0x37},
+	{0x3a1d, 0x00},
+	{0x3a1e, 0xf7},
+	{0x3a1f, 0x00},
+	{0x3a20, 0x3f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x6f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x3f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x5f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x2f},
+	{0x3a21, 0x00},
+	{0x3078, 0x02},
+	{0x3079, 0x00},
+	{0x307a, 0x00},
+	{0x307b, 0x00},
+	{0x3080, 0x02},
+	{0x3081, 0x00},
+	{0x3082, 0x00},
+	{0x3083, 0x00},
+	{0x3088, 0x02},
+	{0x3094, 0x00},
+	{0x3095, 0x00},
+	{0x3096, 0x00},
+	{0x309b, 0x02},
+	{0x309c, 0x00},
+	{0x309d, 0x00},
+	{0x309e, 0x00},
+	{0x30a4, 0x00},
+	{0x30a5, 0x00},
+	{0x3288, 0x21},
+	{0x328a, 0x02},
+	{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},
+	{0x37b0, 0x37},
+	{0x3e04, 0x0e},
+	{0x30e8, 0x50},
+	{0x30e9, 0x00},
+	{0x3e04, 0x0e},
+	{0x3002, 0x00},
+};
+
+/* Sensor mode registers for 1280x720@30fps */
+static const struct imx334_reg mode_1280x720_regs[] = {
+	{0x3000, 0x01},
+	{0x3018, 0x04},
+	{0x3030, 0xca},
+	{0x3031, 0x08},
+	{0x3032, 0x00},
+	{0x3034, 0x4c},
+	{0x3035, 0x04},
+	{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},
+	{0x30C6, 0x00},
+	{0x30c7, 0x00},
+	{0x30ce, 0x00},
+	{0x30cf, 0x00},
+	{0x304c, 0x00},
+	{0x304e, 0x00},
+	{0x304f, 0x00},
+	{0x3050, 0x00},
+	{0x30b6, 0x00},
+	{0x30b7, 0x00},
+	{0x3116, 0x08},
+	{0x3117, 0x00},
+	{0x31a0, 0x20},
+	{0x31a1, 0x0f},
+	{0x300c, 0x3b},
+	{0x300d, 0x29},
+	{0x314c, 0x29},
+	{0x314d, 0x01},
+	{0x315a, 0x0a},
+	{0x3168, 0xa0},
+	{0x316a, 0x7e},
+	{0x319e, 0x02},
+	{0x3199, 0x00},
+	{0x319d, 0x00},
+	{0x31dd, 0x03},
+	{0x3300, 0x00},
+	{0x341c, 0xff},
+	{0x341d, 0x01},
+	{0x3a01, 0x03},
+	{0x3a18, 0x7f},
+	{0x3a19, 0x00},
+	{0x3a1a, 0x37},
+	{0x3a1b, 0x00},
+	{0x3a1c, 0x37},
+	{0x3a1d, 0x00},
+	{0x3a1e, 0xf7},
+	{0x3a1f, 0x00},
+	{0x3a20, 0x3f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x6f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x3f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x5f},
+	{0x3a21, 0x00},
+	{0x3a20, 0x2f},
+	{0x3a21, 0x00},
+	{0x3078, 0x02},
+	{0x3079, 0x00},
+	{0x307a, 0x00},
+	{0x307b, 0x00},
+	{0x3080, 0x02},
+	{0x3081, 0x00},
+	{0x3082, 0x00},
+	{0x3083, 0x00},
+	{0x3088, 0x02},
+	{0x3094, 0x00},
+	{0x3095, 0x00},
+	{0x3096, 0x00},
+	{0x309b, 0x02},
+	{0x309c, 0x00},
+	{0x309d, 0x00},
+	{0x309e, 0x00},
+	{0x30a4, 0x00},
+	{0x30a5, 0x00},
+	{0x3288, 0x21},
+	{0x328a, 0x02},
+	{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},
+	{0x37b0, 0x37},
+	{0x3e04, 0x0e},
+	{0x30e8, 0x50},
+	{0x30e9, 0x00},
+	{0x3e04, 0x0e},
+	{0x3002, 0x00},
+};
+
 /* Sensor mode registers for 1920x1080@30fps */
 static const struct imx334_reg mode_1920x1080_regs[] = {
 	{0x3000, 0x01},
@@ -505,6 +831,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] 5+ messages in thread

* Re: [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
  2024-11-29 10:00 [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
@ 2024-11-29 11:58 ` Kieran Bingham
  2024-12-02  5:20   ` Shravan.Chippa
  0 siblings, 1 reply; 5+ messages in thread
From: Kieran Bingham @ 2024-11-29 11:58 UTC (permalink / raw)
  To: mchehab, sakari.ailus, shravan kumar
  Cc: linux-media, linux-kernel, conor.dooley,
	valentina.fernandezalanis, praveen.kumar, shravan.chippa

Hi Shravan,

Quoting shravan kumar (2024-11-29 10:00:36)
> From: Shravan Chippa <shravan.chippa@microchip.com>
> 
> Add support for 1280x720@30 and 640x480@30 resolutions
> 

Thanks for improving the driver, but I am weary there is a lot of work
required to clean up the imx334 driver.

Do you have the datasheet for this device?

> Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> ---
>  drivers/media/i2c/imx334.c | 352 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 352 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index a544fc3df39c..b7cdebc2382e 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -167,6 +167,332 @@ static const s64 link_freq[] = {
>         IMX334_LINK_FREQ_445M,
>  };
>  
> +/* Sensor mode registers for 640x480@30fps */
> +static const struct imx334_reg mode_640x480_regs[] = {
> +       {0x3000, 0x01},
> +       {0x3018, 0x04},
> +       {0x3030, 0xca},
> +       {0x3031, 0x08},
> +       {0x3032, 0x00},
> +       {0x3034, 0x4c},
> +       {0x3035, 0x04},
> +       {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},
> +       {0x30C6, 0x00},
> +       {0x30c7, 0x00},
> +       {0x30ce, 0x00},
> +       {0x30cf, 0x00},
> +       {0x304c, 0x00},
> +       {0x304e, 0x00},
> +       {0x304f, 0x00},
> +       {0x3050, 0x00},
> +       {0x30b6, 0x00},
> +       {0x30b7, 0x00},
> +       {0x3116, 0x08},
> +       {0x3117, 0x00},
> +       {0x31a0, 0x20},
> +       {0x31a1, 0x0f},
> +       {0x300c, 0x3b},
> +       {0x300d, 0x29},
> +       {0x314c, 0x29},
> +       {0x314d, 0x01},
> +       {0x315a, 0x0a},
> +       {0x3168, 0xa0},
> +       {0x316a, 0x7e},
> +       {0x319e, 0x02},
> +       {0x3199, 0x00},
> +       {0x319d, 0x00},
> +       {0x31dd, 0x03},
> +       {0x3300, 0x00},
> +       {0x341c, 0xff},
> +       {0x341d, 0x01},
> +       {0x3a01, 0x03},
> +       {0x3a18, 0x7f},
> +       {0x3a19, 0x00},
> +       {0x3a1a, 0x37},
> +       {0x3a1b, 0x00},
> +       {0x3a1c, 0x37},
> +       {0x3a1d, 0x00},
> +       {0x3a1e, 0xf7},
> +       {0x3a1f, 0x00},
> +       {0x3a20, 0x3f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x6f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x3f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x5f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x2f},
> +       {0x3a21, 0x00},
> +       {0x3078, 0x02},
> +       {0x3079, 0x00},
> +       {0x307a, 0x00},
> +       {0x307b, 0x00},
> +       {0x3080, 0x02},
> +       {0x3081, 0x00},
> +       {0x3082, 0x00},
> +       {0x3083, 0x00},
> +       {0x3088, 0x02},
> +       {0x3094, 0x00},
> +       {0x3095, 0x00},
> +       {0x3096, 0x00},
> +       {0x309b, 0x02},
> +       {0x309c, 0x00},
> +       {0x309d, 0x00},
> +       {0x309e, 0x00},
> +       {0x30a4, 0x00},
> +       {0x30a5, 0x00},
> +       {0x3288, 0x21},
> +       {0x328a, 0x02},
> +       {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},
> +       {0x37b0, 0x37},
> +       {0x3e04, 0x0e},
> +       {0x30e8, 0x50},
> +       {0x30e9, 0x00},
> +       {0x3e04, 0x0e},
> +       {0x3002, 0x00},
> +};
> +
> +/* Sensor mode registers for 1280x720@30fps */
> +static const struct imx334_reg mode_1280x720_regs[] = {
> +       {0x3000, 0x01},
> +       {0x3018, 0x04},
> +       {0x3030, 0xca},
> +       {0x3031, 0x08},
> +       {0x3032, 0x00},
> +       {0x3034, 0x4c},
> +       {0x3035, 0x04},
> +       {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},
> +       {0x30C6, 0x00},
> +       {0x30c7, 0x00},
> +       {0x30ce, 0x00},
> +       {0x30cf, 0x00},
> +       {0x304c, 0x00},
> +       {0x304e, 0x00},
> +       {0x304f, 0x00},
> +       {0x3050, 0x00},
> +       {0x30b6, 0x00},
> +       {0x30b7, 0x00},
> +       {0x3116, 0x08},
> +       {0x3117, 0x00},
> +       {0x31a0, 0x20},
> +       {0x31a1, 0x0f},
> +       {0x300c, 0x3b},
> +       {0x300d, 0x29},
> +       {0x314c, 0x29},
> +       {0x314d, 0x01},
> +       {0x315a, 0x0a},
> +       {0x3168, 0xa0},
> +       {0x316a, 0x7e},
> +       {0x319e, 0x02},
> +       {0x3199, 0x00},
> +       {0x319d, 0x00},
> +       {0x31dd, 0x03},
> +       {0x3300, 0x00},
> +       {0x341c, 0xff},
> +       {0x341d, 0x01},
> +       {0x3a01, 0x03},
> +       {0x3a18, 0x7f},
> +       {0x3a19, 0x00},
> +       {0x3a1a, 0x37},
> +       {0x3a1b, 0x00},
> +       {0x3a1c, 0x37},
> +       {0x3a1d, 0x00},
> +       {0x3a1e, 0xf7},
> +       {0x3a1f, 0x00},
> +       {0x3a20, 0x3f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x6f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x3f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x5f},
> +       {0x3a21, 0x00},
> +       {0x3a20, 0x2f},
> +       {0x3a21, 0x00},
> +       {0x3078, 0x02},
> +       {0x3079, 0x00},
> +       {0x307a, 0x00},
> +       {0x307b, 0x00},
> +       {0x3080, 0x02},
> +       {0x3081, 0x00},
> +       {0x3082, 0x00},
> +       {0x3083, 0x00},
> +       {0x3088, 0x02},
> +       {0x3094, 0x00},
> +       {0x3095, 0x00},
> +       {0x3096, 0x00},
> +       {0x309b, 0x02},
> +       {0x309c, 0x00},
> +       {0x309d, 0x00},
> +       {0x309e, 0x00},
> +       {0x30a4, 0x00},
> +       {0x30a5, 0x00},
> +       {0x3288, 0x21},
> +       {0x328a, 0x02},
> +       {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},
> +       {0x37b0, 0x37},
> +       {0x3e04, 0x0e},
> +       {0x30e8, 0x50},
> +       {0x30e9, 0x00},
> +       {0x3e04, 0x0e},
> +       {0x3002, 0x00},
> +};

This is an enormous amount of duplicated data that could be factored
out.

These are also /very/ common against the existing mode register tables
too.

I think several things need to happen in this driver:

 1. It should be converted to use the CCI helpers.
 2. Whereever identifiable, the register names should be used instead of
    just the addresses.
 3. The common factors of these tables should be de-duplicated.

In your additions you only have differences in the following registers
from those entire tables:

+	{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},

 4. And ideally - the differences which determine the modes should be
    factored out to calculations so that we are not writing out large
    tables just to write a parameterised frame size.


I would beleive that at least steps 1 and 3 would be achievable, 2 and 4
would depend upon access to the datasheet.

Is that anything you could work on ?

Regards
--
Kieran





> +
>  /* Sensor mode registers for 1920x1080@30fps */
>  static const struct imx334_reg mode_1920x1080_regs[] = {
>         {0x3000, 0x01},
> @@ -505,6 +831,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] 5+ messages in thread

* RE: [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
  2024-11-29 11:58 ` Kieran Bingham
@ 2024-12-02  5:20   ` Shravan.Chippa
  2025-01-06 21:18     ` André Apitzsch
  0 siblings, 1 reply; 5+ messages in thread
From: Shravan.Chippa @ 2024-12-02  5:20 UTC (permalink / raw)
  To: kieran.bingham, mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, Conor.Dooley,
	Valentina.FernandezAlanis, Praveen.Kumar

Hi Kieran,

> -----Original Message-----
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Sent: Friday, November 29, 2024 5:28 PM
> To: mchehab@kernel.org; sakari.ailus@linux.intel.com; shravan Chippa -
> I35088 <Shravan.Chippa@microchip.com>
> Cc: 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: Re: [PATCH] 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,
> 
> Quoting shravan kumar (2024-11-29 10:00:36)
> > From: Shravan Chippa <shravan.chippa@microchip.com>
> >
> > Add support for 1280x720@30 and 640x480@30 resolutions
> >
> 
> Thanks for improving the driver, but I am weary there is a lot of work required
> to clean up the imx334 driver.
> 
> Do you have the datasheet for this device?

I have limited access to datasheet.

> 
> > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> > ---
> >  drivers/media/i2c/imx334.c | 352
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 352 insertions(+)
> >
> > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> > index a544fc3df39c..b7cdebc2382e 100644
> > --- a/drivers/media/i2c/imx334.c
> > +++ b/drivers/media/i2c/imx334.c
> > @@ -167,6 +167,332 @@ static const s64 link_freq[] = {
> >         IMX334_LINK_FREQ_445M,
> >  };
> >
> > +/* Sensor mode registers for 640x480@30fps */ static const struct
> > +imx334_reg mode_640x480_regs[] = {
> > +       {0x3000, 0x01},
> > +       {0x3018, 0x04},
> > +       {0x3030, 0xca},
> > +       {0x3031, 0x08},
> > +       {0x3032, 0x00},
> > +       {0x3034, 0x4c},
> > +       {0x3035, 0x04},
> > +       {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},
> > +       {0x30C6, 0x00},
> > +       {0x30c7, 0x00},
> > +       {0x30ce, 0x00},
> > +       {0x30cf, 0x00},
> > +       {0x304c, 0x00},
> > +       {0x304e, 0x00},
> > +       {0x304f, 0x00},
> > +       {0x3050, 0x00},
> > +       {0x30b6, 0x00},
> > +       {0x30b7, 0x00},
> > +       {0x3116, 0x08},
> > +       {0x3117, 0x00},
> > +       {0x31a0, 0x20},
> > +       {0x31a1, 0x0f},
> > +       {0x300c, 0x3b},
> > +       {0x300d, 0x29},
> > +       {0x314c, 0x29},
> > +       {0x314d, 0x01},
> > +       {0x315a, 0x0a},
> > +       {0x3168, 0xa0},
> > +       {0x316a, 0x7e},
> > +       {0x319e, 0x02},
> > +       {0x3199, 0x00},
> > +       {0x319d, 0x00},
> > +       {0x31dd, 0x03},
> > +       {0x3300, 0x00},
> > +       {0x341c, 0xff},
> > +       {0x341d, 0x01},
> > +       {0x3a01, 0x03},
> > +       {0x3a18, 0x7f},
> > +       {0x3a19, 0x00},
> > +       {0x3a1a, 0x37},
> > +       {0x3a1b, 0x00},
> > +       {0x3a1c, 0x37},
> > +       {0x3a1d, 0x00},
> > +       {0x3a1e, 0xf7},
> > +       {0x3a1f, 0x00},
> > +       {0x3a20, 0x3f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x6f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x3f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x5f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x2f},
> > +       {0x3a21, 0x00},
> > +       {0x3078, 0x02},
> > +       {0x3079, 0x00},
> > +       {0x307a, 0x00},
> > +       {0x307b, 0x00},
> > +       {0x3080, 0x02},
> > +       {0x3081, 0x00},
> > +       {0x3082, 0x00},
> > +       {0x3083, 0x00},
> > +       {0x3088, 0x02},
> > +       {0x3094, 0x00},
> > +       {0x3095, 0x00},
> > +       {0x3096, 0x00},
> > +       {0x309b, 0x02},
> > +       {0x309c, 0x00},
> > +       {0x309d, 0x00},
> > +       {0x309e, 0x00},
> > +       {0x30a4, 0x00},
> > +       {0x30a5, 0x00},
> > +       {0x3288, 0x21},
> > +       {0x328a, 0x02},
> > +       {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},
> > +       {0x37b0, 0x37},
> > +       {0x3e04, 0x0e},
> > +       {0x30e8, 0x50},
> > +       {0x30e9, 0x00},
> > +       {0x3e04, 0x0e},
> > +       {0x3002, 0x00},
> > +};
> > +
> > +/* Sensor mode registers for 1280x720@30fps */ static const struct
> > +imx334_reg mode_1280x720_regs[] = {
> > +       {0x3000, 0x01},
> > +       {0x3018, 0x04},
> > +       {0x3030, 0xca},
> > +       {0x3031, 0x08},
> > +       {0x3032, 0x00},
> > +       {0x3034, 0x4c},
> > +       {0x3035, 0x04},
> > +       {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},
> > +       {0x30C6, 0x00},
> > +       {0x30c7, 0x00},
> > +       {0x30ce, 0x00},
> > +       {0x30cf, 0x00},
> > +       {0x304c, 0x00},
> > +       {0x304e, 0x00},
> > +       {0x304f, 0x00},
> > +       {0x3050, 0x00},
> > +       {0x30b6, 0x00},
> > +       {0x30b7, 0x00},
> > +       {0x3116, 0x08},
> > +       {0x3117, 0x00},
> > +       {0x31a0, 0x20},
> > +       {0x31a1, 0x0f},
> > +       {0x300c, 0x3b},
> > +       {0x300d, 0x29},
> > +       {0x314c, 0x29},
> > +       {0x314d, 0x01},
> > +       {0x315a, 0x0a},
> > +       {0x3168, 0xa0},
> > +       {0x316a, 0x7e},
> > +       {0x319e, 0x02},
> > +       {0x3199, 0x00},
> > +       {0x319d, 0x00},
> > +       {0x31dd, 0x03},
> > +       {0x3300, 0x00},
> > +       {0x341c, 0xff},
> > +       {0x341d, 0x01},
> > +       {0x3a01, 0x03},
> > +       {0x3a18, 0x7f},
> > +       {0x3a19, 0x00},
> > +       {0x3a1a, 0x37},
> > +       {0x3a1b, 0x00},
> > +       {0x3a1c, 0x37},
> > +       {0x3a1d, 0x00},
> > +       {0x3a1e, 0xf7},
> > +       {0x3a1f, 0x00},
> > +       {0x3a20, 0x3f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x6f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x3f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x5f},
> > +       {0x3a21, 0x00},
> > +       {0x3a20, 0x2f},
> > +       {0x3a21, 0x00},
> > +       {0x3078, 0x02},
> > +       {0x3079, 0x00},
> > +       {0x307a, 0x00},
> > +       {0x307b, 0x00},
> > +       {0x3080, 0x02},
> > +       {0x3081, 0x00},
> > +       {0x3082, 0x00},
> > +       {0x3083, 0x00},
> > +       {0x3088, 0x02},
> > +       {0x3094, 0x00},
> > +       {0x3095, 0x00},
> > +       {0x3096, 0x00},
> > +       {0x309b, 0x02},
> > +       {0x309c, 0x00},
> > +       {0x309d, 0x00},
> > +       {0x309e, 0x00},
> > +       {0x30a4, 0x00},
> > +       {0x30a5, 0x00},
> > +       {0x3288, 0x21},
> > +       {0x328a, 0x02},
> > +       {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},
> > +       {0x37b0, 0x37},
> > +       {0x3e04, 0x0e},
> > +       {0x30e8, 0x50},
> > +       {0x30e9, 0x00},
> > +       {0x3e04, 0x0e},
> > +       {0x3002, 0x00},
> > +};
> 
> This is an enormous amount of duplicated data that could be factored out.
> 
> These are also /very/ common against the existing mode register tables too.
> 
> I think several things need to happen in this driver:
> 
>  1. It should be converted to use the CCI helpers.
>  2. Whereever identifiable, the register names should be used instead of
>     just the addresses.
>  3. The common factors of these tables should be de-duplicated.
> 
> In your additions you only have differences in the following registers from
> those entire tables:

I will try to optimize camera resolution array register value usage by writing common register array values.

Thanks,
Shravan

> 
> +       {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},
> 
>  4. And ideally - the differences which determine the modes should be
>     factored out to calculations so that we are not writing out large
>     tables just to write a parameterised frame size.
> 
> 
> I would beleive that at least steps 1 and 3 would be achievable, 2 and 4 would
> depend upon access to the datasheet.
> 
> Is that anything you could work on ?
> 
> Regards
> --
> Kieran
> 
> 
> 
> 
> 
> > +
> >  /* Sensor mode registers for 1920x1080@30fps */  static const struct
> > imx334_reg mode_1920x1080_regs[] = {
> >         {0x3000, 0x01},
> > @@ -505,6 +831,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] 5+ messages in thread

* Re: [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
  2024-12-02  5:20   ` Shravan.Chippa
@ 2025-01-06 21:18     ` André Apitzsch
  2025-01-22  5:00       ` Shravan.Chippa
  0 siblings, 1 reply; 5+ messages in thread
From: André Apitzsch @ 2025-01-06 21:18 UTC (permalink / raw)
  To: Shravan.Chippa, kieran.bingham, mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, Conor.Dooley,
	Valentina.FernandezAlanis, Praveen.Kumar

Am Montag, dem 02.12.2024 um 05:20 +0000 schrieb
Shravan.Chippa@microchip.com:
> Hi Kieran,
> 
> > -----Original Message-----
> > From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Sent: Friday, November 29, 2024 5:28 PM
> > To: mchehab@kernel.org; sakari.ailus@linux.intel.com; shravan
> > Chippa -
> > I35088 <Shravan.Chippa@microchip.com>
> > Cc: 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: Re: [PATCH] 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,
> > 
> > Quoting shravan kumar (2024-11-29 10:00:36)
> > > From: Shravan Chippa <shravan.chippa@microchip.com>
> > > 
> > > Add support for 1280x720@30 and 640x480@30 resolutions
> > > 
> > 
> > Thanks for improving the driver, but I am weary there is a lot of
> > work required to clean up the imx334 driver.
> > 
> > Do you have the datasheet for this device?
> 
> I have limited access to datasheet.

Hi Shravan,

you can find the datasheet for IMX334 here:

https://en.sunnywale.com/uploadfile/2022/1205/IMX334LQR-C%20full%20datasheet_Awin.pdf

from https://en.sunnywale.com/show-75-24264-1.html.

For an inspiration for driver clean-ups see

* https://lore.kernel.org/linux-media/20241220-imx214-v6-0-d90a185d82e5@apitzsch.eu/ (imx214)
* https://lore.kernel.org/linux-media/20230913135638.26277-1-laurent.pinchart@ideasonboard.com/ (imx219)

Best regards,
André

> 
> > 
> > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> > > ---
> > >  drivers/media/i2c/imx334.c | 352
> > > +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 352 insertions(+)
> > > 
> > > diff --git a/drivers/media/i2c/imx334.c
> > > b/drivers/media/i2c/imx334.c
> > > index a544fc3df39c..b7cdebc2382e 100644
> > > --- a/drivers/media/i2c/imx334.c
> > > +++ b/drivers/media/i2c/imx334.c
> > > @@ -167,6 +167,332 @@ static const s64 link_freq[] = {
> > >         IMX334_LINK_FREQ_445M,
> > >  };
> > > 
> > > +/* Sensor mode registers for 640x480@30fps */ static const
> > > struct
> > > +imx334_reg mode_640x480_regs[] = {
> > > +       {0x3000, 0x01},
> > > +       {0x3018, 0x04},
> > > +       {0x3030, 0xca},
> > > +       {0x3031, 0x08},
> > > +       {0x3032, 0x00},
> > > +       {0x3034, 0x4c},
> > > +       {0x3035, 0x04},
> > > +       {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},
> > > +       {0x30C6, 0x00},
> > > +       {0x30c7, 0x00},
> > > +       {0x30ce, 0x00},
> > > +       {0x30cf, 0x00},
> > > +       {0x304c, 0x00},
> > > +       {0x304e, 0x00},
> > > +       {0x304f, 0x00},
> > > +       {0x3050, 0x00},
> > > +       {0x30b6, 0x00},
> > > +       {0x30b7, 0x00},
> > > +       {0x3116, 0x08},
> > > +       {0x3117, 0x00},
> > > +       {0x31a0, 0x20},
> > > +       {0x31a1, 0x0f},
> > > +       {0x300c, 0x3b},
> > > +       {0x300d, 0x29},
> > > +       {0x314c, 0x29},
> > > +       {0x314d, 0x01},
> > > +       {0x315a, 0x0a},
> > > +       {0x3168, 0xa0},
> > > +       {0x316a, 0x7e},
> > > +       {0x319e, 0x02},
> > > +       {0x3199, 0x00},
> > > +       {0x319d, 0x00},
> > > +       {0x31dd, 0x03},
> > > +       {0x3300, 0x00},
> > > +       {0x341c, 0xff},
> > > +       {0x341d, 0x01},
> > > +       {0x3a01, 0x03},
> > > +       {0x3a18, 0x7f},
> > > +       {0x3a19, 0x00},
> > > +       {0x3a1a, 0x37},
> > > +       {0x3a1b, 0x00},
> > > +       {0x3a1c, 0x37},
> > > +       {0x3a1d, 0x00},
> > > +       {0x3a1e, 0xf7},
> > > +       {0x3a1f, 0x00},
> > > +       {0x3a20, 0x3f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x6f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x3f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x5f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x2f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3078, 0x02},
> > > +       {0x3079, 0x00},
> > > +       {0x307a, 0x00},
> > > +       {0x307b, 0x00},
> > > +       {0x3080, 0x02},
> > > +       {0x3081, 0x00},
> > > +       {0x3082, 0x00},
> > > +       {0x3083, 0x00},
> > > +       {0x3088, 0x02},
> > > +       {0x3094, 0x00},
> > > +       {0x3095, 0x00},
> > > +       {0x3096, 0x00},
> > > +       {0x309b, 0x02},
> > > +       {0x309c, 0x00},
> > > +       {0x309d, 0x00},
> > > +       {0x309e, 0x00},
> > > +       {0x30a4, 0x00},
> > > +       {0x30a5, 0x00},
> > > +       {0x3288, 0x21},
> > > +       {0x328a, 0x02},
> > > +       {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},
> > > +       {0x37b0, 0x37},
> > > +       {0x3e04, 0x0e},
> > > +       {0x30e8, 0x50},
> > > +       {0x30e9, 0x00},
> > > +       {0x3e04, 0x0e},
> > > +       {0x3002, 0x00},
> > > +};
> > > +
> > > +/* Sensor mode registers for 1280x720@30fps */ static const
> > > struct
> > > +imx334_reg mode_1280x720_regs[] = {
> > > +       {0x3000, 0x01},
> > > +       {0x3018, 0x04},
> > > +       {0x3030, 0xca},
> > > +       {0x3031, 0x08},
> > > +       {0x3032, 0x00},
> > > +       {0x3034, 0x4c},
> > > +       {0x3035, 0x04},
> > > +       {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},
> > > +       {0x30C6, 0x00},
> > > +       {0x30c7, 0x00},
> > > +       {0x30ce, 0x00},
> > > +       {0x30cf, 0x00},
> > > +       {0x304c, 0x00},
> > > +       {0x304e, 0x00},
> > > +       {0x304f, 0x00},
> > > +       {0x3050, 0x00},
> > > +       {0x30b6, 0x00},
> > > +       {0x30b7, 0x00},
> > > +       {0x3116, 0x08},
> > > +       {0x3117, 0x00},
> > > +       {0x31a0, 0x20},
> > > +       {0x31a1, 0x0f},
> > > +       {0x300c, 0x3b},
> > > +       {0x300d, 0x29},
> > > +       {0x314c, 0x29},
> > > +       {0x314d, 0x01},
> > > +       {0x315a, 0x0a},
> > > +       {0x3168, 0xa0},
> > > +       {0x316a, 0x7e},
> > > +       {0x319e, 0x02},
> > > +       {0x3199, 0x00},
> > > +       {0x319d, 0x00},
> > > +       {0x31dd, 0x03},
> > > +       {0x3300, 0x00},
> > > +       {0x341c, 0xff},
> > > +       {0x341d, 0x01},
> > > +       {0x3a01, 0x03},
> > > +       {0x3a18, 0x7f},
> > > +       {0x3a19, 0x00},
> > > +       {0x3a1a, 0x37},
> > > +       {0x3a1b, 0x00},
> > > +       {0x3a1c, 0x37},
> > > +       {0x3a1d, 0x00},
> > > +       {0x3a1e, 0xf7},
> > > +       {0x3a1f, 0x00},
> > > +       {0x3a20, 0x3f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x6f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x3f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x5f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3a20, 0x2f},
> > > +       {0x3a21, 0x00},
> > > +       {0x3078, 0x02},
> > > +       {0x3079, 0x00},
> > > +       {0x307a, 0x00},
> > > +       {0x307b, 0x00},
> > > +       {0x3080, 0x02},
> > > +       {0x3081, 0x00},
> > > +       {0x3082, 0x00},
> > > +       {0x3083, 0x00},
> > > +       {0x3088, 0x02},
> > > +       {0x3094, 0x00},
> > > +       {0x3095, 0x00},
> > > +       {0x3096, 0x00},
> > > +       {0x309b, 0x02},
> > > +       {0x309c, 0x00},
> > > +       {0x309d, 0x00},
> > > +       {0x309e, 0x00},
> > > +       {0x30a4, 0x00},
> > > +       {0x30a5, 0x00},
> > > +       {0x3288, 0x21},
> > > +       {0x328a, 0x02},
> > > +       {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},
> > > +       {0x37b0, 0x37},
> > > +       {0x3e04, 0x0e},
> > > +       {0x30e8, 0x50},
> > > +       {0x30e9, 0x00},
> > > +       {0x3e04, 0x0e},
> > > +       {0x3002, 0x00},
> > > +};
> > 
> > This is an enormous amount of duplicated data that could be
> > factored out.
> > 
> > These are also /very/ common against the existing mode register
> > tables too.
> > 
> > I think several things need to happen in this driver:
> > 
> >  1. It should be converted to use the CCI helpers.
> >  2. Whereever identifiable, the register names should be used
> > instead of
> >     just the addresses.
> >  3. The common factors of these tables should be de-duplicated.
> > 
> > In your additions you only have differences in the following
> > registers from
> > those entire tables:
> 
> I will try to optimize camera resolution array register value usage
> by writing common register array values.
> 
> Thanks,
> Shravan
> 
> > 
> > +       {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},
> > 
> >  4. And ideally - the differences which determine the modes should
> > be
> >     factored out to calculations so that we are not writing out
> > large
> >     tables just to write a parameterised frame size.
> > 
> > 
> > I would beleive that at least steps 1 and 3 would be achievable, 2
> > and 4 would
> > depend upon access to the datasheet.
> > 
> > Is that anything you could work on ?
> > 
> > Regards
> > --
> > Kieran
> > 
> > 
> > 
> > 
> > 
> > > +
> > >  /* Sensor mode registers for 1920x1080@30fps */  static const
> > > struct
> > > imx334_reg mode_1920x1080_regs[] = {
> > >         {0x3000, 0x01},
> > > @@ -505,6 +831,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] 5+ messages in thread

* RE: [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions
  2025-01-06 21:18     ` André Apitzsch
@ 2025-01-22  5:00       ` Shravan.Chippa
  0 siblings, 0 replies; 5+ messages in thread
From: Shravan.Chippa @ 2025-01-22  5:00 UTC (permalink / raw)
  To: git, kieran.bingham, mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, Conor.Dooley,
	Valentina.FernandezAlanis, Praveen.Kumar

Hi Andre,

> -----Original Message-----
> From: André Apitzsch <git@apitzsch.eu>
> Sent: Tuesday, January 7, 2025 2:49 AM
> To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>;
> kieran.bingham@ideasonboard.com; mchehab@kernel.org;
> sakari.ailus@linux.intel.com
> Cc: 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] 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
> 
> Am Montag, dem 02.12.2024 um 05:20 +0000 schrieb
> Shravan.Chippa@microchip.com:
> > Hi Kieran,
> >
> > > -----Original Message-----
> > > From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > Sent: Friday, November 29, 2024 5:28 PM
> > > To: mchehab@kernel.org; sakari.ailus@linux.intel.com; shravan Chippa
> > > -
> > > I35088 <Shravan.Chippa@microchip.com>
> > > Cc: 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: Re: [PATCH] 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,
> > >
> > > Quoting shravan kumar (2024-11-29 10:00:36)
> > > > From: Shravan Chippa <shravan.chippa@microchip.com>
> > > >
> > > > Add support for 1280x720@30 and 640x480@30 resolutions
> > > >
> > >
> > > Thanks for improving the driver, but I am weary there is a lot of
> > > work required to clean up the imx334 driver.
> > >
> > > Do you have the datasheet for this device?
> >
> > I have limited access to datasheet.
> 
> Hi Shravan,
> 
> you can find the datasheet for IMX334 here:
> 
> https://en.sunnywale.com/uploadfile/2022/1205/IMX334LQR-
> C%20full%20datasheet_Awin.pdf
> 
> from https://en.sunnywale.com/show-75-24264-1.html.
> 
> For an inspiration for driver clean-ups see
> 
> * https://lore.kernel.org/linux-media/20241220-imx214-v6-0-
> d90a185d82e5@apitzsch.eu/ (imx214)
> * https://lore.kernel.org/linux-media/20230913135638.26277-1-
> laurent.pinchart@ideasonboard.com/ (imx219)
> 

Thanks, Andre, for the cleanup patch reference

I have sent V3 for this series for these patches.

I will try to understand clean up patches reference imx219 and imx214.
It needs time and efforts to prepare and test patches.
May be in future I will try to send new series with clean-up patches.
I have submitted the patches witch I have tested on PolarFire development board.

Thanks,
Shravan 

> Best regards,
> André
> 
> >
> > >
> > > > Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
> > > > ---
> > > >  drivers/media/i2c/imx334.c | 352
> > > > +++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 352 insertions(+)
> > > >
> > > > diff --git a/drivers/media/i2c/imx334.c
> > > > b/drivers/media/i2c/imx334.c index a544fc3df39c..b7cdebc2382e
> > > > 100644
> > > > --- a/drivers/media/i2c/imx334.c
> > > > +++ b/drivers/media/i2c/imx334.c
> > > > @@ -167,6 +167,332 @@ static const s64 link_freq[] = {
> > > >         IMX334_LINK_FREQ_445M,
> > > >  };
> > > >
> > > > +/* Sensor mode registers for 640x480@30fps */ static const
> > > > struct
> > > > +imx334_reg mode_640x480_regs[] = {
> > > > +       {0x3000, 0x01},
> > > > +       {0x3018, 0x04},
> > > > +       {0x3030, 0xca},
> > > > +       {0x3031, 0x08},
> > > > +       {0x3032, 0x00},
> > > > +       {0x3034, 0x4c},
> > > > +       {0x3035, 0x04},
> > > > +       {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},
> > > > +       {0x30C6, 0x00},
> > > > +       {0x30c7, 0x00},
> > > > +       {0x30ce, 0x00},
> > > > +       {0x30cf, 0x00},
> > > > +       {0x304c, 0x00},
> > > > +       {0x304e, 0x00},
> > > > +       {0x304f, 0x00},
> > > > +       {0x3050, 0x00},
> > > > +       {0x30b6, 0x00},
> > > > +       {0x30b7, 0x00},
> > > > +       {0x3116, 0x08},
> > > > +       {0x3117, 0x00},
> > > > +       {0x31a0, 0x20},
> > > > +       {0x31a1, 0x0f},
> > > > +       {0x300c, 0x3b},
> > > > +       {0x300d, 0x29},
> > > > +       {0x314c, 0x29},
> > > > +       {0x314d, 0x01},
> > > > +       {0x315a, 0x0a},
> > > > +       {0x3168, 0xa0},
> > > > +       {0x316a, 0x7e},
> > > > +       {0x319e, 0x02},
> > > > +       {0x3199, 0x00},
> > > > +       {0x319d, 0x00},
> > > > +       {0x31dd, 0x03},
> > > > +       {0x3300, 0x00},
> > > > +       {0x341c, 0xff},
> > > > +       {0x341d, 0x01},
> > > > +       {0x3a01, 0x03},
> > > > +       {0x3a18, 0x7f},
> > > > +       {0x3a19, 0x00},
> > > > +       {0x3a1a, 0x37},
> > > > +       {0x3a1b, 0x00},
> > > > +       {0x3a1c, 0x37},
> > > > +       {0x3a1d, 0x00},
> > > > +       {0x3a1e, 0xf7},
> > > > +       {0x3a1f, 0x00},
> > > > +       {0x3a20, 0x3f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x6f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x3f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x5f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x2f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3078, 0x02},
> > > > +       {0x3079, 0x00},
> > > > +       {0x307a, 0x00},
> > > > +       {0x307b, 0x00},
> > > > +       {0x3080, 0x02},
> > > > +       {0x3081, 0x00},
> > > > +       {0x3082, 0x00},
> > > > +       {0x3083, 0x00},
> > > > +       {0x3088, 0x02},
> > > > +       {0x3094, 0x00},
> > > > +       {0x3095, 0x00},
> > > > +       {0x3096, 0x00},
> > > > +       {0x309b, 0x02},
> > > > +       {0x309c, 0x00},
> > > > +       {0x309d, 0x00},
> > > > +       {0x309e, 0x00},
> > > > +       {0x30a4, 0x00},
> > > > +       {0x30a5, 0x00},
> > > > +       {0x3288, 0x21},
> > > > +       {0x328a, 0x02},
> > > > +       {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},
> > > > +       {0x37b0, 0x37},
> > > > +       {0x3e04, 0x0e},
> > > > +       {0x30e8, 0x50},
> > > > +       {0x30e9, 0x00},
> > > > +       {0x3e04, 0x0e},
> > > > +       {0x3002, 0x00},
> > > > +};
> > > > +
> > > > +/* Sensor mode registers for 1280x720@30fps */ static const
> > > > struct
> > > > +imx334_reg mode_1280x720_regs[] = {
> > > > +       {0x3000, 0x01},
> > > > +       {0x3018, 0x04},
> > > > +       {0x3030, 0xca},
> > > > +       {0x3031, 0x08},
> > > > +       {0x3032, 0x00},
> > > > +       {0x3034, 0x4c},
> > > > +       {0x3035, 0x04},
> > > > +       {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},
> > > > +       {0x30C6, 0x00},
> > > > +       {0x30c7, 0x00},
> > > > +       {0x30ce, 0x00},
> > > > +       {0x30cf, 0x00},
> > > > +       {0x304c, 0x00},
> > > > +       {0x304e, 0x00},
> > > > +       {0x304f, 0x00},
> > > > +       {0x3050, 0x00},
> > > > +       {0x30b6, 0x00},
> > > > +       {0x30b7, 0x00},
> > > > +       {0x3116, 0x08},
> > > > +       {0x3117, 0x00},
> > > > +       {0x31a0, 0x20},
> > > > +       {0x31a1, 0x0f},
> > > > +       {0x300c, 0x3b},
> > > > +       {0x300d, 0x29},
> > > > +       {0x314c, 0x29},
> > > > +       {0x314d, 0x01},
> > > > +       {0x315a, 0x0a},
> > > > +       {0x3168, 0xa0},
> > > > +       {0x316a, 0x7e},
> > > > +       {0x319e, 0x02},
> > > > +       {0x3199, 0x00},
> > > > +       {0x319d, 0x00},
> > > > +       {0x31dd, 0x03},
> > > > +       {0x3300, 0x00},
> > > > +       {0x341c, 0xff},
> > > > +       {0x341d, 0x01},
> > > > +       {0x3a01, 0x03},
> > > > +       {0x3a18, 0x7f},
> > > > +       {0x3a19, 0x00},
> > > > +       {0x3a1a, 0x37},
> > > > +       {0x3a1b, 0x00},
> > > > +       {0x3a1c, 0x37},
> > > > +       {0x3a1d, 0x00},
> > > > +       {0x3a1e, 0xf7},
> > > > +       {0x3a1f, 0x00},
> > > > +       {0x3a20, 0x3f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x6f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x3f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x5f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3a20, 0x2f},
> > > > +       {0x3a21, 0x00},
> > > > +       {0x3078, 0x02},
> > > > +       {0x3079, 0x00},
> > > > +       {0x307a, 0x00},
> > > > +       {0x307b, 0x00},
> > > > +       {0x3080, 0x02},
> > > > +       {0x3081, 0x00},
> > > > +       {0x3082, 0x00},
> > > > +       {0x3083, 0x00},
> > > > +       {0x3088, 0x02},
> > > > +       {0x3094, 0x00},
> > > > +       {0x3095, 0x00},
> > > > +       {0x3096, 0x00},
> > > > +       {0x309b, 0x02},
> > > > +       {0x309c, 0x00},
> > > > +       {0x309d, 0x00},
> > > > +       {0x309e, 0x00},
> > > > +       {0x30a4, 0x00},
> > > > +       {0x30a5, 0x00},
> > > > +       {0x3288, 0x21},
> > > > +       {0x328a, 0x02},
> > > > +       {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},
> > > > +       {0x37b0, 0x37},
> > > > +       {0x3e04, 0x0e},
> > > > +       {0x30e8, 0x50},
> > > > +       {0x30e9, 0x00},
> > > > +       {0x3e04, 0x0e},
> > > > +       {0x3002, 0x00},
> > > > +};
> > >
> > > This is an enormous amount of duplicated data that could be factored
> > > out.
> > >
> > > These are also /very/ common against the existing mode register
> > > tables too.
> > >
> > > I think several things need to happen in this driver:
> > >
> > >  1. It should be converted to use the CCI helpers.
> > >  2. Whereever identifiable, the register names should be used
> > > instead of
> > >     just the addresses.
> > >  3. The common factors of these tables should be de-duplicated.
> > >
> > > In your additions you only have differences in the following
> > > registers from those entire tables:
> >
> > I will try to optimize camera resolution array register value usage by
> > writing common register array values.
> >
> > Thanks,
> > Shravan
> >
> > >
> > > +       {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},
> > >
> > >  4. And ideally - the differences which determine the modes should
> > > be
> > >     factored out to calculations so that we are not writing out
> > > large
> > >     tables just to write a parameterised frame size.
> > >
> > >
> > > I would beleive that at least steps 1 and 3 would be achievable, 2
> > > and 4 would depend upon access to the datasheet.
> > >
> > > Is that anything you could work on ?
> > >
> > > Regards
> > > --
> > > Kieran
> > >
> > >
> > >
> > >
> > >
> > > > +
> > > >  /* Sensor mode registers for 1920x1080@30fps */  static const
> > > > struct imx334_reg mode_1920x1080_regs[] = {
> > > >         {0x3000, 0x01},
> > > > @@ -505,6 +831,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] 5+ messages in thread

end of thread, other threads:[~2025-01-22  5:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-29 10:00 [PATCH] media: i2c: imx334: Add support for 1280x720 & 640x480 resolutions shravan kumar
2024-11-29 11:58 ` Kieran Bingham
2024-12-02  5:20   ` Shravan.Chippa
2025-01-06 21:18     ` André Apitzsch
2025-01-22  5:00       ` 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®