[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aYn_Vf1H1m1z44p2@tom-desktop>
Date: Mon, 9 Feb 2026 16:37:57 +0100
From: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
To: Jacopo Mondi <jacopo.mondi@...asonboard.com>
Cc: tomm.merciai@...il.com, linux-renesas-soc@...r.kernel.org,
biju.das.jz@...renesas.com,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
Hans Verkuil <hverkuil@...nel.org>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Daniel Scally <dan.scally+renesas@...asonboard.com>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when
ICnSVC is used
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@...r.kernel.org
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@...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
> >
> >
Powered by blists - more mailing lists