Hi CK, On Wed, 2016-07-20 at 14:53 +0800, CK Hu wrote: > Hi, YT: > > Some comments inline. > > On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote: > > This patch add support for the Mediatek MT2701 DISP subsystem. > > There is only one OVL engine in MT2701. > > > > Signed-off-by: YT Shen <yt.shen@xxxxxxxxxxxx> > > --- > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++ > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++ > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 41 +++++++++++++++++++++++++++ > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 7 +++++ > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 31 ++++++++++++++++++++ > > 6 files changed, 92 insertions(+) > > > > [snip...] > > > > > static const struct of_device_id mtk_ddp_comp_dt_ids[] = { > > + { .compatible = "mediatek,mt2701-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > { .compatible = "mediatek,mt8173-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > + { .compatible = "mediatek,mt2701-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > { .compatible = "mediatek,mt8173-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > + { .compatible = "mediatek,mt2701-disp-wdma", .data = (void *)MTK_DISP_WDMA }, > > Is WDMA different from MT8173 to MT2701. If they are the same, you need > not to add compatible of 'mediatek,mt2701-disp-wdma' because use > 'mediatek,mt8173-disp-wdma' is enough. Yes, the hardware has differences, so we need add new compatible 'mt2701-disp-wdma' > > > { .compatible = "mediatek,mt8173-disp-wdma", .data = (void *)MTK_DISP_WDMA }, > > + { .compatible = "mediatek,mt2701-disp-color", .data = (void *)MTK_DISP_COLOR }, > > { .compatible = "mediatek,mt8173-disp-color", .data = (void *)MTK_DISP_COLOR }, > > { .compatible = "mediatek,mt8173-disp-aal", .data = (void *)MTK_DISP_AAL}, > > { .compatible = "mediatek,mt8173-disp-gamma", .data = (void *)MTK_DISP_GAMMA, }, > > { .compatible = "mediatek,mt8173-disp-ufoe", .data = (void *)MTK_DISP_UFOE }, > > + { .compatible = "mediatek,mt2701-dsi", .data = (void *)MTK_DSI }, > > { .compatible = "mediatek,mt8173-dsi", .data = (void *)MTK_DSI }, > > + { .compatible = "mediatek,mt2701-dpi", .data = (void *)MTK_DPI }, > > Is DPI different from MT8173 to MT2701. If they are the same, you need > not to add compatible of 'mediatek,mt2701-dpi' because use > 'mediatek,mt8173-dpi' is enough. Yes, the hardware has differences, so we need add new compatible 'mt2701-dpi' > > > { .compatible = "mediatek,mt8173-dpi", .data = (void *)MTK_DPI }, > > + { .compatible = "mediatek,mt2701-disp-mutex", .data = (void *)MTK_DISP_MUTEX }, > > { .compatible = "mediatek,mt8173-disp-mutex", .data = (void *)MTK_DISP_MUTEX }, > > + { .compatible = "mediatek,mt2701-disp-pwm", .data = (void *)MTK_DISP_PWM }, > > Is pwm different from MT8173 to MT2701. If they are the same, you need > not to add compatible of 'mediatek,mt2701-disp-pwm' because use > 'mediatek,mt8173-disp-pwm' is enough. Yes, the hardware has differences, so we need add new compatible 'mt2701-disp-pwm' Regards, yt.shen > > > { .compatible = "mediatek,mt8173-disp-pwm", .data = (void *)MTK_DISP_PWM }, > > { .compatible = "mediatek,mt8173-disp-od", .data = (void *)MTK_DISP_OD }, > > { } > > @@ -514,6 +543,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend, > > mtk_drm_sys_resume); > > > > static const struct of_device_id mtk_drm_of_ids[] = { > > + { .compatible = "mediatek,mt2701-mmsys", > > + .data = &mt2701_mmsys_driver_data}, > > { .compatible = "mediatek,mt8173-mmsys", > > .data = &mt8173_mmsys_driver_data}, > > { } > > Regards, > CK > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html