drm/i915: properly alternate between DVI and HDMI

This solves problems that happen when you alternate between HDMI and
DVI on the same port. I can reproduce these problems using DP->HDMI
and DP->DVI adapters on a DP port.

When you first plug HDMI and then plug DVI, you need to stop sending
DIPs, even if the port is in DVI mode (see the HDMI register spec). If
you don't stop sending DIPs, you'll see a pink vertical line on the
left side of the screen, some modes will give you a black screen, some
modes won't work correctly.

When you first plug DVI and then plug HDMI, you need to properly
enable the DIPs, otherwise the HW won't send them. After spending a
lot of time investigating this, I concluded that if the DIPs are
disabled, we should not write to the DIP register again because when
we do this, we also set the AVI InfoFrame frequency to "once", and
this seems to really confuse our hardware. Since this problem was not
exactly easy to debug, I'm adopting the defensive behavior and not
just avoing the "disable twice" sequence, but also explicitly
selecting the AVI InfoFrame and setting its frequency to a correct
one.

Also, move the "is_dvi" check from intel_set_infoframe to the
set_infoframes functions since now they're going to be the first ones
to deal with the DIP registers.

This patch adds the code to fix the problem, but it depends on the
removal of some code that can't be removed right now and will come
later in the patch series. The patch that we need is:
  - drm/i915: don't write 0 to DIP control at HDMI init

[danvet: Paulo clarified that this additional patch is only required
to make the fix complete, this patch here alone doesn't introduce a
regression but only partially solves the problem of randomly clearing
the dip registers.]

V2: Be even more defensive by selecting AVI and setting its frequency
outside the "is_dvi" check.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8d892b0cd8a1185ff2bb22f672c90601e7d15d83..2f2adc4e511c355e7f1fcfbda20f8bd1630dc873 100644 (file)
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -308,9 +308,6 @@ static void intel_set_infoframe(struct drm_encoder *encoder,
 {
        struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
-       if (!intel_hdmi->has_hdmi_sink)
-               return;
-
        intel_dip_infoframe_csum(frame);
        intel_hdmi->write_infoframe(encoder, frame);
 }
@@ -348,6 +345,30 @@ static void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
 static void g4x_set_infoframes(struct drm_encoder *encoder,
                               struct drm_display_mode *adjusted_mode)
 {
+       struct drm_i915_private *dev_priv = encoder->dev->dev_private;
+       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+       u32 reg = VIDEO_DIP_CTL;
+       u32 val = I915_READ(reg);
+
+       /* If the registers were not initialized yet, they might be zeroes,
+        * which means we're selecting the AVI DIP and we're setting its
+        * frequency to once. This seems to really confuse the HW and make
+        * things stop working (the register spec says the AVI always needs to
+        * be sent every VSync). So here we avoid writing to the register more
+        * than we need and also explicitly select the AVI DIP and explicitly
+        * set its frequency to every VSync. Avoiding to write it twice seems to
+        * be enough to solve the problem, but being defensive shouldn't hurt us
+        * either. */
+       val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+
+       if (!intel_hdmi->has_hdmi_sink) {
+               if (!(val & VIDEO_DIP_ENABLE))
+                       return;
+               val &= ~VIDEO_DIP_ENABLE;
+               I915_WRITE(reg, val);
+               return;
+       }
+
        intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
        intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -355,6 +376,23 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
 static void ibx_set_infoframes(struct drm_encoder *encoder,
                               struct drm_display_mode *adjusted_mode)
 {
+       struct drm_i915_private *dev_priv = encoder->dev->dev_private;
+       struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+       u32 reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+       u32 val = I915_READ(reg);
+
+       /* See the big comment in g4x_set_infoframes() */
+       val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+
+       if (!intel_hdmi->has_hdmi_sink) {
+               if (!(val & VIDEO_DIP_ENABLE))
+                       return;
+               val &= ~VIDEO_DIP_ENABLE;
+               I915_WRITE(reg, val);
+               return;
+       }
+
        intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
        intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -362,6 +400,23 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
 static void cpt_set_infoframes(struct drm_encoder *encoder,
                               struct drm_display_mode *adjusted_mode)
 {
+       struct drm_i915_private *dev_priv = encoder->dev->dev_private;
+       struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+       u32 reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+       u32 val = I915_READ(reg);
+
+       /* See the big comment in g4x_set_infoframes() */
+       val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+
+       if (!intel_hdmi->has_hdmi_sink) {
+               if (!(val & VIDEO_DIP_ENABLE))
+                       return;
+               val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI);
+               I915_WRITE(reg, val);
+               return;
+       }
+
        intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
        intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -369,6 +424,23 @@ static void cpt_set_infoframes(struct drm_encoder *encoder,
 static void vlv_set_infoframes(struct drm_encoder *encoder,
                               struct drm_display_mode *adjusted_mode)
 {
+       struct drm_i915_private *dev_priv = encoder->dev->dev_private;
+       struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+       u32 reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+       u32 val = I915_READ(reg);
+
+       /* See the big comment in g4x_set_infoframes() */
+       val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+
+       if (!intel_hdmi->has_hdmi_sink) {
+               if (!(val & VIDEO_DIP_ENABLE))
+                       return;
+               val &= ~VIDEO_DIP_ENABLE;
+               I915_WRITE(reg, val);
+               return;
+       }
+
        intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
        intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -376,6 +448,16 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
 static void hsw_set_infoframes(struct drm_encoder *encoder,
                               struct drm_display_mode *adjusted_mode)
 {
+       struct drm_i915_private *dev_priv = encoder->dev->dev_private;
+       struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+       u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
+
+       if (!intel_hdmi->has_hdmi_sink) {
+               I915_WRITE(reg, 0);
+               return;
+       }
+
        intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
        intel_hdmi_set_spd_infoframe(encoder);
 }