Some bit rot there. I'll fix the numbering. Thanks for pointing it out. I did keep the second, redundant, FDI RX disable in place to limit some closed source driver changes. There is no downside to clearing bit 31 twice. >-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Wednesday, February 17, 2016 9:37 AM >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Paulo Zanoni; Runyan, Arthur J >Subject: Re: [PATCH] drm/i915: Disable FDI RX before DDI_BUF_CTL > >On Tue, Dec 08, 2015 at 04:47:55PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: >> 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. > >Ping. Art, I hear you're back now ;) Any thoughts on this change, and >the slight mess in Bspec? > > >> >> 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 5d20c64d8566..a89a17b7bb76 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -687,6 +687,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); >> @@ -701,10 +705,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); >> @@ -3094,12 +3094,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); >> -- >> 2.4.10 > >-- >Ville Syrjälä >Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
