[PATCH 0/5] drm/i915: Optimize plane updates a bit

[PATCH 0/5] drm/i915: Optimize plane updates a bit

[ville.syrjala]

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

Now that commit e1edbd44e23b ("drm/i915: Complain if we take too
long under vblank evasion.") has expose just how badly we suck,
it seems like a good time to optimize things a little bit.

Prior to this one of my VLV machines exceed the 100 usec vblank
evade deadline pretty regularly, and at ever boot I was hitting
numbers as high as 500 usec. Granted that was with lockdep and
all kinds of other debug things enabled. After these changes
things seem stay below the 33 usec mark, and with all that debug
junk enabled we seem to staying below 22 usec.

Note that I was testing single plane updates mostly, so I'm
not 100% sure multi plane updates couldn't still exceed the
deadline. That will need to be checked.

Ville Syrjälä (5):
  drm/i915: Use I915_READ_FW in i915_get_vblank_counter()
  drm/i915: s/__raw_i915_read32/I915_READ_FW/ in the SKL+ scanline read
    w/a
  drm/i915: Organize plane register writes into tighter bunches
  drm/i915: Use I915_READ_FW for plane updates
  drm/i915: Optimize VLV/CHV display FIFO updates

 drivers/gpu/drm/i915/i915_drv.c      |   1 -
 drivers/gpu/drm/i915/i915_drv.h      |   3 -
 drivers/gpu/drm/i915/i915_irq.c      |  14 +-
 drivers/gpu/drm/i915/intel_display.c | 176 ++++++++++++++---------
 drivers/gpu/drm/i915/intel_pm.c      |  36 +++--
 drivers/gpu/drm/i915/intel_sprite.c  | 269 ++++++++++++++++++++---------------
 6 files changed, 290 insertions(+), 209 deletions(-)

-- 
2.10.2

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

[PATCH 1/5] drm/i915: Use I915_READ_FW in i915_get_vblank_counter()

[ville.syrjala]

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

Optimize the multi-register read in i915_get_vblank_counter() a little
bit by grabbing the uncore lock manually and using I915_READ_FW().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 22ac70374ac1..70214e1fcd10 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -728,6 +728,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
 								pipe);
 	const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
+	unsigned long irqflags;
 
 	htotal = mode->crtc_htotal;
 	hsync_start = mode->crtc_hsync_start;
@@ -744,17 +745,21 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	high_frame = PIPEFRAME(pipe);
 	low_frame = PIPEFRAMEPIXEL(pipe);
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	/*
 	 * High & low register fields aren't synchronized, so make sure
 	 * we get a low value that's stable across two reads of the high
 	 * register.
 	 */
 	do {
-		high1 = I915_READ(high_frame) & PIPE_FRAME_HIGH_MASK;
-		low   = I915_READ(low_frame);
-		high2 = I915_READ(high_frame) & PIPE_FRAME_HIGH_MASK;
+		high1 = I915_READ_FW(high_frame) & PIPE_FRAME_HIGH_MASK;
+		low   = I915_READ_FW(low_frame);
+		high2 = I915_READ_FW(high_frame) & PIPE_FRAME_HIGH_MASK;
 	} while (high1 != high2);
 
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
 	high1 >>= PIPE_FRAME_HIGH_SHIFT;
 	pixel = low & PIPE_PIXEL_MASK;
 	low >>= PIPE_FRAME_LOW_SHIFT;
-- 
2.10.2

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

[PATCH 2/5] drm/i915: s/__raw_i915_read32/I915_READ_FW/ in the SKL+ scanline read w/a

[ville.syrjala]

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

Replace __raw_i915_read32() with I915_READ_FW() in the workaround for
the SKL+ scanline counter hardware fail. The two are the same thing
but everyone else uses I915_READ_FW() so let's follow suit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 70214e1fcd10..3c2109b4be2c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -817,8 +817,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 
 		for (i = 0; i < 100; i++) {
 			udelay(1);
-			temp = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) &
-				DSL_LINEMASK_GEN3;
+			temp = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
 			if (temp != position) {
 				position = temp;
 				break;
-- 
2.10.2

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

[PATCH 3/5] drm/i915: Organize plane register writes into tighter bunches

[ville.syrjala]

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

Pull all the plane register writes closer together to avoid having
a lot of unrelated stuff in between them. This will make things more
clear once we'll grab the uncore lock around the entire bunch. Also
in the future we might even consider moving more of the register
value computation out from the plane update hooks. This should make
that easier to do.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++++++------------
 drivers/gpu/drm/i915/intel_sprite.c  | 59 ++++++++++++++++++------------------
 2 files changed, 54 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5526a196e8a2..253f064bde3e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2984,20 +2984,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	if (INTEL_GEN(dev_priv) < 4) {
 		if (intel_crtc->pipe == PIPE_B)
 			dspcntr |= DISPPLANE_SEL_PIPE_B;
-
-		/* pipesrc and dspsize control the size that is scaled from,
-		 * which should always be the user's requested size.
-		 */
-		I915_WRITE(DSPSIZE(plane),
-			   ((crtc_state->pipe_src_h - 1) << 16) |
-			   (crtc_state->pipe_src_w - 1));
-		I915_WRITE(DSPPOS(plane), 0);
-	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
-		I915_WRITE(PRIMSIZE(plane),
-			   ((crtc_state->pipe_src_h - 1) << 16) |
-			   (crtc_state->pipe_src_w - 1));
-		I915_WRITE(PRIMPOS(plane), 0);
-		I915_WRITE(PRIMCNSTALPHA(plane), 0);
 	}
 
 	switch (fb->format->format) {
@@ -3060,6 +3046,22 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	intel_crtc->adjusted_x = x;
 	intel_crtc->adjusted_y = y;
 
+	if (INTEL_GEN(dev_priv) < 4) {
+		/* pipesrc and dspsize control the size that is scaled from,
+		 * which should always be the user's requested size.
+		 */
+		I915_WRITE(DSPSIZE(plane),
+			   ((crtc_state->pipe_src_h - 1) << 16) |
+			   (crtc_state->pipe_src_w - 1));
+		I915_WRITE(DSPPOS(plane), 0);
+	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
+		I915_WRITE(PRIMSIZE(plane),
+			   ((crtc_state->pipe_src_h - 1) << 16) |
+			   (crtc_state->pipe_src_w - 1));
+		I915_WRITE(PRIMPOS(plane), 0);
+		I915_WRITE(PRIMCNSTALPHA(plane), 0);
+	}
+
 	I915_WRITE(reg, dspcntr);
 
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
@@ -3344,12 +3346,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 
 	plane_ctl = PLANE_CTL_ENABLE;
 
-	if (IS_GEMINILAKE(dev_priv)) {
-		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
-			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
-			   PLANE_COLOR_PIPE_CSC_ENABLE |
-			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
-	} else {
+	if (!IS_GEMINILAKE(dev_priv)) {
 		plane_ctl |=
 			PLANE_CTL_PIPE_GAMMA_ENABLE |
 			PLANE_CTL_PIPE_CSC_ENABLE |
@@ -3371,6 +3368,13 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_x = src_x;
 	intel_crtc->adjusted_y = src_y;
 
+	if (IS_GEMINILAKE(dev_priv)) {
+		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
+			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
+			   PLANE_COLOR_PIPE_CSC_ENABLE |
+			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
+	}
+
 	I915_WRITE(PLANE_CTL(pipe, plane_id), plane_ctl);
 	I915_WRITE(PLANE_OFFSET(pipe, plane_id), (src_y << 16) | src_x);
 	I915_WRITE(PLANE_STRIDE(pipe, plane_id), stride);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 300776b9e935..532db7d62290 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -233,12 +233,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 
 	plane_ctl = PLANE_CTL_ENABLE;
 
-	if (IS_GEMINILAKE(dev_priv)) {
-		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
-			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
-			   PLANE_COLOR_PIPE_CSC_ENABLE |
-			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
-	} else {
+	if (!IS_GEMINILAKE(dev_priv)) {
 		plane_ctl |=
 			PLANE_CTL_PIPE_GAMMA_ENABLE |
 			PLANE_CTL_PIPE_CSC_ENABLE |
@@ -249,12 +244,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
-	if (key->flags) {
-		I915_WRITE(PLANE_KEYVAL(pipe, plane_id), key->min_value);
-		I915_WRITE(PLANE_KEYMAX(pipe, plane_id), key->max_value);
-		I915_WRITE(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
-	}
-
 	if (key->flags & I915_SET_COLORKEY_DESTINATION)
 		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
 	else if (key->flags & I915_SET_COLORKEY_SOURCE)
@@ -266,6 +255,19 @@ skl_update_plane(struct drm_plane *drm_plane,
 	crtc_w--;
 	crtc_h--;
 
+	if (IS_GEMINILAKE(dev_priv)) {
+		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
+			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
+			   PLANE_COLOR_PIPE_CSC_ENABLE |
+			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
+	}
+
+	if (key->flags) {
+		I915_WRITE(PLANE_KEYVAL(pipe, plane_id), key->min_value);
+		I915_WRITE(PLANE_KEYMAX(pipe, plane_id), key->max_value);
+		I915_WRITE(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
+	}
+
 	I915_WRITE(PLANE_OFFSET(pipe, plane_id), (y << 16) | x);
 	I915_WRITE(PLANE_STRIDE(pipe, plane_id), stride);
 	I915_WRITE(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
@@ -433,6 +435,9 @@ vlv_update_plane(struct drm_plane *dplane,
 	if (rotation & DRM_REFLECT_X)
 		sprctl |= SP_MIRROR;
 
+	if (key->flags & I915_SET_COLORKEY_SOURCE)
+		sprctl |= SP_SOURCE_KEY;
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -451,18 +456,14 @@ vlv_update_plane(struct drm_plane *dplane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
+	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
+		chv_update_csc(intel_plane, fb->format->format);
+
 	if (key->flags) {
 		I915_WRITE(SPKEYMINVAL(pipe, plane_id), key->min_value);
 		I915_WRITE(SPKEYMAXVAL(pipe, plane_id), key->max_value);
 		I915_WRITE(SPKEYMSK(pipe, plane_id), key->channel_mask);
 	}
-
-	if (key->flags & I915_SET_COLORKEY_SOURCE)
-		sprctl |= SP_SOURCE_KEY;
-
-	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
-		chv_update_csc(intel_plane, fb->format->format);
-
 	I915_WRITE(SPSTRIDE(pipe, plane_id), fb->pitches[0]);
 	I915_WRITE(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x);
 
@@ -563,6 +564,11 @@ ivb_update_plane(struct drm_plane *plane,
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		sprctl |= SPRITE_PIPE_CSC_ENABLE;
 
+	if (key->flags & I915_SET_COLORKEY_DESTINATION)
+		sprctl |= SPRITE_DEST_KEY;
+	else if (key->flags & I915_SET_COLORKEY_SOURCE)
+		sprctl |= SPRITE_SOURCE_KEY;
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -590,11 +596,6 @@ ivb_update_plane(struct drm_plane *plane,
 		I915_WRITE(SPRKEYMSK(pipe), key->channel_mask);
 	}
 
-	if (key->flags & I915_SET_COLORKEY_DESTINATION)
-		sprctl |= SPRITE_DEST_KEY;
-	else if (key->flags & I915_SET_COLORKEY_SOURCE)
-		sprctl |= SPRITE_SOURCE_KEY;
-
 	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
 
@@ -696,6 +697,11 @@ ilk_update_plane(struct drm_plane *plane,
 	if (IS_GEN6(dev_priv))
 		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 
+	if (key->flags & I915_SET_COLORKEY_DESTINATION)
+		dvscntr |= DVS_DEST_KEY;
+	else if (key->flags & I915_SET_COLORKEY_SOURCE)
+		dvscntr |= DVS_SOURCE_KEY;
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -722,11 +728,6 @@ ilk_update_plane(struct drm_plane *plane,
 		I915_WRITE(DVSKEYMSK(pipe), key->channel_mask);
 	}
 
-	if (key->flags & I915_SET_COLORKEY_DESTINATION)
-		dvscntr |= DVS_DEST_KEY;
-	else if (key->flags & I915_SET_COLORKEY_SOURCE)
-		dvscntr |= DVS_SOURCE_KEY;
-
 	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
 
-- 
2.10.2

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

[PATCH 4/5] drm/i915: Use I915_READ_FW for plane updates

[ville.syrjala]

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

Optimize the plane register accesses a little bit by grabbing
the uncore lock manually across the entire pile of accesses and
using I915_READ_FW().

This helps keep the pipe update vblank evade critical section
below our 100 usec deadline, particularly with lockdep enabled.
And in general we want to keep that critical section as short
as possible as it's executed with interrupts disabled.

Not all plane updates currently happen from within the vblank evade
critical section, so we must use the irqsave/irqrestore variants
of the spinlock functions in the plane hooks.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 158 ++++++++++++++----------
 drivers/gpu/drm/i915/intel_sprite.c  | 228 ++++++++++++++++++++---------------
 2 files changed, 228 insertions(+), 158 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 253f064bde3e..abd05e0d7896 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2976,6 +2976,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	unsigned int rotation = plane_state->base.rotation;
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
+	unsigned long irqflags;
 
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
 
@@ -3046,37 +3047,41 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	intel_crtc->adjusted_x = x;
 	intel_crtc->adjusted_y = y;
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (INTEL_GEN(dev_priv) < 4) {
 		/* pipesrc and dspsize control the size that is scaled from,
 		 * which should always be the user's requested size.
 		 */
-		I915_WRITE(DSPSIZE(plane),
-			   ((crtc_state->pipe_src_h - 1) << 16) |
-			   (crtc_state->pipe_src_w - 1));
-		I915_WRITE(DSPPOS(plane), 0);
+		I915_WRITE_FW(DSPSIZE(plane),
+			      ((crtc_state->pipe_src_h - 1) << 16) |
+			      (crtc_state->pipe_src_w - 1));
+		I915_WRITE_FW(DSPPOS(plane), 0);
 	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
-		I915_WRITE(PRIMSIZE(plane),
-			   ((crtc_state->pipe_src_h - 1) << 16) |
-			   (crtc_state->pipe_src_w - 1));
-		I915_WRITE(PRIMPOS(plane), 0);
-		I915_WRITE(PRIMCNSTALPHA(plane), 0);
+		I915_WRITE_FW(PRIMSIZE(plane),
+			      ((crtc_state->pipe_src_h - 1) << 16) |
+			      (crtc_state->pipe_src_w - 1));
+		I915_WRITE_FW(PRIMPOS(plane), 0);
+		I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
 	}
 
-	I915_WRITE(reg, dspcntr);
+	I915_WRITE_FW(reg, dspcntr);
 
-	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
+	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
 	if (INTEL_GEN(dev_priv) >= 4) {
-		I915_WRITE(DSPSURF(plane),
-			   intel_plane_ggtt_offset(plane_state) +
-			   intel_crtc->dspaddr_offset);
-		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
-		I915_WRITE(DSPLINOFF(plane), linear_offset);
+		I915_WRITE_FW(DSPSURF(plane),
+			      intel_plane_ggtt_offset(plane_state) +
+			      intel_crtc->dspaddr_offset);
+		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
 	} else {
-		I915_WRITE(DSPADDR(plane),
-			   intel_plane_ggtt_offset(plane_state) +
-			   intel_crtc->dspaddr_offset);
+		I915_WRITE_FW(DSPADDR(plane),
+			      intel_plane_ggtt_offset(plane_state) +
+			      intel_crtc->dspaddr_offset);
 	}
-	POSTING_READ(reg);
+	POSTING_READ_FW(reg);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void i9xx_disable_primary_plane(struct drm_plane *primary,
@@ -3086,13 +3091,18 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int plane = intel_crtc->plane;
+	unsigned long irqflags;
 
-	I915_WRITE(DSPCNTR(plane), 0);
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	I915_WRITE_FW(DSPCNTR(plane), 0);
 	if (INTEL_INFO(dev_priv)->gen >= 4)
-		I915_WRITE(DSPSURF(plane), 0);
+		I915_WRITE_FW(DSPSURF(plane), 0);
 	else
-		I915_WRITE(DSPADDR(plane), 0);
-	POSTING_READ(DSPCNTR(plane));
+		I915_WRITE_FW(DSPADDR(plane), 0);
+	POSTING_READ_FW(DSPCNTR(plane));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void ironlake_update_primary_plane(struct drm_plane *primary,
@@ -3110,6 +3120,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	unsigned int rotation = plane_state->base.rotation;
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
+	unsigned long irqflags;
 
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
 	dspcntr |= DISPLAY_PLANE_ENABLE;
@@ -3166,19 +3177,23 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	intel_crtc->adjusted_x = x;
 	intel_crtc->adjusted_y = y;
 
-	I915_WRITE(reg, dspcntr);
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	I915_WRITE_FW(reg, dspcntr);
 
-	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
-	I915_WRITE(DSPSURF(plane),
-		   intel_plane_ggtt_offset(plane_state) +
-		   intel_crtc->dspaddr_offset);
+	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
+	I915_WRITE_FW(DSPSURF(plane),
+		      intel_plane_ggtt_offset(plane_state) +
+		      intel_crtc->dspaddr_offset);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
 	} else {
-		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
-		I915_WRITE(DSPLINOFF(plane), linear_offset);
+		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
 	}
-	POSTING_READ(reg);
+	POSTING_READ_FW(reg);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32
@@ -3343,6 +3358,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	int dst_y = plane_state->base.dst.y1;
 	int dst_w = drm_rect_width(&plane_state->base.dst);
 	int dst_h = drm_rect_height(&plane_state->base.dst);
+	unsigned long irqflags;
 
 	plane_ctl = PLANE_CTL_ENABLE;
 
@@ -3368,17 +3384,19 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_x = src_x;
 	intel_crtc->adjusted_y = src_y;
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (IS_GEMINILAKE(dev_priv)) {
-		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
-			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
-			   PLANE_COLOR_PIPE_CSC_ENABLE |
-			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
+		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
+			      PLANE_COLOR_PIPE_GAMMA_ENABLE |
+			      PLANE_COLOR_PIPE_CSC_ENABLE |
+			      PLANE_COLOR_PLANE_GAMMA_DISABLE);
 	}
 
-	I915_WRITE(PLANE_CTL(pipe, plane_id), plane_ctl);
-	I915_WRITE(PLANE_OFFSET(pipe, plane_id), (src_y << 16) | src_x);
-	I915_WRITE(PLANE_STRIDE(pipe, plane_id), stride);
-	I915_WRITE(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
+	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl);
+	I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (src_y << 16) | src_x);
+	I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride);
+	I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
 
 	if (scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
@@ -3386,19 +3404,21 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 		WARN_ON(!dst_w || !dst_h);
 		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
 			crtc_state->scaler_state.scalers[scaler_id].mode;
-		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
-		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
-		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x << 16) | dst_y);
-		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
-		I915_WRITE(PLANE_POS(pipe, plane_id), 0);
+		I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
+		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
+		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x << 16) | dst_y);
+		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
+		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
 	} else {
-		I915_WRITE(PLANE_POS(pipe, plane_id), (dst_y << 16) | dst_x);
+		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (dst_y << 16) | dst_x);
 	}
 
-	I915_WRITE(PLANE_SURF(pipe, plane_id),
-		   intel_plane_ggtt_offset(plane_state) + surf_addr);
+	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
+		      intel_plane_ggtt_offset(plane_state) + surf_addr);
+
+	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 
-	POSTING_READ(PLANE_SURF(pipe, plane_id));
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void skylake_disable_primary_plane(struct drm_plane *primary,
@@ -3408,10 +3428,15 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum plane_id plane_id = to_intel_plane(primary)->id;
 	enum pipe pipe = to_intel_plane(primary)->pipe;
+	unsigned long irqflags;
 
-	I915_WRITE(PLANE_CTL(pipe, plane_id), 0);
-	I915_WRITE(PLANE_SURF(pipe, plane_id), 0);
-	POSTING_READ(PLANE_SURF(pipe, plane_id));
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
+	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
+	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 /* Assume fb object is pinned & idle & fenced and just update base pointers */
@@ -9174,24 +9199,24 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 		/* On these chipsets we can only modify the base/size/stride
 		 * whilst the cursor is disabled.
 		 */
-		I915_WRITE(CURCNTR(PIPE_A), 0);
-		POSTING_READ(CURCNTR(PIPE_A));
+		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
+		POSTING_READ_FW(CURCNTR(PIPE_A));
 		intel_crtc->cursor_cntl = 0;
 	}
 
 	if (intel_crtc->cursor_base != base) {
-		I915_WRITE(CURBASE(PIPE_A), base);
+		I915_WRITE_FW(CURBASE(PIPE_A), base);
 		intel_crtc->cursor_base = base;
 	}
 
 	if (intel_crtc->cursor_size != size) {
-		I915_WRITE(CURSIZE, size);
+		I915_WRITE_FW(CURSIZE, size);
 		intel_crtc->cursor_size = size;
 	}
 
 	if (intel_crtc->cursor_cntl != cntl) {
-		I915_WRITE(CURCNTR(PIPE_A), cntl);
-		POSTING_READ(CURCNTR(PIPE_A));
+		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
+		POSTING_READ_FW(CURCNTR(PIPE_A));
 		intel_crtc->cursor_cntl = cntl;
 	}
 }
@@ -9231,14 +9256,14 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	}
 
 	if (intel_crtc->cursor_cntl != cntl) {
-		I915_WRITE(CURCNTR(pipe), cntl);
-		POSTING_READ(CURCNTR(pipe));
+		I915_WRITE_FW(CURCNTR(pipe), cntl);
+		POSTING_READ_FW(CURCNTR(pipe));
 		intel_crtc->cursor_cntl = cntl;
 	}
 
 	/* and commit changes on next vblank */
-	I915_WRITE(CURBASE(pipe), base);
-	POSTING_READ(CURBASE(pipe));
+	I915_WRITE_FW(CURBASE(pipe), base);
+	POSTING_READ_FW(CURBASE(pipe));
 
 	intel_crtc->cursor_base = base;
 }
@@ -9252,6 +9277,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
 	u32 base = intel_crtc->cursor_addr;
+	unsigned long irqflags;
 	u32 pos = 0;
 
 	if (plane_state) {
@@ -9278,12 +9304,16 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		}
 	}
 
-	I915_WRITE(CURPOS(pipe), pos);
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	I915_WRITE_FW(CURPOS(pipe), pos);
 
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
 		i845_update_cursor(crtc, base, plane_state);
 	else
 		i9xx_update_cursor(crtc, base, plane_state);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static bool cursor_size_ok(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 532db7d62290..b931d0bd7a64 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -230,6 +230,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	uint32_t y = plane_state->main.y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
 
 	plane_ctl = PLANE_CTL_ENABLE;
 
@@ -255,22 +256,24 @@ skl_update_plane(struct drm_plane *drm_plane,
 	crtc_w--;
 	crtc_h--;
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (IS_GEMINILAKE(dev_priv)) {
-		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
-			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
-			   PLANE_COLOR_PIPE_CSC_ENABLE |
-			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
+		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
+			      PLANE_COLOR_PIPE_GAMMA_ENABLE |
+			      PLANE_COLOR_PIPE_CSC_ENABLE |
+			      PLANE_COLOR_PLANE_GAMMA_DISABLE);
 	}
 
 	if (key->flags) {
-		I915_WRITE(PLANE_KEYVAL(pipe, plane_id), key->min_value);
-		I915_WRITE(PLANE_KEYMAX(pipe, plane_id), key->max_value);
-		I915_WRITE(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
+		I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
+		I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
+		I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), key->channel_mask);
 	}
 
-	I915_WRITE(PLANE_OFFSET(pipe, plane_id), (y << 16) | x);
-	I915_WRITE(PLANE_STRIDE(pipe, plane_id), stride);
-	I915_WRITE(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
+	I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x);
+	I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride);
+	I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
 
 	/* program plane scaler */
 	if (plane_state->scaler_id >= 0) {
@@ -279,22 +282,24 @@ skl_update_plane(struct drm_plane *drm_plane,
 
 		scaler = &crtc_state->scaler_state.scalers[scaler_id];
 
-		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id),
-			   PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode);
-		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
-		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
-		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id),
-			((crtc_w + 1) << 16)|(crtc_h + 1));
+		I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id),
+			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode);
+		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
+		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
+		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
+			      ((crtc_w + 1) << 16)|(crtc_h + 1));
 
-		I915_WRITE(PLANE_POS(pipe, plane_id), 0);
+		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
 	} else {
-		I915_WRITE(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x);
+		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x);
 	}
 
-	I915_WRITE(PLANE_CTL(pipe, plane_id), plane_ctl);
-	I915_WRITE(PLANE_SURF(pipe, plane_id),
-		   intel_plane_ggtt_offset(plane_state) + surf_addr);
-	POSTING_READ(PLANE_SURF(pipe, plane_id));
+	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl);
+	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
+		      intel_plane_ggtt_offset(plane_state) + surf_addr);
+	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -305,11 +310,16 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
 	enum plane_id plane_id = intel_plane->id;
 	enum pipe pipe = intel_plane->pipe;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	I915_WRITE(PLANE_CTL(pipe, plane_id), 0);
+	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 
-	I915_WRITE(PLANE_SURF(pipe, plane_id), 0);
-	POSTING_READ(PLANE_SURF(pipe, plane_id));
+	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
+	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -332,23 +342,23 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
 	 * Cb and Cr apparently come in as signed already, so no
 	 * need for any offset. For Y we need to remove the offset.
 	 */
-	I915_WRITE(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(-64));
-	I915_WRITE(SPCSCCBOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
-	I915_WRITE(SPCSCCROFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
-
-	I915_WRITE(SPCSCC01(plane_id), SPCSC_C1(4769) | SPCSC_C0(6537));
-	I915_WRITE(SPCSCC23(plane_id), SPCSC_C1(-3330) | SPCSC_C0(0));
-	I915_WRITE(SPCSCC45(plane_id), SPCSC_C1(-1605) | SPCSC_C0(4769));
-	I915_WRITE(SPCSCC67(plane_id), SPCSC_C1(4769) | SPCSC_C0(0));
-	I915_WRITE(SPCSCC8(plane_id), SPCSC_C0(8263));
-
-	I915_WRITE(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(940) | SPCSC_IMIN(64));
-	I915_WRITE(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448));
-	I915_WRITE(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448));
-
-	I915_WRITE(SPCSCYGOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
-	I915_WRITE(SPCSCCBOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
-	I915_WRITE(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
+	I915_WRITE_FW(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(-64));
+	I915_WRITE_FW(SPCSCCBOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
+	I915_WRITE_FW(SPCSCCROFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
+
+	I915_WRITE_FW(SPCSCC01(plane_id), SPCSC_C1(4769) | SPCSC_C0(6537));
+	I915_WRITE_FW(SPCSCC23(plane_id), SPCSC_C1(-3330) | SPCSC_C0(0));
+	I915_WRITE_FW(SPCSCC45(plane_id), SPCSC_C1(-1605) | SPCSC_C0(4769));
+	I915_WRITE_FW(SPCSCC67(plane_id), SPCSC_C1(4769) | SPCSC_C0(0));
+	I915_WRITE_FW(SPCSCC8(plane_id), SPCSC_C0(8263));
+
+	I915_WRITE_FW(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(940) | SPCSC_IMIN(64));
+	I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448));
+	I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448));
+
+	I915_WRITE_FW(SPCSCYGOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
+	I915_WRITE_FW(SPCSCCBOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
+	I915_WRITE_FW(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
 }
 
 static void
@@ -374,6 +384,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	uint32_t y = plane_state->base.src.y1 >> 16;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
 
 	sprctl = SP_ENABLE;
 
@@ -456,29 +467,33 @@ vlv_update_plane(struct drm_plane *dplane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
 		chv_update_csc(intel_plane, fb->format->format);
 
 	if (key->flags) {
-		I915_WRITE(SPKEYMINVAL(pipe, plane_id), key->min_value);
-		I915_WRITE(SPKEYMAXVAL(pipe, plane_id), key->max_value);
-		I915_WRITE(SPKEYMSK(pipe, plane_id), key->channel_mask);
+		I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value);
+		I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value);
+		I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask);
 	}
-	I915_WRITE(SPSTRIDE(pipe, plane_id), fb->pitches[0]);
-	I915_WRITE(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x);
+	I915_WRITE_FW(SPSTRIDE(pipe, plane_id), fb->pitches[0]);
+	I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x);
 
 	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
-		I915_WRITE(SPTILEOFF(pipe, plane_id), (y << 16) | x);
+		I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x);
 	else
-		I915_WRITE(SPLINOFF(pipe, plane_id), linear_offset);
+		I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset);
 
-	I915_WRITE(SPCONSTALPHA(pipe, plane_id), 0);
+	I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0);
 
-	I915_WRITE(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w);
-	I915_WRITE(SPCNTR(pipe, plane_id), sprctl);
-	I915_WRITE(SPSURF(pipe, plane_id),
-		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
-	POSTING_READ(SPSURF(pipe, plane_id));
+	I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w);
+	I915_WRITE_FW(SPCNTR(pipe, plane_id), sprctl);
+	I915_WRITE_FW(SPSURF(pipe, plane_id),
+		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
+	POSTING_READ_FW(SPSURF(pipe, plane_id));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -489,11 +504,16 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
 	enum pipe pipe = intel_plane->pipe;
 	enum plane_id plane_id = intel_plane->id;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	I915_WRITE(SPCNTR(pipe, plane_id), 0);
+	I915_WRITE_FW(SPCNTR(pipe, plane_id), 0);
 
-	I915_WRITE(SPSURF(pipe, plane_id), 0);
-	POSTING_READ(SPSURF(pipe, plane_id));
+	I915_WRITE_FW(SPSURF(pipe, plane_id), 0);
+	POSTING_READ_FW(SPSURF(pipe, plane_id));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -518,6 +538,7 @@ ivb_update_plane(struct drm_plane *plane,
 	uint32_t y = plane_state->base.src.y1 >> 16;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
 
 	sprctl = SPRITE_ENABLE;
 
@@ -590,31 +611,35 @@ ivb_update_plane(struct drm_plane *plane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (key->flags) {
-		I915_WRITE(SPRKEYVAL(pipe), key->min_value);
-		I915_WRITE(SPRKEYMAX(pipe), key->max_value);
-		I915_WRITE(SPRKEYMSK(pipe), key->channel_mask);
+		I915_WRITE_FW(SPRKEYVAL(pipe), key->min_value);
+		I915_WRITE_FW(SPRKEYMAX(pipe), key->max_value);
+		I915_WRITE_FW(SPRKEYMSK(pipe), key->channel_mask);
 	}
 
-	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
-	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
+	I915_WRITE_FW(SPRSTRIDE(pipe), fb->pitches[0]);
+	I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
 
 	/* HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET
 	 * register */
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
+		I915_WRITE_FW(SPROFFSET(pipe), (y << 16) | x);
 	else if (fb->modifier == I915_FORMAT_MOD_X_TILED)
-		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
+		I915_WRITE_FW(SPRTILEOFF(pipe), (y << 16) | x);
 	else
-		I915_WRITE(SPRLINOFF(pipe), linear_offset);
+		I915_WRITE_FW(SPRLINOFF(pipe), linear_offset);
 
-	I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
+	I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
 	if (intel_plane->can_scale)
-		I915_WRITE(SPRSCALE(pipe), sprscale);
-	I915_WRITE(SPRCTL(pipe), sprctl);
-	I915_WRITE(SPRSURF(pipe),
-		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
-	POSTING_READ(SPRSURF(pipe));
+		I915_WRITE_FW(SPRSCALE(pipe), sprscale);
+	I915_WRITE_FW(SPRCTL(pipe), sprctl);
+	I915_WRITE_FW(SPRSURF(pipe),
+		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
+	POSTING_READ_FW(SPRSURF(pipe));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -624,14 +649,19 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	I915_WRITE(SPRCTL(pipe), 0);
+	I915_WRITE_FW(SPRCTL(pipe), 0);
 	/* Can't leave the scaler enabled... */
 	if (intel_plane->can_scale)
-		I915_WRITE(SPRSCALE(pipe), 0);
+		I915_WRITE_FW(SPRSCALE(pipe), 0);
 
-	I915_WRITE(SPRSURF(pipe), 0);
-	POSTING_READ(SPRSURF(pipe));
+	I915_WRITE_FW(SPRSURF(pipe), 0);
+	POSTING_READ_FW(SPRSURF(pipe));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -656,6 +686,7 @@ ilk_update_plane(struct drm_plane *plane,
 	uint32_t y = plane_state->base.src.y1 >> 16;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
 
 	dvscntr = DVS_ENABLE;
 
@@ -722,26 +753,30 @@ ilk_update_plane(struct drm_plane *plane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (key->flags) {
-		I915_WRITE(DVSKEYVAL(pipe), key->min_value);
-		I915_WRITE(DVSKEYMAX(pipe), key->max_value);
-		I915_WRITE(DVSKEYMSK(pipe), key->channel_mask);
+		I915_WRITE_FW(DVSKEYVAL(pipe), key->min_value);
+		I915_WRITE_FW(DVSKEYMAX(pipe), key->max_value);
+		I915_WRITE_FW(DVSKEYMSK(pipe), key->channel_mask);
 	}
 
-	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
-	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
+	I915_WRITE_FW(DVSSTRIDE(pipe), fb->pitches[0]);
+	I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
 
 	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
-		I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
+		I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x);
 	else
-		I915_WRITE(DVSLINOFF(pipe), linear_offset);
-
-	I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w);
-	I915_WRITE(DVSSCALE(pipe), dvsscale);
-	I915_WRITE(DVSCNTR(pipe), dvscntr);
-	I915_WRITE(DVSSURF(pipe),
-		   intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
-	POSTING_READ(DVSSURF(pipe));
+		I915_WRITE_FW(DVSLINOFF(pipe), linear_offset);
+
+	I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w);
+	I915_WRITE_FW(DVSSCALE(pipe), dvsscale);
+	I915_WRITE_FW(DVSCNTR(pipe), dvscntr);
+	I915_WRITE_FW(DVSSURF(pipe),
+		      intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
+	POSTING_READ_FW(DVSSURF(pipe));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -751,13 +786,18 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
+	unsigned long irqflags;
 
-	I915_WRITE(DVSCNTR(pipe), 0);
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	I915_WRITE_FW(DVSCNTR(pipe), 0);
 	/* Disable the scaler */
-	I915_WRITE(DVSSCALE(pipe), 0);
+	I915_WRITE_FW(DVSSCALE(pipe), 0);
+
+	I915_WRITE_FW(DVSSURF(pipe), 0);
+	POSTING_READ_FW(DVSSURF(pipe));
 
-	I915_WRITE(DVSSURF(pipe), 0);
-	POSTING_READ(DVSSURF(pipe));
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static int
-- 
2.10.2

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

[PATCH 5/5] drm/i915: Optimize VLV/CHV display FIFO updates

[ville.syrjala]

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

Use I915_{READ,WRITE}_FW() for updating the DSPARB registers on
VLV/CHV. This is less expesive as we can grab the uncore.lock across
the entire sequence of reads and writes instead of each register
access grabbing it.

This also allows us to eliminate the dsparb lock entirely as the
uncore.lock now effectively protects the contents of the DSPARB
registers.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  1 -
 drivers/gpu/drm/i915/i915_drv.h |  3 ---
 drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++++++---------------
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b1e9027a4f80..debb74a2b2a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -827,7 +827,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 
 	spin_lock_init(&dev_priv->mm.object_stat_lock);
 	spin_lock_init(&dev_priv->mmio_flip_lock);
-	spin_lock_init(&dev_priv->wm.dsparb_lock);
 	mutex_init(&dev_priv->sb_lock);
 	mutex_init(&dev_priv->modeset_restore_lock);
 	mutex_init(&dev_priv->av_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3002996ddbed..6af0b1d33cab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2375,9 +2375,6 @@ struct drm_i915_private {
 	} sagv_status;
 
 	struct {
-		/* protects DSPARB registers on pre-g4x/vlv/chv */
-		spinlock_t dsparb_lock;
-
 		/*
 		 * Raw watermark latency values:
 		 * in 0.1us units for WM0,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 99e09f63d4b3..24cdc13a416a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1358,13 +1358,19 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
 
-	spin_lock(&dev_priv->wm.dsparb_lock);
+	/*
+	 * 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.
+	 */
+	spin_lock(&dev_priv->uncore.lock);
 
 	switch (crtc->pipe) {
 		uint32_t dsparb, dsparb2, dsparb3;
 	case PIPE_A:
-		dsparb = I915_READ(DSPARB);
-		dsparb2 = I915_READ(DSPARB2);
+		dsparb = I915_READ_FW(DSPARB);
+		dsparb2 = I915_READ_FW(DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEA, 0xff) |
 			    VLV_FIFO(SPRITEB, 0xff));
@@ -1376,12 +1382,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));
 
-		I915_WRITE(DSPARB, dsparb);
-		I915_WRITE(DSPARB2, dsparb2);
+		I915_WRITE_FW(DSPARB, dsparb);
+		I915_WRITE_FW(DSPARB2, dsparb2);
 		break;
 	case PIPE_B:
-		dsparb = I915_READ(DSPARB);
-		dsparb2 = I915_READ(DSPARB2);
+		dsparb = I915_READ_FW(DSPARB);
+		dsparb2 = I915_READ_FW(DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEC, 0xff) |
 			    VLV_FIFO(SPRITED, 0xff));
@@ -1393,12 +1399,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));
 
-		I915_WRITE(DSPARB, dsparb);
-		I915_WRITE(DSPARB2, dsparb2);
+		I915_WRITE_FW(DSPARB, dsparb);
+		I915_WRITE_FW(DSPARB2, dsparb2);
 		break;
 	case PIPE_C:
-		dsparb3 = I915_READ(DSPARB3);
-		dsparb2 = I915_READ(DSPARB2);
+		dsparb3 = I915_READ_FW(DSPARB3);
+		dsparb2 = I915_READ_FW(DSPARB2);
 
 		dsparb3 &= ~(VLV_FIFO(SPRITEE, 0xff) |
 			     VLV_FIFO(SPRITEF, 0xff));
@@ -1410,16 +1416,16 @@ 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));
 
-		I915_WRITE(DSPARB3, dsparb3);
-		I915_WRITE(DSPARB2, dsparb2);
+		I915_WRITE_FW(DSPARB3, dsparb3);
+		I915_WRITE_FW(DSPARB2, dsparb2);
 		break;
 	default:
 		break;
 	}
 
-	POSTING_READ(DSPARB);
+	POSTING_READ_FW(DSPARB);
 
-	spin_unlock(&dev_priv->wm.dsparb_lock);
+	spin_unlock(&dev_priv->uncore.lock);
 }
 
 #undef VLV_FIFO
-- 
2.10.2

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

Re: [PATCH 0/5] drm/i915: Optimize plane updates a bit

[Maarten Lankhorst]

Hey,

Op 09-03-17 om 16:44 schreef ville.syrjala@linux.intel.com:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that commit e1edbd44e23b ("drm/i915: Complain if we take too
> long under vblank evasion.") has expose just how badly we suck,
> it seems like a good time to optimize things a little bit.
>
> Prior to this one of my VLV machines exceed the 100 usec vblank
> evade deadline pretty regularly, and at ever boot I was hitting
> numbers as high as 500 usec. Granted that was with lockdep and
> all kinds of other debug things enabled. After these changes
> things seem stay below the 33 usec mark, and with all that debug
> junk enabled we seem to staying below 22 usec.
>
> Note that I was testing single plane updates mostly, so I'm
> not 100% sure multi plane updates couldn't still exceed the
> deadline. That will need to be checked.
I've considered doing the same before, but thanks to the commit it's clear that it seems to worth the effort to do so.
Maybe a bit more radical, but could we grab the uncore lock for the entire update perhaps if it's still an issue?

Patch series looks sane, might be worth fixing all chips at some point. :)

Assuming testbot is happy..

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

Re: [PATCH 0/5] drm/i915: Optimize plane updates a bit

[Ville Syrjälä]

On Thu, Mar 09, 2017 at 04:56:23PM +0100, Maarten Lankhorst wrote:
> Hey,
> 
> Op 09-03-17 om 16:44 schreef ville.syrjala@linux.intel.com:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Now that commit e1edbd44e23b ("drm/i915: Complain if we take too
> > long under vblank evasion.") has expose just how badly we suck,
> > it seems like a good time to optimize things a little bit.
> >
> > Prior to this one of my VLV machines exceed the 100 usec vblank
> > evade deadline pretty regularly, and at ever boot I was hitting
> > numbers as high as 500 usec. Granted that was with lockdep and
> > all kinds of other debug things enabled. After these changes
> > things seem stay below the 33 usec mark, and with all that debug
> > junk enabled we seem to staying below 22 usec.
> >
> > Note that I was testing single plane updates mostly, so I'm
> > not 100% sure multi plane updates couldn't still exceed the
> > deadline. That will need to be checked.
> I've considered doing the same before, but thanks to the commit it's clear that it seems to worth the effort to do so.
> Maybe a bit more radical, but could we grab the uncore lock for the entire update perhaps if it's still an issue?

Yes that might be the ultimate goal. One of the problems is that we
don't use pipe_update_start()/end() everywhere we do plane updates. Some
of those we need to fix (eg. crtc disable really should disable things
atomically). The "disable all extra planes at driver load time" case
I'd like to eliminate by having proper state readout/takeover for
planes. And then there's the legacy cursor thing. I guess we could
avoid bigger surgery by simply grabbing the lock manually around the
update_plane/disable_plane call in those places.

Another thing I'd like to figure out is how much are we wasting CPU
cycles for computing  the register values in the .update_plane() hooks.
If it's at all significant we should probably try to move absolutely
everything but the register writes out from those functions.

Oh, and we might even try to take advantage of the hardware's limite
atomic capabilities and only write the absolute minimum set of registers
under the vblank evasion (ie. just the ones that actually arm the
update to occur on the next vblank). Unfortunately the split between
the two classes of registers isn't all that clear. It would also make
it impossible to do the fps>vrefresh thing.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Optimize plane updates a bit

[Patchwork]

== Series Details ==

Series: drm/i915: Optimize plane updates a bit
URL   : https://patchwork.freedesktop.org/series/21002/
State : failure

== Summary ==

Series 21002v1 drm/i915: Optimize plane updates a bit
https://patchwork.freedesktop.org/api/1.0/series/21002/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                incomplete -> PASS       (fi-skl-6700k) fdo#100130
Test gem_exec_reloc:
        Subgroup basic-gtt-read-noreloc:
                pass       -> INCOMPLETE (fi-byt-j1900)

fdo#100130 https://bugs.freedesktop.org/show_bug.cgi?id=100130

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 466s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 615s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 549s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 608s
fi-byt-j1900     total:85   pass:75   dwarn:0   dfail:0   fail:0   skip:9   time: 0s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 498s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 434s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 435s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 437s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 510s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 495s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 479s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 509s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 597s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 496s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 551s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 553s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 423s

510c200742ced5a91d07e48220b669a3c9b30c0c drm-tip: 2017y-03m-09d-15h-21m-14s UTC integration manifest
06676dc drm/i915: Optimize VLV/CHV display FIFO updates
bea48aa drm/i915: Use I915_READ_FW for plane updates
c63745b drm/i915: Organize plane register writes into tighter bunches
6f81bf0 drm/i915: s/__raw_i915_read32/I915_READ_FW/ in the SKL+ scanline read w/a
8fcdc34 drm/i915: Use I915_READ_FW in i915_get_vblank_counter()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4121/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Optimize plane updates a bit

[Patchwork]

== Series Details ==

Series: drm/i915: Optimize plane updates a bit
URL   : https://patchwork.freedesktop.org/series/21002/
State : failure

== Summary ==

Series 21002v1 drm/i915: Optimize plane updates a bit
https://patchwork.freedesktop.org/api/1.0/series/21002/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                incomplete -> PASS       (fi-skl-6700k) fdo#100130
        Subgroup basic-uc-prw-default:
                pass       -> INCOMPLETE (fi-skl-6700hq) fdo#100130
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> INCOMPLETE (fi-bxt-t5700)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-j1900)

fdo#100130 https://bugs.freedesktop.org/show_bug.cgi?id=100130

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 459s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 607s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 541s
fi-bxt-t5700     total:108  pass:95   dwarn:0   dfail:0   fail:0   skip:12  time: 0s
fi-byt-j1900     total:278  pass:250  dwarn:1   dfail:0   fail:0   skip:27  time: 507s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 498s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 441s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 435s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 448s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 506s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 484s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 474s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 520s
fi-skl-6700hq    total:54   pass:46   dwarn:0   dfail:0   fail:0   skip:7   time: 0s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 497s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 560s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 555s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 422s

510c200742ced5a91d07e48220b669a3c9b30c0c drm-tip: 2017y-03m-09d-15h-21m-14s UTC integration manifest
aa6c0ed drm/i915: Optimize VLV/CHV display FIFO updates
7bdb347 drm/i915: Use I915_READ_FW for plane updates
b03cfd4 drm/i915: Organize plane register writes into tighter bunches
6e789b6 drm/i915: s/__raw_i915_read32/I915_READ_FW/ in the SKL+ scanline read w/a
d3af5ca drm/i915: Use I915_READ_FW in i915_get_vblank_counter()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4123/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Optimize VLV/CHV display FIFO updates

[Chris Wilson]

On Thu, Mar 09, 2017 at 05:44:34PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use I915_{READ,WRITE}_FW() for updating the DSPARB registers on
> VLV/CHV. This is less expesive as we can grab the uncore.lock across
> the entire sequence of reads and writes instead of each register
> access grabbing it.
> 
> This also allows us to eliminate the dsparb lock entirely as the
> uncore.lock now effectively protects the contents of the DSPARB
> registers.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  1 -
>  drivers/gpu/drm/i915/i915_drv.h |  3 ---
>  drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++++++---------------
>  3 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b1e9027a4f80..debb74a2b2a9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -827,7 +827,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  
>  	spin_lock_init(&dev_priv->mm.object_stat_lock);
>  	spin_lock_init(&dev_priv->mmio_flip_lock);
> -	spin_lock_init(&dev_priv->wm.dsparb_lock);
>  	mutex_init(&dev_priv->sb_lock);
>  	mutex_init(&dev_priv->modeset_restore_lock);
>  	mutex_init(&dev_priv->av_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3002996ddbed..6af0b1d33cab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2375,9 +2375,6 @@ struct drm_i915_private {
>  	} sagv_status;
>  
>  	struct {
> -		/* protects DSPARB registers on pre-g4x/vlv/chv */
> -		spinlock_t dsparb_lock;
> -
>  		/*
>  		 * Raw watermark latency values:
>  		 * in 0.1us units for WM0,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 99e09f63d4b3..24cdc13a416a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1358,13 +1358,19 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
>  
> -	spin_lock(&dev_priv->wm.dsparb_lock);
> +	/*
> +	 * 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.
> +	 */
> +	spin_lock(&dev_priv->uncore.lock);

Might be nice to document that the irq is disabled by
intel_pipe_update_start() (hence why spin_lock is safe).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Optimize VLV/CHV display FIFO updates

[Ville Syrjälä]

On Thu, Mar 09, 2017 at 09:26:36PM +0000, Chris Wilson wrote:
> On Thu, Mar 09, 2017 at 05:44:34PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use I915_{READ,WRITE}_FW() for updating the DSPARB registers on
> > VLV/CHV. This is less expesive as we can grab the uncore.lock across
> > the entire sequence of reads and writes instead of each register
> > access grabbing it.
> > 
> > This also allows us to eliminate the dsparb lock entirely as the
> > uncore.lock now effectively protects the contents of the DSPARB
> > registers.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |  1 -
> >  drivers/gpu/drm/i915/i915_drv.h |  3 ---
> >  drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++++++---------------
> >  3 files changed, 21 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index b1e9027a4f80..debb74a2b2a9 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -827,7 +827,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
> >  
> >  	spin_lock_init(&dev_priv->mm.object_stat_lock);
> >  	spin_lock_init(&dev_priv->mmio_flip_lock);
> > -	spin_lock_init(&dev_priv->wm.dsparb_lock);
> >  	mutex_init(&dev_priv->sb_lock);
> >  	mutex_init(&dev_priv->modeset_restore_lock);
> >  	mutex_init(&dev_priv->av_mutex);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 3002996ddbed..6af0b1d33cab 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2375,9 +2375,6 @@ struct drm_i915_private {
> >  	} sagv_status;
> >  
> >  	struct {
> > -		/* protects DSPARB registers on pre-g4x/vlv/chv */
> > -		spinlock_t dsparb_lock;
> > -
> >  		/*
> >  		 * Raw watermark latency values:
> >  		 * in 0.1us units for WM0,
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 99e09f63d4b3..24cdc13a416a 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1358,13 +1358,19 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> >  
> > -	spin_lock(&dev_priv->wm.dsparb_lock);
> > +	/*
> > +	 * 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.
> > +	 */
> > +	spin_lock(&dev_priv->uncore.lock);
> 
> Might be nice to document that the irq is disabled by
> intel_pipe_update_start() (hence why spin_lock is safe).

Sure. I'll add a note.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915: s/__raw_i915_read32/I915_READ_FW/ in the SKL+ scanline read w/a

[Mika Kahola]

On Thu, 2017-03-09 at 17:44 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace __raw_i915_read32() with I915_READ_FW() in the workaround for
> the SKL+ scanline counter hardware fail. The two are the same thing
> but everyone else uses I915_READ_FW() so let's follow suit.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 70214e1fcd10..3c2109b4be2c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -817,8 +817,7 @@ static int __intel_get_crtc_scanline(struct
> intel_crtc *crtc)
>  
>  		for (i = 0; i < 100; i++) {
>  			udelay(1);
> -			temp = __raw_i915_read32(dev_priv,
> PIPEDSL(pipe)) &
> -				DSL_LINEMASK_GEN3;
> +			temp = I915_READ_FW(PIPEDSL(pipe)) &
> DSL_LINEMASK_GEN3;
>  			if (temp != position) {
>  				position = temp;
>  				break;
-- 
Mika Kahola - Intel OTC

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

✓ Fi.CI.BAT: success for drm/i915: Optimize plane updates a bit

[Patchwork]

== Series Details ==

Series: drm/i915: Optimize plane updates a bit
URL   : https://patchwork.freedesktop.org/series/21002/
State : success

== Summary ==

Series 21002v1 drm/i915: Optimize plane updates a bit
https://patchwork.freedesktop.org/api/1.0/series/21002/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 461s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 609s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 536s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 592s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 508s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 504s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 441s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 434s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 438s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 502s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 483s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 504s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 584s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 489s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 531s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 550s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 432s

ee2d731bf1079972596e5714dd70f98dd0177b0e drm-tip: 2017y-03m-13d-16h-06m-45s UTC integration manifest
184f5df drm/i915: Optimize VLV/CHV display FIFO updates
c8ffbc1 drm/i915: Use I915_READ_FW for plane updates
2989119 drm/i915: Organize plane register writes into tighter bunches
05c4acc drm/i915: s/__raw_i915_read32/I915_READ_FW/ in the SKL+ scanline read w/a
c53d265 drm/i915: Use I915_READ_FW in i915_get_vblank_counter()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4156/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Optimize VLV/CHV display FIFO updates

[Ville Syrjälä]

On Fri, Mar 10, 2017 at 12:07:53PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 09, 2017 at 09:26:36PM +0000, Chris Wilson wrote:
> > On Thu, Mar 09, 2017 at 05:44:34PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Use I915_{READ,WRITE}_FW() for updating the DSPARB registers on
> > > VLV/CHV. This is less expesive as we can grab the uncore.lock across
> > > the entire sequence of reads and writes instead of each register
> > > access grabbing it.
> > > 
> > > This also allows us to eliminate the dsparb lock entirely as the
> > > uncore.lock now effectively protects the contents of the DSPARB
> > > registers.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c |  1 -
> > >  drivers/gpu/drm/i915/i915_drv.h |  3 ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++++++---------------
> > >  3 files changed, 21 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index b1e9027a4f80..debb74a2b2a9 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -827,7 +827,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
> > >  
> > >  	spin_lock_init(&dev_priv->mm.object_stat_lock);
> > >  	spin_lock_init(&dev_priv->mmio_flip_lock);
> > > -	spin_lock_init(&dev_priv->wm.dsparb_lock);
> > >  	mutex_init(&dev_priv->sb_lock);
> > >  	mutex_init(&dev_priv->modeset_restore_lock);
> > >  	mutex_init(&dev_priv->av_mutex);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 3002996ddbed..6af0b1d33cab 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2375,9 +2375,6 @@ struct drm_i915_private {
> > >  	} sagv_status;
> > >  
> > >  	struct {
> > > -		/* protects DSPARB registers on pre-g4x/vlv/chv */
> > > -		spinlock_t dsparb_lock;
> > > -
> > >  		/*
> > >  		 * Raw watermark latency values:
> > >  		 * in 0.1us units for WM0,
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 99e09f63d4b3..24cdc13a416a 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -1358,13 +1358,19 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > >  
> > >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> > >  
> > > -	spin_lock(&dev_priv->wm.dsparb_lock);
> > > +	/*
> > > +	 * 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.
> > > +	 */
> > > +	spin_lock(&dev_priv->uncore.lock);
> > 
> > Might be nice to document that the irq is disabled by
> > intel_pipe_update_start() (hence why spin_lock is safe).
> 
> Sure. I'll add a note.

+        * intel_pipe_update_start() has already disabled interrupts
+        * for us, so a plain spin_lock() is sufficient here.
	 */

And entire series pushed to dinq. Thanks for the reviews.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx