drm/i915: Use a spin lock to protect the pipe crc struct
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>
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 @@
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 @@
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 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 @@
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)
- return -EAGAIN;
+ int ret;
- if (wait_event_interruptible(pipe_crc->wq,
- pipe_crc_data_count(pipe_crc)))
- return -ERESTARTSYS;
+ if (filep->f_flags & O_NONBLOCK) {
+ spin_unlock_irq(&pipe_crc->lock);
+ return -EAGAIN;
+ }
+
+ 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 @@
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 @@
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 @@
/* 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 @@
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);
}
}