Hi Mody Ho, Thank you for your patch. Some comments below. Missatge de Moudy Ho <moudy.ho@xxxxxxxxxxxx> del dia dc., 23 de juny 2021 a les 9:36: > > From: mtk18742 <moudy.ho@xxxxxxxxxxxx> > Please fix your git configuration, I assume this should be something like Moudy Ho <moudy.ho@xxxxxxxxxxxx>. Note that the author and the Signed-off must match. > Add MDP driver for MT8183 > In general please use a more verbose commit messages, you know what MDP means (I probably also know, but not everyone knows what MDP means) It is a good practice to when you introduce a new driver to explain well what is and for what is used. The first question that I have here is why current MDP driver (that supports other Mediatek chips like the MT8173) can't be extended to support MT8183. How different is this MDP driver and why can't share the code. This single patch is huge, and is unlikely that anyone spend time reviewing it just for that. I'd recommend to split it and introduce step-by-step, functionality by functionally. I also have the impression that you're just reimplementing things that are actually upstream or not taking advantage of what is already upstream. i.e the access to the mmsys registers. Please use current kernel drivers/interfaces API instead of reimplementing it. I'd strongly recommend to look at current Mediatek mdp driver and see if you can add support for MT8183 there, and if not, looks at it as reference. Best regards, Enric > Signed-off-by: Moudy Ho <moudy.ho@xxxxxxxxxxxx> > --- > drivers/media/platform/Kconfig | 17 + > drivers/media/platform/Makefile | 2 + > drivers/media/platform/mtk-mdp3/Makefile | 9 + > drivers/media/platform/mtk-mdp3/isp_reg.h | 37 + > .../media/platform/mtk-mdp3/mdp-platform.h | 58 + > .../media/platform/mtk-mdp3/mdp_reg_ccorr.h | 75 + > .../media/platform/mtk-mdp3/mdp_reg_rdma.h | 206 +++ > drivers/media/platform/mtk-mdp3/mdp_reg_rsz.h | 109 ++ > .../media/platform/mtk-mdp3/mdp_reg_wdma.h | 125 ++ > .../media/platform/mtk-mdp3/mdp_reg_wrot.h | 115 ++ > .../media/platform/mtk-mdp3/mmsys_config.h | 188 +++ > drivers/media/platform/mtk-mdp3/mmsys_mutex.h | 35 + > .../media/platform/mtk-mdp3/mmsys_reg_base.h | 38 + > drivers/media/platform/mtk-mdp3/mtk-img-ipi.h | 282 ++++ > .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.c | 521 +++++++ > .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.h | 54 + > .../media/platform/mtk-mdp3/mtk-mdp3-comp.c | 1329 +++++++++++++++++ > .../media/platform/mtk-mdp3/mtk-mdp3-comp.h | 155 ++ > .../media/platform/mtk-mdp3/mtk-mdp3-core.c | 282 ++++ > .../media/platform/mtk-mdp3/mtk-mdp3-core.h | 86 ++ > .../media/platform/mtk-mdp3/mtk-mdp3-debug.c | 973 ++++++++++++ > .../media/platform/mtk-mdp3/mtk-mdp3-debug.h | 39 + > .../media/platform/mtk-mdp3/mtk-mdp3-m2m.c | 804 ++++++++++ > .../media/platform/mtk-mdp3/mtk-mdp3-m2m.h | 42 + > .../media/platform/mtk-mdp3/mtk-mdp3-regs.c | 748 ++++++++++ > .../media/platform/mtk-mdp3/mtk-mdp3-regs.h | 373 +++++ > .../media/platform/mtk-mdp3/mtk-mdp3-vpu.c | 313 ++++ > .../media/platform/mtk-mdp3/mtk-mdp3-vpu.h | 79 + > 28 files changed, 7094 insertions(+) > create mode 100644 drivers/media/platform/mtk-mdp3/Makefile > create mode 100644 drivers/media/platform/mtk-mdp3/isp_reg.h > create mode 100644 drivers/media/platform/mtk-mdp3/mdp-platform.h > create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_ccorr.h > create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_rdma.h > create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_rsz.h > create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_wdma.h > create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_wrot.h > create mode 100644 drivers/media/platform/mtk-mdp3/mmsys_config.h > create mode 100644 drivers/media/platform/mtk-mdp3/mmsys_mutex.h > create mode 100644 drivers/media/platform/mtk-mdp3/mmsys_reg_base.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-img-ipi.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-core.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-debug.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-debug.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.h > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-vpu.c > create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-vpu.h [snip] > -- > 2.18.0 >