[PATCH 00/11] More HDMI fixes

[PATCH 00/11] More HDMI fixes

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi

This is my final patch series dealing with HDMI InfoFrames. It's on top of the
drm-intel-next-queued tree.

For now, everything seems to follow the specification from Gen 4 to HSW and I'm
happy with the code. The only thing missing is support for even more DIPs, but
these should not change the current functions: just add a few others.

One of the main additions of this series is the 'set_infoframes' function that
now completely resets the state of the DIP registers at every mode set. This
allowed us to implement the remaining fixes of the series in a proper way.

Patches that only change coding style and/or restructure code:
- 1, 3, 4, 10, 11
The most important patches, IMHO, are:
- 2, 5, 7, 8

I tested these patches on Gen4, ILK, SNB and IVB. Some were desktops, some
were laptops.

Thanks,
Paulo

Paulo Zanoni (11):
  drm/i915: add set_infoframes to struct intel_hdmi
  drm/i915: properly alternate between DVI and HDMI
  drm/i915: only set the HDMI port on the DIP once
  drm/i915: enable DIP before enabling each InfoFrame
  drm/i915: don't wait for vblank while writing InfoFrames
  drm/i915: explicitly disable the DIPs we're not using
  drm/i915: disable DIP while changing the port
  drm/i915: don't write 0 to DIP control at HDMI init
  drm/i915: don't set SDVO_BORDER_ENABLE when we're HDMI
  drm/i915: rename sdvox_reg to hdmi_reg on HDMI context
  drm/i915: remove comment about HSW HDMI DIPs

 drivers/gpu/drm/i915/i915_reg.h   |    6 +
 drivers/gpu/drm/i915/intel_ddi.c  |    3 +-
 drivers/gpu/drm/i915/intel_drv.h  |    6 +-
 drivers/gpu/drm/i915/intel_hdmi.c |  322 ++++++++++++++++++++++++++-----------
 4 files changed, 241 insertions(+), 96 deletions(-)

-- 
1.7.10

[PATCH 01/11] drm/i915: add set_infoframes to struct intel_hdmi

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We need a function that is able to fully 'set' the state of the DIP
registers to a known state.

Currently, we have the write_infoframe function that is called twice:
once for AVI and once for SPD. The problem is that write_infoframe
tries to keep the state of the DIP register as it is, changing only
the minimum necessary bits. The second problem is that
write_infoframe does twice (once for each time it is called) some
work that should be done only once (like waiting for vblank and
setting the port). If we add even more DIPs, it will do even more
repeated work.

This patch only adds the infrastructure keeping the code behavior the
same as before.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  |    3 +--
 drivers/gpu/drm/i915/intel_drv.h  |    2 ++
 drivers/gpu/drm/i915/intel_hdmi.c |   43 +++++++++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 46d1e88..f33fe1a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -726,8 +726,7 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
 
 	I915_WRITE(DDI_FUNC_CTL(pipe), temp);
 
-	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
-	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi->set_infoframes(encoder, adjusted_mode);
 }
 
 void intel_ddi_dpms(struct drm_encoder *encoder, int mode)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3e09188..0a1797a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -301,6 +301,8 @@ struct intel_hdmi {
 	enum hdmi_force_audio force_audio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				struct dip_infoframe *frame);
+	void (*set_infoframes)(struct drm_encoder *encoder,
+			       struct drm_display_mode *adjusted_mode);
 };
 
 static inline struct drm_crtc *
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 77f0f8f..0e7da09 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -345,6 +345,41 @@ void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
 	intel_set_infoframe(encoder, &spd_if);
 }
 
+void g4x_set_infoframes(struct drm_encoder *encoder,
+			struct drm_display_mode *adjusted_mode)
+{
+	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
+	intel_hdmi_set_spd_infoframe(encoder);
+}
+
+void ibx_set_infoframes(struct drm_encoder *encoder,
+			struct drm_display_mode *adjusted_mode)
+{
+	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
+	intel_hdmi_set_spd_infoframe(encoder);
+}
+
+void cpt_set_infoframes(struct drm_encoder *encoder,
+			struct drm_display_mode *adjusted_mode)
+{
+	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
+	intel_hdmi_set_spd_infoframe(encoder);
+}
+
+void vlv_set_infoframes(struct drm_encoder *encoder,
+			struct drm_display_mode *adjusted_mode)
+{
+	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
+	intel_hdmi_set_spd_infoframe(encoder);
+}
+
+void hsw_set_infoframes(struct drm_encoder *encoder,
+			struct drm_display_mode *adjusted_mode)
+{
+	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
+	intel_hdmi_set_spd_infoframe(encoder);
+}
+
 static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -388,8 +423,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
 	POSTING_READ(intel_hdmi->sdvox_reg);
 
-	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
-	intel_hdmi_set_spd_infoframe(encoder);
+	intel_hdmi->set_infoframes(encoder, adjusted_mode);
 }
 
 static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
@@ -737,9 +771,11 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 
 	if (!HAS_PCH_SPLIT(dev)) {
 		intel_hdmi->write_infoframe = g4x_write_infoframe;
+		intel_hdmi->set_infoframes = g4x_set_infoframes;
 		I915_WRITE(VIDEO_DIP_CTL, 0);
 	} else if (IS_VALLEYVIEW(dev)) {
 		intel_hdmi->write_infoframe = vlv_write_infoframe;
+		intel_hdmi->set_infoframes = vlv_set_infoframes;
 		for_each_pipe(i)
 			I915_WRITE(VLV_TVIDEO_DIP_CTL(i), 0);
 	} else if (IS_HASWELL(dev)) {
@@ -747,14 +783,17 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 		 * just doing the minimal required for HDMI to work at this stage.
 		 */
 		intel_hdmi->write_infoframe = hsw_write_infoframe;
+		intel_hdmi->set_infoframes = hsw_set_infoframes;
 		for_each_pipe(i)
 			I915_WRITE(HSW_TVIDEO_DIP_CTL(i), 0);
 	} else if (HAS_PCH_IBX(dev)) {
 		intel_hdmi->write_infoframe = ibx_write_infoframe;
+		intel_hdmi->set_infoframes = ibx_set_infoframes;
 		for_each_pipe(i)
 			I915_WRITE(TVIDEO_DIP_CTL(i), 0);
 	} else {
 		intel_hdmi->write_infoframe = cpt_write_infoframe;
+		intel_hdmi->set_infoframes = cpt_set_infoframes;
 		for_each_pipe(i)
 			I915_WRITE(TVIDEO_DIP_CTL(i), 0);
 	}
-- 
1.7.10

[PATCH 02/11] drm/i915: properly alternate between DVI and HDMI

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   85 +++++++++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0e7da09..ca30a68 100644
--- 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 @@ void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
 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 (!intel_hdmi->has_hdmi_sink) {
+		/* 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. */
+		if (!(val & VIDEO_DIP_ENABLE))
+			return;
+		val &= ~VIDEO_DIP_ENABLE;
+		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+		I915_WRITE(reg, val);
+		return;
+	}
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -355,6 +376,22 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 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);
+
+	if (!intel_hdmi->has_hdmi_sink) {
+		/* See the big comment in g4x_set_infoframes() */
+		if (!(val & VIDEO_DIP_ENABLE))
+			return;
+		val &= ~VIDEO_DIP_ENABLE;
+		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+		I915_WRITE(reg, val);
+		return;
+	}
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -362,6 +399,22 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 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);
+
+	if (!intel_hdmi->has_hdmi_sink) {
+		/* See the big comment in g4x_set_infoframes() */
+		if (!(val & VIDEO_DIP_ENABLE))
+			return;
+		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI);
+		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+		I915_WRITE(reg, val);
+		return;
+	}
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -369,6 +422,22 @@ void cpt_set_infoframes(struct drm_encoder *encoder,
 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);
+
+	if (!intel_hdmi->has_hdmi_sink) {
+		/* See the big comment in g4x_set_infoframes() */
+		if (!(val & VIDEO_DIP_ENABLE))
+			return;
+		val &= ~VIDEO_DIP_ENABLE;
+		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+		I915_WRITE(reg, val);
+		return;
+	}
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -376,6 +445,16 @@ void vlv_set_infoframes(struct drm_encoder *encoder,
 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);
 }
-- 
1.7.10

[PATCH 03/11] drm/i915: only set the HDMI port on the DIP once

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Not once for each InfoFrame. Now we have a function that allows us to
do this.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   56 ++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ca30a68..30f040b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -121,18 +121,9 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
 	uint32_t *data = (uint32_t *)frame;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	u32 val = I915_READ(VIDEO_DIP_CTL);
 	unsigned i, len = DIP_HEADER_SIZE + frame->len;
 
-	val &= ~VIDEO_DIP_PORT_MASK;
-	if (intel_hdmi->sdvox_reg == SDVOB)
-		val |= VIDEO_DIP_PORT_B;
-	else if (intel_hdmi->sdvox_reg == SDVOC)
-		val |= VIDEO_DIP_PORT_C;
-	else
-		return;
-
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
 	val |= g4x_infoframe_index(frame);
 
@@ -160,26 +151,10 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
 	unsigned i, len = DIP_HEADER_SIZE + frame->len;
 	u32 val = I915_READ(reg);
 
-	val &= ~VIDEO_DIP_PORT_MASK;
-	switch (intel_hdmi->sdvox_reg) {
-	case HDMIB:
-		val |= VIDEO_DIP_PORT_B;
-		break;
-	case HDMIC:
-		val |= VIDEO_DIP_PORT_C;
-		break;
-	case HDMID:
-		val |= VIDEO_DIP_PORT_D;
-		break;
-	default:
-		return;
-	}
-
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
@@ -369,6 +344,20 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	val &= ~VIDEO_DIP_PORT_MASK;
+	switch (intel_hdmi->sdvox_reg) {
+	case SDVOB:
+		val |= VIDEO_DIP_PORT_B;
+		break;
+	case SDVOC:
+		val |= VIDEO_DIP_PORT_C;
+		break;
+	default:
+		return;
+	}
+
+	I915_WRITE(reg, val);
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -392,6 +381,23 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	val &= ~VIDEO_DIP_PORT_MASK;
+	switch (intel_hdmi->sdvox_reg) {
+	case HDMIB:
+		val |= VIDEO_DIP_PORT_B;
+		break;
+	case HDMIC:
+		val |= VIDEO_DIP_PORT_C;
+		break;
+	case HDMID:
+		val |= VIDEO_DIP_PORT_D;
+		break;
+	default:
+		return;
+	}
+
+	I915_WRITE(reg, val);
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
-- 
1.7.10

[PATCH 04/11] drm/i915: enable DIP before enabling each InfoFrame

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

So the write_infoframe function can assume the DIP is on.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 30f040b..425e676 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -128,7 +128,6 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
 	val |= g4x_infoframe_index(frame);
 
 	val &= ~g4x_infoframe_enable(frame);
-	val |= VIDEO_DIP_ENABLE;
 
 	I915_WRITE(VIDEO_DIP_CTL, val);
 
@@ -161,7 +160,6 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
 	val |= g4x_infoframe_index(frame);
 
 	val &= ~g4x_infoframe_enable(frame);
-	val |= VIDEO_DIP_ENABLE;
 
 	I915_WRITE(reg, val);
 
@@ -195,13 +193,9 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
 
 	/* The DIP control register spec says that we need to update the AVI
 	 * infoframe without clearing its enable bit */
-	if (frame->type == DIP_TYPE_AVI)
-		val |= VIDEO_DIP_ENABLE_AVI;
-	else
+	if (frame->type != DIP_TYPE_AVI)
 		val &= ~g4x_infoframe_enable(frame);
 
-	val |= VIDEO_DIP_ENABLE;
-
 	I915_WRITE(reg, val);
 
 	for (i = 0; i < len; i += 4) {
@@ -233,7 +227,6 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
 	val |= g4x_infoframe_index(frame);
 
 	val &= ~g4x_infoframe_enable(frame);
-	val |= VIDEO_DIP_ENABLE;
 
 	I915_WRITE(reg, val);
 
@@ -356,6 +349,8 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	val |= VIDEO_DIP_ENABLE;
+
 	I915_WRITE(reg, val);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
@@ -396,6 +391,8 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	val |= VIDEO_DIP_ENABLE;
+
 	I915_WRITE(reg, val);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
@@ -421,6 +418,11 @@ void cpt_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	/* Set both together, unset both together: see the spec. */
+	val |= VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI;
+
+	I915_WRITE(reg, val);
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
@@ -444,6 +446,10 @@ void vlv_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
+	val |= VIDEO_DIP_ENABLE;
+
+	I915_WRITE(reg, val);
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
-- 
1.7.10

[PATCH 05/11] drm/i915: don't wait for vblank while writing InfoFrames

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This function is called when the pipe is disabled, so it always gets
the 50ms timeout.

This function is called once for each InfoFrame, so we actually get a
100ms timeout. Will be more if we add more InfoFrames.

Also, the spec says we need to "wait for a VSync to ensure completion
of any pending DIP transmissions", not for a VBlank. OTOH, the
register documentation suggests that the DIPs are sent *during* the
VSync, so shouldn't we be waiting until *after* the VSync to ensure
all DIPs are sent?

So this wait_for_vblank seems, besides useless, totally wrong.

If we ever want to change some specific InfoFrame on-the-fly (outside
of the modeset code), the code that changes the InfoFrame will have to
do the waiting itself, and properly.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 425e676..d84d45e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -154,8 +154,6 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
 	unsigned i, len = DIP_HEADER_SIZE + frame->len;
 	u32 val = I915_READ(reg);
 
-	intel_wait_for_vblank(dev, intel_crtc->pipe);
-
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
 	val |= g4x_infoframe_index(frame);
 
@@ -186,8 +184,6 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
 	unsigned i, len = DIP_HEADER_SIZE + frame->len;
 	u32 val = I915_READ(reg);
 
-	intel_wait_for_vblank(dev, intel_crtc->pipe);
-
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
 	val |= g4x_infoframe_index(frame);
 
@@ -221,8 +217,6 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
 	unsigned i, len = DIP_HEADER_SIZE + frame->len;
 	u32 val = I915_READ(reg);
 
-	intel_wait_for_vblank(dev, intel_crtc->pipe);
-
 	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
 	val |= g4x_infoframe_index(frame);
 
@@ -257,8 +251,6 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
 	if (data_reg == 0)
 		return;
 
-	intel_wait_for_vblank(dev, intel_crtc->pipe);
-
 	val &= ~hsw_infoframe_enable(frame);
 	I915_WRITE(ctl_reg, val);
 
-- 
1.7.10

[PATCH 06/11] drm/i915: explicitly disable the DIPs we're not using

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

>From this point on, the 'set_infoframe' functions always set the DIP
registers to a known state, so anything done will always be undone at
the modeset.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |    6 ++++++
 drivers/gpu/drm/i915/intel_hdmi.c |   13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0f45a18..1855ac7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1717,8 +1717,10 @@
 #define   VIDEO_DIP_PORT_C		(2 << 29)
 #define   VIDEO_DIP_PORT_D		(3 << 29)
 #define   VIDEO_DIP_PORT_MASK		(3 << 29)
+#define   VIDEO_DIP_ENABLE_GCP		(1 << 25)
 #define   VIDEO_DIP_ENABLE_AVI		(1 << 21)
 #define   VIDEO_DIP_ENABLE_VENDOR	(2 << 21)
+#define   VIDEO_DIP_ENABLE_GAMUT	(4 << 21)
 #define   VIDEO_DIP_ENABLE_SPD		(8 << 21)
 #define   VIDEO_DIP_SELECT_AVI		(0 << 19)
 #define   VIDEO_DIP_SELECT_VENDOR	(1 << 19)
@@ -1729,7 +1731,11 @@
 #define   VIDEO_DIP_FREQ_2VSYNC		(2 << 16)
 #define   VIDEO_DIP_FREQ_MASK		(3 << 16)
 /* HSW and later: */
+#define   VIDEO_DIP_ENABLE_VSC_HSW	(1 << 20)
+#define   VIDEO_DIP_ENABLE_GCP_HSW	(1 << 16)
 #define   VIDEO_DIP_ENABLE_AVI_HSW	(1 << 12)
+#define   VIDEO_DIP_ENABLE_VS_HSW	(1 << 8)
+#define   VIDEO_DIP_ENABLE_GMP_HSW	(1 << 4)
 #define   VIDEO_DIP_ENABLE_SPD_HSW	(1 << 0)
 
 /* Panel power sequencing */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d84d45e..ce68ec0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -342,6 +342,7 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 	}
 
 	val |= VIDEO_DIP_ENABLE;
+	val &= ~VIDEO_DIP_ENABLE_VENDOR;
 
 	I915_WRITE(reg, val);
 
@@ -384,6 +385,8 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 	}
 
 	val |= VIDEO_DIP_ENABLE;
+	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
+		 VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
 
@@ -412,6 +415,8 @@ void cpt_set_infoframes(struct drm_encoder *encoder,
 
 	/* Set both together, unset both together: see the spec. */
 	val |= VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI;
+	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
+		 VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
 
@@ -439,6 +444,8 @@ void vlv_set_infoframes(struct drm_encoder *encoder,
 	}
 
 	val |= VIDEO_DIP_ENABLE;
+	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
+		 VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
 
@@ -453,12 +460,18 @@ void hsw_set_infoframes(struct drm_encoder *encoder,
 	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);
+	u32 val = I915_READ(reg);
 
 	if (!intel_hdmi->has_hdmi_sink) {
 		I915_WRITE(reg, 0);
 		return;
 	}
 
+	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
+		 VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW);
+
+	I915_WRITE(reg, val);
+
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
 }
-- 
1.7.10

[PATCH 07/11] drm/i915: disable DIP while changing the port

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The register specification says we need to do this.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ce68ec0..5bcd609 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -309,6 +309,7 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	u32 reg = VIDEO_DIP_CTL;
 	u32 val = I915_READ(reg);
+	u32 port;
 
 	if (!intel_hdmi->has_hdmi_sink) {
 		/* If the registers were not initialized yet, they might be
@@ -329,18 +330,25 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
-	val &= ~VIDEO_DIP_PORT_MASK;
 	switch (intel_hdmi->sdvox_reg) {
 	case SDVOB:
-		val |= VIDEO_DIP_PORT_B;
+		port = VIDEO_DIP_PORT_B;
 		break;
 	case SDVOC:
-		val |= VIDEO_DIP_PORT_C;
+		port = VIDEO_DIP_PORT_C;
 		break;
 	default:
 		return;
 	}
 
+	if (port != (val & VIDEO_DIP_PORT_MASK)) {
+		val &= ~VIDEO_DIP_ENABLE;
+		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+		I915_WRITE(reg, val);
+		val &= ~VIDEO_DIP_PORT_MASK;
+		val |= port;
+	}
+
 	val |= VIDEO_DIP_ENABLE;
 	val &= ~VIDEO_DIP_ENABLE_VENDOR;
 
@@ -358,6 +366,7 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	u32 reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
 	u32 val = I915_READ(reg);
+	u32 port;
 
 	if (!intel_hdmi->has_hdmi_sink) {
 		/* See the big comment in g4x_set_infoframes() */
@@ -369,21 +378,28 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
-	val &= ~VIDEO_DIP_PORT_MASK;
 	switch (intel_hdmi->sdvox_reg) {
 	case HDMIB:
-		val |= VIDEO_DIP_PORT_B;
+		port = VIDEO_DIP_PORT_B;
 		break;
 	case HDMIC:
-		val |= VIDEO_DIP_PORT_C;
+		port = VIDEO_DIP_PORT_C;
 		break;
 	case HDMID:
-		val |= VIDEO_DIP_PORT_D;
+		port = VIDEO_DIP_PORT_D;
 		break;
 	default:
 		return;
 	}
 
+	if (port != (val & VIDEO_DIP_PORT_MASK)) {
+		val &= ~VIDEO_DIP_ENABLE;
+		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
+		I915_WRITE(reg, val);
+		val &= ~VIDEO_DIP_PORT_MASK;
+		val |= port;
+	}
+
 	val |= VIDEO_DIP_ENABLE;
 	val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT |
 		 VIDEO_DIP_ENABLE_GCP);
-- 
1.7.10

[PATCH 08/11] drm/i915: don't write 0 to DIP control at HDMI init

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

At this time, the HDMI port is enabled, and the DIP control register
specification says we need to disable the port *before* disabling the
DIPs. Also, while doing this we risk telling the HW to send the AVI
DIPs once (not every VSync), which really seems to confuse the HW and
trigger bugs where the DIPs are not sent.

This code was here just to set the DIP register to a 'known state'
before using it, but since now the set_infoframes functions already
set the control registers to a known state, this code can go away.

Also, the previous code disables *all* the DIP registers for *each*
HDMI port, so we end disabling each DIP register more than once.

This patch solves a problem I can reproduce on my IVB machine. When I
boot it with just a single HDMI monitor, the AVI InfoFrames are not
sent. With this patch, the InfoFrames are sent. Previously, I wrote a
patch to 'touch the DIP registers after we enable the HDMI port' to
solve this same problem, but that patch doesn't seem to be needed
anymore after this patch.

All this patch does is revert a chunk of the following commit:

    commit 64a8fc0145a1d0fdc25fc9367c2e6c621955fb3b
    Author: Jesse Barnes <jbarnes@virtuousgeek.org>
    Date:   Thu Sep 22 11:16:00 2011 +0530

        drm/i915: fix ILK+ infoframe support

So bugs that can be bisected to that commit may be fixed now.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43256
Cc: jbarnes@virtuousgeek.org
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5bcd609..1aa57bd 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -806,7 +806,6 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
 	struct intel_hdmi *intel_hdmi;
-	int i;
 
 	intel_hdmi = kzalloc(sizeof(struct intel_hdmi), GFP_KERNEL);
 	if (!intel_hdmi)
@@ -884,30 +883,21 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 	if (!HAS_PCH_SPLIT(dev)) {
 		intel_hdmi->write_infoframe = g4x_write_infoframe;
 		intel_hdmi->set_infoframes = g4x_set_infoframes;
-		I915_WRITE(VIDEO_DIP_CTL, 0);
 	} else if (IS_VALLEYVIEW(dev)) {
 		intel_hdmi->write_infoframe = vlv_write_infoframe;
 		intel_hdmi->set_infoframes = vlv_set_infoframes;
-		for_each_pipe(i)
-			I915_WRITE(VLV_TVIDEO_DIP_CTL(i), 0);
 	} else if (IS_HASWELL(dev)) {
 		/* FIXME: Haswell has a new set of DIP frame registers, but we are
 		 * just doing the minimal required for HDMI to work at this stage.
 		 */
 		intel_hdmi->write_infoframe = hsw_write_infoframe;
 		intel_hdmi->set_infoframes = hsw_set_infoframes;
-		for_each_pipe(i)
-			I915_WRITE(HSW_TVIDEO_DIP_CTL(i), 0);
 	} else if (HAS_PCH_IBX(dev)) {
 		intel_hdmi->write_infoframe = ibx_write_infoframe;
 		intel_hdmi->set_infoframes = ibx_set_infoframes;
-		for_each_pipe(i)
-			I915_WRITE(TVIDEO_DIP_CTL(i), 0);
 	} else {
 		intel_hdmi->write_infoframe = cpt_write_infoframe;
 		intel_hdmi->set_infoframes = cpt_set_infoframes;
-		for_each_pipe(i)
-			I915_WRITE(TVIDEO_DIP_CTL(i), 0);
 	}
 
 	if (IS_HASWELL(dev))
-- 
1.7.10

[PATCH 09/11] drm/i915: don't set SDVO_BORDER_ENABLE when we're HDMI

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The register that controls the HDMI port can be used to control the
sDVO port. Some bits are defined only for sDVO, and SDVO_BORDER_ENABLE
is one of those: HDMI ports that can't be used in sDVO mode don't even
have this bit defined in their specifications.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 1aa57bd..178e395 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -502,7 +502,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	u32 sdvox;
 
-	sdvox = SDVO_ENCODING_HDMI | SDVO_BORDER_ENABLE;
+	sdvox = SDVO_ENCODING_HDMI;
 	if (!HAS_PCH_SPLIT(dev))
 		sdvox |= intel_hdmi->color_range;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
-- 
1.7.10

[PATCH 10/11] drm/i915: rename sdvox_reg to hdmi_reg on HDMI context

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Some (but not all) of the HDMI registers can be used to control sDVO,
so these registers have two names. When we're talking about HDMI, we
really should call the HDMI control register "hdmi_reg" instead of
"sdvox_reg". So now "struct intel_hdmi" has a member called "hdmi_reg"
instead of "sdvox_reg".

And don't worry: "struct intel_sdvo" still has a member called
"sdvo_reg".

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |    4 +--
 drivers/gpu/drm/i915/intel_hdmi.c |   70 ++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0a1797a..9a18ca3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -292,7 +292,7 @@ struct dip_infoframe {
 
 struct intel_hdmi {
 	struct intel_encoder base;
-	u32 sdvox_reg;
+	u32 hdmi_reg;
 	int ddc_bus;
 	int ddi_port;
 	uint32_t color_range;
@@ -343,7 +343,7 @@ extern void intel_attach_force_audio_property(struct drm_connector *connector);
 extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 
 extern void intel_crt_init(struct drm_device *dev);
-extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg);
+extern void intel_hdmi_init(struct drm_device *dev, int hdmi_reg);
 extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 extern void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 			    struct drm_display_mode *adjusted_mode);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 178e395..3b0830a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -330,7 +330,7 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
