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 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..ae182af 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; } @@ -1798,14 +1807,11 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep) /* account for \'0' */ #define PIPE_CRC_BUFFER_LEN (PIPE_CRC_LINE_LEN + 1) +/* needs pipe_crc->lock */ 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); + 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