[Intel-gfx] [PATCH] drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

[Ville Syrjälä]

On Mon, Apr 16, 2018 at 06:53:09PM +0300, Imre Deak wrote:
> LSPCON adapters in low-power state may ignore the first I2C write during
> TMDS output buffer enabling, resulting in a blank screen even with an
> otherwise enabled pipe. Fix this by reading back and validating the
> written value a few times.
> 
> The problem was noticed on GLK machines with an onboard LSPCON adapter
> after entering/exiting DC5 power state. Doing an I2C read of the adapter
> ID as the first transaction - instead of the I2C write to enable the
> TMDS buffers - returns the correct value. Based on this we assume that
> the transaction itself is sent properly, it's only the adapter that is
> not ready for some reason to accept this first write after waking from
> low-power state. In my case the second I2C write attempt always
> succeeded.

Yeah, I guess this is an OK approach. Probably not much point in
optimizing the non-LSPCON case since this is modeset stuff,
although we do have the adaptor type passed in so we could if we
wanted to.

Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105854
> Cc: Clinton Taylor <clinton.a.taylor at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 39 +++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> index 02a50929af67..e7f4fe2848a5 100644
> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -350,19 +350,44 @@ int drm_dp_dual_mode_set_tmds_output(enum drm_dp_dual_mode_type type,
>  {
>  	uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE;
>  	ssize_t ret;
> +	int retry;
>  
>  	if (type < DRM_DP_DUAL_MODE_TYPE2_DVI)
>  		return 0;
>  
> -	ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
> -				     &tmds_oen, sizeof(tmds_oen));
> -	if (ret) {
> -		DRM_DEBUG_KMS("Failed to %s TMDS output buffers\n",
> -			      enable ? "enable" : "disable");
> -		return ret;
> +	/*
> +	 * LSPCON adapters in low-power state may ignore the first write, so
> +	 * read back and verify the written value a few times.
> +	 */
> +	for (retry = 0; retry < 3; retry++) {
> +		uint8_t tmp;
> +
> +		ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
> +					     &tmds_oen, sizeof(tmds_oen));
> +		if (ret) {
> +			DRM_DEBUG_KMS("Failed to %s TMDS output buffers (%d attempts)\n",
> +				      enable ? "enable" : "disable",
> +				      retry + 1);
> +			return ret;
> +		}
> +
> +		ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN,
> +					    &tmp, sizeof(tmp));
> +		if (ret) {
> +			DRM_DEBUG_KMS("I2C read failed during TMDS output buffer %s (%d attempts)\n",
> +				      enable ? "enabling" : "disabling",
> +				      retry + 1);
> +			return ret;
> +		}
> +
> +		if (tmp == tmds_oen)
> +			return 0;
>  	}
>  
> -	return 0;
> +	DRM_DEBUG_KMS("I2C write value mismatch during TMDS output buffer %s\n",
> +		      enable ? "enabling" : "disabling");
> +
> +	return -EIO;
>  }
>  EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output);
>  
> -- 
> 2.13.2

-- 
Ville Syrjälä
Intel