On Thu, Jul 09, 2020 at 10:02:36AM +0800, Liu Ying wrote:
It doesn't hurt to add the bridge in the global bridge list also for platform specific dw-hdmi drivers which are based on the component framework. This can be achieved by moving the drm_bridge_add() function call from dw_hdmi_probe() to __dw_hdmi_probe(). A counterpart movement for drm_bridge_remove() is also needed then. Moreover, since drm_bridge_add() initializes &bridge->hpd_mutex, this may help those platform specific dw-hdmi drivers(based on the component framework) avoid accessing the uninitialized mutex in drm_bridge_hpd_notify() which is called in dw_hdmi_irq(). Putting drm_bridge_add() in __dw_hdmi_probe() just before it returns successfully should bring no logic change for platforms based on the DRM bridge API, which is a good choice from safety point of view. Also, __dw_hdmi_probe() is renamed to dw_hdmi_probe() since dw_hdmi_probe() does nothing else but calling __dw_hdmi_probe(). Similar renaming applies to the __dw_hdmi_remove()/dw_hdmi_remove() pair.
Hmm, it took me some attempts to read this. Anyway, applied as-is to drm-misc-next.
Sam
Fixes: ec971aaa6775 ("drm: bridge: dw-hdmi: Make connector creation optional") Cc: Andrzej Hajda a.hajda@samsung.com Cc: Neil Armstrong narmstrong@baylibre.com Cc: Laurent Pinchart Laurent.pinchart@ideasonboard.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Jernej Skrabec jernej.skrabec@siol.net Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Jerome Brunet jbrunet@baylibre.com Cc: Cheng-Yi Chiang cychiang@chromium.org Cc: Dariusz Marcinkiewicz darekm@google.com Cc: Archit Taneja architt@codeaurora.org Cc: Jose Abreu joabreu@synopsys.com Cc: Sam Ravnborg sam@ravnborg.org Cc: dri-devel@lists.freedesktop.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com
v1->v2:
- Put drm_bridge_add() in __dw_hdmi_probe() just before it returns successfully so that the bridge shows up in the global bridge list late enough to avoid potential race condition with other display drivers. (Laurent)
- Rename __dw_hdmi_probe/remove() to dw_hdmi_probe/remove() respectively. (Laurent)
- Cc'ed Sam.
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 44 +++++++++---------------------- 1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 137b6eb..748df1c 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3179,9 +3179,11 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi) hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data); }
-static struct dw_hdmi * -__dw_hdmi_probe(struct platform_device *pdev,
const struct dw_hdmi_plat_data *plat_data)+/* -----------------------------------------------------------------------------
- Probe/remove API, used from platforms based on the DRM bridge API.
- */
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
const struct dw_hdmi_plat_data *plat_data){ struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; @@ -3438,6 +3440,8 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->cec = platform_device_register_full(&pdevinfo); }
- drm_bridge_add(&hdmi->bridge);
- return hdmi;
err_iahb: @@ -3451,9 +3455,12 @@ __dw_hdmi_probe(struct platform_device *pdev,
return ERR_PTR(ret); } +EXPORT_SYMBOL_GPL(dw_hdmi_probe);
-static void __dw_hdmi_remove(struct dw_hdmi *hdmi) +void dw_hdmi_remove(struct dw_hdmi *hdmi) {
- drm_bridge_remove(&hdmi->bridge);
- if (hdmi->audio && !IS_ERR(hdmi->audio)) platform_device_unregister(hdmi->audio); if (!IS_ERR(hdmi->cec))
@@ -3472,31 +3479,6 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) else i2c_put_adapter(hdmi->ddc); }
-/* -----------------------------------------------------------------------------
- Probe/remove API, used from platforms based on the DRM bridge API.
- */
-struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
const struct dw_hdmi_plat_data *plat_data)-{
- struct dw_hdmi *hdmi;
- hdmi = __dw_hdmi_probe(pdev, plat_data);
- if (IS_ERR(hdmi))
return hdmi;- drm_bridge_add(&hdmi->bridge);
- return hdmi;
-} -EXPORT_SYMBOL_GPL(dw_hdmi_probe);
-void dw_hdmi_remove(struct dw_hdmi *hdmi) -{
- drm_bridge_remove(&hdmi->bridge);
- __dw_hdmi_remove(hdmi);
-} EXPORT_SYMBOL_GPL(dw_hdmi_remove);
/* ----------------------------------------------------------------------------- @@ -3509,7 +3491,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev, struct dw_hdmi *hdmi; int ret;
- hdmi = __dw_hdmi_probe(pdev, plat_data);
- hdmi = dw_hdmi_probe(pdev, plat_data); if (IS_ERR(hdmi)) return hdmi;
@@ -3526,7 +3508,7 @@ EXPORT_SYMBOL_GPL(dw_hdmi_bind);
void dw_hdmi_unbind(struct dw_hdmi *hdmi) {
- __dw_hdmi_remove(hdmi);
- dw_hdmi_remove(hdmi);
} EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel