All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH 13/14] drm/i915: add some barriers when changing DIPs
Date: Mon, 28 May 2012 16:43:00 -0300	[thread overview]
Message-ID: <1338234182-5021-14-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1338234182-5021-1-git-send-email-przanoni@gmail.com>

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

On IVB and older, we basically have two registers: the control and the
data register. We write a few consecutitve times to the control
register, and we need these writes to arrive exactly in the specified
order.

Also, when we're changing the data register, we need to guarantee that
anything written to the control register already arrived (since
changing the control register can change where the data register
points to). Also, we need to make sure all the writes to the data
register happen exactly in the specified order, and we also *can't*
read the data register during this process, since reading and/or
writing it will change the place it points to.

So invoke the "better safe than sorry" rule and just be careful and
put barriers everywhere :)

On HSW we still have a control register that we write many times, but
we have many data registers.

Demanded-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d0a466a..f61af21 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -133,16 +133,19 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
 
 	I915_WRITE(VIDEO_DIP_CTL, val);
 
+	mmiowb();
 	for (i = 0; i < len; i += 4) {
 		I915_WRITE(VIDEO_DIP_DATA, *data);
 		data++;
 	}
+	mmiowb();
 
 	val |= g4x_infoframe_enable(frame);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
 	I915_WRITE(VIDEO_DIP_CTL, val);
+	POSTING_READ(VIDEO_DIP_CTL);
 }
 
 static void ibx_write_infoframe(struct drm_encoder *encoder,
@@ -165,16 +168,19 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
 
 	I915_WRITE(reg, val);
 
+	mmiowb();
 	for (i = 0; i < len; i += 4) {
 		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
+	mmiowb();
 
 	val |= g4x_infoframe_enable(frame);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 }
 
 static void cpt_write_infoframe(struct drm_encoder *encoder,
@@ -200,16 +206,19 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
 
 	I915_WRITE(reg, val);
 
+	mmiowb();
 	for (i = 0; i < len; i += 4) {
 		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
+	mmiowb();
 
 	val |= g4x_infoframe_enable(frame);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 }
 
 static void vlv_write_infoframe(struct drm_encoder *encoder,
@@ -232,16 +241,19 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
 
 	I915_WRITE(reg, val);
 
+	mmiowb();
 	for (i = 0; i < len; i += 4) {
 		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
 		data++;
 	}
+	mmiowb();
 
 	val |= g4x_infoframe_enable(frame);
 	val &= ~VIDEO_DIP_FREQ_MASK;
 	val |= VIDEO_DIP_FREQ_VSYNC;
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 }
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
@@ -262,13 +274,16 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
 	val &= ~hsw_infoframe_enable(frame);
 	I915_WRITE(ctl_reg, val);
 
+	mmiowb();
 	for (i = 0; i < len; i += 4) {
 		I915_WRITE(data_reg + i, *data);
 		data++;
 	}
+	mmiowb();
 
 	val |= hsw_infoframe_enable(frame);
 	I915_WRITE(ctl_reg, val);
+	POSTING_READ(ctl_reg);
 }
 
 static void intel_set_infoframe(struct drm_encoder *encoder,
@@ -335,6 +350,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
 			return;
 		val &= ~VIDEO_DIP_ENABLE;
 		I915_WRITE(reg, val);
+		POSTING_READ(reg);
 		return;
 	}
 
@@ -353,6 +369,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
 		if (val & VIDEO_DIP_ENABLE) {
 			val &= ~VIDEO_DIP_ENABLE;
 			I915_WRITE(reg, val);
+			POSTING_READ(reg);
 		}
 		val &= ~VIDEO_DIP_PORT_MASK;
 		val |= port;
@@ -362,6 +379,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
 	val &= ~VIDEO_DIP_ENABLE_VENDOR;
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
@@ -385,6 +403,7 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
 			return;
 		val &= ~VIDEO_DIP_ENABLE;
 		I915_WRITE(reg, val);
+		POSTING_READ(reg);
 		return;
 	}
 
