Re: [PATCH v6 04/11] media: rcar-csi2: Move {enable|disable}_streams() calls

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2026-06-17 14:54:57 +0300, Tomi Valkeinen wrote:
> With multiple streams the operation to enable the CSI-2 hardware and to
> call {enable|disable}_streams() on upstream subdev will need to be
> handled separately.
> 
> Prepare for that by moving {enable|disable}_streams() calls out from
> rcsi2_start() and rcsi2_stop().
> 
> As the error recovery in rcsi2_irq_thread() relies on rcsi2_start() and
> rcsi2_stop(), we need to modify rcsi2_irq_thread() accordingly.
> 
> On Gen3, a side effect of this change is that if the sink side devices
> call .enable_streams() on rcar-csi2 multiple times, the second call will
> fail. This is because we always use stream ID 0, so the second call
> would attempt to enable the same stream again, leading to an error. In
> other words, a normal single-stream setup continues to work, but trying
> to use the current driver's custom VC based routing will fail.
> 
> On Gen4, this doesn't matter as the rcar-isp behaves in a similar way as
> described above, and thus rcar-csi2 will only get a single
> .enable_streams() call.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 59 ++++++++++++++++++++++++------
>  1 file changed, 47 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 7305cc4a04cb..f5d8b06dff51 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1822,20 +1822,12 @@ static int rcsi2_start(struct rcar_csi2 *priv, struct v4l2_subdev_state *state)
>  		return ret;
>  	}
>  
> -	ret = v4l2_subdev_enable_streams(priv->remote, priv->remote_pad,
> -					 BIT_ULL(0));
> -	if (ret) {
> -		rcsi2_enter_standby(priv);
> -		return ret;
> -	}
> -
>  	return 0;
>  }
>  
>  static void rcsi2_stop(struct rcar_csi2 *priv)
>  {
>  	rcsi2_enter_standby(priv);
> -	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad, BIT_ULL(0));
>  }
>  
>  static int rcsi2_enable_streams(struct v4l2_subdev *sd,
> @@ -1857,6 +1849,14 @@ static int rcsi2_enable_streams(struct v4l2_subdev *sd,
>  			return ret;
>  	}
>  
> +	ret = v4l2_subdev_enable_streams(priv->remote, priv->remote_pad,
> +					 BIT_ULL(0));
> +	if (ret) {
> +		if (priv->stream_count == 0)
> +			rcsi2_stop(priv);
> +		return ret;
> +	}
> +
>  	priv->stream_count += 1;
>  
>  	return ret;
> @@ -1867,7 +1867,7 @@ static int rcsi2_disable_streams(struct v4l2_subdev *sd,
>  				 u32 source_pad, u64 source_streams_mask)
>  {
>  	struct rcar_csi2 *priv = sd_to_csi2(sd);
> -	int ret = 0;
> +	int ret;
>  
>  	if (source_streams_mask != 1)
>  		return -EINVAL;
> @@ -1878,9 +1878,14 @@ static int rcsi2_disable_streams(struct v4l2_subdev *sd,
>  	if (priv->stream_count == 1)
>  		rcsi2_stop(priv);
>  
> +	ret = v4l2_subdev_disable_streams(priv->remote, priv->remote_pad,
> +					  BIT_ULL(0));
> +	if (ret)
> +		return ret;
> +
>  	priv->stream_count -= 1;
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
> @@ -1971,14 +1976,44 @@ static irqreturn_t rcsi2_irq_thread(int irq, void *data)
>  {
>  	struct v4l2_subdev_state *state;
>  	struct rcar_csi2 *priv = data;
> +	int ret;
>  
>  	state = v4l2_subdev_lock_and_get_active_state(&priv->subdev);
>  
> +	if (priv->stream_count == 0)
> +		goto out;
> +
>  	rcsi2_stop(priv);
> +
> +	ret = v4l2_subdev_disable_streams(priv->remote, priv->remote_pad,
> +					  BIT_ULL(0));
> +	if (ret) {
> +		dev_warn(priv->dev,
> +			 "Error recovery: failed to disable streams: %d\n",
> +			 ret);
> +		goto out;
> +	}
> +
>  	usleep_range(1000, 2000);
> -	if (rcsi2_start(priv, state))
> -		dev_warn(priv->dev, "Failed to restart CSI-2 receiver\n");
>  
> +	ret = rcsi2_start(priv, state);
> +	if (ret) {
> +		dev_warn(priv->dev,
> +			 "Error recovery: failed to start CSI-2 receiver: %d\n",
> +			 ret);
> +		goto out;
> +	}
> +
> +	ret = v4l2_subdev_enable_streams(priv->remote, priv->remote_pad,
> +					 BIT_ULL(0));
> +	if (ret) {
> +		dev_warn(priv->dev,
> +			 "Error recovery: failed to start streams: %d\n",
> +			 ret);
> +		goto out;
> +	}
> +
> +out:
>  	v4l2_subdev_unlock_state(state);
>  
>  	return IRQ_HANDLED;
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund