public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Add polish to VLV WM shift+mask operations
@ 2015-03-10 14:16 ville.syrjala
  2015-03-10 15:02 ` [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too ville.syrjala
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: ville.syrjala @ 2015-03-10 14:16 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Wrap the FW register value shift+mask operations into a macro to hide
the ugliness a bit. Also might avoid bugs due to typos.

Also rename all the primary/sprite plane low order bit masks to have the
_VLV suffix, so that we can use the FW_WM_VLV() macro instead of the
FW_WM() macro for them in a consistent manner. Cursor and all the high
order bits are left to use the FW_WM() macro as there's no real
confusion with them.

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 10 +++---
 drivers/gpu/drm/i915/intel_pm.c | 74 +++++++++++++++++++++++------------------
 2 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 495b22b..8ff039d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4161,25 +4161,25 @@ enum skl_disp_power_wells {
 #define   DSPFW_SPRITED_WM1_SHIFT	24
 #define   DSPFW_SPRITED_WM1_MASK	(0xff<<24)
 #define   DSPFW_SPRITED_SHIFT		16
-#define   DSPFW_SPRITED_MASK		(0xff<<16)
+#define   DSPFW_SPRITED_MASK_VLV	(0xff<<16)
 #define   DSPFW_SPRITEC_WM1_SHIFT	8
 #define   DSPFW_SPRITEC_WM1_MASK	(0xff<<8)
 #define   DSPFW_SPRITEC_SHIFT		0
-#define   DSPFW_SPRITEC_MASK		(0xff<<0)
+#define   DSPFW_SPRITEC_MASK_VLV	(0xff<<0)
 #define DSPFW8_CHV		(VLV_DISPLAY_BASE + 0x700b8)
 #define   DSPFW_SPRITEF_WM1_SHIFT	24
 #define   DSPFW_SPRITEF_WM1_MASK	(0xff<<24)
 #define   DSPFW_SPRITEF_SHIFT		16
-#define   DSPFW_SPRITEF_MASK		(0xff<<16)
+#define   DSPFW_SPRITEF_MASK_VLV	(0xff<<16)
 #define   DSPFW_SPRITEE_WM1_SHIFT	8
 #define   DSPFW_SPRITEE_WM1_MASK	(0xff<<8)
 #define   DSPFW_SPRITEE_SHIFT		0
-#define   DSPFW_SPRITEE_MASK		(0xff<<0)
+#define   DSPFW_SPRITEE_MASK_VLV	(0xff<<0)
 #define DSPFW9_CHV		(VLV_DISPLAY_BASE + 0x7007c) /* wtf #2? */
 #define   DSPFW_PLANEC_WM1_SHIFT	24
 #define   DSPFW_PLANEC_WM1_MASK		(0xff<<24)
 #define   DSPFW_PLANEC_SHIFT		16
-#define   DSPFW_PLANEC_MASK		(0xff<<16)
+#define   DSPFW_PLANEC_MASK_VLV		(0xff<<16)
 #define   DSPFW_CURSORC_WM1_SHIFT	8
 #define   DSPFW_CURSORC_WM1_MASK	(0x3f<<16)
 #define   DSPFW_CURSORC_SHIFT		0
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0e84558..8ac358d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -835,6 +835,11 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 			      display, cursor);
 }
 
+#define FW_WM(value, plane) \
+	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
+#define FW_WM_VLV(value, plane) \
+	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
+
 static void vlv_write_wm_values(struct intel_crtc *crtc,
 				const struct vlv_wm_values *wm)
 {
@@ -848,50 +853,50 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
 		   (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
 
 	I915_WRITE(DSPFW1,
-		   ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
-		   ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & DSPFW_CURSORB_MASK) |
-		   ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & DSPFW_PLANEB_MASK_VLV) |
-		   ((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & DSPFW_PLANEA_MASK_VLV));
+		   FW_WM(wm->sr.plane, SR) |
+		   FW_WM(wm->pipe[PIPE_B].cursor, CURSORB) |
+		   FW_WM_VLV(wm->pipe[PIPE_B].primary, PLANEB) |
+		   FW_WM_VLV(wm->pipe[PIPE_A].primary, PLANEA));
 	I915_WRITE(DSPFW2,
-		   ((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & DSPFW_SPRITEB_MASK_VLV) |
-		   ((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) & DSPFW_CURSORA_MASK) |
-		   ((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) & DSPFW_SPRITEA_MASK_VLV));
+		   FW_WM_VLV(wm->pipe[PIPE_A].sprite[1], SPRITEB) |
+		   FW_WM(wm->pipe[PIPE_A].cursor, CURSORA) |
+		   FW_WM_VLV(wm->pipe[PIPE_A].sprite[0], SPRITEA));
 	I915_WRITE(DSPFW3,
-		   ((wm->sr.cursor << DSPFW_CURSOR_SR_SHIFT) & DSPFW_CURSOR_SR_MASK));
+		   FW_WM(wm->sr.cursor, CURSOR_SR));
 
 	if (IS_CHERRYVIEW(dev_priv)) {
 		I915_WRITE(DSPFW7_CHV,
-			   ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) & DSPFW_SPRITED_MASK) |
-			   ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) & DSPFW_SPRITEC_MASK));
+			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
+			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
 		I915_WRITE(DSPFW8_CHV,
-			   ((wm->pipe[PIPE_C].sprite[1] << DSPFW_SPRITEF_SHIFT) & DSPFW_SPRITEF_MASK) |
-			   ((wm->pipe[PIPE_C].sprite[0] << DSPFW_SPRITEE_SHIFT) & DSPFW_SPRITEE_MASK));
+			   FW_WM_VLV(wm->pipe[PIPE_C].sprite[1], SPRITEF) |
+			   FW_WM_VLV(wm->pipe[PIPE_C].sprite[0], SPRITEE));
 		I915_WRITE(DSPFW9_CHV,
-			   ((wm->pipe[PIPE_C].primary << DSPFW_PLANEC_SHIFT) & DSPFW_PLANEC_MASK) |
-			   ((wm->pipe[PIPE_C].cursor << DSPFW_CURSORC_SHIFT) & DSPFW_CURSORC_MASK));
+			   FW_WM_VLV(wm->pipe[PIPE_C].primary, PLANEC) |
+			   FW_WM(wm->pipe[PIPE_C].cursor, CURSORC));
 		I915_WRITE(DSPHOWM,
-			   (((wm->sr.plane >> 9) << DSPFW_SR_HI_SHIFT) & DSPFW_SR_HI_MASK) |
-			   (((wm->pipe[PIPE_C].sprite[1] >> 8) << DSPFW_SPRITEF_HI_SHIFT) & DSPFW_SPRITEF_HI_MASK) |
-			   (((wm->pipe[PIPE_C].sprite[0] >> 8) << DSPFW_SPRITEE_HI_SHIFT) & DSPFW_SPRITEE_HI_MASK) |
-			   (((wm->pipe[PIPE_C].primary >> 8) << DSPFW_PLANEC_HI_SHIFT) & DSPFW_PLANEC_HI_MASK) |
-			   (((wm->pipe[PIPE_B].sprite[1] >> 8) << DSPFW_SPRITED_HI_SHIFT) & DSPFW_SPRITED_HI_MASK) |
-			   (((wm->pipe[PIPE_B].sprite[0] >> 8) << DSPFW_SPRITEC_HI_SHIFT) & DSPFW_SPRITEC_HI_MASK) |
-			   (((wm->pipe[PIPE_B].primary >> 8) << DSPFW_PLANEB_HI_SHIFT) & DSPFW_PLANEB_HI_MASK) |
-			   (((wm->pipe[PIPE_A].sprite[1] >> 8) << DSPFW_SPRITEB_HI_SHIFT) & DSPFW_SPRITEB_HI_MASK) |
-			   (((wm->pipe[PIPE_A].sprite[0] >> 8) << DSPFW_SPRITEA_HI_SHIFT) & DSPFW_SPRITEA_HI_MASK) |
-			   (((wm->pipe[PIPE_A].primary >> 8) << DSPFW_PLANEA_HI_SHIFT) & DSPFW_PLANEA_HI_MASK));
+			   FW_WM(wm->sr.plane >> 9, SR_HI) |
+			   FW_WM(wm->pipe[PIPE_C].sprite[1] >> 8, SPRITEF_HI) |
+			   FW_WM(wm->pipe[PIPE_C].sprite[0] >> 8, SPRITEE_HI) |
+			   FW_WM(wm->pipe[PIPE_C].primary >> 8, PLANEC_HI) |
+			   FW_WM(wm->pipe[PIPE_B].sprite[1] >> 8, SPRITED_HI) |
+			   FW_WM(wm->pipe[PIPE_B].sprite[0] >> 8, SPRITEC_HI) |
+			   FW_WM(wm->pipe[PIPE_B].primary >> 8, PLANEB_HI) |
+			   FW_WM(wm->pipe[PIPE_A].sprite[1] >> 8, SPRITEB_HI) |
+			   FW_WM(wm->pipe[PIPE_A].sprite[0] >> 8, SPRITEA_HI) |
+			   FW_WM(wm->pipe[PIPE_A].primary >> 8, PLANEA_HI));
 	} else {
 		I915_WRITE(DSPFW7,
-			   ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) & DSPFW_SPRITED_MASK) |
-			   ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) & DSPFW_SPRITEC_MASK));
+			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
+			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
 		I915_WRITE(DSPHOWM,
-			   (((wm->sr.plane >> 9) << DSPFW_SR_HI_SHIFT) & DSPFW_SR_HI_MASK) |
-			   (((wm->pipe[PIPE_B].sprite[1] >> 8) << DSPFW_SPRITED_HI_SHIFT) & DSPFW_SPRITED_HI_MASK) |
-			   (((wm->pipe[PIPE_B].sprite[0] >> 8) << DSPFW_SPRITEC_HI_SHIFT) & DSPFW_SPRITEC_HI_MASK) |
-			   (((wm->pipe[PIPE_B].primary >> 8) << DSPFW_PLANEB_HI_SHIFT) & DSPFW_PLANEB_HI_MASK) |
-			   (((wm->pipe[PIPE_A].sprite[1] >> 8) << DSPFW_SPRITEB_HI_SHIFT) & DSPFW_SPRITEB_HI_MASK) |
-			   (((wm->pipe[PIPE_A].sprite[0] >> 8) << DSPFW_SPRITEA_HI_SHIFT) & DSPFW_SPRITEA_HI_MASK) |
-			   (((wm->pipe[PIPE_A].primary >> 8) << DSPFW_PLANEA_HI_SHIFT) & DSPFW_PLANEA_HI_MASK));
+			   FW_WM(wm->sr.plane >> 9, SR_HI) |
+			   FW_WM(wm->pipe[PIPE_B].sprite[1] >> 8, SPRITED_HI) |
+			   FW_WM(wm->pipe[PIPE_B].sprite[0] >> 8, SPRITEC_HI) |
+			   FW_WM(wm->pipe[PIPE_B].primary >> 8, PLANEB_HI) |
+			   FW_WM(wm->pipe[PIPE_A].sprite[1] >> 8, SPRITEB_HI) |
+			   FW_WM(wm->pipe[PIPE_A].sprite[0] >> 8, SPRITEA_HI) |
+			   FW_WM(wm->pipe[PIPE_A].primary >> 8, PLANEA_HI));
 	}
 
 	POSTING_READ(DSPFW1);
@@ -899,6 +904,9 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
 	dev_priv->wm.vlv = *wm;
 }
 
+#undef FW_WM
+#undef FW_WM_VLV
+
 static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
 					 struct drm_plane *plane)
 {
-- 
2.0.5

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too
  2015-03-10 14:16 [PATCH] drm/i915: Add polish to VLV WM shift+mask operations ville.syrjala
@ 2015-03-10 15:02 ` ville.syrjala
  2015-03-10 21:50   ` Jesse Barnes
  2015-03-10 21:48 ` [PATCH] drm/i915: Add polish to VLV WM shift+mask operations Jesse Barnes
  2015-03-11 15:02 ` shuang.he
  2 siblings, 1 reply; 6+ messages in thread
From: ville.syrjala @ 2015-03-10 15:02 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the FW_WM() macro from the VLV wm code to polish up the wm
code for older gmch platforms.

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++--
 drivers/gpu/drm/i915/intel_pm.c | 42 +++++++++++++++++++++--------------------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8ff039d..793ed63 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4121,8 +4121,8 @@ enum skl_disp_power_wells {
 #define   DSPFW_SPRITEB_MASK_VLV	(0xff<<16) /* vlv/chv */
 #define   DSPFW_CURSORA_SHIFT		8
 #define   DSPFW_CURSORA_MASK		(0x3f<<8)
-#define   DSPFW_PLANEC_SHIFT_OLD	0
-#define   DSPFW_PLANEC_MASK_OLD		(0x7f<<0) /* pre-gen4 sprite C */
+#define   DSPFW_PLANEC_OLD_SHIFT	0
+#define   DSPFW_PLANEC_OLD_MASK		(0x7f<<0) /* pre-gen4 sprite C */
 #define   DSPFW_SPRITEA_SHIFT		0
 #define   DSPFW_SPRITEA_MASK		(0x7f<<0) /* g4x */
 #define   DSPFW_SPRITEA_MASK_VLV	(0xff<<0) /* vlv/chv */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8ac358d0..9ac9a2f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -301,6 +301,9 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+#define FW_WM(value, plane) \
+	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
+
 void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
 {
 	struct drm_device *dev = dev_priv->dev;
@@ -661,7 +664,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
 					pixel_size, latency->display_sr);
 		reg = I915_READ(DSPFW1);
 		reg &= ~DSPFW_SR_MASK;
-		reg |= wm << DSPFW_SR_SHIFT;
+		reg |= FW_WM(wm, SR);
 		I915_WRITE(DSPFW1, reg);
 		DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
 
@@ -671,7 +674,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
 					pixel_size, latency->cursor_sr);
 		reg = I915_READ(DSPFW3);
 		reg &= ~DSPFW_CURSOR_SR_MASK;
-		reg |= (wm & 0x3f) << DSPFW_CURSOR_SR_SHIFT;
+		reg |= FW_WM(wm, CURSOR_SR);
 		I915_WRITE(DSPFW3, reg);
 
 		/* Display HPLL off SR */
@@ -680,7 +683,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
 					pixel_size, latency->display_hpll_disable);
 		reg = I915_READ(DSPFW3);
 		reg &= ~DSPFW_HPLL_SR_MASK;
-		reg |= wm & DSPFW_HPLL_SR_MASK;
+		reg |= FW_WM(wm, HPLL_SR);
 		I915_WRITE(DSPFW3, reg);
 
 		/* cursor HPLL off SR */
@@ -689,7 +692,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
 					pixel_size, latency->cursor_hpll_disable);
 		reg = I915_READ(DSPFW3);
 		reg &= ~DSPFW_HPLL_CURSOR_MASK;
-		reg |= (wm & 0x3f) << DSPFW_HPLL_CURSOR_SHIFT;
+		reg |= FW_WM(wm, HPLL_CURSOR);
 		I915_WRITE(DSPFW3, reg);
 		DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
 
@@ -835,8 +838,6 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 			      display, cursor);
 }
 
-#define FW_WM(value, plane) \
-	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
 #define FW_WM_VLV(value, plane) \
 	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
 
@@ -904,7 +905,6 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
 	dev_priv->wm.vlv = *wm;
 }
 
-#undef FW_WM
 #undef FW_WM_VLV
 
 static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
@@ -1163,17 +1163,17 @@ static void g4x_update_wm(struct drm_crtc *crtc)
 		      plane_sr, cursor_sr);
 
 	I915_WRITE(DSPFW1,
-		   (plane_sr << DSPFW_SR_SHIFT) |
-		   (cursorb_wm << DSPFW_CURSORB_SHIFT) |
-		   (planeb_wm << DSPFW_PLANEB_SHIFT) |
-		   (planea_wm << DSPFW_PLANEA_SHIFT));
+		   FW_WM(plane_sr, SR) |
+		   FW_WM(cursorb_wm, CURSORB) |
+		   FW_WM(planeb_wm, PLANEB) |
+		   FW_WM(planea_wm, PLANEA));
 	I915_WRITE(DSPFW2,
 		   (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) |
-		   (cursora_wm << DSPFW_CURSORA_SHIFT));
+		   FW_WM(cursora_wm, CURSORA));
 	/* HPLL off in SR has some issues on G4x... disable it */
 	I915_WRITE(DSPFW3,
 		   (I915_READ(DSPFW3) & ~(DSPFW_HPLL_SR_EN | DSPFW_CURSOR_SR_MASK)) |
-		   (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
+		   FW_WM(cursor_sr, CURSOR_SR));
 
 	if (cxsr_enabled)
 		intel_set_memory_cxsr(dev_priv, true);
@@ -1239,19 +1239,21 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
 		      srwm);
 
 	/* 965 has limitations... */
-	I915_WRITE(DSPFW1, (srwm << DSPFW_SR_SHIFT) |
-		   (8 << DSPFW_CURSORB_SHIFT) |
-		   (8 << DSPFW_PLANEB_SHIFT) |
-		   (8 << DSPFW_PLANEA_SHIFT));
-	I915_WRITE(DSPFW2, (8 << DSPFW_CURSORA_SHIFT) |
-		   (8 << DSPFW_PLANEC_SHIFT_OLD));
+	I915_WRITE(DSPFW1, FW_WM(srwm, SR) |
+		   FW_WM(8, CURSORB) |
+		   FW_WM(8, PLANEB) |
+		   FW_WM(8, PLANEA));
+	I915_WRITE(DSPFW2, FW_WM(8, CURSORA) |
+		   FW_WM(8, PLANEC_OLD));
 	/* update cursor SR watermark */
-	I915_WRITE(DSPFW3, (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
+	I915_WRITE(DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
 
 	if (cxsr_enabled)
 		intel_set_memory_cxsr(dev_priv, true);
 }
 
+#undef FW_WM
+
 static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 {
 	struct drm_device *dev = unused_crtc->dev;
-- 
2.0.5

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add polish to VLV WM shift+mask operations
  2015-03-10 14:16 [PATCH] drm/i915: Add polish to VLV WM shift+mask operations ville.syrjala
  2015-03-10 15:02 ` [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too ville.syrjala
@ 2015-03-10 21:48 ` Jesse Barnes
  2015-03-11 15:02 ` shuang.he
  2 siblings, 0 replies; 6+ messages in thread
From: Jesse Barnes @ 2015-03-10 21:48 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On 03/10/2015 07:16 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Wrap the FW register value shift+mask operations into a macro to hide
> the ugliness a bit. Also might avoid bugs due to typos.
> 
> Also rename all the primary/sprite plane low order bit masks to have the
> _VLV suffix, so that we can use the FW_WM_VLV() macro instead of the
> FW_WM() macro for them in a consistent manner. Cursor and all the high
> order bits are left to use the FW_WM() macro as there's no real
> confusion with them.
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 10 +++---
>  drivers/gpu/drm/i915/intel_pm.c | 74 +++++++++++++++++++++++------------------
>  2 files changed, 46 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 495b22b..8ff039d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4161,25 +4161,25 @@ enum skl_disp_power_wells {
>  #define   DSPFW_SPRITED_WM1_SHIFT	24
>  #define   DSPFW_SPRITED_WM1_MASK	(0xff<<24)
>  #define   DSPFW_SPRITED_SHIFT		16
> -#define   DSPFW_SPRITED_MASK		(0xff<<16)
> +#define   DSPFW_SPRITED_MASK_VLV	(0xff<<16)
>  #define   DSPFW_SPRITEC_WM1_SHIFT	8
>  #define   DSPFW_SPRITEC_WM1_MASK	(0xff<<8)
>  #define   DSPFW_SPRITEC_SHIFT		0
> -#define   DSPFW_SPRITEC_MASK		(0xff<<0)
> +#define   DSPFW_SPRITEC_MASK_VLV	(0xff<<0)
>  #define DSPFW8_CHV		(VLV_DISPLAY_BASE + 0x700b8)
>  #define   DSPFW_SPRITEF_WM1_SHIFT	24
>  #define   DSPFW_SPRITEF_WM1_MASK	(0xff<<24)
>  #define   DSPFW_SPRITEF_SHIFT		16
> -#define   DSPFW_SPRITEF_MASK		(0xff<<16)
> +#define   DSPFW_SPRITEF_MASK_VLV	(0xff<<16)
>  #define   DSPFW_SPRITEE_WM1_SHIFT	8
>  #define   DSPFW_SPRITEE_WM1_MASK	(0xff<<8)
>  #define   DSPFW_SPRITEE_SHIFT		0
> -#define   DSPFW_SPRITEE_MASK		(0xff<<0)
> +#define   DSPFW_SPRITEE_MASK_VLV	(0xff<<0)
>  #define DSPFW9_CHV		(VLV_DISPLAY_BASE + 0x7007c) /* wtf #2? */
>  #define   DSPFW_PLANEC_WM1_SHIFT	24
>  #define   DSPFW_PLANEC_WM1_MASK		(0xff<<24)
>  #define   DSPFW_PLANEC_SHIFT		16
> -#define   DSPFW_PLANEC_MASK		(0xff<<16)
> +#define   DSPFW_PLANEC_MASK_VLV		(0xff<<16)
>  #define   DSPFW_CURSORC_WM1_SHIFT	8
>  #define   DSPFW_CURSORC_WM1_MASK	(0x3f<<16)
>  #define   DSPFW_CURSORC_SHIFT		0
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0e84558..8ac358d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -835,6 +835,11 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  			      display, cursor);
>  }
>  
> +#define FW_WM(value, plane) \
> +	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> +#define FW_WM_VLV(value, plane) \
> +	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
> +
>  static void vlv_write_wm_values(struct intel_crtc *crtc,
>  				const struct vlv_wm_values *wm)
>  {
> @@ -848,50 +853,50 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>  		   (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
>  
>  	I915_WRITE(DSPFW1,
> -		   ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> -		   ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & DSPFW_CURSORB_MASK) |
> -		   ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & DSPFW_PLANEB_MASK_VLV) |
> -		   ((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & DSPFW_PLANEA_MASK_VLV));
> +		   FW_WM(wm->sr.plane, SR) |
> +		   FW_WM(wm->pipe[PIPE_B].cursor, CURSORB) |
> +		   FW_WM_VLV(wm->pipe[PIPE_B].primary, PLANEB) |
> +		   FW_WM_VLV(wm->pipe[PIPE_A].primary, PLANEA));
>  	I915_WRITE(DSPFW2,
> -		   ((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & DSPFW_SPRITEB_MASK_VLV) |
> -		   ((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) & DSPFW_CURSORA_MASK) |
> -		   ((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) & DSPFW_SPRITEA_MASK_VLV));
> +		   FW_WM_VLV(wm->pipe[PIPE_A].sprite[1], SPRITEB) |
> +		   FW_WM(wm->pipe[PIPE_A].cursor, CURSORA) |
> +		   FW_WM_VLV(wm->pipe[PIPE_A].sprite[0], SPRITEA));
>  	I915_WRITE(DSPFW3,
> -		   ((wm->sr.cursor << DSPFW_CURSOR_SR_SHIFT) & DSPFW_CURSOR_SR_MASK));
> +		   FW_WM(wm->sr.cursor, CURSOR_SR));
>  
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		I915_WRITE(DSPFW7_CHV,
> -			   ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) & DSPFW_SPRITED_MASK) |
> -			   ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) & DSPFW_SPRITEC_MASK));
> +			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
> +			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
>  		I915_WRITE(DSPFW8_CHV,
> -			   ((wm->pipe[PIPE_C].sprite[1] << DSPFW_SPRITEF_SHIFT) & DSPFW_SPRITEF_MASK) |
> -			   ((wm->pipe[PIPE_C].sprite[0] << DSPFW_SPRITEE_SHIFT) & DSPFW_SPRITEE_MASK));
> +			   FW_WM_VLV(wm->pipe[PIPE_C].sprite[1], SPRITEF) |
> +			   FW_WM_VLV(wm->pipe[PIPE_C].sprite[0], SPRITEE));
>  		I915_WRITE(DSPFW9_CHV,
> -			   ((wm->pipe[PIPE_C].primary << DSPFW_PLANEC_SHIFT) & DSPFW_PLANEC_MASK) |
> -			   ((wm->pipe[PIPE_C].cursor << DSPFW_CURSORC_SHIFT) & DSPFW_CURSORC_MASK));
> +			   FW_WM_VLV(wm->pipe[PIPE_C].primary, PLANEC) |
> +			   FW_WM(wm->pipe[PIPE_C].cursor, CURSORC));
>  		I915_WRITE(DSPHOWM,
> -			   (((wm->sr.plane >> 9) << DSPFW_SR_HI_SHIFT) & DSPFW_SR_HI_MASK) |
> -			   (((wm->pipe[PIPE_C].sprite[1] >> 8) << DSPFW_SPRITEF_HI_SHIFT) & DSPFW_SPRITEF_HI_MASK) |
> -			   (((wm->pipe[PIPE_C].sprite[0] >> 8) << DSPFW_SPRITEE_HI_SHIFT) & DSPFW_SPRITEE_HI_MASK) |
> -			   (((wm->pipe[PIPE_C].primary >> 8) << DSPFW_PLANEC_HI_SHIFT) & DSPFW_PLANEC_HI_MASK) |
> -			   (((wm->pipe[PIPE_B].sprite[1] >> 8) << DSPFW_SPRITED_HI_SHIFT) & DSPFW_SPRITED_HI_MASK) |
> -			   (((wm->pipe[PIPE_B].sprite[0] >> 8) << DSPFW_SPRITEC_HI_SHIFT) & DSPFW_SPRITEC_HI_MASK) |
> -			   (((wm->pipe[PIPE_B].primary >> 8) << DSPFW_PLANEB_HI_SHIFT) & DSPFW_PLANEB_HI_MASK) |
> -			   (((wm->pipe[PIPE_A].sprite[1] >> 8) << DSPFW_SPRITEB_HI_SHIFT) & DSPFW_SPRITEB_HI_MASK) |
> -			   (((wm->pipe[PIPE_A].sprite[0] >> 8) << DSPFW_SPRITEA_HI_SHIFT) & DSPFW_SPRITEA_HI_MASK) |
> -			   (((wm->pipe[PIPE_A].primary >> 8) << DSPFW_PLANEA_HI_SHIFT) & DSPFW_PLANEA_HI_MASK));
> +			   FW_WM(wm->sr.plane >> 9, SR_HI) |
> +			   FW_WM(wm->pipe[PIPE_C].sprite[1] >> 8, SPRITEF_HI) |
> +			   FW_WM(wm->pipe[PIPE_C].sprite[0] >> 8, SPRITEE_HI) |
> +			   FW_WM(wm->pipe[PIPE_C].primary >> 8, PLANEC_HI) |
> +			   FW_WM(wm->pipe[PIPE_B].sprite[1] >> 8, SPRITED_HI) |
> +			   FW_WM(wm->pipe[PIPE_B].sprite[0] >> 8, SPRITEC_HI) |
> +			   FW_WM(wm->pipe[PIPE_B].primary >> 8, PLANEB_HI) |
> +			   FW_WM(wm->pipe[PIPE_A].sprite[1] >> 8, SPRITEB_HI) |
> +			   FW_WM(wm->pipe[PIPE_A].sprite[0] >> 8, SPRITEA_HI) |
> +			   FW_WM(wm->pipe[PIPE_A].primary >> 8, PLANEA_HI));
>  	} else {
>  		I915_WRITE(DSPFW7,
> -			   ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) & DSPFW_SPRITED_MASK) |
> -			   ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) & DSPFW_SPRITEC_MASK));
> +			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
> +			   FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
>  		I915_WRITE(DSPHOWM,
> -			   (((wm->sr.plane >> 9) << DSPFW_SR_HI_SHIFT) & DSPFW_SR_HI_MASK) |
> -			   (((wm->pipe[PIPE_B].sprite[1] >> 8) << DSPFW_SPRITED_HI_SHIFT) & DSPFW_SPRITED_HI_MASK) |
> -			   (((wm->pipe[PIPE_B].sprite[0] >> 8) << DSPFW_SPRITEC_HI_SHIFT) & DSPFW_SPRITEC_HI_MASK) |
> -			   (((wm->pipe[PIPE_B].primary >> 8) << DSPFW_PLANEB_HI_SHIFT) & DSPFW_PLANEB_HI_MASK) |
> -			   (((wm->pipe[PIPE_A].sprite[1] >> 8) << DSPFW_SPRITEB_HI_SHIFT) & DSPFW_SPRITEB_HI_MASK) |
> -			   (((wm->pipe[PIPE_A].sprite[0] >> 8) << DSPFW_SPRITEA_HI_SHIFT) & DSPFW_SPRITEA_HI_MASK) |
> -			   (((wm->pipe[PIPE_A].primary >> 8) << DSPFW_PLANEA_HI_SHIFT) & DSPFW_PLANEA_HI_MASK));
> +			   FW_WM(wm->sr.plane >> 9, SR_HI) |
> +			   FW_WM(wm->pipe[PIPE_B].sprite[1] >> 8, SPRITED_HI) |
> +			   FW_WM(wm->pipe[PIPE_B].sprite[0] >> 8, SPRITEC_HI) |
> +			   FW_WM(wm->pipe[PIPE_B].primary >> 8, PLANEB_HI) |
> +			   FW_WM(wm->pipe[PIPE_A].sprite[1] >> 8, SPRITEB_HI) |
> +			   FW_WM(wm->pipe[PIPE_A].sprite[0] >> 8, SPRITEA_HI) |
> +			   FW_WM(wm->pipe[PIPE_A].primary >> 8, PLANEA_HI));
>  	}
>  
>  	POSTING_READ(DSPFW1);
> @@ -899,6 +904,9 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>  	dev_priv->wm.vlv = *wm;
>  }
>  
> +#undef FW_WM
> +#undef FW_WM_VLV
> +
>  static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
>  					 struct drm_plane *plane)
>  {
> 

I'd rather see the macros put in the header next to the FW definitions,
but that's not a blocker.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too
  2015-03-10 15:02 ` [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too ville.syrjala
@ 2015-03-10 21:50   ` Jesse Barnes
  2015-03-11 10:05     ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2015-03-10 21:50 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On 03/10/2015 08:02 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the FW_WM() macro from the VLV wm code to polish up the wm
> code for older gmch platforms.
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
>  drivers/gpu/drm/i915/intel_pm.c | 42 +++++++++++++++++++++--------------------
>  2 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8ff039d..793ed63 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4121,8 +4121,8 @@ enum skl_disp_power_wells {
>  #define   DSPFW_SPRITEB_MASK_VLV	(0xff<<16) /* vlv/chv */
>  #define   DSPFW_CURSORA_SHIFT		8
>  #define   DSPFW_CURSORA_MASK		(0x3f<<8)
> -#define   DSPFW_PLANEC_SHIFT_OLD	0
> -#define   DSPFW_PLANEC_MASK_OLD		(0x7f<<0) /* pre-gen4 sprite C */
> +#define   DSPFW_PLANEC_OLD_SHIFT	0
> +#define   DSPFW_PLANEC_OLD_MASK		(0x7f<<0) /* pre-gen4 sprite C */
>  #define   DSPFW_SPRITEA_SHIFT		0
>  #define   DSPFW_SPRITEA_MASK		(0x7f<<0) /* g4x */
>  #define   DSPFW_SPRITEA_MASK_VLV	(0xff<<0) /* vlv/chv */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8ac358d0..9ac9a2f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -301,6 +301,9 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +#define FW_WM(value, plane) \
> +	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> +
>  void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -661,7 +664,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
>  					pixel_size, latency->display_sr);
>  		reg = I915_READ(DSPFW1);
>  		reg &= ~DSPFW_SR_MASK;
> -		reg |= wm << DSPFW_SR_SHIFT;
> +		reg |= FW_WM(wm, SR);
>  		I915_WRITE(DSPFW1, reg);
>  		DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
>  
> @@ -671,7 +674,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
>  					pixel_size, latency->cursor_sr);
>  		reg = I915_READ(DSPFW3);
>  		reg &= ~DSPFW_CURSOR_SR_MASK;
> -		reg |= (wm & 0x3f) << DSPFW_CURSOR_SR_SHIFT;
> +		reg |= FW_WM(wm, CURSOR_SR);
>  		I915_WRITE(DSPFW3, reg);
>  
>  		/* Display HPLL off SR */
> @@ -680,7 +683,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
>  					pixel_size, latency->display_hpll_disable);
>  		reg = I915_READ(DSPFW3);
>  		reg &= ~DSPFW_HPLL_SR_MASK;
> -		reg |= wm & DSPFW_HPLL_SR_MASK;
> +		reg |= FW_WM(wm, HPLL_SR);
>  		I915_WRITE(DSPFW3, reg);
>  
>  		/* cursor HPLL off SR */
> @@ -689,7 +692,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
>  					pixel_size, latency->cursor_hpll_disable);
>  		reg = I915_READ(DSPFW3);
>  		reg &= ~DSPFW_HPLL_CURSOR_MASK;
> -		reg |= (wm & 0x3f) << DSPFW_HPLL_CURSOR_SHIFT;
> +		reg |= FW_WM(wm, HPLL_CURSOR);
>  		I915_WRITE(DSPFW3, reg);
>  		DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
>  
> @@ -835,8 +838,6 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  			      display, cursor);
>  }
>  
> -#define FW_WM(value, plane) \
> -	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
>  #define FW_WM_VLV(value, plane) \
>  	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
>  
> @@ -904,7 +905,6 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>  	dev_priv->wm.vlv = *wm;
>  }
>  
> -#undef FW_WM
>  #undef FW_WM_VLV
>  
>  static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
> @@ -1163,17 +1163,17 @@ static void g4x_update_wm(struct drm_crtc *crtc)
>  		      plane_sr, cursor_sr);
>  
>  	I915_WRITE(DSPFW1,
> -		   (plane_sr << DSPFW_SR_SHIFT) |
> -		   (cursorb_wm << DSPFW_CURSORB_SHIFT) |
> -		   (planeb_wm << DSPFW_PLANEB_SHIFT) |
> -		   (planea_wm << DSPFW_PLANEA_SHIFT));
> +		   FW_WM(plane_sr, SR) |
> +		   FW_WM(cursorb_wm, CURSORB) |
> +		   FW_WM(planeb_wm, PLANEB) |
> +		   FW_WM(planea_wm, PLANEA));
>  	I915_WRITE(DSPFW2,
>  		   (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) |
> -		   (cursora_wm << DSPFW_CURSORA_SHIFT));
> +		   FW_WM(cursora_wm, CURSORA));
>  	/* HPLL off in SR has some issues on G4x... disable it */
>  	I915_WRITE(DSPFW3,
>  		   (I915_READ(DSPFW3) & ~(DSPFW_HPLL_SR_EN | DSPFW_CURSOR_SR_MASK)) |
> -		   (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
> +		   FW_WM(cursor_sr, CURSOR_SR));
>  
>  	if (cxsr_enabled)
>  		intel_set_memory_cxsr(dev_priv, true);
> @@ -1239,19 +1239,21 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
>  		      srwm);
>  
>  	/* 965 has limitations... */
> -	I915_WRITE(DSPFW1, (srwm << DSPFW_SR_SHIFT) |
> -		   (8 << DSPFW_CURSORB_SHIFT) |
> -		   (8 << DSPFW_PLANEB_SHIFT) |
> -		   (8 << DSPFW_PLANEA_SHIFT));
> -	I915_WRITE(DSPFW2, (8 << DSPFW_CURSORA_SHIFT) |
> -		   (8 << DSPFW_PLANEC_SHIFT_OLD));
> +	I915_WRITE(DSPFW1, FW_WM(srwm, SR) |
> +		   FW_WM(8, CURSORB) |
> +		   FW_WM(8, PLANEB) |
> +		   FW_WM(8, PLANEA));
> +	I915_WRITE(DSPFW2, FW_WM(8, CURSORA) |
> +		   FW_WM(8, PLANEC_OLD));
>  	/* update cursor SR watermark */
> -	I915_WRITE(DSPFW3, (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
> +	I915_WRITE(DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
>  
>  	if (cxsr_enabled)
>  		intel_set_memory_cxsr(dev_priv, true);
>  }
>  
> +#undef FW_WM
> +
>  static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>  {
>  	struct drm_device *dev = unused_crtc->dev;
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too
  2015-03-10 21:50   ` Jesse Barnes
@ 2015-03-11 10:05     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-03-11 10:05 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Tue, Mar 10, 2015 at 02:50:16PM -0700, Jesse Barnes wrote:
> On 03/10/2015 08:02 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use the FW_WM() macro from the VLV wm code to polish up the wm
> > code for older gmch platforms.
> > 
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
> >  drivers/gpu/drm/i915/intel_pm.c | 42 +++++++++++++++++++++--------------------
> >  2 files changed, 24 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8ff039d..793ed63 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4121,8 +4121,8 @@ enum skl_disp_power_wells {
> >  #define   DSPFW_SPRITEB_MASK_VLV	(0xff<<16) /* vlv/chv */
> >  #define   DSPFW_CURSORA_SHIFT		8
> >  #define   DSPFW_CURSORA_MASK		(0x3f<<8)
> > -#define   DSPFW_PLANEC_SHIFT_OLD	0
> > -#define   DSPFW_PLANEC_MASK_OLD		(0x7f<<0) /* pre-gen4 sprite C */
> > +#define   DSPFW_PLANEC_OLD_SHIFT	0
> > +#define   DSPFW_PLANEC_OLD_MASK		(0x7f<<0) /* pre-gen4 sprite C */
> >  #define   DSPFW_SPRITEA_SHIFT		0
> >  #define   DSPFW_SPRITEA_MASK		(0x7f<<0) /* g4x */
> >  #define   DSPFW_SPRITEA_MASK_VLV	(0xff<<0) /* vlv/chv */
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 8ac358d0..9ac9a2f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -301,6 +301,9 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  }
> >  
> > +#define FW_WM(value, plane) \
> > +	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> > +
> >  void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
> >  {
> >  	struct drm_device *dev = dev_priv->dev;
> > @@ -661,7 +664,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
> >  					pixel_size, latency->display_sr);
> >  		reg = I915_READ(DSPFW1);
> >  		reg &= ~DSPFW_SR_MASK;
> > -		reg |= wm << DSPFW_SR_SHIFT;
> > +		reg |= FW_WM(wm, SR);
> >  		I915_WRITE(DSPFW1, reg);
> >  		DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
> >  
> > @@ -671,7 +674,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
> >  					pixel_size, latency->cursor_sr);
> >  		reg = I915_READ(DSPFW3);
> >  		reg &= ~DSPFW_CURSOR_SR_MASK;
> > -		reg |= (wm & 0x3f) << DSPFW_CURSOR_SR_SHIFT;
> > +		reg |= FW_WM(wm, CURSOR_SR);
> >  		I915_WRITE(DSPFW3, reg);
> >  
> >  		/* Display HPLL off SR */
> > @@ -680,7 +683,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
> >  					pixel_size, latency->display_hpll_disable);
> >  		reg = I915_READ(DSPFW3);
> >  		reg &= ~DSPFW_HPLL_SR_MASK;
> > -		reg |= wm & DSPFW_HPLL_SR_MASK;
> > +		reg |= FW_WM(wm, HPLL_SR);
> >  		I915_WRITE(DSPFW3, reg);
> >  
> >  		/* cursor HPLL off SR */
> > @@ -689,7 +692,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
> >  					pixel_size, latency->cursor_hpll_disable);
> >  		reg = I915_READ(DSPFW3);
> >  		reg &= ~DSPFW_HPLL_CURSOR_MASK;
> > -		reg |= (wm & 0x3f) << DSPFW_HPLL_CURSOR_SHIFT;
> > +		reg |= FW_WM(wm, HPLL_CURSOR);
> >  		I915_WRITE(DSPFW3, reg);
> >  		DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
> >  
> > @@ -835,8 +838,6 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> >  			      display, cursor);
> >  }
> >  
> > -#define FW_WM(value, plane) \
> > -	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> >  #define FW_WM_VLV(value, plane) \
> >  	(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
> >  
> > @@ -904,7 +905,6 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> >  	dev_priv->wm.vlv = *wm;
> >  }
> >  
> > -#undef FW_WM
> >  #undef FW_WM_VLV
> >  
> >  static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
> > @@ -1163,17 +1163,17 @@ static void g4x_update_wm(struct drm_crtc *crtc)
> >  		      plane_sr, cursor_sr);
> >  
> >  	I915_WRITE(DSPFW1,
> > -		   (plane_sr << DSPFW_SR_SHIFT) |
> > -		   (cursorb_wm << DSPFW_CURSORB_SHIFT) |
> > -		   (planeb_wm << DSPFW_PLANEB_SHIFT) |
> > -		   (planea_wm << DSPFW_PLANEA_SHIFT));
> > +		   FW_WM(plane_sr, SR) |
> > +		   FW_WM(cursorb_wm, CURSORB) |
> > +		   FW_WM(planeb_wm, PLANEB) |
> > +		   FW_WM(planea_wm, PLANEA));
> >  	I915_WRITE(DSPFW2,
> >  		   (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) |
> > -		   (cursora_wm << DSPFW_CURSORA_SHIFT));
> > +		   FW_WM(cursora_wm, CURSORA));
> >  	/* HPLL off in SR has some issues on G4x... disable it */
> >  	I915_WRITE(DSPFW3,
> >  		   (I915_READ(DSPFW3) & ~(DSPFW_HPLL_SR_EN | DSPFW_CURSOR_SR_MASK)) |
> > -		   (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
> > +		   FW_WM(cursor_sr, CURSOR_SR));
> >  
> >  	if (cxsr_enabled)
> >  		intel_set_memory_cxsr(dev_priv, true);
> > @@ -1239,19 +1239,21 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
> >  		      srwm);
> >  
> >  	/* 965 has limitations... */
> > -	I915_WRITE(DSPFW1, (srwm << DSPFW_SR_SHIFT) |
> > -		   (8 << DSPFW_CURSORB_SHIFT) |
> > -		   (8 << DSPFW_PLANEB_SHIFT) |
> > -		   (8 << DSPFW_PLANEA_SHIFT));
> > -	I915_WRITE(DSPFW2, (8 << DSPFW_CURSORA_SHIFT) |
> > -		   (8 << DSPFW_PLANEC_SHIFT_OLD));
> > +	I915_WRITE(DSPFW1, FW_WM(srwm, SR) |
> > +		   FW_WM(8, CURSORB) |
> > +		   FW_WM(8, PLANEB) |
> > +		   FW_WM(8, PLANEA));
> > +	I915_WRITE(DSPFW2, FW_WM(8, CURSORA) |
> > +		   FW_WM(8, PLANEC_OLD));
> >  	/* update cursor SR watermark */
> > -	I915_WRITE(DSPFW3, (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
> > +	I915_WRITE(DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
> >  
> >  	if (cxsr_enabled)
> >  		intel_set_memory_cxsr(dev_priv, true);
> >  }
> >  
> > +#undef FW_WM
> > +
> >  static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> >  {
> >  	struct drm_device *dev = unused_crtc->dev;
> > 
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Both merged, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add polish to VLV WM shift+mask operations
  2015-03-10 14:16 [PATCH] drm/i915: Add polish to VLV WM shift+mask operations ville.syrjala
  2015-03-10 15:02 ` [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too ville.syrjala
  2015-03-10 21:48 ` [PATCH] drm/i915: Add polish to VLV WM shift+mask operations Jesse Barnes
@ 2015-03-11 15:02 ` shuang.he
  2 siblings, 0 replies; 6+ messages in thread
From: shuang.he @ 2015-03-11 15:02 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, ville.syrjala

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5926
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -3              281/281              278/281
ILK                                  308/308              308/308
SNB                 -1              284/284              283/284
IVB                 -1              375/375              374/375
BYT                                  294/294              294/294
HSW                                  384/384              384/384
BDW                                  315/315              315/315
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_render_mixed_blits      PASS(1)      FAIL(2)
 PNV  igt_gen3_render_tiledx_blits      FAIL(1)PASS(1)      FAIL(1)
 PNV  igt_gen3_render_tiledy_blits      FAIL(1)PASS(1)      FAIL(1)
*SNB  igt_gem_flink_bad-flink      PASS(1)      DMESG_WARN(1)PASS(1)
*IVB  igt_gem_storedw_batches_loop_normal      PASS(1)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-03-11 15:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-10 14:16 [PATCH] drm/i915: Add polish to VLV WM shift+mask operations ville.syrjala
2015-03-10 15:02 ` [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too ville.syrjala
2015-03-10 21:50   ` Jesse Barnes
2015-03-11 10:05     ` Daniel Vetter
2015-03-10 21:48 ` [PATCH] drm/i915: Add polish to VLV WM shift+mask operations Jesse Barnes
2015-03-11 15:02 ` shuang.he

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox