| Message ID | 20230722074230.30558-1-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: mediatek: vcodec: separate encoder and decoder | expand |
On 25/07/2023 11:58, Hans Verkuil wrote: > On 22/07/2023 09:42, Yunfei Dong wrote: >> From: Yunfei Dong <yunfei.dong@mediatek.corp-partner.google.com> >> >> 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~6 remove the dependency of debug log >> patch 7~8 separate mtk_vcodec_ctx and mtk_vcodec_dev >> patch 9 fix unreasonable parameter >> patch 10 removed unused header files >> patch 11 separate encoder and decoder in different folder >> --- >> Changed from v6: >> - rebase to: https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=fo-v6.6g. >> Changed from v5: >> - fix some words error for patch 3/6/11. >> - rename mtk_vcodec_comm_drv.h to mtk_vcodec_cmn_drv.h for patch 7. >> Changed from v4: >> - add one parameter to record register base for reg_base for patch 3. >> - add debug string for non ctx log for patch 6. >> - change the comment of struct mtk_vcodec_dec_ctx and struct mtk_vcodec_enc_ctx for patch 7. >> - prefer to use struct mtk_vcodec_dec_dev an current period, will re-construct in the future for patch 8. >> Changed from v3: >> - re-write commit message for patch 3. >> Changed from v2: >> - This patch main changed: >> 1: add different macro mtk_dec_debug and mtk_enc_debug calling common >> macro mtk_vcodec_debug in order to use dev_dbg instead of pr_debug. >> 2: add different macro mtk_v4l2_venc_dbg and mtk_v4l2_vdec_dbg calling common >> macro in order to use dev_dbg instead of pr_debug. >> Changed from v1: >> - Change pr_dbg to dev_dbg for mtk_v4l2_level and mtk_vcodec_dbg for patch 4. >> - Change pr_err to dev_err for mtk_v4l2_err and mtk_vcodec_err for patch 5. >> - Fix unreasonable parameter for patch 8. >> --- >> Yunfei Dong (11): >> media: mediatek: vcodec: remove unused parameter >> media: mediatek: vcodec: align fw interface >> media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' for >> shared interface >> media: mediatek: vcodec: Removing useless debug log >> media: mediatek: vcodec: remove the dependency of vcodec debug log >> media: mediatek: vcodec: replace pr_* with dev_* for v4l2 debug >> message >> media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx' >> media: mediatek: vcodec: separate struct mtk_vcodec_dev >> media: mediatek: vcodec: fix unreasonable parameter definition and >> style >> media: mediatek: vcodec: remove unused include header >> media: mediatek: vcodec: separate decoder and encoder > > Besides the missing argument in patch 6/11 I also get this compiler warning: > > drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c: In function 'vpu_enc_ipi_handler': > drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c:64:31: warning: 'vpu' may be used uninitialized [-Wmaybe-uninitialized] > 64 | struct venc_vpu_inst *vpu; > | ^~~ > > and this smatch error: > > drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:138 mtk_vcodec_get_reg_bases() error: buffer overflow 'mtk_dec_reg_names' 11 <= 11 > > However, I believe that was introduced by Nicolas' patch series. > > I'll try to pinpoint the precise patch. That smatch error is now found and fixed in the staging tree. Can you post a v8 fixing the other issues? Thanks! Regards, Hans > > Regards, > > Hans
Hi Hans, Thanks for your help and suggestion. On Fri, 2023-07-28 at 11:25 +0200, Hans Verkuil wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 25/07/2023 11:58, Hans Verkuil wrote: > > On 22/07/2023 09:42, Yunfei Dong wrote: > >> From: Yunfei Dong <yunfei.dong@mediatek.corp-partner.google.com> > >> > >> 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~6 remove the dependency of debug log > >> patch 7~8 separate mtk_vcodec_ctx and mtk_vcodec_dev > >> patch 9 fix unreasonable parameter > >> patch 10 removed unused header files > >> patch 11 separate encoder and decoder in different folder > >> --- > >> Changed from v6: > >> - rebase to: > https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=fo-v6.6g. > >> Changed from v5: > >> - fix some words error for patch 3/6/11. > >> - rename mtk_vcodec_comm_drv.h to mtk_vcodec_cmn_drv.h for patch > 7. > >> Changed from v4: > >> - add one parameter to record register base for reg_base for patch > 3. > >> - add debug string for non ctx log for patch 6. > >> - change the comment of struct mtk_vcodec_dec_ctx and struct > mtk_vcodec_enc_ctx for patch 7. > >> - prefer to use struct mtk_vcodec_dec_dev an current period, will > re-construct in the future for patch 8. > >> Changed from v3: > >> - re-write commit message for patch 3. > >> Changed from v2: > >> - This patch main changed: > >> 1: add different macro mtk_dec_debug and mtk_enc_debug calling > common > >> macro mtk_vcodec_debug in order to use dev_dbg instead of > pr_debug. > >> 2: add different macro mtk_v4l2_venc_dbg and mtk_v4l2_vdec_dbg > calling common > >> macro in order to use dev_dbg instead of pr_debug. > >> Changed from v1: > >> - Change pr_dbg to dev_dbg for mtk_v4l2_level and mtk_vcodec_dbg > for patch 4. > >> - Change pr_err to dev_err for mtk_v4l2_err and mtk_vcodec_err for > patch 5. > >> - Fix unreasonable parameter for patch 8. > >> --- > >> Yunfei Dong (11): > >> media: mediatek: vcodec: remove unused parameter > >> media: mediatek: vcodec: align fw interface > >> media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' > for > >> shared interface > >> media: mediatek: vcodec: Removing useless debug log > >> media: mediatek: vcodec: remove the dependency of vcodec debug > log > >> media: mediatek: vcodec: replace pr_* with dev_* for v4l2 debug > >> message > >> media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx' > >> media: mediatek: vcodec: separate struct mtk_vcodec_dev > >> media: mediatek: vcodec: fix unreasonable parameter definition > and > >> style > >> media: mediatek: vcodec: remove unused include header > >> media: mediatek: vcodec: separate decoder and encoder > > > > Besides the missing argument in patch 6/11 I also get this compiler > warning: > > > > drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c: In > function 'vpu_enc_ipi_handler': > > drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c:64:31: > warning: 'vpu' may be used uninitialized [-Wmaybe-uninitialized] > > 64 | struct venc_vpu_inst *vpu; > > | ^~~ > > > > and this smatch error: > > > > > drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:1 > 38 mtk_vcodec_get_reg_bases() error: buffer overflow > 'mtk_dec_reg_names' 11 <= 11 > > > > However, I believe that was introduced by Nicolas' patch series. > > > > I'll try to pinpoint the precise patch. > > That smatch error is now found and fixed in the staging tree. > > Can you post a v8 fixing the other issues? > > Thanks! The build fail for patch 06 and smatch fail will be fixed in next patch. Best Regards, Yunfei Dong > > Regards, > > Hans > > > > > Regards, > > > > Hans >
From: Yunfei Dong <yunfei.dong@mediatek.corp-partner.google.com> 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~6 remove the dependency of debug log patch 7~8 separate mtk_vcodec_ctx and mtk_vcodec_dev patch 9 fix unreasonable parameter patch 10 removed unused header files patch 11 separate encoder and decoder in different folder --- Changed from v6: - rebase to: https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=fo-v6.6g. Changed from v5: - fix some words error for patch 3/6/11. - rename mtk_vcodec_comm_drv.h to mtk_vcodec_cmn_drv.h for patch 7. Changed from v4: - add one parameter to record register base for reg_base for patch 3. - add debug string for non ctx log for patch 6. - change the comment of struct mtk_vcodec_dec_ctx and struct mtk_vcodec_enc_ctx for patch 7. - prefer to use struct mtk_vcodec_dec_dev an current period, will re-construct in the future for patch 8. Changed from v3: - re-write commit message for patch 3. Changed from v2: - This patch main changed: 1: add different macro mtk_dec_debug and mtk_enc_debug calling common macro mtk_vcodec_debug in order to use dev_dbg instead of pr_debug. 2: add different macro mtk_v4l2_venc_dbg and mtk_v4l2_vdec_dbg calling common macro in order to use dev_dbg instead of pr_debug. Changed from v1: - Change pr_dbg to dev_dbg for mtk_v4l2_level and mtk_vcodec_dbg for patch 4. - Change pr_err to dev_err for mtk_v4l2_err and mtk_vcodec_err for patch 5. - Fix unreasonable parameter for patch 8. --- Yunfei Dong (11): media: mediatek: vcodec: remove unused parameter media: mediatek: vcodec: align fw interface media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' for shared interface media: mediatek: vcodec: Removing useless debug log media: mediatek: vcodec: remove the dependency of vcodec debug log media: mediatek: vcodec: replace pr_* with dev_* for v4l2 debug message media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx' media: mediatek: vcodec: separate struct mtk_vcodec_dev media: mediatek: vcodec: fix unreasonable parameter definition and style media: mediatek: vcodec: remove unused include header media: mediatek: vcodec: separate decoder and encoder .../media/platform/mediatek/vcodec/Makefile | 55 +- .../platform/mediatek/vcodec/common/Makefile | 21 + .../vcodec/common/mtk_vcodec_cmn_drv.h | 147 +++++ .../vcodec/{ => common}/mtk_vcodec_dbgfs.c | 55 +- .../vcodec/{ => common}/mtk_vcodec_dbgfs.h | 24 +- .../vcodec/{ => common}/mtk_vcodec_fw.c | 21 +- .../vcodec/{ => common}/mtk_vcodec_fw.h | 8 +- .../vcodec/{ => common}/mtk_vcodec_fw_priv.h | 14 +- .../vcodec/{ => common}/mtk_vcodec_fw_scp.c | 26 +- .../vcodec/{ => common}/mtk_vcodec_fw_vpu.c | 64 +- .../mediatek/vcodec/common/mtk_vcodec_intr.c | 68 +++ .../vcodec/{ => common}/mtk_vcodec_intr.h | 6 +- .../vcodec/{ => common}/mtk_vcodec_util.c | 71 +-- .../mediatek/vcodec/common/mtk_vcodec_util.h | 74 +++ .../platform/mediatek/vcodec/decoder/Makefile | 25 + .../vcodec/{ => decoder}/mtk_vcodec_dec.c | 182 +++--- .../vcodec/{ => decoder}/mtk_vcodec_dec.h | 10 +- .../vcodec/{ => decoder}/mtk_vcodec_dec_drv.c | 87 ++- .../vcodec/decoder/mtk_vcodec_dec_drv.h | 317 ++++++++++ .../vcodec/{ => decoder}/mtk_vcodec_dec_hw.c | 19 +- .../vcodec/{ => decoder}/mtk_vcodec_dec_hw.h | 6 +- .../vcodec/{ => decoder}/mtk_vcodec_dec_pm.c | 38 +- .../vcodec/{ => decoder}/mtk_vcodec_dec_pm.h | 6 +- .../{ => decoder}/mtk_vcodec_dec_stateful.c | 176 +++--- .../{ => decoder}/mtk_vcodec_dec_stateless.c | 91 +-- .../{ => decoder}/vdec/vdec_av1_req_lat_if.c | 158 +++-- .../vcodec/{ => decoder}/vdec/vdec_h264_if.c | 79 ++- .../{ => decoder}/vdec/vdec_h264_req_common.c | 4 +- .../{ => decoder}/vdec/vdec_h264_req_common.h | 6 +- .../{ => decoder}/vdec/vdec_h264_req_if.c | 75 ++- .../vdec/vdec_h264_req_multi_if.c | 157 +++-- .../vdec/vdec_hevc_req_multi_if.c | 129 ++-- .../vcodec/{ => decoder}/vdec/vdec_vp8_if.c | 70 ++- .../{ => decoder}/vdec/vdec_vp8_req_if.c | 81 ++- .../vcodec/{ => decoder}/vdec/vdec_vp9_if.c | 132 ++--- .../{ => decoder}/vdec/vdec_vp9_req_lat_if.c | 129 ++-- .../vcodec/{ => decoder}/vdec_drv_base.h | 2 +- .../vcodec/{ => decoder}/vdec_drv_if.c | 12 +- .../vcodec/{ => decoder}/vdec_drv_if.h | 10 +- .../vcodec/{ => decoder}/vdec_ipi_msg.h | 0 .../vcodec/{ => decoder}/vdec_msg_queue.c | 64 +- .../vcodec/{ => decoder}/vdec_msg_queue.h | 14 +- .../vcodec/{ => decoder}/vdec_vpu_if.c | 57 +- .../vcodec/{ => decoder}/vdec_vpu_if.h | 6 +- .../platform/mediatek/vcodec/encoder/Makefile | 11 + .../vcodec/{ => encoder}/mtk_vcodec_enc.c | 296 +++++----- .../vcodec/{ => encoder}/mtk_vcodec_enc.h | 12 +- .../vcodec/{ => encoder}/mtk_vcodec_enc_drv.c | 73 +-- .../vcodec/encoder/mtk_vcodec_enc_drv.h | 246 ++++++++ .../vcodec/{ => encoder}/mtk_vcodec_enc_pm.c | 12 +- .../vcodec/{ => encoder}/mtk_vcodec_enc_pm.h | 4 +- .../vcodec/{ => encoder}/venc/venc_h264_if.c | 110 ++-- .../vcodec/{ => encoder}/venc/venc_vp8_if.c | 69 +-- .../vcodec/{ => encoder}/venc_drv_base.h | 4 +- .../vcodec/{ => encoder}/venc_drv_if.c | 10 +- .../vcodec/{ => encoder}/venc_drv_if.h | 11 +- .../vcodec/{ => encoder}/venc_ipi_msg.h | 0 .../vcodec/{ => encoder}/venc_vpu_if.c | 75 +-- .../vcodec/{ => encoder}/venc_vpu_if.h | 3 +- .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 550 ------------------ .../mediatek/vcodec/mtk_vcodec_intr.c | 43 -- .../mediatek/vcodec/mtk_vcodec_util.h | 85 --- 62 files changed, 2222 insertions(+), 2188 deletions(-) create mode 100644 drivers/media/platform/mediatek/vcodec/common/Makefile create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_cmn_drv.h rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_dbgfs.c (77%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_dbgfs.h (62%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_fw.c (75%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_fw.h (86%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_fw_priv.h (75%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_fw_scp.c (70%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_fw_vpu.c (58%) create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_intr.c rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_intr.h (68%) rename drivers/media/platform/mediatek/vcodec/{ => common}/mtk_vcodec_util.c (56%) create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.h create mode 100644 drivers/media/platform/mediatek/vcodec/decoder/Makefile rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec.c (83%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec.h (91%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_drv.c (84%) create mode 100644 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_hw.c (91%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_hw.h (92%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_pm.c (81%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_pm.h (61%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_stateful.c (73%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/mtk_vcodec_dec_stateless.c (84%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_av1_req_lat_if.c (93%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_h264_if.c (84%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_h264_req_common.c (98%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_h264_req_common.h (97%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_h264_req_if.c (86%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_h264_req_multi_if.c (85%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_hevc_req_multi_if.c (90%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_vp8_if.c (88%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_vp8_req_if.c (81%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_vp9_if.c (87%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec/vdec_vp9_req_lat_if.c (94%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_drv_base.h (95%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_drv_if.c (86%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_drv_if.h (89%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_ipi_msg.h (100%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_msg_queue.c (82%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_msg_queue.h (95%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_vpu_if.c (79%) rename drivers/media/platform/mediatek/vcodec/{ => decoder}/vdec_vpu_if.h (97%) create mode 100644 drivers/media/platform/mediatek/vcodec/encoder/Makefile rename drivers/media/platform/mediatek/vcodec/{ => encoder}/mtk_vcodec_enc.c (82%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/mtk_vcodec_enc.h (78%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/mtk_vcodec_enc_drv.c (86%) create mode 100644 drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h rename drivers/media/platform/mediatek/vcodec/{ => encoder}/mtk_vcodec_enc_pm.c (83%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/mtk_vcodec_enc_pm.h (78%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc/venc_h264_if.c (88%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc/venc_vp8_if.c (88%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc_drv_base.h (94%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc_drv_if.c (86%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc_drv_if.h (94%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc_ipi_msg.h (100%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc_vpu_if.c (82%) rename drivers/media/platform/mediatek/vcodec/{ => encoder}/venc_vpu_if.h (96%) delete mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h delete mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.c delete mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h