On ma, 2016-06-20 at 08:54 +0100, Chris Wilson wrote: > 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@xxxxxxxxxxxxxxx> > > > > CC: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > > > 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) Ah, yes, missed that check. > 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? I didn't, the issue this and the next patch fixes was on SKL. But yes commit f30d26e468322b50d5e376bec40658683aff8ece Author: Jani Nikula <jani.nikula@xxxxxxxxx> Date: Wed Jan 16 10:53:40 2013 +0200 drm/i915/eDP: do not write power sequence registers for ghost eDP show that there is such systems out there, although in the above case the LVDS output wasn't a ghost. In any case I agree now that we should consider this case and add proper support for this. However I think if BIOS has enabled the LVDS output that should already give enough confidence that the LVDS is real. In that way we could avoid disabling a properly functioning LVDS output (leading to flicker even with fastboot). I've put something together for this (top three commits): https://github.com/ideak/linux/commits/lvds_edp_init This still misses disabling the LVDS connector/encoder, I could still add that on top. > Either that or we have missed a fuse to override the DP detected bit. We do check the strap bit on ILK, but we have the same issue on SNB/IVB as pointed out by Ville, where we don't. The machine in the above commit was an IVB/port A, so it's possible we could've avoided the issue by checking a strap bit if it exists. I will check the docs. --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
