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

[Imre Deak <imre.deak@intel.com>]

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.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx