Re: [PATCH v2 07/12] media: imx-mipi-csis: Add support to configure specific vc

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Inbaraj,

On Thu, Aug 14, 2025 at 07:39:38PM +0530, Inbaraj E wrote:
> MIPI_CSIS_V3_3 and MIPI_CSIS_V3_6_3 support streaming only on VC0.

That doesn't appear to be true, at least for MIPI_CSIS_V3_6_3. I have a
patch series that adds VC support for v3.6.3 in the i.MX8MP, and it has
been susccessfully tested.

> The
> MIPI_CSIS_V4_3 present in the FSD SoC supports streaming on any one VC
> out of four VCs. To extend support for the FSD SoC, add the ability to
> configure a specific VC. The FSD CSI Rx can configure any one VC and
> start streaming.
> 
> Signed-off-by: Inbaraj E <inbaraj.e@samsung.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index a3e2c8ae332f..4f6c417fdf58 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -54,7 +54,7 @@
>  
>  /* CSIS common control */
>  #define MIPI_CSIS_CMN_CTRL			0x04
> -#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW	BIT(16)
> +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(n)	BIT(((n) + 16))
>  #define MIPI_CSIS_CMN_CTRL_INTER_MODE		BIT(10)
>  #define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL	BIT(2)
>  #define MIPI_CSIS_CMN_CTRL_RESET		BIT(1)
> @@ -319,6 +319,7 @@ struct mipi_csis_device {
>  		u32 hs_settle;
>  		u32 clk_settle;
>  	} debug;
> +	unsigned int vc;
>  };
>  
>  /* -----------------------------------------------------------------------------
> @@ -544,9 +545,10 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
>  				   const struct csis_pix_format *csis_fmt)
>  {
>  	u32 val;
> +	unsigned int vc = csis->vc;
>  
>  	/* Color format */
> -	val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
> +	val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(vc));
>  	val &= ~(MIPI_CSIS_ISPCFG_ALIGN_32BIT | MIPI_CSIS_ISPCFG_FMT_MASK
>  		| MIPI_CSIS_ISPCFG_PIXEL_MASK);
>  
> @@ -567,11 +569,11 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
>  		val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
>  
>  	val |= MIPI_CSIS_ISPCFG_FMT(csis_fmt->data_type);
> -	mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
> +	mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(vc), val);
>  
>  	/* Pixel resolution */
>  	val = format->width | (format->height << 16);
> -	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0), val);
> +	mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(vc), val);
>  }
>  
>  static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> @@ -631,6 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  {
>  	int lanes = csis->bus.num_data_lanes;
>  	u32 val;
> +	unsigned int vc = csis->vc;
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
>  	val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK;
> @@ -648,7 +651,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  	val = (0 << MIPI_CSIS_ISP_SYNC_HSYNC_LINTV_OFFSET)
>  	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_SINTV_OFFSET)
>  	    | (0 << MIPI_CSIS_ISP_SYNC_VSYNC_EINTV_OFFSET);
> -	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0), val);
> +	mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(vc), val);
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
>  	val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC;
> @@ -669,7 +672,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  	/* Update the shadow register. */
>  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
>  	mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> -			val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW |
> +			val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(vc) |
>  			MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
>  }
>  
> @@ -945,6 +948,8 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
>  	struct v4l2_subdev_state *state;
>  	int ret;
>  
> +	csis->vc = 0;
> +

Dynamic VC selection belongs to this patch, not patch 09/12. 09/12 does
too many different things, it has to be split into one patch per
feature.

>  	if (!enable) {
>  		v4l2_subdev_disable_streams(csis->source.sd,
>  					    csis->source.pad->index, BIT(0));

-- 
Regards,

Laurent Pinchart