[PATCH 0/4] Attempt to re-enable FBC on gen3

[PATCH 0/4] Attempt to re-enable FBC on gen3

[Adam Jackson]

Disclaimer: I haven't tested this extensively, it just seems logical, and I
really hate to see us lose FBC on gen3 since that's the family being used in
low-wattage devices.  Wider testing would be greatly appreciated.

The only thing I really don't like about this is how we hit every CRTC on
resume trying to enable FBC.  But that's sort of a problem in the normal case
anyway.  On pre-gen4, we can only compress plane A, so we hardwire that to
pipe B since LVDS is limited to pipe B.  Ideally, you'd like to compress
whichever CRTC has more pixels, assuming they're both mostly static. It's
really awkward to do that in the current one-at-a-time CRTC setup kind of
world though.

On gen4 and later we can compress either plane, so it's a little messier;
we'll compress whichever CRTC gets set up last, before suspend, but then after
suspend we'll compress the highest-numbered CRTC.  Either way, when multiple
CRTCs are active, we're already sometimes compressing a suboptimal plane, so
making more ways for that to happen isn't a huge deal.

(Then, of course, you'd like to switch which pipe you compress to the one with
the more static image, if they have different update rates.  But that's way
into diminishing returns territory.)

- ajax

[PATCH 1/4] drm/i915: Make fbc control wrapper functions

[Adam Jackson]

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |   13 +------
 drivers/gpu/drm/i915/i915_dma.c      |    3 +-
 drivers/gpu/drm/i915/i915_drv.h      |    5 ++-
 drivers/gpu/drm/i915/intel_display.c |   66 ++++++++++++++++++++++-----------
 4 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bb3a4a8..897d8c3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -566,23 +566,14 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
-	struct drm_crtc *crtc;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	bool fbc_enabled = false;
 
-	if (!dev_priv->display.fbc_enabled) {
+	if (!I915_HAS_FBC(dev)) {
 		seq_printf(m, "FBC unsupported on this chipset\n");
 		return 0;
 	}
 
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		if (!crtc->enabled)
-			continue;
-		if (dev_priv->display.fbc_enabled(crtc))
-			fbc_enabled = true;
-	}
-
-	if (fbc_enabled) {
+	if (intel_fbc_enabled(dev)) {
 		seq_printf(m, "FBC enabled\n");
 	} else {
 		seq_printf(m, "FBC disabled: ");
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 75248be..664cd66 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1356,11 +1356,10 @@ static void i915_setup_compression(struct drm_device *dev, int size)
 
 	dev_priv->cfb_size = size;
 
+	intel_disable_fbc(dev);
 	if (IS_GM45(dev)) {
-		g4x_disable_fbc(dev);
 		I915_WRITE(DPFC_CB_BASE, compressed_fb->start);
 	} else {
-		i8xx_disable_fbc(dev);
 		I915_WRITE(FBC_CFB_BASE, cfb_base);
 		I915_WRITE(FBC_LL_BASE, ll_base);
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 790fef3..3a19cb4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -175,7 +175,7 @@ struct drm_i915_error_state {
 
 struct drm_i915_display_funcs {
 	void (*dpms)(struct drm_crtc *crtc, int mode);
-	bool (*fbc_enabled)(struct drm_crtc *crtc);
+	bool (*fbc_enabled)(struct drm_device *dev);
 	void (*enable_fbc)(struct drm_crtc *crtc, unsigned long interval);
 	void (*disable_fbc)(struct drm_device *dev);
 	int (*get_display_clock_speed)(struct drm_device *dev);
@@ -999,6 +999,9 @@ extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void i8xx_disable_fbc(struct drm_device *dev);
 extern void g4x_disable_fbc(struct drm_device *dev);
+extern void intel_disable_fbc(struct drm_device *dev);
+extern void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval);
+extern bool intel_fbc_enabled(struct drm_device *dev);
 
 extern void intel_detect_pch (struct drm_device *dev);
 extern int intel_trans_dp_port_sel (struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 58668c4..9f375ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1047,9 +1047,8 @@ void i8xx_disable_fbc(struct drm_device *dev)
 	DRM_DEBUG_KMS("disabled FBC\n");
 }
 
-static bool i8xx_fbc_enabled(struct drm_crtc *crtc)
+static bool i8xx_fbc_enabled(struct drm_device *dev)
 {
-	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	return I915_READ(FBC_CONTROL) & FBC_CTL_EN;
@@ -1106,14 +1105,43 @@ void g4x_disable_fbc(struct drm_device *dev)
 	DRM_DEBUG_KMS("disabled FBC\n");
 }
 
-static bool g4x_fbc_enabled(struct drm_crtc *crtc)
+static bool g4x_fbc_enabled(struct drm_device *dev)
 {
-	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
+bool intel_fbc_enabled(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!dev_priv->display.fbc_enabled)
+		return false;
+
+	return dev_priv->display.fbc_enabled(dev);
+}
+
+void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+{
+	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+
+	if (!dev_priv->display.enable_fbc)
+		return;
+
+	dev_priv->display.enable_fbc(crtc, interval);
+}
+
+void intel_disable_fbc(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!dev_priv->display.disable_fbc)
+		return;
+
+	dev_priv->display.disable_fbc(dev);
+}
+
 /**
  * intel_update_fbc - enable/disable FBC as needed
  * @crtc: CRTC to point the compressor at
@@ -1148,9 +1176,7 @@ static void intel_update_fbc(struct drm_crtc *crtc,
 	if (!i915_powersave)
 		return;
 
-	if (!dev_priv->display.fbc_enabled ||
-	    !dev_priv->display.enable_fbc ||
-	    !dev_priv->display.disable_fbc)
+	if (!I915_HAS_FBC(dev))
 		return;
 
 	if (!crtc->fb)
@@ -1197,28 +1223,25 @@ static void intel_update_fbc(struct drm_crtc *crtc,
 		goto out_disable;
 	}
 
-	if (dev_priv->display.fbc_enabled(crtc)) {
+	if (intel_fbc_enabled(dev)) {
 		/* We can re-enable it in this case, but need to update pitch */
-		if (fb->pitch > dev_priv->cfb_pitch)
-			dev_priv->display.disable_fbc(dev);
-		if (obj_priv->fence_reg != dev_priv->cfb_fence)
-			dev_priv->display.disable_fbc(dev);
-		if (plane != dev_priv->cfb_plane)
-			dev_priv->display.disable_fbc(dev);
+		if ((fb->pitch > dev_priv->cfb_pitch) ||
+		    (obj_priv->fence_reg != dev_priv->cfb_fence) ||
+		    (plane != dev_priv->cfb_plane))
+			intel_disable_fbc(dev);
 	}
 
-	if (!dev_priv->display.fbc_enabled(crtc)) {
-		/* Now try to turn it back on if possible */
-		dev_priv->display.enable_fbc(crtc, 500);
-	}
+	/* Now try to turn it back on if possible */
+	if (!intel_fbc_enabled(dev))
+		intel_enable_fbc(crtc, 500);
 
 	return;
 
 out_disable:
 	DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
 	/* Multiple disables should be harmless */
-	if (dev_priv->display.fbc_enabled(crtc))
-		dev_priv->display.disable_fbc(dev);
+	if (intel_fbc_enabled(dev))
+		intel_disable_fbc(dev);
 }
 
 static int
@@ -5209,8 +5232,7 @@ static void intel_init_display(struct drm_device *dev)
 	else
 		dev_priv->display.dpms = i9xx_crtc_dpms;
 
-	/* Only mobile has FBC, leave pointers NULL for other chips */
-	if (IS_MOBILE(dev)) {
+	if (I915_HAS_FBC(dev)) {
 		if (IS_GM45(dev)) {
 			dev_priv->display.fbc_enabled = g4x_fbc_enabled;
 			dev_priv->display.enable_fbc = g4x_enable_fbc;
-- 
1.7.0.1

[PATCH 2/4] drm/i915: Push an fbc disable across suspend

[Adam Jackson]

Restore it on the way back up if we disabled it on the way down.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.c     |   13 +++++++++++++
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_suspend.c |    2 --
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 01e91ea..412bade 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -233,6 +233,9 @@ static int i915_drm_freeze(struct drm_device *dev)
 		drm_irq_uninstall(dev);
 	}
 
+	dev_priv->fbc_disabled_for_suspend = intel_fbc_enabled(dev);
+	intel_disable_fbc(dev);
+
 	i915_save_state(dev);
 
 	intel_opregion_free(dev, 1);
@@ -276,6 +279,16 @@ static int i915_drm_thaw(struct drm_device *dev)
 
 	i915_restore_state(dev);
 
+	if (dev_priv->fbc_disabled_for_suspend) {
+		struct drm_crtc *crtc;
+
+		/* XXX might not be the plane we compressed at suspend; meh */
+		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+			intel_enable_fbc(crtc, 500);
+
+		dev_priv->fbc_disabled_for_suspend = false;
+	}
+
 	intel_opregion_init(dev, 1);
 
 	/* KMS EnterVT equivalent */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3a19cb4..e9e8c4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -341,6 +341,7 @@ typedef struct drm_i915_private {
 
 	/* Register state */
 	bool modeset_on_lid;
+	bool fbc_disabled_for_suspend;
 	u8 saveLBB;
 	u32 saveDSPACNTR;
 	u32 saveDSPBCNTR;
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 60a5800..81cdfb4 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -707,10 +707,8 @@ void i915_restore_display(struct drm_device *dev)
 	/* only restore FBC info on the platform that supports FBC*/
 	if (I915_HAS_FBC(dev)) {
 		if (IS_GM45(dev)) {
-			g4x_disable_fbc(dev);
 			I915_WRITE(DPFC_CB_BASE, dev_priv->saveDPFC_CB_BASE);
 		} else {
-			i8xx_disable_fbc(dev);
 			I915_WRITE(FBC_CFB_BASE, dev_priv->saveFBC_CFB_BASE);
 			I915_WRITE(FBC_LL_BASE, dev_priv->saveFBC_LL_BASE);
 			I915_WRITE(FBC_CONTROL2, dev_priv->saveFBC_CONTROL2);
-- 
1.7.0.1

[PATCH 3/4] drm/i915: Hide per-chipset fbc disable routines again

[Adam Jackson]

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 --
 drivers/gpu/drm/i915/intel_display.c |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e9e8c4a..a43a4f5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -998,8 +998,6 @@ static inline void opregion_enable_asle(struct drm_device *dev) { return; }
 extern void intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
-extern void i8xx_disable_fbc(struct drm_device *dev);
-extern void g4x_disable_fbc(struct drm_device *dev);
 extern void intel_disable_fbc(struct drm_device *dev);
 extern void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval);
 extern bool intel_fbc_enabled(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f375ce..7dafa4c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1025,7 +1025,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		  dev_priv->cfb_pitch, crtc->y, dev_priv->cfb_plane);
 }
 
-void i8xx_disable_fbc(struct drm_device *dev)
+static void i8xx_disable_fbc(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 fbc_ctl;
@@ -1091,7 +1091,7 @@ static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
 }
 
-void g4x_disable_fbc(struct drm_device *dev)
+static void g4x_disable_fbc(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpfc_ctl;
-- 
1.7.0.1

[PATCH 4/4] Revert "drm/i915: Disable FBC on 915GM and 945GM."

[Adam Jackson]

Disabling it across suspend cycles should be equivalent, without losing
the power savings.

This reverts commit 8d06a1e1e9c69244f08beb7d17146483f9dcd120.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 412bade..f03640d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -80,14 +80,14 @@ const static struct intel_device_info intel_i915g_info = {
 	.is_i915g = 1, .is_i9xx = 1, .cursor_needs_physical = 1,
 };
 const static struct intel_device_info intel_i915gm_info = {
-	.is_i9xx = 1,  .is_mobile = 1,
+	.is_i9xx = 1,  .is_mobile = 1, .has_fbc = 1,
 	.cursor_needs_physical = 1,
 };
 const static struct intel_device_info intel_i945g_info = {
 	.is_i9xx = 1, .has_hotplug = 1, .cursor_needs_physical = 1,
 };
 const static struct intel_device_info intel_i945gm_info = {
-	.is_i945gm = 1, .is_i9xx = 1, .is_mobile = 1,
+	.is_i945gm = 1, .is_i9xx = 1, .is_mobile = 1, .has_fbc = 1,
 	.has_hotplug = 1, .cursor_needs_physical = 1,
 };
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7dafa4c..38f18c7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5237,7 +5237,7 @@ static void intel_init_display(struct drm_device *dev)
 			dev_priv->display.fbc_enabled = g4x_fbc_enabled;
 			dev_priv->display.enable_fbc = g4x_enable_fbc;
 			dev_priv->display.disable_fbc = g4x_disable_fbc;
-		} else if (IS_I965GM(dev)) {
+		} else if (IS_I965GM(dev) || IS_I945GM(dev) || IS_I915GM(dev)) {
 			dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
 			dev_priv->display.enable_fbc = i8xx_enable_fbc;
 			dev_priv->display.disable_fbc = i8xx_disable_fbc;
-- 
1.7.0.1

Re: [PATCH 0/4] Attempt to re-enable FBC on gen3

[Alexander Lam]

On Fri, Apr 23, 2010 at 11:17 AM, Adam Jackson <ajax@redhat.com> wrote:
> Disclaimer: I haven't tested this extensively, it just seems logical, and I
> really hate to see us lose FBC on gen3 since that's the family being used in
> low-wattage devices.  Wider testing would be greatly appreciated.

Seems to work here on a 945GSE (Acer Aspire One) and save a bit of
power (like .2W)

> The only thing I really don't like about this is how we hit every CRTC on
> resume trying to enable FBC.  But that's sort of a problem in the normal case
> anyway.  On pre-gen4, we can only compress plane A, so we hardwire that to
> pipe B since LVDS is limited to pipe B.  Ideally, you'd like to compress
> whichever CRTC has more pixels, assuming they're both mostly static. It's
> really awkward to do that in the current one-at-a-time CRTC setup kind of
> world though.
>
> On gen4 and later we can compress either plane, so it's a little messier;
> we'll compress whichever CRTC gets set up last, before suspend, but then after
> suspend we'll compress the highest-numbered CRTC.  Either way, when multiple
> CRTCs are active, we're already sometimes compressing a suboptimal plane, so
> making more ways for that to happen isn't a huge deal.
>
> (Then, of course, you'd like to switch which pipe you compress to the one with
> the more static image, if they have different update rates.  But that's way
> into diminishing returns territory.)
>
> - ajax


-- 
Alexander Lam

Re: [PATCH 0/4] Attempt to re-enable FBC on gen3

[Tobias Doerffel]

[-- Attachment #1.1: Type: Text/Plain, Size: 422 bytes --]

Hi,

Am Freitag, 23. April 2010, um 17:17:38 schrieb Adam Jackson:
> Disclaimer: I haven't tested this extensively, it just seems logical, and I
> really hate to see us lose FBC on gen3 since that's the family being used
> in low-wattage devices.  Wider testing would be greatly appreciated.
Works reliable here (Samsung NC10) for all common use cases (including 
external VGA connected and disconnected).

Toby

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Push an fbc disable across suspend

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 379 bytes --]

On Fri, 23 Apr 2010 11:17:40 -0400, Adam Jackson <ajax@redhat.com> wrote:
> Restore it on the way back up if we disabled it on the way down.

I don't think we need to restore it on the way back up.  The
force_mode_set will DPMS the CRTCs which should turn FBC back on, right?

Given that, I wonder if we shouldn't move the FBC register save/restore
to the non-KMS path entirely.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Push an fbc disable across suspend

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 690 bytes --]

On Thu, 2010-04-29 at 15:45 -0700, Eric Anholt wrote:
> On Fri, 23 Apr 2010 11:17:40 -0400, Adam Jackson <ajax@redhat.com> wrote:
> > Restore it on the way back up if we disabled it on the way down.
> 
> I don't think we need to restore it on the way back up.  The
> force_mode_set will DPMS the CRTCs which should turn FBC back on, right?
> 
> Given that, I wonder if we shouldn't move the FBC register save/restore
> to the non-KMS path entirely.

Sounds right to me.

I'm still a little cautious about enabling gen3 fbc again, Jesse made it
sound like there were stability problems with it outside of suspend.
Maybe i915.powersave=2 to enable unstable features?

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] Attempt to re-enable FBC on gen3

[Jesse Barnes]

On Thu, 29 Apr 2010 10:23:58 +0200
Tobias Doerffel <tobias.doerffel@gmail.com> wrote:

> Hi,
> 
> Am Freitag, 23. April 2010, um 17:17:38 schrieb Adam Jackson:
> > Disclaimer: I haven't tested this extensively, it just seems
> > logical, and I really hate to see us lose FBC on gen3 since that's
> > the family being used in low-wattage devices.  Wider testing would
> > be greatly appreciated.
> Works reliable here (Samsung NC10) for all common use cases
> (including external VGA connected and disconnected).

If we can get confirmation that this fixes things from others that were
seeing FBC related problems, I'd be ok with including it.  But until
then it should be hidden under a boot param or something.

Jesse

Re: [PATCH 2/4] drm/i915: Push an fbc disable across suspend

[Daniel Vetter]

On Fri, Apr 30, 2010 at 10:33:22AM -0400, Adam Jackson wrote:
> I'm still a little cautious about enabling gen3 fbc again, Jesse made it
> sound like there were stability problems with it outside of suspend.
> Maybe i915.powersave=2 to enable unstable features?

Just for the record, I can shoot down my i945 with fbc enabled with a few
secs of cairo-perf-trace. With or without your patch-series and no
suspend/resume required. Box works flawless with powersave=0. So with the
current code, fbc on gen 3 should definitely be optional, perhaps even
tainting the kernel (so that random gpu hangs suddenly look rather less
random in bug reports).

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48