On Thu, Nov 29, 2012 at 11:29:31AM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <[email protected]>
> 
> 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 <[email protected]>
> ---
>  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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to