Em Ter, 2016-03-01 às 16:16 +0200, ville.syrjala@xxxxxxxxxxxxxxx escreveu: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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. The sentence above basically says: "forget all the previous paragraphs of this commit message since they're just history and go read BSpec since it's now correct" :) > > v2: Add a note that the spec is now correct With or without changes: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > Cc: Paulo Zanoni <przanoni@xxxxxxxxx> > Cc: Art Runyan <arthur.j.runyan@xxxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_ddi.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 21a9b83f3bfc..fc4ca50f7159 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -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); > @@ -3078,12 +3078,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); _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx