[PATCH v3 00/15] media: rcar: Streams support

[PATCH v3 00/15] media: rcar: Streams support

[Tomi Valkeinen]

Add streams support to Renesas rcar platform driver.

The series attempts to keep compatibility with the current upstream.
However, in upstream there's some kind of custom multi-stream support
implemented to the rcar driver, which breaks at patch "media: rcar-csi2:
Simplify rcsi2_calc_mbps()".

The behavior should not change when using a single stream.

Testing is problematic, as the only way currently for me to get multiple
streams is by using the GMSL2 deserializer add-on board with GMSL2
serializers. These are not supported in upstream. If someone has the
hardware and wants to test, I can share the very-WIP branch that
contains the missing pieces.

 Tomi

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
Changes in v3:
- Rebased on top of latest linux-media
- Dropped dependencies which are already in linux-media (only remaining
  dependency is v4l2_subdev_get_frame_desc_passthrough)
- Tested on white-hawk board, using the staging deser TPG
- Also tested in a WIP branch for GMSL2 (two video streams)
- Link to v2: https://lore.kernel.org/r/20250326-rcar-streams-v2-0-d0d7002c641f@ideasonboard.com

Changes in v2:
- Rebased on top of latest upstream, and updated the dependencies to
  match the latest serieses sent.
- Add new patch "media: rcar-csi2: Use the pad version of v4l2_get_link_freq()"
- Drop "media: rcar-csi2: Fix typo" (it was not a typo)
- Update the code in calc_mbps(). The previous method relied on
  V4L2_CID_LINK_FREQ, but that's not available if the link-freq is
  provided via get_mbus_config().
- Dropped dependencies to Niklas' old series which doesn't apply
  cleanly. It's needed for multi-stream, but not for the current
  upstream which only has a single stream use case.
- Link to v1: https://lore.kernel.org/r/20250219-rcar-streams-v1-0-f1b93e370aab@ideasonboard.com

---
Tomi Valkeinen (15):
      media: rcar-csi2: Use the pad version of v4l2_get_link_freq()
      media: rcar-isp: Improve ISPPROCMODE_DT_PROC_MODE_VC
      media: rcar-isp: Move {enable|disable}_streams() calls
      media: rcar-csi2: Move {enable|disable}_streams() calls
      media: rcar-csi2: Move rcar2_calc_mbps()
      media: rcar-csi2: Simplify rcsi2_calc_mbps()
      media: rcar-csi2: Optimize rcsi2_calc_mbps()
      media: rcar-csi2: Switch to Streams API
      media: rcar-isp: Switch to Streams API
      media: rcar-csi2: Add .get_frame_desc op
      media: rcar-isp: Call get_frame_desc to find out VC & DT
      media: rcar-csi2: Add more stream support to rcsi2_calc_mbps()
      media: rcar-csi2: Call get_frame_desc to find out VC & DT (Gen3)
      media: rcar-csi2: Add full streams support
      media: rcar-isp: Add full streams support

 drivers/media/platform/renesas/rcar-csi2.c      | 426 +++++++++++++++++-------
 drivers/media/platform/renesas/rcar-isp/csisp.c | 228 ++++++++++---
 2 files changed, 479 insertions(+), 175 deletions(-)
---
base-commit: 5e1ff2314797bf53636468a97719a8222deca9ae
change-id: 20250219-rcar-streams-1fdea8860e5e
prerequisite-change-id: 20250218-frame-desc-passthrough-66805e413974:v4
prerequisite-patch-id: bce4a915a29a64f88ed1bb600c08df37d2ba20c6
prerequisite-patch-id: 69b75e7dad9ced905cb39a72f18bebbf3e8f998a
prerequisite-patch-id: 58463f6944c76acd6cf203b14a2836cdb0db2461

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

[PATCH v3 01/15] media: rcar-csi2: Use the pad version of v4l2_get_link_freq()

[Tomi Valkeinen]

Use the new version of v4l2_get_link_freq() which supports media_pad as
a parameter.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 9979de4f6ef1..ddbdde23c122 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -954,6 +954,7 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
 static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
 			   unsigned int lanes)
 {
+	struct media_pad *remote_pad;
 	struct v4l2_subdev *source;
 	s64 freq;
 	u64 mbps;
@@ -962,8 +963,9 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
 		return -ENODEV;
 
 	source = priv->remote;
+	remote_pad = &source->entity.pads[priv->remote_pad];
 
-	freq = v4l2_get_link_freq(source->ctrl_handler, bpp, 2 * lanes);
+	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
 	if (freq < 0) {
 		int ret = (int)freq;
 

-- 
2.43.0

Re: [PATCH v3 01/15] media: rcar-csi2: Use the pad version of v4l2_get_link_freq()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:30PM +0300, Tomi Valkeinen wrote:
> Use the new version of v4l2_get_link_freq() which supports media_pad as
> a parameter.

The commit message should explain why. With that fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 9979de4f6ef1..ddbdde23c122 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -954,6 +954,7 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
>  static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  			   unsigned int lanes)
>  {
> +	struct media_pad *remote_pad;
>  	struct v4l2_subdev *source;
>  	s64 freq;
>  	u64 mbps;
> @@ -962,8 +963,9 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  		return -ENODEV;
>  
>  	source = priv->remote;
> +	remote_pad = &source->entity.pads[priv->remote_pad];
>  
> -	freq = v4l2_get_link_freq(source->ctrl_handler, bpp, 2 * lanes);
> +	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
>  	if (freq < 0) {
>  		int ret = (int)freq;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 01/15] media: rcar-csi2: Use the pad version of v4l2_get_link_freq()

[Niklas Söderlund]

On 2025-06-02 12:43:21 +0300, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, May 30, 2025 at 04:50:30PM +0300, Tomi Valkeinen wrote:
> > Use the new version of v4l2_get_link_freq() which supports media_pad as
> > a parameter.
> 
> The commit message should explain why. With that fixed,

How about this,

The pad aware version of v4l2_get_link_freq() tries to retrieve the link 
frequency from the media bus configuration using the get_mbus_config 
operation, and only if the subdevice do not implement this operation 
fall-back to the old method of getting it using the V4L2_CID_LINK_FREQ 
or V4L2_CID_PIXEL_RATE control.

Update the VIN driver to use the pad aware version to be able to support 
subdevices that only provides the link frequency in the media bus 
configuration. As the implementation falls-back to the old method if the 
subdevice don't support get_mbus_config, or don't provide a link 
frequency in the v4l2_mbus_config struct, this is fully backward 
compatible.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  drivers/media/platform/renesas/rcar-csi2.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> > index 9979de4f6ef1..ddbdde23c122 100644
> > --- a/drivers/media/platform/renesas/rcar-csi2.c
> > +++ b/drivers/media/platform/renesas/rcar-csi2.c
> > @@ -954,6 +954,7 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
> >  static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> >  			   unsigned int lanes)
> >  {
> > +	struct media_pad *remote_pad;
> >  	struct v4l2_subdev *source;
> >  	s64 freq;
> >  	u64 mbps;
> > @@ -962,8 +963,9 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> >  		return -ENODEV;
> >  
> >  	source = priv->remote;
> > +	remote_pad = &source->entity.pads[priv->remote_pad];
> >  
> > -	freq = v4l2_get_link_freq(source->ctrl_handler, bpp, 2 * lanes);
> > +	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> >  	if (freq < 0) {
> >  		int ret = (int)freq;
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v3 01/15] media: rcar-csi2: Use the pad version of v4l2_get_link_freq()

[Laurent Pinchart]

On Wed, Jul 02, 2025 at 05:07:11PM +0200, Niklas Söderlund wrote:
> On 2025-06-02 12:43:21 +0300, Laurent Pinchart wrote:
> > On Fri, May 30, 2025 at 04:50:30PM +0300, Tomi Valkeinen wrote:
> > > Use the new version of v4l2_get_link_freq() which supports media_pad as
> > > a parameter.
> > 
> > The commit message should explain why. With that fixed,
> 
> How about this,
> 
> The pad aware version of v4l2_get_link_freq() tries to retrieve the link 

s/pad aware/pad-aware/

> frequency from the media bus configuration using the get_mbus_config 
> operation, and only if the subdevice do not implement this operation 

s/do not/does not/

> fall-back to the old method of getting it using the V4L2_CID_LINK_FREQ 

s/fall-back/falls back/

> or V4L2_CID_PIXEL_RATE control.
> 
> Update the VIN driver to use the pad aware version to be able to support 

s/pad aware/pad-aware/

> subdevices that only provides the link frequency in the media bus 
> configuration. As the implementation falls-back to the old method if the 

s/falls-back/falls back/

> subdevice don't support get_mbus_config, or don't provide a link 

s/don't/doesn't/g

> frequency in the v4l2_mbus_config struct, this is fully backward 
> compatible.

Looks good to me.

As discussed privately, this patch is needed to avoid breakages with the
latest GMSL2/3 drivers posted in [1] on Gen4 platforms. As the patch
makes sense on its own without the rest of the series, I'll take it in
my tree.

[1] lore.kernel.org/linux-media/20250702132104.1537926-1-demonsingur@gmail.com

> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > > ---
> > >  drivers/media/platform/renesas/rcar-csi2.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> > > index 9979de4f6ef1..ddbdde23c122 100644
> > > --- a/drivers/media/platform/renesas/rcar-csi2.c
> > > +++ b/drivers/media/platform/renesas/rcar-csi2.c
> > > @@ -954,6 +954,7 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
> > >  static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> > >  			   unsigned int lanes)
> > >  {
> > > +	struct media_pad *remote_pad;
> > >  	struct v4l2_subdev *source;
> > >  	s64 freq;
> > >  	u64 mbps;
> > > @@ -962,8 +963,9 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> > >  		return -ENODEV;
> > >  
> > >  	source = priv->remote;
> > > +	remote_pad = &source->entity.pads[priv->remote_pad];
> > >  
> > > -	freq = v4l2_get_link_freq(source->ctrl_handler, bpp, 2 * lanes);
> > > +	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> > >  	if (freq < 0) {
> > >  		int ret = (int)freq;
> > >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 01/15] media: rcar-csi2: Use the pad version of v4l2_get_link_freq()

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:30 +0300, Tomi Valkeinen wrote:
> Use the new version of v4l2_get_link_freq() which supports media_pad as
> a parameter.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 9979de4f6ef1..ddbdde23c122 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -954,6 +954,7 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
>  static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  			   unsigned int lanes)
>  {
> +	struct media_pad *remote_pad;
>  	struct v4l2_subdev *source;
>  	s64 freq;
>  	u64 mbps;
> @@ -962,8 +963,9 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  		return -ENODEV;
>  
>  	source = priv->remote;
> +	remote_pad = &source->entity.pads[priv->remote_pad];
>  
> -	freq = v4l2_get_link_freq(source->ctrl_handler, bpp, 2 * lanes);
> +	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
>  	if (freq < 0) {
>  		int ret = (int)freq;
>  
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 02/15] media: rcar-isp: Improve ISPPROCMODE_DT_PROC_MODE_VC

[Tomi Valkeinen]

Instead of having four macros for ISPPROCMODE_DT_PROC_MODE_VC[0123](pm),
have just one ISPPROCMODE_DT_PROC_MODE_VCn(vc, pm).

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 1eb29a0b774a..8fb2cc3b5650 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -28,10 +28,7 @@
 #define ISPSTART_STOP					0x0000
 
 #define ISPPROCMODE_DT_REG(n)				(0x1100 + (0x4 * (n)))
-#define ISPPROCMODE_DT_PROC_MODE_VC3(pm)		(((pm) & 0x3f) << 24)
-#define ISPPROCMODE_DT_PROC_MODE_VC2(pm)		(((pm) & 0x3f) << 16)
-#define ISPPROCMODE_DT_PROC_MODE_VC1(pm)		(((pm) & 0x3f) << 8)
-#define ISPPROCMODE_DT_PROC_MODE_VC0(pm)		((pm) & 0x3f)
+#define ISPPROCMODE_DT_PROC_MODE_VCn(vc, pm)		(((pm) & 0x3f) << (8 * (vc)))
 
 #define ISPCS_FILTER_ID_CH_REG(n)			(0x3000 + (0x0100 * (n)))
 
@@ -263,10 +260,10 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
 
 	/* Setup processing method. */
 	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
-		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
-		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
-		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
-		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
+		      ISPPROCMODE_DT_PROC_MODE_VCn(3, format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VCn(2, format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VCn(1, format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VCn(0, format->procmode));
 
 	/* Start ISP. */
 	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);

-- 
2.43.0

Re: [PATCH v3 02/15] media: rcar-isp: Improve ISPPROCMODE_DT_PROC_MODE_VC

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:31PM +0300, Tomi Valkeinen wrote:
> Instead of having four macros for ISPPROCMODE_DT_PROC_MODE_VC[0123](pm),
> have just one ISPPROCMODE_DT_PROC_MODE_VCn(vc, pm).
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 1eb29a0b774a..8fb2cc3b5650 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -28,10 +28,7 @@
>  #define ISPSTART_STOP					0x0000
>  
>  #define ISPPROCMODE_DT_REG(n)				(0x1100 + (0x4 * (n)))
> -#define ISPPROCMODE_DT_PROC_MODE_VC3(pm)		(((pm) & 0x3f) << 24)
> -#define ISPPROCMODE_DT_PROC_MODE_VC2(pm)		(((pm) & 0x3f) << 16)
> -#define ISPPROCMODE_DT_PROC_MODE_VC1(pm)		(((pm) & 0x3f) << 8)
> -#define ISPPROCMODE_DT_PROC_MODE_VC0(pm)		((pm) & 0x3f)
> +#define ISPPROCMODE_DT_PROC_MODE_VCn(vc, pm)		(((pm) & 0x3f) << (8 * (vc)))
>  
>  #define ISPCS_FILTER_ID_CH_REG(n)			(0x3000 + (0x0100 * (n)))
>  
> @@ -263,10 +260,10 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  
>  	/* Setup processing method. */
>  	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> -		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(3, format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(2, format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(1, format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(0, format->procmode));
>  
>  	/* Start ISP. */
>  	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 02/15] media: rcar-isp: Improve ISPPROCMODE_DT_PROC_MODE_VC

[Niklas Söderlund]

Hi Tomi,

Thanks for your patch.

On 2025-05-30 16:50:31 +0300, Tomi Valkeinen wrote:
> Instead of having four macros for ISPPROCMODE_DT_PROC_MODE_VC[0123](pm),
> have just one ISPPROCMODE_DT_PROC_MODE_VCn(vc, pm).
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 1eb29a0b774a..8fb2cc3b5650 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -28,10 +28,7 @@
>  #define ISPSTART_STOP					0x0000
>  
>  #define ISPPROCMODE_DT_REG(n)				(0x1100 + (0x4 * (n)))
> -#define ISPPROCMODE_DT_PROC_MODE_VC3(pm)		(((pm) & 0x3f) << 24)
> -#define ISPPROCMODE_DT_PROC_MODE_VC2(pm)		(((pm) & 0x3f) << 16)
> -#define ISPPROCMODE_DT_PROC_MODE_VC1(pm)		(((pm) & 0x3f) << 8)
> -#define ISPPROCMODE_DT_PROC_MODE_VC0(pm)		((pm) & 0x3f)
> +#define ISPPROCMODE_DT_PROC_MODE_VCn(vc, pm)		(((pm) & 0x3f) << (8 * (vc)))
>  
>  #define ISPCS_FILTER_ID_CH_REG(n)			(0x3000 + (0x0100 * (n)))
>  
> @@ -263,10 +260,10 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  
>  	/* Setup processing method. */
>  	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> -		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(3, format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(2, format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(1, format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VCn(0, format->procmode));
>  
>  	/* Start ISP. */
>  	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 03/15] media: rcar-isp: Move {enable|disable}_streams() calls

[Tomi Valkeinen]

With multiple streams the operation to enable the ISP 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
risp_start() and risp_stop().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 8fb2cc3b5650..2337c5d44c40 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -268,18 +268,11 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
 	/* Start ISP. */
 	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
 
-	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
-					 BIT_ULL(0));
-	if (ret)
-		risp_power_off(isp);
-
-	return ret;
+	return 0;
 }
 
 static void risp_stop(struct rcar_isp *isp)
 {
-	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
-
 	/* Stop ISP. */
 	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
 
@@ -305,6 +298,13 @@ static int risp_enable_streams(struct v4l2_subdev *sd,
 			return ret;
 	}
 
+	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
+					 BIT_ULL(0));
+	if (ret) {
+		risp_stop(isp);
+		return ret;
+	}
+
 	isp->stream_count += 1;
 
 	return ret;
@@ -322,6 +322,8 @@ static int risp_disable_streams(struct v4l2_subdev *sd,
 	if (!isp->remote)
 		return -ENODEV;
 
+	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
+
 	if (isp->stream_count == 1)
 		risp_stop(isp);
 

-- 
2.43.0

Re: [PATCH v3 03/15] media: rcar-isp: Move {enable|disable}_streams() calls

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:32PM +0300, Tomi Valkeinen wrote:
> With multiple streams the operation to enable the ISP 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
> risp_start() and risp_stop().
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 8fb2cc3b5650..2337c5d44c40 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -268,18 +268,11 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	/* Start ISP. */
>  	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
>  
> -	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
> -					 BIT_ULL(0));
> -	if (ret)
> -		risp_power_off(isp);
> -
> -	return ret;
> +	return 0;
>  }
>  
>  static void risp_stop(struct rcar_isp *isp)
>  {
> -	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
> -
>  	/* Stop ISP. */
>  	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
>  
> @@ -305,6 +298,13 @@ static int risp_enable_streams(struct v4l2_subdev *sd,
>  			return ret;
>  	}
>  
> +	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
> +					 BIT_ULL(0));

You're now potentially calling v4l2_subdev_disable_streams() multiple
times on the same pad and stream, as this call isn't covered by the
stream_count check anymore. Is that correct ? Maybe because
risp_enable_streams() is guaranteed to never be called multiple times,
with stream_count never becoming larger than 1 ? If so that should be
explained in the commit message, and stream_count should probably be
dropped.

Same when stopping.

> +	if (ret) {
> +		risp_stop(isp);

This is also not covered by the stream_count, while risp_start() is.

> +		return ret;
> +	}
> +
>  	isp->stream_count += 1;
>  
>  	return ret;
> @@ -322,6 +322,8 @@ static int risp_disable_streams(struct v4l2_subdev *sd,
>  	if (!isp->remote)
>  		return -ENODEV;
>  
> +	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
> +
>  	if (isp->stream_count == 1)
>  		risp_stop(isp);
>  

-- 
Regards,

Laurent Pinchart

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

[Tomi Valkeinen]

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().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index ddbdde23c122..698eb0e60f32 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1573,20 +1573,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,
@@ -1608,6 +1600,13 @@ 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) {
+		rcsi2_stop(priv);
+		return ret;
+	}
+
 	priv->stream_count += 1;
 
 	return ret;
@@ -1629,6 +1628,8 @@ static int rcsi2_disable_streams(struct v4l2_subdev *sd,
 	if (priv->stream_count == 1)
 		rcsi2_stop(priv);
 
+	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad, BIT_ULL(0));
+
 	priv->stream_count -= 1;
 
 	return ret;

-- 
2.43.0

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

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:33PM +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().

Same comments as for 03/15. Maybe this will become clearer in subsequent
patches, but the change should then probably be squashed in those
patches.

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index ddbdde23c122..698eb0e60f32 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1573,20 +1573,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,
> @@ -1608,6 +1600,13 @@ 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) {
> +		rcsi2_stop(priv);
> +		return ret;
> +	}
> +
>  	priv->stream_count += 1;
>  
>  	return ret;
> @@ -1629,6 +1628,8 @@ static int rcsi2_disable_streams(struct v4l2_subdev *sd,
>  	if (priv->stream_count == 1)
>  		rcsi2_stop(priv);
>  
> +	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad, BIT_ULL(0));
> +
>  	priv->stream_count -= 1;
>  
>  	return ret;

-- 
Regards,

Laurent Pinchart

[PATCH v3 05/15] media: rcar-csi2: Move rcar2_calc_mbps()

[Tomi Valkeinen]

Move the function so that it can call rcsi2_get_active_lanes() in the
following patch.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 66 +++++++++++++++---------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 698eb0e60f32..8aca35096408 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -951,39 +951,6 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
 	return 0;
 }
 
-static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
-			   unsigned int lanes)
-{
-	struct media_pad *remote_pad;
-	struct v4l2_subdev *source;
-	s64 freq;
-	u64 mbps;
-
-	if (!priv->remote)
-		return -ENODEV;
-
-	source = priv->remote;
-	remote_pad = &source->entity.pads[priv->remote_pad];
-
-	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
-	if (freq < 0) {
-		int ret = (int)freq;
-
-		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
-			source->name, ret);
-
-		return ret;
-	}
-
-	mbps = div_u64(freq * 2, MEGA);
-
-	/* Adjust for C-PHY, divide by 2.8. */
-	if (priv->cphy)
-		mbps = div_u64(mbps * 5, 14);
-
-	return mbps;
-}
-
 static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
 				  unsigned int *lanes)
 {
@@ -1031,6 +998,39 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
 	return 0;
 }
 
+static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
+			   unsigned int lanes)
+{
+	struct media_pad *remote_pad;
+	struct v4l2_subdev *source;
+	s64 freq;
+	u64 mbps;
+
+	if (!priv->remote)
+		return -ENODEV;
+
+	source = priv->remote;
+	remote_pad = &source->entity.pads[priv->remote_pad];
+
+	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
+	if (freq < 0) {
+		int ret = (int)freq;
+
+		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
+			source->name, ret);
+
+		return ret;
+	}
+
+	mbps = div_u64(freq * 2, MEGA);
+
+	/* Adjust for C-PHY, divide by 2.8. */
+	if (priv->cphy)
+		mbps = div_u64(mbps * 5, 14);
+
+	return mbps;
+}
+
 static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
 				     struct v4l2_subdev_state *state)
 {

-- 
2.43.0

Re: [PATCH v3 05/15] media: rcar-csi2: Move rcar2_calc_mbps()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:34PM +0300, Tomi Valkeinen wrote:
> Move the function so that it can call rcsi2_get_active_lanes() in the
> following patch.

I would add "No functional change intended." or something similar.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 66 +++++++++++++++---------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 698eb0e60f32..8aca35096408 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -951,39 +951,6 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
>  	return 0;
>  }
>  
> -static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> -			   unsigned int lanes)
> -{
> -	struct media_pad *remote_pad;
> -	struct v4l2_subdev *source;
> -	s64 freq;
> -	u64 mbps;
> -
> -	if (!priv->remote)
> -		return -ENODEV;
> -
> -	source = priv->remote;
> -	remote_pad = &source->entity.pads[priv->remote_pad];
> -
> -	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> -	if (freq < 0) {
> -		int ret = (int)freq;
> -
> -		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> -			source->name, ret);
> -
> -		return ret;
> -	}
> -
> -	mbps = div_u64(freq * 2, MEGA);
> -
> -	/* Adjust for C-PHY, divide by 2.8. */
> -	if (priv->cphy)
> -		mbps = div_u64(mbps * 5, 14);
> -
> -	return mbps;
> -}
> -
>  static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  				  unsigned int *lanes)
>  {
> @@ -1031,6 +998,39 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  	return 0;
>  }
>  
> +static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> +			   unsigned int lanes)
> +{
> +	struct media_pad *remote_pad;
> +	struct v4l2_subdev *source;
> +	s64 freq;
> +	u64 mbps;
> +
> +	if (!priv->remote)
> +		return -ENODEV;
> +
> +	source = priv->remote;
> +	remote_pad = &source->entity.pads[priv->remote_pad];
> +
> +	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> +	if (freq < 0) {
> +		int ret = (int)freq;
> +
> +		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> +			source->name, ret);
> +
> +		return ret;
> +	}
> +
> +	mbps = div_u64(freq * 2, MEGA);
> +
> +	/* Adjust for C-PHY, divide by 2.8. */
> +	if (priv->cphy)
> +		mbps = div_u64(mbps * 5, 14);
> +
> +	return mbps;
> +}
> +
>  static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  				     struct v4l2_subdev_state *state)
>  {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 05/15] media: rcar-csi2: Move rcar2_calc_mbps()

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:34 +0300, Tomi Valkeinen wrote:
> Move the function so that it can call rcsi2_get_active_lanes() in the
> following patch.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 66 +++++++++++++++---------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 698eb0e60f32..8aca35096408 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -951,39 +951,6 @@ static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
>  	return 0;
>  }
>  
> -static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> -			   unsigned int lanes)
> -{
> -	struct media_pad *remote_pad;
> -	struct v4l2_subdev *source;
> -	s64 freq;
> -	u64 mbps;
> -
> -	if (!priv->remote)
> -		return -ENODEV;
> -
> -	source = priv->remote;
> -	remote_pad = &source->entity.pads[priv->remote_pad];
> -
> -	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> -	if (freq < 0) {
> -		int ret = (int)freq;
> -
> -		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> -			source->name, ret);
> -
> -		return ret;
> -	}
> -
> -	mbps = div_u64(freq * 2, MEGA);
> -
> -	/* Adjust for C-PHY, divide by 2.8. */
> -	if (priv->cphy)
> -		mbps = div_u64(mbps * 5, 14);
> -
> -	return mbps;
> -}
> -
>  static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  				  unsigned int *lanes)
>  {
> @@ -1031,6 +998,39 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  	return 0;
>  }
>  
> +static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> +			   unsigned int lanes)
> +{
> +	struct media_pad *remote_pad;
> +	struct v4l2_subdev *source;
> +	s64 freq;
> +	u64 mbps;
> +
> +	if (!priv->remote)
> +		return -ENODEV;
> +
> +	source = priv->remote;
> +	remote_pad = &source->entity.pads[priv->remote_pad];
> +
> +	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> +	if (freq < 0) {
> +		int ret = (int)freq;
> +
> +		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> +			source->name, ret);
> +
> +		return ret;
> +	}
> +
> +	mbps = div_u64(freq * 2, MEGA);
> +
> +	/* Adjust for C-PHY, divide by 2.8. */
> +	if (priv->cphy)
> +		mbps = div_u64(mbps * 5, 14);
> +
> +	return mbps;
> +}
> +
>  static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  				     struct v4l2_subdev_state *state)
>  {
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 06/15] media: rcar-csi2: Simplify rcsi2_calc_mbps()

[Tomi Valkeinen]

Instead of taking the bpp and the number of lanes as parameters to
rcsi2_calc_mbps(), change the function to get those parameters inside
the function. This centralizes the code a bit and makes it easier to add
streams support.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 45 ++++++++++++++++--------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 8aca35096408..90973f3cba38 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -998,13 +998,18 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
 	return 0;
 }
 
-static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
-			   unsigned int lanes)
+static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
+			   struct v4l2_subdev_state *state)
 {
+	const struct rcar_csi2_format *format;
+	struct v4l2_mbus_framefmt *fmt;
 	struct media_pad *remote_pad;
 	struct v4l2_subdev *source;
+	unsigned int lanes;
+	unsigned int bpp;
 	s64 freq;
 	u64 mbps;
+	int ret;
 
 	if (!priv->remote)
 		return -ENODEV;
@@ -1012,6 +1017,20 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
 	source = priv->remote;
 	remote_pad = &source->entity.pads[priv->remote_pad];
 
+	ret = rcsi2_get_active_lanes(priv, &lanes);
+	if (ret)
+		return ret;
+
+	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
+	if (!fmt)
+		return -EINVAL;
+
+	format = rcsi2_code_to_fmt(fmt->code);
+	if (!format)
+		return -EINVAL;
+
+	bpp = format->bpp;
+
 	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
 	if (freq < 0) {
 		int ret = (int)freq;
@@ -1092,7 +1111,7 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
 	phycnt = PHYCNT_ENABLECLK;
 	phycnt |= (1 << lanes) - 1;
 
-	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
+	mbps = rcsi2_calc_mbps(priv, state);
 	if (mbps < 0)
 		return mbps;
 
@@ -1300,23 +1319,15 @@ static int rcsi2_c_phy_setting_v4h(struct rcar_csi2 *priv, int msps)
 static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
 				    struct v4l2_subdev_state *state)
 {
-	const struct rcar_csi2_format *format;
-	const struct v4l2_mbus_framefmt *fmt;
 	unsigned int lanes;
 	int msps;
 	int ret;
 
-	/* Use the format on the sink pad to compute the receiver config. */
-	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
-	format = rcsi2_code_to_fmt(fmt->code);
-	if (!format)
-		return -EINVAL;
-
 	ret = rcsi2_get_active_lanes(priv, &lanes);
 	if (ret)
 		return ret;
 
-	msps = rcsi2_calc_mbps(priv, format->bpp, lanes);
+	msps = rcsi2_calc_mbps(priv, state);
 	if (msps < 0)
 		return msps;
 
@@ -1494,23 +1505,15 @@ static int rcsi2_init_common_v4m(struct rcar_csi2 *priv, unsigned int mbps)
 static int rcsi2_start_receiver_v4m(struct rcar_csi2 *priv,
 				    struct v4l2_subdev_state *state)
 {
-	const struct rcar_csi2_format *format;
-	const struct v4l2_mbus_framefmt *fmt;
 	unsigned int lanes;
 	int mbps;
 	int ret;
 
-	/* Calculate parameters */
-	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
-	format = rcsi2_code_to_fmt(fmt->code);
-	if (!format)
-		return -EINVAL;
-
 	ret = rcsi2_get_active_lanes(priv, &lanes);
 	if (ret)
 		return ret;
 
-	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
+	mbps = rcsi2_calc_mbps(priv, state);
 	if (mbps < 0)
 		return mbps;
 

-- 
2.43.0

Re: [PATCH v3 06/15] media: rcar-csi2: Simplify rcsi2_calc_mbps()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:35PM +0300, Tomi Valkeinen wrote:
> Instead of taking the bpp and the number of lanes as parameters to
> rcsi2_calc_mbps(), change the function to get those parameters inside
> the function. This centralizes the code a bit and makes it easier to add
> streams support.

This comes at the cost of calling rcsi2_get_active_lanes() multiple
times. I will assume for now that subsequent patches will show that the
benefits outweight the cost.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 45 ++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 8aca35096408..90973f3cba38 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -998,13 +998,18 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  	return 0;
>  }
>  
> -static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> -			   unsigned int lanes)
> +static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> +			   struct v4l2_subdev_state *state)
>  {
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_mbus_framefmt *fmt;
>  	struct media_pad *remote_pad;
>  	struct v4l2_subdev *source;
> +	unsigned int lanes;
> +	unsigned int bpp;
>  	s64 freq;
>  	u64 mbps;
> +	int ret;
>  
>  	if (!priv->remote)
>  		return -ENODEV;
> @@ -1012,6 +1017,20 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  	source = priv->remote;
>  	remote_pad = &source->entity.pads[priv->remote_pad];
>  
> +	ret = rcsi2_get_active_lanes(priv, &lanes);
> +	if (ret)
> +		return ret;
> +
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	format = rcsi2_code_to_fmt(fmt->code);
> +	if (!format)
> +		return -EINVAL;
> +
> +	bpp = format->bpp;
> +
>  	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
>  	if (freq < 0) {
>  		int ret = (int)freq;
> @@ -1092,7 +1111,7 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  	phycnt = PHYCNT_ENABLECLK;
>  	phycnt |= (1 << lanes) - 1;
>  
> -	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	mbps = rcsi2_calc_mbps(priv, state);
>  	if (mbps < 0)
>  		return mbps;
>  
> @@ -1300,23 +1319,15 @@ static int rcsi2_c_phy_setting_v4h(struct rcar_csi2 *priv, int msps)
>  static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
>  				    struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	const struct v4l2_mbus_framefmt *fmt;
>  	unsigned int lanes;
>  	int msps;
>  	int ret;
>  
> -	/* Use the format on the sink pad to compute the receiver config. */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> -
>  	ret = rcsi2_get_active_lanes(priv, &lanes);
>  	if (ret)
>  		return ret;
>  
> -	msps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	msps = rcsi2_calc_mbps(priv, state);
>  	if (msps < 0)
>  		return msps;
>  
> @@ -1494,23 +1505,15 @@ static int rcsi2_init_common_v4m(struct rcar_csi2 *priv, unsigned int mbps)
>  static int rcsi2_start_receiver_v4m(struct rcar_csi2 *priv,
>  				    struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	const struct v4l2_mbus_framefmt *fmt;
>  	unsigned int lanes;
>  	int mbps;
>  	int ret;
>  
> -	/* Calculate parameters */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> -
>  	ret = rcsi2_get_active_lanes(priv, &lanes);
>  	if (ret)
>  		return ret;
>  
> -	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	mbps = rcsi2_calc_mbps(priv, state);
>  	if (mbps < 0)
>  		return mbps;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 06/15] media: rcar-csi2: Simplify rcsi2_calc_mbps()

[Niklas Söderlund]

Hi Tomi,

Thanks for your patch.

On 2025-05-30 16:50:35 +0300, Tomi Valkeinen wrote:
> Instead of taking the bpp and the number of lanes as parameters to
> rcsi2_calc_mbps(), change the function to get those parameters inside
> the function. This centralizes the code a bit and makes it easier to add
> streams support.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 45 ++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 8aca35096408..90973f3cba38 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -998,13 +998,18 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  	return 0;
>  }
>  
> -static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
> -			   unsigned int lanes)
> +static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> +			   struct v4l2_subdev_state *state)
>  {
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_mbus_framefmt *fmt;
>  	struct media_pad *remote_pad;
>  	struct v4l2_subdev *source;
> +	unsigned int lanes;
> +	unsigned int bpp;
>  	s64 freq;
>  	u64 mbps;
> +	int ret;
>  
>  	if (!priv->remote)
>  		return -ENODEV;
> @@ -1012,6 +1017,20 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  	source = priv->remote;
>  	remote_pad = &source->entity.pads[priv->remote_pad];
>  
> +	ret = rcsi2_get_active_lanes(priv, &lanes);
> +	if (ret)
> +		return ret;
> +
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	format = rcsi2_code_to_fmt(fmt->code);
> +	if (!format)
> +		return -EINVAL;
> +
> +	bpp = format->bpp;
> +
>  	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
>  	if (freq < 0) {
>  		int ret = (int)freq;
> @@ -1092,7 +1111,7 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  	phycnt = PHYCNT_ENABLECLK;
>  	phycnt |= (1 << lanes) - 1;
>  
> -	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	mbps = rcsi2_calc_mbps(priv, state);
>  	if (mbps < 0)
>  		return mbps;
>  
> @@ -1300,23 +1319,15 @@ static int rcsi2_c_phy_setting_v4h(struct rcar_csi2 *priv, int msps)
>  static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
>  				    struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	const struct v4l2_mbus_framefmt *fmt;
>  	unsigned int lanes;
>  	int msps;
>  	int ret;
>  
> -	/* Use the format on the sink pad to compute the receiver config. */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> -
>  	ret = rcsi2_get_active_lanes(priv, &lanes);
>  	if (ret)
>  		return ret;
>  
> -	msps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	msps = rcsi2_calc_mbps(priv, state);
>  	if (msps < 0)
>  		return msps;
>  
> @@ -1494,23 +1505,15 @@ static int rcsi2_init_common_v4m(struct rcar_csi2 *priv, unsigned int mbps)
>  static int rcsi2_start_receiver_v4m(struct rcar_csi2 *priv,
>  				    struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	const struct v4l2_mbus_framefmt *fmt;
>  	unsigned int lanes;
>  	int mbps;
>  	int ret;
>  
> -	/* Calculate parameters */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> -
>  	ret = rcsi2_get_active_lanes(priv, &lanes);
>  	if (ret)
>  		return ret;
>  
> -	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	mbps = rcsi2_calc_mbps(priv, state);
>  	if (mbps < 0)
>  		return mbps;
>  
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 07/15] media: rcar-csi2: Optimize rcsi2_calc_mbps()

[Tomi Valkeinen]

With modern drivers supporting link-freq, we don't need to do any
calculations based on the bpp and number of lanes when figuring out the
link frequency. However, the code currently always runs code to get the
bpp and number of lanes.

Optimize the rcsi2_calc_mbps() so that we only do that when needed, i.e.
when querying the link-freq is not supported by the upstream subdevice.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 50 +++++++++++++++++-------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 90973f3cba38..e0a0fd96459b 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1001,15 +1001,10 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
 static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
 			   struct v4l2_subdev_state *state)
 {
-	const struct rcar_csi2_format *format;
-	struct v4l2_mbus_framefmt *fmt;
 	struct media_pad *remote_pad;
 	struct v4l2_subdev *source;
-	unsigned int lanes;
-	unsigned int bpp;
 	s64 freq;
 	u64 mbps;
-	int ret;
 
 	if (!priv->remote)
 		return -ENODEV;
@@ -1017,28 +1012,41 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
 	source = priv->remote;
 	remote_pad = &source->entity.pads[priv->remote_pad];
 
-	ret = rcsi2_get_active_lanes(priv, &lanes);
-	if (ret)
-		return ret;
+	/*
+	 * First try to get the real link freq. If that fails, try the heuristic
+	 * method with bpp and lanes (but that only works for one route).
+	 */
+	freq = v4l2_get_link_freq(remote_pad, 0, 0);
+	if (freq < 0) {
+		const struct rcar_csi2_format *format;
+		struct v4l2_mbus_framefmt *fmt;
+		unsigned int lanes;
+		unsigned int bpp;
+		int ret;
 
-	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
-	if (!fmt)
-		return -EINVAL;
+		ret = rcsi2_get_active_lanes(priv, &lanes);
+		if (ret)
+			return ret;
 
-	format = rcsi2_code_to_fmt(fmt->code);
-	if (!format)
-		return -EINVAL;
+		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
+		if (!fmt)
+			return -EINVAL;
 
-	bpp = format->bpp;
+		format = rcsi2_code_to_fmt(fmt->code);
+		if (!format)
+			return -EINVAL;
 
-	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
-	if (freq < 0) {
-		int ret = (int)freq;
+		bpp = format->bpp;
 
-		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
-			source->name, ret);
+		freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
+		if (freq < 0) {
+			int ret = (int)freq;
 
-		return ret;
+			dev_err(priv->dev, "failed to get link freq for %s: %d\n",
+				source->name, ret);
+
+			return ret;
+		}
 	}
 
 	mbps = div_u64(freq * 2, MEGA);

-- 
2.43.0

Re: [PATCH v3 07/15] media: rcar-csi2: Optimize rcsi2_calc_mbps()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:36PM +0300, Tomi Valkeinen wrote:
> With modern drivers supporting link-freq, we don't need to do any
> calculations based on the bpp and number of lanes when figuring out the
> link frequency. However, the code currently always runs code to get the
> bpp and number of lanes.
> 
> Optimize the rcsi2_calc_mbps() so that we only do that when needed, i.e.
> when querying the link-freq is not supported by the upstream subdevice.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 50 +++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 90973f3cba38..e0a0fd96459b 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1001,15 +1001,10 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  			   struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	struct v4l2_mbus_framefmt *fmt;
>  	struct media_pad *remote_pad;
>  	struct v4l2_subdev *source;
> -	unsigned int lanes;
> -	unsigned int bpp;
>  	s64 freq;
>  	u64 mbps;
> -	int ret;
>  
>  	if (!priv->remote)
>  		return -ENODEV;
> @@ -1017,28 +1012,41 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  	source = priv->remote;
>  	remote_pad = &source->entity.pads[priv->remote_pad];
>  
> -	ret = rcsi2_get_active_lanes(priv, &lanes);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * First try to get the real link freq. If that fails, try the heuristic
> +	 * method with bpp and lanes (but that only works for one route).
> +	 */
> +	freq = v4l2_get_link_freq(remote_pad, 0, 0);
> +	if (freq < 0) {
> +		const struct rcar_csi2_format *format;
> +		struct v4l2_mbus_framefmt *fmt;

This can be const.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +		unsigned int lanes;
> +		unsigned int bpp;
> +		int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> -	if (!fmt)
> -		return -EINVAL;
> +		ret = rcsi2_get_active_lanes(priv, &lanes);
> +		if (ret)
> +			return ret;
>  
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> +		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +		if (!fmt)
> +			return -EINVAL;
>  
> -	bpp = format->bpp;
> +		format = rcsi2_code_to_fmt(fmt->code);
> +		if (!format)
> +			return -EINVAL;
>  
> -	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> -	if (freq < 0) {
> -		int ret = (int)freq;
> +		bpp = format->bpp;
>  
> -		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> -			source->name, ret);
> +		freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> +		if (freq < 0) {
> +			int ret = (int)freq;
>  
> -		return ret;
> +			dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> +				source->name, ret);
> +
> +			return ret;
> +		}
>  	}
>  
>  	mbps = div_u64(freq * 2, MEGA);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 07/15] media: rcar-csi2: Optimize rcsi2_calc_mbps()

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:36 +0300, Tomi Valkeinen wrote:
> With modern drivers supporting link-freq, we don't need to do any
> calculations based on the bpp and number of lanes when figuring out the
> link frequency. However, the code currently always runs code to get the
> bpp and number of lanes.
> 
> Optimize the rcsi2_calc_mbps() so that we only do that when needed, i.e.
> when querying the link-freq is not supported by the upstream subdevice.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

I wonder if it make sens to add a dev_warn_once() for the old call path 
so we won't forget to update all known users of this old way, and once 
fixed we can remove the huristic method all together?

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 50 +++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 90973f3cba38..e0a0fd96459b 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1001,15 +1001,10 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  			   struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	struct v4l2_mbus_framefmt *fmt;
>  	struct media_pad *remote_pad;
>  	struct v4l2_subdev *source;
> -	unsigned int lanes;
> -	unsigned int bpp;
>  	s64 freq;
>  	u64 mbps;
> -	int ret;
>  
>  	if (!priv->remote)
>  		return -ENODEV;
> @@ -1017,28 +1012,41 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  	source = priv->remote;
>  	remote_pad = &source->entity.pads[priv->remote_pad];
>  
> -	ret = rcsi2_get_active_lanes(priv, &lanes);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * First try to get the real link freq. If that fails, try the heuristic
> +	 * method with bpp and lanes (but that only works for one route).
> +	 */
> +	freq = v4l2_get_link_freq(remote_pad, 0, 0);
> +	if (freq < 0) {
> +		const struct rcar_csi2_format *format;
> +		struct v4l2_mbus_framefmt *fmt;
> +		unsigned int lanes;
> +		unsigned int bpp;
> +		int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> -	if (!fmt)
> -		return -EINVAL;
> +		ret = rcsi2_get_active_lanes(priv, &lanes);
> +		if (ret)
> +			return ret;
>  
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> +		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +		if (!fmt)
> +			return -EINVAL;
>  
> -	bpp = format->bpp;
> +		format = rcsi2_code_to_fmt(fmt->code);
> +		if (!format)
> +			return -EINVAL;
>  
> -	freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> -	if (freq < 0) {
> -		int ret = (int)freq;
> +		bpp = format->bpp;
>  
> -		dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> -			source->name, ret);
> +		freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> +		if (freq < 0) {
> +			int ret = (int)freq;
>  
> -		return ret;
> +			dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> +				source->name, ret);
> +
> +			return ret;
> +		}
>  	}
>  
>  	mbps = div_u64(freq * 2, MEGA);
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 08/15] media: rcar-csi2: Switch to Streams API

[Tomi Valkeinen]

Switch to Streams API with a single hardcoded route. This breaks any
existing userspace which depended on the custom rcar streams
implementation, but a single camera use case should continue to work.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 47 +++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index e0a0fd96459b..20bd44274bd2 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1028,7 +1028,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
 		if (ret)
 			return ret;
 
-		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
+		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
 		if (!fmt)
 			return -EINVAL;
 
@@ -1069,7 +1069,7 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
 	int mbps, ret;
 
 	/* Use the format on the sink pad to compute the receiver config. */
-	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
+	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
 
 	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
 		fmt->width, fmt->height,
@@ -1650,8 +1650,7 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
 				struct v4l2_subdev_state *state,
 				struct v4l2_subdev_format *format)
 {
-	struct rcar_csi2 *priv = sd_to_csi2(sd);
-	unsigned int num_pads = rcsi2_num_pads(priv);
+	struct v4l2_mbus_framefmt *fmt;
 
 	if (format->pad > RCAR_CSI2_SINK)
 		return v4l2_subdev_get_fmt(sd, state, format);
@@ -1659,11 +1658,20 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
 	if (!rcsi2_code_to_fmt(format->format.code))
 		format->format.code = rcar_csi2_formats[0].code;
 
-	*v4l2_subdev_state_get_format(state, format->pad) = format->format;
+	/* Set sink format */
+	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
+
+	/* Propagate to source format */
+	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+							   format->stream);
+	if (!fmt)
+		return -EINVAL;
 
-	/* Propagate the format to the source pads. */
-	for (unsigned int i = RCAR_CSI2_SOURCE_VC0; i < num_pads; i++)
-		*v4l2_subdev_state_get_format(state, i) = format->format;
+	*fmt = format->format;
 
 	return 0;
 }
@@ -1683,8 +1691,15 @@ static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
 static int rcsi2_init_state(struct v4l2_subdev *sd,
 			    struct v4l2_subdev_state *state)
 {
-	struct rcar_csi2 *priv = sd_to_csi2(sd);
-	unsigned int num_pads = rcsi2_num_pads(priv);
+	static struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = RCAR_CSI2_SINK,
+			.sink_stream = 0,
+			.source_pad = RCAR_CSI2_SOURCE_VC0,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
 
 	static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
 		.width		= 1920,
@@ -1697,10 +1712,13 @@ static int rcsi2_init_state(struct v4l2_subdev *sd,
 		.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
 	};
 
-	for (unsigned int i = RCAR_CSI2_SINK; i < num_pads; i++)
-		*v4l2_subdev_state_get_format(state, i) = rcar_csi2_default_fmt;
+	static const struct v4l2_subdev_krouting routing = {
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
 
-	return 0;
+	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
+						&rcar_csi2_default_fmt);
 }
 
 static const struct v4l2_subdev_internal_ops rcar_csi2_internal_ops = {
@@ -2356,7 +2374,8 @@ static int rcsi2_probe(struct platform_device *pdev)
 	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
 	snprintf(priv->subdev.name, sizeof(priv->subdev.name), "%s %s",
 		 KBUILD_MODNAME, dev_name(&pdev->dev));
-	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE |
+			     V4L2_SUBDEV_FL_STREAMS;
 
 	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
 	priv->subdev.entity.ops = &rcar_csi2_entity_ops;

-- 
2.43.0

Re: [PATCH v3 08/15] media: rcar-csi2: Switch to Streams API

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:37PM +0300, Tomi Valkeinen wrote:
> Switch to Streams API with a single hardcoded route. This breaks any
> existing userspace which depended on the custom rcar streams
> implementation, but a single camera use case should continue to work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 47 +++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index e0a0fd96459b..20bd44274bd2 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1028,7 +1028,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  		if (ret)
>  			return ret;
>  
> -		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
>  		if (!fmt)
>  			return -EINVAL;
>  
> @@ -1069,7 +1069,7 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  	int mbps, ret;
>  
>  	/* Use the format on the sink pad to compute the receiver config. */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
>  
>  	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
>  		fmt->width, fmt->height,
> @@ -1650,8 +1650,7 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  				struct v4l2_subdev_state *state,
>  				struct v4l2_subdev_format *format)
>  {
> -	struct rcar_csi2 *priv = sd_to_csi2(sd);
> -	unsigned int num_pads = rcsi2_num_pads(priv);
> +	struct v4l2_mbus_framefmt *fmt;
>  
>  	if (format->pad > RCAR_CSI2_SINK)
>  		return v4l2_subdev_get_fmt(sd, state, format);
> @@ -1659,11 +1658,20 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	if (!rcsi2_code_to_fmt(format->format.code))
>  		format->format.code = rcar_csi2_formats[0].code;
>  
> -	*v4l2_subdev_state_get_format(state, format->pad) = format->format;
> +	/* Set sink format */

s/format/format./

> +	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
> +
> +	/* Propagate to source format */

Same here. Although I'd write

	/* Propagate the format to the source pad. */

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
>  
> -	/* Propagate the format to the source pads. */
> -	for (unsigned int i = RCAR_CSI2_SOURCE_VC0; i < num_pads; i++)
> -		*v4l2_subdev_state_get_format(state, i) = format->format;
> +	*fmt = format->format;
>  
>  	return 0;
>  }
> @@ -1683,8 +1691,15 @@ static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
>  static int rcsi2_init_state(struct v4l2_subdev *sd,
>  			    struct v4l2_subdev_state *state)
>  {
> -	struct rcar_csi2 *priv = sd_to_csi2(sd);
> -	unsigned int num_pads = rcsi2_num_pads(priv);
> +	static struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = RCAR_CSI2_SINK,
> +			.sink_stream = 0,
> +			.source_pad = RCAR_CSI2_SOURCE_VC0,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
>  
>  	static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
>  		.width		= 1920,
> @@ -1697,10 +1712,13 @@ static int rcsi2_init_state(struct v4l2_subdev *sd,
>  		.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
>  	};
>  
> -	for (unsigned int i = RCAR_CSI2_SINK; i < num_pads; i++)
> -		*v4l2_subdev_state_get_format(state, i) = rcar_csi2_default_fmt;
> +	static const struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
>  
> -	return 0;
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
> +						&rcar_csi2_default_fmt);
>  }
>  
>  static const struct v4l2_subdev_internal_ops rcar_csi2_internal_ops = {
> @@ -2356,7 +2374,8 @@ static int rcsi2_probe(struct platform_device *pdev)
>  	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
>  	snprintf(priv->subdev.name, sizeof(priv->subdev.name), "%s %s",
>  		 KBUILD_MODNAME, dev_name(&pdev->dev));
> -	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE |
> +			     V4L2_SUBDEV_FL_STREAMS;
>  
>  	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>  	priv->subdev.entity.ops = &rcar_csi2_entity_ops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 08/15] media: rcar-csi2: Switch to Streams API

[Niklas Söderlund]

Hi Tomi,

Thanks for your patch.

On 2025-05-30 16:50:37 +0300, Tomi Valkeinen wrote:
> Switch to Streams API with a single hardcoded route. This breaks any
> existing userspace which depended on the custom rcar streams
> implementation, but a single camera use case should continue to work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 47 +++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index e0a0fd96459b..20bd44274bd2 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1028,7 +1028,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  		if (ret)
>  			return ret;
>  
> -		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
>  		if (!fmt)
>  			return -EINVAL;
>  
> @@ -1069,7 +1069,7 @@ static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  	int mbps, ret;
>  
>  	/* Use the format on the sink pad to compute the receiver config. */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
>  
>  	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
>  		fmt->width, fmt->height,
> @@ -1650,8 +1650,7 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  				struct v4l2_subdev_state *state,
>  				struct v4l2_subdev_format *format)
>  {
> -	struct rcar_csi2 *priv = sd_to_csi2(sd);
> -	unsigned int num_pads = rcsi2_num_pads(priv);
> +	struct v4l2_mbus_framefmt *fmt;
>  
>  	if (format->pad > RCAR_CSI2_SINK)
>  		return v4l2_subdev_get_fmt(sd, state, format);
> @@ -1659,11 +1658,20 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	if (!rcsi2_code_to_fmt(format->format.code))
>  		format->format.code = rcar_csi2_formats[0].code;
>  
> -	*v4l2_subdev_state_get_format(state, format->pad) = format->format;
> +	/* Set sink format */
> +	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
> +
> +	/* Propagate to source format */
> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
>  
> -	/* Propagate the format to the source pads. */
> -	for (unsigned int i = RCAR_CSI2_SOURCE_VC0; i < num_pads; i++)
> -		*v4l2_subdev_state_get_format(state, i) = format->format;
> +	*fmt = format->format;
>  
>  	return 0;
>  }
> @@ -1683,8 +1691,15 @@ static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
>  static int rcsi2_init_state(struct v4l2_subdev *sd,
>  			    struct v4l2_subdev_state *state)
>  {
> -	struct rcar_csi2 *priv = sd_to_csi2(sd);
> -	unsigned int num_pads = rcsi2_num_pads(priv);
> +	static struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = RCAR_CSI2_SINK,
> +			.sink_stream = 0,
> +			.source_pad = RCAR_CSI2_SOURCE_VC0,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
>  
>  	static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
>  		.width		= 1920,
> @@ -1697,10 +1712,13 @@ static int rcsi2_init_state(struct v4l2_subdev *sd,
>  		.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
>  	};
>  
> -	for (unsigned int i = RCAR_CSI2_SINK; i < num_pads; i++)
> -		*v4l2_subdev_state_get_format(state, i) = rcar_csi2_default_fmt;
> +	static const struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
>  
> -	return 0;
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
> +						&rcar_csi2_default_fmt);
>  }
>  
>  static const struct v4l2_subdev_internal_ops rcar_csi2_internal_ops = {
> @@ -2356,7 +2374,8 @@ static int rcsi2_probe(struct platform_device *pdev)
>  	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
>  	snprintf(priv->subdev.name, sizeof(priv->subdev.name), "%s %s",
>  		 KBUILD_MODNAME, dev_name(&pdev->dev));
> -	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE |
> +			     V4L2_SUBDEV_FL_STREAMS;
>  
>  	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>  	priv->subdev.entity.ops = &rcar_csi2_entity_ops;
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 09/15] media: rcar-isp: Switch to Streams API

[Tomi Valkeinen]

Switch to Streams API with a single hardcoded route. This breaks any
existing userspace which depended on the custom rcar streams
implementation, but a single camera use case should continue to work.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 62 ++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 2337c5d44c40..a04cbf96b809 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -124,6 +124,17 @@ static const struct rcar_isp_format rcar_isp_formats[] = {
 	},
 };
 
+static const struct v4l2_mbus_framefmt risp_default_fmt = {
+	.width = 1920,
+	.height = 1080,
+	.code = MEDIA_BUS_FMT_RGB888_1X24,
+	.colorspace = V4L2_COLORSPACE_SRGB,
+	.field = V4L2_FIELD_NONE,
+	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
+	.quantization = V4L2_QUANTIZATION_DEFAULT,
+	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
+};
+
 static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
 {
 	unsigned int i;
@@ -222,7 +233,7 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
 	u32 sel_csi = 0;
 	int ret;
 
-	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK);
+	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
 	if (!fmt)
 		return -EINVAL;
 
@@ -336,7 +347,7 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_state *state,
 			       struct v4l2_subdev_format *format)
 {
-	struct v4l2_mbus_framefmt *framefmt;
+	struct v4l2_mbus_framefmt *fmt;
 
 	if (format->pad > RCAR_ISP_SINK)
 		return v4l2_subdev_get_fmt(sd, state, format);
@@ -344,10 +355,20 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
 	if (!risp_code_to_fmt(format->format.code))
 		format->format.code = rcar_isp_formats[0].code;
 
-	for (unsigned int i = 0; i < RCAR_ISP_NUM_PADS; i++) {
-		framefmt = v4l2_subdev_state_get_format(state, i);
-		*framefmt = format->format;
-	}
+	/* Set sink format */
+	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
+
+	/* Propagate to source format */
+	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+							   format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
 
 	return 0;
 }
@@ -364,6 +385,32 @@ static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {
 	.pad	= &risp_pad_ops,
 };
 
+static int risp_init_state(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_state *state)
+{
+	static struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = RCAR_ISP_SINK,
+			.sink_stream = 0,
+			.source_pad = RCAR_ISP_PORT0,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+
+	static const struct v4l2_subdev_krouting routing = {
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
+
+	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
+						&risp_default_fmt);
+}
+
+static const struct v4l2_subdev_internal_ops risp_internal_ops = {
+	.init_state = risp_init_state,
+};
+
 /* -----------------------------------------------------------------------------
  * Async handling and registration of subdevices and links
  */
@@ -521,11 +568,12 @@ static int risp_probe(struct platform_device *pdev)
 
 	isp->subdev.owner = THIS_MODULE;
 	isp->subdev.dev = &pdev->dev;
+	isp->subdev.internal_ops = &risp_internal_ops;
 	v4l2_subdev_init(&isp->subdev, &rcar_isp_subdev_ops);
 	v4l2_set_subdevdata(&isp->subdev, &pdev->dev);
 	snprintf(isp->subdev.name, sizeof(isp->subdev.name), "%s %s",
 		 KBUILD_MODNAME, dev_name(&pdev->dev));
-	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
 
 	isp->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
 	isp->subdev.entity.ops = &risp_entity_ops;

-- 
2.43.0

Re: [PATCH v3 09/15] media: rcar-isp: Switch to Streams API

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:38PM +0300, Tomi Valkeinen wrote:
> Switch to Streams API with a single hardcoded route. This breaks any
> existing userspace which depended on the custom rcar streams
> implementation, but a single camera use case should continue to work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 62 ++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 2337c5d44c40..a04cbf96b809 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -124,6 +124,17 @@ static const struct rcar_isp_format rcar_isp_formats[] = {
>  	},
>  };
>  
> +static const struct v4l2_mbus_framefmt risp_default_fmt = {
> +	.width = 1920,
> +	.height = 1080,
> +	.code = MEDIA_BUS_FMT_RGB888_1X24,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +	.field = V4L2_FIELD_NONE,
> +	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> +	.quantization = V4L2_QUANTIZATION_DEFAULT,
> +	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
> +};
> +
>  static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
>  {
>  	unsigned int i;
> @@ -222,7 +233,7 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	u32 sel_csi = 0;
>  	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK);
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
>  	if (!fmt)
>  		return -EINVAL;
>  
> @@ -336,7 +347,7 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  			       struct v4l2_subdev_state *state,
>  			       struct v4l2_subdev_format *format)
>  {
> -	struct v4l2_mbus_framefmt *framefmt;
> +	struct v4l2_mbus_framefmt *fmt;
>  
>  	if (format->pad > RCAR_ISP_SINK)
>  		return v4l2_subdev_get_fmt(sd, state, format);
> @@ -344,10 +355,20 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  	if (!risp_code_to_fmt(format->format.code))
>  		format->format.code = rcar_isp_formats[0].code;
>  
> -	for (unsigned int i = 0; i < RCAR_ISP_NUM_PADS; i++) {
> -		framefmt = v4l2_subdev_state_get_format(state, i);
> -		*framefmt = format->format;
> -	}
> +	/* Set sink format */

s/format/format./

> +	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
> +
> +	/* Propagate to source format */

	/* Propagate the format to the source pad. */

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
>  
>  	return 0;
>  }
> @@ -364,6 +385,32 @@ static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {
>  	.pad	= &risp_pad_ops,
>  };
>  
> +static int risp_init_state(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *state)
> +{
> +	static struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = RCAR_ISP_SINK,
> +			.sink_stream = 0,
> +			.source_pad = RCAR_ISP_PORT0,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +
> +	static const struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
> +
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
> +						&risp_default_fmt);
> +}
> +
> +static const struct v4l2_subdev_internal_ops risp_internal_ops = {
> +	.init_state = risp_init_state,
> +};
> +
>  /* -----------------------------------------------------------------------------
>   * Async handling and registration of subdevices and links
>   */
> @@ -521,11 +568,12 @@ static int risp_probe(struct platform_device *pdev)
>  
>  	isp->subdev.owner = THIS_MODULE;
>  	isp->subdev.dev = &pdev->dev;
> +	isp->subdev.internal_ops = &risp_internal_ops;
>  	v4l2_subdev_init(&isp->subdev, &rcar_isp_subdev_ops);
>  	v4l2_set_subdevdata(&isp->subdev, &pdev->dev);
>  	snprintf(isp->subdev.name, sizeof(isp->subdev.name), "%s %s",
>  		 KBUILD_MODNAME, dev_name(&pdev->dev));
> -	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
>  
>  	isp->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
>  	isp->subdev.entity.ops = &risp_entity_ops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 09/15] media: rcar-isp: Switch to Streams API

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:38 +0300, Tomi Valkeinen wrote:
> Switch to Streams API with a single hardcoded route. This breaks any
> existing userspace which depended on the custom rcar streams
> implementation, but a single camera use case should continue to work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

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

> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 62 ++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 2337c5d44c40..a04cbf96b809 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -124,6 +124,17 @@ static const struct rcar_isp_format rcar_isp_formats[] = {
>  	},
>  };
>  
> +static const struct v4l2_mbus_framefmt risp_default_fmt = {
> +	.width = 1920,
> +	.height = 1080,
> +	.code = MEDIA_BUS_FMT_RGB888_1X24,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +	.field = V4L2_FIELD_NONE,
> +	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> +	.quantization = V4L2_QUANTIZATION_DEFAULT,
> +	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
> +};
> +
>  static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
>  {
>  	unsigned int i;
> @@ -222,7 +233,7 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	u32 sel_csi = 0;
>  	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK);
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
>  	if (!fmt)
>  		return -EINVAL;
>  
> @@ -336,7 +347,7 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  			       struct v4l2_subdev_state *state,
>  			       struct v4l2_subdev_format *format)
>  {
> -	struct v4l2_mbus_framefmt *framefmt;
> +	struct v4l2_mbus_framefmt *fmt;
>  
>  	if (format->pad > RCAR_ISP_SINK)
>  		return v4l2_subdev_get_fmt(sd, state, format);
> @@ -344,10 +355,20 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  	if (!risp_code_to_fmt(format->format.code))
>  		format->format.code = rcar_isp_formats[0].code;
>  
> -	for (unsigned int i = 0; i < RCAR_ISP_NUM_PADS; i++) {
> -		framefmt = v4l2_subdev_state_get_format(state, i);
> -		*framefmt = format->format;
> -	}
> +	/* Set sink format */
> +	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
> +
> +	/* Propagate to source format */
> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
>  
>  	return 0;
>  }
> @@ -364,6 +385,32 @@ static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {
>  	.pad	= &risp_pad_ops,
>  };
>  
> +static int risp_init_state(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *state)
> +{
> +	static struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = RCAR_ISP_SINK,
> +			.sink_stream = 0,
> +			.source_pad = RCAR_ISP_PORT0,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +
> +	static const struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
> +
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
> +						&risp_default_fmt);
> +}
> +
> +static const struct v4l2_subdev_internal_ops risp_internal_ops = {
> +	.init_state = risp_init_state,
> +};
> +
>  /* -----------------------------------------------------------------------------
>   * Async handling and registration of subdevices and links
>   */
> @@ -521,11 +568,12 @@ static int risp_probe(struct platform_device *pdev)
>  
>  	isp->subdev.owner = THIS_MODULE;
>  	isp->subdev.dev = &pdev->dev;
> +	isp->subdev.internal_ops = &risp_internal_ops;
>  	v4l2_subdev_init(&isp->subdev, &rcar_isp_subdev_ops);
>  	v4l2_set_subdevdata(&isp->subdev, &pdev->dev);
>  	snprintf(isp->subdev.name, sizeof(isp->subdev.name), "%s %s",
>  		 KBUILD_MODNAME, dev_name(&pdev->dev));
> -	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
>  
>  	isp->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
>  	isp->subdev.entity.ops = &risp_entity_ops;
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

[Tomi Valkeinen]

Add v4l2_subdev_pad_ops.get_frame_desc() implementation.

We also implement a fallback for the case where the upstream subdevice
does not implement .get_frame_desc. It assumes a single stream with VC =
0 and DT based on the configured stream mbus format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 20bd44274bd2..65c7f3040696 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
+					 unsigned int pad,
+					 struct v4l2_mbus_frame_desc *fd)
+{
+	const struct rcar_csi2_format *format;
+	struct v4l2_subdev_state *state;
+	struct v4l2_mbus_framefmt *fmt;
+	int ret = 0;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+
+	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
+	if (!fmt) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	format = rcsi2_code_to_fmt(fmt->code);
+	if (!format) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	fd->num_entries = 1;
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+	fd->entry[0].stream = 0;
+	fd->entry[0].pixelcode = fmt->code;
+	fd->entry[0].bus.csi2.vc = 0;
+	fd->entry[0].bus.csi2.dt = format->datatype;
+
+out:
+	v4l2_subdev_unlock_state(state);
+
+	return ret;
+}
+
+static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				struct v4l2_mbus_frame_desc *fd)
+{
+	struct rcar_csi2 *priv = sd_to_csi2(sd);
+	int ret;
+
+	if (WARN_ON(!priv->info->use_isp))
+		return -ENOTTY;
+
+	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
+		return -EINVAL;
+
+	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
+	if (ret == -ENOIOCTLCMD)
+		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
+	return ret;
+}
+
 static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
 	.enable_streams = rcsi2_enable_streams,
 	.disable_streams = rcsi2_disable_streams,
 
 	.set_fmt = rcsi2_set_pad_format,
 	.get_fmt = v4l2_subdev_get_fmt,
+
+	.get_frame_desc = rcsi2_get_frame_desc,
 };
 
 static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {

-- 
2.43.0

Re: [PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:39PM +0300, Tomi Valkeinen wrote:
> Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
> 
> We also implement a fallback for the case where the upstream subdevice
> does not implement .get_frame_desc. It assumes a single stream with VC =
> 0 and DT based on the configured stream mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 20bd44274bd2..65c7f3040696 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
> +					 unsigned int pad,
> +					 struct v4l2_mbus_frame_desc *fd)
> +{
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_subdev_state *state;
> +	struct v4l2_mbus_framefmt *fmt;
> +	int ret = 0;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);

This reminds me we should pass the state to the .get_frame_desc()
operation.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +	if (!fmt) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	format = rcsi2_code_to_fmt(fmt->code);
> +	if (!format) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	fd->num_entries = 1;
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	fd->entry[0].stream = 0;
> +	fd->entry[0].pixelcode = fmt->code;
> +	fd->entry[0].bus.csi2.vc = 0;
> +	fd->entry[0].bus.csi2.dt = format->datatype;
> +
> +out:
> +	v4l2_subdev_unlock_state(state);
> +
> +	return ret;
> +}
> +
> +static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	int ret;
> +
> +	if (WARN_ON(!priv->info->use_isp))
> +		return -ENOTTY;
> +
> +	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
> +	if (ret == -ENOIOCTLCMD)
> +		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
> +	return ret;
> +}
> +
>  static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>  	.enable_streams = rcsi2_enable_streams,
>  	.disable_streams = rcsi2_disable_streams,
>  
>  	.set_fmt = rcsi2_set_pad_format,
>  	.get_fmt = v4l2_subdev_get_fmt,
> +
> +	.get_frame_desc = rcsi2_get_frame_desc,
>  };
>  
>  static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:39 +0300, Tomi Valkeinen wrote:
> Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
> 
> We also implement a fallback for the case where the upstream subdevice
> does not implement .get_frame_desc. It assumes a single stream with VC =
> 0 and DT based on the configured stream mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 20bd44274bd2..65c7f3040696 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
> +					 unsigned int pad,
> +					 struct v4l2_mbus_frame_desc *fd)
> +{
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_subdev_state *state;
> +	struct v4l2_mbus_framefmt *fmt;
> +	int ret = 0;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +	if (!fmt) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	format = rcsi2_code_to_fmt(fmt->code);
> +	if (!format) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	fd->num_entries = 1;
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	fd->entry[0].stream = 0;
> +	fd->entry[0].pixelcode = fmt->code;
> +	fd->entry[0].bus.csi2.vc = 0;
> +	fd->entry[0].bus.csi2.dt = format->datatype;
> +
> +out:
> +	v4l2_subdev_unlock_state(state);
> +
> +	return ret;
> +}
> +
> +static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	int ret;
> +
> +	if (WARN_ON(!priv->info->use_isp))
> +		return -ENOTTY;

This looks odd, why can't we support the frame descriptor and streams on 
Gen3 that do not use the ISP Channel Selector?

> +
> +	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
> +	if (ret == -ENOIOCTLCMD)
> +		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
> +	return ret;
> +}
> +
>  static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>  	.enable_streams = rcsi2_enable_streams,
>  	.disable_streams = rcsi2_disable_streams,
>  
>  	.set_fmt = rcsi2_set_pad_format,
>  	.get_fmt = v4l2_subdev_get_fmt,
> +
> +	.get_frame_desc = rcsi2_get_frame_desc,
>  };
>  
>  static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 11/15] media: rcar-isp: Call get_frame_desc to find out VC & DT

[Tomi Valkeinen]

Call get_frame_desc to find out VC & DT, instead of hardcoding the VC
routing and deducing the DT based on the mbus format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 108 +++++++++++++++++-------
 1 file changed, 77 insertions(+), 31 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index a04cbf96b809..887d8eb21a3a 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -225,24 +225,86 @@ static void risp_power_off(struct rcar_isp *isp)
 	pm_runtime_put(isp->dev);
 }
 
-static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
+static int risp_configure_routing(struct rcar_isp *isp,
+				  struct v4l2_subdev_state *state)
 {
-	const struct v4l2_mbus_framefmt *fmt;
-	const struct rcar_isp_format *format;
-	unsigned int vc;
-	u32 sel_csi = 0;
+	struct v4l2_mbus_frame_desc source_fd;
+	struct v4l2_subdev_route *route;
 	int ret;
 
-	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
-	if (!fmt)
-		return -EINVAL;
+	ret = v4l2_subdev_call(isp->remote, pad, get_frame_desc,
+			       isp->remote_pad, &source_fd);
+	if (ret)
+		return ret;
 
-	format = risp_code_to_fmt(fmt->code);
-	if (!format) {
-		dev_err(isp->dev, "Unsupported bus format\n");
-		return -EINVAL;
+	/* Clear the channel registers */
+	for (unsigned int ch = 0; ch < 12; ++ch) {
+		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), 0);
+		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch), 0);
 	}
 
+	/* Clear the proc mode registers */
+	for (unsigned int dt = 0; dt < 64; ++dt)
+		risp_write_cs(isp, ISPPROCMODE_DT_REG(dt), 0);
+
+	for_each_active_route(&state->routing, route) {
+		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
+		const struct rcar_isp_format *format;
+		const struct v4l2_mbus_framefmt *fmt;
+		unsigned int i;
+		u8 vc, dt, ch;
+		u32 v;
+
+		for (i = 0; i < source_fd.num_entries; i++) {
+			if (source_fd.entry[i].stream == route->sink_stream) {
+				source_entry = &source_fd.entry[i];
+				break;
+			}
+		}
+
+		if (!source_entry) {
+			dev_err(isp->dev,
+				"Failed to find stream from source frame desc\n");
+			return -EPIPE;
+		}
+
+		vc = source_entry->bus.csi2.vc;
+		dt = source_entry->bus.csi2.dt;
+		/* Channels 4 - 11 go to VIN */
+		ch = route->source_pad - 1 + 4;
+
+		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
+						   route->sink_stream);
+		if (!fmt)
+			return -EINVAL;
+
+		format = risp_code_to_fmt(fmt->code);
+		if (!format) {
+			dev_err(isp->dev, "Unsupported bus format\n");
+			return -EINVAL;
+		}
+
+		/* VC Filtering */
+		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
+
+		/* DT Filtering */
+		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
+			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
+
+		/* Proc mode */
+		v = risp_read_cs(isp, ISPPROCMODE_DT_REG(dt));
+		v |= ISPPROCMODE_DT_PROC_MODE_VCn(vc, format->procmode);
+		risp_write_cs(isp, ISPPROCMODE_DT_REG(dt), v);
+	}
+
+	return 0;
+}
+
+static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
+{
+	u32 sel_csi = 0;
+	int ret;
+
 	ret = risp_power_on(isp);
 	if (ret) {
 		dev_err(isp->dev, "Failed to power on ISP\n");
@@ -256,25 +318,9 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
 	risp_write_cs(isp, ISPINPUTSEL0_REG,
 		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
 
-	/* Configure Channel Selector. */
-	for (vc = 0; vc < 4; vc++) {
-		u8 ch = vc + 4;
-		u8 dt = format->datatype;
-
-		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
-		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
-			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
-			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
-			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
-			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
-	}
-
-	/* Setup processing method. */
-	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
-		      ISPPROCMODE_DT_PROC_MODE_VCn(3, format->procmode) |
-		      ISPPROCMODE_DT_PROC_MODE_VCn(2, format->procmode) |
-		      ISPPROCMODE_DT_PROC_MODE_VCn(1, format->procmode) |
-		      ISPPROCMODE_DT_PROC_MODE_VCn(0, format->procmode));
+	ret = risp_configure_routing(isp, state);
+	if (ret)
+		return ret;
 
 	/* Start ISP. */
 	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);

-- 
2.43.0

Re: [PATCH v3 11/15] media: rcar-isp: Call get_frame_desc to find out VC & DT

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:40PM +0300, Tomi Valkeinen wrote:
> Call get_frame_desc to find out VC & DT, instead of hardcoding the VC
> routing and deducing the DT based on the mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 108 +++++++++++++++++-------
>  1 file changed, 77 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index a04cbf96b809..887d8eb21a3a 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -225,24 +225,86 @@ static void risp_power_off(struct rcar_isp *isp)
>  	pm_runtime_put(isp->dev);
>  }
>  
> -static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
> +static int risp_configure_routing(struct rcar_isp *isp,
> +				  struct v4l2_subdev_state *state)
>  {
> -	const struct v4l2_mbus_framefmt *fmt;
> -	const struct rcar_isp_format *format;
> -	unsigned int vc;
> -	u32 sel_csi = 0;
> +	struct v4l2_mbus_frame_desc source_fd;
> +	struct v4l2_subdev_route *route;
>  	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
> -	if (!fmt)
> -		return -EINVAL;
> +	ret = v4l2_subdev_call(isp->remote, pad, get_frame_desc,
> +			       isp->remote_pad, &source_fd);
> +	if (ret)
> +		return ret;
>  
> -	format = risp_code_to_fmt(fmt->code);
> -	if (!format) {
> -		dev_err(isp->dev, "Unsupported bus format\n");
> -		return -EINVAL;
> +	/* Clear the channel registers */
> +	for (unsigned int ch = 0; ch < 12; ++ch) {

A macro for the number of channels would be nice.

> +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), 0);
> +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch), 0);
>  	}
>  
> +	/* Clear the proc mode registers */
> +	for (unsigned int dt = 0; dt < 64; ++dt)
> +		risp_write_cs(isp, ISPPROCMODE_DT_REG(dt), 0);

Do we really need to clear those ? These registers seem to be used to
select how to process a particular DT, likely to allow overriding the
default processing method. 0 means RAW8, so it's not a magic disable
value as far as I can tell. I think we can leave the registers as-is.

> +
> +	for_each_active_route(&state->routing, route) {
> +		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
> +		const struct rcar_isp_format *format;
> +		const struct v4l2_mbus_framefmt *fmt;
> +		unsigned int i;
> +		u8 vc, dt, ch;
> +		u32 v;
> +
> +		for (i = 0; i < source_fd.num_entries; i++) {
> +			if (source_fd.entry[i].stream == route->sink_stream) {
> +				source_entry = &source_fd.entry[i];
> +				break;
> +			}
> +		}
> +
> +		if (!source_entry) {
> +			dev_err(isp->dev,
> +				"Failed to find stream from source frame desc\n");

Isn't it rather "Failed to find source frame desc for stream" ?

> +			return -EPIPE;
> +		}
> +
> +		vc = source_entry->bus.csi2.vc;
> +		dt = source_entry->bus.csi2.dt;
> +		/* Channels 4 - 11 go to VIN */
> +		ch = route->source_pad - 1 + 4;
> +
> +		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> +						   route->sink_stream);
> +		if (!fmt)
> +			return -EINVAL;
> +
> +		format = risp_code_to_fmt(fmt->code);
> +		if (!format) {
> +			dev_err(isp->dev, "Unsupported bus format\n");
> +			return -EINVAL;
> +		}
> +
> +		/* VC Filtering */
> +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> +
> +		/* DT Filtering */
> +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> +			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> +
> +		/* Proc mode */
> +		v = risp_read_cs(isp, ISPPROCMODE_DT_REG(dt));
> +		v |= ISPPROCMODE_DT_PROC_MODE_VCn(vc, format->procmode);
> +		risp_write_cs(isp, ISPPROCMODE_DT_REG(dt), v);

If we want to minimize the register writes, we could store the
ISPPROCMODE_DT_REG values in a local variable and write all of them in
one go. Possible/probably overkill.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +	}
> +
> +	return 0;
> +}
> +
> +static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
> +{
> +	u32 sel_csi = 0;
> +	int ret;
> +
>  	ret = risp_power_on(isp);
>  	if (ret) {
>  		dev_err(isp->dev, "Failed to power on ISP\n");
> @@ -256,25 +318,9 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	risp_write_cs(isp, ISPINPUTSEL0_REG,
>  		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
>  
> -	/* Configure Channel Selector. */
> -	for (vc = 0; vc < 4; vc++) {
> -		u8 ch = vc + 4;
> -		u8 dt = format->datatype;
> -
> -		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> -		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> -			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> -			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> -			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> -			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> -	}
> -
> -	/* Setup processing method. */
> -	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(3, format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(2, format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(1, format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(0, format->procmode));
> +	ret = risp_configure_routing(isp, state);
> +	if (ret)
> +		return ret;
>  
>  	/* Start ISP. */
>  	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 11/15] media: rcar-isp: Call get_frame_desc to find out VC & DT

[Niklas Söderlund]

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:40 +0300, Tomi Valkeinen wrote:
> Call get_frame_desc to find out VC & DT, instead of hardcoding the VC
> routing and deducing the DT based on the mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 108 +++++++++++++++++-------
>  1 file changed, 77 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index a04cbf96b809..887d8eb21a3a 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -225,24 +225,86 @@ static void risp_power_off(struct rcar_isp *isp)
>  	pm_runtime_put(isp->dev);
>  }
>  
> -static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
> +static int risp_configure_routing(struct rcar_isp *isp,
> +				  struct v4l2_subdev_state *state)
>  {
> -	const struct v4l2_mbus_framefmt *fmt;
> -	const struct rcar_isp_format *format;
> -	unsigned int vc;
> -	u32 sel_csi = 0;
> +	struct v4l2_mbus_frame_desc source_fd;
> +	struct v4l2_subdev_route *route;
>  	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
> -	if (!fmt)
> -		return -EINVAL;
> +	ret = v4l2_subdev_call(isp->remote, pad, get_frame_desc,
> +			       isp->remote_pad, &source_fd);
> +	if (ret)
> +		return ret;
>  
> -	format = risp_code_to_fmt(fmt->code);
> -	if (!format) {
> -		dev_err(isp->dev, "Unsupported bus format\n");
> -		return -EINVAL;
> +	/* Clear the channel registers */
> +	for (unsigned int ch = 0; ch < 12; ++ch) {
> +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), 0);
> +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch), 0);
>  	}
>  
> +	/* Clear the proc mode registers */
> +	for (unsigned int dt = 0; dt < 64; ++dt)
> +		risp_write_cs(isp, ISPPROCMODE_DT_REG(dt), 0);

I agree with Laurent's comments, do we really need to clear these 
registers?

> +
> +	for_each_active_route(&state->routing, route) {
> +		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
> +		const struct rcar_isp_format *format;
> +		const struct v4l2_mbus_framefmt *fmt;
> +		unsigned int i;
> +		u8 vc, dt, ch;
> +		u32 v;
> +
> +		for (i = 0; i < source_fd.num_entries; i++) {
> +			if (source_fd.entry[i].stream == route->sink_stream) {
> +				source_entry = &source_fd.entry[i];
> +				break;
> +			}
> +		}
> +
> +		if (!source_entry) {
> +			dev_err(isp->dev,
> +				"Failed to find stream from source frame desc\n");
> +			return -EPIPE;
> +		}
> +
> +		vc = source_entry->bus.csi2.vc;
> +		dt = source_entry->bus.csi2.dt;
> +		/* Channels 4 - 11 go to VIN */
> +		ch = route->source_pad - 1 + 4;
> +
> +		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> +						   route->sink_stream);
> +		if (!fmt)
> +			return -EINVAL;
> +
> +		format = risp_code_to_fmt(fmt->code);
> +		if (!format) {
> +			dev_err(isp->dev, "Unsupported bus format\n");
> +			return -EINVAL;
> +		}
> +
> +		/* VC Filtering */
> +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> +
> +		/* DT Filtering */
> +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> +			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> +
> +		/* Proc mode */
> +		v = risp_read_cs(isp, ISPPROCMODE_DT_REG(dt));
> +		v |= ISPPROCMODE_DT_PROC_MODE_VCn(vc, format->procmode);
> +		risp_write_cs(isp, ISPPROCMODE_DT_REG(dt), v);

Also as Laurent suggested I thin it would be nicer to build these 
registers up in a local variable and do the writes once outside the 
loop. That way the clearing of the register will take care of itself ;-)

> +	}
> +
> +	return 0;
> +}
> +
> +static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
> +{
> +	u32 sel_csi = 0;
> +	int ret;
> +
>  	ret = risp_power_on(isp);
>  	if (ret) {
>  		dev_err(isp->dev, "Failed to power on ISP\n");
> @@ -256,25 +318,9 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	risp_write_cs(isp, ISPINPUTSEL0_REG,
>  		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
>  
> -	/* Configure Channel Selector. */
> -	for (vc = 0; vc < 4; vc++) {
> -		u8 ch = vc + 4;
> -		u8 dt = format->datatype;
> -
> -		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> -		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> -			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> -			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> -			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> -			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> -	}
> -
> -	/* Setup processing method. */
> -	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(3, format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(2, format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(1, format->procmode) |
> -		      ISPPROCMODE_DT_PROC_MODE_VCn(0, format->procmode));
> +	ret = risp_configure_routing(isp, state);
> +	if (ret)
> +		return ret;
>  
>  	/* Start ISP. */
>  	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 12/15] media: rcar-csi2: Add more stream support to rcsi2_calc_mbps()

[Tomi Valkeinen]

In the case where link-freq is not available, make sure we fail if there
are more than one stream configured, and also use the correct stream
number for that single stream.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 65c7f3040696..b9f83aae725a 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1018,17 +1018,22 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
 	 */
 	freq = v4l2_get_link_freq(remote_pad, 0, 0);
 	if (freq < 0) {
+		struct v4l2_subdev_route *route = &state->routing.routes[0];
 		const struct rcar_csi2_format *format;
 		struct v4l2_mbus_framefmt *fmt;
 		unsigned int lanes;
 		unsigned int bpp;
 		int ret;
 
+		if (state->routing.num_routes > 1)
+			return -EINVAL;
+
 		ret = rcsi2_get_active_lanes(priv, &lanes);
 		if (ret)
 			return ret;
 
-		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
+		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
+						   route->sink_stream);
 		if (!fmt)
 			return -EINVAL;
 

-- 
2.43.0

Re: [PATCH v3 12/15] media: rcar-csi2: Add more stream support to rcsi2_calc_mbps()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:41PM +0300, Tomi Valkeinen wrote:
> In the case where link-freq is not available, make sure we fail if there
> are more than one stream configured, and also use the correct stream
> number for that single stream.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 65c7f3040696..b9f83aae725a 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1018,17 +1018,22 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  	 */
>  	freq = v4l2_get_link_freq(remote_pad, 0, 0);
>  	if (freq < 0) {
> +		struct v4l2_subdev_route *route = &state->routing.routes[0];

const

>  		const struct rcar_csi2_format *format;
>  		struct v4l2_mbus_framefmt *fmt;
>  		unsigned int lanes;
>  		unsigned int bpp;
>  		int ret;
>  
> +		if (state->routing.num_routes > 1)
> +			return -EINVAL;

Do we have to guard against the case where there would be no route ?

> +
>  		ret = rcsi2_get_active_lanes(priv, &lanes);
>  		if (ret)
>  			return ret;
>  
> -		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> +						   route->sink_stream);
>  		if (!fmt)
>  			return -EINVAL;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 12/15] media: rcar-csi2: Add more stream support to rcsi2_calc_mbps()

[Niklas Söderlund]

Hi Tomi,

Thanks for your patch.

On 2025-05-30 16:50:41 +0300, Tomi Valkeinen wrote:
> In the case where link-freq is not available, make sure we fail if there
> are more than one stream configured, and also use the correct stream
> number for that single stream.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

With Laurent's comments addressed,

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

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 65c7f3040696..b9f83aae725a 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1018,17 +1018,22 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  	 */
>  	freq = v4l2_get_link_freq(remote_pad, 0, 0);
>  	if (freq < 0) {
> +		struct v4l2_subdev_route *route = &state->routing.routes[0];
>  		const struct rcar_csi2_format *format;
>  		struct v4l2_mbus_framefmt *fmt;
>  		unsigned int lanes;
>  		unsigned int bpp;
>  		int ret;
>  
> +		if (state->routing.num_routes > 1)
> +			return -EINVAL;
> +
>  		ret = rcsi2_get_active_lanes(priv, &lanes);
>  		if (ret)
>  			return ret;
>  
> -		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> +						   route->sink_stream);
>  		if (!fmt)
>  			return -EINVAL;
>  
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

[PATCH v3 13/15] media: rcar-csi2: Call get_frame_desc to find out VC & DT (Gen3)

[Tomi Valkeinen]

Call get_frame_desc to find out VC & DT, for Gen3 platforms, instead of
hardcoding the VC routing and deducing the DT based on the mbus format.

If the source subdevice doesn't implement .get_frame_desc, we use a
fallback case where we assume there's a single stream with VC = 0 and DT
based on the mbus format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 113 +++++++++++++++++++----------
 1 file changed, 76 insertions(+), 37 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index b9f83aae725a..8f708196ef49 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -71,10 +71,7 @@ struct rcar_csi2;
 #define FLD_REG				0x1c
 #define FLD_FLD_NUM(n)			(((n) & 0xff) << 16)
 #define FLD_DET_SEL(n)			(((n) & 0x3) << 4)
-#define FLD_FLD_EN4			BIT(3)
-#define FLD_FLD_EN3			BIT(2)
-#define FLD_FLD_EN2			BIT(1)
-#define FLD_FLD_EN			BIT(0)
+#define FLD_FLD_EN(ch)			BIT(ch)
 
 /* Automatic Standby Control */
 #define ASTBY_REG			0x20
@@ -1066,52 +1063,94 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
 static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
 				     struct v4l2_subdev_state *state)
 {
-	const struct rcar_csi2_format *format;
-	u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
-	const struct v4l2_mbus_framefmt *fmt;
+	u32 phycnt, vcdt = 0, vcdt2 = 0;
+	u32 fld = FLD_DET_SEL(1);
+	struct v4l2_mbus_frame_desc source_fd;
+	struct v4l2_subdev_route *route;
 	unsigned int lanes;
-	unsigned int i;
 	int mbps, ret;
+	u8 ch = 0;
 
-	/* Use the format on the sink pad to compute the receiver config. */
-	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
+	ret = v4l2_subdev_call(priv->remote, pad, get_frame_desc,
+			       priv->remote_pad, &source_fd);
+	if (ret && ret != -ENOIOCTLCMD) {
+		return ret;
+	} else if (ret == -ENOIOCTLCMD) {
+		/* Create a fallback source_fd */
+		struct v4l2_mbus_frame_desc *fd = &source_fd;
+		const struct rcar_csi2_format *format;
+		struct v4l2_mbus_framefmt *fmt;
 
-	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
-		fmt->width, fmt->height,
-		fmt->field == V4L2_FIELD_NONE ? 'p' : 'i');
+		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
+		if (!fmt)
+			return -EINVAL;
 
-	/* Code is validated in set_fmt. */
-	format = rcsi2_code_to_fmt(fmt->code);
-	if (!format)
-		return -EINVAL;
+		format = rcsi2_code_to_fmt(fmt->code);
+		if (!format)
+			return -EINVAL;
 
-	/*
-	 * Enable all supported CSI-2 channels with virtual channel and
-	 * data type matching.
-	 *
-	 * NOTE: It's not possible to get individual datatype for each
-	 *       source virtual channel. Once this is possible in V4L2
-	 *       it should be used here.
-	 */
-	for (i = 0; i < priv->info->num_channels; i++) {
+		memset(fd, 0, sizeof(*fd));
+
+		fd->num_entries = 1;
+		fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+		fd->entry[0].stream = 0;
+		fd->entry[0].pixelcode = fmt->code;
+		fd->entry[0].bus.csi2.vc = 0;
+		fd->entry[0].bus.csi2.dt = format->datatype;
+	}
+
+	for_each_active_route(&state->routing, route) {
+		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
+		const struct v4l2_mbus_framefmt *fmt;
+		const struct rcar_csi2_format *format;
+		unsigned int i;
+		u8 vc, dt;
 		u32 vcdt_part;
 
-		if (priv->channel_vc[i] < 0)
-			continue;
+		for (i = 0; i < source_fd.num_entries; i++) {
+			if (source_fd.entry[i].stream == route->sink_stream) {
+				source_entry = &source_fd.entry[i];
+				break;
+			}
+		}
+
+		if (!source_entry) {
+			dev_err(priv->dev,
+				"Failed to find stream from source frame desc\n");
+			return -EPIPE;
+		}
 
-		vcdt_part = VCDT_SEL_VC(priv->channel_vc[i]) | VCDT_VCDTN_EN |
-			VCDT_SEL_DTN_ON | VCDT_SEL_DT(format->datatype);
+		vc = source_entry->bus.csi2.vc;
+		dt = source_entry->bus.csi2.dt;
+
+		vcdt_part = VCDT_SEL_VC(vc) | VCDT_VCDTN_EN |
+			VCDT_SEL_DTN_ON | VCDT_SEL_DT(dt);
 
 		/* Store in correct reg and offset. */
-		if (i < 2)
-			vcdt |= vcdt_part << ((i % 2) * 16);
+		if (ch < 2)
+			vcdt |= vcdt_part << ((ch % 2) * 16);
 		else
-			vcdt2 |= vcdt_part << ((i % 2) * 16);
-	}
+			vcdt2 |= vcdt_part << ((ch % 2) * 16);
+
+		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK,
+						   route->sink_stream);
+		if (!fmt)
+			return -EINVAL;
+
+		dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
+			fmt->width, fmt->height,
+			fmt->field == V4L2_FIELD_NONE ? 'p' : 'i');
 
-	if (fmt->field == V4L2_FIELD_ALTERNATE)
-		fld = FLD_DET_SEL(1) | FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2
-			| FLD_FLD_EN;
+		/* Code is validated in set_fmt. */
+		format = rcsi2_code_to_fmt(fmt->code);
+		if (!format)
+			return -EINVAL;
+
+		if (fmt->field == V4L2_FIELD_ALTERNATE)
+			fld |= FLD_FLD_EN(ch);
+
+		ch++;
+	}
 
 	/*
 	 * Get the number of active data lanes inspecting the remote mbus

-- 
2.43.0

Re: [PATCH v3 13/15] media: rcar-csi2: Call get_frame_desc to find out VC & DT (Gen3)

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:42PM +0300, Tomi Valkeinen wrote:
> Call get_frame_desc to find out VC & DT, for Gen3 platforms, instead of
> hardcoding the VC routing and deducing the DT based on the mbus format.
> 
> If the source subdevice doesn't implement .get_frame_desc, we use a
> fallback case where we assume there's a single stream with VC = 0 and DT
> based on the mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 113 +++++++++++++++++++----------
>  1 file changed, 76 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index b9f83aae725a..8f708196ef49 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -71,10 +71,7 @@ struct rcar_csi2;
>  #define FLD_REG				0x1c
>  #define FLD_FLD_NUM(n)			(((n) & 0xff) << 16)
>  #define FLD_DET_SEL(n)			(((n) & 0x3) << 4)
> -#define FLD_FLD_EN4			BIT(3)
> -#define FLD_FLD_EN3			BIT(2)
> -#define FLD_FLD_EN2			BIT(1)
> -#define FLD_FLD_EN			BIT(0)
> +#define FLD_FLD_EN(ch)			BIT(ch)
>  
>  /* Automatic Standby Control */
>  #define ASTBY_REG			0x20
> @@ -1066,52 +1063,94 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
>  static int rcsi2_start_receiver_gen3(struct rcar_csi2 *priv,
>  				     struct v4l2_subdev_state *state)
>  {
> -	const struct rcar_csi2_format *format;
> -	u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
> -	const struct v4l2_mbus_framefmt *fmt;
> +	u32 phycnt, vcdt = 0, vcdt2 = 0;
> +	u32 fld = FLD_DET_SEL(1);
> +	struct v4l2_mbus_frame_desc source_fd;
> +	struct v4l2_subdev_route *route;
>  	unsigned int lanes;
> -	unsigned int i;
>  	int mbps, ret;
> +	u8 ch = 0;
>  
> -	/* Use the format on the sink pad to compute the receiver config. */
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +	ret = v4l2_subdev_call(priv->remote, pad, get_frame_desc,
> +			       priv->remote_pad, &source_fd);
> +	if (ret && ret != -ENOIOCTLCMD) {
> +		return ret;
> +	} else if (ret == -ENOIOCTLCMD) {

	if (ret && ret != -ENOIOCTLCMD)
		return ret;

	if (ret == -ENOIOCTLCMD) {

> +		/* Create a fallback source_fd */
> +		struct v4l2_mbus_frame_desc *fd = &source_fd;
> +		const struct rcar_csi2_format *format;
> +		struct v4l2_mbus_framefmt *fmt;
>  
> -	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
> -		fmt->width, fmt->height,
> -		fmt->field == V4L2_FIELD_NONE ? 'p' : 'i');
> +		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +		if (!fmt)
> +			return -EINVAL;
>  
> -	/* Code is validated in set_fmt. */
> -	format = rcsi2_code_to_fmt(fmt->code);
> -	if (!format)
> -		return -EINVAL;
> +		format = rcsi2_code_to_fmt(fmt->code);
> +		if (!format)
> +			return -EINVAL;
>  
> -	/*
> -	 * Enable all supported CSI-2 channels with virtual channel and
> -	 * data type matching.
> -	 *
> -	 * NOTE: It's not possible to get individual datatype for each
> -	 *       source virtual channel. Once this is possible in V4L2
> -	 *       it should be used here.
> -	 */
> -	for (i = 0; i < priv->info->num_channels; i++) {
> +		memset(fd, 0, sizeof(*fd));
> +
> +		fd->num_entries = 1;
> +		fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +		fd->entry[0].stream = 0;
> +		fd->entry[0].pixelcode = fmt->code;
> +		fd->entry[0].bus.csi2.vc = 0;
> +		fd->entry[0].bus.csi2.dt = format->datatype;
> +	}
> +
> +	for_each_active_route(&state->routing, route) {
> +		struct v4l2_mbus_frame_desc_entry *source_entry = NULL;

const

> +		const struct v4l2_mbus_framefmt *fmt;
> +		const struct rcar_csi2_format *format;
> +		unsigned int i;
> +		u8 vc, dt;
>  		u32 vcdt_part;
>  
> -		if (priv->channel_vc[i] < 0)
> -			continue;
> +		for (i = 0; i < source_fd.num_entries; i++) {
> +			if (source_fd.entry[i].stream == route->sink_stream) {

No need to check the pad ?

> +				source_entry = &source_fd.entry[i];
> +				break;
> +			}
> +		}
> +
> +		if (!source_entry) {
> +			dev_err(priv->dev,
> +				"Failed to find stream from source frame desc\n");
> +			return -EPIPE;
> +		}
>  
> -		vcdt_part = VCDT_SEL_VC(priv->channel_vc[i]) | VCDT_VCDTN_EN |
> -			VCDT_SEL_DTN_ON | VCDT_SEL_DT(format->datatype);
> +		vc = source_entry->bus.csi2.vc;
> +		dt = source_entry->bus.csi2.dt;
> +
> +		vcdt_part = VCDT_SEL_VC(vc) | VCDT_VCDTN_EN |
> +			VCDT_SEL_DTN_ON | VCDT_SEL_DT(dt);

I would drop the vc and dt variables and write

		vcdt_part = VCDT_SEL_VC(source_entry->bus.csi2.vc)
			  | VCDT_VCDTN_EN | VCDT_SEL_DTN_ON
			  | VCDT_SEL_DT(source_entry->bus.csi2.dt);

>  
>  		/* Store in correct reg and offset. */
> -		if (i < 2)
> -			vcdt |= vcdt_part << ((i % 2) * 16);
> +		if (ch < 2)
> +			vcdt |= vcdt_part << ((ch % 2) * 16);
>  		else
> -			vcdt2 |= vcdt_part << ((i % 2) * 16);
> -	}
> +			vcdt2 |= vcdt_part << ((ch % 2) * 16);
> +
> +		fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK,
> +						   route->sink_stream);
> +		if (!fmt)
> +			return -EINVAL;
> +
> +		dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
> +			fmt->width, fmt->height,
> +			fmt->field == V4L2_FIELD_NONE ? 'p' : 'i');
>  
> -	if (fmt->field == V4L2_FIELD_ALTERNATE)
> -		fld = FLD_DET_SEL(1) | FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2
> -			| FLD_FLD_EN;
> +		/* Code is validated in set_fmt. */

Then why don't you drop the !format check ?

> +		format = rcsi2_code_to_fmt(fmt->code);
> +		if (!format)
> +			return -EINVAL;
> +
> +		if (fmt->field == V4L2_FIELD_ALTERNATE)
> +			fld |= FLD_FLD_EN(ch);
> +
> +		ch++;
> +	}
>  
>  	/*
>  	 * Get the number of active data lanes inspecting the remote mbus

-- 
Regards,

Laurent Pinchart

[PATCH v3 14/15] media: rcar-csi2: Add full streams support

[Tomi Valkeinen]

Add the missing pieces to enable full streams support:

- Add set_routing
- Drop the explicit uses of a single stream, and instead use the streams
  mask.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 85 ++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 8f708196ef49..4a73d223229c 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -694,6 +694,17 @@ static const struct rcar_csi2_format rcar_csi2_formats[] = {
 	},
 };
 
+static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
+	.width		= 1920,
+	.height		= 1080,
+	.code		= MEDIA_BUS_FMT_RGB888_1X24,
+	.colorspace	= V4L2_COLORSPACE_SRGB,
+	.field		= V4L2_FIELD_NONE,
+	.ycbcr_enc	= V4L2_YCBCR_ENC_DEFAULT,
+	.quantization	= V4L2_QUANTIZATION_DEFAULT,
+	.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
+};
+
 static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
 {
 	unsigned int i;
@@ -1641,10 +1652,8 @@ static int rcsi2_enable_streams(struct v4l2_subdev *sd,
 				u64 source_streams_mask)
 {
 	struct rcar_csi2 *priv = sd_to_csi2(sd);
-	int ret = 0;
-
-	if (source_streams_mask != 1)
-		return -EINVAL;
+	u64 sink_streams;
+	int ret;
 
 	if (!priv->remote)
 		return -ENODEV;
@@ -1655,8 +1664,13 @@ static int rcsi2_enable_streams(struct v4l2_subdev *sd,
 			return ret;
 	}
 
+	sink_streams = v4l2_subdev_state_xlate_streams(state,
+						       RCAR_CSI2_SOURCE_VC0,
+						       RCAR_CSI2_SINK,
+						       &source_streams_mask);
+
 	ret = v4l2_subdev_enable_streams(priv->remote, priv->remote_pad,
-					 BIT_ULL(0));
+					 sink_streams);
 	if (ret) {
 		rcsi2_stop(priv);
 		return ret;
@@ -1672,10 +1686,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;
-
-	if (source_streams_mask != 1)
-		return -EINVAL;
+	u64 sink_streams;
 
 	if (!priv->remote)
 		return -ENODEV;
@@ -1683,11 +1694,17 @@ static int rcsi2_disable_streams(struct v4l2_subdev *sd,
 	if (priv->stream_count == 1)
 		rcsi2_stop(priv);
 
-	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad, BIT_ULL(0));
+	sink_streams = v4l2_subdev_state_xlate_streams(state,
+						       RCAR_CSI2_SOURCE_VC0,
+						       RCAR_CSI2_SINK,
+						       &source_streams_mask);
+
+	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad,
+				    sink_streams);
 
 	priv->stream_count -= 1;
 
-	return ret;
+	return 0;
 }
 
 static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
@@ -1720,6 +1737,40 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int rcsi2_set_routing(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_state *state,
+			     enum v4l2_subdev_format_whence which,
+			     struct v4l2_subdev_krouting *routing)
+{
+	struct rcar_csi2 *priv = sd_to_csi2(sd);
+	int ret;
+
+	if (!priv->info->use_isp)
+		return -ENOTTY;
+
+	if (routing->num_routes > V4L2_FRAME_DESC_ENTRY_MAX)
+		return -EINVAL;
+
+	if (priv->info->use_isp) {
+		ret = v4l2_subdev_routing_validate(sd, routing,
+						   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
+	} else {
+		ret = v4l2_subdev_routing_validate(sd, routing,
+						   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
+						   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
+	}
+
+	if (ret)
+		return ret;
+
+	ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing,
+					       &rcar_csi2_default_fmt);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
 					 unsigned int pad,
 					 struct v4l2_mbus_frame_desc *fd)
@@ -1781,6 +1832,7 @@ static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
 	.set_fmt = rcsi2_set_pad_format,
 	.get_fmt = v4l2_subdev_get_fmt,
 
+	.set_routing = rcsi2_set_routing,
 	.get_frame_desc = rcsi2_get_frame_desc,
 };
 
@@ -1801,17 +1853,6 @@ static int rcsi2_init_state(struct v4l2_subdev *sd,
 		},
 	};
 
-	static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
-		.width		= 1920,
-		.height		= 1080,
-		.code		= MEDIA_BUS_FMT_RGB888_1X24,
-		.colorspace	= V4L2_COLORSPACE_SRGB,
-		.field		= V4L2_FIELD_NONE,
-		.ycbcr_enc	= V4L2_YCBCR_ENC_DEFAULT,
-		.quantization	= V4L2_QUANTIZATION_DEFAULT,
-		.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
-	};
-
 	static const struct v4l2_subdev_krouting routing = {
 		.num_routes = ARRAY_SIZE(routes),
 		.routes = routes,

-- 
2.43.0

Re: [PATCH v3 14/15] media: rcar-csi2: Add full streams support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:43PM +0300, Tomi Valkeinen wrote:
> Add the missing pieces to enable full streams support:
> 
> - Add set_routing
> - Drop the explicit uses of a single stream, and instead use the streams
>   mask.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 85 ++++++++++++++++++++++--------
>  1 file changed, 63 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 8f708196ef49..4a73d223229c 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -694,6 +694,17 @@ static const struct rcar_csi2_format rcar_csi2_formats[] = {
>  	},
>  };
>  
> +static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
> +	.width		= 1920,
> +	.height		= 1080,
> +	.code		= MEDIA_BUS_FMT_RGB888_1X24,
> +	.colorspace	= V4L2_COLORSPACE_SRGB,
> +	.field		= V4L2_FIELD_NONE,
> +	.ycbcr_enc	= V4L2_YCBCR_ENC_DEFAULT,
> +	.quantization	= V4L2_QUANTIZATION_DEFAULT,
> +	.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
> +};
> +
>  static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
>  {
>  	unsigned int i;
> @@ -1641,10 +1652,8 @@ static int rcsi2_enable_streams(struct v4l2_subdev *sd,
>  				u64 source_streams_mask)
>  {
>  	struct rcar_csi2 *priv = sd_to_csi2(sd);
> -	int ret = 0;
> -
> -	if (source_streams_mask != 1)
> -		return -EINVAL;
> +	u64 sink_streams;
> +	int ret;
>  
>  	if (!priv->remote)
>  		return -ENODEV;
> @@ -1655,8 +1664,13 @@ static int rcsi2_enable_streams(struct v4l2_subdev *sd,
>  			return ret;
>  	}
>  
> +	sink_streams = v4l2_subdev_state_xlate_streams(state,
> +						       RCAR_CSI2_SOURCE_VC0,
> +						       RCAR_CSI2_SINK,
> +						       &source_streams_mask);
> +
>  	ret = v4l2_subdev_enable_streams(priv->remote, priv->remote_pad,
> -					 BIT_ULL(0));
> +					 sink_streams);
>  	if (ret) {
>  		rcsi2_stop(priv);
>  		return ret;
> @@ -1672,10 +1686,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;
> -
> -	if (source_streams_mask != 1)
> -		return -EINVAL;
> +	u64 sink_streams;
>  
>  	if (!priv->remote)
>  		return -ENODEV;
> @@ -1683,11 +1694,17 @@ static int rcsi2_disable_streams(struct v4l2_subdev *sd,
>  	if (priv->stream_count == 1)
>  		rcsi2_stop(priv);
>  
> -	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad, BIT_ULL(0));
> +	sink_streams = v4l2_subdev_state_xlate_streams(state,
> +						       RCAR_CSI2_SOURCE_VC0,
> +						       RCAR_CSI2_SINK,
> +						       &source_streams_mask);
> +
> +	v4l2_subdev_disable_streams(priv->remote, priv->remote_pad,
> +				    sink_streams);
>  
>  	priv->stream_count -= 1;
>  
> -	return ret;
> +	return 0;

This seems to belong to a previous patch.

>  }
>  
>  static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
> @@ -1720,6 +1737,40 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int rcsi2_set_routing(struct v4l2_subdev *sd,
> +			     struct v4l2_subdev_state *state,
> +			     enum v4l2_subdev_format_whence which,
> +			     struct v4l2_subdev_krouting *routing)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	int ret;
> +
> +	if (!priv->info->use_isp)
> +		return -ENOTTY;
> +
> +	if (routing->num_routes > V4L2_FRAME_DESC_ENTRY_MAX)
> +		return -EINVAL;

A comment to explain this check would be nice.

> +
> +	if (priv->info->use_isp) {

You return an error above when this condition is false.

> +		ret = v4l2_subdev_routing_validate(sd, routing,
> +						   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
> +	} else {
> +		ret = v4l2_subdev_routing_validate(sd, routing,
> +						   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
> +						   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing,
> +					       &rcar_csi2_default_fmt);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
>  					 unsigned int pad,
>  					 struct v4l2_mbus_frame_desc *fd)
> @@ -1781,6 +1832,7 @@ static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>  	.set_fmt = rcsi2_set_pad_format,
>  	.get_fmt = v4l2_subdev_get_fmt,
>  
> +	.set_routing = rcsi2_set_routing,
>  	.get_frame_desc = rcsi2_get_frame_desc,
>  };
>  
> @@ -1801,17 +1853,6 @@ static int rcsi2_init_state(struct v4l2_subdev *sd,
>  		},
>  	};
>  
> -	static const struct v4l2_mbus_framefmt rcar_csi2_default_fmt = {
> -		.width		= 1920,
> -		.height		= 1080,
> -		.code		= MEDIA_BUS_FMT_RGB888_1X24,
> -		.colorspace	= V4L2_COLORSPACE_SRGB,
> -		.field		= V4L2_FIELD_NONE,
> -		.ycbcr_enc	= V4L2_YCBCR_ENC_DEFAULT,
> -		.quantization	= V4L2_QUANTIZATION_DEFAULT,
> -		.xfer_func	= V4L2_XFER_FUNC_DEFAULT,
> -	};
> -
>  	static const struct v4l2_subdev_krouting routing = {
>  		.num_routes = ARRAY_SIZE(routes),
>  		.routes = routes,

-- 
Regards,

Laurent Pinchart

[PATCH v3 15/15] media: rcar-isp: Add full streams support

[Tomi Valkeinen]

Add the missing pieces to enable full streams support:

- Add set_routing
- Drop the explicit uses of a single stream, and instead use the streams
  mask.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 41 +++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 887d8eb21a3a..101d69a2eba4 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -342,6 +342,7 @@ static int risp_enable_streams(struct v4l2_subdev *sd,
 {
 	struct rcar_isp *isp = sd_to_isp(sd);
 	int ret = 0;
+	u64 sink_streams;
 
 	if (source_streams_mask != 1)
 		return -EINVAL;
@@ -355,8 +356,13 @@ static int risp_enable_streams(struct v4l2_subdev *sd,
 			return ret;
 	}
 
+	sink_streams = v4l2_subdev_state_xlate_streams(state,
+						       source_pad,
+						       RCAR_ISP_SINK,
+						       &source_streams_mask);
+
 	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
-					 BIT_ULL(0));
+					 sink_streams);
 	if (ret) {
 		risp_stop(isp);
 		return ret;
@@ -372,6 +378,7 @@ static int risp_disable_streams(struct v4l2_subdev *sd,
 				u64 source_streams_mask)
 {
 	struct rcar_isp *isp = sd_to_isp(sd);
+	u64 sink_streams;
 
 	if (source_streams_mask != 1)
 		return -EINVAL;
@@ -379,7 +386,12 @@ static int risp_disable_streams(struct v4l2_subdev *sd,
 	if (!isp->remote)
 		return -ENODEV;
 
-	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
+	sink_streams = v4l2_subdev_state_xlate_streams(state,
+						       source_pad,
+						       RCAR_ISP_SINK,
+						       &source_streams_mask);
+
+	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, sink_streams);
 
 	if (isp->stream_count == 1)
 		risp_stop(isp);
@@ -419,12 +431,37 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int risp_set_routing(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *state,
+			    enum v4l2_subdev_format_whence which,
+			    struct v4l2_subdev_krouting *routing)
+{
+	int ret;
+
+	if (routing->num_routes > V4L2_FRAME_DESC_ENTRY_MAX)
+		return -EINVAL;
+
+	ret = v4l2_subdev_routing_validate(sd, routing,
+					   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
+					   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
+	if (ret)
+		return ret;
+
+	ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing,
+					       &risp_default_fmt);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static const struct v4l2_subdev_pad_ops risp_pad_ops = {
 	.enable_streams = risp_enable_streams,
 	.disable_streams = risp_disable_streams,
 	.set_fmt = risp_set_pad_format,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.link_validate = v4l2_subdev_link_validate_default,
+	.set_routing = risp_set_routing,
 };
 
 static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {

-- 
2.43.0

Re: [PATCH v3 15/15] media: rcar-isp: Add full streams support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:44PM +0300, Tomi Valkeinen wrote:
> Add the missing pieces to enable full streams support:
> 
> - Add set_routing
> - Drop the explicit uses of a single stream, and instead use the streams
>   mask.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 41 +++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 887d8eb21a3a..101d69a2eba4 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -342,6 +342,7 @@ static int risp_enable_streams(struct v4l2_subdev *sd,
>  {
>  	struct rcar_isp *isp = sd_to_isp(sd);
>  	int ret = 0;
> +	u64 sink_streams;

I'd move this before ret.

>  
>  	if (source_streams_mask != 1)
>  		return -EINVAL;
> @@ -355,8 +356,13 @@ static int risp_enable_streams(struct v4l2_subdev *sd,
>  			return ret;
>  	}
>  
> +	sink_streams = v4l2_subdev_state_xlate_streams(state,
> +						       source_pad,
> +						       RCAR_ISP_SINK,
> +						       &source_streams_mask);
> +
>  	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
> -					 BIT_ULL(0));
> +					 sink_streams);
>  	if (ret) {
>  		risp_stop(isp);
>  		return ret;
> @@ -372,6 +378,7 @@ static int risp_disable_streams(struct v4l2_subdev *sd,
>  				u64 source_streams_mask)
>  {
>  	struct rcar_isp *isp = sd_to_isp(sd);
> +	u64 sink_streams;
>  
>  	if (source_streams_mask != 1)
>  		return -EINVAL;
> @@ -379,7 +386,12 @@ static int risp_disable_streams(struct v4l2_subdev *sd,
>  	if (!isp->remote)
>  		return -ENODEV;
>  
> -	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
> +	sink_streams = v4l2_subdev_state_xlate_streams(state,
> +						       source_pad,
> +						       RCAR_ISP_SINK,
> +						       &source_streams_mask);
> +
> +	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, sink_streams);
>  
>  	if (isp->stream_count == 1)
>  		risp_stop(isp);
> @@ -419,12 +431,37 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int risp_set_routing(struct v4l2_subdev *sd,
> +			    struct v4l2_subdev_state *state,
> +			    enum v4l2_subdev_format_whence which,
> +			    struct v4l2_subdev_krouting *routing)
> +{
> +	int ret;
> +
> +	if (routing->num_routes > V4L2_FRAME_DESC_ENTRY_MAX)
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_routing_validate(sd, routing,
> +					   V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
> +					   V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);

Given V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING, shouldn't the
num_routes check be replaced by an intrinsic hardware limit (I would
guess 8, as that's the number of source pads) ?

> +	if (ret)
> +		return ret;
> +
> +	ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing,
> +					       &risp_default_fmt);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static const struct v4l2_subdev_pad_ops risp_pad_ops = {
>  	.enable_streams = risp_enable_streams,
>  	.disable_streams = risp_disable_streams,
>  	.set_fmt = risp_set_pad_format,
>  	.get_fmt = v4l2_subdev_get_fmt,
>  	.link_validate = v4l2_subdev_link_validate_default,
> +	.set_routing = risp_set_routing,
>  };
>  
>  static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 00/15] media: rcar: Streams support

[Niklas Söderlund]

Hi Tomi,

On 2025-05-30 16:50:29 +0300, Tomi Valkeinen wrote:
> Add streams support to Renesas rcar platform driver.
> 
> The series attempts to keep compatibility with the current upstream.
> However, in upstream there's some kind of custom multi-stream support
> implemented to the rcar driver, which breaks at patch "media: rcar-csi2:
> Simplify rcsi2_calc_mbps()".
> 
> The behavior should not change when using a single stream.
> 
> Testing is problematic, as the only way currently for me to get multiple
> streams is by using the GMSL2 deserializer add-on board with GMSL2
> serializers. These are not supported in upstream. If someone has the
> hardware and wants to test, I can share the very-WIP branch that
> contains the missing pieces.

I'm happy to see this new version of this work, it looks so clean! I'm 
equally happy to see all the hard-coded assumptions we needed in the 
pipeline to emulate streams before being replaced with core 
functionality!

I have tested this with the single stream use-cases I have had before on 
Gen2, Gen3 and Gen4 and they all seem to function as before, nice work!

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

As this is a rather large series do you think it would make sens to try 
and get some of the preparation/clean up patches merged before the new 
streams support?

> 
>  Tomi
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
> Changes in v3:
> - Rebased on top of latest linux-media
> - Dropped dependencies which are already in linux-media (only remaining
>   dependency is v4l2_subdev_get_frame_desc_passthrough)
> - Tested on white-hawk board, using the staging deser TPG
> - Also tested in a WIP branch for GMSL2 (two video streams)
> - Link to v2: https://lore.kernel.org/r/20250326-rcar-streams-v2-0-d0d7002c641f@ideasonboard.com
> 
> Changes in v2:
> - Rebased on top of latest upstream, and updated the dependencies to
>   match the latest serieses sent.
> - Add new patch "media: rcar-csi2: Use the pad version of v4l2_get_link_freq()"
> - Drop "media: rcar-csi2: Fix typo" (it was not a typo)
> - Update the code in calc_mbps(). The previous method relied on
>   V4L2_CID_LINK_FREQ, but that's not available if the link-freq is
>   provided via get_mbus_config().
> - Dropped dependencies to Niklas' old series which doesn't apply
>   cleanly. It's needed for multi-stream, but not for the current
>   upstream which only has a single stream use case.
> - Link to v1: https://lore.kernel.org/r/20250219-rcar-streams-v1-0-f1b93e370aab@ideasonboard.com
> 
> ---
> Tomi Valkeinen (15):
>       media: rcar-csi2: Use the pad version of v4l2_get_link_freq()
>       media: rcar-isp: Improve ISPPROCMODE_DT_PROC_MODE_VC
>       media: rcar-isp: Move {enable|disable}_streams() calls
>       media: rcar-csi2: Move {enable|disable}_streams() calls
>       media: rcar-csi2: Move rcar2_calc_mbps()
>       media: rcar-csi2: Simplify rcsi2_calc_mbps()
>       media: rcar-csi2: Optimize rcsi2_calc_mbps()
>       media: rcar-csi2: Switch to Streams API
>       media: rcar-isp: Switch to Streams API
>       media: rcar-csi2: Add .get_frame_desc op
>       media: rcar-isp: Call get_frame_desc to find out VC & DT
>       media: rcar-csi2: Add more stream support to rcsi2_calc_mbps()
>       media: rcar-csi2: Call get_frame_desc to find out VC & DT (Gen3)
>       media: rcar-csi2: Add full streams support
>       media: rcar-isp: Add full streams support
> 
>  drivers/media/platform/renesas/rcar-csi2.c      | 426 +++++++++++++++++-------
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 228 ++++++++++---
>  2 files changed, 479 insertions(+), 175 deletions(-)
> ---
> base-commit: 5e1ff2314797bf53636468a97719a8222deca9ae
> change-id: 20250219-rcar-streams-1fdea8860e5e
> prerequisite-change-id: 20250218-frame-desc-passthrough-66805e413974:v4
> prerequisite-patch-id: bce4a915a29a64f88ed1bb600c08df37d2ba20c6
> prerequisite-patch-id: 69b75e7dad9ced905cb39a72f18bebbf3e8f998a
> prerequisite-patch-id: 58463f6944c76acd6cf203b14a2836cdb0db2461
> 
> Best regards,
> -- 
> Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 

-- 
Kind Regards,
Niklas Söderlund