Re: [PATCH v2 1/2] drm/i915/ilk: Don't attempt to register eDP if LVDS was detected

Chris Wilson
2016-06-20T07:55:08Z

On Sun, Jun 19, 2016 at 01:18:32PM +0300, Imre Deak wrote:
> On Fri, 2016-06-17 at 23:35 +0100, Chris Wilson wrote:
> > On Sat, Jun 18, 2016 at 12:29:24AM +0300, Imre Deak wrote:
> > > Atm on ILK we attempt to detect if eDP is present even if LVDS was
> > > already detected and an encoder for it was registered. This involves
> > > trying to read out the eDP EDID, which in turn needs the same power
> > > sequencer that LVDS uses. Poking at the VDD line at an unexpected time
> > > may or may not interfere with the LVDS panel, but it's probably safer to
> > > prevent this. Registering both an LVDS and an eDP connector would also
> > > present a similar problem accessing the shared PPS at any point later in
> > > an unexpected way.
> > >
> > > We also need this to be able fix PPS initialization before its first use
> > > in the next patch. For that we want to be sure that PPS is not in use
> > > by LVDS.
> > >
> > > v2:
> > > - Split out the PPS init fix to a separate patch. (Chris)
> > > - Add comment about eDP init depending on LVDS init. (Chris)
> > > - Make the use of the intel_encoder ptr less error prone.
> > >
> > > CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > Main worry here is what if the LVDS detection was false?
>
> That wouldn't really work anyway atm. We'd end up with both LVDS and
> eDP registered and the subsequent LVDS modeset toggling the eDP panel
> power out of order with respect to eDP's own pipe, PLL, port enabling
> sequence. In the worst case we'd violate panel specs, for instance with
> an LVDS panel off->eDP forced VDD sequence.
>
> > (I believe that LVDS/eDP doesn't coexist...)
>
> Right, they both use the single PPS we have which can't be shared.
>
> > I'm just wondering if calling lvds_encoder->post_disable() to force the
> > LVDS off in this circumstance is viable. Worst case (false eDP, real
> > LVDS), we lose the output on the console until a mode is restored by fbdev.
> > Best case (false LVDS, real eDP) we don't regress detection of eDP.
> >
> > (Or knowing the internals, we could just do a save restore of LVDS
> > PP_CONTROL around the eDP detection.)
>
> The proper way to implement that kind of workaround would be to
> unregister (or permanently disable) the LVDS encoder/connector if eDP
> detection succeeds. We would also have to disable LVDS unconditionally
> on ILK before eDP detect, even on a correctly detected LVDS output,
> since we run the eDP detection also unconditionally. I think we should
> only add support for this if we know that such broken systems exist.

Ah, but we don't do the eDP detection unconditionally. We only try and
register the eDP ports if the hardware tells us it is present...

has_edp_a() (DP on port D invokes trust in the VBT)

So we are in a situation where the hw claims there is both a LVDS and
eDP connection. You have already demonstrated that such broken HW
exists, have you not? Either that or we have missed a fuse to override
the DP detected bit.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre