* media: mtk-vcodec: reset segment data then trig decoder
@ 2020-01-14 3:32 gtk_ruiwang
2020-01-15 3:47 ` Alexandre Courbot
0 siblings, 1 reply; 3+ messages in thread
From: gtk_ruiwang @ 2020-01-14 3:32 UTC (permalink / raw)
To: Hans Verkuil, Alexandre Courbot, Mauro Carvalho Chehab,
Matthias Brugger, Thomas Gleixner, Tomasz Figa
Cc: gtk_ruiwang, Maoguang Meng, srv_heupstream, Yunfei Dong,
Longfei Wang, linux-kernel, linux-mediatek, Tiffany Lin,
linux-arm-kernel, linux-media
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))) {
+ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: media: mtk-vcodec: reset segment data then trig decoder
2020-01-14 3:32 media: mtk-vcodec: reset segment data then trig decoder gtk_ruiwang
@ 2020-01-15 3:47 ` Alexandre Courbot
2020-01-15 5:14 ` Alexandre Courbot
0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Courbot @ 2020-01-15 3:47 UTC (permalink / raw)
To: gtk_ruiwang
Cc: Tiffany Lin, srv_heupstream, Yunfei Dong, Longfei Wang, LKML,
Tomasz Figa, Hans Verkuil, Maoguang Meng,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Thomas Gleixner, Mauro Carvalho Chehab,
moderated list:ARM/Mediatek SoC support,
Linux Media Mailing List
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: media: mtk-vcodec: reset segment data then trig decoder
2020-01-15 3:47 ` Alexandre Courbot
@ 2020-01-15 5:14 ` Alexandre Courbot
0 siblings, 0 replies; 3+ messages in thread
From: Alexandre Courbot @ 2020-01-15 5:14 UTC (permalink / raw)
To: gtk_ruiwang
Cc: Tiffany Lin, srv_heupstream, Yunfei Dong, Longfei Wang, LKML,
Tomasz Figa, Hans Verkuil, Maoguang Meng,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Thomas Gleixner, Mauro Carvalho Chehab,
moderated list:ARM/Mediatek SoC support,
Linux Media Mailing List
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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-15 5:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 3:32 media: mtk-vcodec: reset segment data then trig decoder gtk_ruiwang
2020-01-15 3:47 ` Alexandre Courbot
2020-01-15 5:14 ` Alexandre Courbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).