diff mbox

[14/41] drm/i915: Use a spin lock to protect the pipe crc struct

Message ID 1407203723-24877-15-git-send-email-dheerajx.s.jamwal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dheeraj Jamwal Aug. 5, 2014, 1:54 a.m. UTC 
From: Damien Lespiau <damien.lespiau@intel.com>

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 Vetter)
v4: Use spin_lock_irq() in the debugfs functions (they can only be
    called from process context),
    Use spin_lock() in the pipe_crc_update() function that can only be
    called from an interrupt handler,
    Use wait_event_interruptible_lock_irq() when waiting for data in the
    cicular buffer to ensure proper locking around the condition we are
    waiting for. (Daniel Vetter)

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit d538bbdfde34028b5c5b0ba92b3c2096c5afb82c)

Signed-off-by: Dheeraj Jamwal <dheerajx.s.jamwal@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   66 ++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_drv.h     |    5 +--
 drivers/gpu/drm/i915/i915_irq.c     |   12 +++++--
 3 files changed, 58 insertions(+), 25 deletions(-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 25fc384..5c45e9e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1772,13 +1772,18 @@  static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
 	struct drm_i915_private *dev_priv = info->dev->dev_private;
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
 
-	if (!atomic_dec_and_test(&pipe_crc->available)) {
-		atomic_inc(&pipe_crc->available);
+	spin_lock_irq(&pipe_crc->lock);
+
+	if (pipe_crc->opened) {
+		spin_unlock_irq(&pipe_crc->lock);
 		return -EBUSY; /* already open */
 	}
 
+	pipe_crc->opened = true;
 	filep->private_data = inode->i_private;
 
+	spin_unlock_irq(&pipe_crc->lock);
+
 	return 0;
 }
 
@@ -1788,7 +1793,9 @@  static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
 	struct drm_i915_private *dev_priv = info->dev->dev_private;
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
 
-	atomic_inc(&pipe_crc->available); /* release the device */
+	spin_lock_irq(&pipe_crc->lock);
+	pipe_crc->opened = false;
+	spin_unlock_irq(&pipe_crc->lock);
 
 	return 0;
 }
@@ -1800,12 +1807,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
@@ -1831,20 +1835,30 @@  i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
 		return 0;
 
 	/* nothing to read */
+	spin_lock_irq(&pipe_crc->lock);
 	while (pipe_crc_data_count(pipe_crc) == 0) {
-		if (filep->f_flags & O_NONBLOCK)
+		int ret;
+
+		if (filep->f_flags & O_NONBLOCK) {
+			spin_unlock_irq(&pipe_crc->lock);
 			return -EAGAIN;
+		}
 
-		if (wait_event_interruptible(pipe_crc->wq,
-					     pipe_crc_data_count(pipe_crc)))
-			 return -ERESTARTSYS;
+		ret = wait_event_interruptible_lock_irq(pipe_crc->wq,
+				pipe_crc_data_count(pipe_crc), pipe_crc->lock);
+		if (ret) {
+			spin_unlock_irq(&pipe_crc->lock);
+			return ret;
+		}
 	}
 
 	/* 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_irq(&pipe_crc->lock);
+
 	bytes_read = 0;
 	n = 0;
 	do {
@@ -1864,10 +1878,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_irq(&pipe_crc->lock);
+	pipe_crc->tail = tail;
+	spin_unlock_irq(&pipe_crc->lock);
+
 	return bytes_read;
 }
 
@@ -2111,8 +2128,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_irq(&pipe_crc->lock);
+		pipe_crc->head = 0;
+		pipe_crc->tail = 0;
+		spin_unlock_irq(&pipe_crc->lock);
 	}
 
 	pipe_crc->source = source;
@@ -2122,13 +2141,19 @@  static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 
 	/* real source -> none transition */
 	if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
+		struct intel_pipe_crc_entry *entries;
+
 		DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
 				 pipe_name(pipe));
 
 		intel_wait_for_vblank(dev, pipe);
 
-		kfree(pipe_crc->entries);
+		spin_lock_irq(&pipe_crc->lock);
+		entries = pipe_crc->entries;
 		pipe_crc->entries = NULL;
+		spin_unlock_irq(&pipe_crc->lock);
+
+		kfree(entries);
 	}
 
 	return 0;
@@ -2823,7 +2848,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 48cbea1..7ec2a4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1251,10 +1251,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 8f7baad..1a7dc77 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1200,15 +1200,19 @@  static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe,
 	struct intel_pipe_crc_entry *entry;
 	int head, tail;
 
+	spin_lock(&pipe_crc->lock);
+
 	if (!pipe_crc->entries) {
+		spin_unlock(&pipe_crc->lock);
 		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) {
+		spin_unlock(&pipe_crc->lock);
 		DRM_ERROR("CRC buffer overflowing\n");
 		return;
 	}
@@ -1223,7 +1227,9 @@  static void display_pipe_crc_irq_handler(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(&pipe_crc->lock);
 
 	wake_up_interruptible(&pipe_crc->wq);
 }