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

[John Stultz]

On Mon, Aug 19, 2019 at 3:27 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> 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.

I also just got a report that our testing is seeing this on the db410c
board as well:
  https://bugs.linaro.org/show_bug.cgi?id=5345

It seems like the change is making it try to attach to the dsi too
early before the dsi has registered?

thanks
-john