On Wed, Aug 09, 2017 at 01:40:00PM -0700, Jim Bride wrote: > According to the eDP spec, when the count field in TEST_SINK_MISC > increments then the six bytes of sink CRC information in the DPCD > should be valid. Unfortunately, this doesn't seem to be the case > on some panels, and as a result we get some incorrect and inconsistent > values from the sink CRC DPCD locations at times. This problem exhibits > itself more on faster processors (relative failure rates HSW < SKL < KBL.) > In order to try and account for this, we try a lot harder to read the sink > CRC until we get consistent values twice in a row before returning what we > read and delay for a time before trying to read. We still see some > occasional failures, but reading the sink CRC is much more reliable, > particularly on SKL, with these changes than without. > > v2: * Reduce number of retries when reading the sink CRC (Jani) > * Refactor to minimize changes to the code (Jani) > * Rebase > v3: * Rebase > v4: * Switch from do-while to for loop when reading CRC values (Jani) > * Rebase > v5: * Checkpatch cleanup and commit message tweaks > * Rebase > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> I think I addressed all previous review comments for this patch. Any thoughts? Jim > Signed-off-by: Jim Bride <jim.bride@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_dp.c | 32 +++++++++++++++++++++++++++++--- > 1 file changed, 29 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 76c8a0b..b64757c 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3906,6 +3906,10 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) > u8 buf; > int count, ret; > int attempts = 6; > + u8 old_crc[6]; > + > + if (crc == NULL) > + return -ENOMEM; > > ret = intel_dp_sink_crc_start(intel_dp); > if (ret) > @@ -3929,11 +3933,33 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) > goto stop; > } > > - if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) { > - ret = -EIO; > - goto stop; > + /* > + * Sometimes it takes a while for the "real" CRC values to land in > + * the DPCD, so try several times until we get two reads in a row > + * that are the same. If we're an eDP panel, delay between reads > + * for a while since the values take a bit longer to propagate. > + */ > + for (attempts = 0; attempts < 6; attempts++) { > + intel_wait_for_vblank(dev_priv, intel_crtc->pipe); > + > + if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, > + crc, 6) < 0) { > + ret = -EIO; > + break; > + } > + > + if (attempts && memcmp(old_crc, crc, 6) == 0) > + break; > + memcpy(old_crc, crc, 6); > + > + if (is_edp(intel_dp)) > + usleep_range(20000, 25000); > } > > + if (attempts == 6) { > + DRM_DEBUG_KMS("Failed to get CRC after 6 attempts.\n"); > + ret = -ETIMEDOUT; > + } > stop: > intel_dp_sink_crc_stop(intel_dp); > return ret; > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx