[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240822153242.sewrferyfh3u5g4h@basti-XPS-13-9310>
Date: Thu, 22 Aug 2024 17:32:42 +0200
From: Sebastian Fricke <sebastian.fricke@...labora.com>
To: Yunfei Dong <yunfei.dong@...iatek.com>
Cc: NĂcolas F . R . A . Prado <nfraprado@...labora.com>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>,
Hans Verkuil <hverkuil-cisco@...all.nl>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Benjamin Gaignard <benjamin.gaignard@...labora.com>,
Nathan Hebert <nhebert@...omium.org>,
Daniel Almeida <daniel.almeida@...labora.com>,
Hsin-Yi Wang <hsinyi@...omium.org>,
Fritz Koenig <frkoenig@...omium.org>,
Daniel Vetter <daniel@...ll.ch>, Steve Cho <stevecho@...omium.org>,
linux-media@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v4 2/7] media: mediatek: vcodec: change flush decode
order when stream off
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@...iatek.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
>
>
Powered by blists - more mailing lists