On 28/07/2021 07:34, Nancy.Lin wrote: > Hi Enric, > > Thanks for your review. > > On Fri, 2021-07-23 at 13:05 +0200, Enric Balletbo Serra wrote: >> Hi Nancy, >> >> Thank you for your patch. >> >> Missatge de Nancy.Lin <nancy.lin@xxxxxxxxxxxx> del dia dj., 22 de >> jul. >> 2021 a les 11:45: >>> >>> Add mt8195 vdosys1 clock driver name and routing table to >>> the driver data of mtk-mmsys. >>> >>> Signed-off-by: Nancy.Lin <nancy.lin@xxxxxxxxxxxx> >>> --- >>> drivers/soc/mediatek/mt8195-mmsys.h | 83 >>> ++++++++++++++++++++++++-- >>> drivers/soc/mediatek/mtk-mmsys.c | 10 ++++ >>> include/linux/soc/mediatek/mtk-mmsys.h | 2 + >>> 3 files changed, 90 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/soc/mediatek/mt8195-mmsys.h >>> b/drivers/soc/mediatek/mt8195-mmsys.h >>> index 73e9e8286d50..104ba575f765 100644 >>> --- a/drivers/soc/mediatek/mt8195-mmsys.h >>> +++ b/drivers/soc/mediatek/mt8195-mmsys.h >>> @@ -64,16 +64,16 @@ >>> #define SOUT_TO_VPP_MERGE0_P1_SEL (1 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MERGE0_ASYNC_SOUT_SEL 0xf40 >>> -#define SOUT_TO_HDR_VDO_FE0 (0 >>> << 0) >> >> This definition was introduced in this patch [1] that didn't land >> yet. >> And you're removing it now. Could you sync with Jason and only >> introduce the bits that are needed for your patches. Also all the >> comments I made to the Jason's patch apply here. >> >> [1] >> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-3-jason-jh.lin@xxxxxxxxxxxx/__;!!CTRNKA9wMg0ARbw!0rDdPxfBPcZC9icK37sCxT55RMqwRngO0BF4-uDwgYZP7UwQkx7iidkINqLBb7yi$ >> >> > OK, I will sync with Jason and modify it. > >>> +#define SOUT_TO_MIXER_IN1_SEL (1 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MERGE1_ASYNC_SOUT_SEL 0xf44 >>> -#define SOUT_TO_HDR_VDO_FE1 (0 >>> << 0) >>> +#define SOUT_TO_MIXER_IN2_SEL (1 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MERGE2_ASYNC_SOUT_SEL 0xf48 >>> -#define SOUT_TO_HDR_GFX_FE0 (0 >>> << 0) >>> +#define SOUT_TO_MIXER_IN3_SEL (1 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MERGE3_ASYNC_SOUT_SEL 0xf4c >>> -#define SOUT_TO_HDR_GFX_FE1 (0 >>> << 0) >>> +#define SOUT_TO_MIXER_IN4_SEL (1 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MIXER_IN1_SOUT_SEL 0xf58 >>> #define MIXER_IN1_SOUT_TO_DISP_MIXER (0 >>> << 0) >>> @@ -88,7 +88,7 @@ >>> #define MIXER_IN4_SOUT_TO_DISP_MIXER (0 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MIXER_OUT_SOUT_SEL 0xf34 >>> -#define MIXER_SOUT_TO_HDR_VDO_BE0 (0 >>> << 0) >>> +#define MIXER_SOUT_TO_MERGE4_ASYNC_SEL (1 >>> << 0) >>> >>> #define >>> MT8195_VDO1_MERGE4_SOUT_SEL 0xf18 >>> #define MERGE4_SOUT_TO_VDOSYS0 (0 >>> << 0) >>> @@ -185,6 +185,79 @@ static const struct mtk_mmsys_routes >>> mmsys_mt8195_routing_table[] = { >>> }, { >>> DDP_COMPONENT_DSC0, DDP_COMPONENT_MERGE0, >>> MT8195_VDO0_SEL_OUT, >>> SOUT_DSC_WRAP0_OUT_TO_VPP_MERGE >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_VPP_MERGE0_P0_SEL_IN, >>> VPP_MERGE0_P0_SEL_IN_FROM_MDP_RDMA0 >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_VPP_MERGE0_P1_SEL_IN, >>> VPP_MERGE0_P1_SEL_IN_FROM_MDP_RDMA1 >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_VPP_MERGE1_P0_SEL_IN, >>> VPP_MERGE1_P0_SEL_IN_FROM_MDP_RDMA2 >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MERGE0_ASYNC_SOUT_SEL, >>> SOUT_TO_MIXER_IN1_SEL >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MERGE1_ASYNC_SOUT_SEL, >>> SOUT_TO_MIXER_IN2_SEL >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MERGE2_ASYNC_SOUT_SEL, >>> SOUT_TO_MIXER_IN3_SEL >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MERGE3_ASYNC_SOUT_SEL, >>> SOUT_TO_MIXER_IN4_SEL >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MIXER_OUT_SOUT_SEL, >>> MIXER_SOUT_TO_MERGE4_ASYNC_SEL >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MIXER_IN1_SEL_IN, >>> MIXER_IN1_SEL_IN_FROM_MERGE0_ASYNC_SOUT >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MIXER_IN2_SEL_IN, >>> MIXER_IN2_SEL_IN_FROM_MERGE1_ASYNC_SOUT >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MIXER_IN3_SEL_IN, >>> MIXER_IN3_SEL_IN_FROM_MERGE2_ASYNC_SOUT >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MIXER_IN4_SEL_IN, >>> MIXER_IN4_SEL_IN_FROM_MERGE3_ASYNC_SOUT >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MIXER_SOUT_SEL_IN, >>> MIXER_SOUT_SEL_IN_FROM_DISP_MIXER >>> + }, >>> + { >>> + DDP_COMPONENT_PSEUDO_OVL, DDP_COMPONENT_MERGE5, >>> + MT8195_VDO1_MERGE4_ASYNC_SEL_IN, >>> MERGE4_ASYNC_SEL_IN_FROM_MIXER_OUT_SOUT >>> + }, >>> + { >>> + DDP_COMPONENT_MERGE5, DDP_COMPONENT_DPI1, >>> + MT8195_VDO1_DISP_DPI1_SEL_IN, >>> DISP_DPI1_SEL_IN_FROM_VPP_MERGE4_MOUT >>> + }, >>> + { >>> + DDP_COMPONENT_MERGE5, DDP_COMPONENT_DPI1, >>> + MT8195_VDO1_MERGE4_SOUT_SEL, >>> MERGE4_SOUT_TO_DPI1_SEL >>> + }, >>> + { >>> + DDP_COMPONENT_MERGE5, DDP_COMPONENT_DP_INTF1, >>> + MT8195_VDO1_DISP_DP_INTF0_SEL_IN, >>> + DISP_DP_INTF0_SEL_IN_FROM_VPP_MERGE4_MOUT >>> + }, >>> + { >>> + DDP_COMPONENT_MERGE5, DDP_COMPONENT_DP_INTF1, >>> + MT8195_VDO1_MERGE4_SOUT_SEL, >>> MERGE4_SOUT_TO_DP_INTF0_SEL >>> } >>> }; >>> >>> diff --git a/drivers/soc/mediatek/mtk-mmsys.c >>> b/drivers/soc/mediatek/mtk-mmsys.c >>> index 1fb241750897..9e31aad6c5c8 100644 >>> --- a/drivers/soc/mediatek/mtk-mmsys.c >>> +++ b/drivers/soc/mediatek/mtk-mmsys.c >>> @@ -59,6 +59,12 @@ static const struct mtk_mmsys_driver_data >>> mt8195_vdosys0_driver_data = { >>> .num_routes = ARRAY_SIZE(mmsys_mt8195_routing_table), >>> }; >>> >>> +static const struct mtk_mmsys_driver_data >>> mt8195_vdosys1_driver_data = { >>> + .clk_driver = "clk-mt8195-vdo1", >>> + .routes = mmsys_mt8195_routing_table, >>> + .num_routes = ARRAY_SIZE(mmsys_mt8195_routing_table), >>> +}; >>> + >>> struct mtk_mmsys { >>> void __iomem *regs; >>> const struct mtk_mmsys_driver_data *data; >>> @@ -168,6 +174,10 @@ static const struct of_device_id >>> of_match_mtk_mmsys[] = { >>> .compatible = "mediatek,mt8195-vdosys0", >>> .data = &mt8195_vdosys0_driver_data, >>> }, >>> + { >>> + .compatible = "mediatek,mt8195-vdosys1", >> >> Why do you need a second compatible, isn't this the same IP block? I >> mean, I understand that you have 2 mmsys blocks, but both are the >> same >> IP block, right? or are they different? >> >> Thanks, >> Enric >> > They(vdosys0 and vdosys1) are different IP block. > Please explain in what they are different. From what I see, you use the same routing table for both compatibles and only register another platform device for a second clock. Is that correct? Regards, Matthias >>> + .data = &mt8195_vdosys1_driver_data, >>> + }, >>> { } >>> }; >>> >>> diff --git a/include/linux/soc/mediatek/mtk-mmsys.h >>> b/include/linux/soc/mediatek/mtk-mmsys.h >>> index 34cb605e5df9..338c71570aeb 100644 >>> --- a/include/linux/soc/mediatek/mtk-mmsys.h >>> +++ b/include/linux/soc/mediatek/mtk-mmsys.h >>> @@ -49,6 +49,8 @@ enum mtk_ddp_comp_id { >>> DDP_COMPONENT_DSC1, >>> DDP_COMPONENT_DSC1_VIRTUAL0, >>> DDP_COMPONENT_DP_INTF0, >>> + DDP_COMPONENT_DP_INTF1, >>> + DDP_COMPONENT_PSEUDO_OVL, >>> DDP_COMPONENT_ID_MAX, >>> }; >>> >>> -- >>> 2.18.0 >>>