linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices
@ 2025-10-09 11:11 Laurent Pinchart
  2025-10-09 13:04 ` Nicolas Dufresne
  2025-10-09 14:40 ` Frank Li
  0 siblings, 2 replies; 4+ messages in thread
From: Laurent Pinchart @ 2025-10-09 11:11 UTC (permalink / raw)
  To: linux-media, imx, linux-arm-kernel, linux-mediatek,
	linux-rockchip, linux-staging, linux-stm32, linux-sunxi
  Cc: Nicolas Dufresne, Andrzej Pietrasiewicz, Bin Liu, Ezequiel Garcia,
	Hans Verkuil, Jacob Chen, Jernej Skrabec, Jiasheng Jiang,
	Matthew Majewski, Mirela Rabulea, Neil Armstrong, Philipp Zabel,
	Shuah Khan, Steve Longerbeam, Uwe Kleine-König,
	Xavier Roumegue

The v4l2_m2m_buf_copy_metadata() function takes a boolean
copy_frame_flags argument. When true, it causes the function to copy the
V4L2_BUF_FLAG_KEYFRAME, V4L2_BUF_FLAG_BFRAME and V4L2_BUF_FLAG_PFRAME
flags from the output buffer to the capture buffer.

Many frame-based M2M drivers (e.g. for JPEG encoders, scalers,
dewarpers, 2D blenders, ...) set the argument to true, while the frame
flags are not applicable to those drivers as they have no concept of
key, B or P frames. Set the argument to false to avoid further
cargo-cult mistakes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c     | 4 ++--
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c   | 4 ++--
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c   | 4 ++--
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c      | 2 +-
 drivers/media/platform/nxp/dw100/dw100.c                 | 2 +-
 drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c           | 2 +-
 drivers/media/platform/rockchip/rga/rga.c                | 2 +-
 drivers/media/platform/st/stm32/dma2d/dma2d.c            | 2 +-
 drivers/media/platform/sunxi/sun8i-di/sun8i-di.c         | 2 +-
 drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
 drivers/media/test-drivers/vim2m.c                       | 2 +-
 drivers/staging/media/imx/imx-media-csc-scaler.c         | 2 +-
 12 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 35c70ec3ad2c..6bd5036430dc 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1619,7 +1619,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
 	if (!dst_buf)
 		goto getbuf_fail;
 
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	mtk_jpegenc_set_hw_param(ctx, hw_id, src_buf, dst_buf);
 	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
@@ -1715,7 +1715,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 	if (!dst_buf)
 		goto getbuf_fail;
 
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
 	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
 
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
index e78e1d11093c..556865100872 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
@@ -530,7 +530,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 
 	src_buf = cjpeg->hw_param.src_buffer;
 	dst_buf = cjpeg->hw_param.dst_buffer;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	mtk_jpeg_dec_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
@@ -560,7 +560,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	ctx = jpeg->hw_param.curr_ctx;
 	src_buf = jpeg->hw_param.src_buffer;
 	dst_buf = jpeg->hw_param.dst_buffer;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
 	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
index 9ab27aee302a..4c8427b3c384 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
@@ -261,7 +261,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 
 	src_buf = cjpeg->hw_param.src_buffer;
 	dst_buf = cjpeg->hw_param.dst_buffer;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	mtk_jpeg_enc_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
@@ -289,7 +289,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	ctx = jpeg->hw_param.curr_ctx;
 	src_buf = jpeg->hw_param.src_buffer;
 	dst_buf = jpeg->hw_param.dst_buffer;
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
 		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
index 59ce5cce0698..dba46a69c6be 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
@@ -51,7 +51,7 @@ static void mdp_m2m_process_done(void *priv, int vb_state)
 	ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
 	src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
 	dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
-	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
+	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, false);
 
 	v4l2_m2m_buf_done(src_vbuf, vb_state);
 	v4l2_m2m_buf_done(dst_vbuf, vb_state);
diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
index 7a0ee44d9e1f..b73302d54635 100644
--- a/drivers/media/platform/nxp/dw100/dw100.c
+++ b/drivers/media/platform/nxp/dw100/dw100.c
@@ -1483,7 +1483,7 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
 				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
 		in_vb->sequence, out_vb->sequence);
 
-	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
+	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
 
 	/* Now, let's deal with hardware ... */
 	dw100_hw_master_bus_disable(dw_dev);
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 37e0670f98c5..e1dda1e834e4 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -1537,7 +1537,7 @@ static void mxc_jpeg_device_run(void *priv)
 	src_buf->sequence = q_data_out->sequence++;
 	dst_buf->sequence = q_data_cap->sequence++;
 
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	jpeg_src_buf = vb2_to_mxc_buf(&src_buf->vb2_buf);
 	if (q_data_cap->fmt->mem_planes != dst_buf->vb2_buf.num_planes) {
diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 48b88da59da0..075b684fb178 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -75,7 +75,7 @@ static irqreturn_t rga_isr(int irq, void *prv)
 		WARN_ON(!src);
 		WARN_ON(!dst);
 
-		v4l2_m2m_buf_copy_metadata(src, dst, true);
+		v4l2_m2m_buf_copy_metadata(src, dst, false);
 
 		dst->sequence = ctx->csequence++;
 
diff --git a/drivers/media/platform/st/stm32/dma2d/dma2d.c b/drivers/media/platform/st/stm32/dma2d/dma2d.c
index 643913adc1f3..4184bdb96e6d 100644
--- a/drivers/media/platform/st/stm32/dma2d/dma2d.c
+++ b/drivers/media/platform/st/stm32/dma2d/dma2d.c
@@ -483,7 +483,7 @@ static void device_run(void *prv)
 
 	src->sequence = frm_out->sequence++;
 	dst->sequence = frm_cap->sequence++;
-	v4l2_m2m_buf_copy_metadata(src, dst, true);
+	v4l2_m2m_buf_copy_metadata(src, dst, false);
 
 	if (clk_enable(dev->gate))
 		goto end;
diff --git a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
index 3e7f2df70408..11a6c7f5212e 100644
--- a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
+++ b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
@@ -71,7 +71,7 @@ static void deinterlace_device_run(void *priv)
 	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
-	v4l2_m2m_buf_copy_metadata(src, dst, true);
+	v4l2_m2m_buf_copy_metadata(src, dst, false);
 
 	deinterlace_write(dev, DEINTERLACE_MOD_ENABLE,
 			  DEINTERLACE_MOD_ENABLE_EN);
diff --git a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
index abd10b218aa1..f6e2f11a20dd 100644
--- a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
+++ b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
@@ -70,7 +70,7 @@ static void rotate_device_run(void *priv)
 	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
-	v4l2_m2m_buf_copy_metadata(src, dst, true);
+	v4l2_m2m_buf_copy_metadata(src, dst, false);
 
 	val = ROTATE_GLB_CTL_MODE(ROTATE_MODE_COPY_ROTATE);
 	if (ctx->hflip)
diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-drivers/vim2m.c
index dc82830a35a5..3e8476165721 100644
--- a/drivers/media/test-drivers/vim2m.c
+++ b/drivers/media/test-drivers/vim2m.c
@@ -482,7 +482,7 @@ static int device_process(struct vim2m_ctx *ctx,
 
 	out_vb->sequence = q_data_out->sequence++;
 	in_vb->sequence = q_data_in->sequence++;
-	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
+	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
 
 	if (ctx->mode & MEM2MEM_VFLIP) {
 		start = height - 1;
diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c b/drivers/staging/media/imx/imx-media-csc-scaler.c
index 19fd31cb9bb0..770ba3fbaba2 100644
--- a/drivers/staging/media/imx/imx-media-csc-scaler.c
+++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
@@ -96,7 +96,7 @@ static void ipu_ic_pp_complete(struct ipu_image_convert_run *run, void *_ctx)
 	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 
-	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
 
 	src_buf->sequence = ctx->sequence++;
 	dst_buf->sequence = src_buf->sequence;

base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
-- 
Regards,

Laurent Pinchart



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices
  2025-10-09 11:11 [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices Laurent Pinchart
@ 2025-10-09 13:04 ` Nicolas Dufresne
  2025-10-15  8:02   ` Hans Verkuil
  2025-10-09 14:40 ` Frank Li
  1 sibling, 1 reply; 4+ messages in thread
From: Nicolas Dufresne @ 2025-10-09 13:04 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media, imx, linux-arm-kernel,
	linux-mediatek, linux-rockchip, linux-staging, linux-stm32,
	linux-sunxi
  Cc: Andrzej Pietrasiewicz, Bin Liu, Ezequiel Garcia, Hans Verkuil,
	Jacob Chen, Jernej Skrabec, Jiasheng Jiang, Matthew Majewski,
	Mirela Rabulea, Neil Armstrong, Philipp Zabel, Shuah Khan,
	Steve Longerbeam, Uwe Kleine-König, Xavier Roumegue

[-- Attachment #1: Type: text/plain, Size: 11810 bytes --]

Hi,

Le jeudi 09 octobre 2025 à 14:11 +0300, Laurent Pinchart a écrit :
> The v4l2_m2m_buf_copy_metadata() function takes a boolean
> copy_frame_flags argument. When true, it causes the function to copy the
> V4L2_BUF_FLAG_KEYFRAME, V4L2_BUF_FLAG_BFRAME and V4L2_BUF_FLAG_PFRAME
> flags from the output buffer to the capture buffer.
> 
> Many frame-based M2M drivers (e.g. for JPEG encoders, scalers,
> dewarpers, 2D blenders, ...) set the argument to true, while the frame
> flags are not applicable to those drivers as they have no concept of
> key, B or P frames. Set the argument to false to avoid further
> cargo-cult mistakes.

There is no use cases in any upstream driver for copying them over (even in
Tegra 20 decoder driver). KEY/P/B are properties of bitstream buffer in some
formats. Once decoded, this is no longer a property of the video frame and
should be discarded. So even decoder supporting that feature should not copy
them over.

I'd remove the boolean completely to be fair, this is always inappropriate to
set this to true. I'd be fine doing it in two steps, but then the list should be
completed, see below:

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c     | 4 ++--
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c   | 4 ++--
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c   | 4 ++--
>  drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c      | 2 +-
>  drivers/media/platform/nxp/dw100/dw100.c                 | 2 +-
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c           | 2 +-
>  drivers/media/platform/rockchip/rga/rga.c                | 2 +-
>  drivers/media/platform/st/stm32/dma2d/dma2d.c            | 2 +-
>  drivers/media/platform/sunxi/sun8i-di/sun8i-di.c         | 2 +-
>  drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
>  drivers/media/test-drivers/vim2m.c                       | 2 +-
>  drivers/staging/media/imx/imx-media-csc-scaler.c         | 2 +-

Missing:

- drivers/media/platform/amphion/vdec.c
- drivers/media/platform/mediatek/vcodec/decoder/vdec/*
- drivers/media/platform/nvidia/tegra-vde/h264.c
- drivers/media/platform/rockchip/rkvdec/rkvdec.c
- drivers/media/platform/verisilicon/hantro_drv.c
- drivers/staging/media/sunxi/cedrus/cedrus_dec.c

cheers,
Nicolas

>  12 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 35c70ec3ad2c..6bd5036430dc 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1619,7 +1619,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  	if (!dst_buf)
>  		goto getbuf_fail;
>  
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	mtk_jpegenc_set_hw_param(ctx, hw_id, src_buf, dst_buf);
>  	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
> @@ -1715,7 +1715,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  	if (!dst_buf)
>  		goto getbuf_fail;
>  
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
>  	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
>  
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> index e78e1d11093c..556865100872 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -530,7 +530,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct
> *work)
>  
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
> @@ -560,7 +560,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq,
> void *priv)
>  	ctx = jpeg->hw_param.curr_ctx;
>  	src_buf = jpeg->hw_param.src_buffer;
>  	dst_buf = jpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
>  	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> index 9ab27aee302a..4c8427b3c384 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -261,7 +261,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct
> *work)
>  
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	mtk_jpeg_enc_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
> @@ -289,7 +289,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq,
> void *priv)
>  	ctx = jpeg->hw_param.curr_ctx;
>  	src_buf = jpeg->hw_param.src_buffer;
>  	dst_buf = jpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
>  		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> index 59ce5cce0698..dba46a69c6be 100644
> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> @@ -51,7 +51,7 @@ static void mdp_m2m_process_done(void *priv, int vb_state)
>  	ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
>  	src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
>  	dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
> -	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
> +	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, false);
>  
>  	v4l2_m2m_buf_done(src_vbuf, vb_state);
>  	v4l2_m2m_buf_done(dst_vbuf, vb_state);
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c
> b/drivers/media/platform/nxp/dw100/dw100.c
> index 7a0ee44d9e1f..b73302d54635 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -1483,7 +1483,7 @@ static void dw100_start(struct dw100_ctx *ctx, struct
> vb2_v4l2_buffer *in_vb,
>  				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
>  		in_vb->sequence, out_vb->sequence);
>  
> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
> +	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>  
>  	/* Now, let's deal with hardware ... */
>  	dw100_hw_master_bus_disable(dw_dev);
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 37e0670f98c5..e1dda1e834e4 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -1537,7 +1537,7 @@ static void mxc_jpeg_device_run(void *priv)
>  	src_buf->sequence = q_data_out->sequence++;
>  	dst_buf->sequence = q_data_cap->sequence++;
>  
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	jpeg_src_buf = vb2_to_mxc_buf(&src_buf->vb2_buf);
>  	if (q_data_cap->fmt->mem_planes != dst_buf->vb2_buf.num_planes) {
> diff --git a/drivers/media/platform/rockchip/rga/rga.c
> b/drivers/media/platform/rockchip/rga/rga.c
> index 48b88da59da0..075b684fb178 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -75,7 +75,7 @@ static irqreturn_t rga_isr(int irq, void *prv)
>  		WARN_ON(!src);
>  		WARN_ON(!dst);
>  
> -		v4l2_m2m_buf_copy_metadata(src, dst, true);
> +		v4l2_m2m_buf_copy_metadata(src, dst, false);
>  
>  		dst->sequence = ctx->csequence++;
>  
> diff --git a/drivers/media/platform/st/stm32/dma2d/dma2d.c
> b/drivers/media/platform/st/stm32/dma2d/dma2d.c
> index 643913adc1f3..4184bdb96e6d 100644
> --- a/drivers/media/platform/st/stm32/dma2d/dma2d.c
> +++ b/drivers/media/platform/st/stm32/dma2d/dma2d.c
> @@ -483,7 +483,7 @@ static void device_run(void *prv)
>  
>  	src->sequence = frm_out->sequence++;
>  	dst->sequence = frm_cap->sequence++;
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>  
>  	if (clk_enable(dev->gate))
>  		goto end;
> diff --git a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> index 3e7f2df70408..11a6c7f5212e 100644
> --- a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> +++ b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> @@ -71,7 +71,7 @@ static void deinterlace_device_run(void *priv)
>  	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>  	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>  
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>  
>  	deinterlace_write(dev, DEINTERLACE_MOD_ENABLE,
>  			  DEINTERLACE_MOD_ENABLE_EN);
> diff --git a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> index abd10b218aa1..f6e2f11a20dd 100644
> --- a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> +++ b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> @@ -70,7 +70,7 @@ static void rotate_device_run(void *priv)
>  	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>  	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>  
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>  
>  	val = ROTATE_GLB_CTL_MODE(ROTATE_MODE_COPY_ROTATE);
>  	if (ctx->hflip)
> diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-
> drivers/vim2m.c
> index dc82830a35a5..3e8476165721 100644
> --- a/drivers/media/test-drivers/vim2m.c
> +++ b/drivers/media/test-drivers/vim2m.c
> @@ -482,7 +482,7 @@ static int device_process(struct vim2m_ctx *ctx,
>  
>  	out_vb->sequence = q_data_out->sequence++;
>  	in_vb->sequence = q_data_in->sequence++;
> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
> +	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>  
>  	if (ctx->mode & MEM2MEM_VFLIP) {
>  		start = height - 1;
> diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c
> b/drivers/staging/media/imx/imx-media-csc-scaler.c
> index 19fd31cb9bb0..770ba3fbaba2 100644
> --- a/drivers/staging/media/imx/imx-media-csc-scaler.c
> +++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
> @@ -96,7 +96,7 @@ static void ipu_ic_pp_complete(struct ipu_image_convert_run
> *run, void *_ctx)
>  	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>  	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>  
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  
>  	src_buf->sequence = ctx->sequence++;
>  	dst_buf->sequence = src_buf->sequence;
> 
> base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices
  2025-10-09 11:11 [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices Laurent Pinchart
  2025-10-09 13:04 ` Nicolas Dufresne
@ 2025-10-09 14:40 ` Frank Li
  1 sibling, 0 replies; 4+ messages in thread
From: Frank Li @ 2025-10-09 14:40 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, imx, linux-arm-kernel, linux-mediatek,
	linux-rockchip, linux-staging, linux-stm32, linux-sunxi,
	Nicolas Dufresne, Andrzej Pietrasiewicz, Bin Liu, Ezequiel Garcia,
	Hans Verkuil, Jacob Chen, Jernej Skrabec, Jiasheng Jiang,
	Matthew Majewski, Mirela Rabulea, Neil Armstrong, Philipp Zabel,
	Shuah Khan, Steve Longerbeam, Uwe Kleine-König,
	Xavier Roumegue

On Thu, Oct 09, 2025 at 02:11:43PM +0300, Laurent Pinchart wrote:
> The v4l2_m2m_buf_copy_metadata() function takes a boolean
> copy_frame_flags argument. When true, it causes the function to copy the
> V4L2_BUF_FLAG_KEYFRAME, V4L2_BUF_FLAG_BFRAME and V4L2_BUF_FLAG_PFRAME
> flags from the output buffer to the capture buffer.
>
> Many frame-based M2M drivers (e.g. for JPEG encoders, scalers,
> dewarpers, 2D blenders, ...) set the argument to true, while the frame
> flags are not applicable to those drivers as they have no concept of
> key, B or P frames. Set the argument to false to avoid further
> cargo-cult mistakes.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c     | 4 ++--
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c   | 4 ++--
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c   | 4 ++--
>  drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c      | 2 +-
>  drivers/media/platform/nxp/dw100/dw100.c                 | 2 +-
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c           | 2 +-
>  drivers/media/platform/rockchip/rga/rga.c                | 2 +-
>  drivers/media/platform/st/stm32/dma2d/dma2d.c            | 2 +-
>  drivers/media/platform/sunxi/sun8i-di/sun8i-di.c         | 2 +-
>  drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
>  drivers/media/test-drivers/vim2m.c                       | 2 +-
>  drivers/staging/media/imx/imx-media-csc-scaler.c         | 2 +-
>  12 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 35c70ec3ad2c..6bd5036430dc 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1619,7 +1619,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>  	if (!dst_buf)
>  		goto getbuf_fail;
>
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	mtk_jpegenc_set_hw_param(ctx, hw_id, src_buf, dst_buf);
>  	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
> @@ -1715,7 +1715,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>  	if (!dst_buf)
>  		goto getbuf_fail;
>
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
>  	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
>
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> index e78e1d11093c..556865100872 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -530,7 +530,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
> @@ -560,7 +560,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  	ctx = jpeg->hw_param.curr_ctx;
>  	src_buf = jpeg->hw_param.src_buffer;
>  	dst_buf = jpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
>  	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> index 9ab27aee302a..4c8427b3c384 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -261,7 +261,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
>
>  	src_buf = cjpeg->hw_param.src_buffer;
>  	dst_buf = cjpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	mtk_jpeg_enc_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
> @@ -289,7 +289,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
>  	ctx = jpeg->hw_param.curr_ctx;
>  	src_buf = jpeg->hw_param.src_buffer;
>  	dst_buf = jpeg->hw_param.dst_buffer;
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
>  		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> index 59ce5cce0698..dba46a69c6be 100644
> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> @@ -51,7 +51,7 @@ static void mdp_m2m_process_done(void *priv, int vb_state)
>  	ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
>  	src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
>  	dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
> -	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
> +	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, false);
>
>  	v4l2_m2m_buf_done(src_vbuf, vb_state);
>  	v4l2_m2m_buf_done(dst_vbuf, vb_state);
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c
> index 7a0ee44d9e1f..b73302d54635 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -1483,7 +1483,7 @@ static void dw100_start(struct dw100_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
>  				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
>  		in_vb->sequence, out_vb->sequence);
>
> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
> +	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>
>  	/* Now, let's deal with hardware ... */
>  	dw100_hw_master_bus_disable(dw_dev);
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 37e0670f98c5..e1dda1e834e4 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -1537,7 +1537,7 @@ static void mxc_jpeg_device_run(void *priv)
>  	src_buf->sequence = q_data_out->sequence++;
>  	dst_buf->sequence = q_data_cap->sequence++;
>
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	jpeg_src_buf = vb2_to_mxc_buf(&src_buf->vb2_buf);
>  	if (q_data_cap->fmt->mem_planes != dst_buf->vb2_buf.num_planes) {
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index 48b88da59da0..075b684fb178 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -75,7 +75,7 @@ static irqreturn_t rga_isr(int irq, void *prv)
>  		WARN_ON(!src);
>  		WARN_ON(!dst);
>
> -		v4l2_m2m_buf_copy_metadata(src, dst, true);
> +		v4l2_m2m_buf_copy_metadata(src, dst, false);
>
>  		dst->sequence = ctx->csequence++;
>
> diff --git a/drivers/media/platform/st/stm32/dma2d/dma2d.c b/drivers/media/platform/st/stm32/dma2d/dma2d.c
> index 643913adc1f3..4184bdb96e6d 100644
> --- a/drivers/media/platform/st/stm32/dma2d/dma2d.c
> +++ b/drivers/media/platform/st/stm32/dma2d/dma2d.c
> @@ -483,7 +483,7 @@ static void device_run(void *prv)
>
>  	src->sequence = frm_out->sequence++;
>  	dst->sequence = frm_cap->sequence++;
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>
>  	if (clk_enable(dev->gate))
>  		goto end;
> diff --git a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> index 3e7f2df70408..11a6c7f5212e 100644
> --- a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> +++ b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
> @@ -71,7 +71,7 @@ static void deinterlace_device_run(void *priv)
>  	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>  	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>
>  	deinterlace_write(dev, DEINTERLACE_MOD_ENABLE,
>  			  DEINTERLACE_MOD_ENABLE_EN);
> diff --git a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> index abd10b218aa1..f6e2f11a20dd 100644
> --- a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> +++ b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
> @@ -70,7 +70,7 @@ static void rotate_device_run(void *priv)
>  	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>  	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>
>  	val = ROTATE_GLB_CTL_MODE(ROTATE_MODE_COPY_ROTATE);
>  	if (ctx->hflip)
> diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-drivers/vim2m.c
> index dc82830a35a5..3e8476165721 100644
> --- a/drivers/media/test-drivers/vim2m.c
> +++ b/drivers/media/test-drivers/vim2m.c
> @@ -482,7 +482,7 @@ static int device_process(struct vim2m_ctx *ctx,
>
>  	out_vb->sequence = q_data_out->sequence++;
>  	in_vb->sequence = q_data_in->sequence++;
> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
> +	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>
>  	if (ctx->mode & MEM2MEM_VFLIP) {
>  		start = height - 1;
> diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c b/drivers/staging/media/imx/imx-media-csc-scaler.c
> index 19fd31cb9bb0..770ba3fbaba2 100644
> --- a/drivers/staging/media/imx/imx-media-csc-scaler.c
> +++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
> @@ -96,7 +96,7 @@ static void ipu_ic_pp_complete(struct ipu_image_convert_run *run, void *_ctx)
>  	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>  	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>
> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>
>  	src_buf->sequence = ctx->sequence++;
>  	dst_buf->sequence = src_buf->sequence;
>
> base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
> --
> Regards,
>
> Laurent Pinchart
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices
  2025-10-09 13:04 ` Nicolas Dufresne
@ 2025-10-15  8:02   ` Hans Verkuil
  0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2025-10-15  8:02 UTC (permalink / raw)
  To: Nicolas Dufresne, Laurent Pinchart, linux-media, imx,
	linux-arm-kernel, linux-mediatek, linux-rockchip, linux-staging,
	linux-stm32, linux-sunxi
  Cc: Andrzej Pietrasiewicz, Bin Liu, Ezequiel Garcia, Hans Verkuil,
	Jacob Chen, Jernej Skrabec, Jiasheng Jiang, Matthew Majewski,
	Mirela Rabulea, Neil Armstrong, Philipp Zabel, Shuah Khan,
	Steve Longerbeam, Uwe Kleine-König, Xavier Roumegue

On 10/9/25 15:04, Nicolas Dufresne wrote:
> Hi,
> 
> Le jeudi 09 octobre 2025 à 14:11 +0300, Laurent Pinchart a écrit :
>> The v4l2_m2m_buf_copy_metadata() function takes a boolean
>> copy_frame_flags argument. When true, it causes the function to copy the
>> V4L2_BUF_FLAG_KEYFRAME, V4L2_BUF_FLAG_BFRAME and V4L2_BUF_FLAG_PFRAME
>> flags from the output buffer to the capture buffer.
>>
>> Many frame-based M2M drivers (e.g. for JPEG encoders, scalers,
>> dewarpers, 2D blenders, ...) set the argument to true, while the frame
>> flags are not applicable to those drivers as they have no concept of
>> key, B or P frames. Set the argument to false to avoid further
>> cargo-cult mistakes.
> 
> There is no use cases in any upstream driver for copying them over (even in
> Tegra 20 decoder driver). KEY/P/B are properties of bitstream buffer in some
> formats. Once decoded, this is no longer a property of the video frame and
> should be discarded. So even decoder supporting that feature should not copy
> them over.

I agree. I think I thought at the time that it could be useful to know if
an uncompressed frame was decoded from a I/P/B compressed frame, and to
preserve that information if that same uncompressed frame was passed through
another M2M device (e.g. a scaler).

But looking back at the documentation it is clear that these flags are meant
for compressed frames, so copying them makes no sense.

Regards,

	Hans

> 
> I'd remove the boolean completely to be fair, this is always inappropriate to
> set this to true. I'd be fine doing it in two steps, but then the list should be
> completed, see below:
> 
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c     | 4 ++--
>>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c   | 4 ++--
>>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c   | 4 ++--
>>  drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c      | 2 +-
>>  drivers/media/platform/nxp/dw100/dw100.c                 | 2 +-
>>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c           | 2 +-
>>  drivers/media/platform/rockchip/rga/rga.c                | 2 +-
>>  drivers/media/platform/st/stm32/dma2d/dma2d.c            | 2 +-
>>  drivers/media/platform/sunxi/sun8i-di/sun8i-di.c         | 2 +-
>>  drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
>>  drivers/media/test-drivers/vim2m.c                       | 2 +-
>>  drivers/staging/media/imx/imx-media-csc-scaler.c         | 2 +-
> 
> Missing:
> 
> - drivers/media/platform/amphion/vdec.c
> - drivers/media/platform/mediatek/vcodec/decoder/vdec/*
> - drivers/media/platform/nvidia/tegra-vde/h264.c
> - drivers/media/platform/rockchip/rkvdec/rkvdec.c
> - drivers/media/platform/verisilicon/hantro_drv.c
> - drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> 
> cheers,
> Nicolas
> 
>>  12 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> index 35c70ec3ad2c..6bd5036430dc 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> @@ -1619,7 +1619,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>>  	if (!dst_buf)
>>  		goto getbuf_fail;
>>  
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	mtk_jpegenc_set_hw_param(ctx, hw_id, src_buf, dst_buf);
>>  	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
>> @@ -1715,7 +1715,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>>  	if (!dst_buf)
>>  		goto getbuf_fail;
>>  
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
>>  	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
>>  
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> index e78e1d11093c..556865100872 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> @@ -530,7 +530,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct
>> *work)
>>  
>>  	src_buf = cjpeg->hw_param.src_buffer;
>>  	dst_buf = cjpeg->hw_param.dst_buffer;
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
>>  	clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
>> @@ -560,7 +560,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq,
>> void *priv)
>>  	ctx = jpeg->hw_param.curr_ctx;
>>  	src_buf = jpeg->hw_param.src_buffer;
>>  	dst_buf = jpeg->hw_param.dst_buffer;
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
>>  	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> index 9ab27aee302a..4c8427b3c384 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> @@ -261,7 +261,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct
>> *work)
>>  
>>  	src_buf = cjpeg->hw_param.src_buffer;
>>  	dst_buf = cjpeg->hw_param.dst_buffer;
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	mtk_jpeg_enc_reset(cjpeg->reg_base);
>>  	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
>> @@ -289,7 +289,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq,
>> void *priv)
>>  	ctx = jpeg->hw_param.curr_ctx;
>>  	src_buf = jpeg->hw_param.src_buffer;
>>  	dst_buf = jpeg->hw_param.dst_buffer;
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
>>  		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
>> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> index 59ce5cce0698..dba46a69c6be 100644
>> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> @@ -51,7 +51,7 @@ static void mdp_m2m_process_done(void *priv, int vb_state)
>>  	ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
>>  	src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
>>  	dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
>> -	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, false);
>>  
>>  	v4l2_m2m_buf_done(src_vbuf, vb_state);
>>  	v4l2_m2m_buf_done(dst_vbuf, vb_state);
>> diff --git a/drivers/media/platform/nxp/dw100/dw100.c
>> b/drivers/media/platform/nxp/dw100/dw100.c
>> index 7a0ee44d9e1f..b73302d54635 100644
>> --- a/drivers/media/platform/nxp/dw100/dw100.c
>> +++ b/drivers/media/platform/nxp/dw100/dw100.c
>> @@ -1483,7 +1483,7 @@ static void dw100_start(struct dw100_ctx *ctx, struct
>> vb2_v4l2_buffer *in_vb,
>>  				V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
>>  		in_vb->sequence, out_vb->sequence);
>>  
>> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
>> +	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>>  
>>  	/* Now, let's deal with hardware ... */
>>  	dw100_hw_master_bus_disable(dw_dev);
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 37e0670f98c5..e1dda1e834e4 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -1537,7 +1537,7 @@ static void mxc_jpeg_device_run(void *priv)
>>  	src_buf->sequence = q_data_out->sequence++;
>>  	dst_buf->sequence = q_data_cap->sequence++;
>>  
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	jpeg_src_buf = vb2_to_mxc_buf(&src_buf->vb2_buf);
>>  	if (q_data_cap->fmt->mem_planes != dst_buf->vb2_buf.num_planes) {
>> diff --git a/drivers/media/platform/rockchip/rga/rga.c
>> b/drivers/media/platform/rockchip/rga/rga.c
>> index 48b88da59da0..075b684fb178 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.c
>> +++ b/drivers/media/platform/rockchip/rga/rga.c
>> @@ -75,7 +75,7 @@ static irqreturn_t rga_isr(int irq, void *prv)
>>  		WARN_ON(!src);
>>  		WARN_ON(!dst);
>>  
>> -		v4l2_m2m_buf_copy_metadata(src, dst, true);
>> +		v4l2_m2m_buf_copy_metadata(src, dst, false);
>>  
>>  		dst->sequence = ctx->csequence++;
>>  
>> diff --git a/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> b/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> index 643913adc1f3..4184bdb96e6d 100644
>> --- a/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> +++ b/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> @@ -483,7 +483,7 @@ static void device_run(void *prv)
>>  
>>  	src->sequence = frm_out->sequence++;
>>  	dst->sequence = frm_cap->sequence++;
>> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
>> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>>  
>>  	if (clk_enable(dev->gate))
>>  		goto end;
>> diff --git a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> index 3e7f2df70408..11a6c7f5212e 100644
>> --- a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> +++ b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> @@ -71,7 +71,7 @@ static void deinterlace_device_run(void *priv)
>>  	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>>  	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>>  
>> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
>> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>>  
>>  	deinterlace_write(dev, DEINTERLACE_MOD_ENABLE,
>>  			  DEINTERLACE_MOD_ENABLE_EN);
>> diff --git a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> index abd10b218aa1..f6e2f11a20dd 100644
>> --- a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> +++ b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> @@ -70,7 +70,7 @@ static void rotate_device_run(void *priv)
>>  	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>>  	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>>  
>> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
>> +	v4l2_m2m_buf_copy_metadata(src, dst, false);
>>  
>>  	val = ROTATE_GLB_CTL_MODE(ROTATE_MODE_COPY_ROTATE);
>>  	if (ctx->hflip)
>> diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-
>> drivers/vim2m.c
>> index dc82830a35a5..3e8476165721 100644
>> --- a/drivers/media/test-drivers/vim2m.c
>> +++ b/drivers/media/test-drivers/vim2m.c
>> @@ -482,7 +482,7 @@ static int device_process(struct vim2m_ctx *ctx,
>>  
>>  	out_vb->sequence = q_data_out->sequence++;
>>  	in_vb->sequence = q_data_in->sequence++;
>> -	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
>> +	v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>>  
>>  	if (ctx->mode & MEM2MEM_VFLIP) {
>>  		start = height - 1;
>> diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c
>> b/drivers/staging/media/imx/imx-media-csc-scaler.c
>> index 19fd31cb9bb0..770ba3fbaba2 100644
>> --- a/drivers/staging/media/imx/imx-media-csc-scaler.c
>> +++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
>> @@ -96,7 +96,7 @@ static void ipu_ic_pp_complete(struct ipu_image_convert_run
>> *run, void *_ctx)
>>  	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>>  	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>>  
>> -	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>  
>>  	src_buf->sequence = ctx->sequence++;
>>  	dst_buf->sequence = src_buf->sequence;
>>
>> base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-15  8:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09 11:11 [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices Laurent Pinchart
2025-10-09 13:04 ` Nicolas Dufresne
2025-10-15  8:02   ` Hans Verkuil
2025-10-09 14:40 ` Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).