| Message ID | 20200310103427.26048-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2] drm/bridge: analogix_dp: Split bind() into probe() and real bind() | expand |
On 3/10/20 6:34 PM, Marek Szyprowski wrote: > Analogix_dp driver acquires all its resources in the ->bind() callback, > what is a bit against the component driver based approach, where the > driver initialization is split into a probe(), where all resources are > gathered, and a bind(), where all objects are created and a compound > driver is initialized. > > Extract all the resource related operations to analogix_dp_probe() and > analogix_dp_remove(), then call them before/after registration of the > device components from the main Exynos DP and Rockchip DP drivers. Also > move the plat_data initialization to the probe() to make it available for > the analogix_dp_probe() function. > > This fixes the multiple calls to the bind() of the DRM compound driver > when the DP PHY driver is not yet loaded/probed: > > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 14400000.fimd (ops fimd_component_ops [exynosdrm]) > exynos-drm exynos-drm: bound 14450000.mixer (ops mixer_component_ops [exynosdrm]) > exynos-dp 145b0000.dp-controller: no DP phy configured > exynos-drm exynos-drm: failed to bind 145b0000.dp-controller (ops exynos_dp_ops [exynosdrm]): -517 > exynos-drm exynos-drm: master bind failed: -517 > ... > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 14400000.fimd (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 14450000.mixer (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 145b0000.dp-controller (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 14530000.hdmi (ops hdmi_enable [exynosdrm]) > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > Console: switching to colour frame buffer device 170x48 > exynos-drm exynos-drm: fb0: exynosdrmfb frame buffer device > [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1 > ... > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Acked-by: Andy Yan <andy.yan@rock-chips.com> > --- > v2: > - moved plat_data initialization to exynos_dp_probe/rockchip_dp_probe as > pointed by Andy Yan > --- > .../drm/bridge/analogix/analogix_dp_core.c | 33 +++++++++++------ > drivers/gpu/drm/exynos/exynos_dp.c | 29 ++++++++------- > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 36 ++++++++++--------- > include/drm/bridge/analogix_dp.h | 5 +-- > 4 files changed, 61 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index 9ded2cef57dd..76736fb8ed94 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -1652,8 +1652,7 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux, > } > > struct analogix_dp_device * > -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > - struct analogix_dp_plat_data *plat_data) > +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data) > { > struct platform_device *pdev = to_platform_device(dev); > struct analogix_dp_device *dp; > @@ -1756,22 +1755,30 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > irq_flags, "analogix-dp", dp); > if (ret) { > dev_err(&pdev->dev, "failed to request irq\n"); > - goto err_disable_pm_runtime; > + return ERR_PTR(ret); > } > disable_irq(dp->irq); > > + return dp; > +} > +EXPORT_SYMBOL_GPL(analogix_dp_probe); > + > +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev) > +{ > + int ret; > + > dp->drm_dev = drm_dev; > dp->encoder = dp->plat_data->encoder; > > dp->aux.name = "DP-AUX"; > dp->aux.transfer = analogix_dpaux_transfer; > - dp->aux.dev = &pdev->dev; > + dp->aux.dev = dp->dev; > > ret = drm_dp_aux_register(&dp->aux); > if (ret) > - return ERR_PTR(ret); > + return ret; > > - pm_runtime_enable(dev); > + pm_runtime_enable(dp->dev); > > ret = analogix_dp_create_bridge(drm_dev, dp); > if (ret) { > @@ -1779,13 +1786,12 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > goto err_disable_pm_runtime; > } > > - return dp; > + return 0; > > err_disable_pm_runtime: > + pm_runtime_disable(dp->dev); > > - pm_runtime_disable(dev); > - > - return ERR_PTR(ret); > + return ret; > } > EXPORT_SYMBOL_GPL(analogix_dp_bind); > > @@ -1802,10 +1808,15 @@ void analogix_dp_unbind(struct analogix_dp_device *dp) > > drm_dp_aux_unregister(&dp->aux); > pm_runtime_disable(dp->dev); > - clk_disable_unprepare(dp->clock); > } > EXPORT_SYMBOL_GPL(analogix_dp_unbind); > > +void analogix_dp_remove(struct analogix_dp_device *dp) > +{ > + clk_disable_unprepare(dp->clock); > +} > +EXPORT_SYMBOL_GPL(analogix_dp_remove); > + > #ifdef CONFIG_PM > int analogix_dp_suspend(struct analogix_dp_device *dp) > { > diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c > index d23d3502ca91..5ee090691390 100644 > --- a/drivers/gpu/drm/exynos/exynos_dp.c > +++ b/drivers/gpu/drm/exynos/exynos_dp.c > @@ -159,15 +159,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) > struct drm_device *drm_dev = data; > int ret; > > - dp->dev = dev; > dp->drm_dev = drm_dev; > > - dp->plat_data.dev_type = EXYNOS_DP; > - dp->plat_data.power_on_start = exynos_dp_poweron; > - dp->plat_data.power_off = exynos_dp_poweroff; > - dp->plat_data.attach = exynos_dp_bridge_attach; > - dp->plat_data.get_modes = exynos_dp_get_modes; > - > if (!dp->plat_data.panel && !dp->ptn_bridge) { > ret = exynos_dp_dt_parse_panel(dp); > if (ret) > @@ -185,13 +178,11 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) > > dp->plat_data.encoder = encoder; > > - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); > - if (IS_ERR(dp->adp)) { > + ret = analogix_dp_bind(dp->adp, dp->drm_dev); > + if (ret) > dp->encoder.funcs->destroy(&dp->encoder); > - return PTR_ERR(dp->adp); > - } > > - return 0; > + return ret; > } > > static void exynos_dp_unbind(struct device *dev, struct device *master, > @@ -222,6 +213,7 @@ static int exynos_dp_probe(struct platform_device *pdev) > if (!dp) > return -ENOMEM; > > + dp->dev = dev; > /* > * We just use the drvdata until driver run into component > * add function, and then we would set drvdata to null, so > @@ -247,16 +239,29 @@ static int exynos_dp_probe(struct platform_device *pdev) > > /* The remote port can be either a panel or a bridge */ > dp->plat_data.panel = panel; > + dp->plat_data.dev_type = EXYNOS_DP; > + dp->plat_data.power_on_start = exynos_dp_poweron; > + dp->plat_data.power_off = exynos_dp_poweroff; > + dp->plat_data.attach = exynos_dp_bridge_attach; > + dp->plat_data.get_modes = exynos_dp_get_modes; > dp->plat_data.skip_connector = !!bridge; > + > dp->ptn_bridge = bridge; > > out: > + dp->adp = analogix_dp_probe(dev, &dp->plat_data); > + if (IS_ERR(dp->adp)) > + return PTR_ERR(dp->adp); > + > return component_add(&pdev->dev, &exynos_dp_ops); > } > > static int exynos_dp_remove(struct platform_device *pdev) > { > + struct exynos_dp_device *dp = platform_get_drvdata(pdev); > + > component_del(&pdev->dev, &exynos_dp_ops); > + analogix_dp_remove(dp->adp); > > return 0; > } > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > index f38f5e113c6b..ce98c08aa8b4 100644 > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > @@ -325,15 +325,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, > void *data) > { > struct rockchip_dp_device *dp = dev_get_drvdata(dev); > - const struct rockchip_dp_chip_data *dp_data; > struct drm_device *drm_dev = data; > int ret; > > - dp_data = of_device_get_match_data(dev); > - if (!dp_data) > - return -ENODEV; > - > - dp->data = dp_data; > dp->drm_dev = drm_dev; > > ret = rockchip_dp_drm_create_encoder(dp); > @@ -344,16 +338,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, > > dp->plat_data.encoder = &dp->encoder; > > - dp->plat_data.dev_type = dp->data->chip_type; > - dp->plat_data.power_on_start = rockchip_dp_poweron_start; > - dp->plat_data.power_off = rockchip_dp_powerdown; > - dp->plat_data.get_modes = rockchip_dp_get_modes; > - > - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); > - if (IS_ERR(dp->adp)) { > - ret = PTR_ERR(dp->adp); > + ret = analogix_dp_bind(dp->adp, drm_dev); > + if (ret) > goto err_cleanup_encoder; > - } > > return 0; > err_cleanup_encoder: > @@ -368,8 +355,6 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, > > analogix_dp_unbind(dp->adp); > dp->encoder.funcs->destroy(&dp->encoder); > - > - dp->adp = ERR_PTR(-ENODEV); > } > > static const struct component_ops rockchip_dp_component_ops = { > @@ -380,10 +365,15 @@ static const struct component_ops rockchip_dp_component_ops = { > static int rockchip_dp_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > + const struct rockchip_dp_chip_data *dp_data; > struct drm_panel *panel = NULL; > struct rockchip_dp_device *dp; > int ret; > > + dp_data = of_device_get_match_data(dev); > + if (!dp_data) > + return -ENODEV; > + > ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); > if (ret < 0) > return ret; > @@ -394,7 +384,12 @@ static int rockchip_dp_probe(struct platform_device *pdev) > > dp->dev = dev; > dp->adp = ERR_PTR(-ENODEV); > + dp->data = dp_data; > dp->plat_data.panel = panel; > + dp->plat_data.dev_type = dp->data->chip_type; > + dp->plat_data.power_on_start = rockchip_dp_poweron_start; > + dp->plat_data.power_off = rockchip_dp_powerdown; > + dp->plat_data.get_modes = rockchip_dp_get_modes; > > ret = rockchip_dp_of_probe(dp); > if (ret < 0) > @@ -402,12 +397,19 @@ static int rockchip_dp_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, dp); > > + dp->adp = analogix_dp_probe(dev, &dp->plat_data); > + if (IS_ERR(dp->adp)) > + return PTR_ERR(dp->adp); > + > return component_add(dev, &rockchip_dp_component_ops); > } > > static int rockchip_dp_remove(struct platform_device *pdev) > { > + struct rockchip_dp_device *dp = platform_get_drvdata(pdev); > + > component_del(&pdev->dev, &rockchip_dp_component_ops); > + analogix_dp_remove(dp->adp); > > return 0; > } > diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h > index 7aa2f93da49c..b0dcc07334a1 100644 > --- a/include/drm/bridge/analogix_dp.h > +++ b/include/drm/bridge/analogix_dp.h > @@ -42,9 +42,10 @@ int analogix_dp_resume(struct analogix_dp_device *dp); > int analogix_dp_suspend(struct analogix_dp_device *dp); > > struct analogix_dp_device * > -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > - struct analogix_dp_plat_data *plat_data); > +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data); > +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev); > void analogix_dp_unbind(struct analogix_dp_device *dp); > +void analogix_dp_remove(struct analogix_dp_device *dp); > > int analogix_dp_start_crc(struct drm_connector *connector); > int analogix_dp_stop_crc(struct drm_connector *connector);
On 10.03.2020 11:34, Marek Szyprowski wrote: > Analogix_dp driver acquires all its resources in the ->bind() callback, > what is a bit against the component driver based approach, where the > driver initialization is split into a probe(), where all resources are > gathered, and a bind(), where all objects are created and a compound > driver is initialized. > > Extract all the resource related operations to analogix_dp_probe() and > analogix_dp_remove(), then call them before/after registration of the > device components from the main Exynos DP and Rockchip DP drivers. Also > move the plat_data initialization to the probe() to make it available for > the analogix_dp_probe() function. > > This fixes the multiple calls to the bind() of the DRM compound driver > when the DP PHY driver is not yet loaded/probed: > > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 14400000.fimd (ops fimd_component_ops [exynosdrm]) > exynos-drm exynos-drm: bound 14450000.mixer (ops mixer_component_ops [exynosdrm]) > exynos-dp 145b0000.dp-controller: no DP phy configured > exynos-drm exynos-drm: failed to bind 145b0000.dp-controller (ops exynos_dp_ops [exynosdrm]): -517 > exynos-drm exynos-drm: master bind failed: -517 > ... > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 14400000.fimd (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 14450000.mixer (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 145b0000.dp-controller (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 14530000.hdmi (ops hdmi_enable [exynosdrm]) > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > Console: switching to colour frame buffer device 170x48 > exynos-drm exynos-drm: fb0: exynosdrmfb frame buffer device > [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1 > ... > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> -- Regards Andrzej
On 10.03.2020 11:34, Marek Szyprowski wrote: > Analogix_dp driver acquires all its resources in the ->bind() callback, > what is a bit against the component driver based approach, where the > driver initialization is split into a probe(), where all resources are > gathered, and a bind(), where all objects are created and a compound > driver is initialized. > > Extract all the resource related operations to analogix_dp_probe() and > analogix_dp_remove(), then call them before/after registration of the > device components from the main Exynos DP and Rockchip DP drivers. Also > move the plat_data initialization to the probe() to make it available for > the analogix_dp_probe() function. > > This fixes the multiple calls to the bind() of the DRM compound driver > when the DP PHY driver is not yet loaded/probed: > > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 14400000.fimd (ops fimd_component_ops [exynosdrm]) > exynos-drm exynos-drm: bound 14450000.mixer (ops mixer_component_ops [exynosdrm]) > exynos-dp 145b0000.dp-controller: no DP phy configured > exynos-drm exynos-drm: failed to bind 145b0000.dp-controller (ops exynos_dp_ops [exynosdrm]): -517 > exynos-drm exynos-drm: master bind failed: -517 > ... > [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 14400000.fimd (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 14450000.mixer (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 145b0000.dp-controller (ops hdmi_enable [exynosdrm]) > exynos-drm exynos-drm: bound 14530000.hdmi (ops hdmi_enable [exynosdrm]) > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > Console: switching to colour frame buffer device 170x48 > exynos-drm exynos-drm: fb0: exynosdrmfb frame buffer device > [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1 > ... > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Queued to fixes. Andrzej
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 9ded2cef57dd..76736fb8ed94 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1652,8 +1652,7 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux, } struct analogix_dp_device * -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, - struct analogix_dp_plat_data *plat_data) +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data) { struct platform_device *pdev = to_platform_device(dev); struct analogix_dp_device *dp; @@ -1756,22 +1755,30 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, irq_flags, "analogix-dp", dp); if (ret) { dev_err(&pdev->dev, "failed to request irq\n"); - goto err_disable_pm_runtime; + return ERR_PTR(ret); } disable_irq(dp->irq); + return dp; +} +EXPORT_SYMBOL_GPL(analogix_dp_probe); + +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev) +{ + int ret; + dp->drm_dev = drm_dev; dp->encoder = dp->plat_data->encoder; dp->aux.name = "DP-AUX"; dp->aux.transfer = analogix_dpaux_transfer; - dp->aux.dev = &pdev->dev; + dp->aux.dev = dp->dev; ret = drm_dp_aux_register(&dp->aux); if (ret) - return ERR_PTR(ret); + return ret; - pm_runtime_enable(dev); + pm_runtime_enable(dp->dev); ret = analogix_dp_create_bridge(drm_dev, dp); if (ret) { @@ -1779,13 +1786,12 @@ analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, goto err_disable_pm_runtime; } - return dp; + return 0; err_disable_pm_runtime: + pm_runtime_disable(dp->dev); - pm_runtime_disable(dev); - - return ERR_PTR(ret); + return ret; } EXPORT_SYMBOL_GPL(analogix_dp_bind); @@ -1802,10 +1808,15 @@ void analogix_dp_unbind(struct analogix_dp_device *dp) drm_dp_aux_unregister(&dp->aux); pm_runtime_disable(dp->dev); - clk_disable_unprepare(dp->clock); } EXPORT_SYMBOL_GPL(analogix_dp_unbind); +void analogix_dp_remove(struct analogix_dp_device *dp) +{ + clk_disable_unprepare(dp->clock); +} +EXPORT_SYMBOL_GPL(analogix_dp_remove); + #ifdef CONFIG_PM int analogix_dp_suspend(struct analogix_dp_device *dp) { diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c index d23d3502ca91..5ee090691390 100644 --- a/drivers/gpu/drm/exynos/exynos_dp.c +++ b/drivers/gpu/drm/exynos/exynos_dp.c @@ -159,15 +159,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) struct drm_device *drm_dev = data; int ret; - dp->dev = dev; dp->drm_dev = drm_dev; - dp->plat_data.dev_type = EXYNOS_DP; - dp->plat_data.power_on_start = exynos_dp_poweron; - dp->plat_data.power_off = exynos_dp_poweroff; - dp->plat_data.attach = exynos_dp_bridge_attach; - dp->plat_data.get_modes = exynos_dp_get_modes; - if (!dp->plat_data.panel && !dp->ptn_bridge) { ret = exynos_dp_dt_parse_panel(dp); if (ret) @@ -185,13 +178,11 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) dp->plat_data.encoder = encoder; - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); - if (IS_ERR(dp->adp)) { + ret = analogix_dp_bind(dp->adp, dp->drm_dev); + if (ret) dp->encoder.funcs->destroy(&dp->encoder); - return PTR_ERR(dp->adp); - } - return 0; + return ret; } static void exynos_dp_unbind(struct device *dev, struct device *master, @@ -222,6 +213,7 @@ static int exynos_dp_probe(struct platform_device *pdev) if (!dp) return -ENOMEM; + dp->dev = dev; /* * We just use the drvdata until driver run into component * add function, and then we would set drvdata to null, so @@ -247,16 +239,29 @@ static int exynos_dp_probe(struct platform_device *pdev) /* The remote port can be either a panel or a bridge */ dp->plat_data.panel = panel; + dp->plat_data.dev_type = EXYNOS_DP; + dp->plat_data.power_on_start = exynos_dp_poweron; + dp->plat_data.power_off = exynos_dp_poweroff; + dp->plat_data.attach = exynos_dp_bridge_attach; + dp->plat_data.get_modes = exynos_dp_get_modes; dp->plat_data.skip_connector = !!bridge; + dp->ptn_bridge = bridge; out: + dp->adp = analogix_dp_probe(dev, &dp->plat_data); + if (IS_ERR(dp->adp)) + return PTR_ERR(dp->adp); + return component_add(&pdev->dev, &exynos_dp_ops); } static int exynos_dp_remove(struct platform_device *pdev) { + struct exynos_dp_device *dp = platform_get_drvdata(pdev); + component_del(&pdev->dev, &exynos_dp_ops); + analogix_dp_remove(dp->adp); return 0; } diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index f38f5e113c6b..ce98c08aa8b4 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -325,15 +325,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, void *data) { struct rockchip_dp_device *dp = dev_get_drvdata(dev); - const struct rockchip_dp_chip_data *dp_data; struct drm_device *drm_dev = data; int ret; - dp_data = of_device_get_match_data(dev); - if (!dp_data) - return -ENODEV; - - dp->data = dp_data; dp->drm_dev = drm_dev; ret = rockchip_dp_drm_create_encoder(dp); @@ -344,16 +338,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, dp->plat_data.encoder = &dp->encoder; - dp->plat_data.dev_type = dp->data->chip_type; - dp->plat_data.power_on_start = rockchip_dp_poweron_start; - dp->plat_data.power_off = rockchip_dp_powerdown; - dp->plat_data.get_modes = rockchip_dp_get_modes; - - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); - if (IS_ERR(dp->adp)) { - ret = PTR_ERR(dp->adp); + ret = analogix_dp_bind(dp->adp, drm_dev); + if (ret) goto err_cleanup_encoder; - } return 0; err_cleanup_encoder: @@ -368,8 +355,6 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, analogix_dp_unbind(dp->adp); dp->encoder.funcs->destroy(&dp->encoder); - - dp->adp = ERR_PTR(-ENODEV); } static const struct component_ops rockchip_dp_component_ops = { @@ -380,10 +365,15 @@ static const struct component_ops rockchip_dp_component_ops = { static int rockchip_dp_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + const struct rockchip_dp_chip_data *dp_data; struct drm_panel *panel = NULL; struct rockchip_dp_device *dp; int ret; + dp_data = of_device_get_match_data(dev); + if (!dp_data) + return -ENODEV; + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); if (ret < 0) return ret; @@ -394,7 +384,12 @@ static int rockchip_dp_probe(struct platform_device *pdev) dp->dev = dev; dp->adp = ERR_PTR(-ENODEV); + dp->data = dp_data; dp->plat_data.panel = panel; + dp->plat_data.dev_type = dp->data->chip_type; + dp->plat_data.power_on_start = rockchip_dp_poweron_start; + dp->plat_data.power_off = rockchip_dp_powerdown; + dp->plat_data.get_modes = rockchip_dp_get_modes; ret = rockchip_dp_of_probe(dp); if (ret < 0) @@ -402,12 +397,19 @@ static int rockchip_dp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, dp); + dp->adp = analogix_dp_probe(dev, &dp->plat_data); + if (IS_ERR(dp->adp)) + return PTR_ERR(dp->adp); + return component_add(dev, &rockchip_dp_component_ops); } static int rockchip_dp_remove(struct platform_device *pdev) { + struct rockchip_dp_device *dp = platform_get_drvdata(pdev); + component_del(&pdev->dev, &rockchip_dp_component_ops); + analogix_dp_remove(dp->adp); return 0; } diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index 7aa2f93da49c..b0dcc07334a1 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -42,9 +42,10 @@ int analogix_dp_resume(struct analogix_dp_device *dp); int analogix_dp_suspend(struct analogix_dp_device *dp); struct analogix_dp_device * -analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, - struct analogix_dp_plat_data *plat_data); +analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data); +int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev); void analogix_dp_unbind(struct analogix_dp_device *dp); +void analogix_dp_remove(struct analogix_dp_device *dp); int analogix_dp_start_crc(struct drm_connector *connector); int analogix_dp_stop_crc(struct drm_connector *connector);
Analogix_dp driver acquires all its resources in the ->bind() callback, what is a bit against the component driver based approach, where the driver initialization is split into a probe(), where all resources are gathered, and a bind(), where all objects are created and a compound driver is initialized. Extract all the resource related operations to analogix_dp_probe() and analogix_dp_remove(), then call them before/after registration of the device components from the main Exynos DP and Rockchip DP drivers. Also move the plat_data initialization to the probe() to make it available for the analogix_dp_probe() function. This fixes the multiple calls to the bind() of the DRM compound driver when the DP PHY driver is not yet loaded/probed: [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 14400000.fimd (ops fimd_component_ops [exynosdrm]) exynos-drm exynos-drm: bound 14450000.mixer (ops mixer_component_ops [exynosdrm]) exynos-dp 145b0000.dp-controller: no DP phy configured exynos-drm exynos-drm: failed to bind 145b0000.dp-controller (ops exynos_dp_ops [exynosdrm]): -517 exynos-drm exynos-drm: master bind failed: -517 ... [drm] Exynos DRM: using 14400000.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 14400000.fimd (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 14450000.mixer (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 145b0000.dp-controller (ops hdmi_enable [exynosdrm]) exynos-drm exynos-drm: bound 14530000.hdmi (ops hdmi_enable [exynosdrm]) [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). Console: switching to colour frame buffer device 170x48 exynos-drm exynos-drm: fb0: exynosdrmfb frame buffer device [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 1 ... Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- v2: - moved plat_data initialization to exynos_dp_probe/rockchip_dp_probe as pointed by Andy Yan --- .../drm/bridge/analogix/analogix_dp_core.c | 33 +++++++++++------ drivers/gpu/drm/exynos/exynos_dp.c | 29 ++++++++------- .../gpu/drm/rockchip/analogix_dp-rockchip.c | 36 ++++++++++--------- include/drm/bridge/analogix_dp.h | 5 +-- 4 files changed, 61 insertions(+), 42 deletions(-)