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

From: Imre Deak <imre.deak@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH] Revert "drm/i915: Implement Link Rate fallback on Link training failure"
Date: Thu, 2 Mar 2017 12:07:05 +0200	[thread overview]
Message-ID: <20170302100705.GC19526@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <20170301175626.GB31595@intel.com>

On Wed, Mar 01, 2017 at 07:56:26PM +0200, Ville Syrjälä 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.
> 
> 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.

Looks like starting with:
[    3.621706] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1
[    3.622794] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1
[    3.623836] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1
[    3.624903] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1
[    3.625983] [drm:drm_dp_i2c_do_msg] I2C nack (result=0, size=1

and so failing to read out EDID. As a result we'll use the default 1024x768
which fails. In the working case we use the native 1920x1200 which succeeds.
Could be that 1024x768 didn't work even before?

I can't see how the patch would affect the I2C transfers.

--Imre

> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx