| 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: <b8dac5e182ed5142506ac730911c4158daeb4fd4.camel@ndufresne.ca>
Date: Wed, 24 Dec 2025 10:21:10 -0500
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 v2 08/22] media: rockchip: rga: move cmdbuf to rga_ctx
Hi,
Le mercredi 03 décembre 2025 à 16:52 +0100, Sven Püschel a écrit :
> Move the command buffer to the rga_ctx struct in preparation to reuse
> an already prepared command buffer. Having a command buffer per context
> allows to reuse the command buffer for other frames even when
> multiple applications are using the RGA in at the same time.
So the command buffer is only holding commands for a single render, and its not
possible to append to it right ? It has no sync point or anything that would let
us program it live ?
In this context, that sounds about right. Though please rework the commit
message, I find it a little miss-leading, since in the code you still program it
in device_run(), which is during the idle portion of the hardware.
To really benefit, you would have to program and flush it on the context qbuf,
so that once device_run() is call, you just have to set the command buf for the
context and trigger. Basically, no performance is gained from this patch I
believe.
>
> Signed-off-by: Sven Püschel <s.pueschel@...gutronix.de>
With a clear commit message:
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> ---
> drivers/media/platform/rockchip/rga/rga-hw.c | 28 +++++++---------
> drivers/media/platform/rockchip/rga/rga.c | 48 ++++++++++++++++-----------
> -
> drivers/media/platform/rockchip/rga/rga.h | 5 +--
> 3 files changed, 41 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c
> b/drivers/media/platform/rockchip/rga/rga-hw.c
> index 7419784efc693..79bf911e74779 100644
> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
> @@ -113,8 +113,7 @@ static struct rga_addr_offset *rga_lookup_draw_pos(struct
>
> static void rga_cmd_set_src_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
> {
> - struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> unsigned int reg;
>
> reg = RGA_MMU_SRC_BASE - RGA_MODE_BASE_REG;
> @@ -126,8 +125,7 @@ static void rga_cmd_set_src_addr(struct rga_ctx *ctx,
> dma_addr_t dma_addr)
>
> static void rga_cmd_set_src1_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
> {
> - struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> unsigned int reg;
>
> reg = RGA_MMU_SRC1_BASE - RGA_MODE_BASE_REG;
> @@ -139,8 +137,7 @@ static void rga_cmd_set_src1_addr(struct rga_ctx *ctx,
> dma_addr_t dma_addr)
>
> static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
> {
> - struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> unsigned int reg;
>
> reg = RGA_MMU_DST_BASE - RGA_MODE_BASE_REG;
> @@ -153,7 +150,7 @@ static void rga_cmd_set_dst_addr(struct rga_ctx *ctx,
> dma_addr_t dma_addr)
> static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
> {
> struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> unsigned int scale_dst_w, scale_dst_h;
> unsigned int src_h, src_w, dst_h, dst_w;
> union rga_src_info src_info;
> @@ -313,8 +310,7 @@ static void rga_cmd_set_src_info(struct rga_ctx *ctx,
> struct rga_addr_offset *offset)
> {
> struct rga_corners_addr_offset src_offsets;
> - struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> unsigned int src_h, src_w, src_x, src_y;
>
> src_h = ctx->in.crop.height;
> @@ -341,8 +337,7 @@ static void rga_cmd_set_dst_info(struct rga_ctx *ctx,
> {
> struct rga_addr_offset *dst_offset;
> struct rga_corners_addr_offset offsets;
> - struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> unsigned int dst_h, dst_w, dst_x, dst_y;
> unsigned int mir_mode = 0;
> unsigned int rot_mode = 0;
> @@ -388,8 +383,7 @@ static void rga_cmd_set_dst_info(struct rga_ctx *ctx,
>
> static void rga_cmd_set_mode(struct rga_ctx *ctx)
> {
> - struct rockchip_rga *rga = ctx->rga;
> - u32 *dest = rga->cmdbuf_virt;
> + u32 *dest = ctx->cmdbuf_virt;
> union rga_mode_ctrl mode;
> union rga_alpha_ctrl0 alpha_ctrl0;
> union rga_alpha_ctrl1 alpha_ctrl1;
> @@ -414,7 +408,7 @@ static void rga_cmd_set(struct rga_ctx *ctx,
> {
> struct rockchip_rga *rga = ctx->rga;
>
> - memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE * 4);
> + memset(ctx->cmdbuf_virt, 0, RGA_CMDBUF_SIZE * 4);
>
> rga_cmd_set_src_addr(ctx, src->dma_desc_pa);
> /*
> @@ -430,11 +424,11 @@ static void rga_cmd_set(struct rga_ctx *ctx,
> rga_cmd_set_dst_info(ctx, &dst->offset);
> rga_cmd_set_trans_info(ctx);
>
> - rga_write(rga, RGA_CMD_BASE, rga->cmdbuf_phy);
> + rga_write(rga, RGA_CMD_BASE, ctx->cmdbuf_phy);
>
> /* sync CMD buf for RGA */
> - dma_sync_single_for_device(rga->dev, rga->cmdbuf_phy,
> - PAGE_SIZE, DMA_BIDIRECTIONAL);
> + dma_sync_single_for_device(rga->dev, ctx->cmdbuf_phy,
> + PAGE_SIZE, DMA_BIDIRECTIONAL);
> }
>
> static void rga_hw_start(struct rockchip_rga *rga,
> diff --git a/drivers/media/platform/rockchip/rga/rga.c
> b/drivers/media/platform/rockchip/rga/rga.c
> index d8b14c2d9f6a4..74d94c7fe31ff 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -219,6 +219,16 @@ static int rga_open(struct file *file)
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> if (!ctx)
> return -ENOMEM;
> +
> + /* Create CMD buffer */
> + ctx->cmdbuf_virt = dma_alloc_attrs(rga->dev, rga->hw->cmdbuf_size,
> + &ctx->cmdbuf_phy, GFP_KERNEL,
> + DMA_ATTR_WRITE_COMBINE);
> + if (!ctx->cmdbuf_virt) {
> + ret = -ENOMEM;
> + goto rel_ctx;
> + }
> +
> ctx->rga = rga;
> /* Set default formats */
> ctx->in = def_frame;
> @@ -230,15 +240,13 @@ static int rga_open(struct file *file)
> ctx->out.fmt->fourcc, def_width, def_height);
>
> if (mutex_lock_interruptible(&rga->mutex)) {
> - kfree(ctx);
> - return -ERESTARTSYS;
> + ret = -ERESTARTSYS;
> + goto rel_cmdbuf;
> }
> ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(rga->m2m_dev, ctx, &queue_init);
> if (IS_ERR(ctx->fh.m2m_ctx)) {
> ret = PTR_ERR(ctx->fh.m2m_ctx);
> - mutex_unlock(&rga->mutex);
> - kfree(ctx);
> - return ret;
> + goto unlock_mutex;
> }
> v4l2_fh_init(&ctx->fh, video_devdata(file));
> v4l2_fh_add(&ctx->fh, file);
> @@ -252,6 +260,15 @@ static int rga_open(struct file *file)
> mutex_unlock(&rga->mutex);
>
> return 0;
> +
> +unlock_mutex:
> + mutex_unlock(&rga->mutex);
> +rel_cmdbuf:
> + dma_free_attrs(rga->dev, rga->hw->cmdbuf_size, ctx->cmdbuf_virt,
> + ctx->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
> +rel_ctx:
> + kfree(ctx);
> + return ret;
> }
>
> static int rga_release(struct file *file)
> @@ -266,6 +283,10 @@ static int rga_release(struct file *file)
> v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> v4l2_fh_del(&ctx->fh, file);
> v4l2_fh_exit(&ctx->fh);
> +
> + dma_free_attrs(rga->dev, rga->hw->cmdbuf_size, ctx->cmdbuf_virt,
> + ctx->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
> +
> kfree(ctx);
>
> mutex_unlock(&rga->mutex);
> @@ -674,19 +695,10 @@ static int rga_probe(struct platform_device *pdev)
>
> pm_runtime_put(rga->dev);
>
> - /* Create CMD buffer */
> - rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, rga->hw->cmdbuf_size,
> - &rga->cmdbuf_phy, GFP_KERNEL,
> - DMA_ATTR_WRITE_COMBINE);
> - if (!rga->cmdbuf_virt) {
> - ret = -ENOMEM;
> - goto rel_m2m;
> - }
> -
> ret = video_register_device(vfd, VFL_TYPE_VIDEO, -1);
> if (ret) {
> v4l2_err(&rga->v4l2_dev, "Failed to register video
> device\n");
> - goto free_dma;
> + goto rel_m2m;
> }
>
> v4l2_info(&rga->v4l2_dev, "Registered %s as /dev/%s\n",
> @@ -694,9 +706,6 @@ static int rga_probe(struct platform_device *pdev)
>
> return 0;
>
> -free_dma:
> - dma_free_attrs(rga->dev, rga->hw->cmdbuf_size, rga->cmdbuf_virt,
> - rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
> rel_m2m:
> v4l2_m2m_release(rga->m2m_dev);
> rel_vdev:
> @@ -713,9 +722,6 @@ static void rga_remove(struct platform_device *pdev)
> {
> struct rockchip_rga *rga = platform_get_drvdata(pdev);
>
> - dma_free_attrs(rga->dev, rga->hw->cmdbuf_size, rga->cmdbuf_virt,
> - rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
> -
> v4l2_info(&rga->v4l2_dev, "Removing\n");
>
> v4l2_m2m_release(rga->m2m_dev);
> diff --git a/drivers/media/platform/rockchip/rga/rga.h
> b/drivers/media/platform/rockchip/rga/rga.h
> index 98635ddd50d7c..f8c932fdec21a 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -56,6 +56,9 @@ struct rga_ctx {
> struct rga_frame out;
> struct v4l2_ctrl_handler ctrl_handler;
>
> + void *cmdbuf_virt;
> + dma_addr_t cmdbuf_phy;
> +
> int osequence;
> int csequence;
>
> @@ -91,8 +94,6 @@ struct rockchip_rga {
> spinlock_t ctrl_lock;
>
> struct rga_ctx *curr;
> - dma_addr_t cmdbuf_phy;
> - void *cmdbuf_virt;
>
> const struct rga_hw *hw;
> };
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists