Re: 2.6.32 regression (bisected): Video tearing/glitching with T400 laptops

[Jesse Barnes <jbarnes@virtuousgeek.org>]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On Tue, 13 Oct 2009 12:14:26 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> According to the chipset team that form of display corruption is
> likely related to RAM self-refresh... Sounds like the display plane
> isn't getting its memory requests serviced fast enough when in
> self-refresh mode, which might mean we have to program the
> self-refresh watermarks more aggressively on GM45.

Ok, like any good bug there was more than one thing wrong:
  - we weren't setting up a fence for the object before enabling FBC.
    Chris caught this and posted a patch to
    intel-gfx@lists.freedesktop.org titled "drm/i915: Install a fence
    register for fbc on g4x" (attached for convenience)
  - turns out we *do* need to set watermarks on G4x, despite some hw
    documentation indicating otherwise, patch for that attached

Hopefully with these two you'll have a solid display and some power
saving!

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: i915-g4x-watermarks-hack.patch --]
[-- Type: text/x-patch, Size: 1645 bytes --]

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a0f6bbe..ed11591 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1047,7 +1047,7 @@ static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	dev_priv->cfb_fence = obj_priv->fence_reg;
 	dev_priv->cfb_plane = intel_crtc->plane;
 
-	dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
+	dpfc_ctl = plane | DPFC_CTL_LIMIT_1X;
 	if (obj_priv->tiling_mode != I915_TILING_NONE) {
 		dpfc_ctl |= DPFC_CTL_FENCE_EN | dev_priv->cfb_fence;
 		I915_WRITE(DPFC_CHICKEN, DPFC_HT_MODIFY);
@@ -1055,14 +1055,13 @@ static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		I915_WRITE(DPFC_CHICKEN, ~DPFC_HT_MODIFY);
 	}
 
-	I915_WRITE(DPFC_CONTROL, dpfc_ctl);
 	I915_WRITE(DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
 		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
 		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
 
 	/* enable it... */
-	I915_WRITE(DPFC_CONTROL, I915_READ(DPFC_CONTROL) | DPFC_CTL_EN);
+	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
 	DRM_DEBUG("enabled fbc on plane %d\n", intel_crtc->plane);
 }
@@ -2443,6 +2442,10 @@ static void g4x_update_wm(struct drm_device *dev, int unused, int unused2,
 	else
 		fw_blc_self &= ~FW_BLC_SELF_EN;
 	I915_WRITE(FW_BLC_SELF, fw_blc_self);
+
+	I915_WRITE(DSPFW1, (64 << 23) | (32 << 16) | (32 << 8) | (32 << 0));
+	I915_WRITE(DSPFW2, (32 << 8) | (32 << 0));
+	I915_WRITE(DSPFW3, 32 << 24);
 }
 
 static void i965_update_wm(struct drm_device *dev, int unused, int unused2,

[-- Attachment #3: i915-install-fence.patch --]
[-- Type: text/x-patch, Size: 3015 bytes --]

Received: from virtuous by box514.bluehost.com with local-bsmtp (Exim 4.69)
	(envelope-from <chris@chris-wilson.co.uk>)
	id 1My9I5-000222-1X
	for jbarnes@virtuousgeek.org; Wed, 14 Oct 2009 13:13:41 -0600
X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on box514.bluehost.com
X-Spam-Level: 
X-Spam-Status: No, score=-6.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED
	shortcircuit=no autolearn=ham version=3.2.5
Received: from mga05.intel.com ([192.55.52.89] helo=fmsmga101.fm.intel.com)
	by box514.bluehost.com with esmtp (Exim 4.69)
	(envelope-from <chris@chris-wilson.co.uk>)
	id 1My9I4-0001zV-9H
	for jbarnes@virtuousgeek.org; Wed, 14 Oct 2009 13:13:40 -0600
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
  by fmsmga101.fm.intel.com with ESMTP; 14 Oct 2009 12:07:40 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="4.44,560,1249282800"; 
   d="scan'208";a="736384260"
Received: from unknown (HELO localhost.localdomain) ([10.255.17.79])
  by fmsmga001.fm.intel.com with ESMTP; 14 Oct 2009 12:16:28 -0700
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>, Jesse Barnes <jbarnes@virtuousgeek.org>
Subject: [PATCH] drm/i915: Install a fence register for fbc on g4x
Date: Wed, 14 Oct 2009 20:12:46 +0100
Message-Id: <1255547566-9426-1-git-send-email-chris@chris-wilson.co.uk>
X-Mailer: git-send-email 1.6.4.3
X-Identified-User: {10642:box514.bluehost.com:virtuous:virtuousgeek.org} {sentby:program running on server}

To enable framebuffer compression on a g4x, we not only need the buffer
to tiled (X only), we also need to hold a fence register for the buffer.
Currently we only install a fence register for pre-i965s when setting up
the scanout buffer. Rather than adding some convoluted logic to
g4x_enable_fbc() to acquire a fence register, and perhaps to
g4x_disable_fbc() to release it again, we can extend the acquisition
during setup to all chipsets.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1a40b9a..9dfb82f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1262,9 +1262,11 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	/* Pre-i965 needs to install a fence for tiled scan-out */
-	if (!IS_I965G(dev) &&
-	    obj_priv->fence_reg == I915_FENCE_REG_NONE &&
+	/* Install a fence for tiled scan-out. Pre-i965 always needs a fence,
+	 * whereas 965+ only requires a fence if using framebuffer compression.
+	 * For simplicity, we always install a fence as the cost is not that onerous.
+	 */
+	if (obj_priv->fence_reg == I915_FENCE_REG_NONE &&
 	    obj_priv->tiling_mode != I915_TILING_NONE) {
 		ret = i915_gem_object_get_fence_reg(obj);
 		if (ret != 0) {
-- 
1.6.4.3