| Message ID | 20240807082444.21280-4-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: mediatek: vcodec: fix v4l2_ctrl_request_complete fail | expand |
Hey Yunfei, On 07.08.2024 16:24, Yunfei Dong wrote: >Flush decoder will reset the driver to flush status. If lat or core >work queue in active before flush when stream off, will lead to get >dst buffer NULL or buff done with one non-existent source buffer. > >Flush decoder when stream off no matter output or capture queue >calling stream off firstly. > >Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> >--- > .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 45 ++++++++++--------- > 1 file changed, 23 insertions(+), 22 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 7080ca3e18b0..fc4ee1fb7cd1 100644 >--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c >+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c >@@ -882,6 +882,29 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) > mtk_v4l2_vdec_dbg(3, ctx, "[%d] (%d) state=(%x) ctx->decoded_frame_cnt=%d", > ctx->id, q->type, ctx->state, ctx->decoded_frame_cnt); > >+ if (ctx->state >= MTK_STATE_HEADER && ctx->state != MTK_STATE_FLUSH) { >+ /* >+ * 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); >+ >+ ctx->state = MTK_STATE_FLUSH; >+ } >+ > if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { > while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx))) { > if (src_buf != &ctx->empty_flush_buf.vb) { >@@ -894,28 +917,6 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) > } > } > >- 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); >- } >- >- ctx->state = MTK_STATE_FLUSH; you just changed this routine in patch 2/7, why was patch 2/7 needed if you remove it right away in the next patch? regards, Sebastian Fricke > return; > } > >-- >2.46.0 > >
Hi Sebastian, Thanks for your suggestion. On Thu, 2024-08-22 at 18:11 +0200, Sebastian Fricke wrote: > Hey Yunfei, > > On 07.08.2024 16:24, Yunfei Dong wrote: > > Flush decoder will reset the driver to flush status. If lat or core > > work queue in active before flush when stream off, will lead to get > > dst buffer NULL or buff done with one non-existent source buffer. > > > > Flush decoder when stream off no matter output or capture queue > > calling stream off firstly. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 45 ++++++++++---- > > ----- > > 1 file changed, 23 insertions(+), 22 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 7080ca3e18b0..fc4ee1fb7cd1 100644 > > --- > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c > > +++ > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c > > @@ -882,6 +882,29 @@ void vb2ops_vdec_stop_streaming(struct > > vb2_queue *q) > > mtk_v4l2_vdec_dbg(3, ctx, "[%d] (%d) state=(%x) ctx- > > >decoded_frame_cnt=%d", > > ctx->id, q->type, ctx->state, ctx- > > >decoded_frame_cnt); > > > > + if (ctx->state >= MTK_STATE_HEADER && ctx->state != > > MTK_STATE_FLUSH) { > > + /* > > + * 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); > > + > > + ctx->state = MTK_STATE_FLUSH; > > + } > > + > > if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { > > while ((src_buf = v4l2_m2m_src_buf_remove(ctx- > > >m2m_ctx))) { > > if (src_buf != &ctx->empty_flush_buf.vb) { > > @@ -894,28 +917,6 @@ void vb2ops_vdec_stop_streaming(struct > > vb2_queue *q) > > } > > } > > > > - 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); > > - } > > - > > - ctx->state = MTK_STATE_FLUSH; > > you just changed this routine in patch 2/7, why was patch 2/7 needed > if > you remove it right away in the next patch? > Patch 2/7 is used to fix that ctrl request complete and buffer done are separated into two works for output queue, flush decoder on capture queue may lead to ctrl request complete fail when user space stop output queue firstly. Patch 3/7 is used to fix that user space may call stop capture or output queue first randomly. Need to combine these two patches together? Best Regards, Yunfei Dong > regards, > Sebastian Fricke > > > return; > > } > > > > -- > > 2.46.0 > > > >
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 7080ca3e18b0..fc4ee1fb7cd1 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c @@ -882,6 +882,29 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) mtk_v4l2_vdec_dbg(3, ctx, "[%d] (%d) state=(%x) ctx->decoded_frame_cnt=%d", ctx->id, q->type, ctx->state, ctx->decoded_frame_cnt); + if (ctx->state >= MTK_STATE_HEADER && ctx->state != MTK_STATE_FLUSH) { + /* + * 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); + + ctx->state = MTK_STATE_FLUSH; + } + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx))) { if (src_buf != &ctx->empty_flush_buf.vb) { @@ -894,28 +917,6 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) } } - 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); - } - - ctx->state = MTK_STATE_FLUSH; return; }
Flush decoder will reset the driver to flush status. If lat or core work queue in active before flush when stream off, will lead to get dst buffer NULL or buff done with one non-existent source buffer. Flush decoder when stream off no matter output or capture queue calling stream off firstly. Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-)