[2/2] drm/i915: Replace use of pipe_crc->lock with an atomic

Commit Message

Maarten Lankhorst June 28, 2018, 7:23 a.m. UTC

pipe_crc->lock only protects pipe_crc->skipped. Replace the lock with
atomic operations instead, it should work just as well, without the
spinlock. In the case we don't skip CRC in the irq, the fastpath is
only a single atomic_read().

Signed-off-by: Maarten Lankhorst <[e-mail]>
Cc: Ville Syrjälä <[e-mail]>
---
 drivers/gpu/drm/i915/i915_drv.c       |  1 -
 drivers/gpu/drm/i915/i915_drv.h       |  5 +----
 drivers/gpu/drm/i915/i915_irq.c       | 15 ++++++++-------
 drivers/gpu/drm/i915/intel_pipe_crc.c | 20 +++-----------------
 4 files changed, 12 insertions(+), 29 deletions(-)

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0db3c83cce29..5ad69b488d2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -923,7 +923,6 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	intel_init_display_hooks(dev_priv);
 	intel_init_clock_gating_hooks(dev_priv);
 	intel_init_audio_hooks(dev_priv);
-	intel_display_crc_init(dev_priv);
 
 	intel_detect_preproduction_hw(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 298b6497cdc5..a8961adde497 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1278,8 +1278,7 @@  enum intel_pipe_crc_source {
 
 #define INTEL_PIPE_CRC_ENTRIES_NR	128
 struct intel_pipe_crc {
-	spinlock_t lock;
-	int skipped;
+	atomic_t skipped;
 	enum intel_pipe_crc_source source;
 };
 
@@ -3335,12 +3334,10 @@  u32 i915_gem_fence_alignment(struct drm_i915_private *dev_priv, u32 size,
 #ifdef CONFIG_DEBUG_FS
 int i915_debugfs_register(struct drm_i915_private *dev_priv);
 int i915_debugfs_connector_add(struct drm_connector *connector);
-void intel_display_crc_init(struct drm_i915_private *dev_priv);
 #else
 static inline int i915_debugfs_register(struct drm_i915_private *dev_priv) {return 0;}
 static inline int i915_debugfs_connector_add(struct drm_connector *connector)
 { return 0; }
-static inline void intel_display_crc_init(struct drm_i915_private *dev_priv) {}
 #endif
 
 const char *i915_cache_level_str(struct drm_i915_private *i915, int type);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 08928830fa03..5688cf1e1225 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1705,8 +1705,8 @@  static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
 	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
 	uint32_t crcs[5];
+	int to_skip;
 
-	spin_lock(&pipe_crc->lock);
 	/*
 	 * For some not yet identified reason, the first CRC is
 	 * bonkers. So let's just wait for the next vblank and read
@@ -1715,13 +1715,14 @@  static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
 	 * On GEN8+ sometimes the second CRC is bonkers as well, so
 	 * don't trust that one either.
 	 */
-	if (pipe_crc->skipped <= 0 ||
-	    (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
-		pipe_crc->skipped++;
-		spin_unlock(&pipe_crc->lock);
+
+	if (INTEL_GEN(dev_priv) >= 8)
+		to_skip = 2;
+	else
+		to_skip = 1;
+
+	if (atomic_add_unless(&pipe_crc->skipped, 1, to_skip))
 		return;
-	}
-	spin_unlock(&pipe_crc->lock);
 
 	crcs[0] = crc0;
 	crcs[1] = crc1;
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 849e1b69ba73..25ec1c2a64b7 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -457,17 +457,6 @@  display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
 	return 0;
 }
 
-void intel_display_crc_init(struct drm_i915_private *dev_priv)
-{
-	enum pipe pipe;
-
-	for_each_pipe(dev_priv, pipe) {
-		struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
-
-		spin_lock_init(&pipe_crc->lock);
-	}
-}
-
 int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 			      size_t *values_cnt)
 {
@@ -507,7 +496,7 @@  int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 			hsw_pipe_A_crc_wa(dev_priv, false);
 	}
 
-	pipe_crc->skipped = 0;
+	atomic_set(&pipe_crc->skipped, 0);
 	*values_cnt = 5;
 
 out:
@@ -529,8 +518,7 @@  void intel_crtc_enable_pipe_crc(struct intel_crtc *intel_crtc)
 	if (get_new_crc_ctl_reg(dev_priv, crtc->index, &pipe_crc->source, &val, false) < 0)
 		return;
 
-	/* Don't need pipe_crc->lock here, IRQs are not generated. */
-	pipe_crc->skipped = 0;
+	atomic_set(&pipe_crc->skipped, 0);
 
 	I915_WRITE(PIPE_CRC_CTL(crtc->index), val);
 	POSTING_READ(PIPE_CRC_CTL(crtc->index));
@@ -543,9 +531,7 @@  void intel_crtc_disable_pipe_crc(struct intel_crtc *intel_crtc)
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[crtc->index];
 
 	/* Swallow crc's until we stop generating them. */
-	spin_lock_irq(&pipe_crc->lock);
-	pipe_crc->skipped = INT_MIN;
-	spin_unlock_irq(&pipe_crc->lock);
+	atomic_set(&pipe_crc->skipped, INT_MIN);
 
 	I915_WRITE(PIPE_CRC_CTL(crtc->index), 0);
 	POSTING_READ(PIPE_CRC_CTL(crtc->index));