> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > ville.syrjala@xxxxxxxxxxxxxxx > Sent: Tuesday, May 05, 2015 7:06 AM > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: [PATCH v2 5/9] drm/i915: Fix 12bpc HDMI enable for IBX > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Follow the procedure listed in Bspec to toggle the port enable bit off > and on when enabling HDMI with 12bpc and pixel repeat on IBX. The old > code didn't actually enable the port before "toggling" the bit back off, > so the whole workaround was essentially a nop. > > Also take the opportunity to clarify the code by splitting the gmch > platforms to a separate (much more straightforward) function. Per prior reviews seen before, it would be better if there are separate patches for fixing issue and refactoring code. I think it is fine either way, but is bit easy for reviewing code having separate patches. With that, Reviewed-by: Chandra Konduru <Chandra.konduru@xxxxxxxxx> > > v2: Rebased due to crtc->config changes > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_hdmi.c | 78 ++++++++++++++++++++++++++--------- > ---- > 1 file changed, 53 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 2e98e33..766bdb1 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -944,47 +944,73 @@ static void intel_enable_hdmi_audio(struct > intel_encoder *encoder) > intel_audio_codec_enable(encoder); > } > > -static void intel_enable_hdmi(struct intel_encoder *encoder) > +static void g4x_enable_hdmi(struct intel_encoder *encoder) > { > struct drm_device *dev = encoder->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); > + struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); Unrelated change > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > u32 temp; > - u32 enable_bits = SDVO_ENABLE; > - > - if (intel_crtc->config->has_audio) > - enable_bits |= SDVO_AUDIO_ENABLE; > > temp = I915_READ(intel_hdmi->hdmi_reg); > > - /* HW workaround for IBX, we need to move the port to transcoder A > - * before disabling it, so restore the transcoder select bit here. */ > - if (HAS_PCH_IBX(dev)) > - enable_bits |= SDVO_PIPE_SEL(intel_crtc->pipe); > + temp |= SDVO_ENABLE; > + if (crtc->config->has_audio) > + temp |= SDVO_AUDIO_ENABLE; > > - /* HW workaround, need to toggle enable bit off and on for 12bpc, but > - * we do this anyway which shows more stable in testing. > - */ > - if (HAS_PCH_SPLIT(dev)) { > - I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE); > - POSTING_READ(intel_hdmi->hdmi_reg); > - } > + I915_WRITE(intel_hdmi->hdmi_reg, temp); > + POSTING_READ(intel_hdmi->hdmi_reg); > > - temp |= enable_bits; > + if (crtc->config->has_audio) > + intel_enable_hdmi_audio(encoder); > +} > > +static void ibx_enable_hdmi(struct intel_encoder *encoder) > +{ > + struct drm_device *dev = encoder->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > + u32 temp; > + > + temp = I915_READ(intel_hdmi->hdmi_reg); > + > + temp |= SDVO_ENABLE; > + if (crtc->config->has_audio) > + temp |= SDVO_AUDIO_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); > > - /* HW workaround, need to write this twice for issue that may result > - * in first write getting masked. > + /* > + * HW workaround, need to toggle enable bit off and on > + * for 12bpc with pixel repeat. > + * > + * FIXME: BSpec says this should be done at the end of > + * of the modeset sequence, so not sure if this isn't too soon. > */ > - if (HAS_PCH_SPLIT(dev)) { > + if (crtc->config->pipe_bpp > 24 && > + crtc->config->pixel_multiplier > 1) { > + I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE); > + POSTING_READ(intel_hdmi->hdmi_reg); > + > + /* > + * 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); > } > > - if (intel_crtc->config->has_audio) > + if (crtc->config->has_audio) > intel_enable_hdmi_audio(encoder); > } > > @@ -1509,7 +1535,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder > *encoder) > intel_crtc->config->has_hdmi_sink, > adjusted_mode); > > - intel_enable_hdmi(encoder); > + g4x_enable_hdmi(encoder); > > vlv_wait_port_ready(dev_priv, dport, 0x0); > } > @@ -1828,7 +1854,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder > *encoder) > > chv_powergate_phy_lanes(encoder, 0x0); > > - intel_enable_hdmi(encoder); > + g4x_enable_hdmi(encoder); > > vlv_wait_port_ready(dev_priv, dport, 0x0); > } > @@ -2012,8 +2038,10 @@ void intel_hdmi_init(struct drm_device *dev, int > hdmi_reg, enum port port) > intel_encoder->pre_enable = intel_hdmi_pre_enable; > if (HAS_PCH_CPT(dev)) > intel_encoder->enable = cpt_enable_hdmi; > + else if (HAS_PCH_IBX(dev)) > + intel_encoder->enable = ibx_enable_hdmi; > else > - intel_encoder->enable = intel_enable_hdmi; > + intel_encoder->enable = g4x_enable_hdmi; > } > > intel_encoder->type = INTEL_OUTPUT_HDMI; > -- > 2.0.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
