[PATCH v3 0/4] media: dw100: Dynamic vertex map updates and fixes for PREEMPT_RT

[PATCH v3 0/4] media: dw100: Dynamic vertex map updates and fixes for PREEMPT_RT

[Stefan Klug]

Hi all,

This series contains a few updates to the dw100 dewarper. Patches 1-2
implement V4L2 requests support and dynamic updates of the vertex map.
This enables advanced use cases like interactive pan, zoom and rotate. A
first attempt to implement the dynamic update was posted a while back in
[1] but wasn't merged due to races in the implementation. By switching
to V4L2 requests the implementation is a lot simpler now.

Patch 3 fixes an issue on PREEMPT_RT enabled kernels.

Patch 4 does a bit of cleanup.

Best regards,
Stefan

[1] https://lore.kernel.org/linux-media/20241022063155.506191-1-umang.jain@ideasonboard.com/

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
Changes in v3:
- Split the irq handler into two halves
- Added cleanup patch 4/4
- Link to v2: https://lore.kernel.org/r/20260122-sklug-v6-16-topic-dw100-v3-1-dev-v2-0-ea1e29f18cf1@ideasonboard.com

Changes in v2:
- Use v4l2_m2m_buf_done_and_job_finish() for correct signal order
- Use dev_warn_once() instead of manual tracking
- Dropped threaded interrupt handler
- Renamed user_map_needs_update to user_map_is_dirty
- Link to v1: https://lore.kernel.org/r/20260105-sklug-v6-16-topic-dw100-v3-1-dev-v1-0-65af34d04fd8@ideasonboard.com

---
Stefan Klug (4):
      media: dw100: Implement V4L2 requests support
      media: dw100: Implement dynamic vertex map update
      media: dw100: Fix kernel oops with PREEMPT_RT enabled
      media: dw100: Merge dw100_device_run and dw100_start

 drivers/media/platform/nxp/dw100/dw100.c | 129 ++++++++++++++++++++-----------
 1 file changed, 86 insertions(+), 43 deletions(-)
---
base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb
change-id: 20260105-sklug-v6-16-topic-dw100-v3-1-dev-b856657db695

Best regards,
-- 
Stefan Klug <stefan.klug@ideasonboard.com>

[PATCH v3 1/4] media: dw100: Implement V4L2 requests support

[Stefan Klug]

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

---

Changes in v2:
- Use v4l2_m2m_buf_done_and_job_finish() to mark the buffers as done in
  the correct order.

Changes in v1:
- Moved v4l2_ctrl_request_complete into dw100_device_run
---
 drivers/media/platform/nxp/dw100/dw100.c | 49 +++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
index 4aaf9c3fff5397f0441944ee926f2c8ba6fc864a..1cb895da9912371a2b23ca62412c572d9cb75c00 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)
@@ -1058,7 +1077,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 +1087,13 @@ 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, curr_ctx->fh.m2m_ctx,
+					 buf_state);
 
 	dev_dbg(&dw_dev->pdev->dev, "Finishing transaction with%s error(s)\n",
 		with_error ? "" : "out");
@@ -1460,6 +1475,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);
+
 	dw100_start(ctx, src_buf, dst_buf);
 }
 
@@ -1467,6 +1488,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 +1604,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);

-- 
2.51.0

Re: [PATCH v3 1/4] media: dw100: Implement V4L2 requests support

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Thu, Jan 29, 2026 at 12:43:10PM +0100, Stefan Klug wrote:
> 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

Once this gets merged it won't be a patch any more :-) As commit
messages are written in an imperative mood style,

s/This patch adds/Add/

> prepare for dynamic vertex map updates.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Use v4l2_m2m_buf_done_and_job_finish() to mark the buffers as done in
>   the correct order.
> 
> Changes in v1:
> - Moved v4l2_ctrl_request_complete into dw100_device_run
> ---
>  drivers/media/platform/nxp/dw100/dw100.c | 49 +++++++++++++++++++++++++-------
>  1 file changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 4aaf9c3fff5397f0441944ee926f2c8ba6fc864a..1cb895da9912371a2b23ca62412c572d9cb75c00 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)
> @@ -1058,7 +1077,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 +1087,13 @@ 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, curr_ctx->fh.m2m_ctx,
> +					 buf_state);
>  
>  	dev_dbg(&dw_dev->pdev->dev, "Finishing transaction with%s error(s)\n",
>  		with_error ? "" : "out");
> @@ -1460,6 +1475,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);
>  

I'd add a comment here, as the complete() call before start() can be
confusing.

	/*
	 * Apply controls from the request to the device and copy back the value
	 * of volatile controls to the request. We can do the latter before
	 * starting the dewarper as no controls are updated as a result of the
	 * hardware operation.
	 /

It could be me not being familiar enough with the API, but I think
discussions during review of v2 showed that this confused other people
too.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	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);
> +
>  	dw100_start(ctx, src_buf, dst_buf);
>  }
>  
> @@ -1467,6 +1488,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 +1604,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,

Laurent Pinchart

Re: [PATCH v3 1/4] media: dw100: Implement V4L2 requests support

[Stefan Klug]

Hi Laurent,

Thank you for the review.

Quoting Laurent Pinchart (2026-02-10 18:50:35)
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Thu, Jan 29, 2026 at 12:43:10PM +0100, Stefan Klug wrote:
> > 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
> 
> Once this gets merged it won't be a patch any more :-) As commit
> messages are written in an imperative mood style,
> 
> s/This patch adds/Add/
> 
> > prepare for dynamic vertex map updates.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v2:
> > - Use v4l2_m2m_buf_done_and_job_finish() to mark the buffers as done in
> >   the correct order.
> > 
> > Changes in v1:
> > - Moved v4l2_ctrl_request_complete into dw100_device_run
> > ---
> >  drivers/media/platform/nxp/dw100/dw100.c | 49 +++++++++++++++++++++++++-------
> >  1 file changed, 38 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> > index 4aaf9c3fff5397f0441944ee926f2c8ba6fc864a..1cb895da9912371a2b23ca62412c572d9cb75c00 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)
> > @@ -1058,7 +1077,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 +1087,13 @@ 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, curr_ctx->fh.m2m_ctx,
> > +                                      buf_state);
> >  
> >       dev_dbg(&dw_dev->pdev->dev, "Finishing transaction with%s error(s)\n",
> >               with_error ? "" : "out");
> > @@ -1460,6 +1475,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);
> >  
> 
> I'd add a comment here, as the complete() call before start() can be
> confusing.
> 
>         /*
>          * Apply controls from the request to the device and copy back the value
>          * of volatile controls to the request. We can do the latter before
>          * starting the dewarper as no controls are updated as a result of the
>          * hardware operation.
>          /
> 
> It could be me not being familiar enough with the API, but I think
> discussions during review of v2 showed that this confused other people
> too.

I was just applying the comment to the code when I realized that I had
difficulties parsing it. Main cause is that it is just above the call to
v4l2_ctrl_request_setup() which is imho not the questionable call. Would
you be fine with a shorter comment above v4l2_ctrl_request_complete()
only?

	/*
         * As the hardware does not update any volatile controls, we can
         * complete control handling before starting the dewarper.
         */
	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
				   &ctx->hdl);

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks,
Stefan

> 
> > +     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);
> > +
> >       dw100_start(ctx, src_buf, dst_buf);
> >  }
> >  
> > @@ -1467,6 +1488,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 +1604,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,
> 
> Laurent Pinchart

Re: [PATCH v3 1/4] media: dw100: Implement V4L2 requests support

[Laurent Pinchart]

On Wed, Feb 25, 2026 at 08:23:35AM +0100, Stefan Klug wrote:
> Quoting Laurent Pinchart (2026-02-10 18:50:35)
> > On Thu, Jan 29, 2026 at 12:43:10PM +0100, Stefan Klug wrote:
> > > 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
> > 
> > Once this gets merged it won't be a patch any more :-) As commit
> > messages are written in an imperative mood style,
> > 
> > s/This patch adds/Add/
> > 
> > > prepare for dynamic vertex map updates.
> > > 
> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > > 
> > > ---
> > > 
> > > Changes in v2:
> > > - Use v4l2_m2m_buf_done_and_job_finish() to mark the buffers as done in
> > >   the correct order.
> > > 
> > > Changes in v1:
> > > - Moved v4l2_ctrl_request_complete into dw100_device_run
> > > ---
> > >  drivers/media/platform/nxp/dw100/dw100.c | 49 +++++++++++++++++++++++++-------
> > >  1 file changed, 38 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> > > index 4aaf9c3fff5397f0441944ee926f2c8ba6fc864a..1cb895da9912371a2b23ca62412c572d9cb75c00 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)
> > > @@ -1058,7 +1077,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 +1087,13 @@ 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, curr_ctx->fh.m2m_ctx,
> > > +                                      buf_state);
> > >  
> > >       dev_dbg(&dw_dev->pdev->dev, "Finishing transaction with%s error(s)\n",
> > >               with_error ? "" : "out");
> > > @@ -1460,6 +1475,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);
> > >  
> > 
> > I'd add a comment here, as the complete() call before start() can be
> > confusing.
> > 
> >         /*
> >          * Apply controls from the request to the device and copy back the value
> >          * of volatile controls to the request. We can do the latter before
> >          * starting the dewarper as no controls are updated as a result of the
> >          * hardware operation.
> >          /
> > 
> > It could be me not being familiar enough with the API, but I think
> > discussions during review of v2 showed that this confused other people
> > too.
> 
> I was just applying the comment to the code when I realized that I had
> difficulties parsing it. Main cause is that it is just above the call to
> v4l2_ctrl_request_setup() which is imho not the questionable call. Would
> you be fine with a shorter comment above v4l2_ctrl_request_complete()
> only?
> 
> 	/*
>          * As the hardware does not update any volatile controls, we can
>          * complete control handling before starting the dewarper.
>          */
> 	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> 				   &ctx->hdl);

That's fine with me.

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > > +     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);
> > > +
> > >       dw100_start(ctx, src_buf, dst_buf);
> > >  }
> > >  
> > > @@ -1467,6 +1488,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 +1604,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,

Laurent Pinchart

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

[Stefan Klug]

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>

---

Changes in v2:
- Replaced the manual warn once by dev_warn_once(). This changes the
  frequency from once per context to once per boot which was agreed to
  be enough.
- Renamed user_map_needs_update to user_map_is_dirty as it is more
  expressive and fits to user_map_is_set
- Fixed indentation issue found by Media CI
---
 drivers/media/platform/nxp/dw100/dw100.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
index 1cb895da9912371a2b23ca62412c572d9cb75c00..d2b1c62b52db47ea1d2242caaf334fff30c6f366 100644
--- a/drivers/media/platform/nxp/dw100/dw100.c
+++ b/drivers/media/platform/nxp/dw100/dw100.c
@@ -98,6 +98,7 @@ struct dw100_ctx {
 	unsigned int			map_width;
 	unsigned int			map_height;
 	bool				user_map_is_set;
+	bool				user_map_is_dirty;
 
 	/* Source and destination queue data */
 	struct dw100_q_data		q_data[2];
@@ -293,11 +294,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_is_dirty)
+		return;
+
+	memcpy(ctx->map, ctrl->p_cur.p_u32, ctx->map_size);
+	ctx->user_map_is_dirty = false;
 }
 
 /*
@@ -306,8 +311,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 +321,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_is_dirty = 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 +354,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_is_dirty = true;
 		break;
 	}
 
@@ -405,6 +409,7 @@ static void dw100_ctrl_dewarping_map_init(const struct v4l2_ctrl *ctrl,
 	}
 
 	ctx->user_map_is_set = false;
+	ctx->user_map_is_dirty = true;
 }
 
 static const struct v4l2_ctrl_type_ops dw100_ctrl_type_ops = {
@@ -1478,6 +1483,13 @@ 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_is_dirty)
+		dev_warn_once(&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

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

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Thu, Jan 29, 2026 at 12:43:11PM +0100, Stefan Klug wrote:
> 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>
> 
> ---
> 
> Changes in v2:
> - Replaced the manual warn once by dev_warn_once(). This changes the
>   frequency from once per context to once per boot which was agreed to
>   be enough.
> - Renamed user_map_needs_update to user_map_is_dirty as it is more
>   expressive and fits to user_map_is_set
> - Fixed indentation issue found by Media CI
> ---
>  drivers/media/platform/nxp/dw100/dw100.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 1cb895da9912371a2b23ca62412c572d9cb75c00..d2b1c62b52db47ea1d2242caaf334fff30c6f366 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -98,6 +98,7 @@ struct dw100_ctx {
>  	unsigned int			map_width;
>  	unsigned int			map_height;
>  	bool				user_map_is_set;
> +	bool				user_map_is_dirty;
>  
>  	/* Source and destination queue data */
>  	struct dw100_q_data		q_data[2];
> @@ -293,11 +294,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_is_dirty)
> +		return;
> +
> +	memcpy(ctx->map, ctrl->p_cur.p_u32, ctx->map_size);
> +	ctx->user_map_is_dirty = false;
>  }
>  
>  /*
> @@ -306,8 +311,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 +321,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_is_dirty = 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 +354,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_is_dirty = true;
>  		break;
>  	}
>  
> @@ -405,6 +409,7 @@ static void dw100_ctrl_dewarping_map_init(const struct v4l2_ctrl *ctrl,
>  	}
>  
>  	ctx->user_map_is_set = false;
> +	ctx->user_map_is_dirty = true;
>  }
>  
>  static const struct v4l2_ctrl_type_ops dw100_ctrl_type_ops = {
> @@ -1478,6 +1483,13 @@ 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_is_dirty)
> +		dev_warn_once(&ctx->dw_dev->pdev->dev,
> +			      "V4L2 requests are required to update the vertex map dynamically"

Missing \n

> +		);

The closing parenthesis goes at the end of the previous line.

With those small issues fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

-- 
Regards,

Laurent Pinchart

[PATCH v3 3/4] media: dw100: Fix kernel oops with PREEMPT_RT enabled

[Stefan Klug]

On kernels with PREEMPT_RT enabled, a "BUG: scheduling while atomic"
kernel oops occurs inside dw100_irq_handler -> vb2_buffer_done. This is
because vb2_buffer_done takes a spinlock which is not allowed within
interrupt context on PREEMPT_RT.

The first attempt to fix this was to just drop the IRQF_ONESHOT so that
the interrupt is handled threaded on PREEMPT_RT systems. This introduced
a new issue. The dw100 has an internal timeout counter that is gated by
the DW100_BUS_CTRL_AXI_MASTER_ENABLE bit. Depending on the time it takes
for the threaded handler to run and the geometry of the data being
processed it is possible to reach the timeout resulting in
DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT being set and "dw100
32e30000.dwe: Interrupt error: 0x1" errors in dmesg.

To properly fix that, split the interrupt into two halves, reset the
DW100_BUS_CTRL_AXI_MASTER_ENABLE bit in the hard interrupt handler and
do the v4l2 buffer handling in the threaded half. The IRQF_ONESHOT can
still be dropped as the interrupt gets disabled in the hard handler and
will only be reenabled on the next dw100_device_run which will not be
called before the current job has finished.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---

Thank you Xavier for the technical support and further details on the
interrupt bit.

Changes in v3:
- Split interrupt in two halves to prevent timeout error
- Dropped rby tags, as the patch changed substantially

Changes in v2:
- Dropped the IRQF_ONESHOT instead of making the interrupt handler
  threaded to fix the issue.
- I didn't keep the r-by tag from Nicolas as the solution is now a
  different one.
---
 drivers/media/platform/nxp/dw100/dw100.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
index d2b1c62b52db47ea1d2242caaf334fff30c6f366..46e3a7b74fb777aa479110a52229f36b8632db44 100644
--- a/drivers/media/platform/nxp/dw100/dw100.c
+++ b/drivers/media/platform/nxp/dw100/dw100.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/debugfs.h>
 #include <linux/interrupt.h>
+#include <linux/irqreturn.h>
 #include <linux/io.h>
 #include <linux/minmax.h>
 #include <linux/module.h>
@@ -74,6 +75,7 @@ struct dw100_device {
 	struct clk_bulk_data		*clks;
 	int				num_clks;
 	struct dentry			*debugfs_root;
+	bool				frame_failed;
 };
 
 struct dw100_q_data {
@@ -1406,7 +1408,8 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
 {
 	struct dw100_device *dw_dev = dev_id;
 	u32 pending_irqs, err_irqs, frame_done_irq;
-	bool with_error = true;
+
+	dw_dev->frame_failed = true;
 
 	pending_irqs = dw_hw_get_pending_irqs(dw_dev);
 	frame_done_irq = pending_irqs & DW100_INTERRUPT_STATUS_INT_FRAME_DONE;
@@ -1414,7 +1417,7 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
 
 	if (frame_done_irq) {
 		dev_dbg(&dw_dev->pdev->dev, "Frame done interrupt\n");
-		with_error = false;
+		dw_dev->frame_failed = false;
 		err_irqs &= ~DW100_INTERRUPT_STATUS_INT_ERR_STATUS
 			(DW100_INTERRUPT_STATUS_INT_ERR_FRAME_DONE);
 	}
@@ -1427,7 +1430,14 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
 	dw100_hw_clear_irq(dw_dev, pending_irqs |
 			   DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT);
 
-	dw100_job_finish(dw_dev, with_error);
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
+{
+	struct dw100_device *dw_dev = dev_id;
+
+	dw100_job_finish(dw_dev, dw_dev->frame_failed);
 
 	return IRQ_HANDLED;
 }
@@ -1593,8 +1603,9 @@ static int dw100_probe(struct platform_device *pdev)
 
 	pm_runtime_put_sync(&pdev->dev);
 
-	ret = devm_request_irq(&pdev->dev, irq, dw100_irq_handler, IRQF_ONESHOT,
-			       dev_name(&pdev->dev), dw_dev);
+	ret = devm_request_threaded_irq(&pdev->dev, irq, dw100_irq_handler,
+					dw100_irq_thread_fn, 0,
+					dev_name(&pdev->dev), dw_dev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
 		goto err_pm;

-- 
2.51.0

Re: [PATCH v3 3/4] media: dw100: Fix kernel oops with PREEMPT_RT enabled

[Xavier Roumegue (OSS)]

Hi Stefan,

On 1/29/26 12:43 PM, Stefan Klug wrote:
> On kernels with PREEMPT_RT enabled, a "BUG: scheduling while atomic"
> kernel oops occurs inside dw100_irq_handler -> vb2_buffer_done. This is
> because vb2_buffer_done takes a spinlock which is not allowed within
> interrupt context on PREEMPT_RT.
>
> The first attempt to fix this was to just drop the IRQF_ONESHOT so that
> the interrupt is handled threaded on PREEMPT_RT systems. This introduced
> a new issue. The dw100 has an internal timeout counter that is gated by
> the DW100_BUS_CTRL_AXI_MASTER_ENABLE bit. Depending on the time it takes
> for the threaded handler to run and the geometry of the data being
> processed it is possible to reach the timeout resulting in
> DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT being set and "dw100
> 32e30000.dwe: Interrupt error: 0x1" errors in dmesg.
>
> To properly fix that, split the interrupt into two halves, reset the
> DW100_BUS_CTRL_AXI_MASTER_ENABLE bit in the hard interrupt handler and
> do the v4l2 buffer handling in the threaded half. The IRQF_ONESHOT can
> still be dropped as the interrupt gets disabled in the hard handler and
> will only be reenabled on the next dw100_device_run which will not be
> called before the current job has finished.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Reviewed-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>


> ---
>
> Thank you Xavier for the technical support and further details on the
> interrupt bit.
Welcome :)

Regards,

  Xavier

>
> Changes in v3:
> - Split interrupt in two halves to prevent timeout error
> - Dropped rby tags, as the patch changed substantially
>
> Changes in v2:
> - Dropped the IRQF_ONESHOT instead of making the interrupt handler
>    threaded to fix the issue.
> - I didn't keep the r-by tag from Nicolas as the solution is now a
>    different one.
> ---
>   drivers/media/platform/nxp/dw100/dw100.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index d2b1c62b52db47ea1d2242caaf334fff30c6f366..46e3a7b74fb777aa479110a52229f36b8632db44 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -10,6 +10,7 @@
>   #include <linux/clk.h>
>   #include <linux/debugfs.h>
>   #include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
>   #include <linux/io.h>
>   #include <linux/minmax.h>
>   #include <linux/module.h>
> @@ -74,6 +75,7 @@ struct dw100_device {
>   	struct clk_bulk_data		*clks;
>   	int				num_clks;
>   	struct dentry			*debugfs_root;
> +	bool				frame_failed;
>   };
>   
>   struct dw100_q_data {
> @@ -1406,7 +1408,8 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
>   {
>   	struct dw100_device *dw_dev = dev_id;
>   	u32 pending_irqs, err_irqs, frame_done_irq;
> -	bool with_error = true;
> +
> +	dw_dev->frame_failed = true;
>   
>   	pending_irqs = dw_hw_get_pending_irqs(dw_dev);
>   	frame_done_irq = pending_irqs & DW100_INTERRUPT_STATUS_INT_FRAME_DONE;
> @@ -1414,7 +1417,7 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
>   
>   	if (frame_done_irq) {
>   		dev_dbg(&dw_dev->pdev->dev, "Frame done interrupt\n");
> -		with_error = false;
> +		dw_dev->frame_failed = false;
>   		err_irqs &= ~DW100_INTERRUPT_STATUS_INT_ERR_STATUS
>   			(DW100_INTERRUPT_STATUS_INT_ERR_FRAME_DONE);
>   	}
> @@ -1427,7 +1430,14 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
>   	dw100_hw_clear_irq(dw_dev, pending_irqs |
>   			   DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT);
>   
> -	dw100_job_finish(dw_dev, with_error);
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
> +{
> +	struct dw100_device *dw_dev = dev_id;
> +
> +	dw100_job_finish(dw_dev, dw_dev->frame_failed);
>   
>   	return IRQ_HANDLED;
>   }
> @@ -1593,8 +1603,9 @@ static int dw100_probe(struct platform_device *pdev)
>   
>   	pm_runtime_put_sync(&pdev->dev);
>   
> -	ret = devm_request_irq(&pdev->dev, irq, dw100_irq_handler, IRQF_ONESHOT,
> -			       dev_name(&pdev->dev), dw_dev);
> +	ret = devm_request_threaded_irq(&pdev->dev, irq, dw100_irq_handler,
> +					dw100_irq_thread_fn, 0,
> +					dev_name(&pdev->dev), dw_dev);
>   	if (ret < 0) {
>   		dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
>   		goto err_pm;
>

Re: [PATCH v3 3/4] media: dw100: Fix kernel oops with PREEMPT_RT enabled

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Thu, Jan 29, 2026 at 12:43:12PM +0100, Stefan Klug wrote:
> On kernels with PREEMPT_RT enabled, a "BUG: scheduling while atomic"
> kernel oops occurs inside dw100_irq_handler -> vb2_buffer_done. This is
> because vb2_buffer_done takes a spinlock which is not allowed within
> interrupt context on PREEMPT_RT.
> 
> The first attempt to fix this was to just drop the IRQF_ONESHOT so that
> the interrupt is handled threaded on PREEMPT_RT systems. This introduced
> a new issue. The dw100 has an internal timeout counter that is gated by
> the DW100_BUS_CTRL_AXI_MASTER_ENABLE bit. Depending on the time it takes
> for the threaded handler to run and the geometry of the data being
> processed it is possible to reach the timeout resulting in
> DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT being set and "dw100
> 32e30000.dwe: Interrupt error: 0x1" errors in dmesg.
> 
> To properly fix that, split the interrupt into two halves, reset the
> DW100_BUS_CTRL_AXI_MASTER_ENABLE bit in the hard interrupt handler and
> do the v4l2 buffer handling in the threaded half. The IRQF_ONESHOT can
> still be dropped as the interrupt gets disabled in the hard handler and
> will only be reenabled on the next dw100_device_run which will not be
> called before the current job has finished.

It worries me a bit that we're essentially relying on luck to avoid the
timeout. I'm also wondering if we couldn't just ignore the timeout
interrupt, but that's something we should investigate separately. This
patch fixes a real issue without any real drawback, so I think we can
merge it as a first step.

> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
> 
> Thank you Xavier for the technical support and further details on the
> interrupt bit.
> 
> Changes in v3:
> - Split interrupt in two halves to prevent timeout error
> - Dropped rby tags, as the patch changed substantially
> 
> Changes in v2:
> - Dropped the IRQF_ONESHOT instead of making the interrupt handler
>   threaded to fix the issue.
> - I didn't keep the r-by tag from Nicolas as the solution is now a
>   different one.
> ---
>  drivers/media/platform/nxp/dw100/dw100.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index d2b1c62b52db47ea1d2242caaf334fff30c6f366..46e3a7b74fb777aa479110a52229f36b8632db44 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk.h>
>  #include <linux/debugfs.h>
>  #include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
>  #include <linux/io.h>

irqreturn goes after io

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  #include <linux/minmax.h>
>  #include <linux/module.h>
> @@ -74,6 +75,7 @@ struct dw100_device {
>  	struct clk_bulk_data		*clks;
>  	int				num_clks;
>  	struct dentry			*debugfs_root;
> +	bool				frame_failed;
>  };
>  
>  struct dw100_q_data {
> @@ -1406,7 +1408,8 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
>  {
>  	struct dw100_device *dw_dev = dev_id;
>  	u32 pending_irqs, err_irqs, frame_done_irq;
> -	bool with_error = true;
> +
> +	dw_dev->frame_failed = true;
>  
>  	pending_irqs = dw_hw_get_pending_irqs(dw_dev);
>  	frame_done_irq = pending_irqs & DW100_INTERRUPT_STATUS_INT_FRAME_DONE;
> @@ -1414,7 +1417,7 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
>  
>  	if (frame_done_irq) {
>  		dev_dbg(&dw_dev->pdev->dev, "Frame done interrupt\n");
> -		with_error = false;
> +		dw_dev->frame_failed = false;
>  		err_irqs &= ~DW100_INTERRUPT_STATUS_INT_ERR_STATUS
>  			(DW100_INTERRUPT_STATUS_INT_ERR_FRAME_DONE);
>  	}
> @@ -1427,7 +1430,14 @@ static irqreturn_t dw100_irq_handler(int irq, void *dev_id)
>  	dw100_hw_clear_irq(dw_dev, pending_irqs |
>  			   DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT);
>  
> -	dw100_job_finish(dw_dev, with_error);
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
> +{
> +	struct dw100_device *dw_dev = dev_id;
> +
> +	dw100_job_finish(dw_dev, dw_dev->frame_failed);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -1593,8 +1603,9 @@ static int dw100_probe(struct platform_device *pdev)
>  
>  	pm_runtime_put_sync(&pdev->dev);
>  
> -	ret = devm_request_irq(&pdev->dev, irq, dw100_irq_handler, IRQF_ONESHOT,
> -			       dev_name(&pdev->dev), dw_dev);
> +	ret = devm_request_threaded_irq(&pdev->dev, irq, dw100_irq_handler,
> +					dw100_irq_thread_fn, 0,
> +					dev_name(&pdev->dev), dw_dev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
>  		goto err_pm;

-- 
Regards,

Laurent Pinchart

[PATCH v3 4/4] media: dw100: Merge dw100_device_run and dw100_start

[Stefan Klug]

The dw100_start() function is only called from dw100_device_run(). As
both functions are not too big, move the code directly into
dw100_device_run() and drop dw100_start() to improve readability.

This patch contains no functional changes.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Changes in v3:
- Added this patch as proposed in the review of v1
---
 drivers/media/platform/nxp/dw100/dw100.c | 61 ++++++++++++++------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
index 46e3a7b74fb777aa479110a52229f36b8632db44..c7c4249f5769467fb2b1f3c87f5685c4463a0a9d 100644
--- a/drivers/media/platform/nxp/dw100/dw100.c
+++ b/drivers/media/platform/nxp/dw100/dw100.c
@@ -1442,25 +1442,42 @@ static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
-			struct vb2_v4l2_buffer *out_vb)
+static void dw100_device_run(void *priv)
 {
+	struct dw100_ctx *ctx = priv;
 	struct dw100_device *dw_dev = ctx->dw_dev;
+	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 
-	out_vb->sequence =
-		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)->sequence++;
-	in_vb->sequence =
+	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);
+
+	if (src_buf->vb2_buf.req_obj.req)
+		dw100_update_mapping(ctx);
+	else if (ctx->user_map_is_dirty)
+		dev_warn_once(&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);
+
+	src_buf->sequence =
 		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)->sequence++;
+	dst_buf->sequence =
+		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)->sequence++;
 
-	dev_dbg(&ctx->dw_dev->pdev->dev,
+	dev_dbg(&dw_dev->pdev->dev,
 		"Starting queues %p->%p, sequence %u->%u\n",
 		v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
 				V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE),
 		v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
 				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
-		in_vb->sequence, out_vb->sequence);
+		src_buf->sequence, dst_buf->sequence);
 
-	v4l2_m2m_buf_copy_metadata(in_vb, out_vb);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
 
 	/* Now, let's deal with hardware ... */
 	dw100_hw_master_bus_disable(dw_dev);
