Commit d538bbdf authored by Damien Lespiau's avatar Damien Lespiau Committed by Daniel Vetter

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: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarDamien Lespiau <damien.lespiau@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent c4597872
...@@ -1772,13 +1772,18 @@ static int i915_pipe_crc_open(struct inode *inode, struct file *filep) ...@@ -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 drm_i915_private *dev_priv = info->dev->dev_private;
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
if (!atomic_dec_and_test(&pipe_crc->available)) { spin_lock_irq(&pipe_crc->lock);
atomic_inc(&pipe_crc->available);
if (pipe_crc->opened) {
spin_unlock_irq(&pipe_crc->lock);
return -EBUSY; /* already open */ return -EBUSY; /* already open */
} }
pipe_crc->opened = true;
filep->private_data = inode->i_private; filep->private_data = inode->i_private;
spin_unlock_irq(&pipe_crc->lock);
return 0; return 0;
} }
...@@ -1788,7 +1793,9 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep) ...@@ -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 drm_i915_private *dev_priv = info->dev->dev_private;
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; 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; return 0;
} }
...@@ -1800,12 +1807,9 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep) ...@@ -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) static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
{ {
int head, tail; assert_spin_locked(&pipe_crc->lock);
return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
head = atomic_read(&pipe_crc->head); INTEL_PIPE_CRC_ENTRIES_NR);
tail = atomic_read(&pipe_crc->tail);
return CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR);
} }
static ssize_t static ssize_t
...@@ -1831,20 +1835,30 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count, ...@@ -1831,20 +1835,30 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
return 0; return 0;
/* nothing to read */ /* nothing to read */
spin_lock_irq(&pipe_crc->lock);
while (pipe_crc_data_count(pipe_crc) == 0) { 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; return -EAGAIN;
}
if (wait_event_interruptible(pipe_crc->wq, ret = wait_event_interruptible_lock_irq(pipe_crc->wq,
pipe_crc_data_count(pipe_crc))) pipe_crc_data_count(pipe_crc), pipe_crc->lock);
return -ERESTARTSYS; if (ret) {
spin_unlock_irq(&pipe_crc->lock);
return ret;
}
} }
/* We now have one or more entries to read */ /* We now have one or more entries to read */
head = atomic_read(&pipe_crc->head); head = pipe_crc->head;
tail = atomic_read(&pipe_crc->tail); tail = pipe_crc->tail;
n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR),
count / PIPE_CRC_LINE_LEN); count / PIPE_CRC_LINE_LEN);
spin_unlock_irq(&pipe_crc->lock);
bytes_read = 0; bytes_read = 0;
n = 0; n = 0;
do { do {
...@@ -1864,10 +1878,13 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count, ...@@ -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); BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
tail = (tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1); tail = (tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
atomic_set(&pipe_crc->tail, tail);
n++; n++;
} while (--n_entries); } while (--n_entries);
spin_lock_irq(&pipe_crc->lock);
pipe_crc->tail = tail;
spin_unlock_irq(&pipe_crc->lock);
return bytes_read; return bytes_read;
} }
...@@ -2111,8 +2128,10 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, ...@@ -2111,8 +2128,10 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
if (!pipe_crc->entries) if (!pipe_crc->entries)
return -ENOMEM; return -ENOMEM;
atomic_set(&pipe_crc->head, 0); spin_lock_irq(&pipe_crc->lock);
atomic_set(&pipe_crc->tail, 0); pipe_crc->head = 0;
pipe_crc->tail = 0;
spin_unlock_irq(&pipe_crc->lock);
} }
pipe_crc->source = source; pipe_crc->source = source;
...@@ -2122,13 +2141,19 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, ...@@ -2122,13 +2141,19 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
/* real source -> none transition */ /* real source -> none transition */
if (source == INTEL_PIPE_CRC_SOURCE_NONE) { if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
struct intel_pipe_crc_entry *entries;
DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
pipe_name(pipe)); pipe_name(pipe));
intel_wait_for_vblank(dev, 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; pipe_crc->entries = NULL;
spin_unlock_irq(&pipe_crc->lock);
kfree(entries);
} }
return 0; return 0;
...@@ -2823,7 +2848,8 @@ void intel_display_crc_init(struct drm_device *dev) ...@@ -2823,7 +2848,8 @@ void intel_display_crc_init(struct drm_device *dev)
for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) { for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) {
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[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); init_waitqueue_head(&pipe_crc->wq);
} }
} }
......
...@@ -1249,10 +1249,11 @@ struct intel_pipe_crc_entry { ...@@ -1249,10 +1249,11 @@ struct intel_pipe_crc_entry {
#define INTEL_PIPE_CRC_ENTRIES_NR 128 #define INTEL_PIPE_CRC_ENTRIES_NR 128
struct intel_pipe_crc { 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; struct intel_pipe_crc_entry *entries;
enum intel_pipe_crc_source source; enum intel_pipe_crc_source source;
atomic_t head, tail; int head, tail;
wait_queue_head_t wq; wait_queue_head_t wq;
}; };
......
...@@ -1200,15 +1200,19 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe, ...@@ -1200,15 +1200,19 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe,
struct intel_pipe_crc_entry *entry; struct intel_pipe_crc_entry *entry;
int head, tail; int head, tail;
spin_lock(&pipe_crc->lock);
if (!pipe_crc->entries) { if (!pipe_crc->entries) {
spin_unlock(&pipe_crc->lock);
DRM_ERROR("spurious interrupt\n"); DRM_ERROR("spurious interrupt\n");
return; return;
} }
head = atomic_read(&pipe_crc->head); head = pipe_crc->head;
tail = atomic_read(&pipe_crc->tail); tail = pipe_crc->tail;
if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) { if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
spin_unlock(&pipe_crc->lock);
DRM_ERROR("CRC buffer overflowing\n"); DRM_ERROR("CRC buffer overflowing\n");
return; return;
} }
...@@ -1223,7 +1227,9 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe, ...@@ -1223,7 +1227,9 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe,
entry->crc[4] = crc4; entry->crc[4] = crc4;
head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1); 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); wake_up_interruptible(&pipe_crc->wq);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment