* [PATCH 0/3] media: rzg2l-cru: Fixes and improvements
@ 2025-12-30 17:09 Tommaso Merciai
2025-12-30 17:09 ` [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Tommaso Merciai
` (2 more replies)
0 siblings, 3 replies; 12+ 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
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used
2025-12-30 17:09 [PATCH 0/3] media: rzg2l-cru: Fixes and improvements Tommaso Merciai
@ 2025-12-30 17:09 ` Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 14:21 ` Jacopo Mondi
2025-12-30 17:09 ` [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state Tommaso Merciai
2025-12-30 17:09 ` [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing Tommaso Merciai
2 siblings, 2 replies; 12+ 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] 12+ messages in thread
* [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state
2025-12-30 17:09 [PATCH 0/3] media: rzg2l-cru: Fixes and improvements Tommaso Merciai
2025-12-30 17:09 ` [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Tommaso Merciai
@ 2025-12-30 17:09 ` Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 15:13 ` Jacopo Mondi
2025-12-30 17:09 ` [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing Tommaso Merciai
2 siblings, 2 replies; 12+ 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
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing
2025-12-30 17:09 [PATCH 0/3] media: rzg2l-cru: Fixes and improvements Tommaso Merciai
2025-12-30 17:09 ` [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Tommaso Merciai
2025-12-30 17:09 ` [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state Tommaso Merciai
@ 2025-12-30 17:09 ` Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 15:41 ` Jacopo Mondi
2 siblings, 2 replies; 12+ 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
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
^ permalink raw reply related [flat|nested] 12+ 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; 12+ 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] 12+ messages in thread
* Re: [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state
2025-12-30 17:09 ` [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state Tommaso Merciai
@ 2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 15:13 ` Jacopo Mondi
1 sibling, 0 replies; 12+ 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
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
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing
2025-12-30 17:09 ` [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing Tommaso Merciai
@ 2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 15:41 ` Jacopo Mondi
1 sibling, 0 replies; 12+ 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
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
>
>
^ permalink raw reply [flat|nested] 12+ 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; 12+ 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] 12+ messages in thread
* Re: [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state
2025-12-30 17:09 ` [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
@ 2026-02-09 15:13 ` Jacopo Mondi
1 sibling, 0 replies; 12+ messages in thread
From: Jacopo Mondi @ 2026-02-09 15:13 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
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
>
^ permalink raw reply [flat|nested] 12+ 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; 12+ 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] 12+ messages in thread
* Re: [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing
2025-12-30 17:09 ` [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
@ 2026-02-09 15:41 ` Jacopo Mondi
1 sibling, 0 replies; 12+ messages in thread
From: Jacopo Mondi @ 2026-02-09 15:41 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
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
>
>
^ permalink raw reply [flat|nested] 12+ 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; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2026-02-09 16:05 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-30 17:09 [PATCH 0/3] media: rzg2l-cru: Fixes and improvements Tommaso Merciai
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
2025-12-30 17:09 ` [PATCH 2/3] media: rzg2l-cru: Use only frame end interrupts for DMA stopping state Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 15:13 ` Jacopo Mondi
2025-12-30 17:09 ` [PATCH 3/3] media: rzg2l-cru: Drop redundant buffer address clearing Tommaso Merciai
2026-01-09 22:10 ` Lad, Prabhakar
2026-02-09 15:41 ` Jacopo Mondi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox