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

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

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
> > >
> > >