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;
 		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"
+		);
+	}
+
 	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
 				   &ctx->hdl);
 

-- 
2.51.0

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

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);
>  

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

Hi,

Le mardi 06 janvier 2026 à 02:42 +0200, Laurent Pinchart a écrit :
> 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 +++++++++++++++++++++------
> > > 

[...]

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

Sorry for my confusion, after reading back, you are correct, this is called
v4l2_ctrl_request_setup() inside the device_run() function. You can dismiss the
bit about user_map_needs_update in my review (the paragraph below).

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

It is also a miss-use of dev_warn which is meant to indicate a problem with the
driver or the hardware. The V4L2 core uses dev_dbg() or customer log level for
that in general. Also, don't re-implement _once() variants with
warned_dynamic_update please. Personally, I would just let the out-of request
change the control on the next device_run(), even if that means its out of sync.

Nicolas

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

Hi Nicolas,

On Tue, Jan 06, 2026 at 08:47:59AM -0500, Nicolas Dufresne wrote:
> Le mardi 06 janvier 2026 à 02:42 +0200, Laurent Pinchart a écrit :
> > 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 +++++++++++++++++++++------
> > > > 
> 
> [...]
> 
> > > >  	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
> > ?
> 
> Sorry for my confusion, after reading back, you are correct, this is called
> v4l2_ctrl_request_setup() inside the device_run() function. You can dismiss the
> bit about user_map_needs_update in my review (the paragraph below).

Thanks for the clarification.

> > > 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.
> 
> It is also a miss-use of dev_warn which is meant to indicate a problem with the
> driver or the hardware. The V4L2 core uses dev_dbg() or customer log level for
> that in general. Also, don't re-implement _once() variants with
> warned_dynamic_update please.

Agreed about the _once() variant. That will move to printing the message
once per context to once globally, but I think it's fine given the
purpose of warning application developers of incorrect usage.

> Personally, I would just let the out-of request
> change the control on the next device_run(), even if that means its out of sync.

So you mean changing the current behaviour of applying the control at
next streamon to next .device_run() even if it means synchronization
between the control and the frame being processed can't be guaranteed ?
I don't think that's a good idea, synchronization is a key requirement.
We carried a local kernel patch that would do just that, and to make it
usable we had to ensure in libcamera that we never queued more than one
buffer at a time, to guarantee synchronous behaviour. I think
applications should use the request API for dynamic updates of the
dewarp map.

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

-- 
Regards,

Laurent Pinchart

Hi,

Quoting Nicolas Dufresne (2026-01-06 14:47:59)
> Hi,
> 
> Le mardi 06 janvier 2026 à 02:42 +0200, Laurent Pinchart a écrit :
> > 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 +++++++++++++++++++++------
> > > > 
> 
> [...]
> 
> > > >   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
> > ?
> 
> Sorry for my confusion, after reading back, you are correct, this is called
> v4l2_ctrl_request_setup() inside the device_run() function. You can dismiss the
> bit about user_map_needs_update in my review (the paragraph below).
> 

That means nothing has to change here, right?

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

My idea was that I'd like to see the warning once per context and not
once per boot. Afaik I can't use dev_warn_once() for that.

> > 
> > 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.
> 
> It is also a miss-use of dev_warn which is meant to indicate a problem with the
> driver or the hardware. The V4L2 core uses dev_dbg() or customer log level for
> that in general. Also, don't re-implement _once() variants with
> warned_dynamic_update please. Personally, I would just let the out-of request
> change the control on the next device_run(), even if that means its out of sync.

But then you end up with potentially difficult to debug issues, because
users would not know that they should use requests. Not warning (or
dev_dbg) has the same effect from my point of view, because users just
see a device not working as expected. Is a customer log level the
solution?

Best regards,
Stefan

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

	Le mardi 06 janvier 2026 à 15:29 +0100, Stefan Klug a écrit :
> Hi,
> 
> Quoting Nicolas Dufresne (2026-01-06 14:47:59)
> > Hi,
> > 
> > Le mardi 06 janvier 2026 à 02:42 +0200, Laurent Pinchart a écrit :
> > > 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 +++++++++++++++++++++------
> > > > > 
> > 
> > [...]
> > 
> > > > >   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
> > > ?
> > 
> > Sorry for my confusion, after reading back, you are correct, this is called
> > v4l2_ctrl_request_setup() inside the device_run() function. You can dismiss the
> > bit about user_map_needs_update in my review (the paragraph below).
> > 
> 
> That means nothing has to change here, right?

Correct.

> 
> > > 
> > > > 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.
> 
> My idea was that I'd like to see the warning once per context and not
> once per boot. Afaik I can't use dev_warn_once() for that.

I didn't catch this detail (commit comment welcome), fair enough if that's the
direction we are heading. Again, I don't understand why we don't just apply that
on next device_run in "legacy" mode.

> 
> > > 
> > > 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.
> > 
> > It is also a miss-use of dev_warn which is meant to indicate a problem with the
> > driver or the hardware. The V4L2 core uses dev_dbg() or customer log level for
> > that in general. Also, don't re-implement _once() variants with
> > warned_dynamic_update please. Personally, I would just let the out-of request
> > change the control on the next device_run(), even if that means its out of sync.
> 
> But then you end up with potentially difficult to debug issues, because
> users would not know that they should use requests. Not warning (or
> dev_dbg) has the same effect from my point of view, because users just
> see a device not working as expected. Is a customer log level the
> solution?

Custom level are hard to discover for sure, but the rules around dev_warn()
aren't mine. We can perhaps makes an argument that the driver is broken, but why
don't we fix it to match the V4L2 spec is still unknown, specially that its an
easy fix match the V4L2 spec (even if in practice, this is not quite usable).
Normally everything else we add above the V4L2 spec, specially stuff using
request get a domain specific spec. With that in hand, we could create better
rules, but otherwise I don't have much foundation to make a good call here.

Nicolas

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

On Tue, Jan 06, 2026 at 10:27:31AM -0500, Nicolas Dufresne wrote:
> Le mardi 06 janvier 2026 à 15:29 +0100, Stefan Klug a écrit :
> > Quoting Nicolas Dufresne (2026-01-06 14:47:59)
> > > Le mardi 06 janvier 2026 à 02:42 +0200, Laurent Pinchart a écrit :
> > > > 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 +++++++++++++++++++++------
> > > > > > 
> > > 
> > > [...]
> > > 
> > > > > >   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
> > > > ?
> > > 
> > > Sorry for my confusion, after reading back, you are correct, this is called
> > > v4l2_ctrl_request_setup() inside the device_run() function. You can dismiss the
> > > bit about user_map_needs_update in my review (the paragraph below).
> > 
> > That means nothing has to change here, right?
> 
> Correct.
> 
> > > > > 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.
> > 
> > My idea was that I'd like to see the warning once per context and not
> > once per boot. Afaik I can't use dev_warn_once() for that.
> 
> I didn't catch this detail (commit comment welcome), fair enough if that's the
> direction we are heading. Again, I don't understand why we don't just apply that
> on next device_run in "legacy" mode.

Because to be really useful, dynamic mode requires synchronization with
buffers. Sure, it's possible to implement a non-synchronized legacy mode
and let userspace handle synchronization, but that's suboptimal as it
will require never queuing more than one buffer to the kernel at a time.
I don't think we should implement APIs that let userspace do things in
the wrong way when it's possible and simple enough to do them right.
That would add code to the kernel that would increase the driver
complexity with no real upside.

> > > > 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.
> > > 
> > > It is also a miss-use of dev_warn which is meant to indicate a problem with the
> > > driver or the hardware. The V4L2 core uses dev_dbg() or customer log level for
> > > that in general. Also, don't re-implement _once() variants with
> > > warned_dynamic_update please. Personally, I would just let the out-of request
> > > change the control on the next device_run(), even if that means its out of sync.
> > 
> > But then you end up with potentially difficult to debug issues, because
> > users would not know that they should use requests. Not warning (or
> > dev_dbg) has the same effect from my point of view, because users just
> > see a device not working as expected. Is a customer log level the
> > solution?
> 
> Custom level are hard to discover for sure, but the rules around dev_warn()
> aren't mine. We can perhaps makes an argument that the driver is broken, but why
> don't we fix it to match the V4L2 spec is still unknown, specially that its an
> easy fix match the V4L2 spec (even if in practice, this is not quite usable).
> Normally everything else we add above the V4L2 spec, specially stuff using
> request get a domain specific spec. With that in hand, we could create better
> rules, but otherwise I don't have much foundation to make a good call here.
> 
> > > > > > +         );
> > > > > > + }
> > > > > > +
> > > > > >   v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> > > > > >                              &ctx->hdl);

-- 
Regards,

Laurent Pinchart