[PATCH 0/3] media: rzg2l-cru: Fixes and improvements

[PATCH 0/3] media: rzg2l-cru: Fixes and improvements

[Tommaso Merciai]

Dear All,

This patchset contains fixes and improvements for RZG2L CRU driver.
In particular the first patch skips the ICnMC configuration when is not
needed, for example on RZ/{G3E, V2H/P} SoC.
The second patch uses only frame end interrupts for DMA stopping state.
Removed also redundant buffer address clearing into rzg3e_cru_irq().

Thanks & Regards,
Tommaso

Tommaso Merciai (3):
  media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used
  media: rzg2l-cru: Use only frame end interrupts for DMA stopping state
  media: rzg2l-cru: Drop redundant buffer address clearing

 .../platform/renesas/rzg2l-cru/rzg2l-cru-regs.h      |  1 +
 .../media/platform/renesas/rzg2l-cru/rzg2l-video.c   | 12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
2.43.0

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

[PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state

[Tommaso Merciai]

On RZ/G3E the CRU driver relies on frame end interrupts to detect the
completion of an active frame when stopping DMA.

Update the driver to enable only frame end interrupts (CRUnIE2_FExE),
dropping the use of frame start interrupts, which are not required for
this flow.

Fix the interrupt status handling in the DMA stopping state by checking
the correct frame end status bits (FExS) instead of the frame start ones
(FSxS). Add a dedicated CRUnINTS2_FExS() macro to reflect the actual
register bit layout.

This ensures that DMA stopping is triggered by the intended frame end
events and avoids incorrect interrupt handling.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 .../media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h    | 1 +
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c   | 9 ++++-----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
index a5a57369ef0e..102a2fec5037 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
@@ -19,6 +19,7 @@
 
 #define CRUnINTS_SFS			BIT(16)
 
+#define CRUnINTS2_FExS(x)		BIT(((x) * 3) + 1)
 #define CRUnINTS2_FSxS(x)		BIT(((x) * 3))
 
 #define CRUnRST_VRESETN			BIT(0)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 480e9b5dbcfe..34e74e5796e8 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -437,7 +437,6 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
 
 void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
 {
-	rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FSxE(cru->svc_channel));
 	rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FExE(cru->svc_channel));
 }
 
@@ -697,10 +696,10 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
 		}
 
 		if (cru->state == RZG2L_CRU_DMA_STOPPING) {
-			if (irq_status & CRUnINTS2_FSxS(0) ||
-			    irq_status & CRUnINTS2_FSxS(1) ||
-			    irq_status & CRUnINTS2_FSxS(2) ||
-			    irq_status & CRUnINTS2_FSxS(3))
+			if (irq_status & CRUnINTS2_FExS(0) ||
+			    irq_status & CRUnINTS2_FExS(1) ||
+			    irq_status & CRUnINTS2_FExS(2) ||
+			    irq_status & CRUnINTS2_FExS(3))
 				dev_dbg(cru->dev, "IRQ while state stopping\n");
 			return IRQ_HANDLED;
 		}
-- 
2.43.0

Re: [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state

[Lad, Prabhakar]

On Tue, Dec 30, 2025 at 5:16 PM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
>
> On RZ/G3E the CRU driver relies on frame end interrupts to detect the
> completion of an active frame when stopping DMA.
>
> Update the driver to enable only frame end interrupts (CRUnIE2_FExE),
> dropping the use of frame start interrupts, which are not required for
> this flow.
>
> Fix the interrupt status handling in the DMA stopping state by checking
> the correct frame end status bits (FExS) instead of the frame start ones
> (FSxS). Add a dedicated CRUnINTS2_FExS() macro to reflect the actual
> register bit layout.
>
> This ensures that DMA stopping is triggered by the intended frame end
> events and avoids incorrect interrupt handling.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  .../media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h    | 1 +
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c   | 9 ++++-----
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> index a5a57369ef0e..102a2fec5037 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> @@ -19,6 +19,7 @@
>
>  #define CRUnINTS_SFS                   BIT(16)
>
> +#define CRUnINTS2_FExS(x)              BIT(((x) * 3) + 1)
>  #define CRUnINTS2_FSxS(x)              BIT(((x) * 3))
>
>  #define CRUnRST_VRESETN                        BIT(0)
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 480e9b5dbcfe..34e74e5796e8 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -437,7 +437,6 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
>
>  void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
>  {
> -       rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FSxE(cru->svc_channel));
>         rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FExE(cru->svc_channel));
>  }
>
> @@ -697,10 +696,10 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>                 }
>
>                 if (cru->state == RZG2L_CRU_DMA_STOPPING) {
> -                       if (irq_status & CRUnINTS2_FSxS(0) ||
> -                           irq_status & CRUnINTS2_FSxS(1) ||
> -                           irq_status & CRUnINTS2_FSxS(2) ||
> -                           irq_status & CRUnINTS2_FSxS(3))
> +                       if (irq_status & CRUnINTS2_FExS(0) ||
> +                           irq_status & CRUnINTS2_FExS(1) ||
> +                           irq_status & CRUnINTS2_FExS(2) ||
> +                           irq_status & CRUnINTS2_FExS(3))
>                                 dev_dbg(cru->dev, "IRQ while state stopping\n");
>                         return IRQ_HANDLED;
>                 }
> --
> 2.43.0
>
>

