| Message ID | 20230912075805.11432-1-moudy.ho@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | add support MDP3 on MT8195 platform | expand |
On Tue, 2023-09-12 at 10:23 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 12/09/2023 09:57, Moudy Ho wrote: > > Add device nodes for Media Data Path 3 (MDP3) modules. > > > > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com> > > --- > > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 378 > +++++++++++++++++++++++ > > 1 file changed, 378 insertions(+) > > Why is this targeting media? No, don't. DTS goes via SoC, not media. > Don't mix patches. > > Best regards, > Krzysztof > Hi Krzysztof, My apologies. I'll split these DTS into separate series. Sincerely, Moudy
Hi Angelo, On Tue, 2023-09-12 at 11:18 +0200, AngeloGioacchino Del Regno wrote: > Il 12/09/23 09:57, Moudy Ho ha scritto: > > MT8195 has two MMSYS sets, VPPSYS0 and VPPSYS1. > > These sets coordinate and control the clock, power, and > > register settings needed for the components of MDP3. > > > > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com> > > --- > > .../platform/mediatek/mdp3/mdp_cfg_data.c | 44 +++++++++----- > > ----- > > .../platform/mediatek/mdp3/mtk-mdp3-comp.h | 1 + > > .../platform/mediatek/mdp3/mtk-mdp3-core.c | 40 +++++++++++--- > > --- > > .../platform/mediatek/mdp3/mtk-mdp3-core.h | 3 ++ > > 4 files changed, 53 insertions(+), 35 deletions(-) (snip) > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > index cc44be10fdb7..9c33d3aaf9cd 100644 > > --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > @@ -26,39 +26,45 @@ static const struct of_device_id mdp_of_ids[] = > > { > > MODULE_DEVICE_TABLE(of, mdp_of_ids); > > > > static struct platform_device *__get_pdev_by_id(struct > > platform_device *pdev, > > + struct platform_device > > *from, > > enum mdp_infra_id id) > > { > > - struct device_node *node; > > + struct device_node *node, *f = NULL; > > struct platform_device *mdp_pdev = NULL; > > const struct mtk_mdp_driver_data *mdp_data; > > const char *compat; > > > > if (!pdev) > > - return NULL; > > + return ERR_PTR(-ENODEV); > > > > Fixing the error handling shall be done in a different commit, which > shall also > have a Fixes tag: this is both for backporting purposes and because > those fixes > are not relevant to adding support for secondary sets of MMSYS (so, > those are > not relevant for this specific commit). > Thanks forreminding me. I will address this error handing separately in a dedicated fix patch. > > if (id < MDP_INFRA_MMSYS || id >= MDP_INFRA_MAX) { > > dev_err(&pdev->dev, "Illegal infra id %d\n", id); > > - return NULL; > > + return ERR_PTR(-ENODEV); > > } > > > > mdp_data = of_device_get_match_data(&pdev->dev); > > if (!mdp_data) { > > dev_err(&pdev->dev, "have no driver data to find > > node\n"); > > - return NULL; > > + return ERR_PTR(-ENODEV); > > } > > + > > compat = mdp_data->mdp_probe_infra[id].compatible; > > + if (strlen(compat) == 0) > > + return NULL; > > > > - node = of_find_compatible_node(NULL, NULL, compat); > > + if (from) > > + f = from->dev.of_node; > > + node = of_find_compatible_node(f, NULL, compat); > > if (WARN_ON(!node)) { > > dev_err(&pdev->dev, "find node from id %d failed\n", > > id); > > - return NULL; > > + return ERR_PTR(-ENODEV); > > } > > > > mdp_pdev = of_find_device_by_node(node); > > of_node_put(node); > > if (WARN_ON(!mdp_pdev)) { > > dev_err(&pdev->dev, "find pdev from id %d failed\n", > > id); > > - return NULL; > > + return ERR_PTR(-ENODEV); > > } > > > > return mdp_pdev; > > @@ -152,7 +158,7 @@ static int mdp_probe(struct platform_device > > *pdev) > > { > > struct device *dev = &pdev->dev; > > struct mdp_dev *mdp; > > - struct platform_device *mm_pdev; > > + struct platform_device *mm_pdev, *mm2_pdev; > > int ret, i, mutex_id; > > > > mdp = kzalloc(sizeof(*mdp), GFP_KERNEL); > > @@ -164,15 +170,23 @@ static int mdp_probe(struct platform_device > > *pdev) > > mdp->pdev = pdev; > > mdp->mdp_data = of_device_get_match_data(&pdev->dev); > > > > - mm_pdev = __get_pdev_by_id(pdev, MDP_INFRA_MMSYS); > > - if (!mm_pdev) { > > + mm_pdev = __get_pdev_by_id(pdev, NULL, MDP_INFRA_MMSYS); > > + if (IS_ERR_OR_NULL(mm_pdev)) { > > ret = -ENODEV; > > goto err_destroy_device; > > } > > mdp->mdp_mmsys = &mm_pdev->dev; > > > > - mm_pdev = __get_pdev_by_id(pdev, MDP_INFRA_MUTEX); > > - if (WARN_ON(!mm_pdev)) { > > + /* MMSYS2 is not available on all chips, so the config may be > > null. */ > > + mm2_pdev = __get_pdev_by_id(pdev, mm_pdev, MDP_INFRA_MMSYS2); > > + if (IS_ERR(mm2_pdev)) { > > + ret = PTR_ERR(mm2_pdev); > > + goto err_destroy_device; > > + } > > + mdp->mdp_mmsys2 = &mm2_pdev->dev; > > + > > + mm_pdev = __get_pdev_by_id(pdev, NULL, MDP_INFRA_MUTEX); > > + if (IS_ERR_OR_NULL(mm_pdev)) { > > ret = -ENODEV; > > goto err_destroy_device; > > } > > @@ -208,7 +222,7 @@ static int mdp_probe(struct platform_device > > *pdev) > > goto err_destroy_job_wq; > > } > > > > - mm_pdev = __get_pdev_by_id(pdev, MDP_INFRA_SCP); > > + mm_pdev = __get_pdev_by_id(pdev, NULL, MDP_INFRA_SCP); > > if (WARN_ON(!mm_pdev)) { > > dev_err(&pdev->dev, "Could not get scp device\n"); > > ret = -ENODEV; > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > index 7e21d226ceb8..0434b70e1fc9 100644 > > --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > @@ -20,6 +20,7 @@ > > > > enum mdp_infra_id { > > MDP_INFRA_MMSYS, > > + MDP_INFRA_MMSYS2, > > MDP_INFRA_MUTEX, > > MDP_INFRA_SCP, > > MDP_INFRA_MAX > > @@ -68,6 +69,7 @@ struct mtk_mdp_driver_data { > > struct mdp_dev { > > struct platform_device *pdev; > > struct device *mdp_mmsys; > > + struct device *mdp_mmsys2; > > I'm wondering if this would become more readable by doing it like so: > > struct mdp_dev_infra { > struct device *mmsys; > struct mtk_mutex *mtk_mutex[MDP_PIPE_MAX]; > } > > struct mdp_dev { > struct platform_device *pdev; > struct mdp_dev_infra mdp_infra; > ..... > } > > so that you can add the secondary MMSYS (that goes along with the > secondary > MUTEX anyway) by then changing mdp_infra to > > struct mtk_dev_infra mdp_infra[NUM_MMSYS]; > > and then referencing that like > > mdp->mdp_infra[MDP_INFRA_MMSYS0]->mmsys > mdp->mdp_infra[MDP_INFRA_MMSYS0]->mtk_mutex > mdp->mdp_infra[MDP_INFRA_MMSYS1]->mmsys > mdp->mdp_infra[MDP_INFRA_MMSYS1]->mtk_mutex > > What do you think? > > Regards, > Angelo > Thanks for the advice. I will follow your suggestion to simplify the settings. Sincerely, Moudy > > struct mtk_mutex *mdp_mutex[MDP_PIPE_MAX]; > > struct mdp_comp *comp[MDP_MAX_COMP_ > > COUNT]; > > const struct mtk_mdp_driver_data *mdp_data; > > @@ -96,6 +98,7 @@ struct mdp_dev { > > > > struct mdp_pipe_info { > > enum mdp_pipe_id pipe_id; > > + u32 mmsys_id; > > u32 mutex_id; > > }; > > > >
On Fri, 2023-09-15 at 12:27 +0200, Hans Verkuil wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi Moudy, > > On 12/09/2023 09:57, Moudy Ho wrote: > > Changes since v4: > > - Rebase on v6.6-rc1 > > - Remove any unnecessary DTS settings. > > - Adjust the usage of MOD and clock in blending components. > > > > Changes since v3: > > - Depend on : > > [1] > https://patchwork.kernel.org/project/linux-media/list/?series=719841 > > - Suggested by Krzysztof, integrating all newly added bindings for > > the mt8195 MDP3 into the file "mediatek,mt8195-mdp3.yaml". > > - Revise MDP3 nodes with generic names. > > > > Changes since v2: > > - Depend on : > > [1] MMSYS/MUTEX: > https://patchwork.kernel.org/project/linux-mediatek/list/?series=711592 > > [2] MDP3: > https://patchwork.kernel.org/project/linux-mediatek/list/?series=711618 > > - Suggested by Rob to revise MDP3 bindings to pass dtbs check > > - Add parallel paths feature. > > - Add blended components settings. > > > > Changes since v1: > > - Depend on : > > [1] MDP3 : > https://patchwork.kernel.org/project/linux-mediatek/list/?series=698872 > > [2] MMSYS/MUTEX: > https://patchwork.kernel.org/project/linux-mediatek/list/?series=684959 > > - Fix compilation failure due to use of undeclared identifier in > file "mtk-mdp3-cmdq.c" > > > > Hello, > > > > This patch is used to add support for MDP3 on the MT8195 platform > that > > contains more picture quality components, and can arrange more > pipelines > > through two sets of MMSYS and MUTEX respectively. > > I ran this series through our build system and I got the following > compile warning: > > drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c: In function > 'mdp_path_config.isra': > drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:449:51: warning: > 'ctx' may be used uninitialized [-Wmaybe-uninitialized] > 449 | out = CFG_COMP(MT8195, ctx->param, > outputs[0]); > | ~~~^~~~~~~ > drivers/media/platform/mediatek/mdp3/mtk-img-ipi.h:137:25: note: in > definition of macro 'CFG_COMP' > 137 | (IS_ERR_OR_NULL(comp) ? 0 : _CFG_COMP(plat, comp, > mem)) > | ^~~~ > drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:402:30: note: > 'ctx' was declared here > 402 | struct mdp_comp_ctx *ctx; > | ^~~ > > And also a few smatch warnings/errors: > > drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c:871 > wait_wrot_event() warn: variable dereferenced before check 'mdp_cfg' > (see line 864) > drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c:1024 > reset_luma_hist() warn: variable dereferenced before check 'mdp_cfg' > (see line 1015) > drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:447 > mdp_path_config() error: potentially dereferencing uninitialized > 'ctx'. > drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c:449 > mdp_path_config() error: potentially dereferencing uninitialized > 'ctx'. > > You can run the same tests yourself, see this announcement: > > https://lore.kernel.org/linux-media/18989016-6392-a77b-6cf7-1223c9161def@xs4all.nl/ > > Regards, > > Hans > Hi Hans, Thanks for the reminder. I will run the build script to test this series and rectify any warnings. Sincerely, Moudy > > > > Moudy Ho (14): > > arm64: dts: mediatek: mt8183: correct MDP3 DMA-related nodes > > arm64: dts: mediatek: mt8195: add MDP3 nodes > > media: platform: mtk-mdp3: add support second sets of MMSYS > > media: platform: mtk-mdp3: add support second sets of MUTEX > > media: platform: mtk-mdp3: introduce more pipelines from MT8195 > > media: platform: mtk-mdp3: introduce more MDP3 components > > media: platform: mtk-mdp3: add checks for dummy components > > media: platform: mtk-mdp3: avoid multiple driver registrations > > media: platform: mtk-mdp3: extend GCE event waiting in RDMA and > WROT > > media: platform: mtk-mdp3: add support for blending multiple > > components > > media: platform: mtk-mdp3: add mt8195 platform configuration > > media: platform: mtk-mdp3: add mt8195 shared memory > configurations > > media: platform: mtk-mdp3: add mt8195 MDP3 component settings > > media: platform: mtk-mdp3: add support for parallel pipe to > improve > > FPS > > > > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6 +- > > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 378 ++++++++ > > .../platform/mediatek/mdp3/mdp_cfg_data.c | 729 > ++++++++++++++- > > .../platform/mediatek/mdp3/mdp_reg_aal.h | 25 + > > .../platform/mediatek/mdp3/mdp_reg_color.h | 31 + > > .../media/platform/mediatek/mdp3/mdp_reg_fg.h | 23 + > > .../platform/mediatek/mdp3/mdp_reg_hdr.h | 31 + > > .../platform/mediatek/mdp3/mdp_reg_merge.h | 25 + > > .../platform/mediatek/mdp3/mdp_reg_ovl.h | 25 + > > .../platform/mediatek/mdp3/mdp_reg_pad.h | 21 + > > .../platform/mediatek/mdp3/mdp_reg_rdma.h | 24 + > > .../platform/mediatek/mdp3/mdp_reg_rsz.h | 2 + > > .../platform/mediatek/mdp3/mdp_reg_tdshp.h | 34 + > > .../platform/mediatek/mdp3/mdp_reg_wrot.h | 8 + > > .../platform/mediatek/mdp3/mdp_sm_mt8195.h | 283 ++++++ > > .../platform/mediatek/mdp3/mtk-img-ipi.h | 4 + > > .../platform/mediatek/mdp3/mtk-mdp3-cfg.h | 2 + > > .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 447 +++++++-- > > .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h | 1 + > > .../platform/mediatek/mdp3/mtk-mdp3-comp.c | 860 > +++++++++++++++++- > > .../platform/mediatek/mdp3/mtk-mdp3-comp.h | 93 +- > > .../platform/mediatek/mdp3/mtk-mdp3-core.c | 103 ++- > > .../platform/mediatek/mdp3/mtk-mdp3-core.h | 33 +- > > .../platform/mediatek/mdp3/mtk-mdp3-m2m.c | 15 + > > .../platform/mediatek/mdp3/mtk-mdp3-regs.c | 18 + > > .../platform/mediatek/mdp3/mtk-mdp3-regs.h | 1 + > > .../platform/mediatek/mdp3/mtk-mdp3-vpu.c | 3 +- > > 27 files changed, 3051 insertions(+), 174 deletions(-) > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_aal.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_color.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_fg.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_hdr.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_merge.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_ovl.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_pad.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_reg_tdshp.h > > create mode 100644 > drivers/media/platform/mediatek/mdp3/mdp_sm_mt8195.h > > >
Changes since v4: - Rebase on v6.6-rc1 - Remove any unnecessary DTS settings. - Adjust the usage of MOD and clock in blending components. Changes since v3: - Depend on : [1] https://patchwork.kernel.org/project/linux-media/list/?series=719841 - Suggested by Krzysztof, integrating all newly added bindings for the mt8195 MDP3 into the file "mediatek,mt8195-mdp3.yaml". - Revise MDP3 nodes with generic names. Changes since v2: - Depend on : [1] MMSYS/MUTEX: https://patchwork.kernel.org/project/linux-mediatek/list/?series=711592 [2] MDP3: https://patchwork.kernel.org/project/linux-mediatek/list/?series=711618 - Suggested by Rob to revise MDP3 bindings to pass dtbs check - Add parallel paths feature. - Add blended components settings. Changes since v1: - Depend on : [1] MDP3 : https://patchwork.kernel.org/project/linux-mediatek/list/?series=698872 [2] MMSYS/MUTEX: https://patchwork.kernel.org/project/linux-mediatek/list/?series=684959 - Fix compilation failure due to use of undeclared identifier in file "mtk-mdp3-cmdq.c" Hello, This patch is used to add support for MDP3 on the MT8195 platform that contains more picture quality components, and can arrange more pipelines through two sets of MMSYS and MUTEX respectively. Moudy Ho (14): arm64: dts: mediatek: mt8183: correct MDP3 DMA-related nodes arm64: dts: mediatek: mt8195: add MDP3 nodes media: platform: mtk-mdp3: add support second sets of MMSYS media: platform: mtk-mdp3: add support second sets of MUTEX media: platform: mtk-mdp3: introduce more pipelines from MT8195 media: platform: mtk-mdp3: introduce more MDP3 components media: platform: mtk-mdp3: add checks for dummy components media: platform: mtk-mdp3: avoid multiple driver registrations media: platform: mtk-mdp3: extend GCE event waiting in RDMA and WROT media: platform: mtk-mdp3: add support for blending multiple components media: platform: mtk-mdp3: add mt8195 platform configuration media: platform: mtk-mdp3: add mt8195 shared memory configurations media: platform: mtk-mdp3: add mt8195 MDP3 component settings media: platform: mtk-mdp3: add support for parallel pipe to improve FPS arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6 +- arch/arm64/boot/dts/mediatek/mt8195.dtsi | 378 ++++++++ .../platform/mediatek/mdp3/mdp_cfg_data.c | 729 ++++++++++++++- .../platform/mediatek/mdp3/mdp_reg_aal.h | 25 + .../platform/mediatek/mdp3/mdp_reg_color.h | 31 + .../media/platform/mediatek/mdp3/mdp_reg_fg.h | 23 + .../platform/mediatek/mdp3/mdp_reg_hdr.h | 31 + .../platform/mediatek/mdp3/mdp_reg_merge.h | 25 + .../platform/mediatek/mdp3/mdp_reg_ovl.h | 25 + .../platform/mediatek/mdp3/mdp_reg_pad.h | 21 + .../platform/mediatek/mdp3/mdp_reg_rdma.h | 24 + .../platform/mediatek/mdp3/mdp_reg_rsz.h | 2 + .../platform/mediatek/mdp3/mdp_reg_tdshp.h | 34 + .../platform/mediatek/mdp3/mdp_reg_wrot.h | 8 + .../platform/mediatek/mdp3/mdp_sm_mt8195.h | 283 ++++++ .../platform/mediatek/mdp3/mtk-img-ipi.h | 4 + .../platform/mediatek/mdp3/mtk-mdp3-cfg.h | 2 + .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 447 +++++++-- .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h | 1 + .../platform/mediatek/mdp3/mtk-mdp3-comp.c | 860 +++++++++++++++++- .../platform/mediatek/mdp3/mtk-mdp3-comp.h | 93 +- .../platform/mediatek/mdp3/mtk-mdp3-core.c | 103 ++- .../platform/mediatek/mdp3/mtk-mdp3-core.h | 33 +- .../platform/mediatek/mdp3/mtk-mdp3-m2m.c | 15 + .../platform/mediatek/mdp3/mtk-mdp3-regs.c | 18 + .../platform/mediatek/mdp3/mtk-mdp3-regs.h | 1 + .../platform/mediatek/mdp3/mtk-mdp3-vpu.c | 3 +- 27 files changed, 3051 insertions(+), 174 deletions(-) create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_aal.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_color.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_fg.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_hdr.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_merge.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_ovl.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_pad.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_tdshp.h create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_sm_mt8195.h