| Message ID | 20241108033219.19804-6-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: mediatek: vcodec: fix v4l2_ctrl_request_complete fail | expand |
On Fri, Nov 8, 2024 at 11:32 AM Yunfei Dong <yunfei.dong@mediatek.com> wrote: > > Setting the buffer status to error if the media request of > each source buffer is NULL, then schedule the work to process > again in case of access NULL pointer. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c > index 3f94654ebc73..251111678fd8 100644 > --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c > +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c > @@ -363,10 +363,14 @@ static void mtk_vdec_worker(struct work_struct *work) > ctx->id, bs_src->va, &bs_src->dma_addr, bs_src->size, vb2_src); > /* Apply request controls. */ > src_buf_req = vb2_src->req_obj.req; > - if (src_buf_req) > + if (src_buf_req) { > v4l2_ctrl_request_setup(src_buf_req, &ctx->ctrl_hdl); > - else > + } else { > mtk_v4l2_vdec_err(ctx, "vb2 buffer media request is NULL"); > + v4l2_m2m_buf_done(vb2_v4l2_src, VB2_BUF_STATE_ERROR); > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > + return; Is this something that actually happens? I would assume that having the `requires_requests` flag set on the queue would block any QBUF call that doesn't have a request attached. > + } > > ret = vdec_if_decode(ctx, bs_src, NULL, &res_chg); > if (ret && ret != -EAGAIN) { > @@ -384,8 +388,7 @@ static void mtk_vdec_worker(struct work_struct *work) > state = ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE; > if (!IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch) || > ctx->current_codec == V4L2_PIX_FMT_VP8_FRAME) { > - if (src_buf_req) > - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); > + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); Unrelated change. Please do the cleanup in a separate patch. v4l2_ctrl_request_setup() and v4l2_ctrl_request_complete() are both no-ops if either argument is NULL, so you can do one patch going over the whole driver to clean it up. > vb2_v4l2_dst = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); > v4l2_m2m_buf_done(vb2_v4l2_dst, state); > v4l2_m2m_buf_done(vb2_v4l2_src, state); > @@ -403,8 +406,7 @@ static void mtk_vdec_worker(struct work_struct *work) > */ > ctx->cur_src_buffer = (ret != -EAGAIN) ? NULL : vb2_v4l2_src; > if (ret && ret != -EAGAIN) { > - if (src_buf_req) > - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); > + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); Unrelated change. Same as above. ChenYu > v4l2_m2m_buf_done(vb2_v4l2_src, state); > } > > -- > 2.46.0 >
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c index 3f94654ebc73..251111678fd8 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c @@ -363,10 +363,14 @@ static void mtk_vdec_worker(struct work_struct *work) ctx->id, bs_src->va, &bs_src->dma_addr, bs_src->size, vb2_src); /* Apply request controls. */ src_buf_req = vb2_src->req_obj.req; - if (src_buf_req) + if (src_buf_req) { v4l2_ctrl_request_setup(src_buf_req, &ctx->ctrl_hdl); - else + } else { mtk_v4l2_vdec_err(ctx, "vb2 buffer media request is NULL"); + v4l2_m2m_buf_done(vb2_v4l2_src, VB2_BUF_STATE_ERROR); + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); + return; + } ret = vdec_if_decode(ctx, bs_src, NULL, &res_chg); if (ret && ret != -EAGAIN) { @@ -384,8 +388,7 @@ static void mtk_vdec_worker(struct work_struct *work) state = ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE; if (!IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch) || ctx->current_codec == V4L2_PIX_FMT_VP8_FRAME) { - if (src_buf_req) - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); vb2_v4l2_dst = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx); v4l2_m2m_buf_done(vb2_v4l2_dst, state); v4l2_m2m_buf_done(vb2_v4l2_src, state); @@ -403,8 +406,7 @@ static void mtk_vdec_worker(struct work_struct *work) */ ctx->cur_src_buffer = (ret != -EAGAIN) ? NULL : vb2_v4l2_src; if (ret && ret != -EAGAIN) { - if (src_buf_req) - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); v4l2_m2m_buf_done(vb2_v4l2_src, state); }
Setting the buffer status to error if the media request of each source buffer is NULL, then schedule the work to process again in case of access NULL pointer. Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)