[PATCH 1/2] drm/i915: Add pineview_update_wm callback to update watermark for pineview

[PATCH 1/2] drm/i915: Add pineview_update_wm callback to update watermark for pineview

[Zhenyu Wang]

From: Zhao Yakui <yakui.zhao@intel.com>

Now we don't define the update_wm callback function explicitly for
pineview platform. But as it also belongs to mobile platform, we will
use the function of i9xx_update_wm function to update its watermark again
after we configure the watermark. This is incorrect.

So we will add a update_wm callback function for pineview to update watermark.

BTW: We will disable the self-refresh and never enable it any more if we
can't find the appropriate the latency on pineview plaftorm. In such case
the update_wm callback will be NULL.

The bitmask macro is also defined to access the corresponding fifo
watermark register.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |    9 ++
 drivers/gpu/drm/i915/intel_display.c |  153 +++++++++++++++++----------------
 2 files changed, 88 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8de8194..33967dd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1986,15 +1986,24 @@
 
 #define DSPFW1			0x70034
 #define   DSPFW_SR_SHIFT	23
+#define   DSPFW_SR_MASK 	(0x1ff<<23)
 #define   DSPFW_CURSORB_SHIFT	16
+#define   DSPFW_CURSORB_MASK	(0x3f<<16)
 #define   DSPFW_PLANEB_SHIFT	8
+#define   DSPFW_PLANEB_MASK	(0x7f<<8)
+#define   DSPFW_PLANEA_MASK	(0x7f)
 #define DSPFW2			0x70038
 #define   DSPFW_CURSORA_MASK	0x00003f00
 #define   DSPFW_CURSORA_SHIFT	8
+#define   DSPFW_PLANEC_MASK	(0x7f)
 #define DSPFW3			0x7003c
 #define   DSPFW_HPLL_SR_EN	(1<<31)
 #define   DSPFW_CURSOR_SR_SHIFT	24
 #define   PINEVIEW_SELF_REFRESH_EN	(1<<30)
+#define   DSPFW_CURSOR_SR_MASK		(0x3f<<24)
+#define   DSPFW_HPLL_CURSOR_SHIFT	16
+#define   DSPFW_HPLL_CURSOR_MASK	(0x3f<<16)
+#define   DSPFW_HPLL_SR_MASK		(0x1ff)
 
 /* FIFO watermark sizes etc */
 #define G4X_FIFO_LINE_SIZE	64
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1a7c7ac..e1653d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2662,66 +2662,6 @@ static void pineview_disable_cxsr(struct drm_device *dev)
 	DRM_INFO("Big FIFO is disabled\n");
 }
 
-static void pineview_enable_cxsr(struct drm_device *dev, unsigned long clock,
-				 int pixel_size)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 reg;
-	unsigned long wm;
-	struct cxsr_latency *latency;
-
-	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev), dev_priv->fsb_freq,
-		dev_priv->mem_freq);
-	if (!latency) {
-		DRM_DEBUG_KMS("Unknown FSB/MEM found, disable CxSR\n");
-		pineview_disable_cxsr(dev);
-		return;
-	}
-
-	/* Display SR */
-	wm = intel_calculate_wm(clock, &pineview_display_wm, pixel_size,
-				latency->display_sr);
-	reg = I915_READ(DSPFW1);
-	reg &= 0x7fffff;
-	reg |= wm << 23;
-	I915_WRITE(DSPFW1, reg);
-	DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
-
-	/* cursor SR */
-	wm = intel_calculate_wm(clock, &pineview_cursor_wm, pixel_size,
-				latency->cursor_sr);
-	reg = I915_READ(DSPFW3);
-	reg &= ~(0x3f << 24);
-	reg |= (wm & 0x3f) << 24;
-	I915_WRITE(DSPFW3, reg);
-
-	/* Display HPLL off SR */
-	wm = intel_calculate_wm(clock, &pineview_display_hplloff_wm,
-		latency->display_hpll_disable, I915_FIFO_LINE_SIZE);
-	reg = I915_READ(DSPFW3);
-	reg &= 0xfffffe00;
-	reg |= wm & 0x1ff;
-	I915_WRITE(DSPFW3, reg);
-
-	/* cursor HPLL off SR */
-	wm = intel_calculate_wm(clock, &pineview_cursor_hplloff_wm, pixel_size,
-				latency->cursor_hpll_disable);
-	reg = I915_READ(DSPFW3);
-	reg &= ~(0x3f << 16);
-	reg |= (wm & 0x3f) << 16;
-	I915_WRITE(DSPFW3, reg);
-	DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
-
-	/* activate cxsr */
-	reg = I915_READ(DSPFW3);
-	reg |= PINEVIEW_SELF_REFRESH_EN;
-	I915_WRITE(DSPFW3, reg);
-
-	DRM_INFO("Big FIFO is enabled\n");
-
-	return;
-}
-
 /*
  * Latency for FIFO fetches is dependent on several factors:
  *   - memory configuration (speed, channels)
@@ -2806,6 +2746,71 @@ static int i830_get_fifo_size(struct drm_device *dev, int plane)
 	return size;
 }
 
+static void pineview_update_wm(struct drm_device *dev,  int planea_clock,
+			  int planeb_clock, int sr_hdisplay, int pixel_size)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 reg;
+	unsigned long wm;
+	struct cxsr_latency *latency;
+	int sr_clock;
+
+	latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev), dev_priv->fsb_freq,
+					 dev_priv->mem_freq);
+	if (!latency) {
+		DRM_DEBUG_KMS("Unknown FSB/MEM found, disable CxSR\n");
+		pineview_disable_cxsr(dev);
+		return;
+	}
+
+	if (!planea_clock || !planeb_clock) {
+		sr_clock = planea_clock ? planea_clock : planeb_clock;
+
+		/* Display SR */
+		wm = intel_calculate_wm(sr_clock, &pineview_display_wm,
+					pixel_size, latency->display_sr);
+		reg = I915_READ(DSPFW1);
+		reg &= ~DSPFW_SR_MASK;
+		reg |= wm << DSPFW_SR_SHIFT;
+		I915_WRITE(DSPFW1, reg);
+		DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
+
+		/* cursor SR */
+		wm = intel_calculate_wm(sr_clock, &pineview_cursor_wm,
+					pixel_size, latency->cursor_sr);
+		reg = I915_READ(DSPFW3);
+		reg &= ~DSPFW_CURSOR_SR_MASK;
+		reg |= (wm & 0x3f) << DSPFW_CURSOR_SR_SHIFT;
+		I915_WRITE(DSPFW3, reg);
+
+		/* Display HPLL off SR */
+		wm = intel_calculate_wm(sr_clock, &pineview_display_hplloff_wm,
+					pixel_size, latency->display_hpll_disable);
+		reg = I915_READ(DSPFW3);
+		reg &= ~DSPFW_HPLL_SR_MASK;
+		reg |= wm & DSPFW_HPLL_SR_MASK;
+		I915_WRITE(DSPFW3, reg);
+
+		/* cursor HPLL off SR */
+		wm = intel_calculate_wm(sr_clock, &pineview_cursor_hplloff_wm,
+					pixel_size, latency->cursor_hpll_disable);
+		reg = I915_READ(DSPFW3);
+		reg &= ~DSPFW_HPLL_CURSOR_MASK;
+		reg |= (wm & 0x3f) << DSPFW_HPLL_CURSOR_SHIFT;
+		I915_WRITE(DSPFW3, reg);
+		DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
+
+		/* activate cxsr */
+		reg = I915_READ(DSPFW3);
+		reg |= PINEVIEW_SELF_REFRESH_EN;
+		I915_WRITE(DSPFW3, reg);
+		DRM_DEBUG_KMS("Self-refresh is enabled\n");
+	} else {
+		pineview_disable_cxsr(dev);
+		DRM_DEBUG_KMS("Self-refresh is disabled\n");
+	}
+}
+
 static void g4x_update_wm(struct drm_device *dev,  int planea_clock,
 			  int planeb_clock, int sr_hdisplay, int pixel_size)
 {
@@ -3095,12 +3100,6 @@ static void intel_update_watermarks(struct drm_device *dev)
 	if (enabled <= 0)
 		return;
 
-	/* Single plane configs can enable self refresh */
-	if (enabled == 1 && IS_PINEVIEW(dev))
-		pineview_enable_cxsr(dev, sr_clock, pixel_size);
-	else if (IS_PINEVIEW(dev))
-		pineview_disable_cxsr(dev);
-
 	dev_priv->display.update_wm(dev, planea_clock, planeb_clock,
 				    sr_hdisplay, pixel_size);
 }
