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@xxxxxxxx>
Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
---
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
