[Intel-gfx] [PATCH] drm/i915: Retry on every aux read.

[Vivi, Rodrigo]

On Tue, 2015-10-20 at 09:39 +0200, Daniel Vetter wrote:
> On Tue, Oct 20, 2015 at 10:02:21AM +0300, Jani Nikula wrote:
> > On Tue, 20 Oct 2015, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> > > We have an inconsistency on our code on using 
> > > intel_dp_dpcd_read_wake with
> > > retries and when using drm_dp_dpcd_read helper without retry.
> > 
> > We're supposed to do the retries when the sink may be in a power 
> > down
> > state. We're not very good at tracking that, and we've cargo culted 
> > the
> > retry variants all over the place without thinking. This is why 
> > we're
> > inconsistent.
> > 
> > > Since the retries help in many cases let's be consistent and be 
> > > on
> > > the safe side retrying on every aux read, including i2c ones.
> > 
> > Please let's not add superfluous retries at different levels of the
> > stack just to be safe. It's like a variant of the C programmer's
> > disease.

I'm not the one who is adding, but just being consistent on our side. 

> > 
> > If the retries really help [citation needed] we need to figure out 
> > what
> > we're doing wrong and where, and preferrably fix this at the DP 
> > helper
> > level for all drivers, if possible.

Yes, I'm sorry, I forgot to copy the citation from the first patch in
this thread that mentions this fix sink crc for Skylake... and also for
some Broadwells what made me to remind about this patch....

> 
> Yeah, same comment as I've done last time around. If we need this, we 
> need
> to do this in core dp helpers.

If it is "superfluous" and "disease" to make the consistence in our
driver I can't imagine the words that I'm going to hear if I try to
touch other drivers...

To see if we should actually do this in our level or on the drm level I
was trying to understand where does:
" Sinks are *supposed* to come up within 1ms from an off state, but
we're also- * supposed to retry 3 times per the spec."

But I couldn't find anything in Vesa spec that tells that neither on
BSPec... at BSpec for some platforms I found the 3 times retry for aux
ctl programming what we do already...

So, yeah, I'm not convinced that we should through this to another
level... So it is either this or the first one that fix the sink crc
case individually and let sink crc reliable...

> -Daniel