[PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes

Tarang Raval posted 6 patches 11 months ago
There is a newer version of this series
[PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes
Posted by Tarang Raval 11 months ago
imx334 can support both 4 and 8 lane configurations.
Extend the driver to configure the lane mode accordingly.

Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
 drivers/media/i2c/imx334.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index 24ccfd1d0986..23bfc64969cc 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -47,6 +47,8 @@
 #define IMX334_EXPOSURE_DEFAULT	0x0648
 
 #define IMX334_REG_LANEMODE            CCI_REG8(0x3a01)
+#define IMX334_CSI_4_LANE_MODE         3
+#define IMX334_CSI_8_LANE_MODE         7
 
 /* Window cropping Settings */
 #define IMX334_REG_AREA3_ST_ADR_1      CCI_REG16_LE(0x3074)
@@ -107,7 +109,6 @@
 /* CSI2 HW configuration */
 #define IMX334_LINK_FREQ_891M	891000000
 #define IMX334_LINK_FREQ_445M	445500000
-#define IMX334_NUM_DATA_LANES	4
 
 #define IMX334_REG_MIN		0x00
 #define IMX334_REG_MAX		0xfffff
@@ -181,6 +182,7 @@ struct imx334_mode {
  * @exp_ctrl: Pointer to exposure control
  * @again_ctrl: Pointer to analog gain control
  * @vblank: Vertical blanking in lines
+ * @lane_mode: Mode for number of connected data lanes
  * @cur_mode: Pointer to current selected sensor mode
  * @mutex: Mutex for serializing sensor controls
  * @link_freq_bitmap: Menu bitmap for link_freq_ctrl
@@ -204,6 +206,7 @@ struct imx334 {
 		struct v4l2_ctrl *again_ctrl;
 	};
 	u32 vblank;
+	u32 lane_mode;
 	const struct imx334_mode *cur_mode;
 	struct mutex mutex;
 	unsigned long link_freq_bitmap;
@@ -240,7 +243,6 @@ static const struct cci_reg_sequence common_mode_regs[] = {
 	{ IMX334_REG_HADD_VADD, 0x00},
 	{ IMX334_REG_VALID_EXPAND, 0x03},
 	{ IMX334_REG_TCYCLE, 0x00},
-	{ IMX334_REG_LANEMODE, 0x03},
 	{ IMX334_REG_TCLKPOST, 0x007f},
 	{ IMX334_REG_TCLKPREPARE, 0x0037},
 	{ IMX334_REG_TCLKTRAIL, 0x0037},
@@ -876,6 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334)
 		return ret;
 	}
 
+	ret = cci_write(imx334->cci, IMX334_REG_LANEMODE,
+			imx334->lane_mode, NULL);
+	if (ret) {
+		dev_err(imx334->dev, "failed to configure lanes\n");
+		return ret;
+	}
+
 	ret = imx334_set_framefmt(imx334);
 	if (ret) {
 		dev_err(imx334->dev, "%s failed to set frame format: %d\n",
@@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
 	if (ret)
 		return ret;
 
-	if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) {
+	switch (bus_cfg.bus.mipi_csi2.num_data_lanes) {
+	case 4:
+		imx334->lane_mode = IMX334_CSI_4_LANE_MODE;
+		break;
+	case 8:
+		imx334->lane_mode = IMX334_CSI_8_LANE_MODE;
+		break;
+	default:
 		dev_err(imx334->dev,
 			"number of CSI2 data lanes %d is not supported\n",
 			bus_cfg.bus.mipi_csi2.num_data_lanes);
-- 
2.34.1
Re: [PATCH 4/6] media: i2c: imx334: Support 4 or 8 lane operation modes
Posted by Sakari Ailus 10 months, 2 weeks ago
Hi Tarang,

On Mon, Mar 10, 2025 at 12:47:46PM +0530, Tarang Raval wrote:
> imx334 can support both 4 and 8 lane configurations.
> Extend the driver to configure the lane mode accordingly.
> 
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
>  drivers/media/i2c/imx334.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index 24ccfd1d0986..23bfc64969cc 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -47,6 +47,8 @@
>  #define IMX334_EXPOSURE_DEFAULT	0x0648
>  
>  #define IMX334_REG_LANEMODE            CCI_REG8(0x3a01)
> +#define IMX334_CSI_4_LANE_MODE         3
> +#define IMX334_CSI_8_LANE_MODE         7
>  
>  /* Window cropping Settings */
>  #define IMX334_REG_AREA3_ST_ADR_1      CCI_REG16_LE(0x3074)
> @@ -107,7 +109,6 @@
>  /* CSI2 HW configuration */
>  #define IMX334_LINK_FREQ_891M	891000000
>  #define IMX334_LINK_FREQ_445M	445500000
> -#define IMX334_NUM_DATA_LANES	4
>  
>  #define IMX334_REG_MIN		0x00
>  #define IMX334_REG_MAX		0xfffff
> @@ -181,6 +182,7 @@ struct imx334_mode {
>   * @exp_ctrl: Pointer to exposure control
>   * @again_ctrl: Pointer to analog gain control
>   * @vblank: Vertical blanking in lines
> + * @lane_mode: Mode for number of connected data lanes
>   * @cur_mode: Pointer to current selected sensor mode
>   * @mutex: Mutex for serializing sensor controls
>   * @link_freq_bitmap: Menu bitmap for link_freq_ctrl
> @@ -204,6 +206,7 @@ struct imx334 {
>  		struct v4l2_ctrl *again_ctrl;
>  	};
>  	u32 vblank;
> +	u32 lane_mode;
>  	const struct imx334_mode *cur_mode;
>  	struct mutex mutex;
>  	unsigned long link_freq_bitmap;
> @@ -240,7 +243,6 @@ static const struct cci_reg_sequence common_mode_regs[] = {
>  	{ IMX334_REG_HADD_VADD, 0x00},
>  	{ IMX334_REG_VALID_EXPAND, 0x03},
>  	{ IMX334_REG_TCYCLE, 0x00},
> -	{ IMX334_REG_LANEMODE, 0x03},

Not a fault of this patch but also the closing brace should have a space
before it. Could you address it in the earlier patches?

>  	{ IMX334_REG_TCLKPOST, 0x007f},
>  	{ IMX334_REG_TCLKPREPARE, 0x0037},
>  	{ IMX334_REG_TCLKTRAIL, 0x0037},
> @@ -876,6 +878,13 @@ static int imx334_start_streaming(struct imx334 *imx334)
>  		return ret;
>  	}
>  
> +	ret = cci_write(imx334->cci, IMX334_REG_LANEMODE,
> +			imx334->lane_mode, NULL);
> +	if (ret) {
> +		dev_err(imx334->dev, "failed to configure lanes\n");
> +		return ret;
> +	}
> +
>  	ret = imx334_set_framefmt(imx334);
>  	if (ret) {
>  		dev_err(imx334->dev, "%s failed to set frame format: %d\n",
> @@ -1022,7 +1031,14 @@ static int imx334_parse_hw_config(struct imx334 *imx334)
>  	if (ret)
>  		return ret;
>  
> -	if (bus_cfg.bus.mipi_csi2.num_data_lanes != IMX334_NUM_DATA_LANES) {
> +	switch (bus_cfg.bus.mipi_csi2.num_data_lanes) {
> +	case 4:
> +		imx334->lane_mode = IMX334_CSI_4_LANE_MODE;
> +		break;
> +	case 8:
> +		imx334->lane_mode = IMX334_CSI_8_LANE_MODE;

Doesn't this affect the PLL configuration? Presumably higher frame rates
could be achieved at least.

> +		break;
> +	default:
>  		dev_err(imx334->dev,
>  			"number of CSI2 data lanes %d is not supported\n",
>  			bus_cfg.bus.mipi_csi2.num_data_lanes);

-- 
Regards,

Sakari Ailus