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
