* [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used [not found] <cover.1767114395.git.tommaso.merciai.xr@bp.renesas.com> @ 2025-12-30 17:09 ` Tommaso Merciai 2026-01-09 22:10 ` Lad, Prabhakar 2026-02-09 14:21 ` Jacopo Mondi 0 siblings, 2 replies; 5+ messages in thread From: Tommaso Merciai @ 2025-12-30 17:09 UTC (permalink / raw) To: tomm.merciai Cc: linux-renesas-soc, biju.das.jz, Tommaso Merciai, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Lad Prabhakar, Daniel Scally, Jacopo Mondi, linux-media, linux-kernel, stable 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used 2025-12-30 17:09 ` [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Tommaso Merciai @ 2026-01-09 22:10 ` Lad, Prabhakar 2026-02-09 14:21 ` Jacopo Mondi 1 sibling, 0 replies; 5+ messages in thread From: Lad, Prabhakar @ 2026-01-09 22:10 UTC (permalink / raw) To: Tommaso Merciai Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Lad Prabhakar, Daniel Scally, Jacopo Mondi, linux-media, linux-kernel, stable 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used 2025-12-30 17:09 ` [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Tommaso Merciai 2026-01-09 22:10 ` Lad, Prabhakar @ 2026-02-09 14:21 ` Jacopo Mondi 2026-02-09 15:37 ` Tommaso Merciai 1 sibling, 1 reply; 5+ messages in thread From: Jacopo Mondi @ 2026-02-09 14:21 UTC (permalink / raw) To: Tommaso Merciai Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Lad Prabhakar, Daniel Scally, Jacopo Mondi, linux-media, linux-kernel, stable 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used 2026-02-09 14:21 ` Jacopo Mondi @ 2026-02-09 15:37 ` Tommaso Merciai 2026-02-09 16:05 ` Jacopo Mondi 0 siblings, 1 reply; 5+ messages in thread From: Tommaso Merciai @ 2026-02-09 15:37 UTC (permalink / raw) To: Jacopo Mondi Cc: tomm.merciai, linux-renesas-soc, biju.das.jz, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Lad Prabhakar, Daniel Scally, linux-media, linux-kernel, stable 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 > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used 2026-02-09 15:37 ` Tommaso Merciai @ 2026-02-09 16:05 ` Jacopo Mondi 0 siblings, 0 replies; 5+ messages in thread From: Jacopo Mondi @ 2026-02-09 16:05 UTC (permalink / raw) To: Tommaso Merciai Cc: Jacopo Mondi, tomm.merciai, linux-renesas-soc, biju.das.jz, Mauro Carvalho Chehab, Laurent Pinchart, Hans Verkuil, Lad Prabhakar, Daniel Scally, linux-media, linux-kernel, stable 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 > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-09 16:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1767114395.git.tommaso.merciai.xr@bp.renesas.com>
2025-12-30 17:09 ` [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 14:21 ` Jacopo Mondi
2026-02-09 15:37 ` Tommaso Merciai
2026-02-09 16:05 ` Jacopo Mondi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox