[PATCH] Revert "drm/bridge: adv7511: Attach to DSI host at probe time"

[PATCH] Revert "drm/bridge: adv7511: Attach to DSI host at probe time"

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

This reverts commit 83f35bc3a852f1c3892c7474998c5cec707c7ba3.

This commit the wrong direction, we should really be changing panel
framework to attach dsi host after probe, rather than introducing
the same probe-order problem that panels already have to bridges.

The reason is, that in order to deal with devices where display is
enabled by bootloader and efifb/simplefb is used until the real
driver probes, we need to be careful to not touch the hardware
until we have all the pieces probed and ready to go, otherwise you
will kill the working display, leaving yourself (at least, in the
case of real consumer devices that do not have a debug UART) with
no good way to debug what went wrong.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 98bccace8c1c..f6d2681f6927 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -874,6 +874,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
 				 &adv7511_connector_helper_funcs);
 	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
 
+	if (adv->type == ADV7533)
+		ret = adv7533_attach_dsi(adv);
+
 	if (adv->i2c_main->irq)
 		regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0),
 			     ADV7511_INT0_HPD);
@@ -1219,17 +1222,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 	drm_bridge_add(&adv7511->bridge);
 
 	adv7511_audio_init(dev, adv7511);
-
-	if (adv7511->type == ADV7533) {
-		ret = adv7533_attach_dsi(adv7511);
-		if (ret)
-			goto err_remove_bridge;
-	}
-
 	return 0;
 
-err_remove_bridge:
-	drm_bridge_remove(&adv7511->bridge);
 err_unregister_cec:
 	i2c_unregister_device(adv7511->i2c_cec);
 	if (adv7511->cec_clk)
-- 
2.21.0

Re: [PATCH] Revert "drm/bridge: adv7511: Attach to DSI host at probe time"

[Andrzej Hajda]

On 29.08.2019 20:08, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> This reverts commit 83f35bc3a852f1c3892c7474998c5cec707c7ba3.
>
> This commit the wrong direction, we should really be changing panel
> framework to attach dsi host after probe, rather than introducing
> the same probe-order problem that panels already have to bridges.
>
> The reason is, that in order to deal with devices where display is
> enabled by bootloader and efifb/simplefb is used until the real
> driver probes, we need to be careful to not touch the hardware
> until we have all the pieces probed and ready to go, otherwise you
> will kill the working display, leaving yourself (at least, in the
> case of real consumer devices that do not have a debug UART) with
> no good way to debug what went wrong.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>


Applied to drm-misc-next-fixes, with changed commit message - I hope it
is OK for you.


Regards

Andrzej


> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 98bccace8c1c..f6d2681f6927 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -874,6 +874,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
>  				 &adv7511_connector_helper_funcs);
>  	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
>  
> +	if (adv->type == ADV7533)
> +		ret = adv7533_attach_dsi(adv);
> +
>  	if (adv->i2c_main->irq)
>  		regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0),
>  			     ADV7511_INT0_HPD);
> @@ -1219,17 +1222,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
>  	drm_bridge_add(&adv7511->bridge);
>  
>  	adv7511_audio_init(dev, adv7511);
> -
> -	if (adv7511->type == ADV7533) {
> -		ret = adv7533_attach_dsi(adv7511);
> -		if (ret)
> -			goto err_remove_bridge;
> -	}
> -
>  	return 0;
>  
> -err_remove_bridge:
> -	drm_bridge_remove(&adv7511->bridge);
>  err_unregister_cec:
>  	i2c_unregister_device(adv7511->i2c_cec);
>  	if (adv7511->cec_clk)