| 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: <40dfd12b585681d8edf9641967160c086cc120c3.camel@ndufresne.ca>
Date: Mon, 05 Jan 2026 13:46:46 -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 1/4] media: dw100: Implement V4L2 requests support
Hi,
Le lundi 05 janvier 2026 à 12:35 +0100, Stefan Klug a écrit :
> The dw100 dewarper hardware present on the NXP i.MX8MP allows very
> flexible dewarping using a freely configurable vertex map. Aside from
> lens dewarping the vertex map can be used to implement things like
> arbitrary zoom, pan and rotation. The current driver supports setting
> that vertex map before calling VIDIOC_STREAMON.
>
> To control above mentioned features during streaming it is necessary to
> update the vertex map dynamically. To do that in a race free manner V4L2
> requests support is required. This patch adds V4L2 requests support to
> prepare for dynamic vertex map updates.
>
> Signed-off-by: Stefan Klug <stefan.klug@...asonboard.com>
>
> ---
>
> Changes in v1:
> - Moved v4l2_ctrl_request_complete into dw100_device_run
> ---
> drivers/media/platform/nxp/dw100/dw100.c | 41 ++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 4aaf9c3fff5397f0441944ee926f2c8ba6fc864a..7f14b82c15a071605c124dbb868f8003856c4fc0 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -459,6 +459,15 @@ static int dw100_queue_setup(struct vb2_queue *vq,
> return 0;
> }
>
> +static int dw100_buf_out_validate(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> + vbuf->field = V4L2_FIELD_NONE;
> +
> + return 0;
> +}
> +
> static int dw100_buf_prepare(struct vb2_buffer *vb)
> {
> unsigned int i;
> @@ -500,6 +509,13 @@ static void dw100_buf_queue(struct vb2_buffer *vb)
> v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
> }
>
> +static void dw100_buf_request_complete(struct vb2_buffer *vb)
> +{
> + struct dw100_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
> +}
> +
> static void dw100_return_all_buffers(struct vb2_queue *q,
> enum vb2_buffer_state state)
> {
> @@ -553,11 +569,13 @@ static void dw100_stop_streaming(struct vb2_queue *q)
> }
>
> static const struct vb2_ops dw100_qops = {
> - .queue_setup = dw100_queue_setup,
> - .buf_prepare = dw100_buf_prepare,
> - .buf_queue = dw100_buf_queue,
> - .start_streaming = dw100_start_streaming,
> - .stop_streaming = dw100_stop_streaming,
> + .queue_setup = dw100_queue_setup,
> + .buf_out_validate = dw100_buf_out_validate,
> + .buf_prepare = dw100_buf_prepare,
> + .buf_queue = dw100_buf_queue,
> + .start_streaming = dw100_start_streaming,
> + .stop_streaming = dw100_stop_streaming,
> + .buf_request_complete = dw100_buf_request_complete,
> };
>
> static int dw100_m2m_queue_init(void *priv, struct vb2_queue *src_vq,
> @@ -575,6 +593,7 @@ static int dw100_m2m_queue_init(void *priv, struct vb2_queue *src_vq,
> src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> src_vq->lock = &ctx->vq_mutex;
> src_vq->dev = ctx->dw_dev->v4l2_dev.dev;
> + src_vq->supports_requests = true;
>
> ret = vb2_queue_init(src_vq);
> if (ret)
> @@ -1460,6 +1479,12 @@ static void dw100_device_run(void *priv)
> src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> + v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
> + &ctx->hdl);
> +
> + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> + &ctx->hdl);
The request should always be signalled last, so nothing wrong with applying the
controls as soon as possible in this case. Complete is a bit of a miss-leading
name, this function actually changes the global controls value (apply) and
removes its participation in request completion. Since the OUTPUT buffer for
that request is still queued, the request is not signalled yet.
But you have to flip over the order to buffer signalling in dw100_job_finish()
though. My recommendation is to use the helper
v4l2_m2m_buf_done_and_job_finish(). Something like this (not tested):
diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
index 4aaf9c3fff53..c5f9ee238345 100644
--- a/drivers/media/platform/nxp/dw100/dw100.c
+++ b/drivers/media/platform/nxp/dw100/dw100.c
@@ -1058,7 +1058,6 @@ static const struct v4l2_ioctl_ops dw100_ioctl_ops = {
static void dw100_job_finish(struct dw100_device *dw_dev, bool with_error)
{
struct dw100_ctx *curr_ctx;
- struct vb2_v4l2_buffer *src_vb, *dst_vb;
enum vb2_buffer_state buf_state;
curr_ctx = v4l2_m2m_get_curr_priv(dw_dev->m2m_dev);
@@ -1069,16 +1068,12 @@ static void dw100_job_finish(struct dw100_device *dw_dev, bool with_error)
return;
}
- src_vb = v4l2_m2m_src_buf_remove(curr_ctx->fh.m2m_ctx);
- dst_vb = v4l2_m2m_dst_buf_remove(curr_ctx->fh.m2m_ctx);
-
if (likely(!with_error))
buf_state = VB2_BUF_STATE_DONE;
else
buf_state = VB2_BUF_STATE_ERROR;
- v4l2_m2m_buf_done(src_vb, buf_state);
- v4l2_m2m_buf_done(dst_vb, buf_state);
+ v4l2_m2m_buf_done_and_job_finish(dw_dev->m2m_dev, buf_state);
dev_dbg(&dw_dev->pdev->dev, "Finishing transaction with%s error(s)\n",
with_error ? "" : "out");
You might be tempted to keep the logical order, and move the
v4l2_ctrl_request_complete() call into dw100_job_finish(), unfortunately this
does not work, since nothing mandate that any control was included in this
request, and that will lead to calling v4l2_ctrl_request_complete() on an
already completed request. Since its a single function HW, I don't see why you'd
want this, but it would require the manual request completion.
> +
> dw100_start(ctx, src_buf, dst_buf);
nit: I really don't see why this is a separate function ...
> }
>
> @@ -1467,6 +1492,11 @@ static const struct v4l2_m2m_ops dw100_m2m_ops = {
> .device_run = dw100_device_run,
> };
>
> +static const struct media_device_ops dw100_m2m_media_ops = {
> + .req_validate = vb2_request_validate,
> + .req_queue = v4l2_m2m_request_queue,
> +};
> +
> static struct video_device *dw100_init_video_device(struct dw100_device *dw_dev)
> {
> struct video_device *vfd = &dw_dev->vfd;
> @@ -1578,6 +1608,7 @@ static int dw100_probe(struct platform_device *pdev)
> dw_dev->mdev.dev = &pdev->dev;
> strscpy(dw_dev->mdev.model, "dw100", sizeof(dw_dev->mdev.model));
> media_device_init(&dw_dev->mdev);
> + dw_dev->mdev.ops = &dw100_m2m_media_ops;
> dw_dev->v4l2_dev.mdev = &dw_dev->mdev;
>
> ret = video_register_device(vfd, VFL_TYPE_VIDEO, -1);
regards,
Nicolas
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists