All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path
@ 2020-07-09  2:02 Liu Ying
  2020-07-09  2:02 ` [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list Liu Ying
  2020-07-10 17:31 ` [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path Sam Ravnborg
  0 siblings, 2 replies; 5+ messages in thread
From: Liu Ying @ 2020-07-09  2:02 UTC (permalink / raw)
  To: dri-devel
  Cc: Jose Abreu, Jernej Skrabec, Dariusz Marcinkiewicz, Jonas Karlman,
	David Airlie, Neil Armstrong, Archit Taneja, Andrzej Hajda,
	Boris Brezillon, Laurent Pinchart, NXP Linux Team,
	Cheng-Yi Chiang, Jerome Brunet

It's unnecessary to cleanup the i2c adapter and the ddc pointer in
the bailout path of  __dw_hdmi_probe(), since the adapter is not
added and the ddc pointer is not set.

Fixes: a23d6265f033 (drm: bridge: dw-hdmi: Extract PHY interrupt setup to a function)
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: dri-devel@lists.freedesktop.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Signed-off-by: Liu Ying <victor.liu@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v1->v2:
* Add Laurent's R-b tag.

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 6148a02..137b6eb 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3441,11 +3441,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	return hdmi;
 
 err_iahb:
-	if (hdmi->i2c) {
-		i2c_del_adapter(&hdmi->i2c->adap);
-		hdmi->ddc = NULL;
-	}
-
 	clk_disable_unprepare(hdmi->iahb_clk);
 	if (hdmi->cec_clk)
 		clk_disable_unprepare(hdmi->cec_clk);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list
  2020-07-09  2:02 [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path Liu Ying
@ 2020-07-09  2:02 ` Liu Ying
  2020-07-10 17:32   ` Sam Ravnborg
  2020-07-10 17:31 ` [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path Sam Ravnborg
  1 sibling, 1 reply; 5+ messages in thread
From: Liu Ying @ 2020-07-09  2:02 UTC (permalink / raw)
  To: dri-devel
  Cc: Jose Abreu, Jernej Skrabec, Dariusz Marcinkiewicz, Jonas Karlman,
	David Airlie, Neil Armstrong, Archit Taneja, Andrzej Hajda,
	Boris Brezillon, Laurent Pinchart, NXP Linux Team,
	Cheng-Yi Chiang, Sam Ravnborg, Jerome Brunet

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.

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path
  2020-07-09  2:02 [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path Liu Ying
  2020-07-09  2:02 ` [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list Liu Ying
@ 2020-07-10 17:31 ` Sam Ravnborg
  1 sibling, 0 replies; 5+ messages in thread
From: Sam Ravnborg @ 2020-07-10 17:31 UTC (permalink / raw)
  To: Liu Ying
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Neil Armstrong,
	Archit Taneja, dri-devel, Andrzej Hajda, Jose Abreu,
	Laurent Pinchart, Boris Brezillon, Cheng-Yi Chiang,
	Dariusz Marcinkiewicz, Jerome Brunet, NXP Linux Team

On Thu, Jul 09, 2020 at 10:02:35AM +0800, Liu Ying wrote:
> It's unnecessary to cleanup the i2c adapter and the ddc pointer in
> the bailout path of  __dw_hdmi_probe(), since the adapter is not
> added and the ddc pointer is not set.
> 
> Fixes: a23d6265f033 (drm: bridge: dw-hdmi: Extract PHY interrupt setup to a function)
> 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: dri-devel@lists.freedesktop.org
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, applied to drm-misc-next.

	Sam

> ---
> v1->v2:
> * Add Laurent's R-b tag.
> 
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 6148a02..137b6eb 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -3441,11 +3441,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  	return hdmi;
>  
>  err_iahb:
> -	if (hdmi->i2c) {
> -		i2c_del_adapter(&hdmi->i2c->adap);
> -		hdmi->ddc = NULL;
> -	}
> -
>  	clk_disable_unprepare(hdmi->iahb_clk);
>  	if (hdmi->cec_clk)
>  		clk_disable_unprepare(hdmi->cec_clk);
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list
  2020-07-09  2:02 ` [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list Liu Ying
@ 2020-07-10 17:32   ` Sam Ravnborg
  2020-07-11  0:59     ` Liu Ying
  0 siblings, 1 reply; 5+ messages in thread
From: Sam Ravnborg @ 2020-07-10 17:32 UTC (permalink / raw)
  To: Liu Ying
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Neil Armstrong,
	Archit Taneja, dri-devel, Andrzej Hajda, Jose Abreu,
	Laurent Pinchart, Boris Brezillon, Cheng-Yi Chiang,
	Dariusz Marcinkiewicz, Jerome Brunet, NXP Linux Team

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list
  2020-07-10 17:32   ` Sam Ravnborg
@ 2020-07-11  0:59     ` Liu Ying
  0 siblings, 0 replies; 5+ messages in thread
From: Liu Ying @ 2020-07-11  0:59 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Neil Armstrong,
	Archit Taneja, dri-devel, Andrzej Hajda, Jose Abreu,
	Laurent Pinchart, Boris Brezillon, Cheng-Yi Chiang,
	Dariusz Marcinkiewicz, Jerome Brunet, NXP Linux Team

On Fri, 2020-07-10 at 19:32 +0200, Sam Ravnborg wrote:
> 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.

Thank you, Sam.

Liu Ying

> 
> 	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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cvictor.liu%40nxp.com%7C0f923c54af014f54175608d824f72c5d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637299991302447959&amp;sdata=kgoboL8o%2Ft0qvvlQw4nyrHlcusib5Ynuc%2BY%2Fn70fu14%3D&amp;reserved=0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-14  7:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09  2:02 [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path Liu Ying
2020-07-09  2:02 ` [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list Liu Ying
2020-07-10 17:32   ` Sam Ravnborg
2020-07-11  0:59     ` Liu Ying
2020-07-10 17:31 ` [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path Sam Ravnborg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.