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

[Tomi Valkeinen]

Hi,

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@xxxxxxxxxxxxxxxx>
> > ---
> >   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?

Did this work earlier with the custom VC based routing?

 Tomi

> Error handling was introduced in
>
> commit 4ab44ff0841b9a825f9875623d24809d29e37a10
> Author: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> 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 ?