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 16133f10ffaae10b6bf1c1093a62b31f65dca754..9074300fed8d8a4527018591a768cf7305b0e4fb 100644 (file)
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -72,12 +72,15 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
        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 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
                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 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
                seq_printf(m, "\n");
        }
 
-       if (lock)
-           spin_unlock(lock);
+       mutex_unlock(&dev->struct_mutex);
        return 0;
 }
 
@@ -176,6 +175,11 @@ static int i915_gem_request_info(struct seq_file *m, void *data)
        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 @@ static int i915_gem_request_info(struct seq_file *m, void *data)
                           gem_request->seqno,
                           (int) (jiffies - gem_request->emitted_jiffies));
        }
+       mutex_unlock(&dev->struct_mutex);
+
        return 0;
 }
 
@@ -192,6 +198,11 @@ static int i915_gem_seqno_info(struct seq_file *m, void *data)
        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 @@ static int i915_gem_seqno_info(struct seq_file *m, void *data)
        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 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
        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 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
                   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 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data)
        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 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data)
                        seq_printf(m, "\n");
                }
        }
+       mutex_unlock(&dev->struct_mutex);
 
        return 0;
 }
@@ -319,10 +345,10 @@ static void i915_dump_pages(struct seq_file *m, struct page **pages, int page_co
        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 @@ static int i915_batchbuffer_info(struct seq_file *m, void *data)
        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 @@ static int i915_batchbuffer_info(struct seq_file *m, void *data)
                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 @@ static int i915_batchbuffer_info(struct seq_file *m, void *data)
                }
        }
 
-       spin_unlock(&dev_priv->mm.active_list_lock);
+       mutex_unlock(&dev->struct_mutex);
 
        return 0;
 }
@@ -365,20 +392,24 @@ static int i915_ringbuffer_data(struct seq_file *m, void *data)
        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;
-       }
-
-       virt = dev_priv->render_ring.virtual_start;
+       } else {
+               u8 *virt = dev_priv->render_ring.virtual_start;
+               uint32_t off;
 
-       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 @@ static int i915_emon_status(struct seq_file *m, void *unused)
        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 634e1c463dec76fdb0dc9fd2a82656e41d5e200d..e6fbeb43d59c851f452363896e24133a3450a04e 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -524,8 +524,6 @@ typedef struct drm_i915_private {
                 */
                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 afe4a9b0a03d2526a6869215db453c98b42da848..b6e4b60724ec6de99ef2c5467d0ac7f77f9d8192 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1486,7 +1486,6 @@ i915_gem_object_move_to_active(struct drm_gem_object *obj,
                               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 @@ i915_gem_object_move_to_active(struct drm_gem_object *obj,
        }
 
        /* 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 @@ static void
 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 @@ i915_gem_retire_request(struct drm_device *dev,
                 * 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 @@ i915_gem_retire_request(struct drm_device *dev,
 
                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 @@ int
 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 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
        }
 
        /* 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 @@ i915_gpu_idle(struct drm_device *dev)
        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 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void *data,
                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 @@ i915_gem_load(struct drm_device *dev)
        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 @@ i915_gpu_is_active(struct drm_device *dev)
        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 72cae3cccad8802641d973542ab6c440445b81db..82430e21c7ab18c5e7f6af04d4cb330396d7b9f7 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -212,14 +212,11 @@ i915_gem_evict_everything(struct drm_device *dev)
        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 @@ i915_gem_evict_everything(struct drm_device *dev)
        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;