[Intel-gfx] [PATCH 1/3] drm: Add retries for lspcon status check

[Sharma, Shashank]

Regards

Shashank


On 8/22/2017 8:24 PM, Imre Deak wrote:
> On Tue, Aug 22, 2017 at 08:23:49PM +0530, Shashank Sharma wrote:
>> It's an observation during some CI tests that few LSPCON chips
>> respond slow while system is under load, and need some delay
>> while reading current mode status using i2c-over-aux channel.
>>
>> This patch:
>> - Adds few retries and delays before declaring a read
>>    failure from LSPCON hardware.
>> - Changes the debug level of the print from ERROR->DEBUG
>>    whereas another patch in I915 will add an ERROR message
>>    from the caller when we have timed out all our limits.
>>
>> V2: addressed review comments from Imre, and added r-b
>>      - use int instead of u8 for counter
>>      - use for loop instead of do...while();
>> V3: Rebase
>>
>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>> Cc: Imre Deak <imre.deak at intel.com>
>>
>> Reviewed-by: Imre Deak <imre.deak at intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
>> ---
>>   drivers/gpu/drm/drm_dp_dual_mode_helper.c | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> index 80e62f6..fc8c7ac 100644
>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> @@ -409,6 +409,7 @@ int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>>   			enum drm_lspcon_mode *mode)
>>   {
>>   	u8 data;
>> +	int retry;
>>   	int ret = 0;
>>   
>>   	if (!mode) {
>> @@ -417,10 +418,17 @@ int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>>   	}
>>   
>>   	/* Read Status: i2c over aux */
>> -	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_LSPCON_CURRENT_MODE,
>> -				    &data, sizeof(data));
>> +	for (retry = 5; ; retry--) {
>> +		ret = drm_dp_dual_mode_read(adapter,
>> +					    DP_DUAL_MODE_LSPCON_CURRENT_MODE,
>> +					    &data, sizeof(data));
> Why do we still need the retry here with patch 3/3?
The only reason I can think of would be I am paranoid :) ? I was 
thinking of going with this one first and see
the CI results, and then later remove and optimize. But I guess you are 
right, this would be too many retries
here, and I should do it other way around.

- Shashank
>> +		if (!ret || !retry)
>> +			break;
>> +		usleep_range(500, 1000);
>> +	}
>> +
>>   	if (ret < 0) {
>> -		DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
>> +		DRM_DEBUG_KMS("LSPCON read(0x80, 0x41) failed\n");
>>   		return -EFAULT;
>>   	}
>>   
>> -- 
>> 2.7.4
>>