drm/i915: Disable FDI RX before DDI_BUF_CTL
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Tue, 1 Mar 2016 14:16:23 +0000 (16:16 +0200)
committerVille Syrjälä <ville.syrjala@linux.intel.com>
Fri, 1 Apr 2016 19:04:14 +0000 (22:04 +0300)
Bspec is confused w.r.t. the HSW/BDW FDI disable sequence. It lists
FDI RX disable both as step 13 and step 18 in the sequence. But I dug
up an old BUN mail from Art that moved the FDI RX disable to happen
before DDI_BUF_CTL disable. That BUN did not renumber the steps and just
added a note:
"Workaround: Disable PCH FDI Receiver before disabling DDI_BUF_CTL."

The BUN described the symptoms of the fixed issue as:
"PCH display underflow and a black screen on the analog CRT port that
happened after a FDI re-train"

I suppose later someone tried to renumber the steps to match, but forgot
to remove the FDI RX disable from its old position in the sequence.

They also forgot to update the note describing what should be done in
case of an FDI training failure. Currently it says:
"To retry FDI training, follow the Disable Sequence steps to Disable FDI,
but skip the steps related to clocks and PLLs (16, 19, and 20), ..."

It should really say "17, 20, and 21" with the current sequence because
those are the steps that deal with PLLs and whatnot, after step 13 became
FDI RX disable. And had the step 18 FDI RX disable been removed, as I
suspect it should have, the note should actually say "17, 19, and 20".

So, let's move the FDI RX disable to happen before DDI_BUF_CTL disable,
as that would appear to be the correct order based on the BUN.

Note that Art has since unconfused the spec, and so this patch should
now match the steps listed in the spec.

v2: Add a note that the spec is now correct

Cc: Paulo Zanoni <przanoni@gmail.com>
Cc: Art Runyan <arthur.j.runyan@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1456841783-4779-1-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
drivers/gpu/drm/i915/intel_ddi.c

index 2758622a5c2d01d721c67b9e150e778793ebb575..766156f88ef4a91f109790f06a75b6e24ab28d48 100644 (file)
@@ -629,6 +629,10 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
                        break;
                }
 
+               rx_ctl_val &= ~FDI_RX_ENABLE;
+               I915_WRITE(FDI_RX_CTL(PIPE_A), rx_ctl_val);
+               POSTING_READ(FDI_RX_CTL(PIPE_A));
+
                temp = I915_READ(DDI_BUF_CTL(PORT_E));
                temp &= ~DDI_BUF_CTL_ENABLE;
                I915_WRITE(DDI_BUF_CTL(PORT_E), temp);
@@ -643,10 +647,6 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 
                intel_wait_ddi_buf_idle(dev_priv, PORT_E);
 
-               rx_ctl_val &= ~FDI_RX_ENABLE;
-               I915_WRITE(FDI_RX_CTL(PIPE_A), rx_ctl_val);
-               POSTING_READ(FDI_RX_CTL(PIPE_A));
-
                /* Reset FDI_RX_MISC pwrdn lanes */
                temp = I915_READ(FDI_RX_MISC(PIPE_A));
                temp &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
@@ -1911,12 +1911,18 @@ void intel_ddi_fdi_disable(struct drm_crtc *crtc)
        struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
        uint32_t val;
 
-       intel_ddi_post_disable(intel_encoder);
-
+       /*
+        * Bspec lists this as both step 13 (before DDI_BUF_CTL disable)
+        * and step 18 (after clearing PORT_CLK_SEL). Based on a BUN,
+        * step 13 is the correct place for it. Step 18 is where it was
+        * originally before the BUN.
+        */
        val = I915_READ(FDI_RX_CTL(PIPE_A));
        val &= ~FDI_RX_ENABLE;
        I915_WRITE(FDI_RX_CTL(PIPE_A), val);
 
+       intel_ddi_post_disable(intel_encoder);
+
        val = I915_READ(FDI_RX_MISC(PIPE_A));
        val &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
        val |= FDI_RX_PWRDN_LANE1_VAL(2) | FDI_RX_PWRDN_LANE0_VAL(2);