Re: [PATCH 07/11] drm/i915: Fix for DP CTS test 4.2.2.5 - I2C DEFER handling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Apr 07, 2015 at 11:29:43AM -0300, Paulo Zanoni wrote:
> 2015-04-06 23:11 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.
> >
> > V2:
> > - Added a check on the number of I2C Defers to limit the number
> >   of times that the retries variable will be decremented. This
> >   is to address review feedback regarding possible infinite loops
> >   from misbehaving sink devices.
> >
> > Signed-off-by: Todd Previte <tprevite@gmail.com>
> > Cc: dri-devel@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 79968e3..23025cf 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -469,6 +469,12 @@ 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
> > +                        */
> > +                       if (aux->i2c_defer_count < 8)
> 
> I don't think this is the way to go. During normal
> (non-compliance-testing) operation we never zero i2c_defer_count, so
> we can't expect this to work, since we may start drm_dp_i2c_do_msg
> with a i2c_defer_count value different than zero. Also, during i915.ko
> DP compliance we only zero i2c_defer_count at the very beginning of
> each test, not at every aux transaction, and I really think we need a
> solution that is not specific to compliance testing.

What I was suggesting earlier (or trying to at least) would be
simply something like this:

int defer_native = 0, defer_i2c = 0;
while (defer_native < 7 && defer_i2c < 7) {
	...
	case DP_AUX_NATIVE_REPLY_NACK:
		...
		defer_native++;
		continue;
	}
	...
	case DP_AUX_I2C_REPLY_DEFER:
		...
		defer_i2c++;
		continue;
	}
	...
}

> 
> 
> > +                               retry--;
> >                         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
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel