mbox series

[0/8] media: mediatek: vcodec: separate encoder and decoder

Message ID 20230601030256.29875-1-yunfei.dong@mediatek.com (mailing list archive)
Headers show
Series media: mediatek: vcodec: separate encoder and decoder | expand

Message

Yunfei Dong June 1, 2023, 3:02 a.m. UTC
With the driver more and more complex, encoder and decoder need to add more parameter
in shared struct 'mtk_vcodec_ctx' and 'mtk_vcodec_dev'. Encoder use about 40% and
decoder use 60% parameter. Need to allocate extra unused memory when encoder and decoder
working.

Separate encoder and decoder in different folder and use independent data struct.

patch 1 remove unused parameter.
patch 2~3 align fw and interrupt related interface.
patch 4 remove the dependency of debug log
patch 5~6 separate mtk_vcodec_ctx and mtk_vcodec_dev
patch 7 removed unused header files
patch 8 separate encoder and decoder in different folder
---
Yunfei Dong (8):
  media: mediatek: vcodec: remove unused parameter
  mediatek: vcodec: align fw interface
  mediatek: vcodec: re-write shared interface
  mediatek: vcodec: remove the dependency of debug log
  mediatek: vcodec: separate struct mtk_vcodec_ctx
  mediatek: vcodec: separate struct mtk_vcodec_dev
  mediatek: vcodec: remove unused include header
  mediatek: vcodec: separate decoder and encoder

 .../media/platform/mediatek/vcodec/Makefile   |  55 +--
 .../mediatek/vcodec/mtk_vcodec_intr.c         |  43 --
 .../mediatek/vcodec/mtk_vcodec_util.h         |  83 ----
 .../mediatek/vcodec/video-common/Makefile     |  21 +
 .../vcodec/video-common/mtk_vcodec_com_drv.h  | 147 ++++++
 .../{ => video-common}/mtk_vcodec_dbgfs.c     |  57 ++-
 .../{ => video-common}/mtk_vcodec_dbgfs.h     |  22 +-
 .../vcodec/{ => video-common}/mtk_vcodec_fw.c |  12 +-
 .../vcodec/{ => video-common}/mtk_vcodec_fw.h |   6 +-
 .../{ => video-common}/mtk_vcodec_fw_priv.h   |  14 +-
 .../{ => video-common}/mtk_vcodec_fw_scp.c    |  16 +-
 .../{ => video-common}/mtk_vcodec_fw_vpu.c    |  43 +-
 .../vcodec/video-common/mtk_vcodec_intr.c     |  65 +++
 .../{ => video-common}/mtk_vcodec_intr.h      |   6 +-
 .../{ => video-common}/mtk_vcodec_util.c      |  77 ++--
 .../vcodec/video-common/mtk_vcodec_util.h     |  76 ++++
 .../mediatek/vcodec/video-decoder/Makefile    |  25 ++
 .../{ => video-decoder}/mtk_vcodec_dec.c      |  66 ++-
 .../{ => video-decoder}/mtk_vcodec_dec.h      |  10 +-
 .../{ => video-decoder}/mtk_vcodec_dec_drv.c  |  31 +-
 .../mtk_vcodec_dec_drv.h}                     | 424 ++++--------------
 .../{ => video-decoder}/mtk_vcodec_dec_hw.c   |  10 +-
 .../{ => video-decoder}/mtk_vcodec_dec_hw.h   |   6 +-
 .../{ => video-decoder}/mtk_vcodec_dec_pm.c   |  19 +-
 .../{ => video-decoder}/mtk_vcodec_dec_pm.h   |   6 +-
 .../mtk_vcodec_dec_stateful.c                 |  33 +-
 .../mtk_vcodec_dec_stateless.c                |  29 +-
 .../vdec/vdec_av1_req_lat_if.c                | 101 ++---
 .../{ => video-decoder}/vdec/vdec_h264_if.c   |  53 ++-
 .../vdec/vdec_h264_req_common.c               |   4 +-
 .../vdec/vdec_h264_req_common.h               |   6 +-
 .../vdec/vdec_h264_req_if.c                   |  41 +-
 .../vdec/vdec_h264_req_multi_if.c             |  75 ++--
 .../vdec/vdec_hevc_req_multi_if.c             |  71 +--
 .../{ => video-decoder}/vdec/vdec_vp8_if.c    |  57 ++-
 .../vdec/vdec_vp8_req_if.c                    |  47 +-
 .../{ => video-decoder}/vdec/vdec_vp9_if.c    | 136 +++---
 .../vdec/vdec_vp9_req_lat_if.c                |  86 ++--
 .../{ => video-decoder}/vdec_drv_base.h       |   2 +-
 .../vcodec/{ => video-decoder}/vdec_drv_if.c  |   8 +-
 .../vcodec/{ => video-decoder}/vdec_drv_if.h  |  10 +-
 .../vcodec/{ => video-decoder}/vdec_ipi_msg.h |   0
 .../{ => video-decoder}/vdec_msg_queue.c      |  12 +-
 .../{ => video-decoder}/vdec_msg_queue.h      |  12 +-
 .../vcodec/{ => video-decoder}/vdec_vpu_if.c  |  52 ++-
 .../vcodec/{ => video-decoder}/vdec_vpu_if.h  |   6 +-
 .../mediatek/vcodec/video-encoder/Makefile    |  11 +
 .../{ => video-encoder}/mtk_vcodec_enc.c      |  85 ++--
 .../{ => video-encoder}/mtk_vcodec_enc.h      |  12 +-
 .../{ => video-encoder}/mtk_vcodec_enc_drv.c  |  25 +-
 .../vcodec/video-encoder/mtk_vcodec_enc_drv.h | 245 ++++++++++
 .../{ => video-encoder}/mtk_vcodec_enc_pm.c   |   4 +-
 .../{ => video-encoder}/mtk_vcodec_enc_pm.h   |   4 +-
 .../{ => video-encoder}/venc/venc_h264_if.c   |  91 ++--
 .../{ => video-encoder}/venc/venc_vp8_if.c    |  57 ++-
 .../{ => video-encoder}/venc_drv_base.h       |   4 +-
 .../vcodec/{ => video-encoder}/venc_drv_if.c  |  10 +-
 .../vcodec/{ => video-encoder}/venc_drv_if.h  |  11 +-
 .../vcodec/{ => video-encoder}/venc_ipi_msg.h |   0
 .../vcodec/{ => video-encoder}/venc_vpu_if.c  |  57 ++-
 .../vcodec/{ => video-encoder}/venc_vpu_if.h  |   3 +-
 61 files changed, 1502 insertions(+), 1298 deletions(-)
 delete mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.c
 delete mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-common/Makefile
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-common/mtk_vcodec_com_drv.h
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_dbgfs.c (77%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_dbgfs.h (64%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_fw.c (85%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_fw.h (88%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_fw_priv.h (75%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_fw_scp.c (79%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_fw_vpu.c (68%)
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-common/mtk_vcodec_intr.c
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_intr.h (68%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-common}/mtk_vcodec_util.c (59%)
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-common/mtk_vcodec_util.h
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-decoder/Makefile
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec.c (94%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec.h (91%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_drv.c (94%)
 rename drivers/media/platform/mediatek/vcodec/{mtk_vcodec_drv.h => video-decoder/mtk_vcodec_dec_drv.h} (51%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_hw.c (95%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_hw.h (92%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_pm.c (90%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_pm.h (61%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_stateful.c (94%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/mtk_vcodec_dec_stateless.c (95%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_av1_req_lat_if.c (95%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_h264_if.c (87%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_h264_req_common.c (98%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_h264_req_common.h (97%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_h264_req_if.c (91%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_h264_req_multi_if.c (91%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_hevc_req_multi_if.c (93%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_vp8_if.c (88%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_vp8_req_if.c (87%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_vp9_if.c (87%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec/vdec_vp9_req_lat_if.c (95%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_drv_base.h (95%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_drv_if.c (90%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_drv_if.h (89%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_ipi_msg.h (100%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_msg_queue.c (97%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_msg_queue.h (96%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_vpu_if.c (79%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-decoder}/vdec_vpu_if.h (97%)
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-encoder/Makefile
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/mtk_vcodec_enc.c (94%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/mtk_vcodec_enc.h (78%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/mtk_vcodec_enc_drv.c (95%)
 create mode 100644 drivers/media/platform/mediatek/vcodec/video-encoder/mtk_vcodec_enc_drv.h
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/mtk_vcodec_enc_pm.c (95%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/mtk_vcodec_enc_pm.h (78%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc/venc_h264_if.c (89%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc/venc_vp8_if.c (89%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc_drv_base.h (94%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc_drv_if.c (86%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc_drv_if.h (94%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc_ipi_msg.h (100%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc_vpu_if.c (83%)
 rename drivers/media/platform/mediatek/vcodec/{ => video-encoder}/venc_vpu_if.h (96%)

Comments

AngeloGioacchino Del Regno June 1, 2023, 9:29 a.m. UTC | #1
Il 01/06/23 05:02, Yunfei Dong ha scritto:
> 'mtk_vcodec_debug' and 'mtk_vcodec_err' depends on 'mtk_vcodec_ctx'
> to get the index of each instance, using the index directly instead
> of with 'mtk_vcodec_ctx'.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>   .../mediatek/vcodec/mtk_vcodec_util.h         |  36 +++--
>   .../vcodec/vdec/vdec_av1_req_lat_if.c         |  78 +++++------
>   .../mediatek/vcodec/vdec/vdec_h264_if.c       |  46 +++----
>   .../mediatek/vcodec/vdec/vdec_h264_req_if.c   |  32 ++---
>   .../vcodec/vdec/vdec_h264_req_multi_if.c      |  62 ++++-----
>   .../vcodec/vdec/vdec_hevc_req_multi_if.c      |  54 ++++----
>   .../mediatek/vcodec/vdec/vdec_vp8_if.c        |  38 +++---
>   .../mediatek/vcodec/vdec/vdec_vp8_req_if.c    |  36 ++---
>   .../mediatek/vcodec/vdec/vdec_vp9_if.c        | 128 +++++++++---------
>   .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  67 ++++-----
>   .../platform/mediatek/vcodec/vdec_vpu_if.c    |  48 +++----
>   .../mediatek/vcodec/venc/venc_h264_if.c       |  74 +++++-----
>   .../mediatek/vcodec/venc/venc_vp8_if.c        |  44 +++---
>   .../platform/mediatek/vcodec/venc_vpu_if.c    |  54 ++++----
>   14 files changed, 399 insertions(+), 398 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> index ecb0bdf3a4f4..f018af38b39d 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> @@ -31,42 +31,38 @@ struct mtk_vcodec_dev;
>   #define mtk_v4l2_err(fmt, args...)                \
>   	pr_err("[MTK_V4L2][ERROR] " fmt "\n", ##args)
>   
> -#define mtk_vcodec_err(h, fmt, args...)				\
> -	pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n",		\
> -	       ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
> +#define mtk_vcodec_err(inst_id, fmt, args...)                          \
> +	pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n", inst_id, ##args)
>   

This *huge* cleanup (congrats btw!) would be a great occasion to change those
pr_err() to dev_err(). Can we do that?

Regards,
Angelo
AngeloGioacchino Del Regno June 1, 2023, 9:32 a.m. UTC | #2
Il 01/06/23 05:02, Yunfei Dong ha scritto:
> With the driver more and more complex, encoder and decoder need to add more parameter
> in shared struct 'mtk_vcodec_ctx' and 'mtk_vcodec_dev'. Encoder use about 40% and
> decoder use 60% parameter. Need to allocate extra unused memory when encoder and decoder
> working.
> 
> Separate encoder and decoder in different folder and use independent data struct.
> 

Great cleanup initiative! Love it!

My sugggestion here is to change the new folder names to drop "video-":
this is already in folder vcodec/ which means that it is video codec related.

So, instead of having folders "video-common", "video-decoder", "video-encoder",
we should have "common", "decoder", "encoder" instead.

Cheers,
Angelo
Yunfei Dong June 1, 2023, 9:48 a.m. UTC | #3
Hi AngeloGioacchino,

Thanks for your suggestion.
On Thu, 2023-06-01 at 11:32 +0200, AngeloGioacchino Del Regno wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Il 01/06/23 05:02, Yunfei Dong ha scritto:
> > With the driver more and more complex, encoder and decoder need to
> add more parameter
> > in shared struct 'mtk_vcodec_ctx' and 'mtk_vcodec_dev'. Encoder use
> about 40% and
> > decoder use 60% parameter. Need to allocate extra unused memory
> when encoder and decoder
> > working.
> > 
> > Separate encoder and decoder in different folder and use
> independent data struct.
> > 
> 
> Great cleanup initiative! Love it!
> 
> My sugggestion here is to change the new folder names to drop "video-
> ":
> this is already in folder vcodec/ which means that it is video codec
> related.
> 
> So, instead of having folders "video-common", "video-decoder",
> "video-encoder",
> we should have "common", "decoder", "encoder" instead.
> 
> Cheers,
> Angelo
> 
I will change the folder name in the future patch.

Best Regards,
Yunfei Dong
Yunfei Dong June 1, 2023, 9:52 a.m. UTC | #4
Hi AngeloGioacchino,

Thanks for your suggestion.
On Thu, 2023-06-01 at 11:29 +0200, AngeloGioacchino Del Regno wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Il 01/06/23 05:02, Yunfei Dong ha scritto:
> > 'mtk_vcodec_debug' and 'mtk_vcodec_err' depends on 'mtk_vcodec_ctx'
> > to get the index of each instance, using the index directly instead
> > of with 'mtk_vcodec_ctx'.
> > 
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >   .../mediatek/vcodec/mtk_vcodec_util.h         |  36 +++--
> >   .../vcodec/vdec/vdec_av1_req_lat_if.c         |  78 +++++------
> >   .../mediatek/vcodec/vdec/vdec_h264_if.c       |  46 +++----
> >   .../mediatek/vcodec/vdec/vdec_h264_req_if.c   |  32 ++---
> >   .../vcodec/vdec/vdec_h264_req_multi_if.c      |  62 ++++-----
> >   .../vcodec/vdec/vdec_hevc_req_multi_if.c      |  54 ++++----
> >   .../mediatek/vcodec/vdec/vdec_vp8_if.c        |  38 +++---
> >   .../mediatek/vcodec/vdec/vdec_vp8_req_if.c    |  36 ++---
> >   .../mediatek/vcodec/vdec/vdec_vp9_if.c        | 128 +++++++++--
> -------
> >   .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  67 ++++-----
> >   .../platform/mediatek/vcodec/vdec_vpu_if.c    |  48 +++----
> >   .../mediatek/vcodec/venc/venc_h264_if.c       |  74 +++++-----
> >   .../mediatek/vcodec/venc/venc_vp8_if.c        |  44 +++---
> >   .../platform/mediatek/vcodec/venc_vpu_if.c    |  54 ++++----
> >   14 files changed, 399 insertions(+), 398 deletions(-)
> > 
> > diff --git
> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> > index ecb0bdf3a4f4..f018af38b39d 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> > @@ -31,42 +31,38 @@ struct mtk_vcodec_dev;
> >   #define mtk_v4l2_err(fmt, args...)                \
> >   pr_err("[MTK_V4L2][ERROR] " fmt "\n", ##args)
> >   
> > -#define mtk_vcodec_err(h, fmt, args...)\
> > -pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n",\
> > -       ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
> > +#define mtk_vcodec_err(inst_id, fmt,
> args...)                          \
> > +pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n", inst_id, ##args)
> >   
> 
> This *huge* cleanup (congrats btw!) would be a great occasion to
> change those
> pr_err() to dev_err(). Can we do that?
> 

Can change pr_err to dev_err, but need to add one extra parameter "dev"
in mtk_vcodec_err. Whether need to change mtk_vcodec_debug?

> Regards,
> Angelo
> 
Best Regards,
Yunfei Dong
AngeloGioacchino Del Regno June 1, 2023, 10:08 a.m. UTC | #5
Il 01/06/23 11:52, Yunfei Dong (董云飞) ha scritto:
> Hi AngeloGioacchino,
> 
> Thanks for your suggestion.
> On Thu, 2023-06-01 at 11:29 +0200, AngeloGioacchino Del Regno wrote:
>>   	
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>   Il 01/06/23 05:02, Yunfei Dong ha scritto:
>>> 'mtk_vcodec_debug' and 'mtk_vcodec_err' depends on 'mtk_vcodec_ctx'
>>> to get the index of each instance, using the index directly instead
>>> of with 'mtk_vcodec_ctx'.
>>>
>>> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>>> ---
>>>    .../mediatek/vcodec/mtk_vcodec_util.h         |  36 +++--
>>>    .../vcodec/vdec/vdec_av1_req_lat_if.c         |  78 +++++------
>>>    .../mediatek/vcodec/vdec/vdec_h264_if.c       |  46 +++----
>>>    .../mediatek/vcodec/vdec/vdec_h264_req_if.c   |  32 ++---
>>>    .../vcodec/vdec/vdec_h264_req_multi_if.c      |  62 ++++-----
>>>    .../vcodec/vdec/vdec_hevc_req_multi_if.c      |  54 ++++----
>>>    .../mediatek/vcodec/vdec/vdec_vp8_if.c        |  38 +++---
>>>    .../mediatek/vcodec/vdec/vdec_vp8_req_if.c    |  36 ++---
>>>    .../mediatek/vcodec/vdec/vdec_vp9_if.c        | 128 +++++++++--
>> -------
>>>    .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  67 ++++-----
>>>    .../platform/mediatek/vcodec/vdec_vpu_if.c    |  48 +++----
>>>    .../mediatek/vcodec/venc/venc_h264_if.c       |  74 +++++-----
>>>    .../mediatek/vcodec/venc/venc_vp8_if.c        |  44 +++---
>>>    .../platform/mediatek/vcodec/venc_vpu_if.c    |  54 ++++----
>>>    14 files changed, 399 insertions(+), 398 deletions(-)
>>>
>>> diff --git
>> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
>> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
>>> index ecb0bdf3a4f4..f018af38b39d 100644
>>> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
>>> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
>>> @@ -31,42 +31,38 @@ struct mtk_vcodec_dev;
>>>    #define mtk_v4l2_err(fmt, args...)                \
>>>    pr_err("[MTK_V4L2][ERROR] " fmt "\n", ##args)
>>>    
>>> -#define mtk_vcodec_err(h, fmt, args...)\
>>> -pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n",\
>>> -       ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
>>> +#define mtk_vcodec_err(inst_id, fmt,
>> args...)                          \
>>> +pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n", inst_id, ##args)
>>>    
>>
>> This *huge* cleanup (congrats btw!) would be a great occasion to
>> change those
>> pr_err() to dev_err(). Can we do that?
>>
> 
> Can change pr_err to dev_err, but need to add one extra parameter "dev"

Yes I'm aware of the fact that you have to add a `dev` extra param, of course.

> in mtk_vcodec_err. Whether need to change mtk_vcodec_debug?
> 

Yes please, change everything you can to use dev_* prints, that'd be great.

Cheers!

>> Regards,
>> Angelo
>>
> Best Regards,
> Yunfei Dong
Yunfei Dong June 7, 2023, 8:50 a.m. UTC | #6
Hi AngeloGioacchino,

Thanks for your suggestion.
On Thu, 2023-06-01 at 11:32 +0200, AngeloGioacchino Del Regno wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Il 01/06/23 05:02, Yunfei Dong ha scritto:
> > With the driver more and more complex, encoder and decoder need to
> add more parameter
> > in shared struct 'mtk_vcodec_ctx' and 'mtk_vcodec_dev'. Encoder use
> about 40% and
> > decoder use 60% parameter. Need to allocate extra unused memory
> when encoder and decoder
> > working.
> > 
> > Separate encoder and decoder in different folder and use
> independent data struct.
> > 
> 
> Great cleanup initiative! Love it!
> 
> My sugggestion here is to change the new folder names to drop "video-
> ":
> this is already in folder vcodec/ which means that it is video codec
> related.
> 
> So, instead of having folders "video-common", "video-decoder",
> "video-encoder",
> we should have "common", "decoder", "encoder" instead.
> 
> Cheers,
> Angelo
> 
Fixed in patch v2.

Best Regards,
Yunfei Dong