* [RFC PATCH v1] media: rppx1: Update shadowed registers
@ 2026-10-26 13:58 Barnabás Pőcze
2026-10-26 14:16 ` Niklas Söderlund
0 siblings, 1 reply; 2+ messages in thread
From: Barnabás Pőcze @ 2026-10-26 13:58 UTC (permalink / raw)
To: Jacopo Mondi, Jai Luthra, Niklas Söderlund, Mauro Carvalho Chehab
Cc: linux-media, linux-kernel, Barnabás Pőcze
Certain registers are shadowed and changes to those must be explicly
applied. Updating the shadowed registers after a frame is complete can
be done by writing `RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD` to the
`RPPX1_RPP_HDR_UPD_REG` register.
This is already done in `rppx1_start()`, but this auto-update feature
is documented as "self reset at picture end". Therefore setting it
at start will only auto-update the registers once, after the first
frame.
For example, this causes that the LSC module cannot be toggled after
the first frame. This behaviour can be tested by reading the first
bit of `LSC_STATUS_REG`, which will show the same value after the
first frame regardless of what was last written to `LSC_CTRL_REG`.
Fix this by unconditionall triggering an auto-update when a parameter
buffer is processed in `rppx1_params()`.
Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
---
I made the minimal amount of changes to get the desired behaviour because
I don't know how the maintainers would prefer the registers in headers.
So I simply copied only a subset, but it feels a bit suboptimal like this,
hence this is an RFC.
---
.../platform/dreamchip/rppx1/rpp_params.c | 3 +++
.../media/platform/dreamchip/rppx1/rppx1.c | 23 -------------------
.../media/platform/dreamchip/rppx1/rppx1.h | 21 +++++++++++++++++
3 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
index a75a27a8afd09..cbc9847ba5be6 100644
--- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
+++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
@@ -110,6 +110,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
}
}
+ /* Trigger update of shadowed registers after frame is done. */
+ write(priv, RPPX1_RPP_HDR_UPD_REG, RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD);
+
return 0;
}
EXPORT_SYMBOL_GPL(rppx1_params);
diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1.c b/drivers/media/platform/dreamchip/rppx1/rppx1.c
index c502be7979146..4794672e088cf 100644
--- a/drivers/media/platform/dreamchip/rppx1/rppx1.c
+++ b/drivers/media/platform/dreamchip/rppx1/rppx1.c
@@ -15,7 +15,6 @@
#include "rppx1.h"
/* RPP_HDR Base Addresses */
-#define RPPX1_HDRREGS_BASE 0x0000
#define RPPX1_HDR_IRQ_BASE 0x0200
#define RPPX1_RPP_OUT_BASE 0x0800
#define RPPX1_RPP_RMAP_BASE 0x0c00
@@ -26,25 +25,6 @@
#define RPPX1_RPP_MVOUT_BASE 0xc000
#define RPPX1_RPP_FUSA_BASE 0xf000
-#define RPPX1_RPP_HDRREGS_VERSION_REG (RPPX1_HDRREGS_BASE + 0x0000)
-
-#define RPPX1_RPP_HDR_UPD_REG (RPPX1_HDRREGS_BASE + 0x0004)
-#define RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD BIT(1)
-#define RPPX1_RPP_HDR_UPD_REGS_CFG_UPD BIT(0)
-
-#define RPPX1_RESERVED_3_REG (RPPX1_HDRREGS_BASE + 0x0008)
-
-#define RPPX1_RPP_HDR_INFORM_ENABLE_REG (RPPX1_HDRREGS_BASE + 0x000c)
-#define RPPX1_RPP_HDR_INFORM_ENABLE_ENABLE 1
-#define RPPX1_RPP_HDR_INFORM_ENABLE_DISABLE 0
-
-#define RPPX1_RPP_HDR_OUT_IF_ON_REG (RPPX1_HDRREGS_BASE + 0x0010)
-#define RPPX1_RPP_HDR_OUT_IF_OFF_REG (RPPX1_HDRREGS_BASE + 0x0014)
-
-#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_REG (RPPX1_HDRREGS_BASE + 0x0018)
-#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_ENABLE 1
-#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_DISABLE 0
-
#define RPPX1_RPP_ISM (RPPX1_HDR_IRQ_BASE + 0x00)
#define RPPX1_RPP_RIS (RPPX1_HDR_IRQ_BASE + 0x04)
#define RPPX1_RPP_MIS (RPPX1_HDR_IRQ_BASE + 0x08)
@@ -349,9 +329,6 @@ int rppx1_start(struct rppx1 *rpp,
/* Immediate commit update for shadows. */
rppx1_write(rpp, RPPX1_RPP_HDR_UPD_REG, RPPX1_RPP_HDR_UPD_REGS_CFG_UPD);
- /* Then for operation update shadows with picture synchronization. */
- rppx1_write(rpp, RPPX1_RPP_HDR_UPD_REG, RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD);
-
/* Clear any pending interrupts. */
rppx1_interrupt(rpp, NULL);
diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1.h b/drivers/media/platform/dreamchip/rppx1/rppx1.h
index 3bf3955afbaed..c6bd82f504560 100644
--- a/drivers/media/platform/dreamchip/rppx1/rppx1.h
+++ b/drivers/media/platform/dreamchip/rppx1/rppx1.h
@@ -13,6 +13,27 @@
#include "rpp_module.h"
+#define RPPX1_HDRREGS_BASE 0x0000
+
+#define RPPX1_RPP_HDRREGS_VERSION_REG (RPPX1_HDRREGS_BASE + 0x0000)
+
+#define RPPX1_RPP_HDR_UPD_REG (RPPX1_HDRREGS_BASE + 0x0004)
+#define RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD BIT(1)
+#define RPPX1_RPP_HDR_UPD_REGS_CFG_UPD BIT(0)
+
+#define RPPX1_RESERVED_3_REG (RPPX1_HDRREGS_BASE + 0x0008)
+
+#define RPPX1_RPP_HDR_INFORM_ENABLE_REG (RPPX1_HDRREGS_BASE + 0x000c)
+#define RPPX1_RPP_HDR_INFORM_ENABLE_ENABLE 1
+#define RPPX1_RPP_HDR_INFORM_ENABLE_DISABLE 0
+
+#define RPPX1_RPP_HDR_OUT_IF_ON_REG (RPPX1_HDRREGS_BASE + 0x0010)
+#define RPPX1_RPP_HDR_OUT_IF_OFF_REG (RPPX1_HDRREGS_BASE + 0x0014)
+
+#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_REG (RPPX1_HDRREGS_BASE + 0x0018)
+#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_ENABLE 1
+#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_DISABLE 0
+
#define RPPX1_IRQ_ID_256HIST BIT(27)
#define RPPX1_IRQ_ID_PRE2_DPCC BIT(25)
#define RPPX1_IRQ_ID_PRE1_DPCC BIT(24)
--
2.56.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [RFC PATCH v1] media: rppx1: Update shadowed registers
2026-10-26 13:58 [RFC PATCH v1] media: rppx1: Update shadowed registers Barnabás Pőcze
@ 2026-10-26 14:16 ` Niklas Söderlund
0 siblings, 0 replies; 2+ messages in thread
From: Niklas Söderlund @ 2026-10-26 14:16 UTC (permalink / raw)
To: Barnabás Pőcze
Cc: Jacopo Mondi, Jai Luthra, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi Barnabás,
Thanks for your patch.
On 2026-10-26 14:58:14 +0100, Barnabás Pőcze wrote:
> Certain registers are shadowed and changes to those must be explicly
> applied. Updating the shadowed registers after a frame is complete can
> be done by writing `RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD` to the
> `RPPX1_RPP_HDR_UPD_REG` register.
>
> This is already done in `rppx1_start()`, but this auto-update feature
> is documented as "self reset at picture end". Therefore setting it
> at start will only auto-update the registers once, after the first
> frame.
>
> For example, this causes that the LSC module cannot be toggled after
> the first frame. This behaviour can be tested by reading the first
> bit of `LSC_STATUS_REG`, which will show the same value after the
> first frame regardless of what was last written to `LSC_CTRL_REG`.
>
> Fix this by unconditionall triggering an auto-update when a parameter
> buffer is processed in `rppx1_params()`.
Nice catch. Indeed all my CI test of the different modules setup one
use-case and do not change any shadowed registers.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> ---
>
> I made the minimal amount of changes to get the desired behaviour because
> I don't know how the maintainers would prefer the registers in headers.
> So I simply copied only a subset, but it feels a bit suboptimal like this,
> hence this is an RFC.
I prefers to have as small scope as possible for the defines, but of
course it needs to be useful. As the scope prior to this change fit
inside the .c file I kept it there.
Now it does not and I think it would make sens to move all of the
register defines in rppx1.c to rppx1.h to keep them together. With that
changed I think I'm happy with this change.
>
> ---
> .../platform/dreamchip/rppx1/rpp_params.c | 3 +++
> .../media/platform/dreamchip/rppx1/rppx1.c | 23 -------------------
> .../media/platform/dreamchip/rppx1/rppx1.h | 21 +++++++++++++++++
> 3 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> index a75a27a8afd09..cbc9847ba5be6 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
> @@ -110,6 +110,9 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
> }
> }
>
> + /* Trigger update of shadowed registers after frame is done. */
> + write(priv, RPPX1_RPP_HDR_UPD_REG, RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD);
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(rppx1_params);
> diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1.c b/drivers/media/platform/dreamchip/rppx1/rppx1.c
> index c502be7979146..4794672e088cf 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rppx1.c
> +++ b/drivers/media/platform/dreamchip/rppx1/rppx1.c
> @@ -15,7 +15,6 @@
> #include "rppx1.h"
>
> /* RPP_HDR Base Addresses */
> -#define RPPX1_HDRREGS_BASE 0x0000
> #define RPPX1_HDR_IRQ_BASE 0x0200
> #define RPPX1_RPP_OUT_BASE 0x0800
> #define RPPX1_RPP_RMAP_BASE 0x0c00
> @@ -26,25 +25,6 @@
> #define RPPX1_RPP_MVOUT_BASE 0xc000
> #define RPPX1_RPP_FUSA_BASE 0xf000
>
> -#define RPPX1_RPP_HDRREGS_VERSION_REG (RPPX1_HDRREGS_BASE + 0x0000)
> -
> -#define RPPX1_RPP_HDR_UPD_REG (RPPX1_HDRREGS_BASE + 0x0004)
> -#define RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD BIT(1)
> -#define RPPX1_RPP_HDR_UPD_REGS_CFG_UPD BIT(0)
> -
> -#define RPPX1_RESERVED_3_REG (RPPX1_HDRREGS_BASE + 0x0008)
> -
> -#define RPPX1_RPP_HDR_INFORM_ENABLE_REG (RPPX1_HDRREGS_BASE + 0x000c)
> -#define RPPX1_RPP_HDR_INFORM_ENABLE_ENABLE 1
> -#define RPPX1_RPP_HDR_INFORM_ENABLE_DISABLE 0
> -
> -#define RPPX1_RPP_HDR_OUT_IF_ON_REG (RPPX1_HDRREGS_BASE + 0x0010)
> -#define RPPX1_RPP_HDR_OUT_IF_OFF_REG (RPPX1_HDRREGS_BASE + 0x0014)
> -
> -#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_REG (RPPX1_HDRREGS_BASE + 0x0018)
> -#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_ENABLE 1
> -#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_DISABLE 0
> -
> #define RPPX1_RPP_ISM (RPPX1_HDR_IRQ_BASE + 0x00)
> #define RPPX1_RPP_RIS (RPPX1_HDR_IRQ_BASE + 0x04)
> #define RPPX1_RPP_MIS (RPPX1_HDR_IRQ_BASE + 0x08)
> @@ -349,9 +329,6 @@ int rppx1_start(struct rppx1 *rpp,
> /* Immediate commit update for shadows. */
> rppx1_write(rpp, RPPX1_RPP_HDR_UPD_REG, RPPX1_RPP_HDR_UPD_REGS_CFG_UPD);
>
> - /* Then for operation update shadows with picture synchronization. */
> - rppx1_write(rpp, RPPX1_RPP_HDR_UPD_REG, RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD);
> -
> /* Clear any pending interrupts. */
> rppx1_interrupt(rpp, NULL);
>
> diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1.h b/drivers/media/platform/dreamchip/rppx1/rppx1.h
> index 3bf3955afbaed..c6bd82f504560 100644
> --- a/drivers/media/platform/dreamchip/rppx1/rppx1.h
> +++ b/drivers/media/platform/dreamchip/rppx1/rppx1.h
> @@ -13,6 +13,27 @@
>
> #include "rpp_module.h"
>
> +#define RPPX1_HDRREGS_BASE 0x0000
> +
> +#define RPPX1_RPP_HDRREGS_VERSION_REG (RPPX1_HDRREGS_BASE + 0x0000)
> +
> +#define RPPX1_RPP_HDR_UPD_REG (RPPX1_HDRREGS_BASE + 0x0004)
> +#define RPPX1_RPP_HDR_UPD_REGS_GEN_CFG_UPD BIT(1)
> +#define RPPX1_RPP_HDR_UPD_REGS_CFG_UPD BIT(0)
> +
> +#define RPPX1_RESERVED_3_REG (RPPX1_HDRREGS_BASE + 0x0008)
> +
> +#define RPPX1_RPP_HDR_INFORM_ENABLE_REG (RPPX1_HDRREGS_BASE + 0x000c)
> +#define RPPX1_RPP_HDR_INFORM_ENABLE_ENABLE 1
> +#define RPPX1_RPP_HDR_INFORM_ENABLE_DISABLE 0
> +
> +#define RPPX1_RPP_HDR_OUT_IF_ON_REG (RPPX1_HDRREGS_BASE + 0x0010)
> +#define RPPX1_RPP_HDR_OUT_IF_OFF_REG (RPPX1_HDRREGS_BASE + 0x0014)
> +
> +#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_REG (RPPX1_HDRREGS_BASE + 0x0018)
> +#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_ENABLE 1
> +#define RPPX1_RPP_HDR_SAFETY_ACCESS_PROTECTION_DISABLE 0
> +
> #define RPPX1_IRQ_ID_256HIST BIT(27)
> #define RPPX1_IRQ_ID_PRE2_DPCC BIT(25)
> #define RPPX1_IRQ_ID_PRE1_DPCC BIT(24)
> --
> 2.56.0
>
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-26 14:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-26 13:58 [RFC PATCH v1] media: rppx1: Update shadowed registers Barnabás Pőcze
2026-10-26 14:16 ` Niklas Söderlund
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®