[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f4e0b1f13ee54d88d1035828af548f5cf3a25c16.camel@ndufresne.ca>
Date: Mon, 05 Jan 2026 13:58:25 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Stefan Klug <stefan.klug@...asonboard.com>, Xavier Roumegue
<xavier.roumegue@....nxp.com>, Mauro Carvalho Chehab <mchehab@...nel.org>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Clark Williams
<clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>, Laurent
Pinchart <laurent.pinchart@...asonboard.com>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH 2/4] media: dw100: Implement dynamic vertex map update
Hi,
Le lundi 05 janvier 2026 à 12:35 +0100, Stefan Klug a écrit :
> Implement dynamic vertex map updates by handling the
> V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP control during streaming. This
> allows to implement features like dynamic zoom, pan, rotate and dewarp.
>
> To stay compatible with the old version, updates of
> V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP are ignored during streaming
> when requests are not used. Print a corresponding warning once.
>
> Signed-off-by: Stefan Klug <stefan.klug@...asonboard.com>
> ---
> drivers/media/platform/nxp/dw100/dw100.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 7f14b82c15a071605c124dbb868f8003856c4fc0..8a421059a1c9b55f514a29d3c2c5a6ffb76e0a64 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -98,6 +98,8 @@ struct dw100_ctx {
> unsigned int map_width;
> unsigned int map_height;
> bool user_map_is_set;
> + bool user_map_needs_update;
> + bool warned_dynamic_update;
>
> /* Source and destination queue data */
> struct dw100_q_data q_data[2];
> @@ -293,11 +295,15 @@ static u32 dw100_map_format_coordinates(u16 xq, u16 yq)
> return (u32)((yq << 16) | xq);
> }
>
> -static u32 *dw100_get_user_map(struct dw100_ctx *ctx)
> +static void dw100_update_mapping(struct dw100_ctx *ctx)
> {
> struct v4l2_ctrl *ctrl = ctx->ctrls[DW100_CTRL_DEWARPING_MAP];
>
> - return ctrl->p_cur.p_u32;
> + if (!ctx->user_map_needs_update)
> + return;
> +
> + memcpy(ctx->map, ctrl->p_cur.p_u32, ctx->map_size);
> + ctx->user_map_needs_update = false;
> }
>
> /*
> @@ -306,8 +312,6 @@ static u32 *dw100_get_user_map(struct dw100_ctx *ctx)
> */
> static int dw100_create_mapping(struct dw100_ctx *ctx)
> {
> - u32 *user_map;
> -
> if (ctx->map)
> dma_free_coherent(&ctx->dw_dev->pdev->dev, ctx->map_size,
> ctx->map, ctx->map_dma);
> @@ -318,8 +322,8 @@ static int dw100_create_mapping(struct dw100_ctx *ctx)
> if (!ctx->map)
> return -ENOMEM;
>
> - user_map = dw100_get_user_map(ctx);
> - memcpy(ctx->map, user_map, ctx->map_size);
> + ctx->user_map_needs_update = true;
> + dw100_update_mapping(ctx);
>
> dev_dbg(&ctx->dw_dev->pdev->dev,
> "%ux%u %s mapping created (d:%pad-c:%p) for stream %ux%u->%ux%u\n",
> @@ -351,6 +355,7 @@ static int dw100_s_ctrl(struct v4l2_ctrl *ctrl)
> switch (ctrl->id) {
> case V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP:
> ctx->user_map_is_set = true;
> + ctx->user_map_needs_update = true;
This will be called before the new mapping is applied by
v4l2_ctrl_request_complete(), and then may be cleared too soon if you queue
depth is high enough.
Instead, you should check in the request for the presence of
V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP to optimize the updates. Yo may still
set this to true if if there is no request, in the case you also wanted to
change the original behaviour of only updating that vertex on streamon, but I
don't see much point though.
> break;
> }
>
> @@ -405,6 +410,7 @@ static void dw100_ctrl_dewarping_map_init(const struct v4l2_ctrl *ctrl,
> }
>
> ctx->user_map_is_set = false;
> + ctx->user_map_needs_update = true;
> }
>
> static const struct v4l2_ctrl_type_ops dw100_ctrl_type_ops = {
> @@ -1482,6 +1488,15 @@ static void dw100_device_run(void *priv)
> v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
> &ctx->hdl);
>
> + if (src_buf->vb2_buf.req_obj.req) {
> + dw100_update_mapping(ctx);
> + } else if (ctx->user_map_needs_update && !ctx->warned_dynamic_update) {
> + ctx->warned_dynamic_update = true;
> + dev_warn(&ctx->dw_dev->pdev->dev,
> + "V4L2 requests are required to update the vertex map dynamically"
Do you know about dev_warn_once() ? That being said, the driver is usable
without requests and a static vertex (and must stay this way to not break the
ABI). I don't think you should warn here at all.
Nicolas
> + );
> + }
> +
> v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> &ctx->hdl);
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists