| Message ID | 20221112094144.4256-4-yunfei.dong@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: mediatek: vcodec: Fix decode random crash for PLT test | expand |
Il 12/11/22 10:41, Yunfei Dong ha scritto: > Will set lat buffer to lat_list two times when lat decode timeout for > inner racing mode. > > If core thread can't get frame buffer, need to return error value. > > Fixes: 59fba9eed5a7 ("media: mediatek: vcodec: support stateless H.264 decoding for mt8192") > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > .../vcodec/vdec/vdec_h264_req_multi_if.c | 23 ++++++++++++------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c > index 4cc92700692b..2b7576265f48 100644 > --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c > +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c > @@ -471,6 +471,12 @@ static int vdec_h264_slice_core_decode(struct vdec_lat_buf *lat_buf) > sizeof(share_info->h264_slice_params)); > > fb = ctx->dev->vdec_pdata->get_cap_buffer(ctx); > + if (!fb) { > + err = -EBUSY; > + mtk_vcodec_err(inst, "fb buffer is NULL"); > + goto vdec_dec_end; > + } > + > y_fb_dma = fb ? (u64)fb->base_y.dma_addr : 0; If you're jumping to vdec_dec_end, this check becomes pointless as y_fb_dma will never be set to 0, so you can remove the conditional on assigning the dma address to `y_fb_dma`. Same comment a few lines later for `c_fb_dma`. Regards, Angelo
Hi AngeloGioacchino, Thanks for your detail comments. Change the driver according to your suggestion in v2. Best Regards, Yunfei Dong On Mon, 2022-11-14 at 12:26 +0100, AngeloGioacchino Del Regno wrote: > Il 12/11/22 10:41, Yunfei Dong ha scritto: > > Will set lat buffer to lat_list two times when lat decode timeout > > for > > inner racing mode. > > > > If core thread can't get frame buffer, need to return error value. > > > > Fixes: 59fba9eed5a7 ("media: mediatek: vcodec: support stateless > > H.264 decoding for mt8192") > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../vcodec/vdec/vdec_h264_req_multi_if.c | 23 ++++++++++++ > > ------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git > > a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i > > f.c > > b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i > > f.c > > index 4cc92700692b..2b7576265f48 100644 > > --- > > a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i > > f.c > > +++ > > b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i > > f.c > > @@ -471,6 +471,12 @@ static int vdec_h264_slice_core_decode(struct > > vdec_lat_buf *lat_buf) > > sizeof(share_info->h264_slice_params)); > > > > fb = ctx->dev->vdec_pdata->get_cap_buffer(ctx); > > + if (!fb) { > > + err = -EBUSY; > > + mtk_vcodec_err(inst, "fb buffer is NULL"); > > + goto vdec_dec_end; > > + } > > + > > y_fb_dma = fb ? (u64)fb->base_y.dma_addr : 0; > > If you're jumping to vdec_dec_end, this check becomes pointless as > y_fb_dma > will never be set to 0, so you can remove the conditional on > assigning > the dma address to `y_fb_dma`. > > Same comment a few lines later for `c_fb_dma`. > > Regards, > Angelo > >
diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c index 4cc92700692b..2b7576265f48 100644 --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c @@ -471,6 +471,12 @@ static int vdec_h264_slice_core_decode(struct vdec_lat_buf *lat_buf) sizeof(share_info->h264_slice_params)); fb = ctx->dev->vdec_pdata->get_cap_buffer(ctx); + if (!fb) { + err = -EBUSY; + mtk_vcodec_err(inst, "fb buffer is NULL"); + goto vdec_dec_end; + } + y_fb_dma = fb ? (u64)fb->base_y.dma_addr : 0; vdec_fb_va = (unsigned long)fb; @@ -630,7 +636,7 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs, err = vpu_dec_start(vpu, data, 2); if (err) { mtk_vcodec_debug(inst, "lat decode err: %d", err); - goto err_scp_decode; + goto err_free_fb_out; } share_info->trans_end = inst->ctx->msg_queue.wdma_addr.dma_addr + @@ -647,12 +653,17 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs, /* wait decoder done interrupt */ timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED, WAIT_INTR_TIMEOUT_MS, MTK_VDEC_LAT0); + if (timeout) + mtk_vcodec_err(inst, "lat decode timeout: pic_%d", inst->slice_dec_num); inst->vsi->dec.timeout = !!timeout; err = vpu_dec_end(vpu); - if (err == SLICE_HEADER_FULL || timeout || err == TRANS_BUFFER_FULL) { - err = -EINVAL; - goto err_scp_decode; + if (err == SLICE_HEADER_FULL || err == TRANS_BUFFER_FULL) { + if (!IS_VDEC_INNER_RACING(inst->ctx->dev->dec_capability)) + vdec_msg_queue_qbuf(&inst->ctx->msg_queue.lat_ctx, lat_buf); + inst->slice_dec_num++; + mtk_vcodec_err(inst, "lat dec fail: pic_%d err:%d", inst->slice_dec_num, err); + return -EINVAL; } share_info->trans_end = inst->ctx->msg_queue.wdma_addr.dma_addr + @@ -669,10 +680,6 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs, inst->slice_dec_num++; return 0; - -err_scp_decode: - if (!IS_VDEC_INNER_RACING(inst->ctx->dev->dec_capability)) - vdec_msg_queue_qbuf(&inst->ctx->msg_queue.lat_ctx, lat_buf); err_free_fb_out: vdec_msg_queue_qbuf(&inst->ctx->msg_queue.lat_ctx, lat_buf); mtk_vcodec_err(inst, "slice dec number: %d err: %d", inst->slice_dec_num, err);
Will set lat buffer to lat_list two times when lat decode timeout for inner racing mode. If core thread can't get frame buffer, need to return error value. Fixes: 59fba9eed5a7 ("media: mediatek: vcodec: support stateless H.264 decoding for mt8192") Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../vcodec/vdec/vdec_h264_req_multi_if.c | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-)