The buffer remove and buffer done of output queue is separated into
two works, the value of owned_by_drv_count isn't zero when output
queue stream off before flush decode.
Changing the flush decode from capture to output when stream off to
make sure all the output queue buffers are set to done list.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 45 +++++++++----------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index 2b787e60a1f9..7080ca3e18b0 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -893,32 +893,31 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q)
 				v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
 			}
 		}
-		return;
-	}
-
-	if (ctx->state >= MTK_STATE_HEADER) {
-
-		/* Until STREAMOFF is called on the CAPTURE queue
-		 * (acknowledging the event), the driver operates
-		 * as if the resolution hasn't changed yet, i.e.
-		 * VIDIOC_G_FMT< etc. return previous resolution.
-		 * So we update picinfo here
-		 */
-		ctx->picinfo = ctx->last_decoded_picinfo;
 
-		mtk_v4l2_vdec_dbg(2, ctx,
-				  "[%d]-> new(%d,%d), old(%d,%d), real(%d,%d)",
-				  ctx->id, ctx->last_decoded_picinfo.pic_w,
-				  ctx->last_decoded_picinfo.pic_h,
-				  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
-				  ctx->last_decoded_picinfo.buf_w,
-				  ctx->last_decoded_picinfo.buf_h);
+		if (ctx->state >= MTK_STATE_HEADER) {
+			/*
+			 * The resolution hasn't been changed when STREAMOFF is called.
+			 * Update the picinfo here with previous resolution if VIDIOC_G_FMT
+			 * is called.
+			 */
+			ctx->picinfo = ctx->last_decoded_picinfo;
+
+			mtk_v4l2_vdec_dbg(2, ctx,
+					  "[%d]-> new(%d,%d), old(%d,%d), real(%d,%d)",
+					  ctx->id, ctx->last_decoded_picinfo.pic_w,
+					  ctx->last_decoded_picinfo.pic_h,
+					  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
+					  ctx->last_decoded_picinfo.buf_w,
+					  ctx->last_decoded_picinfo.buf_h);
+
+			ret = ctx->dev->vdec_pdata->flush_decoder(ctx);
+			if (ret)
+				mtk_v4l2_vdec_err(ctx, "DecodeFinal failed, ret=%d", ret);
+		}
 
-		ret = ctx->dev->vdec_pdata->flush_decoder(ctx);
-		if (ret)
-			mtk_v4l2_vdec_err(ctx, "DecodeFinal failed, ret=%d", ret);
+		ctx->state = MTK_STATE_FLUSH;
+		return;
 	}
-	ctx->state = MTK_STATE_FLUSH;
 
 	while ((dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx))) {
 		vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
-- 
2.46.0

Hey Yunfei,

On 07.08.2024 16:24, Yunfei Dong wrote:
>The buffer remove and buffer done of output queue is separated into
>two works, the value of owned_by_drv_count isn't zero when output
>queue stream off before flush decode.

You have to try this again, I cannot make sense out of your message.

What do you mean with:
"The buffer remove and buffer done of output queue is separated into two works"

I suppose this:
the value of owned_by_drv_count isn't zero when output queue stream off
before flush decode.

should be:
therefore the value of `owned_by_drv_count` isn't zero while flushing
the decoder in the STREAMOFF(OUTPUT queue) IOCTL.

right?

>Changing the flush decode from capture to output when stream off to
>make sure all the output queue buffers are set to done list.

I'd change this section to:

Flushing the decoder during STREAMOFF(OUTPUT) instead of during
STREAMOFF(CAPTURE) makes sure that all buffers on the OUTPUT queue are
set done.

The rest looks fine.

Regards,
Sebastian Fricke

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 45 +++++++++----------
> 1 file changed, 22 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>index 2b787e60a1f9..7080ca3e18b0 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>@@ -893,32 +893,31 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q)
> 				v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
> 			}
> 		}
>-		return;
>-	}
>-
>-	if (ctx->state >= MTK_STATE_HEADER) {
>-
>-		/* Until STREAMOFF is called on the CAPTURE queue
>-		 * (acknowledging the event), the driver operates
>-		 * as if the resolution hasn't changed yet, i.e.
>-		 * VIDIOC_G_FMT< etc. return previous resolution.
>-		 * So we update picinfo here
>-		 */
>-		ctx->picinfo = ctx->last_decoded_picinfo;
>
>-		mtk_v4l2_vdec_dbg(2, ctx,
>-				  "[%d]-> new(%d,%d), old(%d,%d), real(%d,%d)",
>-				  ctx->id, ctx->last_decoded_picinfo.pic_w,
>-				  ctx->last_decoded_picinfo.pic_h,
>-				  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
>-				  ctx->last_decoded_picinfo.buf_w,
>-				  ctx->last_decoded_picinfo.buf_h);
>+		if (ctx->state >= MTK_STATE_HEADER) {
>+			/*
>+			 * The resolution hasn't been changed when STREAMOFF is called.
>+			 * Update the picinfo here with previous resolution if VIDIOC_G_FMT
>+			 * is called.
>+			 */
>+			ctx->picinfo = ctx->last_decoded_picinfo;
>+
>+			mtk_v4l2_vdec_dbg(2, ctx,
>+					  "[%d]-> new(%d,%d), old(%d,%d), real(%d,%d)",
>+					  ctx->id, ctx->last_decoded_picinfo.pic_w,
>+					  ctx->last_decoded_picinfo.pic_h,
>+					  ctx->picinfo.pic_w, ctx->picinfo.pic_h,
>+					  ctx->last_decoded_picinfo.buf_w,
>+					  ctx->last_decoded_picinfo.buf_h);
>+
>+			ret = ctx->dev->vdec_pdata->flush_decoder(ctx);
>+			if (ret)
>+				mtk_v4l2_vdec_err(ctx, "DecodeFinal failed, ret=%d", ret);
>+		}
>
>-		ret = ctx->dev->vdec_pdata->flush_decoder(ctx);
>-		if (ret)
>-			mtk_v4l2_vdec_err(ctx, "DecodeFinal failed, ret=%d", ret);
>+		ctx->state = MTK_STATE_FLUSH;
>+		return;
> 	}
>-	ctx->state = MTK_STATE_FLUSH;
>
> 	while ((dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx))) {
> 		vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
>-- 
>2.46.0
>
>