On Tue, 2015-10-20 at 08:38 -0700, Rodrigo Vivi wrote: > 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@xxxxxxxxx> 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... Besides, I noticed on drm level we have 32 times retry already but when aux return -EBUSY... So I wonder if it would be acceptable to retry for all errors not only for -EBUSY or if we should find out when to return EBUSY propperly on our aux transfer... > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
