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

[John Stultz]

On Thu, Jun 27, 2019 at 8:18 AM Matt Redfearn <matt.redfearn@xxxxxxxxxx> wrote:
>
> In contrast to all of the DSI panel drivers in drivers/gpu/drm/panel
> which attach to the DSI host via mipi_dsi_attach() at probe time, the
> ADV7533 bridge device does not. Instead it defers this to the point that
> the upstream device connects to its bridge via drm_bridge_attach().
> The generic Synopsys MIPI DSI host driver does not register it's own
> drm_bridge until the MIPI DSI has attached. But it does not call
> drm_bridge_attach() on the downstream device until the upstream device
> has attached. This leads to a chicken and the egg failure and the DRM
> pipeline does not complete.
> Since all other mipi_dsi_device drivers call mipi_dsi_attach() in
> probe(), make the adv7533 mipi_dsi_device do the same. This ensures that
> the Synopsys MIPI DSI host registers it's bridge such that it is
> available for the upstream device to connect to.
>
> Signed-off-by: Matt Redfearn <matt.redfearn@xxxxxxxxxx>
>
> ---
>
> Changes in v2:
> Cleanup if adv7533_attach_dsi fails.
>
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index e7ddd3e3db9..807827bd910 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -874,9 +874,6 @@ 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);
> @@ -1222,8 +1219,17 @@ 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)
> --

As a heads up, I just did some testing on drm-misc-next and this patch
seems to be breaking the HiKey board.  On bootup, I'm seeing:
[    4.209615] adv7511 2-0039: 2-0039 supply avdd not found, using
dummy regulator
[    4.217075] adv7511 2-0039: 2-0039 supply dvdd not found, using
dummy regulator
[    4.224453] adv7511 2-0039: 2-0039 supply pvdd not found, using
dummy regulator
[    4.231804] adv7511 2-0039: 2-0039 supply a2vdd not found, using
dummy regulator
[    4.239242] adv7511 2-0039: 2-0039 supply v3p3 not found, using
dummy regulator
[    4.246615] adv7511 2-0039: 2-0039 supply v1p2 not found, using
dummy regulator
[    4.272970] adv7511 2-0039: failed to find dsi host

over and over.  The dummy regulator messages are normal, but usually
[    4.444315] kirin-drm f4100000.ade: bound f4107800.dsi (ops dsi_ops)

Starts up right afterward.

Reverting the change above seems to get things working again.  I've
not had much time to dig as to whats going wrong, but will keep
looking and wanted to raise the issue in the meantime.

thanks
-john