diff mbox

[RFC,13/14] WIP/drm/i915: Protect the entire pipe update with uncore.lock

Message ID 20170317211808.14693-14-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä March 17, 2017, 9:18 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'm thinking we'll want to actually spliut the uncore.lock into
two locks (display vs. gt). But this is just a proof of concept and
it is indeed quote effective, especially with lockdep and whatnot
enabled.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      | 66 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/i915_trace.h    | 28 +++++++--------
 drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  7 ++--
 drivers/gpu/drm/i915/intel_pm.c      | 11 +++---
 drivers/gpu/drm/i915/intel_sprite.c  | 57 ++++++-------------------------
 6 files changed, 104 insertions(+), 116 deletions(-)

Comments

Chris Wilson March 17, 2017, 9:32 p.m. UTC | #1
On Fri, Mar 17, 2017 at 11:18:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I'm thinking we'll want to actually spliut the uncore.lock into
> two locks (display vs. gt).

Yes, a single lock for display seems reasonable, esp. given the proof of
principle here.

Hmm, I was thinking we would need a separate one for fw_domains handling,
but that's actually just GT. Splitting into two seems doable.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index cb20c9408b12..113de4309b41 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -715,15 +715,13 @@  static void i915_enable_asle_pipestat(struct drm_i915_private *dev_priv)
 /* Called from drm generic code, passed a 'crtc', which
  * we use as a pipe index
  */
-static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
+static u32 __i915_get_vblank_counter(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
 	i915_reg_t high_frame, low_frame;
 	u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
-	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;
+	const struct drm_display_mode *mode = &crtc->base.hwmode;
 
 	htotal = mode->crtc_htotal;
 	hsync_start = mode->crtc_hsync_start;
@@ -740,8 +738,6 @@  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
@@ -753,8 +749,6 @@  static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 		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;
@@ -767,15 +761,57 @@  static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
 }
 
+static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+	unsigned long irqflags;
+	u32 ret;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+	ret = __i915_get_vblank_counter(crtc);
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+	return ret;
+}
+
+static u32 __g4x_get_vblank_counter(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+
+	return I915_READ_FW(PIPE_FRMCOUNT_G4X(pipe));
+}
+
 static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+	unsigned long irqflags;
+	u32 ret;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+	ret = __g4x_get_vblank_counter(crtc);
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+	return ret;
+}
 
-	return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
+u32 __intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	if (IS_GEN2(dev_priv)) {
+		return 0;
+	} else if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
+		return __g4x_get_vblank_counter(crtc);
+	} else {
+		return __i915_get_vblank_counter(crtc);
+	}
 }
 
 /* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */
-static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
+int __intel_crtc_get_scanline(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -878,7 +914,7 @@  static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 		/* No obvious pixelcount register. Only query vertical
 		 * scanout position from Display scan line register.
 		 */
-		position = __intel_get_crtc_scanline(intel_crtc);
+		position = __intel_crtc_get_scanline(intel_crtc);
 	} else {
 		/* Have access to pixelcount since start of frame.
 		 * We can split this into vertical and horizontal
@@ -951,14 +987,14 @@  static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 	return ret;
 }
 
-int intel_get_crtc_scanline(struct intel_crtc *crtc)
+int intel_crtc_get_scanline(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	unsigned long irqflags;
 	int position;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-	position = __intel_get_crtc_scanline(crtc);
+	position = __intel_crtc_get_scanline(crtc);
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 
 	return position;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 66404c5aee82..66491e3db344 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -29,7 +29,7 @@  TRACE_EVENT(intel_cpu_fifo_underrun,
 	    TP_fast_assign(
 			   __entry->pipe = pipe;
 			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   __entry->scanline = intel_crtc_get_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
 			   ),
 
 	    TP_printk("pipe %c, frame=%u, scanline=%u",
@@ -51,7 +51,7 @@  TRACE_EVENT(intel_pch_fifo_underrun,
 			   enum pipe pipe = (enum pipe)pch_transcoder;
 			   __entry->pipe = pipe;
 			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   __entry->scanline = intel_crtc_get_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
 			   ),
 
 	    TP_printk("pch transcoder %c, frame=%u, scanline=%u",
@@ -76,7 +76,7 @@  TRACE_EVENT(intel_memory_cxsr,
 				   __entry->frame[pipe] =
 					   dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
 				   __entry->scanline[pipe] =
-					   intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+					   intel_crtc_get_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
 			   }
 			   __entry->old = old;
 			   __entry->new = new;
@@ -111,7 +111,7 @@  TRACE_EVENT(vlv_wm,
 			   __entry->pipe = crtc->pipe;
 			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
 										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->scanline = intel_crtc_get_scanline(crtc);
 			   __entry->level = wm->level;
 			   __entry->cxsr = wm->cxsr;
 			   __entry->primary = wm->pipe[crtc->pipe].plane[PLANE_PRIMARY];
@@ -144,9 +144,8 @@  TRACE_EVENT(vlv_fifo_size,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   __entry->sprite0_start = sprite0_start;
 			   __entry->sprite1_start = sprite1_start;
 			   __entry->fifo_size = fifo_size;
@@ -176,9 +175,8 @@  TRACE_EVENT(intel_update_plane,
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
 			   __entry->name = plane->name;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   memcpy(__entry->src, &plane->state->src, sizeof(__entry->src));
 			   memcpy(__entry->dst, &plane->state->dst, sizeof(__entry->dst));
 			   ),
@@ -204,9 +202,8 @@  TRACE_EVENT(intel_disable_plane,
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
 			   __entry->name = plane->name;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   ),
 
 	    TP_printk("pipe %c, plane %s, frame=%u, scanline=%u",
@@ -230,9 +227,8 @@  TRACE_EVENT(i915_pipe_update_start,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   __entry->min = crtc->debug.min_vbl;
 			   __entry->max = crtc->debug.max_vbl;
 			   ),
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 34d833975b32..c35818b42762 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1931,7 +1931,7 @@  static void intel_enable_pipe(struct intel_crtc *crtc)
 	 * drm_crtc_vblank_on()
 	 */
 	if (dev->max_vblank_count == 0 &&
-	    wait_for(intel_get_crtc_scanline(crtc) != crtc->scanline_offset, 50))
+	    wait_for(intel_crtc_get_scanline(crtc) != crtc->scanline_offset, 50))
 		DRM_ERROR("pipe %c didn't start\n", pipe_name(pipe));
 }
 
@@ -2749,8 +2749,11 @@  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 				to_intel_plane_state(plane_state),
 				false);
 	intel_pre_disable_primary_noatomic(&intel_crtc->base);
+
+	spin_lock_irq(&dev_priv->uncore.lock);
 	trace_intel_disable_plane(primary, intel_crtc);
 	intel_plane->disable_plane(primary, &intel_crtc->base);
+	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	return;
 
@@ -3076,7 +3079,6 @@  static void i9xx_update_primary_plane(struct drm_plane *primary,
 	i915_reg_t reg = DSPCNTR(plane);
 	int x = plane_state->main.x;
 	int y = plane_state->main.y;
-	unsigned long irqflags;
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
@@ -3088,8 +3090,6 @@  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.
@@ -3126,8 +3126,6 @@  static void i9xx_update_primary_plane(struct drm_plane *primary,
 			      intel_crtc->dspaddr_offset);
 	}
 	POSTING_READ_FW(reg);
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void i9xx_disable_primary_plane(struct drm_plane *primary,
@@ -3137,9 +3135,6 @@  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;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(DSPCNTR(plane), 0);
 	if (INTEL_INFO(dev_priv)->gen >= 4)
@@ -3147,8 +3142,6 @@  static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	else
 		I915_WRITE_FW(DSPADDR(plane), 0);
 	POSTING_READ_FW(DSPCNTR(plane));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32
@@ -3344,7 +3337,6 @@  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;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -3357,8 +3349,6 @@  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_FW(PLANE_COLOR_CTL(pipe, plane_id),
 			      PLANE_COLOR_PIPE_GAMMA_ENABLE |
@@ -3390,8 +3380,6 @@  static void skylake_update_primary_plane(struct drm_plane *plane,
 		      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 skylake_disable_primary_plane(struct drm_plane *primary,
@@ -3401,15 +3389,10 @@  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;
-
-	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 */
@@ -3433,8 +3416,11 @@  static void intel_complete_page_flips(struct drm_i915_private *dev_priv)
 
 static void intel_update_primary_planes(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *crtc;
 
+	spin_lock_irq(&dev_priv->uncore.lock);
+
 	for_each_crtc(dev, crtc) {
 		struct intel_plane *plane = to_intel_plane(crtc->primary);
 		struct intel_plane_state *plane_state =
@@ -3449,6 +3435,8 @@  static void intel_update_primary_planes(struct drm_device *dev)
 					    plane_state);
 		}
 	}
+
+	spin_unlock_irq(&dev_priv->uncore.lock);
 }
 
 static int
@@ -5089,22 +5077,24 @@  static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 
 static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_plane *p;
 	int pipe = intel_crtc->pipe;
 
 	intel_crtc_dpms_overlay_disable(intel_crtc);
 
-	drm_for_each_plane_mask(p, dev, plane_mask)
+	spin_lock_irq(&dev_priv->uncore.lock);
+	drm_for_each_plane_mask(p, &dev_priv->drm, plane_mask)
 		to_intel_plane(p)->disable_plane(p, crtc);
+	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	/*
 	 * FIXME: Once we grow proper nuclear flip support out of this we need
 	 * to compute the mask of flip planes precisely. For the time being
 	 * consider this a flip to a NULL plane.
 	 */
-	intel_frontbuffer_flip(to_i915(dev), INTEL_FRONTBUFFER_ALL_MASK(pipe));
+	intel_frontbuffer_flip(dev_priv, INTEL_FRONTBUFFER_ALL_MASK(pipe));
 }
 
 static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc,
@@ -9273,7 +9263,6 @@  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) {
@@ -9300,16 +9289,12 @@  static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		}
 	}
 
-	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,
@@ -13521,6 +13506,8 @@  intel_legacy_cursor_update(struct drm_plane *plane,
 	new_plane_state->fb = old_fb;
 	to_intel_plane_state(new_plane_state)->vma = old_vma;
 
+	spin_lock_irq(&dev_priv->uncore.lock);
+
 	if (plane->state->visible) {
 		trace_intel_update_plane(plane, to_intel_crtc(crtc));
 		intel_plane->update_plane(plane,
@@ -13531,6 +13518,8 @@  intel_legacy_cursor_update(struct drm_plane *plane,
 		intel_plane->disable_plane(plane, crtc);
 	}
 
+	spin_unlock_irq(&dev_priv->uncore.lock);
+
 	intel_cleanup_plane_fb(plane, new_plane_state);
 
 out_unlock:
@@ -15174,6 +15163,8 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 
 		drm_crtc_vblank_on(&crtc->base);
 
+		spin_lock_irq(&dev_priv->uncore.lock);
+
 		/* Disable everything but the primary plane */
 		for_each_intel_plane_on_crtc(dev, crtc, plane) {
 			if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
@@ -15182,6 +15173,8 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 			trace_intel_disable_plane(&plane->base, crtc);
 			plane->disable_plane(&plane->base, &crtc->base);
 		}
+
+		spin_unlock_irq(&dev_priv->uncore.lock);
 	}
 
 	/* We need to sanitize the plane -> pipe mapping first because this will
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 15e3eb2824e3..8c6bf32244fb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1209,7 +1209,10 @@  static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
 	return dev_priv->pm.irqs_enabled;
 }
 
-int intel_get_crtc_scanline(struct intel_crtc *crtc);
+int __intel_crtc_get_scanline(struct intel_crtc *crtc);
+int intel_crtc_get_scanline(struct intel_crtc *crtc);
+u32 __intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
+u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
 void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
 				     unsigned int pipe_mask);
 void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
@@ -1350,8 +1353,6 @@  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, int pipe)
 		intel_wait_for_vblank(dev_priv, pipe);
 }
 
-u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
-
 int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 			 struct intel_digital_port *dport,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aece0ff88a5d..622ae89f3be1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1367,7 +1367,6 @@  static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 	 * intel_pipe_update_start() has already disabled interrupts
 	 * for us, so a plain spin_lock() is sufficient here.
 	 */
-	spin_lock(&dev_priv->uncore.lock);
 
 	switch (crtc->pipe) {
 		uint32_t dsparb, dsparb2, dsparb3;
@@ -1427,8 +1426,6 @@  static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 	}
 
 	POSTING_READ_FW(DSPARB);
-
-	spin_unlock(&dev_priv->uncore.lock);
 }
 
 #undef VLV_FIFO
@@ -4003,9 +4000,9 @@  static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
 				const struct skl_ddb_entry *entry)
 {
 	if (entry->end)
-		I915_WRITE(reg, (entry->end - 1) << 16 | entry->start);
+		I915_WRITE_FW(reg, (entry->end - 1) << 16 | entry->start);
 	else
-		I915_WRITE(reg, 0);
+		I915_WRITE_FW(reg, 0);
 }
 
 static void skl_write_wm_level(struct drm_i915_private *dev_priv,
@@ -4020,7 +4017,7 @@  static void skl_write_wm_level(struct drm_i915_private *dev_priv,
 		val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
 	}
 
-	I915_WRITE(reg, val);
+	I915_WRITE_FW(reg, val);
 }
 
 static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
@@ -4376,7 +4373,7 @@  static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 	if (!(state->wm_results.dirty_pipes & drm_crtc_mask(&crtc->base)))
 		return;
 
-	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
+	I915_WRITE_FW(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
 
 	for_each_plane_id_on_crtc(crtc, plane_id) {
 		if (plane_id != PLANE_CURSOR)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 337e884252e5..06c89deca36b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -83,6 +83,7 @@  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
  */
 void intel_pipe_update_start(struct intel_crtc *crtc)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 	long timeout = msecs_to_jiffies_timeout(1);
 	int scanline, min, max, vblank_start;
@@ -106,6 +107,8 @@  void intel_pipe_update_start(struct intel_crtc *crtc)
 	if (WARN_ON(drm_crtc_vblank_get(&crtc->base)))
 		return;
 
+	spin_lock(&dev_priv->uncore.lock);
+
 	crtc->debug.min_vbl = min;
 	crtc->debug.max_vbl = max;
 	trace_i915_pipe_update_start(crtc);
@@ -118,7 +121,7 @@  void intel_pipe_update_start(struct intel_crtc *crtc)
 		 */
 		prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE);
 
-		scanline = intel_get_crtc_scanline(crtc);
+		scanline = __intel_crtc_get_scanline(crtc);
 		if (scanline < min || scanline > max)
 			break;
 
@@ -128,18 +131,18 @@  void intel_pipe_update_start(struct intel_crtc *crtc)
 			break;
 		}
 
-		local_irq_enable();
+		spin_unlock_irq(&dev_priv->uncore.lock);
 
 		timeout = schedule_timeout(timeout);
 
-		local_irq_disable();
+		spin_lock_irq(&dev_priv->uncore.lock);
 	}
 
 	finish_wait(wq, &wait);
 
 	crtc->debug.scanline_start = scanline;
 	crtc->debug.start_vbl_time = ktime_get();
-	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
+	crtc->debug.start_vbl_count = __intel_crtc_get_vblank_counter(crtc);
 
 	trace_i915_pipe_update_vblank_evaded(crtc);
 }
@@ -156,8 +159,8 @@  void intel_pipe_update_start(struct intel_crtc *crtc)
 void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work)
 {
 	enum pipe pipe = crtc->pipe;
-	int scanline_end = intel_get_crtc_scanline(crtc);
-	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
+	int scanline_end = __intel_crtc_get_scanline(crtc);
+	u32 end_vbl_count = __intel_crtc_get_vblank_counter(crtc);
 	ktime_t end_vbl_time = ktime_get();
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
@@ -169,6 +172,8 @@  void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 
 	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
+	spin_unlock(&dev_priv->uncore.lock);
+
 	/* We're still in the vblank-evade critical section, this can't race.
 	 * Would be slightly nice to just grab the vblank count and arm the
 	 * event outside of the critical section - the spinlock might spin for a
@@ -237,7 +242,6 @@  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;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -245,8 +249,6 @@  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_FW(PLANE_COLOR_CTL(pipe, plane_id),
 			      PLANE_COLOR_PIPE_GAMMA_ENABLE |
@@ -287,8 +289,6 @@  skl_update_plane(struct drm_plane *drm_plane,
 	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
@@ -299,16 +299,11 @@  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_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);
 }
 
 static void
@@ -435,7 +430,6 @@  vlv_update_plane(struct drm_plane *dplane,
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
 	uint32_t x = plane_state->main.x;
 	uint32_t y = plane_state->main.y;
-	unsigned long irqflags;
 
 	/* Sizes are 0 based */
 	crtc_w--;
@@ -443,8 +437,6 @@  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);
 
@@ -468,8 +460,6 @@  vlv_update_plane(struct drm_plane *dplane,
 	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
@@ -480,16 +470,11 @@  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_FW(SPCNTR(pipe, plane_id), 0);
 
 	I915_WRITE_FW(SPSURF(pipe, plane_id), 0);
 	POSTING_READ_FW(SPSURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -570,7 +555,6 @@  ivb_update_plane(struct drm_plane *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;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -583,8 +567,6 @@  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_FW(SPRKEYVAL(pipe), key->min_value);
 		I915_WRITE_FW(SPRKEYMAX(pipe), key->max_value);
@@ -610,8 +592,6 @@  ivb_update_plane(struct drm_plane *plane,
 	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
@@ -621,9 +601,6 @@  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_FW(SPRCTL(pipe), 0);
 	/* Can't leave the scaler enabled... */
@@ -632,8 +609,6 @@  ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 
 	I915_WRITE_FW(SPRSURF(pipe), 0);
 	POSTING_READ_FW(SPRSURF(pipe));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -711,7 +686,6 @@  ilk_update_plane(struct drm_plane *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;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -724,8 +698,6 @@  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_FW(DVSKEYVAL(pipe), key->min_value);
 		I915_WRITE_FW(DVSKEYMAX(pipe), key->max_value);
@@ -746,8 +718,6 @@  ilk_update_plane(struct drm_plane *plane,
 	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
@@ -757,9 +727,6 @@  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;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(DVSCNTR(pipe), 0);
 	/* Disable the scaler */
@@ -767,8 +734,6 @@  ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 
 	I915_WRITE_FW(DVSSURF(pipe), 0);
 	POSTING_READ_FW(DVSSURF(pipe));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static int