@@ -406,6 +425,7 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
 		if (val & VIDEO_DIP_ENABLE) {
 			val &= ~VIDEO_DIP_ENABLE;
 			I915_WRITE(reg, val);
+			POSTING_READ(reg);
 		}
 		val &= ~VIDEO_DIP_PORT_MASK;
 		val |= port;
@@ -416,6 +436,7 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
 		 VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
@@ -438,6 +459,7 @@ static void cpt_set_infoframes(struct drm_encoder *encoder,
 			return;
 		val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI);
 		I915_WRITE(reg, val);
+		POSTING_READ(reg);
 		return;
 	}
 
@@ -447,6 +469,7 @@ static void cpt_set_infoframes(struct drm_encoder *encoder,
 		 VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
@@ -469,6 +492,7 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
 			return;
 		val &= ~VIDEO_DIP_ENABLE;
 		I915_WRITE(reg, val);
+		POSTING_READ(reg);
 		return;
 	}
 
@@ -477,6 +501,7 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
 		 VIDEO_DIP_ENABLE_GCP);
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
@@ -493,6 +518,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
 
 	if (!intel_hdmi->has_hdmi_sink) {
 		I915_WRITE(reg, 0);
+		POSTING_READ(reg);
 		return;
 	}
 
@@ -500,6 +526,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
 		 VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW);
 
 	I915_WRITE(reg, val);
+	POSTING_READ(reg);
 
 	intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
 	intel_hdmi_set_spd_infoframe(encoder);
-- 
1.7.10

  parent reply	other threads:[~2012-05-28 19:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-28 19:42 [PATCH 00/14] More HDMI fixes V2 Paulo Zanoni
2012-05-28 19:42 ` [PATCH 01/14] drm/i915: add set_infoframes to struct intel_hdmi Paulo Zanoni
2012-05-28 19:42 ` [PATCH 02/14] drm/i915: properly alternate between DVI and HDMI Paulo Zanoni
2012-05-28 19:53   ` Chris Wilson
2012-05-28 20:04     ` Paulo Zanoni
2012-05-28 19:42 ` [PATCH 03/14] drm/i915: only set the HDMI port on the DIP once Paulo Zanoni
2012-05-28 19:42 ` [PATCH 04/14] drm/i915: enable DIP before enabling each InfoFrame Paulo Zanoni
2012-05-28 19:42 ` [PATCH 05/14] drm/i915: don't wait for vblank while writing InfoFrames Paulo Zanoni
2012-05-28 19:42 ` [PATCH 06/14] drm/i915: explicitly disable the DIPs we're not using Paulo Zanoni
2012-05-28 19:42 ` [PATCH 07/14] drm/i915: disable DIP while changing the port Paulo Zanoni
2012-05-28 19:42 ` [PATCH 08/14] drm/i915: don't write 0 to DIP control at HDMI init Paulo Zanoni
2012-05-28 19:42 ` [PATCH 09/14] drm/i915: don't set SDVO_BORDER_ENABLE when we're HDMI Paulo Zanoni
2012-05-28 19:42 ` [PATCH 10/14] drm/i915: rename sdvox_reg to hdmi_reg on HDMI context Paulo Zanoni
2012-05-28 19:42 ` [PATCH 11/14] drm/i915: remove comment about HSW HDMI DIPs Paulo Zanoni
2012-05-28 19:42 ` [PATCH 12/14] drm/i915: clarify confusion between HDMI and SDVO registers Paulo Zanoni
2012-05-28 19:43 ` Paulo Zanoni [this message]
2012-05-30 21:08   ` [PATCH 13/14] drm/i915: add some barriers when changing DIPs Daniel Vetter
2012-05-28 19:43 ` [PATCH 14/14] drm/i915: make sure HDMI port is disabled inside set_infoframes Paulo Zanoni
2012-05-30 12:15   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1338234182-5021-14-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.