| Message ID | 1427822106-29617-8-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-03-31 14:15 GMT-03:00 Todd Previte <tprevite@gmail.com>: > For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source > device must attempt at least 7 times to read the EDID when it receives an > I2C defer. The normal DRM code makes only 7 retries, regardless of whether > or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails > since there are native defers interspersed with the I2C defers which > results in less than 7 EDID read attempts. > > The solution is to decrement the retry counter when an I2C DEFER is returned > such that another read attempt will be made. This situation should normally > only occur in compliance testing, however, as a worse case real-world > scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers) > for a single transaction to complete. The net result is a slightly slower > response to an EDID read that shouldn't significantly impact overall > performance. > > Signed-off-by: Todd Previte <tprevite@gmail.com> > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/drm_dp_helper.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 79968e3..0539758 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > case DP_AUX_I2C_REPLY_DEFER: > DRM_DEBUG_KMS("I2C defer\n"); > aux->i2c_defer_count++; > + /* DP Compliance Test 4.2.2.5 Requirement: > + * Must have at least 7 retries for I2C defers on the > + * transaction to pass this test > + */ > + retry--; I wouldn't be surprised if someone discovers a monitor or some sort of dongle that keeps sending I2C defer errors forever, keeping us in an infinite loop. Shouldn't we count each error in separate? Or maybe just loop up to 14 times, in case that doesn't violate any spec (I didn't check)? > usleep_range(400, 500); > continue; > > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 4/6/15 5:05 PM, Paulo Zanoni wrote: > 2015-03-31 14:15 GMT-03:00 Todd Previte <tprevite@gmail.com>: >> For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source >> device must attempt at least 7 times to read the EDID when it receives an >> I2C defer. The normal DRM code makes only 7 retries, regardless of whether >> or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails >> since there are native defers interspersed with the I2C defers which >> results in less than 7 EDID read attempts. >> >> The solution is to decrement the retry counter when an I2C DEFER is returned >> such that another read attempt will be made. This situation should normally >> only occur in compliance testing, however, as a worse case real-world >> scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers) >> for a single transaction to complete. The net result is a slightly slower >> response to an EDID read that shouldn't significantly impact overall >> performance. >> >> Signed-off-by: Todd Previte <tprevite@gmail.com> >> Cc: dri-devel@lists.freedesktop.org >> --- >> drivers/gpu/drm/drm_dp_helper.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >> index 79968e3..0539758 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> case DP_AUX_I2C_REPLY_DEFER: >> DRM_DEBUG_KMS("I2C defer\n"); >> aux->i2c_defer_count++; >> + /* DP Compliance Test 4.2.2.5 Requirement: >> + * Must have at least 7 retries for I2C defers on the >> + * transaction to pass this test >> + */ >> + retry--; > I wouldn't be surprised if someone discovers a monitor or some sort of > dongle that keeps sending I2C defer errors forever, keeping us in an > infinite loop. Shouldn't we count each error in separate? Or maybe > just loop up to 14 times, in case that doesn't violate any spec (I > didn't check)? I think the safest thing to do would be to put a failsafe on the i2c_defer_counter. That would ensure that the compliance test gets its 7 retries and that if we do encounter a misbehaving device, the driver won't let the unending defers cause an infinite loop. Updated patch shortly. >> usleep_range(400, 500); >> continue; >> >> -- >> 1.9.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > >
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 79968e3..0539758 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -469,6 +469,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_I2C_REPLY_DEFER: DRM_DEBUG_KMS("I2C defer\n"); aux->i2c_defer_count++; + /* DP Compliance Test 4.2.2.5 Requirement: + * Must have at least 7 retries for I2C defers on the + * transaction to pass this test + */ + retry--; usleep_range(400, 500); continue;
For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source device must attempt at least 7 times to read the EDID when it receives an I2C defer. The normal DRM code makes only 7 retries, regardless of whether or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails since there are native defers interspersed with the I2C defers which results in less than 7 EDID read attempts. The solution is to decrement the retry counter when an I2C DEFER is returned such that another read attempt will be made. This situation should normally only occur in compliance testing, however, as a worse case real-world scenario, it would result in 13 attempts ( 6 native defers, 7 I2C defers) for a single transaction to complete. The net result is a slightly slower response to an EDID read that shouldn't significantly impact overall performance. Signed-off-by: Todd Previte <tprevite@gmail.com> Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/drm_dp_helper.c | 5 +++++ 1 file changed, 5 insertions(+)