Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* 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	[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, back to index

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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git