| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a57669be1d58e71229dedcf4acbe6eda201e45ea.camel@ndufresne.ca>
Date: Tue, 07 Oct 2025 14:47:38 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Sven Püschel <s.pueschel@...gutronix.de>, Jacob Chen
<jacob-chen@...wrt.com>, Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
Mauro Carvalho Chehab
<mchehab@...nel.org>, Heiko Stuebner <heiko@...ech.de>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>
Cc: linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH 12/16] media: rockchip: rga: handle error interrupt
Le mardi 07 octobre 2025 à 10:32 +0200, Sven Püschel a écrit :
> Handle the error interrupt status in preparation of the RGA3 addition.
> This allows the buffer to be marked as done, as it would otherwise
> be stuck in the queue.
>
> The RGA3 needs a soft reset to properly work after an error occurred,
> as it would otherwise cease to deliver new interrupts. Also the soft
> reset avoids additional error interrupts to be triggered, which are
> currently not supported by the rga_isr function.
> As it is unknown how the RGA2 behaves in the error case, no
> error interrupt was enabled and handled.
>
> Signed-off-by: Sven Püschel <s.pueschel@...gutronix.de>
> ---
> drivers/media/platform/rockchip/rga/rga-hw.c | 6 ++++--
> drivers/media/platform/rockchip/rga/rga.c | 32 +++++++++++++++++----------
> -
> drivers/media/platform/rockchip/rga/rga.h | 8 ++++++-
> 3 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c
> b/drivers/media/platform/rockchip/rga/rga-hw.c
> index
> d54183d224b3e9c42d5503acf172257f2e736f7b..93822b5b8b15e76862bd022759eaa5cb9552
> dd76 100644
> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
> @@ -459,7 +459,7 @@ static void rga_hw_start(struct rockchip_rga *rga,
> rga_write(rga, RGA_CMD_CTRL, 0x1);
> }
>
> -static bool rga_handle_irq(struct rockchip_rga *rga)
> +static enum rga_irq_result rga_handle_irq(struct rockchip_rga *rga)
> {
> int intr;
>
> @@ -467,7 +467,9 @@ static bool rga_handle_irq(struct rockchip_rga *rga)
>
> rga_mod(rga, RGA_INT, intr << 4, 0xf << 4);
>
> - return intr & 0x04;
> + if (intr & 0x04)
> + return RGA_IRQ_DONE;
Since you reuse an old driver, would be nice to create proper defines for
RGA_INT bit 3 (current command finish interrupt flag).
> + return RGA_IRQ_IGNORE;
> }
>
> static void rga_get_version(struct rockchip_rga *rga)
> diff --git a/drivers/media/platform/rockchip/rga/rga.c
> b/drivers/media/platform/rockchip/rga/rga.c
> index
> 0a725841b0cfa41bbc5b861b8f5ceac2452fc2b5..3b5d2eb8e109f44af76dd2240a239b1fa8a7
> 8cee 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -56,30 +56,38 @@ static void device_run(void *prv)
> static irqreturn_t rga_isr(int irq, void *prv)
> {
> struct rockchip_rga *rga = prv;
> + struct vb2_v4l2_buffer *src, *dst;
> + struct rga_ctx *ctx = rga->curr;
> + enum rga_irq_result result;
>
> - if (rga->hw->handle_irq(rga)) {
> - struct vb2_v4l2_buffer *src, *dst;
> - struct rga_ctx *ctx = rga->curr;
> + result = rga->hw->handle_irq(rga);
> + if (result == RGA_IRQ_IGNORE)
> + return IRQ_HANDLED;
>
> - WARN_ON(!ctx);
> + WARN_ON(!ctx);
>
> - rga->curr = NULL;
> + rga->curr = NULL;
>
> - src = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> - dst = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> + src = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + dst = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>
> - WARN_ON(!src);
> - WARN_ON(!dst);
> + WARN_ON(!src);
> + WARN_ON(!dst);
>
> - v4l2_m2m_buf_copy_metadata(src, dst, true);
> + v4l2_m2m_buf_copy_metadata(src, dst, true);
>
> - dst->sequence = ctx->csequence++;
> + dst->sequence = ctx->csequence++;
>
> + if (result == RGA_IRQ_DONE) {
> v4l2_m2m_buf_done(src, VB2_BUF_STATE_DONE);
> v4l2_m2m_buf_done(dst, VB2_BUF_STATE_DONE);
> - v4l2_m2m_job_finish(rga->m2m_dev, ctx->fh.m2m_ctx);
> + } else {
> + v4l2_m2m_buf_done(src, VB2_BUF_STATE_ERROR);
> + v4l2_m2m_buf_done(dst, VB2_BUF_STATE_ERROR)
I'm not fan of assumption that its an error on else case. If often lead to
multiple calls. Please use an explicit error return.
> ;
> }
>
> + v4l2_m2m_job_finish(rga->m2m_dev, ctx->fh.m2m_ctx);
What if you get an IRQ and none of the flags are raised ? I did see that in the
past, and that least to bad things happening.
> +
> return IRQ_HANDLED;
> }
>
> diff --git a/drivers/media/platform/rockchip/rga/rga.h
> b/drivers/media/platform/rockchip/rga/rga.h
> index
> e19c4c82aca5ae2056f52d525138093fbbb81af8..dc4bb85707d12f5378c4891098cd7ea4a4d7
> 5e2d 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -143,6 +143,12 @@ static inline void rga_mod(struct rockchip_rga *rga, u32
> reg, u32 val, u32 mask)
> rga_write(rga, reg, temp);
> };
>
> +enum rga_irq_result {
> + RGA_IRQ_IGNORE,
> + RGA_IRQ_DONE,
> + RGA_IRQ_ERROR,
> +};
> +
> struct rga_hw {
> const char *card_type;
> bool has_internal_iommu;
> @@ -152,7 +158,7 @@ struct rga_hw {
>
> void (*start)(struct rockchip_rga *rga,
> struct rga_vb_buffer *src, struct rga_vb_buffer *dst);
> - bool (*handle_irq)(struct rockchip_rga *rga);
> + enum rga_irq_result (*handle_irq)(struct rockchip_rga *rga);
> void (*get_version)(struct rockchip_rga *rga);
> void *(*try_format)(u32 *fourcc, bool is_output);
> int (*enum_format)(struct v4l2_fmtdesc *f);
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists