On Fri, Jun 17, 2016 at 06:48:45PM +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.
>
> This was caught by CI with the PPS sanity checks in place and the
> initial eDP EDID readout waiting for the panel power cycle timeout
> without the PPS registers being initialized. To solve this latter
> problem move PPS register init earlier before the first use of the
> PPS.
>
> CC: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index be08351..fe543d7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5316,8 +5316,22 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> if (!is_edp(intel_dp))
> return true;
>
> + /*
> + * On ILK we may get here with LVDS already registered. Since the
ILK,SNB,IVB, if you want to accurately list the platforms.
> + * driver uses the only internal power sequencer available for both
> + * eDP and LVDS bail out early in this case to prevent interfering
> + * with an already powered-on LVDS power sequencer.
> + */
> + for_each_intel_encoder(dev, intel_encoder) {
> + if (intel_encoder->type == INTEL_OUTPUT_LVDS) {
> + DRM_INFO("LVDS was detected, not registering eDP\n");
> + return false;
> + }
> + }
We could abort much earlier, in the encoder init.
> +
> pps_lock(intel_dp);
> intel_edp_panel_vdd_sanitize(intel_dp);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> pps_unlock(intel_dp);
For VLV/CHV we do this stuff in the encoder init already. Maybe we
should move all that stuff here as well?
>
> /* Cache DPCD and EDID for edp. */
> @@ -5334,11 +5348,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> return false;
> }
>
> - /* We now know it's not a ghost, init power sequence regs. */
> - pps_lock(intel_dp);
> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> - pps_unlock(intel_dp);
> -
> mutex_lock(&dev->mode_config.mutex);
> edid = drm_get_edid(connector, &intel_dp->aux.ddc);
> if (edid) {
> --
> 2.5.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
