On Wed, Jul 04, 2018 at 06:53:02PM +0530, Archit Taneja wrote:
>
>
> On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
> > On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> 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@xxxxxxxxx>
> > > Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> >
> > Tested-by: Rob Clark <robdclark@xxxxxxxxx>
> >
> > > ---
> > > 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.
>
> The adv7511 connector's detect() op should have re-configured the
> registers. I'm assuming it was never called after the cable is
> plugged in again in the wetson usecase?
Right. detect() isn't guaranteed to be called on connection (it just usually
is).
>
> With this patch, in the case of fb emulation/X, I'm wondering if
> we will end up trying to re-enable ADV7511 twice. First, in the new code
> in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the
> connector's detect op).
>
> I'm guessing the 'hpd' variable in the check in adv7511_detect() below
> should ideally be false, and avoid us trying to configure the registers
> again, but I'm not entirely sure.
It's entirely possible to call detect() multiple times per connection
(ie, run modetest a bunch), so that hpd check is pretty critical for even
non-hotplug cases.
>
> if (status == connector_status_connected && hpd && adv7511->powered) {
> regcache_mark_dirty(adv7511->regmap);
> ...
>
> In any case:
>
> Reviewed-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
Thanks for your review, I'll apply to -misc-fixes.
Sean
>
> >
> > Arguably this perhaps isn't what weston *wanted* to do, but in the end
> > the bug is in the bridge.
> >
> > 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
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