Re: [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state

[Jacopo Mondi]

Hi Tommaso

On Tue, Dec 30, 2025 at 06:09:16PM +0100, Tommaso Merciai wrote:
> On RZ/G3E the CRU driver relies on frame end interrupts to detect the
> completion of an active frame when stopping DMA.

Why do you think it does so ?

I read in rzg2l_cru_stop_image_processing()

	/* Stop the operation of image conversion */
	rzg2l_cru_write(cru, ICnEN, 0);

	/* Wait for streaming to stop */
	while ((rzg2l_cru_read(cru, ICnMS) & ICnMS_IA) && retries++ < RZG2L_RETRIES) {
		spin_unlock_irqrestore(&cru->qlock, flags);
		msleep(RZG2L_TIMEOUT_MS);
		spin_lock_irqsave(&cru->qlock, flags);
	}

Which seems to suggest the driver is polling a register to detect when
the Image Converter is still processing data or not.

>
> Update the driver to enable only frame end interrupts (CRUnIE2_FExE),
> dropping the use of frame start interrupts, which are not required for
> this flow.

This might be ok, but I don't think it's related to to detecting when
the DMA has actually stopped.

>
> Fix the interrupt status handling in the DMA stopping state by checking
> the correct frame end status bits (FExS) instead of the frame start ones
> (FSxS). Add a dedicated CRUnINTS2_FExS() macro to reflect the actual
> register bit layout.
>
> This ensures that DMA stopping is triggered by the intended frame end
> events and avoids incorrect interrupt handling.

I don't see where the frame end interrupt triggers the DMA stopping,
sorry

>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  .../media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h    | 1 +
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c   | 9 ++++-----
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> index a5a57369ef0e..102a2fec5037 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> @@ -19,6 +19,7 @@
>
>  #define CRUnINTS_SFS			BIT(16)
>
> +#define CRUnINTS2_FExS(x)		BIT(((x) * 3) + 1)
>  #define CRUnINTS2_FSxS(x)		BIT(((x) * 3))
>
>  #define CRUnRST_VRESETN			BIT(0)
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 480e9b5dbcfe..34e74e5796e8 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -437,7 +437,6 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
>
>  void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
>  {
> -	rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FSxE(cru->svc_channel));
>  	rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FExE(cru->svc_channel));
>  }
>
> @@ -697,10 +696,10 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>  		}
>
>  		if (cru->state == RZG2L_CRU_DMA_STOPPING) {
> -			if (irq_status & CRUnINTS2_FSxS(0) ||
> -			    irq_status & CRUnINTS2_FSxS(1) ||
> -			    irq_status & CRUnINTS2_FSxS(2) ||
> -			    irq_status & CRUnINTS2_FSxS(3))
> +			if (irq_status & CRUnINTS2_FExS(0) ||
> +			    irq_status & CRUnINTS2_FExS(1) ||
> +			    irq_status & CRUnINTS2_FExS(2) ||
> +			    irq_status & CRUnINTS2_FExS(3))

As the cru->state flag is accessed and set to RZG2L_CRU_DMA_STOPPING
without any lock and concurrently inspected by the IRQ handler, I guess
litterally anything can happen. Which makes me wonder
1) is the STOPPING condition useful at all
2) what is the purpose of this error detection

Thanks
  j

>  				dev_dbg(cru->dev, "IRQ while state stopping\n");
>  			return IRQ_HANDLED;
>  		}
> --
> 2.43.0
>

[PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing

[Tommaso Merciai]

Remove the clearing of cru->buf_addr[slot] in rzg3e_cru_irq().

The buffer address is already managed by rzg2l_cru_set_slot_addr(),
and explicitly setting it to zero here has no effect on the driver
behavior. Removing this assignment simplifies the code and avoids
unnecessary operations.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 34e74e5796e8..8ae6ef82a0da 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -709,7 +709,6 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
 			return IRQ_HANDLED;
 
 		dev_dbg(cru->dev, "Current written slot: %d\n", slot);
-		cru->buf_addr[slot] = 0;
 
 		/*
 		 * To hand buffers back in a known order to userspace start
-- 
2.43.0

Re: [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing

[Lad, Prabhakar]

On Tue, Dec 30, 2025 at 5:16 PM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
>
> Remove the clearing of cru->buf_addr[slot] in rzg3e_cru_irq().
>
> The buffer address is already managed by rzg2l_cru_set_slot_addr(),
> and explicitly setting it to zero here has no effect on the driver
> behavior. Removing this assignment simplifies the code and avoids
> unnecessary operations.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 1 -
>  1 file changed, 1 deletion(-)
>
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 34e74e5796e8..8ae6ef82a0da 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -709,7 +709,6 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>                         return IRQ_HANDLED;
>
>                 dev_dbg(cru->dev, "Current written slot: %d\n", slot);
> -               cru->buf_addr[slot] = 0;
>
>                 /*
>                  * To hand buffers back in a known order to userspace start
> --
> 2.43.0
>
>

Re: [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing

[Jacopo Mondi]

Hi Tommaso

On Tue, Dec 30, 2025 at 06:09:17PM +0100, Tommaso Merciai wrote:
> Remove the clearing of cru->buf_addr[slot] in rzg3e_cru_irq().
>
> The buffer address is already managed by rzg2l_cru_set_slot_addr(),
> and explicitly setting it to zero here has no effect on the driver
> behavior. Removing this assignment simplifies the code and avoids
> unnecessary operations.

The CRU driver in mainline is already unstable enough. I can capture
up to a few K of frames before hitting:

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

With this change I can capture up to a few hundred of frames before
hitting that.

I still have to investigate the actual reasons for the above error,
but before piling up more changes for this driver I think we should
try to fix it.

>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 34e74e5796e8..8ae6ef82a0da 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -709,7 +709,6 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>  			return IRQ_HANDLED;
>
>  		dev_dbg(cru->dev, "Current written slot: %d\n", slot);
> -		cru->buf_addr[slot] = 0;
>
>  		/*
>  		 * To hand buffers back in a known order to userspace start
> --
> 2.43.0
>
>