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@xxxxxxxx>
Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
---
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 --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 061182a..fd80064 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;
}
@@ -2026,8 +2043,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;
@@ -2037,13 +2056,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;
@@ -2738,7 +2763,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..0e50dfd 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_update(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_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(&pipe_crc->lock);
wake_up_interruptible(&pipe_crc->wq);
}
--
1.8.3.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
