[Intel-gfx] [PATCH 1/3] drm/i915: fix hsw_fdi_link_train "retry" code

[Daniel Vetter]

On Thu, Nov 29, 2012 at 11:29:31AM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> We were previously doing exactly what the "mode set sequence for CRT"
> document mandates, but whenever we failed to train the link in the
> first tentative, all the other subsequent retries always failed. In
> one of my monitors that has 47 modes, I was usually getting around 3
> failures when running "testdisplay -a".
> 
> After this patch, even if we fail in the first tentative, we can
> succeed in the next ones. So now when running "testdisplay -a" I see
> around 3 times the message "FDI link training done on step 1" and no
> failures.
> 
> Notice that now the "retry" code looks a lot like the DP retry code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 43 +++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> I'd like this to go to 3.8 somehow.
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 852012b..3264cb4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -138,6 +138,19 @@ static const long hsw_ddi_buf_ctl_values[] = {
>  	DDI_BUF_EMP_800MV_3_5DB_HSW
>  };
>  
> +static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> +				    enum port port)
> +{
> +	uint32_t reg = DDI_BUF_CTL(port);
> +	int i;
> +
> +	for (i = 0; i < 8; i++) {
> +		udelay(1);
> +		if (I915_READ(reg) & DDI_BUF_IS_IDLE)
> +			return;
> +	}
> +	DRM_ERROR("Timeout waiting for DDI BUF %c idle bit\n", port_name(port));
> +}
>  
>  /* Starting with Haswell, different DDI ports can work in FDI mode for
>   * connection to the PCH-located connectors. For this, it is necessary to train
> @@ -231,18 +244,30 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  			return;
>  		}
>  
> +		temp = I915_READ(DDI_BUF_CTL(PORT_E));
> +		temp &= ~DDI_BUF_CTL_ENABLE;
> +		I915_WRITE(DDI_BUF_CTL(PORT_E), temp);
> +		POSTING_READ(DDI_BUF_CTL(PORT_E));
> +
>  		/* Disable DP_TP_CTL and FDI_RX_CTL and retry */
> -		I915_WRITE(DP_TP_CTL(PORT_E),
> -			   I915_READ(DP_TP_CTL(PORT_E)) & ~DP_TP_CTL_ENABLE);
> +		temp = I915_READ(DP_TP_CTL(PORT_E));
> +		temp &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
> +		temp |= DP_TP_CTL_LINK_TRAIN_PAT1;
> +		I915_WRITE(DP_TP_CTL(PORT_E), temp);
> +		POSTING_READ(DP_TP_CTL(PORT_E));
> +
> +		intel_wait_ddi_buf_idle(dev_priv, PORT_E);
>  
>  		rx_ctl_val &= ~FDI_RX_ENABLE;
>  		I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> +		POSTING_READ(_FDI_RXA_CTL);
>  
>  		/* Reset FDI_RX_MISC pwrdn lanes */
>  		temp = I915_READ(_FDI_RXA_MISC);
>  		temp &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
>  		temp |= FDI_RX_PWRDN_LANE1_VAL(2) | FDI_RX_PWRDN_LANE0_VAL(2);
>  		I915_WRITE(_FDI_RXA_MISC, temp);
> +		POSTING_READ(_FDI_RXA_MISC);

What now slightly irks me here is that this sequence and the one in
intel_ddi_fdi_disable don't match exactly. Imo it would make sense to have
both the same (after all, we disable the same piece of hw) - have you
tried that out (there's obviously some slight unification required first)?
-Daniel

>  	}
>  
>  	DRM_ERROR("FDI link training failed!\n");
> @@ -1222,20 +1247,6 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>  	}
>  }
>  
> -static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> -				    enum port port)
> -{
> -	uint32_t reg = DDI_BUF_CTL(port);
> -	int i;
> -
> -	for (i = 0; i < 8; i++) {
> -		udelay(1);
> -		if (I915_READ(reg) & DDI_BUF_IS_IDLE)
> -			return;
> -	}
> -	DRM_ERROR("Timeout waiting for DDI BUF %c idle bit\n", port_name(port));
> -}
> -
>  static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  {
>  	struct drm_encoder *encoder = &intel_encoder->base;
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch