| Message ID | 20200226105419.632771-5-enric.balletbo@collabora.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | arm64: mediatek: Fix mt8173 mmsys device probing | expand |
Hi, Enric: On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote: > In the actual implementation the same compatible string > "mediatek,<chip>-mmsys" is used to bind the clock drivers > (drivers/soc/mediatek) as well as to the gpu driver > (drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem > that the only probed driver is the clock driver and there is no display > at all. > > In any case having the same compatible string for two drivers is not > correct and should be fixed. To fix this, and maintain backward > compatibility, we can consider that the mmsys driver is the top-level > entry point for the multimedia subsystem, so is not a pure clock > controller but a system controller, and the drm driver is instantiated > by that MMSYS driver. > Reviewed-by: CK Hu <ck.hu@mediatek.com> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > --- > > Changes in v9: > - Do not move the display routing from the drm driver (CK) > > Changes in v8: > - New patch introduced in this series. > > Changes in v7: None > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++++++++++++++------------ > drivers/soc/mediatek/mt8173-mmsys.c | 6 +++++ > 2 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index b68837ea02b3..17f118ee0e57 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -422,9 +422,21 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = { > { } > }; > > +static const struct of_device_id mtk_drm_of_ids[] = { > + { .compatible = "mediatek,mt2701-mmsys", > + .data = &mt2701_mmsys_driver_data}, > + { .compatible = "mediatek,mt2712-mmsys", > + .data = &mt2712_mmsys_driver_data}, > + { .compatible = "mediatek,mt8173-mmsys", > + .data = &mt8173_mmsys_driver_data}, > + { } > +}; > + > static int mtk_drm_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > + struct device_node *phandle = dev->parent->of_node; > + const struct of_device_id *of_id; > struct mtk_drm_private *private; > struct device_node *node; > struct component_match *match = NULL; > @@ -435,15 +447,18 @@ static int mtk_drm_probe(struct platform_device *pdev) > if (!private) > return -ENOMEM; > > - private->data = of_device_get_match_data(dev); > + of_id = of_match_node(mtk_drm_of_ids, phandle); > + if (!of_id) > + return -ENODEV; > + > + private->data = of_id->data; > > - private->config_regs = syscon_node_to_regmap(dev->of_node); > + private->config_regs = syscon_node_to_regmap(phandle); > if (IS_ERR(private->config_regs)) > return PTR_ERR(private->config_regs); > > /* Iterate over sibling DISP function blocks */ > - for_each_child_of_node(dev->of_node->parent, node) { > - const struct of_device_id *of_id; > + for_each_child_of_node(phandle->parent, node) { > enum mtk_ddp_comp_type comp_type; > int comp_id; > > @@ -576,22 +591,11 @@ static int mtk_drm_sys_resume(struct device *dev) > 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,mt2712-mmsys", > - .data = &mt2712_mmsys_driver_data}, > - { .compatible = "mediatek,mt8173-mmsys", > - .data = &mt8173_mmsys_driver_data}, > - { } > -}; > - > static struct platform_driver mtk_drm_platform_driver = { > .probe = mtk_drm_probe, > .remove = mtk_drm_remove, > .driver = { > .name = "mediatek-drm", > - .of_match_table = mtk_drm_of_ids, > .pm = &mtk_drm_pm_ops, > }, > }; > diff --git a/drivers/soc/mediatek/mt8173-mmsys.c b/drivers/soc/mediatek/mt8173-mmsys.c > index 48e6c157d28e..c894db5b6ca9 100644 > --- a/drivers/soc/mediatek/mt8173-mmsys.c > +++ b/drivers/soc/mediatek/mt8173-mmsys.c > @@ -103,6 +103,7 @@ static int mt8173_mmsys_probe(struct platform_device *pdev) > { > struct device_node *node = pdev->dev.of_node; > struct clk_onecell_data *clk_data; > + struct platform_device *drm; > int ret; > > clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); > @@ -118,6 +119,11 @@ static int mt8173_mmsys_probe(struct platform_device *pdev) > if (ret) > return ret; > > + drm = platform_device_register_data(&pdev->dev, "mediatek-drm", > + PLATFORM_DEVID_NONE, NULL, 0); > + if (IS_ERR(drm)) > + return PTR_ERR(drm); > + > return 0; > } >
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b68837ea02b3..17f118ee0e57 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -422,9 +422,21 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = { { } }; +static const struct of_device_id mtk_drm_of_ids[] = { + { .compatible = "mediatek,mt2701-mmsys", + .data = &mt2701_mmsys_driver_data}, + { .compatible = "mediatek,mt2712-mmsys", + .data = &mt2712_mmsys_driver_data}, + { .compatible = "mediatek,mt8173-mmsys", + .data = &mt8173_mmsys_driver_data}, + { } +}; + static int mtk_drm_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + struct device_node *phandle = dev->parent->of_node; + const struct of_device_id *of_id; struct mtk_drm_private *private; struct device_node *node; struct component_match *match = NULL; @@ -435,15 +447,18 @@ static int mtk_drm_probe(struct platform_device *pdev) if (!private) return -ENOMEM; - private->data = of_device_get_match_data(dev); + of_id = of_match_node(mtk_drm_of_ids, phandle); + if (!of_id) + return -ENODEV; + + private->data = of_id->data; - private->config_regs = syscon_node_to_regmap(dev->of_node); + private->config_regs = syscon_node_to_regmap(phandle); if (IS_ERR(private->config_regs)) return PTR_ERR(private->config_regs); /* Iterate over sibling DISP function blocks */ - for_each_child_of_node(dev->of_node->parent, node) { - const struct of_device_id *of_id; + for_each_child_of_node(phandle->parent, node) { enum mtk_ddp_comp_type comp_type; int comp_id; @@ -576,22 +591,11 @@ static int mtk_drm_sys_resume(struct device *dev) 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,mt2712-mmsys", - .data = &mt2712_mmsys_driver_data}, - { .compatible = "mediatek,mt8173-mmsys", - .data = &mt8173_mmsys_driver_data}, - { } -}; - static struct platform_driver mtk_drm_platform_driver = { .probe = mtk_drm_probe, .remove = mtk_drm_remove, .driver = { .name = "mediatek-drm", - .of_match_table = mtk_drm_of_ids, .pm = &mtk_drm_pm_ops, }, }; diff --git a/drivers/soc/mediatek/mt8173-mmsys.c b/drivers/soc/mediatek/mt8173-mmsys.c index 48e6c157d28e..c894db5b6ca9 100644 --- a/drivers/soc/mediatek/mt8173-mmsys.c +++ b/drivers/soc/mediatek/mt8173-mmsys.c @@ -103,6 +103,7 @@ static int mt8173_mmsys_probe(struct platform_device *pdev) { struct device_node *node = pdev->dev.of_node; struct clk_onecell_data *clk_data; + struct platform_device *drm; int ret; clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); @@ -118,6 +119,11 @@ static int mt8173_mmsys_probe(struct platform_device *pdev) if (ret) return ret; + drm = platform_device_register_data(&pdev->dev, "mediatek-drm", + PLATFORM_DEVID_NONE, NULL, 0); + if (IS_ERR(drm)) + return PTR_ERR(drm); + return 0; }
In the actual implementation the same compatible string "mediatek,<chip>-mmsys" is used to bind the clock drivers (drivers/soc/mediatek) as well as to the gpu driver (drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem that the only probed driver is the clock driver and there is no display at all. In any case having the same compatible string for two drivers is not correct and should be fixed. To fix this, and maintain backward compatibility, we can consider that the mmsys driver is the top-level entry point for the multimedia subsystem, so is not a pure clock controller but a system controller, and the drm driver is instantiated by that MMSYS driver. Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> --- Changes in v9: - Do not move the display routing from the drm driver (CK) Changes in v8: - New patch introduced in this series. Changes in v7: None drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++++++++++++++------------ drivers/soc/mediatek/mt8173-mmsys.c | 6 +++++ 2 files changed, 25 insertions(+), 15 deletions(-)