@@ -1469,10 +1486,10 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
 	dw100_hw_set_src_crop(dw_dev, &ctx->q_data[DW100_QUEUE_SRC],
 			      &ctx->q_data[DW100_QUEUE_DST]);
 	dw100_hw_set_source(dw_dev, &ctx->q_data[DW100_QUEUE_SRC],
-			    &in_vb->vb2_buf);
+			    &src_buf->vb2_buf);
 	dw100_hw_set_destination(dw_dev, &ctx->q_data[DW100_QUEUE_DST],
 				 ctx->q_data[DW100_QUEUE_SRC].fmt,
-				 &out_vb->vb2_buf);
+				 &dst_buf->vb2_buf);
 	dw100_hw_set_mapping(dw_dev, ctx->map_dma,
 			     ctx->map_width, ctx->map_height);
 	dw100_hw_enable_irq(dw_dev);
@@ -1482,30 +1499,6 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
 	dw100_hw_master_bus_enable(dw_dev);
 }
 
-static void dw100_device_run(void *priv)
-{
-	struct dw100_ctx *ctx = priv;
-	struct vb2_v4l2_buffer *src_buf, *dst_buf;
-
-	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);
-
-	if (src_buf->vb2_buf.req_obj.req)
-		dw100_update_mapping(ctx);
-	else if (ctx->user_map_is_dirty)
-		dev_warn_once(&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);
-
-	dw100_start(ctx, src_buf, dst_buf);
-}
-
 static const struct v4l2_m2m_ops dw100_m2m_ops = {
 	.device_run	= dw100_device_run,
 };

-- 
2.51.0

Re: [PATCH v3 4/4] media: dw100: Merge dw100_device_run and dw100_start

[Xavier Roumegue (OSS)]

On 1/29/26 12:43 PM, Stefan Klug wrote:
> The dw100_start() function is only called from dw100_device_run(). As
> both functions are not too big, move the code directly into
> dw100_device_run() and drop dw100_start() to improve readability.
>
> This patch contains no functional changes.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Reviewed-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>


>
> ---
>
> Changes in v3:
> - Added this patch as proposed in the review of v1
> ---
>   drivers/media/platform/nxp/dw100/dw100.c | 61 ++++++++++++++------------------
>   1 file changed, 27 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 46e3a7b74fb777aa479110a52229f36b8632db44..c7c4249f5769467fb2b1f3c87f5685c4463a0a9d 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -1442,25 +1442,42 @@ static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
>   	return IRQ_HANDLED;
>   }
>   
> -static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> -			struct vb2_v4l2_buffer *out_vb)
> +static void dw100_device_run(void *priv)
>   {
> +	struct dw100_ctx *ctx = priv;
>   	struct dw100_device *dw_dev = ctx->dw_dev;
> +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
>   
> -	out_vb->sequence =
> -		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)->sequence++;
> -	in_vb->sequence =
> +	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);
> +
> +	if (src_buf->vb2_buf.req_obj.req)
> +		dw100_update_mapping(ctx);
> +	else if (ctx->user_map_is_dirty)
> +		dev_warn_once(&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);
> +
> +	src_buf->sequence =
>   		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)->sequence++;
> +	dst_buf->sequence =
> +		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)->sequence++;
>   
> -	dev_dbg(&ctx->dw_dev->pdev->dev,
> +	dev_dbg(&dw_dev->pdev->dev,
>   		"Starting queues %p->%p, sequence %u->%u\n",
>   		v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
>   				V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE),
>   		v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
>   				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
> -		in_vb->sequence, out_vb->sequence);
> +		src_buf->sequence, dst_buf->sequence);
>   
> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
>   
>   	/* Now, let's deal with hardware ... */
>   	dw100_hw_master_bus_disable(dw_dev);
> @@ -1469,10 +1486,10 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
>   	dw100_hw_set_src_crop(dw_dev, &ctx->q_data[DW100_QUEUE_SRC],
>   			      &ctx->q_data[DW100_QUEUE_DST]);
>   	dw100_hw_set_source(dw_dev, &ctx->q_data[DW100_QUEUE_SRC],
> -			    &in_vb->vb2_buf);
> +			    &src_buf->vb2_buf);
>   	dw100_hw_set_destination(dw_dev, &ctx->q_data[DW100_QUEUE_DST],
>   				 ctx->q_data[DW100_QUEUE_SRC].fmt,
> -				 &out_vb->vb2_buf);
> +				 &dst_buf->vb2_buf);
>   	dw100_hw_set_mapping(dw_dev, ctx->map_dma,
>   			     ctx->map_width, ctx->map_height);
>   	dw100_hw_enable_irq(dw_dev);
> @@ -1482,30 +1499,6 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
>   	dw100_hw_master_bus_enable(dw_dev);
>   }
>   
> -static void dw100_device_run(void *priv)
> -{
> -	struct dw100_ctx *ctx = priv;
> -	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> -
> -	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);
> -
> -	if (src_buf->vb2_buf.req_obj.req)
> -		dw100_update_mapping(ctx);
> -	else if (ctx->user_map_is_dirty)
> -		dev_warn_once(&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);
> -
> -	dw100_start(ctx, src_buf, dst_buf);
> -}
> -
>   static const struct v4l2_m2m_ops dw100_m2m_ops = {
>   	.device_run	= dw100_device_run,
>   };
>

