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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2dbb2c41422cb4dbc8c3692646aeaf70c4d87df0.camel@ndufresne.ca>
Date: Tue, 06 Jan 2026 08:47:59 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: 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>,
 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 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@...asonboard.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);
> > >  

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)