Re: [PATCH] Revert "drm/i915: Implement Link Rate fallback on Link training failure"

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 01 Mar 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Mar 01, 2017 at 07:35:15PM +0200, Jani Nikula wrote:
>> On Wed, 01 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote:
>> >> This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c.
>> >> 
>> >> I assumed it's ok, but really should have double-checked - CI caught
>> >> tons of fail :(
>> 
>> Considering the velocity of drm-tip, I think any CI results for patches
>> have a rather limited best before date. The patch should've been resent
>> and gone through testing again before merging.
>> 
>> > For the record, the failure comes from the error message in
>> > intel_dp_get_link_train_fallback_values() as take the fallback path. As
>> > userspace is informed, we don't need an *ERROR* at that point.
>> >
>> > The really interesting question is why we are seeing link-training
>> > failures in CI at all, and whether igt should be checking and reporting
>> > link-status=BAD.
>> 
>> It's possible (I didn't check the logs) this pertains to the failure
>> mode I've sometimes seen, where clock recovery fails, but as we continue
>> with channel equalization anyway (without this patch), everything
>> succeeds there. At worst we need to root cause and fix that issue
>> first. :(
>
> The skl case seems pretty clear. We register DP for both port A and port
> E even though we should register it only for port E (I think). They
> both end up both using AUX A and so we think the same sink is connected
> to both, and then we try to enable port A which fail for obvious reasons.
>
> The culprit is init_vbt_defaults() which always sets .supports_dp=true
> for port A unless later overridden by the VBT. In this case the VBT has
> no port A, so we leave the .supports_dp flag set. So presumably we
> should just nuke this stuff from init_vbt_defaults().
>
> IIRC this was discussed at some point between Imre and Paulo, but I
> can't remember what the conclusion was, or if in fact there was one.

I think one idea was simply

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index e144f033f4b5..0261c841756f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1377,10 +1377,6 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
 			&dev_priv->vbt.ddi_port_info[port];
 
 		info->hdmi_level_shift = HDMI_LEVEL_SHIFT_UNKNOWN;
-
-		info->supports_dvi = (port != PORT_A && port != PORT_E);
-		info->supports_hdmi = info->supports_dvi;
-		info->supports_dp = (port != PORT_E);
 	}
 }
 
---

but apparently it's problematic for machines without VBT, specifically
some Chromebooks [citation needed].

Jani.


>
> The ilk failure case is a lot less clear. It's one of those cases
> where the sink just keeps requesting the same vswing/preemph all
> the time. I've seen it sometime in the past, but I've never been
> able to figure out what has caused it to happen with any specific
> sink.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx