[Intel-gfx] [PATCH v3] drm/i915: Use a spin lock to protect the pipe crc struct

Damien Lespiau damien.lespiau at intel.com
Thu Oct 17 18:32:28 CEST 2013


Daniel pointed out that it was hard to get anything lockless to work
correctly, so don't even try for this non critical piece of code and
just use a spin lock.

v2: Make intel_pipe_crc->opened a bool
v3: Use assert_spin_locked() instead of a comment (Daniel)

Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 53 ++++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_drv.h     |  5 ++--
 drivers/gpu/drm/i915/i915_irq.c     | 11 +++++---
 3 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 061182a..ebbb50e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1771,14 +1771,20 @@ static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
 	struct pipe_crc_info *info = inode->i_private;
 	struct drm_i915_private *dev_priv = info->dev->dev_private;
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
+	unsigned long irqflags;
 
-	if (!atomic_dec_and_test(&pipe_crc->available)) {
-		atomic_inc(&pipe_crc->available);
+	spin_lock_irqsave(&pipe_crc->lock, irqflags);
+
+	if (pipe_crc->opened) {
+		spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
 		return -EBUSY; /* already open */
 	}
 
+	pipe_crc->opened = true;
 	filep->private_data = inode->i_private;
 
+	spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
+
 	return 0;
 }
 
@@ -1787,8 +1793,11 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
 	struct pipe_crc_info *info = inode->i_private;
 	struct drm_i915_private *dev_priv = info->dev->dev_private;
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
+	unsigned long irqflags;
 
-	atomic_inc(&pipe_crc->available); /* release the device */
+	spin_lock_irqsave(&pipe_crc->lock, irqflags);
+	pipe_crc->opened = false;
+	spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
 
 	return 0;
 }
@@ -1800,12 +1809,9 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
 
 static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
 {
-	int head, tail;
-
-	head = atomic_read(&pipe_crc->head);
-	tail = atomic_read(&pipe_crc->tail);
-
-	return CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR);
+	assert_spin_locked(&pipe_crc->lock);
+	return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
+			INTEL_PIPE_CRC_ENTRIES_NR);
 }
 
 static ssize_t
@@ -1819,6 +1825,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
 	char buf[PIPE_CRC_BUFFER_LEN];
 	int head, tail, n_entries, n;
 	ssize_t bytes_read;
+	unsigned long irqflags;
 
 	/*
 	 * Don't allow user space to provide buffers not big enough to hold
@@ -1830,21 +1837,29 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
 	if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE)
 		return 0;
 
+
 	/* nothing to read */
+	spin_lock_irqsave(&pipe_crc->lock, irqflags);
 	while (pipe_crc_data_count(pipe_crc) == 0) {
+		spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
+
 		if (filep->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 
 		if (wait_event_interruptible(pipe_crc->wq,
 					     pipe_crc_data_count(pipe_crc)))
 			 return -ERESTARTSYS;
+
+		spin_lock_irqsave(&pipe_crc->lock, irqflags);
 	}
 
 	/* We now have one or more entries to read */
-	head = atomic_read(&pipe_crc->head);
-	tail = atomic_read(&pipe_crc->tail);
+	head = pipe_crc->head;
+	tail = pipe_crc->tail;
 	n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR),
 			count / PIPE_CRC_LINE_LEN);
+	spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
+
 	bytes_read = 0;
 	n = 0;
 	do {
@@ -1864,10 +1879,13 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
 
 		BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
 		tail = (tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
-		atomic_set(&pipe_crc->tail, tail);
 		n++;
 	} while (--n_entries);
 
+	spin_lock_irqsave(&pipe_crc->lock, irqflags);
+	pipe_crc->tail = tail;
+	spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
+
 	return bytes_read;
 }
 
@@ -2017,6 +2035,8 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 
 	/* none -> real source transition */
 	if (source) {
+		unsigned long irqflags;
+
 		DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n",
 				 pipe_name(pipe), pipe_crc_source_name(source));
 
@@ -2026,8 +2046,10 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 		if (!pipe_crc->entries)
 			return -ENOMEM;
 
-		atomic_set(&pipe_crc->head, 0);
-		atomic_set(&pipe_crc->tail, 0);
+		spin_lock_irqsave(&pipe_crc->lock, irqflags);
+		pipe_crc->head = 0;
+		pipe_crc->tail = 0;
+		spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
 	}
 
 	pipe_crc->source = source;
@@ -2738,7 +2760,8 @@ void intel_display_crc_init(struct drm_device *dev)
 	for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) {
 		struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[i];
 
-		atomic_set(&pipe_crc->available, 1);
+		pipe_crc->opened = false;
+		spin_lock_init(&pipe_crc->lock);
 		init_waitqueue_head(&pipe_crc->wq);
 	}
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2ea33ee..0699b93 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1234,10 +1234,11 @@ struct intel_pipe_crc_entry {
 
 #define INTEL_PIPE_CRC_ENTRIES_NR	128
 struct intel_pipe_crc {
-	atomic_t available;		/* exclusive access to the device */
+	spinlock_t lock;
+	bool opened;		/* exclusive access to the result file */
 	struct intel_pipe_crc_entry *entries;
 	enum intel_pipe_crc_source source;
-	atomic_t head, tail;
+	int head, tail;
 	wait_queue_head_t wq;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 156a1a4..a9ea8be 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1198,15 +1198,18 @@ static void display_pipe_crc_update(struct drm_device *dev, enum pipe pipe,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
 	struct intel_pipe_crc_entry *entry;
+	unsigned long irqflags;
 	int head, tail;
 
+	spin_lock_irqsave(&pipe_crc->lock, irqflags);
+
 	if (!pipe_crc->entries) {
 		DRM_ERROR("spurious interrupt\n");
 		return;
 	}
 
-	head = atomic_read(&pipe_crc->head);
-	tail = atomic_read(&pipe_crc->tail);
+	head = pipe_crc->head;
+	tail = pipe_crc->tail;
 
 	if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
 		DRM_ERROR("CRC buffer overflowing\n");
@@ -1223,7 +1226,9 @@ static void display_pipe_crc_update(struct drm_device *dev, enum pipe pipe,
 	entry->crc[4] = crc4;
 
 	head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
-	atomic_set(&pipe_crc->head, head);
+	pipe_crc->head = head;
+
+	spin_unlock_irqrestore(&pipe_crc->lock, irqflags);
 
 	wake_up_interruptible(&pipe_crc->wq);
 }
-- 
1.8.3.1




More information about the Intel-gfx mailing list