[PATCH] drm/bridge: adv7511: Reset registers on hotplug

[Sean Paul]

On Tue, Jul 3, 2018 at 2:58 PM Rob Clark <robdclark at gmail.com> wrote:
>
> On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul <seanpaul at chromium.org> wrote:
> > The bridge loses its hw state when the cable is unplugged. If we detect
> > this case in the hpd handler, reset its state.
> >
> > Reported-by: Rob Clark <robdclark at gmail.com>
> > Signed-off-by: Sean Paul <seanpaul at chromium.org>
>
> Tested-by: Rob Clark <robdclark at gmail.com>
>
> > ---
> > This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no
> > active planes". I incorrectly assumed the modeset was required from the
> > msm side, but Rob pointed out that he thought it might be a bridge
> > issue.
>
> To elaborate, this is an atomic userspace (weston), when it sees the
> display disconnected, it removes the planes from the CRTC but does not
> disable the CRTC.  So drm/msm sets up the LM to scanout a solid color,
> and leaves the encoder and dsi cranking along happily.  When
> replugging the display, the atomic helpers see the CRTC is still
> active and (correctly) doesn't do a full modeset.  So the bridge is
> not re-configured/re-enabled.
>
> Arguably this perhaps isn't what weston *wanted* to do, but in the end
> the bug is in the bridge.
>

Fwiw, I've also filed
https://gitlab.freedesktop.org/wayland/weston/issues/123 against
weston so they can save some power on disconnected displays. But yeah,
multiple pieces all working against each other here.

Sean

> We could possibly set the connector link_status to BAD as an
> alternative.  But at least the build of weston I'm using atm doesn't
> handle the link_status propery, this approach requires each atomic
> userspace to handle this case.  Compared to Sean's approach which is
> transparent to userspace, which seems attractive.
>
> BR,
> -R
>
> >
> >  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > index 73021b388e12..dd3ff2f2cdce 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > @@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct *work)
> >         else
> >                 status = connector_status_disconnected;
> >
> > +       /*
> > +        * The bridge resets its registers on unplug. So when we get a plug
> > +        * event and we're already supposed to be powered, cycle the bridge to
> > +        * restore its state.
> > +        */
> > +       if (status == connector_status_connected &&
> > +           adv7511->connector.status == connector_status_disconnected &&
> > +           adv7511->powered) {
> > +               regcache_mark_dirty(adv7511->regmap);
> > +               adv7511_power_on(adv7511);
> > +       }
> > +
> >         if (adv7511->connector.status != status) {
> >                 adv7511->connector.status = status;
> >                 if (status == connector_status_disconnected)
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> >