Re: [PATCH v3 4/4] media: dw100: Merge dw100_device_run and dw100_start

[Laurent Pinchart]

On Thu, Jan 29, 2026 at 12:43:13PM +0100, Stefan Klug wrote:
> The dw100_start() function is only called from dw100_device_run(). As
> both functions are not too big, move the code directly into
> dw100_device_run() and drop dw100_start() to improve readability.
> 
> This patch contains no functional changes.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
> Changes in v3:
> - Added this patch as proposed in the review of v1
> ---
>  drivers/media/platform/nxp/dw100/dw100.c | 61 ++++++++++++++------------------
>  1 file changed, 27 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 46e3a7b74fb777aa479110a52229f36b8632db44..c7c4249f5769467fb2b1f3c87f5685c4463a0a9d 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -1442,25 +1442,42 @@ static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> -			struct vb2_v4l2_buffer *out_vb)
> +static void dw100_device_run(void *priv)
>  {
> +	struct dw100_ctx *ctx = priv;
>  	struct dw100_device *dw_dev = ctx->dw_dev;
> +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
>  
> -	out_vb->sequence =
> -		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)->sequence++;
> -	in_vb->sequence =
> +	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);
> +
> +	if (src_buf->vb2_buf.req_obj.req)
> +		dw100_update_mapping(ctx);
> +	else if (ctx->user_map_is_dirty)
> +		dev_warn_once(&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);
> +
> +	src_buf->sequence =
>  		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)->sequence++;
> +	dst_buf->sequence =
> +		dw100_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)->sequence++;
>  
> -	dev_dbg(&ctx->dw_dev->pdev->dev,
> +	dev_dbg(&dw_dev->pdev->dev,
>  		"Starting queues %p->%p, sequence %u->%u\n",
>  		v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
>  				V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE),
>  		v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
>  				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
> -		in_vb->sequence, out_vb->sequence);
> +		src_buf->sequence, dst_buf->sequence);
>  
> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
>  
>  	/* Now, let's deal with hardware ... */
>  	dw100_hw_master_bus_disable(dw_dev);
> @@ -1469,10 +1486,10 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
>  	dw100_hw_set_src_crop(dw_dev, &ctx->q_data[DW100_QUEUE_SRC],
>  			      &ctx->q_data[DW100_QUEUE_DST]);
>  	dw100_hw_set_source(dw_dev, &ctx->q_data[DW100_QUEUE_SRC],
> -			    &in_vb->vb2_buf);
> +			    &src_buf->vb2_buf);
>  	dw100_hw_set_destination(dw_dev, &ctx->q_data[DW100_QUEUE_DST],
>  				 ctx->q_data[DW100_QUEUE_SRC].fmt,
> -				 &out_vb->vb2_buf);
> +				 &dst_buf->vb2_buf);
>  	dw100_hw_set_mapping(dw_dev, ctx->map_dma,
>  			     ctx->map_width, ctx->map_height);
>  	dw100_hw_enable_irq(dw_dev);
> @@ -1482,30 +1499,6 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
>  	dw100_hw_master_bus_enable(dw_dev);
>  }
>  
> -static void dw100_device_run(void *priv)
> -{
> -	struct dw100_ctx *ctx = priv;
> -	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> -
> -	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);
> -
> -	if (src_buf->vb2_buf.req_obj.req)
> -		dw100_update_mapping(ctx);
> -	else if (ctx->user_map_is_dirty)
> -		dev_warn_once(&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);
> -
> -	dw100_start(ctx, src_buf, dst_buf);
> -}
> -
>  static const struct v4l2_m2m_ops dw100_m2m_ops = {
>  	.device_run	= dw100_device_run,
>  };

-- 
Regards,

Laurent Pinchart