@@ -5116,7 +5115,20 @@ static void intel_init_display(struct drm_device *dev)
 	/* For FIFO watermark updates */
 	if (HAS_PCH_SPLIT(dev))
 		dev_priv->display.update_wm = NULL;
-	else if (IS_G4X(dev))
+	else if (IS_PINEVIEW(dev)) {
+		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
+					    dev_priv->fsb_freq,
+					    dev_priv->mem_freq)) {
+			DRM_INFO("failed to find known CxSR latency "
+				 "(found fsb freq %d, mem freq %d), "
+				 "disabling CxSR\n",
+				 dev_priv->fsb_freq, dev_priv->mem_freq);
+			/* Disable CxSR and never update its watermark again */
+			pineview_disable_cxsr(dev);
+			dev_priv->display.update_wm = NULL;
+		} else
+			dev_priv->display.update_wm = pineview_update_wm;
+	} else if (IS_G4X(dev))
 		dev_priv->display.update_wm = g4x_update_wm;
 	else if (IS_I965G(dev))
 		dev_priv->display.update_wm = i965_update_wm;
@@ -5189,13 +5201,6 @@ void intel_modeset_init(struct drm_device *dev)
 		    (unsigned long)dev);
 
 	intel_setup_overlay(dev);
-
-	if (IS_PINEVIEW(dev) && !intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
-							dev_priv->fsb_freq,
-							dev_priv->mem_freq))
-		DRM_INFO("failed to find known CxSR latency "
-			 "(found fsb freq %d, mem freq %d), disabling CxSR\n",
-			 dev_priv->fsb_freq, dev_priv->mem_freq);
 }
 
 void intel_modeset_cleanup(struct drm_device *dev)
-- 
1.6.3.3

[PATCH 2/2] drm/i915: Add the support of memory self-refresh on Ironlake

[Zhenyu Wang]

Update the self-refresh watermark for display plane/cursor and enable
the memory self-refresh on Ironlake. The watermark is also updated for
the active display plane.

More than 1W idle power is saved on one Ironlake laptop after enabling
memory self-refresh.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   44 +++++++++
 drivers/gpu/drm/i915/intel_display.c |  160 +++++++++++++++++++++++++++++++++-
 2 files changed, 201 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 33967dd..0a3dbfc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2030,6 +2030,43 @@
 #define PINEVIEW_CURSOR_DFT_WM	0
 #define PINEVIEW_CURSOR_GUARD_WM	5
 
+
+/* define the Watermark register on Ironlake */
+#define WM0_PIPEA_ILK		0x45100
+#define  WM0_PIPE_PLANE_MASK	(0x7f<<16)
+#define  WM0_PIPE_PLANE_SHIFT	16
+#define  WM0_PIPE_SPRITE_MASK	(0x3f<<8)
+#define  WM0_PIPE_SPRITE_SHIFT	8
+#define  WM0_PIPE_CURSOR_MASK	(0x1f)
+
+#define WM0_PIPEB_ILK		0x45104
+#define WM1_LP_ILK		0x45108
+#define  WM1_LP_SR_EN		(1<<31)
+#define  WM1_LP_LATENCY_SHIFT	24
+#define  WM1_LP_LATENCY_MASK	(0x7f<<24)
+#define  WM1_LP_SR_MASK		(0x1ff<<8)
+#define  WM1_LP_SR_SHIFT	8
+#define  WM1_LP_CURSOR_MASK	(0x3f)
+
+/* Memory latency timer register */
+#define MLTR_ILK		0x11222
+/* the unit of memory self-refresh latency time is 0.5us */
+#define  ILK_SRLT_MASK		0x3f
+
+/* define the fifo size on Ironlake */
+#define ILK_DISPLAY_FIFO	128
+#define ILK_DISPLAY_MAXWM	64
+#define ILK_DISPLAY_DFTWM	8
+
+#define ILK_DISPLAY_SR_FIFO	512
+#define ILK_DISPLAY_MAX_SRWM	0x1ff
+#define ILK_DISPLAY_DFT_SRWM	0x3f
+#define ILK_CURSOR_SR_FIFO	64
+#define ILK_CURSOR_MAX_SRWM	0x3f
+#define ILK_CURSOR_DFT_SRWM	8
+
+#define ILK_FIFO_LINE_SIZE	64
+
 /*
  * The two pipe frame counter registers are not synchronized, so
  * reading a stable value is somewhat tricky. The following code
@@ -2310,8 +2347,15 @@
 #define GTIIR   0x44018
 #define GTIER   0x4401c
 
+#define ILK_DISPLAY_CHICKEN2	0x42004
+#define  ILK_DPARB_GATE	(1<<22)
+#define  ILK_VSDPFD_FULL	(1<<21)
+#define ILK_DSPCLK_GATE		0x42020
+#define  ILK_DPARB_CLK_GATE	(1<<5)
+
 #define DISP_ARB_CTL	0x45000
 #define  DISP_TILE_SURFACE_SWIZZLING	(1<<13)
+#define  DISP_FBC_WM_DIS		(1<<15)
 
 /* PCH */
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e1653d1..18294b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2544,6 +2544,30 @@ static struct intel_watermark_params i830_wm_info = {
 	I830_FIFO_LINE_SIZE
 };
 
+static struct intel_watermark_params ironlake_display_wm_info = {
+	ILK_DISPLAY_FIFO,
+	ILK_DISPLAY_MAXWM,
+	ILK_DISPLAY_DFTWM,
+	2,
+	ILK_FIFO_LINE_SIZE
+};
+
+static struct intel_watermark_params ironlake_display_srwm_info = {
+	ILK_DISPLAY_SR_FIFO,
+	ILK_DISPLAY_MAX_SRWM,
+	ILK_DISPLAY_DFT_SRWM,
+	2,
+	ILK_FIFO_LINE_SIZE
+};
+
+static struct intel_watermark_params ironlake_cursor_srwm_info = {
+	ILK_CURSOR_SR_FIFO,
+	ILK_CURSOR_MAX_SRWM,
+	ILK_CURSOR_DFT_SRWM,
+	2,
+	ILK_FIFO_LINE_SIZE
+};
+
 /**
  * intel_calculate_wm - calculate watermark level
  * @clock_in_khz: pixel clock
@@ -3031,6 +3055,108 @@ static void i830_update_wm(struct drm_device *dev, int planea_clock, int unused,
 	I915_WRITE(FW_BLC, fwater_lo);
 }
 
+#define ILK_LP0_PLANE_LATENCY		700
+
+static void ironlake_update_wm(struct drm_device *dev,  int planea_clock,
+		       int planeb_clock, int sr_hdisplay, int pixel_size)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
+	int sr_wm, cursor_wm;
+	unsigned long line_time_us;
+	int sr_clock, entries_required;
+	u32 reg_value;
+
+	/* Calculate and update the watermark for plane A */
+	if (planea_clock) {
+		entries_required = ((planea_clock / 1000) * pixel_size *
+				     ILK_LP0_PLANE_LATENCY) / 1000;
+		entries_required = DIV_ROUND_UP(entries_required,
+				   ironlake_display_wm_info.cacheline_size);
+		planea_wm = entries_required +
+			    ironlake_display_wm_info.guard_size;
+
+		if (planea_wm > (int)ironlake_display_wm_info.max_wm)
+			planea_wm = ironlake_display_wm_info.max_wm;
+
+		cursora_wm = 16;
+		reg_value = I915_READ(WM0_PIPEA_ILK);
+		reg_value &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
+		reg_value |= (planea_wm << WM0_PIPE_PLANE_SHIFT) |
+			     (cursora_wm & WM0_PIPE_CURSOR_MASK);
+		I915_WRITE(WM0_PIPEA_ILK, reg_value);
+		DRM_DEBUG_KMS("FIFO watermarks For pipe A - plane %d, "
+				"cursor: %d\n", planea_wm, cursora_wm);
+	}
+	/* Calculate and update the watermark for plane B */
+	if (planeb_clock) {
+		entries_required = ((planeb_clock / 1000) * pixel_size *
+				     ILK_LP0_PLANE_LATENCY) / 1000;
+		entries_required = DIV_ROUND_UP(entries_required,
+				   ironlake_display_wm_info.cacheline_size);
+		planeb_wm = entries_required +
+			    ironlake_display_wm_info.guard_size;
+
+		if (planeb_wm > (int)ironlake_display_wm_info.max_wm)
+			planeb_wm = ironlake_display_wm_info.max_wm;
+
+		cursorb_wm = 16;
+		reg_value = I915_READ(WM0_PIPEB_ILK);
+		reg_value &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
+		reg_value |= (planeb_wm << WM0_PIPE_PLANE_SHIFT) |
+			     (cursorb_wm & WM0_PIPE_CURSOR_MASK);
+		I915_WRITE(WM0_PIPEB_ILK, reg_value);
+		DRM_DEBUG_KMS("FIFO watermarks For pipe B - plane %d, "
+				"cursor: %d\n", planeb_wm, cursorb_wm);
+	}
+
+	/*
+	 * Calculate and update the self-refresh watermark only when one
+	 * display plane is used.
+	 */
+	if (!planea_clock || !planeb_clock) {
+		int line_count;
+		/* Read the self-refresh latency. The unit is 0.5us */
+		int ilk_sr_latency = I915_READ(MLTR_ILK) & ILK_SRLT_MASK;
+
+		sr_clock = planea_clock ? planea_clock : planeb_clock;
+		line_time_us = ((sr_hdisplay * 1000) / sr_clock);
+
+		/* Use ns/us then divide to preserve precision */
+		line_count = ((ilk_sr_latency * 500) / line_time_us + 1000)
+			       / 1000;
+
+		/* calculate the self-refresh watermark for display plane */
+		entries_required = line_count * sr_hdisplay * pixel_size;
+		entries_required = DIV_ROUND_UP(entries_required,
+				   ironlake_display_srwm_info.cacheline_size);
+		sr_wm = entries_required +
+			ironlake_display_srwm_info.guard_size;
+
+		/* calculate the self-refresh watermark for display cursor */
+		entries_required = line_count * pixel_size * 64;
+		entries_required = DIV_ROUND_UP(entries_required,
+				   ironlake_cursor_srwm_info.cacheline_size);
+		cursor_wm = entries_required +
+			    ironlake_cursor_srwm_info.guard_size;
+
+		/* configure watermark and enable self-refresh */
+		reg_value = I915_READ(WM1_LP_ILK);
+		reg_value &= ~(WM1_LP_LATENCY_MASK | WM1_LP_SR_MASK |
+			       WM1_LP_CURSOR_MASK);
+		reg_value |= WM1_LP_SR_EN |
+			     (ilk_sr_latency << WM1_LP_LATENCY_SHIFT) |
+			     (sr_wm << WM1_LP_SR_SHIFT) | cursor_wm;
+
+		I915_WRITE(WM1_LP_ILK, reg_value);
+		DRM_DEBUG_KMS("self-refresh watermark: display plane %d "
+				"cursor %d\n", sr_wm, cursor_wm);
+
+	} else {
+		/* Turn off self refresh if both pipes are enabled */
+		I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+	}
+}
 /**
  * intel_update_watermarks - update FIFO watermark values based on current modes
  *
@@ -4998,6 +5124,25 @@ void intel_init_clock_gating(struct drm_device *dev)
 		}
 
 		I915_WRITE(PCH_DSPCLK_GATE_D, dspclk_gate);
+
+		/*
+		 * According to the spec the following bits should be set in
+		 * order to enable memory self-refresh
+		 * The bit 22/21 of 0x42004
+		 * The bit 5 of 0x42020
+		 * The bit 15 of 0x45000
+		 */
+		if (IS_IRONLAKE(dev)) {
+			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));
+			I915_WRITE(DISP_ARB_CTL,
+					(I915_READ(DISP_ARB_CTL) |
+						DISP_FBC_WM_DIS));
+		}
 		return;
 	} else if (IS_G4X(dev)) {
 		uint32_t dspclk_gate;
@@ -5113,9 +5258,18 @@ static void intel_init_display(struct drm_device *dev)
 			i830_get_display_clock_speed;
 
 	/* For FIFO watermark updates */
-	if (HAS_PCH_SPLIT(dev))
-		dev_priv->display.update_wm = NULL;
-	else if (IS_PINEVIEW(dev)) {
+	if (HAS_PCH_SPLIT(dev)) {
+		if (IS_IRONLAKE(dev)) {
+			if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK)
+				dev_priv->display.update_wm = ironlake_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)) {
 		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
 					    dev_priv->fsb_freq,
 					    dev_priv->mem_freq)) {
-- 
1.6.3.3

Re: [PATCH 2/2] drm/i915: Add the support of memory self-refresh on Ironlake

[Adam Jackson]

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

On Mon, 2010-04-12 at 17:09 +0800, Zhenyu Wang wrote:
> Update the self-refresh watermark for display plane/cursor and enable
> the memory self-refresh on Ironlake. The watermark is also updated for
> the active display plane.
> 
> More than 1W idle power is saved on one Ironlake laptop after enabling
> memory self-refresh.

Looks good at a quick read, though I haven't tested it yet.

> +	/* Calculate and update the watermark for plane A */
> +	if (planea_clock) {
> +		entries_required = ((planea_clock / 1000) * pixel_size *
> +				     ILK_LP0_PLANE_LATENCY) / 1000;
> +		entries_required = DIV_ROUND_UP(entries_required,
> +				   ironlake_display_wm_info.cacheline_size);
> +		planea_wm = entries_required +
> +			    ironlake_display_wm_info.guard_size;
> +
> +		if (planea_wm > (int)ironlake_display_wm_info.max_wm)
> +			planea_wm = ironlake_display_wm_info.max_wm;
> +
> +		cursora_wm = 16;
> +		reg_value = I915_READ(WM0_PIPEA_ILK);
> +		reg_value &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> +		reg_value |= (planea_wm << WM0_PIPE_PLANE_SHIFT) |
> +			     (cursora_wm & WM0_PIPE_CURSOR_MASK);
> +		I915_WRITE(WM0_PIPEA_ILK, reg_value);
> +		DRM_DEBUG_KMS("FIFO watermarks For pipe A - plane %d, "
> +				"cursor: %d\n", planea_wm, cursora_wm);
> +	}

This doesn't adjust the video sprite plane watermark.  I suppose it
doesn't really matter, since we're not enabling it yet, but for clarity
I'd also mask off WM0_PIPE_SPRITE_MASK.

Also, the cursor watermark is a magic number.  I assume it's derived
from the hardware cursor size.  In which case, it'd be nice to show the
derivation; Ironlake can do hardware cursors up to 256x256, it'd be nice
to have that work automatically when and if we enable that in the
driver.

> +		/* configure watermark and enable self-refresh */
> +		reg_value = I915_READ(WM1_LP_ILK);
> +		reg_value &= ~(WM1_LP_LATENCY_MASK | WM1_LP_SR_MASK |
> +			       WM1_LP_CURSOR_MASK);
> +		reg_value |= WM1_LP_SR_EN |
> +			     (ilk_sr_latency << WM1_LP_LATENCY_SHIFT) |
> +			     (sr_wm << WM1_LP_SR_SHIFT) | cursor_wm;
> +
> +		I915_WRITE(WM1_LP_ILK, reg_value);
> +		DRM_DEBUG_KMS("self-refresh watermark: display plane %d "
> +				"cursor %d\n", sr_wm, cursor_wm);

What are the conditions for entering LP2 or LP3 state, and is it worth
trying to set them up too?

> +		/*
> +		 * According to the spec the following bits should be set in
> +		 * order to enable memory self-refresh
> +		 * The bit 22/21 of 0x42004
> +		 * The bit 5 of 0x42020
> +		 * The bit 15 of 0x45000
> +		 */
> +		if (IS_IRONLAKE(dev)) {
> +			I915_WRITE(ILK_DISPLAY_CHICKEN2,
> +					(I915_READ(ILK_DISPLAY_CHICKEN2) |
> +					ILK_DPARB_GATE | ILK_VSDPFD_FULL));

I can only hope this is named "chicken" in allusion to:

http://en.wikipedia.org/wiki/Chicken_(game)

I certainly can't find any reference to it in the documentation.

- 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 2/2] drm/i915: Add the support of memory self-refresh on Ironlake

[Jesse Barnes]

On Mon, 12 Apr 2010 10:43:04 -0400
Adam Jackson <ajax@redhat.com> wrote:

> On Mon, 2010-04-12 at 17:09 +0800, Zhenyu Wang wrote:
> > Update the self-refresh watermark for display plane/cursor and enable
> > the memory self-refresh on Ironlake. The watermark is also updated for
> > the active display plane.
> > 
> > More than 1W idle power is saved on one Ironlake laptop after enabling
> > memory self-refresh.
> 
> Looks good at a quick read, though I haven't tested it yet.
> 
> > +	/* Calculate and update the watermark for plane A */
> > +	if (planea_clock) {
> > +		entries_required = ((planea_clock / 1000) * pixel_size *
> > +				     ILK_LP0_PLANE_LATENCY) / 1000;
> > +		entries_required = DIV_ROUND_UP(entries_required,
> > +				   ironlake_display_wm_info.cacheline_size);
> > +		planea_wm = entries_required +
> > +			    ironlake_display_wm_info.guard_size;
> > +
> > +		if (planea_wm > (int)ironlake_display_wm_info.max_wm)
> > +			planea_wm = ironlake_display_wm_info.max_wm;
> > +
> > +		cursora_wm = 16;
> > +		reg_value = I915_READ(WM0_PIPEA_ILK);
> > +		reg_value &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> > +		reg_value |= (planea_wm << WM0_PIPE_PLANE_SHIFT) |
> > +			     (cursora_wm & WM0_PIPE_CURSOR_MASK);
> > +		I915_WRITE(WM0_PIPEA_ILK, reg_value);
> > +		DRM_DEBUG_KMS("FIFO watermarks For pipe A - plane %d, "
> > +				"cursor: %d\n", planea_wm, cursora_wm);
> > +	}
> 
> This doesn't adjust the video sprite plane watermark.  I suppose it
> doesn't really matter, since we're not enabling it yet, but for clarity
> I'd also mask off WM0_PIPE_SPRITE_MASK.
> 
> Also, the cursor watermark is a magic number.  I assume it's derived
> from the hardware cursor size.  In which case, it'd be nice to show the
> derivation; Ironlake can do hardware cursors up to 256x256, it'd be nice
> to have that work automatically when and if we enable that in the
> driver.
> 
> > +		/* configure watermark and enable self-refresh */
> > +		reg_value = I915_READ(WM1_LP_ILK);
> > +		reg_value &= ~(WM1_LP_LATENCY_MASK | WM1_LP_SR_MASK |
> > +			       WM1_LP_CURSOR_MASK);
> > +		reg_value |= WM1_LP_SR_EN |
> > +			     (ilk_sr_latency << WM1_LP_LATENCY_SHIFT) |
> > +			     (sr_wm << WM1_LP_SR_SHIFT) | cursor_wm;
> > +
> > +		I915_WRITE(WM1_LP_ILK, reg_value);
> > +		DRM_DEBUG_KMS("self-refresh watermark: display plane %d "
> > +				"cursor %d\n", sr_wm, cursor_wm);
> 
> What are the conditions for entering LP2 or LP3 state, and is it worth
> trying to set them up too?
> 
> > +		/*
> > +		 * According to the spec the following bits should be set in
> > +		 * order to enable memory self-refresh
> > +		 * The bit 22/21 of 0x42004
> > +		 * The bit 5 of 0x42020
> > +		 * The bit 15 of 0x45000
> > +		 */
> > +		if (IS_IRONLAKE(dev)) {
> > +			I915_WRITE(ILK_DISPLAY_CHICKEN2,
> > +					(I915_READ(ILK_DISPLAY_CHICKEN2) |
> > +					ILK_DPARB_GATE | ILK_VSDPFD_FULL));
> 
> I can only hope this is named "chicken" in allusion to:
> 
> http://en.wikipedia.org/wiki/Chicken_(game)
> 
> I certainly can't find any reference to it in the documentation.

We should probably add a comment about that.  The chicken bits
generally control whether new features are enabled in hw, so-called
because the designers were too chicken to just enable them
unconditionally. :)

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 2/2] drm/i915: Add the support of memory self-refresh on Ironlake

[ykzhao]

On Mon, 2010-04-12 at 22:43 +0800, Adam Jackson wrote:
> On Mon, 2010-04-12 at 17:09 +0800, Zhenyu Wang wrote:
> > Update the self-refresh watermark for display plane/cursor and enable
> > the memory self-refresh on Ironlake. The watermark is also updated for
> > the active display plane.
> > 
> > More than 1W idle power is saved on one Ironlake laptop after enabling
> > memory self-refresh.
> 
> Looks good at a quick read, though I haven't tested it yet.
> 
> > +	/* Calculate and update the watermark for plane A */
> > +	if (planea_clock) {
> > +		entries_required = ((planea_clock / 1000) * pixel_size *
> > +				     ILK_LP0_PLANE_LATENCY) / 1000;
> > +		entries_required = DIV_ROUND_UP(entries_required,
> > +				   ironlake_display_wm_info.cacheline_size);
> > +		planea_wm = entries_required +
> > +			    ironlake_display_wm_info.guard_size;
> > +
> > +		if (planea_wm > (int)ironlake_display_wm_info.max_wm)
> > +			planea_wm = ironlake_display_wm_info.max_wm;
> > +
> > +		cursora_wm = 16;
> > +		reg_value = I915_READ(WM0_PIPEA_ILK);
> > +		reg_value &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> > +		reg_value |= (planea_wm << WM0_PIPE_PLANE_SHIFT) |
> > +			     (cursora_wm & WM0_PIPE_CURSOR_MASK);
> > +		I915_WRITE(WM0_PIPEA_ILK, reg_value);
> > +		DRM_DEBUG_KMS("FIFO watermarks For pipe A - plane %d, "
> > +				"cursor: %d\n", planea_wm, cursora_wm);
> > +	}
> 
> This doesn't adjust the video sprite plane watermark.  I suppose it
> doesn't really matter, since we're not enabling it yet, but for clarity
> I'd also mask off WM0_PIPE_SPRITE_MASK.

As the sprite plane is not touched, it will be better to reserve the
corresponding sprite watermark. 

> 
> Also, the cursor watermark is a magic number.  I assume it's derived
> from the hardware cursor size.  In which case, it'd be nice to show the
> derivation; Ironlake can do hardware cursors up to 256x256, it'd be nice
> to have that work automatically when and if we enable that in the
> driver.

Very good point. Yes. It seems that the Ironlake can support up to
256X256@32bpp cursor. This is not considered in this patch. 


We can add another patch to calculate the cursor watermark.

> 
> > +		/* configure watermark and enable self-refresh */
> > +		reg_value = I915_READ(WM1_LP_ILK);
> > +		reg_value &= ~(WM1_LP_LATENCY_MASK | WM1_LP_SR_MASK |
> > +			       WM1_LP_CURSOR_MASK);
> > +		reg_value |= WM1_LP_SR_EN |
> > +			     (ilk_sr_latency << WM1_LP_LATENCY_SHIFT) |
> > +			     (sr_wm << WM1_LP_SR_SHIFT) | cursor_wm;
> > +
> > +		I915_WRITE(WM1_LP_ILK, reg_value);
> > +		DRM_DEBUG_KMS("self-refresh watermark: display plane %d "
> > +				"cursor %d\n", sr_wm, cursor_wm);
> 
> What are the conditions for entering LP2 or LP3 state, and is it worth
> trying to set them up too?

LP3 is not supported on ironlake.

After consulting the hardware group, it is not worth setting up the LP2.
It is complex to set up the LP2 but the power saving is very small. (Not
more than 0.1W).

> 
> > +		/*
> > +		 * According to the spec the following bits should be set in
> > +		 * order to enable memory self-refresh
> > +		 * The bit 22/21 of 0x42004
> > +		 * The bit 5 of 0x42020
> > +		 * The bit 15 of 0x45000
> > +		 */
> > +		if (IS_IRONLAKE(dev)) {
> > +			I915_WRITE(ILK_DISPLAY_CHICKEN2,
> > +					(I915_READ(ILK_DISPLAY_CHICKEN2) |
> > +					ILK_DPARB_GATE | ILK_VSDPFD_FULL));
> 
> I can only hope this is named "chicken" in allusion to:
> 
> http://en.wikipedia.org/wiki/Chicken_(game)
> 
> I certainly can't find any reference to it in the documentation.

This is defined in the spec from hardware group. 

What Jesse said is right. Maybe we should add more comments about it.

> 
> - ajax