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