-	switch (intel_hdmi->sdvox_reg) {
+	switch (intel_hdmi->hdmi_reg) {
 	case SDVOB:
 		port = VIDEO_DIP_PORT_B;
 		break;
@@ -378,7 +378,7 @@ void ibx_set_infoframes(struct drm_encoder *encoder,
 		return;
 	}
 
-	switch (intel_hdmi->sdvox_reg) {
+	switch (intel_hdmi->hdmi_reg) {
 	case HDMIB:
 		port = VIDEO_DIP_PORT_B;
 		break;
@@ -500,40 +500,40 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
-	u32 sdvox;
+	u32 hdmi_val;
 
-	sdvox = SDVO_ENCODING_HDMI;
+	hdmi_val = SDVO_ENCODING_HDMI;
 	if (!HAS_PCH_SPLIT(dev))
-		sdvox |= intel_hdmi->color_range;
+		hdmi_val |= intel_hdmi->color_range;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
-		sdvox |= SDVO_VSYNC_ACTIVE_HIGH;
+		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
-		sdvox |= SDVO_HSYNC_ACTIVE_HIGH;
+		hdmi_val |= SDVO_HSYNC_ACTIVE_HIGH;
 
 	if (intel_crtc->bpp > 24)
-		sdvox |= COLOR_FORMAT_12bpc;
+		hdmi_val |= COLOR_FORMAT_12bpc;
 	else
-		sdvox |= COLOR_FORMAT_8bpc;
+		hdmi_val |= COLOR_FORMAT_8bpc;
 
 	/* Required on CPT */
 	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
-		sdvox |= HDMI_MODE_SELECT;
+		hdmi_val |= HDMI_MODE_SELECT;
 
 	if (intel_hdmi->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
 				 pipe_name(intel_crtc->pipe));
-		sdvox |= SDVO_AUDIO_ENABLE;
-		sdvox |= SDVO_NULL_PACKETS_DURING_VSYNC;
+		hdmi_val |= SDVO_AUDIO_ENABLE;
+		hdmi_val |= SDVO_NULL_PACKETS_DURING_VSYNC;
 		intel_write_eld(encoder, adjusted_mode);
 	}
 
 	if (HAS_PCH_CPT(dev))
-		sdvox |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
+		hdmi_val |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
 	else if (intel_crtc->pipe == 1)
-		sdvox |= SDVO_PIPE_B_SELECT;
+		hdmi_val |= SDVO_PIPE_B_SELECT;
 
-	I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
-	POSTING_READ(intel_hdmi->sdvox_reg);
+	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
+	POSTING_READ(intel_hdmi->hdmi_reg);
 
 	intel_hdmi->set_infoframes(encoder, adjusted_mode);
 }
@@ -549,14 +549,14 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
 	if (intel_hdmi->has_audio)
 		enable_bits |= SDVO_AUDIO_ENABLE;
 
-	temp = I915_READ(intel_hdmi->sdvox_reg);
+	temp = I915_READ(intel_hdmi->hdmi_reg);
 
 	/* 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->sdvox_reg, temp & ~SDVO_ENABLE);
-		POSTING_READ(intel_hdmi->sdvox_reg);
+		I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
+		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
 
 	if (mode != DRM_MODE_DPMS_ON) {
@@ -565,15 +565,15 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
 		temp |= enable_bits;
 	}
 
-	I915_WRITE(intel_hdmi->sdvox_reg, temp);
-	POSTING_READ(intel_hdmi->sdvox_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.
 	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(intel_hdmi->sdvox_reg, temp);
-		POSTING_READ(intel_hdmi->sdvox_reg);
+		I915_WRITE(intel_hdmi->hdmi_reg, temp);
+		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
 }
 
@@ -604,7 +604,7 @@ static bool g4x_hdmi_connected(struct intel_hdmi *intel_hdmi)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t bit;
 
-	switch (intel_hdmi->sdvox_reg) {
+	switch (intel_hdmi->hdmi_reg) {
 	case HDMIB:
 		bit = HDMIB_HOTPLUG_LIVE_STATUS;
 		break;
@@ -799,7 +799,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	intel_attach_broadcast_rgb_property(connector);
 }
 
-void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
+void intel_hdmi_init(struct drm_device *dev, int hdmi_reg)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_connector *connector;
@@ -834,51 +834,51 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 
 	/* Set up the DDC bus. */
-	if (sdvox_reg == SDVOB) {
+	if (hdmi_reg == SDVOB) {
 		intel_encoder->clone_mask = (1 << INTEL_HDMIB_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
 		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == SDVOC) {
+	} else if (hdmi_reg == SDVOC) {
 		intel_encoder->clone_mask = (1 << INTEL_HDMIC_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
 		dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == HDMIB) {
+	} else if (hdmi_reg == HDMIB) {
 		intel_encoder->clone_mask = (1 << INTEL_HDMID_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
 		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == HDMIC) {
+	} else if (hdmi_reg == HDMIC) {
 		intel_encoder->clone_mask = (1 << INTEL_HDMIE_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
 		dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == HDMID) {
+	} else if (hdmi_reg == HDMID) {
 		intel_encoder->clone_mask = (1 << INTEL_HDMIF_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
 		dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == DDI_BUF_CTL(PORT_B)) {
+	} else if (hdmi_reg == DDI_BUF_CTL(PORT_B)) {
 		DRM_DEBUG_DRIVER("LPT: detected output on DDI B\n");
 		intel_encoder->clone_mask = (1 << INTEL_HDMIB_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
 		intel_hdmi->ddi_port = PORT_B;
 		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == DDI_BUF_CTL(PORT_C)) {
+	} else if (hdmi_reg == DDI_BUF_CTL(PORT_C)) {
 		DRM_DEBUG_DRIVER("LPT: detected output on DDI C\n");
 		intel_encoder->clone_mask = (1 << INTEL_HDMIC_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
 		intel_hdmi->ddi_port = PORT_C;
 		dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == DDI_BUF_CTL(PORT_D)) {
+	} else if (hdmi_reg == DDI_BUF_CTL(PORT_D)) {
 		DRM_DEBUG_DRIVER("LPT: detected output on DDI D\n");
 		intel_encoder->clone_mask = (1 << INTEL_HDMID_CLONE_BIT);
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
 		intel_hdmi->ddi_port = PORT_D;
 		dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
 	} else {
-		/* If we got an unknown sdvox_reg, things are pretty much broken
+		/* If we got an unknown hdmi_reg, things are pretty much broken
 		 * in a way that we should let the kernel know about it */
 		BUG();
 	}
 
-	intel_hdmi->sdvox_reg = sdvox_reg;
+	intel_hdmi->hdmi_reg = hdmi_reg;
 
 	if (!HAS_PCH_SPLIT(dev)) {
 		intel_hdmi->write_infoframe = g4x_write_infoframe;
-- 
1.7.10

[PATCH 11/11] drm/i915: remove comment about HSW HDMI DIPs

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

HSW support is now just like all the other generations: we send AVI
and SPD InfoFrames. There are other DIPs for HSW, but there are other
DIPs for the previous generations too. For each gen, we can see which
DIPs are missing by looking at the 'set_infoframes' function: we
explicitly disable the DIPs we're not using.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 3b0830a..9ec09a2 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -887,9 +887,6 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg)
 		intel_hdmi->write_infoframe = vlv_write_infoframe;
 		intel_hdmi->set_infoframes = vlv_set_infoframes;
 	} else if (IS_HASWELL(dev)) {
-		/* FIXME: Haswell has a new set of DIP frame registers, but we are
-		 * just doing the minimal required for HDMI to work at this stage.
-		 */
 		intel_hdmi->write_infoframe = hsw_write_infoframe;
 		intel_hdmi->set_infoframes = hsw_set_infoframes;
 	} else if (HAS_PCH_IBX(dev)) {
-- 
1.7.10

Re: [PATCH 08/11] drm/i915: don't write 0 to DIP control at HDMI init

[Jesse Barnes]

On Thu, 24 May 2012 17:49:50 -0300
Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> At this time, the HDMI port is enabled, and the DIP control register
> specification says we need to disable the port *before* disabling the
> DIPs. Also, while doing this we risk telling the HW to send the AVI
> DIPs once (not every VSync), which really seems to confuse the HW and
> trigger bugs where the DIPs are not sent.
> 
> This code was here just to set the DIP register to a 'known state'
> before using it, but since now the set_infoframes functions already
> set the control registers to a known state, this code can go away.
> 
> Also, the previous code disables *all* the DIP registers for *each*
> HDMI port, so we end disabling each DIP register more than once.
> 
> This patch solves a problem I can reproduce on my IVB machine. When I
> boot it with just a single HDMI monitor, the AVI InfoFrames are not
> sent. With this patch, the InfoFrames are sent. Previously, I wrote a
> patch to 'touch the DIP registers after we enable the HDMI port' to
> solve this same problem, but that patch doesn't seem to be needed
> anymore after this patch.
> 
> All this patch does is revert a chunk of the following commit:
> 
>     commit 64a8fc0145a1d0fdc25fc9367c2e6c621955fb3b
>     Author: Jesse Barnes <jbarnes@virtuousgeek.org>
>     Date:   Thu Sep 22 11:16:00 2011 +0530
> 
>         drm/i915: fix ILK+ infoframe support
> 
> So bugs that can be bisected to that commit may be fixed now.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43256
> Cc: jbarnes@virtuousgeek.org
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Thanks for tracking down the right disable and enable order on all the
generations Paulo! \o/

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 01/11] drm/i915: add set_infoframes to struct intel_hdmi

[Chris Wilson]

On Thu, 24 May 2012 17:49:43 -0300, Paulo Zanoni <przanoni@gmail.com> wrote:
> +void g4x_set_infoframes(struct drm_encoder *encoder,
> +			struct drm_display_mode *adjusted_mode)

This and all that follow should be static. Adding a non-intel function
to our module's namespace is indicative of a gross hack, or as in this
case, a mistake.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 07/11] drm/i915: disable DIP while changing the port

[Chris Wilson]

On Thu, 24 May 2012 17:49:49 -0300, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> The register specification says we need to do this.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c |   30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ce68ec0..5bcd609 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -309,6 +309,7 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>  	u32 reg = VIDEO_DIP_CTL;
>  	u32 val = I915_READ(reg);
> +	u32 port;
>  
>  	if (!intel_hdmi->has_hdmi_sink) {
>  		/* If the registers were not initialized yet, they might be
> @@ -329,18 +330,25 @@ void g4x_set_infoframes(struct drm_encoder *encoder,
>  		return;
>  	}
>  
> -	val &= ~VIDEO_DIP_PORT_MASK;
>  	switch (intel_hdmi->sdvox_reg) {
>  	case SDVOB:
> -		val |= VIDEO_DIP_PORT_B;
> +		port = VIDEO_DIP_PORT_B;
>  		break;
>  	case SDVOC:
> -		val |= VIDEO_DIP_PORT_C;
> +		port = VIDEO_DIP_PORT_C;
>  		break;
>  	default:
>  		return;
>  	}
>  
> +	if (port != (val & VIDEO_DIP_PORT_MASK)) {
> +		val &= ~VIDEO_DIP_ENABLE;
> +		val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC;
> +		I915_WRITE(reg, val);

When does this write latch? (i.e. do we need to wait upon any event or
timeout?) At the very least you probably want to flush the write (with a
 POSTING_READ) before continuing.

> +		val &= ~VIDEO_DIP_PORT_MASK;
> +		val |= port;
> +	}
> +
>  	val |= VIDEO_DIP_ENABLE;
>  	val &= ~VIDEO_DIP_ENABLE_VENDOR;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre