diff mbox series

[3/5] media: mediatek: vcodec: Fix h264 set lat buffer error

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

Commit Message

Yunfei Dong (董云飞) Nov. 12, 2022, 9:41 a.m. UTC
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(-)

Comments

AngeloGioacchino Del Regno Nov. 14, 2022, 11:26 a.m. UTC | #1
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
Yunfei Dong (董云飞) Nov. 17, 2022, 9:29 a.m. UTC | #2
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 mbox series

Patch

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);