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

[Jani Nikula <jani.nikula@linux.intel.com>]

Hi Todd -

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.

I agree in principle; defines good, magic values bad.

However we don't currently have a retry max for AUX DEFER in native
transactions. Maybe we should. But I'm wary of adding 7 there, because
we don't have a max now, and the spec is vague on this. It just might
break stuff.

Also, for i2c over AUX the retry count of 7 is the minimum by the
spec. And it goes on to say that depending on the moon position and some
other variables, like i2c bit rate, it is recommended the source adjusts
the retry count and interval.

So the retry count currently isn't the same in our code, and I don't
think it's reasonable to force them to be the same either.


BR,
Jani.



>
>
> -T
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center