On 05/05/2015 07:17 AM, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > Currently the IBX transcoder B workarounds are not working correctly. > Well, the HDMI one seems to be working somewhat, but the DP one is > definitely busted. > > After a bit of experimentation it looks like the best way to make this > work is first disable the port on transcoder B, and then re-enable it > transcoder A, and immediately disable it again. > > We can also clean up the code by noting that we can't be called without > a valid crtc. And also note that port A on ILK does not need the > workaround, so let's check for that one too. > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/intel_dp.c | 37 ++++++++++++++++------------- > drivers/gpu/drm/i915/intel_hdmi.c | 50 > ++++++++++++++++++--------------------- > drivers/gpu/drm/i915/intel_sdvo.c | 41 +++++++++++++------------------- > 3 files changed, 60 insertions(+), 68 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 17b006c..3401cde 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3821,6 +3821,7 @@ static void > intel_dp_link_down(struct intel_dp *intel_dp) > { > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc); > enum port port = intel_dig_port->port; > struct drm_device *dev = intel_dig_port->base.base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -3837,34 +3838,38 @@ intel_dp_link_down(struct intel_dp *intel_dp) > if ((IS_GEN7(dev) && port == PORT_A) || > (HAS_PCH_CPT(dev) && port != PORT_A)) { > DP &= ~DP_LINK_TRAIN_MASK_CPT; > - I915_WRITE(intel_dp->output_reg, DP | > DP_LINK_TRAIN_PAT_IDLE_CPT); > + DP |= DP_LINK_TRAIN_PAT_IDLE_CPT; > } else { > if (IS_CHERRYVIEW(dev)) > DP &= ~DP_LINK_TRAIN_MASK_CHV; > else > DP &= ~DP_LINK_TRAIN_MASK; > - I915_WRITE(intel_dp->output_reg, DP | DP_LINK_TRAIN_PAT_IDLE); > + DP |= DP_LINK_TRAIN_PAT_IDLE; > } > + I915_WRITE(intel_dp->output_reg, DP); > POSTING_READ(intel_dp->output_reg); > > - if (HAS_PCH_IBX(dev) && > - I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { > - /* Hardware workaround: leaving our transcoder select > - * set to transcoder B while it's off will prevent the > - * corresponding HDMI output on transcoder A. > - * > - * Combine this with another hardware workaround: > - * transcoder select bit can only be cleared while the > - * port is enabled. > - */ > - DP &= ~DP_PIPEB_SELECT; > + DP &= ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE); > + I915_WRITE(intel_dp->output_reg, DP); > + POSTING_READ(intel_dp->output_reg); > + > + /* > + * HW workaround for IBX, we need to move the port > + * to transcoder A after disabling it to allow the > + * matching HDMI port to be enabled on transcoder A. > + */ > + if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B && port != PORT_A) { > + /* always enable with pattern 1 (as per spec) */ > + DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK); > + DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1; > + I915_WRITE(intel_dp->output_reg, DP); > + POSTING_READ(intel_dp->output_reg); > + > + DP &= ~DP_PORT_EN; > I915_WRITE(intel_dp->output_reg, DP); > POSTING_READ(intel_dp->output_reg); > } > > - DP &= ~DP_AUDIO_OUTPUT_ENABLE; > - I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN); > - POSTING_READ(intel_dp->output_reg); > msleep(intel_dp->panel_power_down_delay); > } > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 308015e..9b9a69e 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1096,39 +1096,13 @@ static void intel_disable_hdmi(struct intel_encoder > *encoder) > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > u32 temp; > - u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; > > if (crtc->config->has_audio) > intel_audio_codec_disable(encoder); > > temp = I915_READ(intel_hdmi->hdmi_reg); > > - /* HW workaround for IBX, we need to move the port to transcoder A > - * before disabling it. */ > - if (HAS_PCH_IBX(dev)) { > - struct drm_crtc *crtc = encoder->base.crtc; > - int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1; > - > - if (temp & SDVO_PIPE_B_SELECT) { > - temp &= ~SDVO_PIPE_B_SELECT; > - I915_WRITE(intel_hdmi->hdmi_reg, temp); > - POSTING_READ(intel_hdmi->hdmi_reg); > - > - /* Again we need to write this twice. */ > - I915_WRITE(intel_hdmi->hdmi_reg, temp); > - POSTING_READ(intel_hdmi->hdmi_reg); > - > - /* Transcoder selection bits only update > - * effectively on vblank. */ > - if (crtc) > - intel_wait_for_vblank(dev, pipe); > - else > - msleep(50); > - } > - } > - > - temp &= ~enable_bits; > - > + temp &= ~(SDVO_ENABLE | SDVO_AUDIO_ENABLE); > I915_WRITE(intel_hdmi->hdmi_reg, temp); > POSTING_READ(intel_hdmi->hdmi_reg); > > @@ -1136,6 +1110,28 @@ static void intel_disable_hdmi(struct intel_encoder > *encoder) > chv_powergate_phy_lanes(encoder, 0xf); > > intel_hdmi->set_infoframes(&encoder->base, false, NULL); > + > + /* > + * HW workaround for IBX, we need to move the port > + * to transcoder A after disabling it to allow the > + * matching DP port to be enabled on transcoder A. > + */ > + if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B) { > + temp &= ~SDVO_PIPE_B_SELECT; > + temp |= SDVO_ENABLE; > + /* > + * HW workaround, need to write this twice for issue > + * that may result in first write getting masked. > + */ > + I915_WRITE(intel_hdmi->hdmi_reg, temp); > + POSTING_READ(intel_hdmi->hdmi_reg); > + I915_WRITE(intel_hdmi->hdmi_reg, temp); > + POSTING_READ(intel_hdmi->hdmi_reg); > + > + temp &= ~SDVO_ENABLE; > + I915_WRITE(intel_hdmi->hdmi_reg, temp); > + POSTING_READ(intel_hdmi->hdmi_reg); > + } > } > > static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool > respect_dvi_limit) > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > b/drivers/gpu/drm/i915/intel_sdvo.c > index e3e9c98..4a87691 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -1437,6 +1437,7 @@ static void intel_disable_sdvo(struct intel_encoder > *encoder) > { > struct drm_i915_private *dev_priv = encoder->base.dev->dev_private; > struct intel_sdvo *intel_sdvo = to_sdvo(encoder); > + struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > u32 temp; > > intel_sdvo_set_active_outputs(intel_sdvo, 0); > @@ -1445,32 +1446,22 @@ static void intel_disable_sdvo(struct intel_encoder > *encoder) > DRM_MODE_DPMS_OFF); > > temp = I915_READ(intel_sdvo->sdvo_reg); > - if ((temp & SDVO_ENABLE) != 0) { > - /* HW workaround for IBX, we need to move the port to > - * transcoder A before disabling it. */ > - if (HAS_PCH_IBX(encoder->base.dev)) { > - struct drm_crtc *crtc = encoder->base.crtc; > - int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1; > - > - if (temp & SDVO_PIPE_B_SELECT) { > - temp &= ~SDVO_PIPE_B_SELECT; > - I915_WRITE(intel_sdvo->sdvo_reg, temp); > - POSTING_READ(intel_sdvo->sdvo_reg); > - > - /* Again we need to write this twice. */ > - I915_WRITE(intel_sdvo->sdvo_reg, temp); > - POSTING_READ(intel_sdvo->sdvo_reg); > - > - /* Transcoder selection bits only update > - * effectively on vblank. */ > - if (crtc) > - > intel_wait_for_vblank(encoder->base.dev, pipe); > - else > - msleep(50); > - } > - } > > - intel_sdvo_write_sdvox(intel_sdvo, temp & ~SDVO_ENABLE); > + temp &= ~SDVO_ENABLE; > + intel_sdvo_write_sdvox(intel_sdvo, temp); > + > + /* > + * HW workaround for IBX, we need to move the port > + * to transcoder A after disabling it to allow the > + * matching DP port to be enabled on transcoder A. > + */ > + if (HAS_PCH_IBX(dev_priv) && crtc->pipe == PIPE_B) { > + temp &= ~SDVO_PIPE_B_SELECT; > + temp |= SDVO_ENABLE; > + intel_sdvo_write_sdvox(intel_sdvo, temp); > + > + temp &= ~SDVO_ENABLE; > + intel_sdvo_write_sdvox(intel_sdvo, temp); > } > } > >
Cool, again, testing wins. Reviewed-by: Jesse Barnes <[email protected]> _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
