drm/i915: Kill the active list spinlock
This spinlock only served debugging purposes in a time when we could not
be sure of the mutex ever being released upon a GPU hang. As we now
should be able rely on hangcheck to do the job for us (and that error
reporting should not itself require the struct mutex) we can kill the
incomplete attempt at protection.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 16133f1..9074300 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -72,12 +72,15 @@
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj_priv;
- spinlock_t *lock = NULL;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
switch (list) {
case ACTIVE_LIST:
seq_printf(m, "Active:\n");
- lock = &dev_priv->mm.active_list_lock;
head = &dev_priv->render_ring.active_list;
break;
case INACTIVE_LIST:
@@ -89,14 +92,11 @@
head = &dev_priv->mm.flushing_list;
break;
default:
- DRM_INFO("Ooops, unexpected list\n");
- return 0;
+ mutex_unlock(&dev->struct_mutex);
+ return -EINVAL;
}
- if (lock)
- spin_lock(lock);
- list_for_each_entry(obj_priv, head, list)
- {
+ list_for_each_entry(obj_priv, head, list) {
seq_printf(m, " %p: %s %8zd %08x %08x %d%s%s",
&obj_priv->base,
get_pin_flag(obj_priv),
@@ -117,8 +117,7 @@
seq_printf(m, "\n");
}
- if (lock)
- spin_unlock(lock);
+ mutex_unlock(&dev->struct_mutex);
return 0;
}
@@ -176,6 +175,11 @@
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_gem_request *gem_request;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
seq_printf(m, "Request:\n");
list_for_each_entry(gem_request, &dev_priv->render_ring.request_list,
@@ -184,6 +188,8 @@
gem_request->seqno,
(int) (jiffies - gem_request->emitted_jiffies));
}
+ mutex_unlock(&dev->struct_mutex);
+
return 0;
}
@@ -192,6 +198,11 @@
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
if (dev_priv->render_ring.status_page.page_addr != NULL) {
seq_printf(m, "Current sequence: %d\n",
@@ -202,6 +213,9 @@
seq_printf(m, "Waiter sequence: %d\n",
dev_priv->mm.waiting_gem_seqno);
seq_printf(m, "IRQ sequence: %d\n", dev_priv->mm.irq_gem_seqno);
+
+ mutex_unlock(&dev->struct_mutex);
+
return 0;
}
@@ -211,6 +225,11 @@
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
if (!HAS_PCH_SPLIT(dev)) {
seq_printf(m, "Interrupt enable: %08x\n",
@@ -255,6 +274,8 @@
dev_priv->mm.waiting_gem_seqno);
seq_printf(m, "IRQ sequence: %d\n",
dev_priv->mm.irq_gem_seqno);
+ mutex_unlock(&dev->struct_mutex);
+
return 0;
}
@@ -263,7 +284,11 @@
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- int i;
+ int i, ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
seq_printf(m, "Reserved fences = %d\n", dev_priv->fence_reg_start);
seq_printf(m, "Total fences = %d\n", dev_priv->num_fence_regs);
@@ -289,6 +314,7 @@
seq_printf(m, "\n");
}
}
+ mutex_unlock(&dev->struct_mutex);
return 0;
}
@@ -319,10 +345,10 @@
uint32_t *mem;
for (page = 0; page < page_count; page++) {
- mem = kmap_atomic(pages[page], KM_USER0);
+ mem = kmap(pages[page]);
for (i = 0; i < PAGE_SIZE; i += 4)
seq_printf(m, "%08x : %08x\n", i, mem[i / 4]);
- kunmap_atomic(mem, KM_USER0);
+ kunmap(pages[page]);
}
}
@@ -335,7 +361,9 @@
struct drm_i915_gem_object *obj_priv;
int ret;
- spin_lock(&dev_priv->mm.active_list_lock);
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
list_for_each_entry(obj_priv, &dev_priv->render_ring.active_list,
list) {
@@ -343,8 +371,7 @@
if (obj->read_domains & I915_GEM_DOMAIN_COMMAND) {
ret = i915_gem_object_get_pages(obj, 0);
if (ret) {
- DRM_ERROR("Failed to get pages: %d\n", ret);
- spin_unlock(&dev_priv->mm.active_list_lock);
+ mutex_unlock(&dev->struct_mutex);
return ret;
}
@@ -355,7 +382,7 @@
}
}
- spin_unlock(&dev_priv->mm.active_list_lock);
+ mutex_unlock(&dev->struct_mutex);
return 0;
}
@@ -365,20 +392,24 @@
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- u8 *virt;
- uint32_t *ptr, off;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
if (!dev_priv->render_ring.gem_object) {
seq_printf(m, "No ringbuffer setup\n");
- return 0;
- }
+ } else {
+ u8 *virt = dev_priv->render_ring.virtual_start;
+ uint32_t off;
- virt = dev_priv->render_ring.virtual_start;
-
- for (off = 0; off < dev_priv->render_ring.size; off += 4) {
- ptr = (uint32_t *)(virt + off);
- seq_printf(m, "%08x : %08x\n", off, *ptr);
+ for (off = 0; off < dev_priv->render_ring.size; off += 4) {
+ uint32_t *ptr = (uint32_t *)(virt + off);
+ seq_printf(m, "%08x : %08x\n", off, *ptr);
+ }
}
+ mutex_unlock(&dev->struct_mutex);
return 0;
}
@@ -694,10 +725,16 @@
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
unsigned long temp, chipset, gfx;
+ int ret;
+
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
temp = i915_mch_val(dev_priv);
chipset = i915_chipset_val(dev_priv);
gfx = i915_gfx_val(dev_priv);
+ mutex_unlock(&dev->struct_mutex);
seq_printf(m, "GMCH temp: %ld\n", temp);
seq_printf(m, "Chipset power: %ld\n", chipset);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 634e1c4..e6fbeb4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -524,8 +524,6 @@
*/
struct list_head shrink_list;
- spinlock_t active_list_lock;
-
/**
* List of objects which are not in the ringbuffer but which
* still have a write_domain which needs to be flushed before
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index afe4a9b..b6e4b60 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1486,7 +1486,6 @@
struct intel_ring_buffer *ring)
{
struct drm_device *dev = obj->dev;
- drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
uint32_t seqno = i915_gem_next_request_seqno(dev, ring);
@@ -1500,9 +1499,7 @@
}
/* Move from whatever list we were on to the tail of execution. */
- spin_lock(&dev_priv->mm.active_list_lock);
list_move_tail(&obj_priv->list, &ring->active_list);
- spin_unlock(&dev_priv->mm.active_list_lock);
obj_priv->last_rendering_seqno = seqno;
}
@@ -1676,14 +1673,11 @@
i915_gem_retire_request(struct drm_device *dev,
struct drm_i915_gem_request *request)
{
- drm_i915_private_t *dev_priv = dev->dev_private;
-
trace_i915_gem_request_retire(dev, request->seqno);
/* Move any buffers on the active list that are no longer referenced
* by the ringbuffer to the flushing/inactive lists as appropriate.
*/
- spin_lock(&dev_priv->mm.active_list_lock);
while (!list_empty(&request->ring->active_list)) {
struct drm_gem_object *obj;
struct drm_i915_gem_object *obj_priv;
@@ -1698,7 +1692,7 @@
* this seqno.
*/
if (obj_priv->last_rendering_seqno != request->seqno)
- goto out;
+ return;
#if WATCH_LRU
DRM_INFO("%s: retire %d moves to inactive list %p\n",
@@ -1707,22 +1701,9 @@
if (obj->write_domain != 0)
i915_gem_object_move_to_flushing(obj);
- else {
- /* Take a reference on the object so it won't be
- * freed while the spinlock is held. The list
- * protection for this spinlock is safe when breaking
- * the lock like this since the next thing we do
- * is just get the head of the list again.
- */
- drm_gem_object_reference(obj);
+ else
i915_gem_object_move_to_inactive(obj);
- spin_unlock(&dev_priv->mm.active_list_lock);
- drm_gem_object_unreference(obj);
- spin_lock(&dev_priv->mm.active_list_lock);
- }
}
-out:
- spin_unlock(&dev_priv->mm.active_list_lock);
}
/**
@@ -1972,7 +1953,6 @@
i915_gem_object_unbind(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
- drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
int ret = 0;
@@ -2027,10 +2007,8 @@
}
/* Remove ourselves from the LRU list if present. */
- spin_lock(&dev_priv->mm.active_list_lock);
if (!list_empty(&obj_priv->list))
list_del_init(&obj_priv->list);
- spin_unlock(&dev_priv->mm.active_list_lock);
if (i915_gem_object_is_purgeable(obj_priv))
i915_gem_object_truncate(obj);
@@ -2047,13 +2025,10 @@
bool lists_empty;
int ret;
- spin_lock(&dev_priv->mm.active_list_lock);
lists_empty = (list_empty(&dev_priv->mm.flushing_list) &&
list_empty(&dev_priv->render_ring.active_list) &&
(!HAS_BSD(dev) ||
list_empty(&dev_priv->bsd_ring.active_list)));
- spin_unlock(&dev_priv->mm.active_list_lock);
-
if (lists_empty)
return 0;
@@ -4550,11 +4525,8 @@
return ret;
}
- spin_lock(&dev_priv->mm.active_list_lock);
BUG_ON(!list_empty(&dev_priv->render_ring.active_list));
BUG_ON(HAS_BSD(dev) && !list_empty(&dev_priv->bsd_ring.active_list));
- spin_unlock(&dev_priv->mm.active_list_lock);
-
BUG_ON(!list_empty(&dev_priv->mm.flushing_list));
BUG_ON(!list_empty(&dev_priv->mm.inactive_list));
BUG_ON(!list_empty(&dev_priv->render_ring.request_list));
@@ -4606,7 +4578,6 @@
int i;
drm_i915_private_t *dev_priv = dev->dev_private;
- spin_lock_init(&dev_priv->mm.active_list_lock);
INIT_LIST_HEAD(&dev_priv->mm.flushing_list);
INIT_LIST_HEAD(&dev_priv->mm.gpu_write_list);
INIT_LIST_HEAD(&dev_priv->mm.inactive_list);
@@ -4862,12 +4833,10 @@
drm_i915_private_t *dev_priv = dev->dev_private;
int lists_empty;
- spin_lock(&dev_priv->mm.active_list_lock);
lists_empty = list_empty(&dev_priv->mm.flushing_list) &&
list_empty(&dev_priv->render_ring.active_list);
if (HAS_BSD(dev))
lists_empty &= list_empty(&dev_priv->bsd_ring.active_list);
- spin_unlock(&dev_priv->mm.active_list_lock);
return !lists_empty;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 72cae3c..82430e2 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -212,14 +212,11 @@
int ret;
bool lists_empty;
- spin_lock(&dev_priv->mm.active_list_lock);
lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
list_empty(&dev_priv->mm.flushing_list) &&
list_empty(&dev_priv->render_ring.active_list) &&
(!HAS_BSD(dev)
|| list_empty(&dev_priv->bsd_ring.active_list)));
- spin_unlock(&dev_priv->mm.active_list_lock);
-
if (lists_empty)
return -ENOSPC;
@@ -234,13 +231,11 @@
if (ret)
return ret;
- spin_lock(&dev_priv->mm.active_list_lock);
lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
list_empty(&dev_priv->mm.flushing_list) &&
list_empty(&dev_priv->render_ring.active_list) &&
(!HAS_BSD(dev)
|| list_empty(&dev_priv->bsd_ring.active_list)));
- spin_unlock(&dev_priv->mm.active_list_lock);
BUG_ON(!lists_empty);
return 0;