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

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

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