[Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

uncore->lock only protects the forcewake domain itself,
not the register accesses.

uncore's _fw alternatives are for cases where the domains
are not needed because we are sure that they are already
awake.

So the move towards the uncore's _fw alternatives seems
right, however using the uncore-lock to protect the dsparb
registers seems an abuse of the uncore-lock.

Let's restore the previous individual lock and try to get
rid of the direct uncore accesses from the display code.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
 drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
 drivers/gpu/drm/i915/i915_driver.c                |  1 +
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
index caef72d38798..8fe0b5c63d3a 100644
--- a/drivers/gpu/drm/i915/display/i9xx_wm.c
+++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
@@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
 
-	/*
-	 * uncore.lock serves a double purpose here. It allows us to
-	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
-	 * it protects the DSPARB registers from getting clobbered by
-	 * parallel updates from multiple pipes.
-	 *
-	 * intel_pipe_update_start() has already disabled interrupts
-	 * for us, so a plain spin_lock() is sufficient here.
-	 */
-	spin_lock(&uncore->lock);
+	spin_lock(&dev_priv->display.wm.dsparb_lock);
 
 	switch (crtc->pipe) {
 	case PIPE_A:
@@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	intel_uncore_posting_read_fw(uncore, DSPARB);
 
-	spin_unlock(&uncore->lock);
+	spin_unlock(&dev_priv->display.wm.dsparb_lock);
 }
 
 #undef VLV_FIFO
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index fdab7bb93a7d..68c6bfb91dbe 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -253,6 +253,9 @@ struct intel_wm {
 	 */
 	struct mutex wm_mutex;
 
+	/* protects DSPARB registers on pre-g4x/vlv/chv */
+	spinlock_t dsparb_lock;
+
 	bool ipc_enabled;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index a53fd339e2cc..c78e36444a12 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->display.pps.mutex);
 	mutex_init(&dev_priv->display.hdcp.comp_mutex);
 	spin_lock_init(&dev_priv->display.dkl.phy_lock);
+	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
 
 	i915_memcpy_init_early(dev_priv);
 	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
-- 
2.39.2

[Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

[Rodrigo Vivi]

Let's get rid of the intel_uncore calls on display side.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/i9xx_wm.c  | 238 ++++++++++++------------
 drivers/gpu/drm/i915/display/intel_de.h |  12 ++
 2 files changed, 131 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
index 8fe0b5c63d3a..28a9a920a2bc 100644
--- a/drivers/gpu/drm/i915/display/i9xx_wm.c
+++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
@@ -6,6 +6,7 @@
 #include "i915_drv.h"
 #include "i9xx_wm.h"
 #include "intel_atomic.h"
+#include "intel_de.h"
 #include "intel_display.h"
 #include "intel_display_trace.h"
 #include "intel_mchbar_regs.h"
@@ -141,39 +142,39 @@ static bool _intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enabl
 	u32 val;
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		was_enabled = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
-		intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
-		intel_uncore_posting_read(&dev_priv->uncore, FW_BLC_SELF_VLV);
+		was_enabled = intel_de_read(dev_priv, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
+		intel_de_write(dev_priv, FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
+		intel_de_posting_read(dev_priv, FW_BLC_SELF_VLV);
 	} else if (IS_G4X(dev_priv) || IS_I965GM(dev_priv)) {
-		was_enabled = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF) & FW_BLC_SELF_EN;
-		intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
-		intel_uncore_posting_read(&dev_priv->uncore, FW_BLC_SELF);
+		was_enabled = intel_de_read(dev_priv, FW_BLC_SELF) & FW_BLC_SELF_EN;
+		intel_de_write(dev_priv, FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
+		intel_de_posting_read(dev_priv, FW_BLC_SELF);
 	} else if (IS_PINEVIEW(dev_priv)) {
-		val = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+		val = intel_de_read(dev_priv, DSPFW3);
 		was_enabled = val & PINEVIEW_SELF_REFRESH_EN;
 		if (enable)
 			val |= PINEVIEW_SELF_REFRESH_EN;
 		else
 			val &= ~PINEVIEW_SELF_REFRESH_EN;
-		intel_uncore_write(&dev_priv->uncore, DSPFW3, val);
-		intel_uncore_posting_read(&dev_priv->uncore, DSPFW3);
+		intel_de_write(dev_priv, DSPFW3, val);
+		intel_de_posting_read(dev_priv, DSPFW3);
 	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv)) {
-		was_enabled = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF) & FW_BLC_SELF_EN;
+		was_enabled = intel_de_read(dev_priv, FW_BLC_SELF) & FW_BLC_SELF_EN;
 		val = enable ? _MASKED_BIT_ENABLE(FW_BLC_SELF_EN) :
 			       _MASKED_BIT_DISABLE(FW_BLC_SELF_EN);
-		intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF, val);
-		intel_uncore_posting_read(&dev_priv->uncore, FW_BLC_SELF);
+		intel_de_write(dev_priv, FW_BLC_SELF, val);
+		intel_de_posting_read(dev_priv, FW_BLC_SELF);
 	} else if (IS_I915GM(dev_priv)) {
 		/*
 		 * FIXME can't find a bit like this for 915G, and
 		 * yet it does have the related watermark in
 		 * FW_BLC_SELF. What's going on?
 		 */
-		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
+		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
 		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
 			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
-		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
-		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
+		intel_de_write(dev_priv, INSTPM, val);
+		intel_de_posting_read(dev_priv, INSTPM);
 	} else {
 		return false;
 	}
@@ -269,20 +270,20 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
 
 	switch (pipe) {
 	case PIPE_A:
-		dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
-		dsparb2 = intel_uncore_read(&dev_priv->uncore, DSPARB2);
+		dsparb = intel_de_read(dev_priv, DSPARB);
+		dsparb2 = intel_de_read(dev_priv, DSPARB2);
 		sprite0_start = VLV_FIFO_START(dsparb, dsparb2, 0, 0);
 		sprite1_start = VLV_FIFO_START(dsparb, dsparb2, 8, 4);
 		break;
 	case PIPE_B:
-		dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
-		dsparb2 = intel_uncore_read(&dev_priv->uncore, DSPARB2);
+		dsparb = intel_de_read(dev_priv, DSPARB);
+		dsparb2 = intel_de_read(dev_priv, DSPARB2);
 		sprite0_start = VLV_FIFO_START(dsparb, dsparb2, 16, 8);
 		sprite1_start = VLV_FIFO_START(dsparb, dsparb2, 24, 12);
 		break;
 	case PIPE_C:
-		dsparb2 = intel_uncore_read(&dev_priv->uncore, DSPARB2);
-		dsparb3 = intel_uncore_read(&dev_priv->uncore, DSPARB3);
+		dsparb2 = intel_de_read(dev_priv, DSPARB2);
+		dsparb3 = intel_de_read(dev_priv, DSPARB3);
 		sprite0_start = VLV_FIFO_START(dsparb3, dsparb2, 0, 16);
 		sprite1_start = VLV_FIFO_START(dsparb3, dsparb2, 8, 20);
 		break;
@@ -300,7 +301,7 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
 static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv,
 			      enum i9xx_plane_id i9xx_plane)
 {
-	u32 dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
+	u32 dsparb = intel_de_read(dev_priv, DSPARB);
 	int size;
 
 	size = dsparb & 0x7f;
@@ -316,7 +317,7 @@ static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv,
 static int i830_get_fifo_size(struct drm_i915_private *dev_priv,
 			      enum i9xx_plane_id i9xx_plane)
 {
-	u32 dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
+	u32 dsparb = intel_de_read(dev_priv, DSPARB);
 	int size;
 
 	size = dsparb & 0x1ff;
@@ -333,7 +334,7 @@ static int i830_get_fifo_size(struct drm_i915_private *dev_priv,
 static int i845_get_fifo_size(struct drm_i915_private *dev_priv,
 			      enum i9xx_plane_id i9xx_plane)
 {
-	u32 dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
+	u32 dsparb = intel_de_read(dev_priv, DSPARB);
 	int size;
 
 	size = dsparb & 0x7f;
@@ -655,33 +656,33 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
 		wm = intel_calculate_wm(pixel_rate, &pnv_display_wm,
 					pnv_display_wm.fifo_size,
 					cpp, latency->display_sr);
-		reg = intel_uncore_read(&dev_priv->uncore, DSPFW1);
+		reg = intel_de_read(dev_priv, DSPFW1);
 		reg &= ~DSPFW_SR_MASK;
 		reg |= FW_WM(wm, SR);
-		intel_uncore_write(&dev_priv->uncore, DSPFW1, reg);
+		intel_de_write(dev_priv, DSPFW1, reg);
 		drm_dbg_kms(&dev_priv->drm, "DSPFW1 register is %x\n", reg);
 
 		/* cursor SR */
 		wm = intel_calculate_wm(pixel_rate, &pnv_cursor_wm,
 					pnv_display_wm.fifo_size,
 					4, latency->cursor_sr);
-		intel_uncore_rmw(&dev_priv->uncore, DSPFW3, DSPFW_CURSOR_SR_MASK,
+		intel_de_rmw(dev_priv, DSPFW3, DSPFW_CURSOR_SR_MASK,
 				 FW_WM(wm, CURSOR_SR));
 
 		/* Display HPLL off SR */
 		wm = intel_calculate_wm(pixel_rate, &pnv_display_hplloff_wm,
 					pnv_display_hplloff_wm.fifo_size,
 					cpp, latency->display_hpll_disable);
-		intel_uncore_rmw(&dev_priv->uncore, DSPFW3, DSPFW_HPLL_SR_MASK, FW_WM(wm, HPLL_SR));
+		intel_de_rmw(dev_priv, DSPFW3, DSPFW_HPLL_SR_MASK, FW_WM(wm, HPLL_SR));
 
 		/* cursor HPLL off SR */
 		wm = intel_calculate_wm(pixel_rate, &pnv_cursor_hplloff_wm,
 					pnv_display_hplloff_wm.fifo_size,
 					4, latency->cursor_hpll_disable);
-		reg = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+		reg = intel_de_read(dev_priv, DSPFW3);
 		reg &= ~DSPFW_HPLL_CURSOR_MASK;
 		reg |= FW_WM(wm, HPLL_CURSOR);
-		intel_uncore_write(&dev_priv->uncore, DSPFW3, reg);
+		intel_de_write(dev_priv, DSPFW3, reg);
 		drm_dbg_kms(&dev_priv->drm, "DSPFW3 register is %x\n", reg);
 
 		intel_set_memory_cxsr(dev_priv, true);
@@ -715,25 +716,25 @@ static void g4x_write_wm_values(struct drm_i915_private *dev_priv,
 	for_each_pipe(dev_priv, pipe)
 		trace_g4x_wm(intel_crtc_for_pipe(dev_priv, pipe), wm);
 
-	intel_uncore_write(&dev_priv->uncore, DSPFW1,
+	intel_de_write(dev_priv, DSPFW1,
 			   FW_WM(wm->sr.plane, SR) |
 			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
 			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY], PLANEB) |
 			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY], PLANEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW2,
+	intel_de_write(dev_priv, DSPFW2,
 			   (wm->fbc_en ? DSPFW_FBC_SR_EN : 0) |
 			   FW_WM(wm->sr.fbc, FBC_SR) |
 			   FW_WM(wm->hpll.fbc, FBC_HPLL_SR) |
 			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEB) |
 			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_CURSOR], CURSORA) |
 			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0], SPRITEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW3,
+	intel_de_write(dev_priv, DSPFW3,
 			   (wm->hpll_en ? DSPFW_HPLL_SR_EN : 0) |
 			   FW_WM(wm->sr.cursor, CURSOR_SR) |
 			   FW_WM(wm->hpll.cursor, HPLL_CURSOR) |
 			   FW_WM(wm->hpll.plane, HPLL_SR));
 
-	intel_uncore_posting_read(&dev_priv->uncore, DSPFW1);
+	intel_de_posting_read(dev_priv, DSPFW1);
 }
 
 #define FW_WM_VLV(value, plane) \
@@ -747,7 +748,7 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 	for_each_pipe(dev_priv, pipe) {
 		trace_vlv_wm(intel_crtc_for_pipe(dev_priv, pipe), wm);
 
-		intel_uncore_write(&dev_priv->uncore, VLV_DDL(pipe),
+		intel_de_write(dev_priv, VLV_DDL(pipe),
 				   (wm->ddl[pipe].plane[PLANE_CURSOR] << DDL_CURSOR_SHIFT) |
 				   (wm->ddl[pipe].plane[PLANE_SPRITE1] << DDL_SPRITE_SHIFT(1)) |
 				   (wm->ddl[pipe].plane[PLANE_SPRITE0] << DDL_SPRITE_SHIFT(0)) |
@@ -759,35 +760,35 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 	 * high order bits so that there are no out of bounds values
 	 * present in the registers during the reprogramming.
 	 */
-	intel_uncore_write(&dev_priv->uncore, DSPHOWM, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPHOWM1, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPFW4, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPFW5, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPFW6, 0);
+	intel_de_write(dev_priv, DSPHOWM, 0);
+	intel_de_write(dev_priv, DSPHOWM1, 0);
+	intel_de_write(dev_priv, DSPFW4, 0);
+	intel_de_write(dev_priv, DSPFW5, 0);
+	intel_de_write(dev_priv, DSPFW6, 0);
 
-	intel_uncore_write(&dev_priv->uncore, DSPFW1,
+	intel_de_write(dev_priv, DSPFW1,
 			   FW_WM(wm->sr.plane, SR) |
 			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
 			   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_PRIMARY], PLANEB) |
 			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_PRIMARY], PLANEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW2,
+	intel_de_write(dev_priv, DSPFW2,
 			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE1], SPRITEB) |
 			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_CURSOR], CURSORA) |
 			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE0], SPRITEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW3,
+	intel_de_write(dev_priv, DSPFW3,
 			   FW_WM(wm->sr.cursor, CURSOR_SR));
 
 	if (IS_CHERRYVIEW(dev_priv)) {
-		intel_uncore_write(&dev_priv->uncore, DSPFW7_CHV,
+		intel_de_write(dev_priv, DSPFW7_CHV,
 				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |
 				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEC));
-		intel_uncore_write(&dev_priv->uncore, DSPFW8_CHV,
+		intel_de_write(dev_priv, DSPFW8_CHV,
 				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE1], SPRITEF) |
 				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE0], SPRITEE));
-		intel_uncore_write(&dev_priv->uncore, DSPFW9_CHV,
+		intel_de_write(dev_priv, DSPFW9_CHV,
 				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_PRIMARY], PLANEC) |
 				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_CURSOR], CURSORC));
-		intel_uncore_write(&dev_priv->uncore, DSPHOWM,
+		intel_de_write(dev_priv, DSPHOWM,
 				   FW_WM(wm->sr.plane >> 9, SR_HI) |
 				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_SPRITE1] >> 8, SPRITEF_HI) |
 				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_SPRITE0] >> 8, SPRITEE_HI) |
@@ -799,10 +800,10 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0] >> 8, SPRITEA_HI) |
 				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
 	} else {
-		intel_uncore_write(&dev_priv->uncore, DSPFW7,
+		intel_de_write(dev_priv, DSPFW7,
 				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |
 				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEC));
-		intel_uncore_write(&dev_priv->uncore, DSPHOWM,
+		intel_de_write(dev_priv, DSPHOWM,
 				   FW_WM(wm->sr.plane >> 9, SR_HI) |
 				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE1] >> 8, SPRITED_HI) |
 				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0] >> 8, SPRITEC_HI) |
@@ -812,7 +813,7 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
 	}
 
-	intel_uncore_posting_read(&dev_priv->uncore, DSPFW1);
+	intel_de_posting_read(dev_priv, DSPFW1);
 }
 
 #undef FW_WM_VLV
@@ -1751,7 +1752,6 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 				   struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_uncore *uncore = &dev_priv->uncore;
 	const struct intel_crtc_state *crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct vlv_fifo_state *fifo_state =
@@ -1775,8 +1775,8 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	switch (crtc->pipe) {
 	case PIPE_A:
-		dsparb = intel_uncore_read_fw(uncore, DSPARB);
-		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
+		dsparb = intel_de_read_fw(dev_priv, DSPARB);
+		dsparb2 = intel_de_read_fw(dev_priv, DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEA, 0xff) |
 			    VLV_FIFO(SPRITEB, 0xff));
@@ -1788,12 +1788,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEA_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITEB_HI, sprite1_start >> 8));
 
-		intel_uncore_write_fw(uncore, DSPARB, dsparb);
-		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
+		intel_de_write_fw(dev_priv, DSPARB, dsparb);
+		intel_de_write_fw(dev_priv, DSPARB2, dsparb2);
 		break;
 	case PIPE_B:
-		dsparb = intel_uncore_read_fw(uncore, DSPARB);
-		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
+		dsparb = intel_de_read_fw(dev_priv, DSPARB);
+		dsparb2 = intel_de_read_fw(dev_priv, DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEC, 0xff) |
 			    VLV_FIFO(SPRITED, 0xff));
@@ -1805,12 +1805,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEC_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITED_HI, sprite1_start >> 8));
 
-		intel_uncore_write_fw(uncore, DSPARB, dsparb);
-		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
+		intel_de_write_fw(dev_priv, DSPARB, dsparb);
+		intel_de_write_fw(dev_priv, DSPARB2, dsparb2);
 		break;
 	case PIPE_C:
-		dsparb3 = intel_uncore_read_fw(uncore, DSPARB3);
-		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
+		dsparb3 = intel_de_read_fw(dev_priv, DSPARB3);
+		dsparb2 = intel_de_read_fw(dev_priv, DSPARB2);
 
 		dsparb3 &= ~(VLV_FIFO(SPRITEE, 0xff) |
 			     VLV_FIFO(SPRITEF, 0xff));
@@ -1822,14 +1822,14 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEE_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITEF_HI, sprite1_start >> 8));
 
-		intel_uncore_write_fw(uncore, DSPARB3, dsparb3);
-		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
+		intel_de_write_fw(dev_priv, DSPARB3, dsparb3);
+		intel_de_write_fw(dev_priv, DSPARB2, dsparb2);
 		break;
 	default:
 		break;
 	}
 
-	intel_uncore_posting_read_fw(uncore, DSPARB);
+	intel_de_posting_read_fw(dev_priv, DSPARB);
 
 	spin_unlock(&dev_priv->display.wm.dsparb_lock);
 }
@@ -2053,14 +2053,14 @@ static void i965_update_wm(struct drm_i915_private *dev_priv)
 		    srwm);
 
 	/* 965 has limitations... */
-	intel_uncore_write(&dev_priv->uncore, DSPFW1, FW_WM(srwm, SR) |
+	intel_de_write(dev_priv, DSPFW1, FW_WM(srwm, SR) |
 		   FW_WM(8, CURSORB) |
 		   FW_WM(8, PLANEB) |
 		   FW_WM(8, PLANEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW2, FW_WM(8, CURSORA) |
+	intel_de_write(dev_priv, DSPFW2, FW_WM(8, CURSORA) |
 		   FW_WM(8, PLANEC_OLD));
 	/* update cursor SR watermark */
-	intel_uncore_write(&dev_priv->uncore, DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
+	intel_de_write(dev_priv, DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
 
 	if (cxsr_enabled)
 		intel_set_memory_cxsr(dev_priv, true);
@@ -2201,10 +2201,10 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
 			srwm = 1;
 
 		if (IS_I945G(dev_priv) || IS_I945GM(dev_priv))
-			intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF,
+			intel_de_write(dev_priv, FW_BLC_SELF,
 				   FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));
 		else
-			intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF, srwm & 0x3f);
+			intel_de_write(dev_priv, FW_BLC_SELF, srwm & 0x3f);
 	}
 
 	drm_dbg_kms(&dev_priv->drm,
@@ -2218,8 +2218,8 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
 	fwater_lo = fwater_lo | (1 << 24) | (1 << 8);
 	fwater_hi = fwater_hi | (1 << 8);
 
-	intel_uncore_write(&dev_priv->uncore, FW_BLC, fwater_lo);
-	intel_uncore_write(&dev_priv->uncore, FW_BLC2, fwater_hi);
+	intel_de_write(dev_priv, FW_BLC, fwater_lo);
+	intel_de_write(dev_priv, FW_BLC2, fwater_hi);
 
 	if (crtc)
 		intel_set_memory_cxsr(dev_priv, true);
@@ -2239,13 +2239,13 @@ static void i845_update_wm(struct drm_i915_private *dev_priv)
 				       &i845_wm_info,
 				       i845_get_fifo_size(dev_priv, PLANE_A),
 				       4, pessimal_latency_ns);
-	fwater_lo = intel_uncore_read(&dev_priv->uncore, FW_BLC) & ~0xfff;
+	fwater_lo = intel_de_read(dev_priv, FW_BLC) & ~0xfff;
 	fwater_lo |= (3<<8) | planea_wm;
 
 	drm_dbg_kms(&dev_priv->drm,
 		    "Setting FIFO watermarks - A: %d\n", planea_wm);
 
-	intel_uncore_write(&dev_priv->uncore, FW_BLC, fwater_lo);
+	intel_de_write(dev_priv, FW_BLC, fwater_lo);
 }
 
 /* latency must be in 0.1us units. */
@@ -2603,7 +2603,7 @@ static void hsw_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 
 	i915->display.wm.num_levels = 5;
 
-	sskpd = intel_uncore_read64(&i915->uncore, MCH_SSKPD);
+	sskpd = intel_de_read64(i915, MCH_SSKPD);
 
 	wm[0] = REG_FIELD_GET64(SSKPD_NEW_WM0_MASK_HSW, sskpd);
 	if (wm[0] == 0)
@@ -2620,7 +2620,7 @@ static void snb_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 
 	i915->display.wm.num_levels = 4;
 
-	sskpd = intel_uncore_read(&i915->uncore, MCH_SSKPD);
+	sskpd = intel_de_read(i915, MCH_SSKPD);
 
 	wm[0] = REG_FIELD_GET(SSKPD_WM0_MASK_SNB, sskpd);
 	wm[1] = REG_FIELD_GET(SSKPD_WM1_MASK_SNB, sskpd);
@@ -2634,7 +2634,7 @@ static void ilk_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 
 	i915->display.wm.num_levels = 3;
 
-	mltr = intel_uncore_read(&i915->uncore, MLTR_ILK);
+	mltr = intel_de_read(i915, MLTR_ILK);
 
 	/* ILK primary LP0 latency is 700 ns */
 	wm[0] = 7;
@@ -3163,17 +3163,17 @@ static bool _ilk_disable_lp_wm(struct drm_i915_private *dev_priv,
 
 	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM_LP_ENABLE) {
 		previous->wm_lp[2] &= ~WM_LP_ENABLE;
-		intel_uncore_write(&dev_priv->uncore, WM3_LP_ILK, previous->wm_lp[2]);
+		intel_de_write(dev_priv, WM3_LP_ILK, previous->wm_lp[2]);
 		changed = true;
 	}
 	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM_LP_ENABLE) {
 		previous->wm_lp[1] &= ~WM_LP_ENABLE;
-		intel_uncore_write(&dev_priv->uncore, WM2_LP_ILK, previous->wm_lp[1]);
+		intel_de_write(dev_priv, WM2_LP_ILK, previous->wm_lp[1]);
 		changed = true;
 	}
 	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM_LP_ENABLE) {
 		previous->wm_lp[0] &= ~WM_LP_ENABLE;
-		intel_uncore_write(&dev_priv->uncore, WM1_LP_ILK, previous->wm_lp[0]);
+		intel_de_write(dev_priv, WM1_LP_ILK, previous->wm_lp[0]);
 		changed = true;
 	}
 
@@ -3202,44 +3202,44 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 	_ilk_disable_lp_wm(dev_priv, dirty);
 
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
-		intel_uncore_write(&dev_priv->uncore, WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
+		intel_de_write(dev_priv, WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
-		intel_uncore_write(&dev_priv->uncore, WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
+		intel_de_write(dev_priv, WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_C))
-		intel_uncore_write(&dev_priv->uncore, WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
+		intel_de_write(dev_priv, WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_DDB) {
 		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			intel_uncore_rmw(&dev_priv->uncore, WM_MISC, WM_MISC_DATA_PARTITION_5_6,
+			intel_de_rmw(dev_priv, WM_MISC, WM_MISC_DATA_PARTITION_5_6,
 					 results->partitioning == INTEL_DDB_PART_1_2 ? 0 :
 					 WM_MISC_DATA_PARTITION_5_6);
 		else
-			intel_uncore_rmw(&dev_priv->uncore, DISP_ARB_CTL2, DISP_DATA_PARTITION_5_6,
+			intel_de_rmw(dev_priv, DISP_ARB_CTL2, DISP_DATA_PARTITION_5_6,
 					 results->partitioning == INTEL_DDB_PART_1_2 ? 0 :
 					 DISP_DATA_PARTITION_5_6);
 	}
 
 	if (dirty & WM_DIRTY_FBC)
-		intel_uncore_rmw(&dev_priv->uncore, DISP_ARB_CTL, DISP_FBC_WM_DIS,
+		intel_de_rmw(dev_priv, DISP_ARB_CTL, DISP_FBC_WM_DIS,
 				 results->enable_fbc_wm ? 0 : DISP_FBC_WM_DIS);
 
 	if (dirty & WM_DIRTY_LP(1) &&
 	    previous->wm_lp_spr[0] != results->wm_lp_spr[0])
-		intel_uncore_write(&dev_priv->uncore, WM1S_LP_ILK, results->wm_lp_spr[0]);
+		intel_de_write(dev_priv, WM1S_LP_ILK, results->wm_lp_spr[0]);
 
 	if (DISPLAY_VER(dev_priv) >= 7) {
 		if (dirty & WM_DIRTY_LP(2) && previous->wm_lp_spr[1] != results->wm_lp_spr[1])
-			intel_uncore_write(&dev_priv->uncore, WM2S_LP_IVB, results->wm_lp_spr[1]);
+			intel_de_write(dev_priv, WM2S_LP_IVB, results->wm_lp_spr[1]);
 		if (dirty & WM_DIRTY_LP(3) && previous->wm_lp_spr[2] != results->wm_lp_spr[2])
-			intel_uncore_write(&dev_priv->uncore, WM3S_LP_IVB, results->wm_lp_spr[2]);
+			intel_de_write(dev_priv, WM3S_LP_IVB, results->wm_lp_spr[2]);
 	}
 
 	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != results->wm_lp[0])
-		intel_uncore_write(&dev_priv->uncore, WM1_LP_ILK, results->wm_lp[0]);
+		intel_de_write(dev_priv, WM1_LP_ILK, results->wm_lp[0]);
 	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] != results->wm_lp[1])
-		intel_uncore_write(&dev_priv->uncore, WM2_LP_ILK, results->wm_lp[1]);
+		intel_de_write(dev_priv, WM2_LP_ILK, results->wm_lp[1]);
 	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] != results->wm_lp[2])
-		intel_uncore_write(&dev_priv->uncore, WM3_LP_ILK, results->wm_lp[2]);
+		intel_de_write(dev_priv, WM3_LP_ILK, results->wm_lp[2]);
 
 	dev_priv->display.wm.hw = *results;
 }
@@ -3337,7 +3337,7 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 	struct intel_pipe_wm *active = &crtc_state->wm.ilk.optimal;
 	enum pipe pipe = crtc->pipe;
 
-	hw->wm_pipe[pipe] = intel_uncore_read(&dev_priv->uncore, WM0_PIPE_ILK(pipe));
+	hw->wm_pipe[pipe] = intel_de_read(dev_priv, WM0_PIPE_ILK(pipe));
 
 	memset(active, 0, sizeof(*active));
 
@@ -3502,13 +3502,13 @@ static void g4x_read_wm_values(struct drm_i915_private *dev_priv,
 {
 	u32 tmp;
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW1);
+	tmp = intel_de_read(dev_priv, DSPFW1);
 	wm->sr.plane = _FW_WM(tmp, SR);
 	wm->pipe[PIPE_B].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORB);
 	wm->pipe[PIPE_B].plane[PLANE_PRIMARY] = _FW_WM(tmp, PLANEB);
 	wm->pipe[PIPE_A].plane[PLANE_PRIMARY] = _FW_WM(tmp, PLANEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW2);
+	tmp = intel_de_read(dev_priv, DSPFW2);
 	wm->fbc_en = tmp & DSPFW_FBC_SR_EN;
 	wm->sr.fbc = _FW_WM(tmp, FBC_SR);
 	wm->hpll.fbc = _FW_WM(tmp, FBC_HPLL_SR);
@@ -3516,7 +3516,7 @@ static void g4x_read_wm_values(struct drm_i915_private *dev_priv,
 	wm->pipe[PIPE_A].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORA);
 	wm->pipe[PIPE_A].plane[PLANE_SPRITE0] = _FW_WM(tmp, SPRITEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+	tmp = intel_de_read(dev_priv, DSPFW3);
 	wm->hpll_en = tmp & DSPFW_HPLL_SR_EN;
 	wm->sr.cursor = _FW_WM(tmp, CURSOR_SR);
 	wm->hpll.cursor = _FW_WM(tmp, HPLL_CURSOR);
@@ -3530,7 +3530,7 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv,
 	u32 tmp;
 
 	for_each_pipe(dev_priv, pipe) {
-		tmp = intel_uncore_read(&dev_priv->uncore, VLV_DDL(pipe));
+		tmp = intel_de_read(dev_priv, VLV_DDL(pipe));
 
 		wm->ddl[pipe].plane[PLANE_PRIMARY] =
 			(tmp >> DDL_PLANE_SHIFT) & (DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK);
@@ -3542,34 +3542,34 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv,
 			(tmp >> DDL_SPRITE_SHIFT(1)) & (DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK);
 	}
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW1);
+	tmp = intel_de_read(dev_priv, DSPFW1);
 	wm->sr.plane = _FW_WM(tmp, SR);
 	wm->pipe[PIPE_B].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORB);
 	wm->pipe[PIPE_B].plane[PLANE_PRIMARY] = _FW_WM_VLV(tmp, PLANEB);
 	wm->pipe[PIPE_A].plane[PLANE_PRIMARY] = _FW_WM_VLV(tmp, PLANEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW2);
+	tmp = intel_de_read(dev_priv, DSPFW2);
 	wm->pipe[PIPE_A].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITEB);
 	wm->pipe[PIPE_A].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORA);
 	wm->pipe[PIPE_A].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+	tmp = intel_de_read(dev_priv, DSPFW3);
 	wm->sr.cursor = _FW_WM(tmp, CURSOR_SR);
 
 	if (IS_CHERRYVIEW(dev_priv)) {
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW7_CHV);
+		tmp = intel_de_read(dev_priv, DSPFW7_CHV);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITED);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEC);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW8_CHV);
+		tmp = intel_de_read(dev_priv, DSPFW8_CHV);
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITEF);
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEE);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW9_CHV);
+		tmp = intel_de_read(dev_priv, DSPFW9_CHV);
 		wm->pipe[PIPE_C].plane[PLANE_PRIMARY] = _FW_WM_VLV(tmp, PLANEC);
 		wm->pipe[PIPE_C].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORC);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPHOWM);
+		tmp = intel_de_read(dev_priv, DSPHOWM);
 		wm->sr.plane |= _FW_WM(tmp, SR_HI) << 9;
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE1] |= _FW_WM(tmp, SPRITEF_HI) << 8;
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE0] |= _FW_WM(tmp, SPRITEE_HI) << 8;
@@ -3581,11 +3581,11 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv,
 		wm->pipe[PIPE_A].plane[PLANE_SPRITE0] |= _FW_WM(tmp, SPRITEA_HI) << 8;
 		wm->pipe[PIPE_A].plane[PLANE_PRIMARY] |= _FW_WM(tmp, PLANEA_HI) << 8;
 	} else {
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW7);
+		tmp = intel_de_read(dev_priv, DSPFW7);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITED);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEC);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPHOWM);
+		tmp = intel_de_read(dev_priv, DSPHOWM);
 		wm->sr.plane |= _FW_WM(tmp, SR_HI) << 9;
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE1] |= _FW_WM(tmp, SPRITED_HI) << 8;
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE0] |= _FW_WM(tmp, SPRITEC_HI) << 8;
@@ -3606,7 +3606,7 @@ static void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv)
 
 	g4x_read_wm_values(dev_priv, wm);
 
-	wm->cxsr = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF) & FW_BLC_SELF_EN;
+	wm->cxsr = intel_de_read(dev_priv, FW_BLC_SELF) & FW_BLC_SELF_EN;
 
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
 		struct intel_crtc_state *crtc_state =
@@ -3755,7 +3755,7 @@ static void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv)
 
 	vlv_read_wm_values(dev_priv, wm);
 
-	wm->cxsr = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
+	wm->cxsr = intel_de_read(dev_priv, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
 	wm->level = VLV_WM_LEVEL_PM2;
 
 	if (IS_CHERRYVIEW(dev_priv)) {
@@ -3905,9 +3905,9 @@ static void vlv_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915)
  */
 static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
 {
-	intel_uncore_rmw(&dev_priv->uncore, WM3_LP_ILK, WM_LP_ENABLE, 0);
-	intel_uncore_rmw(&dev_priv->uncore, WM2_LP_ILK, WM_LP_ENABLE, 0);
-	intel_uncore_rmw(&dev_priv->uncore, WM1_LP_ILK, WM_LP_ENABLE, 0);
+	intel_de_rmw(dev_priv, WM3_LP_ILK, WM_LP_ENABLE, 0);
+	intel_de_rmw(dev_priv, WM2_LP_ILK, WM_LP_ENABLE, 0);
+	intel_de_rmw(dev_priv, WM1_LP_ILK, WM_LP_ENABLE, 0);
 
 	/*
 	 * Don't touch WM_LP_SPRITE_ENABLE here.
@@ -3925,27 +3925,27 @@ static void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
 	for_each_intel_crtc(&dev_priv->drm, crtc)
 		ilk_pipe_wm_get_hw_state(crtc);
 
-	hw->wm_lp[0] = intel_uncore_read(&dev_priv->uncore, WM1_LP_ILK);
-	hw->wm_lp[1] = intel_uncore_read(&dev_priv->uncore, WM2_LP_ILK);
-	hw->wm_lp[2] = intel_uncore_read(&dev_priv->uncore, WM3_LP_ILK);
+	hw->wm_lp[0] = intel_de_read(dev_priv, WM1_LP_ILK);
+	hw->wm_lp[1] = intel_de_read(dev_priv, WM2_LP_ILK);
+	hw->wm_lp[2] = intel_de_read(dev_priv, WM3_LP_ILK);
 
-	hw->wm_lp_spr[0] = intel_uncore_read(&dev_priv->uncore, WM1S_LP_ILK);
+	hw->wm_lp_spr[0] = intel_de_read(dev_priv, WM1S_LP_ILK);
 	if (DISPLAY_VER(dev_priv) >= 7) {
-		hw->wm_lp_spr[1] = intel_uncore_read(&dev_priv->uncore, WM2S_LP_IVB);
-		hw->wm_lp_spr[2] = intel_uncore_read(&dev_priv->uncore, WM3S_LP_IVB);
+		hw->wm_lp_spr[1] = intel_de_read(dev_priv, WM2S_LP_IVB);
+		hw->wm_lp_spr[2] = intel_de_read(dev_priv, WM3S_LP_IVB);
 	}
 
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hw->partitioning = (intel_uncore_read(&dev_priv->uncore, WM_MISC) &
+		hw->partitioning = (intel_de_read(dev_priv, WM_MISC) &
 				    WM_MISC_DATA_PARTITION_5_6) ?
 			INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
 	else if (IS_IVYBRIDGE(dev_priv))
-		hw->partitioning = (intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL2) &
+		hw->partitioning = (intel_de_read(dev_priv, DISP_ARB_CTL2) &
 				    DISP_DATA_PARTITION_5_6) ?
 			INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
 
 	hw->enable_fbc_wm =
-		!(intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
+		!(intel_de_read(dev_priv, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
 }
 
 static const struct intel_wm_funcs ilk_wm_funcs = {
diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
index 42552d8c151e..0c64245a181d 100644
--- a/drivers/gpu/drm/i915/display/intel_de.h
+++ b/drivers/gpu/drm/i915/display/intel_de.h
@@ -22,6 +22,12 @@ intel_de_read8(struct drm_i915_private *i915, i915_reg_t reg)
 	return intel_uncore_read8(&i915->uncore, reg);
 }
 
+static inline u8
+intel_de_read64(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	return intel_uncore_read64(&i915->uncore, reg);
+}
+
 static inline u64
 intel_de_read64_2x32(struct drm_i915_private *i915,
 		     i915_reg_t lower_reg, i915_reg_t upper_reg)
@@ -104,6 +110,12 @@ intel_de_read_fw(struct drm_i915_private *i915, i915_reg_t reg)
 	return val;
 }
 
+static inline void
+intel_de_posting_read_fw(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	intel_uncore_posting_read_fw(&i915->uncore, reg);
+}
+
 static inline void
 intel_de_write_fw(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
 {
-- 
2.39.2

Re: [Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

[Ville Syrjälä]

On Wed, Mar 08, 2023 at 11:58:59AM -0500, Rodrigo Vivi wrote:
>  	} else if (IS_I915GM(dev_priv)) {
>  		/*
>  		 * FIXME can't find a bit like this for 915G, and
>  		 * yet it does have the related watermark in
>  		 * FW_BLC_SELF. What's going on?
>  		 */
> -		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
> +		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
>  		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
>  			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
> -		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
> -		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
> +		intel_de_write(dev_priv, INSTPM, val);
> +		intel_de_posting_read(dev_priv, INSTPM);

I'm still not really convinced that we want to
use intel_de_*() for non-display registers.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

[Rodrigo Vivi]

On Wed, Mar 08, 2023 at 07:50:27PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 08, 2023 at 11:58:59AM -0500, Rodrigo Vivi wrote:
> >  	} else if (IS_I915GM(dev_priv)) {
> >  		/*
> >  		 * FIXME can't find a bit like this for 915G, and
> >  		 * yet it does have the related watermark in
> >  		 * FW_BLC_SELF. What's going on?
> >  		 */
> > -		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
> > +		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
> >  		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
> >  			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
> > -		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
> > -		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
> > +		intel_de_write(dev_priv, INSTPM, val);
> > +		intel_de_posting_read(dev_priv, INSTPM);
> 
> I'm still not really convinced that we want to
> use intel_de_*() for non-display registers.

hmmm... I see...
so should we create a new component out of i915/display and move
these calls there?

but in the end of the day it is the same uncore functions that
are getting calling underneath anyway, right?!

I believe i915/display should only call intel_de for mmio, so it
gets easier on the code reuse on Xe.

> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

[Ville Syrjälä]

On Wed, Mar 08, 2023 at 12:56:28PM -0500, Rodrigo Vivi wrote:
> On Wed, Mar 08, 2023 at 07:50:27PM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 08, 2023 at 11:58:59AM -0500, Rodrigo Vivi wrote:
> > >  	} else if (IS_I915GM(dev_priv)) {
> > >  		/*
> > >  		 * FIXME can't find a bit like this for 915G, and
> > >  		 * yet it does have the related watermark in
> > >  		 * FW_BLC_SELF. What's going on?
> > >  		 */
> > > -		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
> > > +		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
> > >  		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
> > >  			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
> > > -		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
> > > -		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
> > > +		intel_de_write(dev_priv, INSTPM, val);
> > > +		intel_de_posting_read(dev_priv, INSTPM);
> > 
> > I'm still not really convinced that we want to
> > use intel_de_*() for non-display registers.
> 
> hmmm... I see...
> so should we create a new component out of i915/display and move
> these calls there?
> 
> but in the end of the day it is the same uncore functions that
> are getting calling underneath anyway, right?!

Currently yes. Though I have occasionally thought about
splitting it up lower down, since no display registers need
forcewake, and IIRC the RM unclaimed stuff only really works
for display registers. So we could perhaps lighten each
side a bit by knowing ahead of time what kind of register
we're dealing with.

> 
> I believe i915/display should only call intel_de for mmio, so it
> gets easier on the code reuse on Xe.

Yeah, I get idea. However I think it might also be nice 
to check that we are not touching registers that we're
not supposed to touch from the display code. So having
intel_de*() validate the register offset might be nice.
Would be especially important if we did do the lower
level register accessor split.

Though admittedly on these old platforms that valiation
is perhaps a bit moot since the display vs. not split
is far from clear, and even the truly dedicated display
registers can live at rather weird offsets.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

[Ville Syrjälä]

On Wed, Mar 08, 2023 at 08:18:52PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 08, 2023 at 12:56:28PM -0500, Rodrigo Vivi wrote:
> > On Wed, Mar 08, 2023 at 07:50:27PM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 08, 2023 at 11:58:59AM -0500, Rodrigo Vivi wrote:
> > > >  	} else if (IS_I915GM(dev_priv)) {
> > > >  		/*
> > > >  		 * FIXME can't find a bit like this for 915G, and
> > > >  		 * yet it does have the related watermark in
> > > >  		 * FW_BLC_SELF. What's going on?
> > > >  		 */
> > > > -		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
> > > > +		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
> > > >  		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
> > > >  			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
> > > > -		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
> > > > -		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
> > > > +		intel_de_write(dev_priv, INSTPM, val);
> > > > +		intel_de_posting_read(dev_priv, INSTPM);
> > > 
> > > I'm still not really convinced that we want to
> > > use intel_de_*() for non-display registers.
> > 
> > hmmm... I see...
> > so should we create a new component out of i915/display and move
> > these calls there?
> > 
> > but in the end of the day it is the same uncore functions that
> > are getting calling underneath anyway, right?!
> 
> Currently yes. Though I have occasionally thought about
> splitting it up lower down, since no display registers need
> forcewake, and IIRC the RM unclaimed stuff only really works
> for display registers. So we could perhaps lighten each
> side a bit by knowing ahead of time what kind of register
> we're dealing with.
> 
> > 
> > I believe i915/display should only call intel_de for mmio, so it
> > gets easier on the code reuse on Xe.
> 
> Yeah, I get idea. However I think it might also be nice 
> to check that we are not touching registers that we're
> not supposed to touch from the display code. So having
> intel_de*() validate the register offset might be nice.
> Would be especially important if we did do the lower
> level register accessor split.
> 
> Though admittedly on these old platforms that valiation
> is perhaps a bit moot since the display vs. not split
> is far from clear, and even the truly dedicated display
> registers can live at rather weird offsets.

I guess we can always backpedal a bit if we do decide
to do those things. There shouldn't be that many non-display
registers in the mix anyway.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Ville Syrjälä]

On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> uncore->lock only protects the forcewake domain itself,
> not the register accesses.
> 
> uncore's _fw alternatives are for cases where the domains
> are not needed because we are sure that they are already
> awake.
> 
> So the move towards the uncore's _fw alternatives seems
> right, however using the uncore-lock to protect the dsparb
> registers seems an abuse of the uncore-lock.
> 
> Let's restore the previous individual lock and try to get
> rid of the direct uncore accesses from the display code.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
>  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
>  drivers/gpu/drm/i915/i915_driver.c                |  1 +
>  3 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> index caef72d38798..8fe0b5c63d3a 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
>  
> -	/*
> -	 * uncore.lock serves a double purpose here. It allows us to
> -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> -	 * it protects the DSPARB registers from getting clobbered by
> -	 * parallel updates from multiple pipes.
> -	 *
> -	 * intel_pipe_update_start() has already disabled interrupts
> -	 * for us, so a plain spin_lock() is sufficient here.
> -	 */

I was wondering if we need to preserve the comment about irqs,
but since this is the only place using this lock, and it's never
called from an irq handler a non-irq disabling spinlock will suffice
anyway.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> -	spin_lock(&uncore->lock);
> +	spin_lock(&dev_priv->display.wm.dsparb_lock);
>  
>  	switch (crtc->pipe) {
>  	case PIPE_A:
> @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	intel_uncore_posting_read_fw(uncore, DSPARB);
>  
> -	spin_unlock(&uncore->lock);
> +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
>  }
>  
>  #undef VLV_FIFO
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index fdab7bb93a7d..68c6bfb91dbe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -253,6 +253,9 @@ struct intel_wm {
>  	 */
>  	struct mutex wm_mutex;
>  
> +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> +	spinlock_t dsparb_lock;
> +
>  	bool ipc_enabled;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index a53fd339e2cc..c78e36444a12 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  	mutex_init(&dev_priv->display.pps.mutex);
>  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
>  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
>  
>  	i915_memcpy_init_early(dev_priv);
>  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev2)

[Patchwork]

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

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev2)
URL   : https://patchwork.freedesktop.org/series/114868/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12832 -> Patchwork_114868v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_114868v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_114868v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/index.html

Participating hosts (36 -> 35)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (2): bat-atsm-1 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_114868v2:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1:
    - fi-kbl-7567u:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12832/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html

  
Known issues
------------

  Here are the changes found in Patchwork_114868v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-nick:        [PASS][5] -> [ABORT][6] ([i915#7911] / [i915#7913])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12832/fi-bsw-nick/igt@i915_selftest@live@execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-bsw-nick/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-kbl-soraka:      NOTRUN -> [INCOMPLETE][7] ([i915#7913])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-soraka/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][8] ([i915#1886])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@requests:
    - bat-rpls-2:         [PASS][9] -> [ABORT][10] ([i915#4983] / [i915#7694] / [i915#7913] / [i915#7981])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12832/bat-rpls-2/igt@i915_selftest@live@requests.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/bat-rpls-2/igt@i915_selftest@live@requests.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][11] ([fdo#109271]) +16 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@dmabuf:
    - bat-rplp-1:         [DMESG-WARN][12] ([i915#2867]) -> [PASS][13] +29 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12832/bat-rplp-1/igt@i915_selftest@live@dmabuf.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/bat-rplp-1/igt@i915_selftest@live@dmabuf.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#7694]: https://gitlab.freedesktop.org/drm/intel/issues/7694
  [i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981


Build changes
-------------

  * Linux: CI_DRM_12832 -> Patchwork_114868v2

  CI-20190529: 20190529
  CI_DRM_12832: 40eaf607cfcc6f514cd3ea652525704137bcea15 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7188: b35bfa32fe672d67ced8555557e3e707ace211ad @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_114868v2: 40eaf607cfcc6f514cd3ea652525704137bcea15 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

7074ff4fe54d drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.
6742758dfd63 drm/i915/display: Restore dsparb_lock.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/index.html

[-- Attachment #2: Type: text/html, Size: 6525 bytes --]

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

On Thu, Mar 09, 2023 at 12:03:19AM +0200, Ville Syrjälä wrote:
> On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> > uncore->lock only protects the forcewake domain itself,
> > not the register accesses.
> > 
> > uncore's _fw alternatives are for cases where the domains
> > are not needed because we are sure that they are already
> > awake.
> > 
> > So the move towards the uncore's _fw alternatives seems
> > right, however using the uncore-lock to protect the dsparb
> > registers seems an abuse of the uncore-lock.
> > 
> > Let's restore the previous individual lock and try to get
> > rid of the direct uncore accesses from the display code.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> >  3 files changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > index caef72d38798..8fe0b5c63d3a 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> >  
> > -	/*
> > -	 * uncore.lock serves a double purpose here. It allows us to
> > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > -	 * it protects the DSPARB registers from getting clobbered by
> > -	 * parallel updates from multiple pipes.
> > -	 *
> > -	 * intel_pipe_update_start() has already disabled interrupts
> > -	 * for us, so a plain spin_lock() is sufficient here.
> > -	 */
> 
> I was wondering if we need to preserve the comment about irqs,
> but since this is the only place using this lock, and it's never
> called from an irq handler a non-irq disabling spinlock will suffice
> anyway.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

thoughts on this: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html

maybe related to the usage of this uncore.lock in

drivers/gpu/drm/i915/display/intel_vblank.c

?

Should we create another spin lock and include both of these cases?
(Then the irq comment is relevant again :))

> 
> > -	spin_lock(&uncore->lock);
> > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> >  
> >  	switch (crtc->pipe) {
> >  	case PIPE_A:
> > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> >  
> > -	spin_unlock(&uncore->lock);
> > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> >  }
> >  
> >  #undef VLV_FIFO
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > index fdab7bb93a7d..68c6bfb91dbe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > @@ -253,6 +253,9 @@ struct intel_wm {
> >  	 */
> >  	struct mutex wm_mutex;
> >  
> > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > +	spinlock_t dsparb_lock;
> > +
> >  	bool ipc_enabled;
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index a53fd339e2cc..c78e36444a12 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> >  	mutex_init(&dev_priv->display.pps.mutex);
> >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> >  
> >  	i915_memcpy_init_early(dev_priv);
> >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> > -- 
> > 2.39.2
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Ville Syrjälä]

On Thu, Mar 09, 2023 at 05:03:52PM -0500, Rodrigo Vivi wrote:
> On Thu, Mar 09, 2023 at 12:03:19AM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> > > uncore->lock only protects the forcewake domain itself,
> > > not the register accesses.
> > > 
> > > uncore's _fw alternatives are for cases where the domains
> > > are not needed because we are sure that they are already
> > > awake.
> > > 
> > > So the move towards the uncore's _fw alternatives seems
> > > right, however using the uncore-lock to protect the dsparb
> > > registers seems an abuse of the uncore-lock.
> > > 
> > > Let's restore the previous individual lock and try to get
> > > rid of the direct uncore accesses from the display code.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> > >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> > >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> > >  3 files changed, 6 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > index caef72d38798..8fe0b5c63d3a 100644
> > > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > >  
> > >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> > >  
> > > -	/*
> > > -	 * uncore.lock serves a double purpose here. It allows us to
> > > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > > -	 * it protects the DSPARB registers from getting clobbered by
> > > -	 * parallel updates from multiple pipes.
> > > -	 *
> > > -	 * intel_pipe_update_start() has already disabled interrupts
> > > -	 * for us, so a plain spin_lock() is sufficient here.
> > > -	 */
> > 
> > I was wondering if we need to preserve the comment about irqs,
> > but since this is the only place using this lock, and it's never
> > called from an irq handler a non-irq disabling spinlock will suffice
> > anyway.
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> thoughts on this: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html

This code doesn't run on that platform, so unrelated.

> 
> maybe related to the usage of this uncore.lock in
> 
> drivers/gpu/drm/i915/display/intel_vblank.c
> 
> ?
> 
> Should we create another spin lock and include both of these cases?
> (Then the irq comment is relevant again :))

We're already 4 spinlocks deep when in vblank code. Let's not add more ;)

> 
> > 
> > > -	spin_lock(&uncore->lock);
> > > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> > >  
> > >  	switch (crtc->pipe) {
> > >  	case PIPE_A:
> > > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > >  
> > >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> > >  
> > > -	spin_unlock(&uncore->lock);
> > > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> > >  }
> > >  
> > >  #undef VLV_FIFO
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > index fdab7bb93a7d..68c6bfb91dbe 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > @@ -253,6 +253,9 @@ struct intel_wm {
> > >  	 */
> > >  	struct mutex wm_mutex;
> > >  
> > > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > > +	spinlock_t dsparb_lock;
> > > +
> > >  	bool ipc_enabled;
> > >  };
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > > index a53fd339e2cc..c78e36444a12 100644
> > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> > >  	mutex_init(&dev_priv->display.pps.mutex);
> > >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> > >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> > >  
> > >  	i915_memcpy_init_early(dev_priv);
> > >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> > > -- 
> > > 2.39.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

On Fri, Mar 10, 2023 at 06:26:54PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 09, 2023 at 05:03:52PM -0500, Rodrigo Vivi wrote:
> > On Thu, Mar 09, 2023 at 12:03:19AM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> > > > uncore->lock only protects the forcewake domain itself,
> > > > not the register accesses.
> > > > 
> > > > uncore's _fw alternatives are for cases where the domains
> > > > are not needed because we are sure that they are already
> > > > awake.
> > > > 
> > > > So the move towards the uncore's _fw alternatives seems
> > > > right, however using the uncore-lock to protect the dsparb
> > > > registers seems an abuse of the uncore-lock.
> > > > 
> > > > Let's restore the previous individual lock and try to get
> > > > rid of the direct uncore accesses from the display code.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> > > >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> > > >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> > > >  3 files changed, 6 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > > index caef72d38798..8fe0b5c63d3a 100644
> > > > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > > >  
> > > >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> > > >  
> > > > -	/*
> > > > -	 * uncore.lock serves a double purpose here. It allows us to
> > > > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > > > -	 * it protects the DSPARB registers from getting clobbered by
> > > > -	 * parallel updates from multiple pipes.
> > > > -	 *
> > > > -	 * intel_pipe_update_start() has already disabled interrupts
> > > > -	 * for us, so a plain spin_lock() is sufficient here.
> > > > -	 */
> > > 
> > > I was wondering if we need to preserve the comment about irqs,
> > > but since this is the only place using this lock, and it's never
> > > called from an irq handler a non-irq disabling spinlock will suffice
> > > anyway.
> > > 
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > thoughts on this: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html
> 
> This code doesn't run on that platform, so unrelated.

oh! indeed.
okay, I just triggered a rerun to get the full round... luckly...

> 
> > 
> > maybe related to the usage of this uncore.lock in
> > 
> > drivers/gpu/drm/i915/display/intel_vblank.c
> > 
> > ?
> > 
> > Should we create another spin lock and include both of these cases?
> > (Then the irq comment is relevant again :))
> 
> We're already 4 spinlocks deep when in vblank code. Let's not add more ;)
> 
> > 
> > > 
> > > > -	spin_lock(&uncore->lock);
> > > > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> > > >  
> > > >  	switch (crtc->pipe) {
> > > >  	case PIPE_A:
> > > > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > > >  
> > > >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> > > >  
> > > > -	spin_unlock(&uncore->lock);
> > > > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> > > >  }
> > > >  
> > > >  #undef VLV_FIFO
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > > index fdab7bb93a7d..68c6bfb91dbe 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > > @@ -253,6 +253,9 @@ struct intel_wm {
> > > >  	 */
> > > >  	struct mutex wm_mutex;
> > > >  
> > > > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > > > +	spinlock_t dsparb_lock;
> > > > +
> > > >  	bool ipc_enabled;
> > > >  };
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > > > index a53fd339e2cc..c78e36444a12 100644
> > > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> > > >  	mutex_init(&dev_priv->display.pps.mutex);
> > > >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> > > >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > > > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> > > >  
> > > >  	i915_memcpy_init_early(dev_priv);
> > > >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> > > > -- 
> > > > 2.39.2
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev3)

[Patchwork]

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

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev3)
URL   : https://patchwork.freedesktop.org/series/114868/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12841 -> Patchwork_114868v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_114868v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_114868v3, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/index.html

Participating hosts (35 -> 36)
------------------------------

  Additional (1): bat-atsm-1 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_114868v3:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@execlists:
    - fi-apl-guc:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12841/fi-apl-guc/igt@i915_selftest@live@execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/fi-apl-guc/igt@i915_selftest@live@execlists.html

  
Known issues
------------

  Here are the changes found in Patchwork_114868v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@fbdev@eof:
    - bat-atsm-1:         NOTRUN -> [SKIP][3] ([i915#2582]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-atsm-1/igt@fbdev@eof.html

  * igt@gem_mmap@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][4] ([i915#4083])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-atsm-1/igt@gem_mmap@basic.html

  * igt@gem_sync@basic-each:
    - bat-atsm-1:         NOTRUN -> [FAIL][5] ([i915#8062]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-atsm-1/igt@gem_sync@basic-each.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][6] ([i915#4077]) +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-atsm-1/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][7] ([i915#4079]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-atsm-1/igt@gem_tiled_pread_basic.html

  * igt@i915_hangman@error-state-basic:
    - bat-atsm-1:         NOTRUN -> [ABORT][8] ([i915#8060])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-atsm-1/igt@i915_hangman@error-state-basic.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      [PASS][9] -> [DMESG-FAIL][10] ([i915#5334] / [i915#7872])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12841/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-rpls-1:         NOTRUN -> [SKIP][11] ([i915#7828])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-rpls-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][12] ([i915#5354]) +2 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-rpls-1:         NOTRUN -> [SKIP][13] ([i915#1845])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - bat-rpls-1:         [ABORT][14] ([i915#6687] / [i915#7978]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12841/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-skl-6600u:       [DMESG-FAIL][16] ([i915#5334]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12841/fi-skl-6600u/igt@i915_selftest@live@gt_heartbeat.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/fi-skl-6600u/igt@i915_selftest@live@gt_heartbeat.html

  
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#8060]: https://gitlab.freedesktop.org/drm/intel/issues/8060
  [i915#8062]: https://gitlab.freedesktop.org/drm/intel/issues/8062


Build changes
-------------

  * Linux: CI_DRM_12841 -> Patchwork_114868v3

  CI-20190529: 20190529
  CI_DRM_12841: 8c85bb9e3577801873ca704c6c6cf85e175f244f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7190: f9d49501eaaadd39ae471094bc45a76a1ff93e42 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_114868v3: 8c85bb9e3577801873ca704c6c6cf85e175f244f @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

0e6a8754d048 drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.
27fb19a6e9de drm/i915/display: Restore dsparb_lock.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v3/index.html

[-- Attachment #2: Type: text/html, Size: 7209 bytes --]

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)
URL   : https://patchwork.freedesktop.org/series/114868/
State : warning

== Summary ==

Error: dim checkpatch failed
4a0ced9e598f drm/i915/display: Restore dsparb_lock.
6ac1060f9408 drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.
-:155: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#155: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:670:
+		intel_de_rmw(dev_priv, DSPFW3, DSPFW_CURSOR_SR_MASK,
 				 FW_WM(wm, CURSOR_SR));

-:183: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#183: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:720:
+	intel_de_write(dev_priv, DSPFW1,
 			   FW_WM(wm->sr.plane, SR) |

-:189: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#189: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:725:
+	intel_de_write(dev_priv, DSPFW2,
 			   (wm->fbc_en ? DSPFW_FBC_SR_EN : 0) |

-:197: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#197: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:732:
+	intel_de_write(dev_priv, DSPFW3,
 			   (wm->hpll_en ? DSPFW_HPLL_SR_EN : 0) |

-:213: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#213: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:752:
+		intel_de_write(dev_priv, VLV_DDL(pipe),
 				   (wm->ddl[pipe].plane[PLANE_CURSOR] << DDL_CURSOR_SHIFT) |

-:233: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#233: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:770:
+	intel_de_write(dev_priv, DSPFW1,
 			   FW_WM(wm->sr.plane, SR) |

-:239: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#239: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:775:
+	intel_de_write(dev_priv, DSPFW2,
 			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE1], SPRITEB) |

-:244: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#244: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:779:
+	intel_de_write(dev_priv, DSPFW3,
 			   FW_WM(wm->sr.cursor, CURSOR_SR));

-:249: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#249: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:783:
+		intel_de_write(dev_priv, DSPFW7_CHV,
 				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |

-:253: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#253: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:786:
+		intel_de_write(dev_priv, DSPFW8_CHV,
 				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE1], SPRITEF) |

-:257: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#257: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:789:
+		intel_de_write(dev_priv, DSPFW9_CHV,
 				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_PRIMARY], PLANEC) |

-:261: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#261: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:792:
+		intel_de_write(dev_priv, DSPHOWM,
 				   FW_WM(wm->sr.plane >> 9, SR_HI) |

-:270: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#270: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:804:
+		intel_de_write(dev_priv, DSPFW7,
 				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |

-:274: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#274: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:807:
+		intel_de_write(dev_priv, DSPHOWM,
 				   FW_WM(wm->sr.plane >> 9, SR_HI) |

-:381: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#381: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:2205:
+			intel_de_write(dev_priv, FW_BLC_SELF,
 				   FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));

-:480: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#480: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:3214:
+			intel_de_rmw(dev_priv, WM_MISC, WM_MISC_DATA_PARTITION_5_6,
 					 results->partitioning == INTEL_DDB_PART_1_2 ? 0 :

-:485: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#485: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:3218:
+			intel_de_rmw(dev_priv, DISP_ARB_CTL2, DISP_DATA_PARTITION_5_6,
 					 results->partitioning == INTEL_DDB_PART_1_2 ? 0 :

-:492: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#492: FILE: drivers/gpu/drm/i915/display/i9xx_wm.c:3224:
+		intel_de_rmw(dev_priv, DISP_ARB_CTL, DISP_FBC_WM_DIS,
 				 results->enable_fbc_wm ? 0 : DISP_FBC_WM_DIS);

total: 0 errors, 0 warnings, 18 checks, 655 lines checked

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)
URL   : https://patchwork.freedesktop.org/series/114868/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)

[Patchwork]

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

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)
URL   : https://patchwork.freedesktop.org/series/114868/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12911 -> Patchwork_114868v4
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/index.html

Participating hosts (37 -> 35)
------------------------------

  Missing    (2): fi-tgl-1115g4 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_114868v4 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@guc:
    - bat-rpls-1:         [PASS][1] -> [DMESG-WARN][2] ([i915#7852])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/bat-rpls-1/igt@i915_selftest@live@guc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-rpls-1/igt@i915_selftest@live@guc.html

  * igt@i915_selftest@live@migrate:
    - bat-adlp-9:         [PASS][3] -> [DMESG-FAIL][4] ([i915#7699])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/bat-adlp-9/igt@i915_selftest@live@migrate.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-adlp-9/igt@i915_selftest@live@migrate.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-rpls-2:         NOTRUN -> [SKIP][5] ([i915#7828])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-rpls-2/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@read-crc:
    - bat-adlp-9:         NOTRUN -> [SKIP][6] ([i915#3546]) +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-adlp-9/igt@kms_pipe_crc_basic@read-crc.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-rpls-2:         NOTRUN -> [SKIP][7] ([i915#1845])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-rpls-2/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Possible fixes ####

  * igt@i915_pm_rps@basic-api:
    - bat-dg1-5:          [FAIL][8] ([i915#8308]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/bat-dg1-5/igt@i915_pm_rps@basic-api.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-dg1-5/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@hangcheck:
    - fi-skl-guc:         [DMESG-WARN][10] ([i915#8073]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/fi-skl-guc/igt@i915_selftest@live@hangcheck.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/fi-skl-guc/igt@i915_selftest@live@hangcheck.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-2:         [ABORT][12] ([i915#6687] / [i915#7978]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/bat-rpls-2/igt@i915_suspend@basic-s3-without-i915.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-rpls-2/igt@i915_suspend@basic-s3-without-i915.html

  
#### Warnings ####

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         [DMESG-FAIL][14] ([i915#6997] / [i915#7913]) -> [DMESG-FAIL][15] ([i915#6367] / [i915#7913])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/bat-rpls-2/igt@i915_selftest@live@slpc.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/bat-rpls-2/igt@i915_selftest@live@slpc.html

  
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7852]: https://gitlab.freedesktop.org/drm/intel/issues/7852
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#8073]: https://gitlab.freedesktop.org/drm/intel/issues/8073
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308


Build changes
-------------

  * Linux: CI_DRM_12911 -> Patchwork_114868v4

  CI-20190529: 20190529
  CI_DRM_12911: 57d579dfa8400021292fdca447983cfb59246061 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7218: 8036123f781059c54a31240756794b17bd3d15dc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_114868v4: 57d579dfa8400021292fdca447983cfb59246061 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

d7d06741d3b6 drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.
8355000cbe5d drm/i915/display: Restore dsparb_lock.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/index.html

[-- Attachment #2: Type: text/html, Size: 5885 bytes --]

[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)

[Patchwork]

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

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock. (rev4)
URL   : https://patchwork.freedesktop.org/series/114868/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12911_full -> Patchwork_114868v4_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_114868v4_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_114868v4_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_114868v4_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rps@reset:
    - shard-snb:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-snb7/igt@i915_pm_rps@reset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-snb4/igt@i915_pm_rps@reset.html

  
Known issues
------------

  Here are the changes found in Patchwork_114868v4_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-apl:          [PASS][3] -> [FAIL][4] ([i915#2842])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-apl2/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-apl7/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-glk:          NOTRUN -> [FAIL][5] ([i915#4767])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-glk3/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_vblank@pipe-d-wait-busy-hang:
    - shard-glk:          NOTRUN -> [SKIP][6] ([fdo#109271]) +33 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-glk3/igt@kms_vblank@pipe-d-wait-busy-hang.html

  
#### Possible fixes ####

  * igt@gem_eio@hibernate:
    - {shard-tglu}:       [ABORT][7] ([i915#7975]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-tglu-10/igt@gem_eio@hibernate.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-tglu-1/igt@gem_eio@hibernate.html

  * igt@gem_eio@reset-stress:
    - {shard-dg1}:        [FAIL][9] ([i915#5784]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-dg1-14/igt@gem_eio@reset-stress.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-dg1-15/igt@gem_eio@reset-stress.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [FAIL][11] ([i915#2842]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-glk1/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-glk2/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_reloc@basic-cpu-gtt:
    - {shard-rkl}:        [SKIP][13] ([i915#3281]) -> [PASS][14] +5 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-3/igt@gem_exec_reloc@basic-cpu-gtt.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-5/igt@gem_exec_reloc@basic-cpu-gtt.html

  * igt@gem_tiled_partial_pwrite_pread@writes:
    - {shard-rkl}:        [SKIP][15] ([i915#3282]) -> [PASS][16] +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-3/igt@gem_tiled_partial_pwrite_pread@writes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-5/igt@gem_tiled_partial_pwrite_pread@writes.html

  * igt@gen9_exec_parse@batch-zero-length:
    - {shard-rkl}:        [SKIP][17] ([i915#2527]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-3/igt@gen9_exec_parse@batch-zero-length.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-5/igt@gen9_exec_parse@batch-zero-length.html

  * igt@i915_pm_rpm@modeset-lpsp:
    - {shard-dg1}:        [SKIP][19] ([i915#1397]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-dg1-16/igt@i915_pm_rpm@modeset-lpsp.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-dg1-14/igt@i915_pm_rpm@modeset-lpsp.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-apl:          [DMESG-FAIL][21] ([i915#5334]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-apl7/igt@i915_selftest@live@gt_heartbeat.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-apl1/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_atomic@plane-immutable-zpos:
    - {shard-tglu}:       [SKIP][23] ([i915#1845] / [i915#7651]) -> [PASS][24] +12 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-tglu-9/igt@kms_atomic@plane-immutable-zpos.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-tglu-2/igt@kms_atomic@plane-immutable-zpos.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - {shard-rkl}:        [SKIP][25] ([i915#1845] / [i915#4098]) -> [PASS][26] +12 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-5/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-6/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [FAIL][27] ([i915#2346]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-apl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - {shard-tglu}:       [SKIP][29] ([i915#1849]) -> [PASS][30] +8 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-tglu-9/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-tglu-2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbc-tiling-linear:
    - {shard-rkl}:        [SKIP][31] ([i915#1849] / [i915#4098]) -> [PASS][32] +16 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-2/igt@kms_frontbuffer_tracking@fbc-tiling-linear.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-tiling-linear.html

  * igt@kms_properties@plane-properties-atomic:
    - {shard-rkl}:        [SKIP][33] ([i915#1849]) -> [PASS][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-2/igt@kms_properties@plane-properties-atomic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-6/igt@kms_properties@plane-properties-atomic.html

  * igt@kms_psr@cursor_render:
    - {shard-rkl}:        [SKIP][35] ([i915#1072]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-2/igt@kms_psr@cursor_render.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-6/igt@kms_psr@cursor_render.html

  * igt@kms_vblank@pipe-d-query-forked:
    - {shard-tglu}:       [SKIP][37] ([i915#1845]) -> [PASS][38] +23 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-tglu-10/igt@kms_vblank@pipe-d-query-forked.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-tglu-6/igt@kms_vblank@pipe-d-query-forked.html

  * {igt@perf@gen12-group-exclusive-stream-ctx-handle}:
    - {shard-rkl}:        [SKIP][39] -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-5/igt@perf@gen12-group-exclusive-stream-ctx-handle.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-6/igt@perf@gen12-group-exclusive-stream-ctx-handle.html

  * igt@perf@mi-rpc:
    - {shard-rkl}:        [SKIP][41] ([i915#2434]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-rkl-3/igt@perf@mi-rpc.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-rkl-5/igt@perf@mi-rpc.html

  * {igt@perf@stress-open-close@0-rcs0}:
    - shard-glk:          [ABORT][43] -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-glk4/igt@perf@stress-open-close@0-rcs0.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-glk3/igt@perf@stress-open-close@0-rcs0.html

  * igt@sysfs_timeslice_duration@timeout@vecs0:
    - {shard-dg1}:        [FAIL][45] ([i915#1755]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12911/shard-dg1-16/igt@sysfs_timeslice_duration@timeout@vecs0.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/shard-dg1-14/igt@sysfs_timeslice_duration@timeout@vecs0.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/2
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1722]: https://gitlab.freedesktop.org/drm/intel/issues/1722
  [i915#1755]: https://gitlab.freedesktop.org/drm/intel/issues/1755
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1850]: https://gitlab.freedesktop.org/drm/intel/issues/1850
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2433]: https://gitlab.freedesktop.org/drm/intel/issues/2433
  [i915#2434]: https://gitlab.freedesktop.org/drm/intel/issues/2434
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2532]: https://gitlab.freedesktop.org/drm/intel/issues/2532
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3361]: https://gitlab.freedesktop.org/drm/intel/issues/3361
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
  [i915#3826]: https://gitlab.freedesktop.org/drm/intel/issues/3826
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3936]: https://gitlab.freedesktop.org/drm/intel/issues/3936
  [i915#3952]: https://gitlab.freedesktop.org/drm/intel/issues/3952
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4854]: https://gitlab.freedesktop.org/drm/intel/issues/4854
  [i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4885]: https://gitlab.freedesktop.org/drm/intel/issues/4885
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6227]: https://gitlab.freedesktop.org/drm/intel/issues/6227
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6355]: https://gitlab.freedesktop.org/drm/intel/issues/6355
  [i915#6493]: https://gitlab.freedesktop.org/drm/intel/issues/6493
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#7037]: https://gitlab.freedesktop.org/drm/intel/issues/7037
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7128]: https://gitlab.freedesktop.org/drm/intel/issues/7128
  [i915#7294]: https://gitlab.freedesktop.org/drm/intel/issues/7294
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7949]: https://gitlab.freedesktop.org/drm/intel/issues/7949
  [i915#7957]: https://gitlab.freedesktop.org/drm/intel/issues/7957
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8152]: https://gitlab.freedesktop.org/drm/intel/issues/8152
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308


Build changes
-------------

  * Linux: CI_DRM_12911 -> Patchwork_114868v4

  CI-20190529: 20190529
  CI_DRM_12911: 57d579dfa8400021292fdca447983cfb59246061 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7218: 8036123f781059c54a31240756794b17bd3d15dc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_114868v4: 57d579dfa8400021292fdca447983cfb59246061 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v4/index.html

[-- Attachment #2: Type: text/html, Size: 13179 bytes --]

[Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

uncore->lock only protects the forcewake domain itself,
not the register accesses.

uncore's _fw alternatives are for cases where the domains
are not needed because we are sure that they are already
awake.

So the move towards the uncore's _fw alternatives seems
right, however using the uncore-lock to protect the dsparb
registers seems an abuse of the uncore-lock.

Let's restore the previous individual lock and try to get
rid of the direct uncore accesses from the display code.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-1-rodrigo.vivi@intel.com
---
 drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
 drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
 drivers/gpu/drm/i915/i915_driver.c                |  1 +
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
index caef72d38798..8fe0b5c63d3a 100644
--- a/drivers/gpu/drm/i915/display/i9xx_wm.c
+++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
@@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
 
-	/*
-	 * uncore.lock serves a double purpose here. It allows us to
-	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
-	 * it protects the DSPARB registers from getting clobbered by
-	 * parallel updates from multiple pipes.
-	 *
-	 * intel_pipe_update_start() has already disabled interrupts
-	 * for us, so a plain spin_lock() is sufficient here.
-	 */
-	spin_lock(&uncore->lock);
+	spin_lock(&dev_priv->display.wm.dsparb_lock);
 
 	switch (crtc->pipe) {
 	case PIPE_A:
@@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	intel_uncore_posting_read_fw(uncore, DSPARB);
 
-	spin_unlock(&uncore->lock);
+	spin_unlock(&dev_priv->display.wm.dsparb_lock);
 }
 
 #undef VLV_FIFO
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index 0b5509f268a7..e4da8902c878 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -264,6 +264,9 @@ struct intel_wm {
 	 */
 	struct mutex wm_mutex;
 
+	/* protects DSPARB registers on pre-g4x/vlv/chv */
+	spinlock_t dsparb_lock;
+
 	bool ipc_enabled;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 12b5296ee744..e90a0c0403a6 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->display.pps.mutex);
 	mutex_init(&dev_priv->display.hdcp.comp_mutex);
 	spin_lock_init(&dev_priv->display.dkl.phy_lock);
+	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
 
 	i915_memcpy_init_early(dev_priv);
 	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
-- 
2.39.2

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Jani Nikula]

On Mon, 27 Mar 2023, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> uncore->lock only protects the forcewake domain itself,
> not the register accesses.
>
> uncore's _fw alternatives are for cases where the domains
> are not needed because we are sure that they are already
> awake.
>
> So the move towards the uncore's _fw alternatives seems
> right, however using the uncore-lock to protect the dsparb
> registers seems an abuse of the uncore-lock.
>
> Let's restore the previous individual lock and try to get
> rid of the direct uncore accesses from the display code.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-1-rodrigo.vivi@intel.com
> ---
>  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
>  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
>  drivers/gpu/drm/i915/i915_driver.c                |  1 +
>  3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> index caef72d38798..8fe0b5c63d3a 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
>  
> -	/*
> -	 * uncore.lock serves a double purpose here. It allows us to
> -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> -	 * it protects the DSPARB registers from getting clobbered by
> -	 * parallel updates from multiple pipes.
> -	 *
> -	 * intel_pipe_update_start() has already disabled interrupts
> -	 * for us, so a plain spin_lock() is sufficient here.
> -	 */
> -	spin_lock(&uncore->lock);
> +	spin_lock(&dev_priv->display.wm.dsparb_lock);
>  
>  	switch (crtc->pipe) {
>  	case PIPE_A:
> @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	intel_uncore_posting_read_fw(uncore, DSPARB);
>  
> -	spin_unlock(&uncore->lock);
> +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
>  }
>  
>  #undef VLV_FIFO
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 0b5509f268a7..e4da8902c878 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -264,6 +264,9 @@ struct intel_wm {
>  	 */
>  	struct mutex wm_mutex;
>  
> +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> +	spinlock_t dsparb_lock;
> +
>  	bool ipc_enabled;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 12b5296ee744..e90a0c0403a6 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  	mutex_init(&dev_priv->display.pps.mutex);
>  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
>  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);

Can we do this in i9xx_wm_init() instead?


>  
>  	i915_memcpy_init_early(dev_priv);
>  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

On Tue, Mar 28, 2023 at 07:22:24PM +0300, Jani Nikula wrote:
> On Mon, 27 Mar 2023, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > uncore->lock only protects the forcewake domain itself,
> > not the register accesses.
> >
> > uncore's _fw alternatives are for cases where the domains
> > are not needed because we are sure that they are already
> > awake.
> >
> > So the move towards the uncore's _fw alternatives seems
> > right, however using the uncore-lock to protect the dsparb
> > registers seems an abuse of the uncore-lock.
> >
> > Let's restore the previous individual lock and try to get
> > rid of the direct uncore accesses from the display code.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-1-rodrigo.vivi@intel.com
> > ---
> >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> >  3 files changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > index caef72d38798..8fe0b5c63d3a 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> >  
> > -	/*
> > -	 * uncore.lock serves a double purpose here. It allows us to
> > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > -	 * it protects the DSPARB registers from getting clobbered by
> > -	 * parallel updates from multiple pipes.
> > -	 *
> > -	 * intel_pipe_update_start() has already disabled interrupts
> > -	 * for us, so a plain spin_lock() is sufficient here.
> > -	 */
> > -	spin_lock(&uncore->lock);
> > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> >  
> >  	switch (crtc->pipe) {
> >  	case PIPE_A:
> > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> >  
> > -	spin_unlock(&uncore->lock);
> > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> >  }
> >  
> >  #undef VLV_FIFO
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > index 0b5509f268a7..e4da8902c878 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > @@ -264,6 +264,9 @@ struct intel_wm {
> >  	 */
> >  	struct mutex wm_mutex;
> >  
> > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > +	spinlock_t dsparb_lock;
> > +
> >  	bool ipc_enabled;
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index 12b5296ee744..e90a0c0403a6 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> >  	mutex_init(&dev_priv->display.pps.mutex);
> >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> 
> Can we do this in i9xx_wm_init() instead?

I was going to modify it here right now, but then I noticed
the cases above and remembered why I have put it here.
All the display locks are getting set in here.

Probably better to move with this patch as is and then add
a new on top moving the various locks to its individual inits?

> 
> 
> >  
> >  	i915_memcpy_init_early(dev_priv);
> >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center