Re: [PATCH 1/4] drm/i915/dp: retry i2c-over-aux seven times on AUX DEFER

Jani Nikula
2013-09-27T11:49:16Z

On Fri, 20 Sep 2013, Todd Previte <tprevite@gmail.com> wrote:
> On 09/20/2013 06:42 AM, Jani Nikula wrote:
>> Per DP1.2 spec.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 9770160..6626514 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -654,7 +654,12 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
>> break;
>> }
>>
>> - for (retry = 0; retry < 5; retry++) {
>> + /*
>> + * DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device is
>> + * required to retry at least seven times upon receiving AUX_DEFER
>> + * before giving up the AUX transaction.
>> + */
>> + for (retry = 0; retry < 7; retry++) {
>> ret = intel_dp_aux_ch(intel_dp,
>> msg, msg_bytes,
>> reply, reply_bytes);
>>
> Hey Jani,
>
> Although it's not explicitly stated in the specification (I went through
> both the 1.1a and 1.2a specifications and couldn't find it), I think the
> the retry count of 7 applies to all AUX transactions, both native and
> I2C. That's alluded to in the description of the link training sequence
> (pg 382, 2nd paragraph from the bottom) where the sink may defer up to
> seven times before the source can abort training.
>
> With that in mind, it might be a good idea to use a defined constant for
> the retry count for both native and I2C AUX transactions. That would
> keep it consistent throughout the code and be easier to update moving
> forward should the specification change.

Todd, are you willing to give your reviewed-by on this one patch per our
irc discussion? We can do further cleanups later.

I've sent a new version of 3/4 with the #defines separately.

BR,
Jani.

--
Jani Nikula, Intel Open Source Technology Center