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

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Tue Apr 10 15:26:39 UTC 2018


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 <maarten.lankhorst at linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 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(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f770be18b2d7..8cb61e51eb33 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -938,7 +938,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 4f139ab95547..648c41bb5bd1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1264,8 +1264,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;
 };
 
@@ -3323,12 +3322,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 d8751881b0d2..20174b98aad9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1682,8 +1682,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
@@ -1692,13 +1692,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 0e38c382e231..5320c1fcb3dc 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -458,17 +458,6 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
 	return -EINVAL;
 }
 
-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)
 {
@@ -508,7 +497,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:
@@ -530,8 +519,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));
@@ -544,9 +532,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));
-- 
2.16.3



More information about the Intel-gfx mailing list