Re: [PATCH 2/4] media: dw100: Implement dynamic vertex map update

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, Jan 05, 2026 at 01:58:25PM -0500, Nicolas Dufresne wrote:
> 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@ideasonboard.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.

v4l2_ctrl_request_complete() does not apply a mapping, what am I missing
?

> 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.

Applications should move to using requests. We'll do so in libcamera
unconditionally. I don't expect many other direct users, so warning that
the mapping won't be applied when an application sets the corresponding
control during streaming without using requests seems fair to me. It
will help debugging issues.

> > +		);
> > +	}
> > +
> >  	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> >  				   &ctx->hdl);
> >  

-- 
Regards,

Laurent Pinchart