[Intel-gfx] [PATCH 1/2] drm: add retries for lspcon status check

[Imre Deak]

On Fri, Aug 11, 2017 at 06:58:26PM +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.

Hm yea, I was wondering if this is the same issue we saw earlier due to
HPD not being asserted. But looking at the logs it's not the case. After
HPD is asserted it really takes this much time (~36ms) for the adaptor
to respond. This is against the DP 1.4 spec which specifies a 20ms
maximum wake up time, but what can you do.

> 
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Cc: 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..c63eac8 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;
> +	u8 retry = 5;

Nitpick: no reason for a specific type so just int?

>  	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));
> +	do {

Nitpick: a for loop whenever possible is generally clearer.

With the above optionally changed:
Reviewed-by: Imre Deak <imre.deak at intel.com>

> +		ret = drm_dp_dual_mode_read(adapter,
> +					    DP_DUAL_MODE_LSPCON_CURRENT_MODE,
> +					    &data, sizeof(data));
> +		if (!ret || !retry--)
> +			break;
> +		usleep_range(500, 1000);
> +	} while (1);
> +
>  	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
>