| Message ID | 20200114033226.16786-1-gtk_ruiwang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mtk-vcodec: reset segment data then trig decoder | expand |
On Tue, Jan 14, 2020 at 12:32 PM <gtk_ruiwang@mediatek.com> wrote: > > From: gtk_ruiwang <gtk_ruiwang@mediatek.com> > > VP9 bitstream specification indicate segment data should reset to > default when meet key frames, intra only frames or enable error > resilience mode. So memset segmentation map buffer before every > decode process is not appropriate. > > Reset segment data only when needed, then trig decoder hardware > > Signed-off-by: Rui Wang <gtk_ruiwang@mediatek.com> > --- > .../platform/mtk-vcodec/vdec/vdec_vp9_if.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c > index 24c1f0bf2147..42c9c3c98076 100644 > --- a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c > +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c > @@ -110,7 +110,9 @@ struct vp9_sf_ref_fb { > * @buf_len_sz_c : size used to store cbcr plane ufo info (AP-R, VPU-W) > > * @profile : profile sparsed from vpu (AP-R, VPU-W) > - * @show_frame : display this frame or not (AP-R, VPU-W) > + * @show_frame : [BIT(0)] display this frame or not (AP-R, VPU-W) > + * [BIT(14)] reset segment data or not (AP-R, VPU-W) > + * [BIT(15)] trig decoder hardware or not (AP-R, VPU-W) > * @show_existing_frame : inform this frame is show existing frame > * (AP-R, VPU-W) > * @frm_to_show_idx : index to show frame (AP-R, VPU-W) > @@ -494,12 +496,12 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst) > frm_to_show->fb->base_y.size); > } > if (!vp9_is_sf_ref_fb(inst, inst->cur_fb)) { > - if (vsi->show_frame) > + if (vsi->show_frame & BIT(0)) > vp9_add_to_fb_disp_list(inst, inst->cur_fb); > } > } else { > if (!vp9_is_sf_ref_fb(inst, inst->cur_fb)) { > - if (vsi->show_frame) > + if (vsi->show_frame & BIT(0)) > vp9_add_to_fb_disp_list(inst, frm_to_show->fb); > } > } > @@ -870,13 +872,22 @@ static int vdec_vp9_decode(void *h_vdec, struct mtk_vcodec_mem *bs, > vsi->sf_frm_sz[idx]); > } > } > - memset(inst->seg_id_buf.va, 0, inst->seg_id_buf.size); > ret = vpu_dec_start(&inst->vpu, data, 3); > if (ret) { > mtk_vcodec_err(inst, "vpu_dec_start failed"); > goto DECODE_ERROR; > } > > + if ((vsi->show_frame & BIT(15)) && > + (vsi->show_frame & BIT(14))) { Using the new bits in this manner means this patch is not compatible with the older firmware. On an older firmware, these bits will be 0, which means the decoder will never be started. To preserve compatibility, the behavior should be reversed: *do not* reset and/or start the decoder if the bits are set. Also both bits are only used together - we should either separate the data segment reset and decoder start, or rely on only one bit for this. > + memset(inst->seg_id_buf.va, 0, inst->seg_id_buf.size); > + ret = vpu_dec_start(&inst->vpu, NULL, 0); > + if (ret) { > + mtk_vcodec_err(inst, "vpu trig decoder failed"); > + goto DECODE_ERROR; > + } > + } > + > ret = validate_vsi_array_indexes(inst, vsi); > if (ret) { > mtk_vcodec_err(inst, "Invalid values from VPU."); > -- > 2.18.0
On Wed, Jan 15, 2020 at 12:47 PM Alexandre Courbot <acourbot@chromium.org> wrote: > > On Tue, Jan 14, 2020 at 12:32 PM <gtk_ruiwang@mediatek.com> wrote: > > > > From: gtk_ruiwang <gtk_ruiwang@mediatek.com> > > > > VP9 bitstream specification indicate segment data should reset to > > default when meet key frames, intra only frames or enable error > > resilience mode. So memset segmentation map buffer before every > > decode process is not appropriate. > > > > Reset segment data only when needed, then trig decoder hardware > > > > Signed-off-by: Rui Wang <gtk_ruiwang@mediatek.com> > > --- > > .../platform/mtk-vcodec/vdec/vdec_vp9_if.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c > > index 24c1f0bf2147..42c9c3c98076 100644 > > --- a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c > > +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c > > @@ -110,7 +110,9 @@ struct vp9_sf_ref_fb { > > * @buf_len_sz_c : size used to store cbcr plane ufo info (AP-R, VPU-W) > > > > * @profile : profile sparsed from vpu (AP-R, VPU-W) > > - * @show_frame : display this frame or not (AP-R, VPU-W) > > + * @show_frame : [BIT(0)] display this frame or not (AP-R, VPU-W) > > + * [BIT(14)] reset segment data or not (AP-R, VPU-W) > > + * [BIT(15)] trig decoder hardware or not (AP-R, VPU-W) > > * @show_existing_frame : inform this frame is show existing frame > > * (AP-R, VPU-W) > > * @frm_to_show_idx : index to show frame (AP-R, VPU-W) > > @@ -494,12 +496,12 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst) > > frm_to_show->fb->base_y.size); > > } > > if (!vp9_is_sf_ref_fb(inst, inst->cur_fb)) { > > - if (vsi->show_frame) > > + if (vsi->show_frame & BIT(0)) > > vp9_add_to_fb_disp_list(inst, inst->cur_fb); > > } > > } else { > > if (!vp9_is_sf_ref_fb(inst, inst->cur_fb)) { > > - if (vsi->show_frame) > > + if (vsi->show_frame & BIT(0)) > > vp9_add_to_fb_disp_list(inst, frm_to_show->fb); > > } > > } > > @@ -870,13 +872,22 @@ static int vdec_vp9_decode(void *h_vdec, struct mtk_vcodec_mem *bs, > > vsi->sf_frm_sz[idx]); > > } > > } > > - memset(inst->seg_id_buf.va, 0, inst->seg_id_buf.size); > > ret = vpu_dec_start(&inst->vpu, data, 3); > > if (ret) { > > mtk_vcodec_err(inst, "vpu_dec_start failed"); > > goto DECODE_ERROR; > > } > > > > + if ((vsi->show_frame & BIT(15)) && > > + (vsi->show_frame & BIT(14))) { > > Using the new bits in this manner means this patch is not compatible > with the older firmware. > > On an older firmware, these bits will be 0, which means the decoder > will never be started. To preserve compatibility, the behavior should > be reversed: *do not* reset and/or start the decoder if the bits are > set. > > Also both bits are only used together - we should either separate the > data segment reset and decoder start, or rely on only one bit for > this. Ah, looks like I missed the fact that the decoder is still started even when these bits are not set. So this indeed looks backward-compatible. Please ignore my comment.
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c index 24c1f0bf2147..42c9c3c98076 100644 --- a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c @@ -110,7 +110,9 @@ struct vp9_sf_ref_fb { * @buf_len_sz_c : size used to store cbcr plane ufo info (AP-R, VPU-W) * @profile : profile sparsed from vpu (AP-R, VPU-W) - * @show_frame : display this frame or not (AP-R, VPU-W) + * @show_frame : [BIT(0)] display this frame or not (AP-R, VPU-W) + * [BIT(14)] reset segment data or not (AP-R, VPU-W) + * [BIT(15)] trig decoder hardware or not (AP-R, VPU-W) * @show_existing_frame : inform this frame is show existing frame * (AP-R, VPU-W) * @frm_to_show_idx : index to show frame (AP-R, VPU-W) @@ -494,12 +496,12 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst) frm_to_show->fb->base_y.size); } if (!vp9_is_sf_ref_fb(inst, inst->cur_fb)) { - if (vsi->show_frame) + if (vsi->show_frame & BIT(0)) vp9_add_to_fb_disp_list(inst, inst->cur_fb); } } else { if (!vp9_is_sf_ref_fb(inst, inst->cur_fb)) { - if (vsi->show_frame) + if (vsi->show_frame & BIT(0)) vp9_add_to_fb_disp_list(inst, frm_to_show->fb); } } @@ -870,13 +872,22 @@ static int vdec_vp9_decode(void *h_vdec, struct mtk_vcodec_mem *bs, vsi->sf_frm_sz[idx]); } } - memset(inst->seg_id_buf.va, 0, inst->seg_id_buf.size); ret = vpu_dec_start(&inst->vpu, data, 3); if (ret) { mtk_vcodec_err(inst, "vpu_dec_start failed"); goto DECODE_ERROR; } + if ((vsi->show_frame & BIT(15)) && + (vsi->show_frame & BIT(14))) { + memset(inst->seg_id_buf.va, 0, inst->seg_id_buf.size); + ret = vpu_dec_start(&inst->vpu, NULL, 0); + if (ret) { + mtk_vcodec_err(inst, "vpu trig decoder failed"); + goto DECODE_ERROR; + } + } + ret = validate_vsi_array_indexes(inst, vsi); if (ret) { mtk_vcodec_err(inst, "Invalid values from VPU.");