Patchwork [3.5.y.z,extended,stable] Patch "drm/i915: make sure we write all the DIP data bytes" has been added to staging queue

login
register
mail settings
Submitter Herton Ronaldo Krzesinski
Date Jan. 14, 2013, 8:57 p.m.
Message ID <1358197062-17400-1-git-send-email-herton.krzesinski@canonical.com>
Download mbox | patch
Permalink /patch/211913/
State New
Headers show

Comments

Herton Ronaldo Krzesinski - Jan. 14, 2013, 8:57 p.m.
This is a note to let you know that I have just added a patch titled

    drm/i915: make sure we write all the DIP data bytes

to the linux-3.5.y-queue branch of the 3.5.y.z extended stable tree 
which can be found at:

 http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.5.y-queue

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.5.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Herton

------

From 5f8759726307930592618252f37e42b0e0205f26 Mon Sep 17 00:00:00 2001
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date: Tue, 25 Sep 2012 13:23:34 -0300
Subject: [PATCH] drm/i915: make sure we write all the DIP data bytes

commit adf00b26d18e1b3570451296e03bcb20e4798cdd upstream.

... even if the actual infoframe is smaller than the maximum possible
size.

If we don't write all the 32 DIP data bytes the InfoFrame ECC may not
be correctly calculated in some cases (e.g., when changing the port),
and this will lead to black screens on HDMI monitors. The ECC value is
generated by the hardware.

I don't see how this should break anything since we're writing 0 and
that should be the correct value, so this patch should be safe.

Notice that on IVB and older we actually have 64 bytes available for
VIDEO_DIP_DATA, but only bytes 0-31 actually store infoframe data: the
others are either read-only ECC values or marked as "reserved". On HSW
we only have 32 bytes, and the ECC value is stored on its own separate
read-only register. See BSpec.

This patch fixes bug #46761, which is marked as a regression
introduced by commit 4e89ee174bb2da341bf90a84321c7008a3c9210d:
    drm/i915: set the DIP port on ibx_write_infoframe

Before commit 4e89 we were just failing to send AVI infoframes when we
needed to change the port, which can lead to black screens in some
cases. After commit 4e89 we started sending infoframes, but with a
possibly wrong ECC value. After this patch I hope we start sending
correct infoframes.

Version 2:
  - Improve commit message
  - Try to make the code more clear

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46761
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |    4 ++++
 drivers/gpu/drm/i915/intel_hdmi.c |   15 +++++++++++++++
 2 files changed, 19 insertions(+)

--
1.7.9.5

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 19398df..8ccfe28 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1707,6 +1707,10 @@ 

 /* Video Data Island Packet control */
 #define VIDEO_DIP_DATA		0x61178
+/* Read the description of VIDEO_DIP_DATA (before Haswel) or VIDEO_DIP_ECC
+ * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
+ * of the infoframe structure specified by CEA-861. */
+#define   VIDEO_DIP_DATA_SIZE	32
 #define VIDEO_DIP_CTL		0x61170
 /* Pre HSW: */
 #define   VIDEO_DIP_ENABLE		(1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a0194e5..3bf9d16 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -146,6 +146,9 @@  static void g4x_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(VIDEO_DIP_DATA, *data);
 		data++;
 	}
+	/* Write every possible data byte to force correct ECC calculation. */
+	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+		I915_WRITE(VIDEO_DIP_DATA, 0);
 	mmiowb();

 	val |= g4x_infoframe_enable(frame);
@@ -198,6 +201,9 @@  static void ibx_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
+	/* Write every possible data byte to force correct ECC calculation. */
+	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 	mmiowb();

 	val |= g4x_infoframe_enable(frame);
@@ -240,6 +246,9 @@  static void cpt_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
+	/* Write every possible data byte to force correct ECC calculation. */
+	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 	mmiowb();

 	val |= g4x_infoframe_enable(frame);
@@ -276,6 +285,9 @@  static void vlv_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
+	/* Write every possible data byte to force correct ECC calculation. */
+	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
 	mmiowb();

 	val |= g4x_infoframe_enable(frame);
@@ -311,6 +323,9 @@  static void hsw_write_infoframe(struct drm_encoder *encoder,
 		I915_WRITE(data_reg + i, *data);
 		data++;
 	}
+	/* Write every possible data byte to force correct ECC calculation. */
+	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+		I915_WRITE(data_reg + i, 0);
 	mmiowb();

 	val |= hsw_infoframe_enable(frame);