[PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[Tommaso Merciai]

When the CRU is configured to use ICnSVC for virtual channel mapping,
as on the RZ/{G3E, V2H/P} SoC, the ICnMC register must not be
programmed.

Return early after setting up ICnSVC to avoid overriding the ICnMC
register, which is not applicable in this mode.

This prevents unintended register programming when ICnSVC is enabled.

Fixes: 3c5ca0a48bb0 ("media: rzg2l-cru: Drop function pointer to configure CSI")
Cc: stable@vger.kernel.org
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 162e2ace6931..480e9b5dbcfe 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -268,6 +268,8 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
 		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
 		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
 				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
+
+		return;
 	}
 
 	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
-- 
2.43.0

Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[Lad, Prabhakar]

On Tue, Dec 30, 2025 at 5:16 PM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
>
> When the CRU is configured to use ICnSVC for virtual channel mapping,
> as on the RZ/{G3E, V2H/P} SoC, the ICnMC register must not be
> programmed.
>
> Return early after setting up ICnSVC to avoid overriding the ICnMC
> register, which is not applicable in this mode.
>
> This prevents unintended register programming when ICnSVC is enabled.
>
> Fixes: 3c5ca0a48bb0 ("media: rzg2l-cru: Drop function pointer to configure CSI")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 162e2ace6931..480e9b5dbcfe 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -268,6 +268,8 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
>                 rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
>                 rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
>                                 ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> +
> +               return;
>         }
>
>         icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
> --
> 2.43.0
>
>

Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[Jacopo Mondi]

Hi Tommaso

On Tue, Dec 30, 2025 at 06:09:15PM +0100, Tommaso Merciai wrote:
> When the CRU is configured to use ICnSVC for virtual channel mapping,
> as on the RZ/{G3E, V2H/P} SoC, the ICnMC register must not be
> programmed.

I see a difference indeed between the [G3E, V2H/P] and the G2L version
of the IP in the presence of the VCSEL[1:0] field in register ICnMC.

On [G3E, V2H/P] the selection of which virtual channel to accept
doesn't go through VCSEL[1:0] in ICnMC but a dedicated register ICnSVC
is provided for that purpose. So far so good.

>
> Return early after setting up ICnSVC to avoid overriding the ICnMC
> register, which is not applicable in this mode.
>
> This prevents unintended register programming when ICnSVC is enabled.
>
> Fixes: 3c5ca0a48bb0 ("media: rzg2l-cru: Drop function pointer to configure CSI")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 162e2ace6931..480e9b5dbcfe 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -268,6 +268,8 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
>  		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
>  		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
>  				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> +
> +		return;
>  	}
>
>  	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;

However, the rest of the rzg2l_cru_csi2_setup() doesn't only program
the virtual channel filtering through ICnMC (which should be skept for
G3E, V2H/P) but also programs DT filtering in INF[5:0] of register
ICnIPMC_C0 which seems to be available on G3E and V2H/P as well.

Section "9.2.4.3.6 VC/Data Type Selector (for Image Processing)" of
the chip manual (V2H/P) prescribes:

Only one data type can be handled by each SVC and the data types are
selected in the ICnIPMC_C0 to 3.INF[5:0] registers.

And this patch makes the driver skips that part.
Has this patch been tested ? It breaks my V2H/P setup:

 rzg2l-cru 16010000.cru1: Invalid MB address 0x0 (out of range)


> --
> 2.43.0
>
>

Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[Tommaso Merciai]

Hi Jacopo,
Thanks for your review!

On Mon, Feb 09, 2026 at 03:21:59PM +0100, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Tue, Dec 30, 2025 at 06:09:15PM +0100, Tommaso Merciai wrote:
> > When the CRU is configured to use ICnSVC for virtual channel mapping,
> > as on the RZ/{G3E, V2H/P} SoC, the ICnMC register must not be
> > programmed.
> 
> I see a difference indeed between the [G3E, V2H/P] and the G2L version
> of the IP in the presence of the VCSEL[1:0] field in register ICnMC.
> 
> On [G3E, V2H/P] the selection of which virtual channel to accept
> doesn't go through VCSEL[1:0] in ICnMC but a dedicated register ICnSVC
> is provided for that purpose. So far so good.
> 
> >
> > Return early after setting up ICnSVC to avoid overriding the ICnMC
> > register, which is not applicable in this mode.
> >
> > This prevents unintended register programming when ICnSVC is enabled.
> >
> > Fixes: 3c5ca0a48bb0 ("media: rzg2l-cru: Drop function pointer to configure CSI")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index 162e2ace6931..480e9b5dbcfe 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -268,6 +268,8 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> >  		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
> >  		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
> >  				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> > +
> > +		return;
> >  	}
> >
> >  	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
> 
> However, the rest of the rzg2l_cru_csi2_setup() doesn't only program
> the virtual channel filtering through ICnMC (which should be skept for
> G3E, V2H/P) but also programs DT filtering in INF[5:0] of register
> ICnIPMC_C0 which seems to be available on G3E and V2H/P as well.
> 
> Section "9.2.4.3.6 VC/Data Type Selector (for Image Processing)" of
> the chip manual (V2H/P) prescribes:
> 
> Only one data type can be handled by each SVC and the data types are
> selected in the ICnIPMC_C0 to 3.INF[5:0] registers.

You are correct, thanks.
I miss that part.

Maybe we can go for:

static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
				 const struct rzg2l_cru_ip_format *ip_fmt,
				 u8 csi_vc)
{
	const struct rzg2l_cru_info *info = cru->info;
	u32 icnmc = ICnMC_INF(ip_fmt->datatype);

	if (cru->info->regs[ICnSVC]) {
		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
	} else {
		/* Set virtual channel CSI2 */
		icnmc |= ICnMC_VCSEL(csi_vc);
	}

	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
	rzg2l_cru_write(cru, info->image_conv, icnmc);
}

?

> 
> And this patch makes the driver skips that part.
> Has this patch been tested ? It breaks my V2H/P setup:
> 
>  rzg2l-cru 16010000.cru1: Invalid MB address 0x0 (out of range)

I've not seeing that on RZ/G3E.
Thanks for sharing.


Kind Regards,
Tommaso

> 
> 
> > --
> > 2.43.0
> >
> >

Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[Jacopo Mondi]

Hi Tommaso

On Mon, Feb 09, 2026 at 04:37:57PM +0100, Tommaso Merciai wrote:
> Hi Jacopo,
> Thanks for your review!
>
> On Mon, Feb 09, 2026 at 03:21:59PM +0100, Jacopo Mondi wrote:
> > Hi Tommaso
> >
> > On Tue, Dec 30, 2025 at 06:09:15PM +0100, Tommaso Merciai wrote:
> > > When the CRU is configured to use ICnSVC for virtual channel mapping,
> > > as on the RZ/{G3E, V2H/P} SoC, the ICnMC register must not be
> > > programmed.
> >
> > I see a difference indeed between the [G3E, V2H/P] and the G2L version
> > of the IP in the presence of the VCSEL[1:0] field in register ICnMC.
> >
> > On [G3E, V2H/P] the selection of which virtual channel to accept
> > doesn't go through VCSEL[1:0] in ICnMC but a dedicated register ICnSVC
> > is provided for that purpose. So far so good.
> >
> > >
> > > Return early after setting up ICnSVC to avoid overriding the ICnMC
> > > register, which is not applicable in this mode.
> > >
> > > This prevents unintended register programming when ICnSVC is enabled.
> > >
> > > Fixes: 3c5ca0a48bb0 ("media: rzg2l-cru: Drop function pointer to configure CSI")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > index 162e2ace6931..480e9b5dbcfe 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > @@ -268,6 +268,8 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> > >  		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
> > >  		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
> > >  				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> > > +
> > > +		return;
> > >  	}
> > >
> > >  	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
> >
> > However, the rest of the rzg2l_cru_csi2_setup() doesn't only program
> > the virtual channel filtering through ICnMC (which should be skept for
> > G3E, V2H/P) but also programs DT filtering in INF[5:0] of register
> > ICnIPMC_C0 which seems to be available on G3E and V2H/P as well.
> >
> > Section "9.2.4.3.6 VC/Data Type Selector (for Image Processing)" of
> > the chip manual (V2H/P) prescribes:
> >
> > Only one data type can be handled by each SVC and the data types are
> > selected in the ICnIPMC_C0 to 3.INF[5:0] registers.
>
> You are correct, thanks.
> I miss that part.
>
> Maybe we can go for:
>
> static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> 				 const struct rzg2l_cru_ip_format *ip_fmt,
> 				 u8 csi_vc)
> {
> 	const struct rzg2l_cru_info *info = cru->info;
> 	u32 icnmc = ICnMC_INF(ip_fmt->datatype);
>
> 	if (cru->info->regs[ICnSVC]) {
> 		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
> 		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
> 				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> 	} else {
> 		/* Set virtual channel CSI2 */
> 		icnmc |= ICnMC_VCSEL(csi_vc);
> 	}
>
> 	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
> 	rzg2l_cru_write(cru, info->image_conv, icnmc);
> }
>
> ?

yes, I ended up with something similar locally to be able to move
forward and test more patches from this series.

static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
				 const struct rzg2l_cru_ip_format *ip_fmt,
				 u8 csi_vc)
{
	const struct rzg2l_cru_info *info = cru->info;
	u32 icnmc = rzg2l_cru_read(cru, info->image_conv) & ~(ICnMC_INF_MASK |
						              ICnMC_VCSEL_MASK);
	icnmc |= ICnMC_INF(ip_fmt->datatype);

	/*
	 * VC filtering goes through SVC register on G3E/V2H and through ICnMC
	 * on G2L.
	 */
	if (cru->info->regs[ICnSVC]) {
	 	/* FIXME: This only works if 'csi_vc' is 0. */
		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
	} else {
		icnmc |= ICnMC_VCSEL(csi_vc);
	}

	rzg2l_cru_write(cru, info->image_conv, icnmc);
}

>
> >
> > And this patch makes the driver skips that part.
> > Has this patch been tested ? It breaks my V2H/P setup:
> >
> >  rzg2l-cru 16010000.cru1: Invalid MB address 0x0 (out of range)
>
> I've not seeing that on RZ/G3E.
> Thanks for sharing.

Without programming which DT to filter I don't see how this could
work to be honest..

>
>
> Kind Regards,
> Tommaso
>
> >
> >
> > > --
> > > 2.43.0
> > >
> > >