Re: [PATCH v3] drm/i915/dp: Give up link training clock recovery after 10 failed attempts

From: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
To: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
Cc: Marc Herbert <Marc.Herbert@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v3] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
Date: Tue, 17 Jul 2018 20:00:35 -0700	[thread overview]
Message-ID: <1531882835.28553.43.camel@intel.com> (raw)
In-Reply-To: <20180718010551.GB18153@nc-new>

On Tue, 2018-07-17 at 18:05 -0700, Nathan Ciobanu wrote:
> On Tue, Jul 17, 2018 at 03:21:17PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Mon, 2018-07-16 at 16:51 -0700, Marc Herbert wrote:
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > I think the bug is with this infinite loop which is at the
> > > > > mercy
> > > > > of an external device
> > > > > and in my case I have this MST hub which appears to be DP 1.2
> > > > > that triggers the
> > > > > infinite loop case. We have to limit the number of iterations
> > > > > and
> > > > > DP 1.4 spec happened to fix this issue. I'm a newbie in this
> > > > > area
> > > > > but in this case
> > > > > shouldn't we apply the same counter to all <= "DP 1.4"
> > > > > devices?
> > > > ok, the infinite loop situation is really bad... but I don't
> > > > believe
> > > > we are going with the right fix...
> > > > and a good indication is that your fix is for DP-1.4 while your
> > > > bug
> > > > is seeing on DP-1.2
> > > I thought the only reason the infinite loop fix isn't in 1.2 is
> > > just
> > > because there's
> > > no 1.2.1 spec... (plus the naive assumption that buggy sinks
> > > don't
> > > exist)
> > > 
> > Irrespective of whether the sink is DP1.2 or 1.4, if there are
> > sinks
> > out there that keep toggling between two values there should be an
> > overall limit to how many times this loop gets executed. Even if
> > this
> > isn't right fix for the issue at hand, the loop has to break.
> > 
> > Then there's the question of what the limit should be. We could use
> > the
> > DP1.4 limit as a reference and apply it widely. But there's a
> > problem
> > here, we have 4 vswing values and 4 pre-emphasis values. If the
> > sink
> > progressively changes one variable at a time, we'll need at least
> > 16
> > iterations. Nathan's patch here will prematurely error out and that
> > doesn't sound reasonable to impose on non DP1.4 sinks.
> I think it would be a max of 13 iterations since one of the vswing
> values will be max_vswing and the loop will terminate at that point
> without trying the other 3 preemph values for that voltage, correct?
> 
The spec defines maximum voltage swing level as the "sum of the
VOLTAGE_SWING_LANEx and PREEMPHASIS_LEVEL_LANEx value". So, we should
be looping through all values.

Can you check if https://patchwork.freedesktop.org/patch/239520/ helps?
Also, please file a fdo bug with dmesg.

-DK

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