[PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[Yunfei Dong]

Need to get dec_capability from scp first, then initialize decoder
supported format and other parameters according to dec_capability value.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c     | 2 --
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 1465ddff1c6b..41589470da32 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -112,8 +112,6 @@ void mtk_vcodec_dec_set_default_params(struct mtk_vcodec_ctx *ctx)
 {
 	struct mtk_q_data *q_data;
 
-	ctx->dev->vdec_pdata->init_vdec_params(ctx);
-
 	ctx->m2m_ctx->q_lock = &ctx->dev->dev_mutex;
 	ctx->fh.m2m_ctx = ctx->m2m_ctx;
 	ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
index 4103d7c1b638..99d7b15f2b9d 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
@@ -208,6 +208,8 @@ static int fops_vcodec_open(struct file *file)
 
 		dev->dec_capability =
 			mtk_vcodec_fw_get_vdec_capa(dev->fw_handler);
+		ctx->dev->vdec_pdata->init_vdec_params(ctx);
+
 		mtk_v4l2_debug(0, "decoder capability %x", dev->dec_capability);
 	}
 
-- 
2.18.0

Re: [PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[Chen-Yu Tsai]

On Sat, Jun 18, 2022 at 3:29 PM Yunfei Dong <yunfei.dong@mediatek.com> wrote:
>
> Need to get dec_capability from scp first, then initialize decoder
> supported format and other parameters according to dec_capability value.
>
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>

Tested on MT8183 on mainline 20220617-next with the vcodec-dec DT node
copied from the ChromeOS v5.10 kernel.

This fixes an issue where the first attempt to enumerate formats on the
device right after boot returns an empty list.

Re: [PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[Chen-Yu Tsai]

On Mon, Jun 20, 2022 at 12:54 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Sat, Jun 18, 2022 at 3:29 PM Yunfei Dong <yunfei.dong@mediatek.com> wrote:
> >
> > Need to get dec_capability from scp first, then initialize decoder
> > supported format and other parameters according to dec_capability value.
> >
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
>
> Tested on MT8183 on mainline 20220617-next with the vcodec-dec DT node
> copied from the ChromeOS v5.10 kernel.
>
> This fixes an issue where the first attempt to enumerate formats on the
> device right after boot returns an empty list.

BTW, this should have a Fixes tag.

Either

Fixes: 7a7ae26fd458 ("media: mediatek: vcodec: support stateless VP8 decoding")

which looks like the first instance of when firmware capability really is
considered, or

Fixes: fd00d90330d1 ("media: mtk-vcodec: vdec: move stateful ops into
their own file")

which matches when ctx->dev->vdec_pdata->init_vdec_params(ctx) was first added.


ChenYu

Re: [PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[yunfei.dong]

Hi Chen-Yu,

Thanks for your suggestion.
On Mon, 2022-06-20 at 13:25 +0800, Chen-Yu Tsai wrote:
> On Mon, Jun 20, 2022 at 12:54 PM Chen-Yu Tsai <wenst@chromium.org>
> wrote:
> > 
> > On Sat, Jun 18, 2022 at 3:29 PM Yunfei Dong <
> > yunfei.dong@mediatek.com> wrote:
> > > 
> > > Need to get dec_capability from scp first, then initialize
> > > decoder
> > > supported format and other parameters according to dec_capability
> > > value.
> > > 
> > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > 
> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > 
> > Tested on MT8183 on mainline 20220617-next with the vcodec-dec DT
> > node
> > copied from the ChromeOS v5.10 kernel.
> > 
> > This fixes an issue where the first attempt to enumerate formats on
> > the
> > device right after boot returns an empty list.
> 
> BTW, this should have a Fixes tag.
> 
> Either
> 
> Fixes: 7a7ae26fd458 ("media: mediatek: vcodec: support stateless VP8
> decoding")
> 
> which looks like the first instance of when firmware capability
> really is
> considered, or
> 
> Fixes: fd00d90330d1 ("media: mtk-vcodec: vdec: move stateful ops into
> their own file")
> 
> which matches when ctx->dev->vdec_pdata->init_vdec_params(ctx) was
> first added.
> 
> 
> ChenYu

I will fix the patch in v2.

Best Regards,
Yunfei Dong

Re: [PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[Nicolas Dufresne]

Hi Yunfei,

Le samedi 18 juin 2022 à 15:29 +0800, Yunfei Dong a écrit :
> Need to get dec_capability from scp first, then initialize decoder
> supported format and other parameters according to dec_capability value.

Perhaps something to improve in the future. On top of describing the fix, it
could be useful to describe what issues is being fixed, and which platform will
benefit.

> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

To add to this, this looks like a bug fix, can you relate it to an original
commit and add a Fixes: tag here ?

regards,
Nicolas

> ---
>  drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c     | 2 --
>  drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> index 1465ddff1c6b..41589470da32 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> @@ -112,8 +112,6 @@ void mtk_vcodec_dec_set_default_params(struct mtk_vcodec_ctx *ctx)
>  {
>  	struct mtk_q_data *q_data;
>  
> -	ctx->dev->vdec_pdata->init_vdec_params(ctx);
> -
>  	ctx->m2m_ctx->q_lock = &ctx->dev->dev_mutex;
>  	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  	ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> index 4103d7c1b638..99d7b15f2b9d 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> @@ -208,6 +208,8 @@ static int fops_vcodec_open(struct file *file)
>  
>  		dev->dec_capability =
>  			mtk_vcodec_fw_get_vdec_capa(dev->fw_handler);
> +		ctx->dev->vdec_pdata->init_vdec_params(ctx);
> +
>  		mtk_v4l2_debug(0, "decoder capability %x", dev->dec_capability);
>  	}
>  

Re: [PATCH] media: mediatek: vcodec: Initialize decoder parameters after getting dec_capability

[yunfei.dong]

Hi Nicolas,

Thanks for your comments.

I already sent patch v2 to add fixes table in June 20.

Best Regards,
Yunfei Dong
On Tue, 2022-06-21 at 10:20 -0400, Nicolas Dufresne wrote:
> Hi Yunfei,
> 
> Le samedi 18 juin 2022 à 15:29 +0800, Yunfei Dong a écrit :
> > Need to get dec_capability from scp first, then initialize decoder
> > supported format and other parameters according to dec_capability
> > value.
> 
> Perhaps something to improve in the future. On top of describing the
> fix, it
> could be useful to describe what issues is being fixed, and which
> platform will
> benefit.
> 
> > 
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> 
> To add to this, this looks like a bug fix, can you relate it to an
> original
> commit and add a Fixes: tag here ?
> 
> regards,
> Nicolas
> 
> > ---
> >  drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c     | 2 --
> >  drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c | 2 ++
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > index 1465ddff1c6b..41589470da32 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
> > @@ -112,8 +112,6 @@ void mtk_vcodec_dec_set_default_params(struct
> > mtk_vcodec_ctx *ctx)
> >  {
> >  	struct mtk_q_data *q_data;
> >  
> > -	ctx->dev->vdec_pdata->init_vdec_params(ctx);
> > -
> >  	ctx->m2m_ctx->q_lock = &ctx->dev->dev_mutex;
> >  	ctx->fh.m2m_ctx = ctx->m2m_ctx;
> >  	ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> > index 4103d7c1b638..99d7b15f2b9d 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> > @@ -208,6 +208,8 @@ static int fops_vcodec_open(struct file *file)
> >  
> >  		dev->dec_capability =
> >  			mtk_vcodec_fw_get_vdec_capa(dev->fw_handler);
> > +		ctx->dev->vdec_pdata->init_vdec_params(ctx);
> > +
> >  		mtk_v4l2_debug(0, "decoder capability %x", dev-
> > >dec_capability);
> >  	}
> >  
> 
>