Re: [PATCH v5 03/10] media: rcar-csi2: Move {enable|disable}_streams() calls

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

On Tue, Jun 16, 2026 at 02:20:06PM +0300, Tomi Valkeinen wrote:
> On 18/03/2026 22:54, Laurent Pinchart wrote:
> > On Wed, Mar 11, 2026 at 03:53:16PM +0200, 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().
> >>
> >> 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.
> > 
> > I assume this gets addressed later in the series.
> 
> Yes and no.
> 
> The previous patch does the same for rcar-isp, which affects the gen4 
> custom VC based routing the same was this does for gen3.
> 
> At the end of the series we support full multi-stream with the upstream 
> API. The custom VC based routing is no longer supported, and will 
> continue to 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@ideasonboard.com>
> >> ---
> >>   drivers/media/platform/renesas/rcar-csi2.c | 25 +++++++++++++++----------
> >>   1 file changed, 15 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> >> index 7305cc4a04cb..158fa447e668 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;
> >>   }
> > 
> > rcsi2_irq_thread() also calls rcsi2_stop(), followed by rcsi2_start().
> > This is to handle errors reported by the AFIFO_OF, ERRSOTHS and
> > ERRSOTSYNCHS interrupts. If the source isn't restarted, such an attempt
> > to recover from errors will likely fail. On the other hand, restarting
> > the source will likely not lead to great results either.
> 
> Indeed. I think for single-stream use cases the behavior should still be 
> the same, but for multi-stream use, any enabled stream will keep the 
> csi2 enabled.
> 
> This kind of error handling sounds a bit fragile. If a restart helps, 
> don't we need to restart the whole pipeline, not just from csi2-rx 
> upwards? Or is it guaranteed that the ISP/CS and VIN will continue working?

My feeling is that these kind of errors would be best handled in
userspace.

> Did this work earlier with the custom VC based routing?

That I don't know.

> > Error handling was introduced in
> > 
> > commit 4ab44ff0841b9a825f9875623d24809d29e37a10
> > Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Date:   Thu Apr 11 16:30:58 2019 -0400
> > 
> >      media: rcar-csi2: restart CSI-2 link if error is detected
> > 
> >      Restart the CSI-2 link if the CSI-2 receiver detects an error during
> >      reception. The driver did nothing when a link error happened and the
> >      data flow simply stopped without the user knowing why.
> > 
> >      Change the driver to try and recover from errors by restarting the link
> >      and informing the user that something is not right. For obvious reasons
> >      it's not possible to recover from all errors (video source disconnected
> >      for example) but in such cases the user is at least informed of the
> >      error and the same behavior of the stopped data flow is retained.
> > 
> > Niklas, do you recall anything about the errors you saw ?

-- 
Regards,

Laurent Pinchart