Hi Matthias,
Thanks for the review.
On Fri, 2021-08-06 at 14:20 +0200, Matthias Brugger wrote:
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@mediatek.com 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@mediatek.com
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-media...
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
From a hardware point of view, the HW engines of vdosys0 and vdosys1 are different. 1. The components on meiatek-drm of vdosys0 are OVL0, RDMA0, COLOR0, CCORR, AAL0, GAMMA, DITHER, DSC0, MERGE0, DP_INTF0. Its output panel is eDP. 2. The components on meiatek-drm of vdosys1 are OVL_ADAPTOR, MERGE5, DP_INTF1. Its output panel is DP.
In the SoC before, such as mt8173, it has 2 pipelines binding to one mmsys with the same clock driver and the same power domain.
In mt8195, 2 pipelines are binding to different mmsys, such as vdosys0 and vdosys1. Each mmsys uses different clock drivers and different power domains.
So I think it is more appropriate to use 2 compatibles to identify which mmsys represents the pipeline. Another reason for using two compatibles is that we use different driver data in mtk-mmsys.c and mtk_drm_drv.c to identify the corresponding mmsys is vdosys0 or vdosys1.
Their driver data in mtk_drm_drv.c is defined here: [v7,13/13] drm/mediatek: add mediatek-drm of vdosys0 support for mt8195
https://patchwork.kernel.org/project/linux-mediatek/patch/20210815145610.205...
[v2,14/14] drm/mediatek: add mediatek-drm of vdosys1 support for MT8195
https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.152...
There is another series talking about the two mmsys compatible.
https://patchwork.kernel.org/project/linux-mediatek/patch/20210722092624.144...
Regards, Nancy
.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