[PATCH 0/2] drm/i915: enable CxSR and FBC on Sandybridge

[PATCH 0/2] drm/i915: enable CxSR and FBC on Sandybridge

[Yuanhan Liu]

Hi, 

This two patches enable self refresh and frame buffer compression
on Sandybridge.

FYI, with my test, enable CxSR would save about 0.7W, while enable
FBC would save about 0.1~0.2W.

Thanks.
Yuanhan Liu

--
Yuanhan Liu (2):
  drm/i915: Add self-refresh support on Sandybridge
  drm/i915: Add frame buffer compression on Sandybridge

 drivers/gpu/drm/i915/i915_debugfs.c  |    2 +-
 drivers/gpu/drm/i915/i915_dma.c      |    4 +-
 drivers/gpu/drm/i915/i915_drv.c      |    1 +
 drivers/gpu/drm/i915/i915_reg.h      |   58 ++++++
 drivers/gpu/drm/i915/intel_display.c |  315 +++++++++++++++++++++++++++++++++-
 5 files changed, 376 insertions(+), 4 deletions(-)

-- 
1.7.2.3

[PATCH 1/2] drm/i915: Add self-refresh support on Sandybridge

[Yuanhan Liu]

Add the support of memory self-refresh on Sandybridge, which is now
support 3 levels of watermarks and the source of the latency values
for watermarks has changed.

On Sandybridge, the LP0 WM value is not hardcoded any more. All the
latency value is now should be extracted from MCHBAR SSKPD register.
And the MCHBAR base address is changed, too.

For the WM values, if any calculated watermark values is larger than
the maximum value that can be programmed into the associated watermark
register, that watermark must be disabled.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |    2 +-
 drivers/gpu/drm/i915/i915_reg.h      |   46 +++++
 drivers/gpu/drm/i915/intel_display.c |  307 ++++++++++++++++++++++++++++++++++
 3 files changed, 354 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2282199..864e75d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -933,7 +933,7 @@ static int i915_sr_status(struct seq_file *m, void *unused)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	bool sr_enabled = false;
 
-	if (IS_GEN5(dev))
+	if (HAS_PCH_SPLIT(dev))
 		sr_enabled = I915_READ(WM1_LP_ILK) & WM1_LP_SR_EN;
 	else if (IS_CRESTLINE(dev) || IS_I945G(dev) || IS_I945GM(dev))
 		sr_enabled = I915_READ(FW_BLC_SELF) & FW_BLC_SELF_EN;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b284c13..5bba63a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -596,6 +596,8 @@
 
 #define ILK_DISPLAY_CHICKEN1	0x42000
 #define   ILK_FBCQ_DIS		(1<<22)
+#define   ILK_PABSTRETCH_DIS 	(1<<21)
+
 
 /*
  * GPIO regs
@@ -955,6 +957,8 @@
  */
 #define MCHBAR_MIRROR_BASE	0x10000
 
+#define MCHBAR_MIRROR_BASE_SNB	0x140000
+
 /** 915-945 and GM965 MCH register controlling DRAM channel access */
 #define DCC			0x10200
 #define DCC_ADDRESSING_MODE_SINGLE_CHANNEL		(0 << 0)
@@ -2346,6 +2350,46 @@
 
 #define ILK_FIFO_LINE_SIZE	64
 
+
+/* define the WM info on Sandybridge */ 
+#define SNB_DISPLAY_FIFO	128
+#define SNB_DISPLAY_MAXWM	0x7f	/* bit 16:22 */
+#define SNB_DISPLAY_DFTWM	8
+#define SNB_CURSOR_FIFO		32
+#define SNB_CURSOR_MAXWM	0x1f	/* bit 4:0 */
+#define SNB_CURSOR_DFTWM	8
+
+#define SNB_DISPLAY_SR_FIFO	512
+#define SNB_DISPLAY_MAX_SRWM	0x1ff	/* bit 16:8 */
+#define SNB_DISPLAY_DFT_SRWM	0x3f
+#define SNB_CURSOR_SR_FIFO	64
+#define SNB_CURSOR_MAX_SRWM	0x3f	/* bit 5:0 */
+#define SNB_CURSOR_DFT_SRWM	8
+
+#define SNB_FBC_MAX_SRWM	0xf	/* bit 23:20 */
+
+#define SNB_FIFO_LINE_SIZE	64
+
+
+/* the address where we get all kinds of latency value */
+#define SSKPD			0x5d10
+#define SSKPD_WM0_MASK		0x3f
+#define SSKPD_WM0_SHIFT		0
+#define SSKPD_WM1_MASK		0x3f00
+#define SSKPD_WM1_SHIFT		8
+#define SSKPD_WM2_MASK		0x3f0000
+#define SSKPD_WM2_SHIFT		16
+#define SSKPD_WM3_MASK		0x3f000000
+#define SSKPD_WM3_SHIFT		24
+
+#define SNB_LATENCY(mask,shift)	((I915_READ(MCHBAR_MIRROR_BASE_SNB + SSKPD) &  	\
+				 mask) >> shift)
+#define SNB_WM0_LATENCY		(SNB_LATENCY(SSKPD_WM0_MASK, SSKPD_WM0_SHIFT))
+#define SNB_WM1_LATENCY		(SNB_LATENCY(SSKPD_WM1_MASK, SSKPD_WM1_SHIFT))
+#define SNB_WM2_LATENCY		(SNB_LATENCY(SSKPD_WM2_MASK, SSKPD_WM2_SHIFT))
+#define SNB_WM3_LATENCY		(SNB_LATENCY(SSKPD_WM3_MASK, SSKPD_WM3_SHIFT))
+
+
 /*
  * The two pipe frame counter registers are not synchronized, so
  * reading a stable value is somewhat tricky. The following code
@@ -2651,6 +2695,8 @@
 #define  ILK_VSDPFD_FULL	(1<<21)
 #define ILK_DSPCLK_GATE		0x42020
 #define  ILK_DPARB_CLK_GATE	(1<<5)
+#define  ILK_DPFD_CLK_GATE	(1<<7)
+
 /* According to spec this bit 7/8/9 of 0x42020 should be set to enable FBC */
 #define   ILK_CLK_FBC		(1<<7)
 #define   ILK_DPFC_DIS1		(1<<8)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 17c213f..59a6d89 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2845,6 +2845,39 @@ static struct intel_watermark_params ironlake_cursor_srwm_info = {
 	ILK_FIFO_LINE_SIZE
 };
 
+static struct intel_watermark_params sandybridge_display_wm_info = {
+	SNB_DISPLAY_FIFO,
+	SNB_DISPLAY_MAXWM,
+	SNB_DISPLAY_DFTWM,
+	2,
+	SNB_FIFO_LINE_SIZE
+};
+
+static struct intel_watermark_params sandybridge_cursor_wm_info = {
+	SNB_CURSOR_FIFO,
+	SNB_CURSOR_MAXWM,
+	SNB_CURSOR_DFTWM,
+	2,
+	SNB_FIFO_LINE_SIZE
+};
+
+static struct intel_watermark_params sandybridge_display_srwm_info = {
+	SNB_DISPLAY_SR_FIFO,
+	SNB_DISPLAY_MAX_SRWM,
+	SNB_DISPLAY_DFT_SRWM,
+	2,
+	SNB_FIFO_LINE_SIZE
+};
+
+static struct intel_watermark_params sandybridge_cursor_srwm_info = {
+	SNB_CURSOR_SR_FIFO,
+	SNB_CURSOR_MAX_SRWM,
+	SNB_CURSOR_DFT_SRWM,
+	2,
+	SNB_FIFO_LINE_SIZE
+};
+
+
 /**
  * intel_calculate_wm - calculate watermark level
  * @clock_in_khz: pixel clock
@@ -3500,6 +3533,243 @@ static void ironlake_update_wm(struct drm_device *dev,
 	/* XXX setup WM2 and WM3 */
 }
 
+static bool sandybridge_compute_wm0(struct drm_device *dev,
+				 int pipe,
+				 int *plane_wm,
+				 int *cursor_wm)
+{
+	struct drm_crtc *crtc;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int htotal, hdisplay, clock, pixel_size = 0;
+	int line_time_us, line_count, entries;
+
+	crtc = intel_get_crtc_for_pipe(dev, pipe);
+	if (crtc->fb == NULL || !crtc->enabled)
+		return false;
+
+	htotal = crtc->mode.htotal;
+	hdisplay = crtc->mode.hdisplay;
+	clock = crtc->mode.clock;
+	pixel_size = crtc->fb->bits_per_pixel / 8;
+
+	/* Use the small buffer method to calculate plane watermark */
+	entries = ((clock * pixel_size / 1000) * SNB_WM0_LATENCY * 100) / 1000;
+	entries = DIV_ROUND_UP(entries,
+			       sandybridge_display_wm_info.cacheline_size);
+	*plane_wm = entries + sandybridge_display_wm_info.guard_size;
+	if (*plane_wm > (int)sandybridge_display_wm_info.max_wm)
+		*plane_wm = sandybridge_display_wm_info.max_wm;
+
+	/* Use the large buffer method to calculate cursor watermark */
+	line_time_us = ((htotal * 1000) / clock);
+	line_count = (SNB_WM0_LATENCY * 100 / line_time_us + 1000) / 1000;
+	entries = line_count * 64 * pixel_size;
+	entries = DIV_ROUND_UP(entries,
+			       sandybridge_cursor_wm_info.cacheline_size);
+	*cursor_wm = entries + sandybridge_cursor_wm_info.guard_size;
+	if (*cursor_wm > sandybridge_cursor_wm_info.max_wm)
+		*cursor_wm = sandybridge_cursor_wm_info.max_wm;
+
+	return true;
+}
+
+/* 
+ * compute watermark values of WM[1-3], SNB support 3 levels of watermark
+ */
+static void sandybridge_compute_srwm(int hdisplay, int htotal, int pixel_size, 
+				     int clock, int latency_ns, int *fbc_wm, 
+				     int *display_wm, int *cursor_wm)
+{
+
+	unsigned long line_time_us;
+	int small, large;
+	int entries;
+	int line_count, line_size;
+
+	if (!latency_ns) {
+		*fbc_wm = *display_wm = *cursor_wm = 0;
+		return;
+	}	
+
+	line_time_us = (htotal * 1000) / clock;
+	line_count = (latency_ns / line_time_us + 1000) / 1000;
+	line_size = hdisplay * pixel_size;
+
+	/* Use the minimum of the small and large buffer method for primary */
+	small = ((clock * pixel_size / 1000) * latency_ns) / 1000;
+	large = line_count * line_size;
+
+	entries = DIV_ROUND_UP(min(small, large), 
+				sandybridge_display_srwm_info.cacheline_size);
+	*display_wm = entries + sandybridge_display_srwm_info.guard_size;
+
+	/* 
+	 * Spec said: 
+	 * FBC WM = ((Final Primary WM * 64) / number of bytes per line) + 2
+	 */
+	*fbc_wm = DIV_ROUND_UP(*display_wm * 64, line_size) + 2;
+
+	/* calculate the self-refresh watermark for display cursor */
+	entries = line_count * pixel_size * 64;
+	entries = DIV_ROUND_UP(entries, 
+				sandybridge_cursor_srwm_info.cacheline_size);
+	*cursor_wm = entries + sandybridge_cursor_srwm_info.guard_size;
+}
+
+/*
+ * Check the wm result.
+ *
+ * If any calculated watermark values is larger than the maximum value that
+ * can be programmed into the associated watermark register, that watermark 
+ * must be disabled.
+ *
+ * Also return true if all of those watermark values is 0, which is set by 
+ * sandybridge_compute_srwm, to indicate the latency is ZERO.
+ */
+static bool sandybridge_check_srwm(struct drm_device *dev, int fbc_wm, 
+				   int display_wm, int cursor_wm, int level)
+{
+	int err = false;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	DRM_DEBUG_KMS("watermark %d: display plane %d, fbc lines %d,"
+		      " cursor %d\n", level, display_wm, fbc_wm, cursor_wm);
+
+	if (fbc_wm > SNB_FBC_MAX_SRWM) {
+		err = true;
+		DRM_DEBUG_KMS("fbc watermark(%d) is too large(%d)\n", 
+				fbc_wm, SNB_FBC_MAX_SRWM);
+
+		/* fbc has it's way to disable FBC WM */
+		I915_WRITE(DISP_ARB_CTL, (I915_READ(DISP_ARB_CTL) |
+					  DISP_FBC_WM_DIS));
+
+	}
+	if (display_wm > SNB_DISPLAY_MAX_SRWM) {
+		err = true;
+		DRM_DEBUG_KMS("display watermark(%d) is too large(%d)\n",
+				display_wm, SNB_DISPLAY_MAX_SRWM);
+	}
+	if (cursor_wm > SNB_CURSOR_MAX_SRWM) {
+		err = true;
+		DRM_DEBUG_KMS("cursor watermark(%d) is too large(%d)\n",
+				cursor_wm, SNB_CURSOR_MAX_SRWM);
+	}
+
+	if (!(fbc_wm || display_wm || cursor_wm)) {
+		err = true;
+		DRM_DEBUG_KMS("latency %d is 0\n", level);
+	}
+
+	if (err)
+		DRM_DEBUG_KMS("disable watermark %d\n", level);
+
+	return err;
+}
+
+static void sandybridge_update_wm(struct drm_device *dev,
+			       int planea_clock, int planeb_clock,
+			       int hdisplay, int htotal,
+			       int pixel_size)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int fbc_wm, plane_wm, cursor_wm, enabled;
+	int clock;
+	int tmp;
+
+	enabled = 0;
+	if (sandybridge_compute_wm0(dev, 0, &plane_wm, &cursor_wm)) {
+		I915_WRITE(WM0_PIPEA_ILK,
+			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
+			      " plane %d, " "cursor: %d\n",
+			      plane_wm, cursor_wm);
+		enabled++;
+	}
+
+	if (sandybridge_compute_wm0(dev, 1, &plane_wm, &cursor_wm)) {
+		I915_WRITE(WM0_PIPEB_ILK,
+			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
+			      " plane %d, cursor: %d\n",
+			      plane_wm, cursor_wm);
+		enabled++;
+	}
+
+	/*
+	 * Calculate and update the self-refresh watermark only when one
+	 * display plane is used.
+	 *
+	 * WM1/WM2/WM2 watermarks have to be enabled in the ascending order,
+	 * and disabled in the descending order
+	 *
+	 */
+	if (enabled != 1) 
+		goto disable_wm1;
+
+	clock = planea_clock ? planea_clock : planeb_clock;
+
+	/* WM1 */
+	sandybridge_compute_srwm(hdisplay, htotal, pixel_size,
+				 clock, SNB_WM1_LATENCY * 500,
+				 &fbc_wm, &plane_wm, &cursor_wm);
+	if (sandybridge_check_srwm(dev, fbc_wm, plane_wm, cursor_wm, 1))
+		goto disable_wm1;
+	tmp = (WM1_LP_SR_EN | 
+	       (SNB_WM1_LATENCY << WM1_LP_LATENCY_SHIFT) |
+	       (fbc_wm << WM1_LP_FBC_SHIFT) |
+	       (plane_wm << WM1_LP_SR_SHIFT) |
+	       cursor_wm);
+	I915_WRITE(WM1_LP_ILK, tmp);
+
+	/* WM2 */
+	sandybridge_compute_srwm(hdisplay, htotal, pixel_size,
+				 clock, SNB_WM2_LATENCY * 500,
+				 &fbc_wm, &plane_wm, &cursor_wm);
+	/* 
+	 * if watermark check failed, do not enable the rest 
+	 * level watermarks
+	 */
+	if (sandybridge_check_srwm(dev, fbc_wm, plane_wm, cursor_wm, 2))
+		goto disable_wm2;
+	tmp = (WM2_LP_EN |
+	       (SNB_WM2_LATENCY << WM1_LP_LATENCY_SHIFT) |
+	       (fbc_wm << WM1_LP_FBC_SHIFT) |
+	       (plane_wm << WM1_LP_SR_SHIFT) |
+	       cursor_wm);
+	I915_WRITE(WM2_LP_ILK, tmp);
+
+	/* WM3 */
+	sandybridge_compute_srwm(hdisplay, htotal, pixel_size,
+			         clock, SNB_WM3_LATENCY * 500,
+				 &fbc_wm, &plane_wm, &cursor_wm);
+	if (sandybridge_check_srwm(dev, fbc_wm, plane_wm, cursor_wm, 3))
+		goto disable_wm3;
+	tmp = (WM3_LP_EN |
+	       (SNB_WM3_LATENCY << WM1_LP_LATENCY_SHIFT) |
+	       (fbc_wm << WM1_LP_FBC_SHIFT) |
+	       (plane_wm << WM1_LP_SR_SHIFT) |
+	       cursor_wm);
+	I915_WRITE(WM3_LP_ILK, tmp);
+
+	return;
+
+disable_wm1:
+	I915_WRITE(WM3_LP_ILK, 0);
+	I915_WRITE(WM2_LP_ILK, 0);
+	I915_WRITE(WM1_LP_ILK, 0);
+	return;
+
+disable_wm2:
+	I915_WRITE(WM3_LP_ILK, 0);
+	I915_WRITE(WM2_LP_ILK, 0);
+	return;
+
+disable_wm3:
+	I915_WRITE(WM3_LP_ILK, 0);
+	return;
+}
+
 /**
  * intel_update_watermarks - update FIFO watermark values based on current modes
  *
@@ -6010,6 +6280,35 @@ void intel_enable_clock_gating(struct drm_device *dev)
 				   _3D_CHICKEN2_WM_READ_PIPELINED);
 		}
 
+		/*
+		 * According to the spec the following bits should be set in
+		 * order to enable memory self-refresh and fbc:
+		 * The bit21 and bit22 of 0x42000
+		 * The bit21 and bit22 of 0x42004 
+		 * The bit5 and bit7 of 0x42020 
+		 * The bit14 of 0x70180 
+		 * The bit14 of 0x71180 
+		 */
+		if (IS_GEN6(dev)) {
+			I915_WRITE(ILK_DISPLAY_CHICKEN1,
+					(I915_READ(ILK_DISPLAY_CHICKEN1) |
+					ILK_FBCQ_DIS | ILK_PABSTRETCH_DIS));
+			I915_WRITE(ILK_DISPLAY_CHICKEN2,
+					(I915_READ(ILK_DISPLAY_CHICKEN2) |
+					ILK_DPARB_GATE | ILK_VSDPFD_FULL));
+			I915_WRITE(ILK_DSPCLK_GATE,
+					(I915_READ(ILK_DSPCLK_GATE) |
+						ILK_DPARB_CLK_GATE  |
+						ILK_DPFD_CLK_GATE));
+			I915_WRITE(DSPACNTR,
+					(I915_READ(DSPACNTR) | 
+					DISPPLANE_TRICKLE_FEED_DISABLE));
+			I915_WRITE(DSPBCNTR,
+					(I915_READ(DSPBCNTR) | 
+					DISPPLANE_TRICKLE_FEED_DISABLE));
+		}
+
+
 	} else if (IS_G4X(dev)) {
 		uint32_t dspclk_gate;
 		I915_WRITE(RENCLK_GATE_D1, 0);
@@ -6176,6 +6475,14 @@ static void intel_init_display(struct drm_device *dev)
 					      "Disable CxSR\n");
 				dev_priv->display.update_wm = NULL;
 			}
+		} else if (IS_GEN6(dev)) {
+			if (SNB_WM0_LATENCY) {
+				dev_priv->display.update_wm = sandybridge_update_wm;
+			} else {
+				DRM_DEBUG_KMS("Failed to get proper latency. "
+					      "Disable CxSR\n");
+				dev_priv->display.update_wm = NULL;
+			}
 		} else
 			dev_priv->display.update_wm = NULL;
 	} else if (IS_PINEVIEW(dev)) {
-- 
1.7.2.3

[PATCH 2/2] drm/i915: Add frame buffer compression on Sandybridge

[Yuanhan Liu]

Add frame buffer compression on Sandybridge. The method is similar to
Ironlake, except that two new registers of type GTTMMADR must be written
with the right fence info.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |    4 ++--
 drivers/gpu/drm/i915/i915_drv.c      |    1 +
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    8 +++++++-
 4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e9fb895..3f7b203 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1078,7 +1078,7 @@ static void i915_setup_compression(struct drm_device *dev, int size)
 	if (!cfb_base)
 		goto err_fb;
 
-	if (!(IS_GM45(dev) || IS_IRONLAKE_M(dev))) {
+	if (!(IS_GM45(dev) || HAS_PCH_SPLIT(dev))) {
 		compressed_llb = drm_mm_search_free(&dev_priv->mm.stolen,
 						    4096, 4096, 0);
 		if (compressed_llb)
@@ -1096,7 +1096,7 @@ static void i915_setup_compression(struct drm_device *dev, int size)
 
 	intel_disable_fbc(dev);
 	dev_priv->compressed_fb = compressed_fb;
-	if (IS_IRONLAKE_M(dev))
+	if (HAS_PCH_SPLIT(dev))
 		I915_WRITE(ILK_DPFC_CB_BASE, compressed_fb->start);
 	else if (IS_GM45(dev)) {
 		I915_WRITE(DPFC_CB_BASE, compressed_fb->start);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5f20cd9..bdb29b2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -165,6 +165,7 @@ static const struct intel_device_info intel_sandybridge_d_info = {
 static const struct intel_device_info intel_sandybridge_m_info = {
 	.gen = 6, .is_mobile = 1,
 	.need_gfx_hws = 1, .has_hotplug = 1,
+	.has_fbc = 1,
 	.has_bsd_ring = 1,
 	.has_blt_ring = 1,
 };
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5bba63a..811aeb4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -599,6 +599,18 @@
 #define   ILK_PABSTRETCH_DIS 	(1<<21)
 
 
+/* 
+ * Framebuffer compression for Sandybridge
+ *
+ * The following two registers are of type GTTMMADR
+ */
+#define SNB_DPFC_CTL_SA		0x100100
+#define   SNB_CPU_FENCE_ENABLE	(1<<29)
+#define DPFC_CPU_FENCE_OFFSET	0x100104
+
+
+
+
 /*
  * GPIO regs
  */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 59a6d89..9887172 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1264,6 +1264,12 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
+	if (IS_GEN6(dev)) {
+		I915_WRITE(SNB_DPFC_CTL_SA, 
+				SNB_CPU_FENCE_ENABLE | dev_priv->cfb_fence);
+		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
+	}
+
 	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
 }
 
@@ -6426,7 +6432,7 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.dpms = i9xx_crtc_dpms;
 
 	if (I915_HAS_FBC(dev)) {
-		if (IS_IRONLAKE_M(dev)) {
+		if (HAS_PCH_SPLIT(dev)) {
 			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
 			dev_priv->display.enable_fbc = ironlake_enable_fbc;
 			dev_priv->display.disable_fbc = ironlake_disable_fbc;
-- 
1.7.2.3

Re: [PATCH 0/2] drm/i915: enable CxSR and FBC on Sandybridge

[Chris Wilson]

On Wed, 15 Dec 2010 15:42:30 +0800, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> Hi, 
> 
> This two patches enable self refresh and frame buffer compression
> on Sandybridge.
> 
> FYI, with my test, enable CxSR would save about 0.7W, while enable
> FBC would save about 0.1~0.2W.

Do you have some numbers comparable to
https://bugs.freedesktop.org/show_bug.cgi?id=31588 ?
I'm interested to know if, first, we are able to reproduce those numbers
and, secondly, if we've started to narrow the 3W gap for enabling a pipe
between the Windows driver and ourselves.

The patches themselves look good. A minor bit of code duplication that we
could possibly eliminate in the first...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/2] drm/i915: enable CxSR and FBC on Sandybridge

[Yuanhan Liu]

On Wed, Dec 15, 2010 at 10:32:50AM +0000, Chris Wilson wrote:
> On Wed, 15 Dec 2010 15:42:30 +0800, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> > Hi, 
> > 
> > This two patches enable self refresh and frame buffer compression
> > on Sandybridge.
> > 
> > FYI, with my test, enable CxSR would save about 0.7W, while enable
> > FBC would save about 0.1~0.2W.
> 
> Do you have some numbers comparable to
> https://bugs.freedesktop.org/show_bug.cgi?id=31588 ?
> I'm interested to know if, first, we are able to reproduce those numbers
> and, 

Will update it at bugzilla soon.

> secondly, if we've started to narrow the 3W gap for enabling a pipe
> between the Windows driver and ourselves.

This two patches mostly save just about 1W;)


Thanks.
Yuanhan Liu

Re: [PATCH 0/2] drm/i915: enable CxSR and FBC on Sandybridge

[Fu Michael]

On 2010/12/16 18:09, Yuanhan Liu wrote:
> On Wed, Dec 15, 2010 at 10:32:50AM +0000, Chris Wilson wrote:
>> On Wed, 15 Dec 2010 15:42:30 +0800, Yuanhan Liu<yuanhan.liu@linux.intel.com>  wrote:
>>> Hi,
>>>
>>> This two patches enable self refresh and frame buffer compression
>>> on Sandybridge.
>>>
>>> FYI, with my test, enable CxSR would save about 0.7W, while enable
>>> FBC would save about 0.1~0.2W.
>>
>> Do you have some numbers comparable to
>> https://bugs.freedesktop.org/show_bug.cgi?id=31588 ?
>> I'm interested to know if, first, we are able to reproduce those numbers
>> and,
>
> Will update it at bugzilla soon.
>
>> secondly, if we've started to narrow the 3W gap for enabling a pipe
>> between the Windows driver and ourselves.
>
> This two patches mostly save just about 1W;)
>

Based on experience from previous generation, it's pretty much that 
number, and another 1W idle power can be saved from Render-C. those are 
pretty much the HW power saving feature we can leverage now.

Another feature called DPST which basically adjust backlight based on 
color histogram can't be open sourced. Not sure if it's enabled in the 
windows test environment in bug# 31588.

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