drm/i915: Remove the list of pinned inactive objects
authorChris Wilson <chris@chris-wilson.co.uk>
Tue, 24 Apr 2012 14:47:30 +0000 (15:47 +0100)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 3 May 2012 09:18:11 +0000 (11:18 +0200)
Simplify object tracking by removing the inactive but pinned list. The
only place where this was used is for counting the available memory,
which is just as easy performed by checking all objects on the rare
occasions it is required (application startup). For ease of debugging,
we keep the reporting of pinned objects through the error-state and
debugfs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/i915_debugfs.c
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/i915_gem_debug.c
drivers/gpu/drm/i915/i915_gem_evict.c
drivers/gpu/drm/i915/i915_irq.c

index 54a10667fe82b9f62d34426e2be93d0d1bb990c1..ecf746837b23f24230c68a85388e3f40f209c85f 100644 (file)
@@ -178,10 +178,6 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
                seq_printf(m, "Inactive:\n");
                head = &dev_priv->mm.inactive_list;
                break;
-       case PINNED_LIST:
-               seq_printf(m, "Pinned:\n");
-               head = &dev_priv->mm.pinned_list;
-               break;
        case FLUSHING_LIST:
                seq_printf(m, "Flushing:\n");
                head = &dev_priv->mm.flushing_list;
@@ -251,11 +247,6 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
        seq_printf(m, "  %u [%u] active objects, %zu [%zu] bytes\n",
                   count, mappable_count, size, mappable_size);
 
-       size = count = mappable_size = mappable_count = 0;
-       count_objects(&dev_priv->mm.pinned_list, mm_list);
-       seq_printf(m, "  %u [%u] pinned objects, %zu [%zu] bytes\n",
-                  count, mappable_count, size, mappable_size);
-
        size = count = mappable_size = mappable_count = 0;
        count_objects(&dev_priv->mm.inactive_list, mm_list);
        seq_printf(m, "  %u [%u] inactive objects, %zu [%zu] bytes\n",
@@ -294,6 +285,7 @@ static int i915_gem_gtt_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;
+       uintptr_t list = (uintptr_t) node->info_ent->data;
        struct drm_i915_private *dev_priv = dev->dev_private;
        struct drm_i915_gem_object *obj;
        size_t total_obj_size, total_gtt_size;
@@ -305,6 +297,9 @@ static int i915_gem_gtt_info(struct seq_file *m, void* data)
 
        total_obj_size = total_gtt_size = count = 0;
        list_for_each_entry(obj, &dev_priv->mm.gtt_list, gtt_list) {
+               if (list == PINNED_LIST && obj->pin_count == 0)
+                       continue;
+
                seq_printf(m, "   ");
                describe_obj(m, obj);
                seq_printf(m, "\n");
@@ -321,7 +316,6 @@ static int i915_gem_gtt_info(struct seq_file *m, void* data)
        return 0;
 }
 
-
 static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 {
        struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -1842,10 +1836,10 @@ static struct drm_info_list i915_debugfs_list[] = {
        {"i915_capabilities", i915_capabilities, 0},
        {"i915_gem_objects", i915_gem_object_info, 0},
        {"i915_gem_gtt", i915_gem_gtt_info, 0},
+       {"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
        {"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
        {"i915_gem_flushing", i915_gem_object_list_info, 0, (void *) FLUSHING_LIST},
        {"i915_gem_inactive", i915_gem_object_list_info, 0, (void *) INACTIVE_LIST},
-       {"i915_gem_pinned", i915_gem_object_list_info, 0, (void *) PINNED_LIST},
        {"i915_gem_deferred_free", i915_gem_object_list_info, 0, (void *) DEFERRED_FREE_LIST},
        {"i915_gem_pageflip", i915_gem_pageflip_info, 0},
        {"i915_gem_request", i915_gem_request_info, 0},
index 57f60fa719f47c4d718d365a2e641a273163a129..560ce7f44a3ba5b3f55caa1fb708eebca6fd96d9 100644 (file)
@@ -689,12 +689,6 @@ typedef struct drm_i915_private {
                 */
                struct list_head inactive_list;
 
-               /**
-                * LRU list of objects which are not in the ringbuffer but
-                * are still pinned in the GTT.
-                */
-               struct list_head pinned_list;
-
                /** LRU list of objects with fence regs on them. */
                struct list_head fence_list;
 
index 2fc7c55e0e0583132856bfd846134fb8305709d9..bf4683c9aed85ded7e154e8af1e23afbc5dc1baa 100644 (file)
@@ -132,7 +132,7 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 static inline bool
 i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
 {
-       return !obj->active && obj->pin_count == 0;
+       return !obj->active;
 }
 
 int
@@ -171,8 +171,9 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 
        pinned = 0;
        mutex_lock(&dev->struct_mutex);
-       list_for_each_entry(obj, &dev_priv->mm.pinned_list, mm_list)
-               pinned += obj->gtt_space->size;
+       list_for_each_entry(obj, &dev_priv->mm.gtt_list, gtt_list)
+               if (obj->pin_count)
+                       pinned += obj->gtt_space->size;
        mutex_unlock(&dev->struct_mutex);
 
        args->aper_size = dev_priv->mm.gtt_total;
@@ -1455,10 +1456,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
        struct drm_device *dev = obj->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
 
-       if (obj->pin_count != 0)
-               list_move_tail(&obj->mm_list, &dev_priv->mm.pinned_list);
-       else
-               list_move_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
+       list_move_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
 
        BUG_ON(!list_empty(&obj->gpu_write_list));
        BUG_ON(!obj->active);
@@ -3063,12 +3061,9 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
                    uint32_t alignment,
                    bool map_and_fenceable)
 {
-       struct drm_device *dev = obj->base.dev;
-       struct drm_i915_private *dev_priv = dev->dev_private;
        int ret;
 
        BUG_ON(obj->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT);
-       WARN_ON(i915_verify_lists(dev));
 
        if (obj->gtt_space != NULL) {
                if ((alignment && obj->gtt_offset & (alignment - 1)) ||
@@ -3096,34 +3091,20 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
        if (!obj->has_global_gtt_mapping && map_and_fenceable)
                i915_gem_gtt_bind_object(obj, obj->cache_level);
 
-       if (obj->pin_count++ == 0) {
-               if (!obj->active)
-                       list_move_tail(&obj->mm_list,
-                                      &dev_priv->mm.pinned_list);
-       }
+       obj->pin_count++;
        obj->pin_mappable |= map_and_fenceable;
 
-       WARN_ON(i915_verify_lists(dev));
        return 0;
 }
 
 void
 i915_gem_object_unpin(struct drm_i915_gem_object *obj)
 {
-       struct drm_device *dev = obj->base.dev;
-       drm_i915_private_t *dev_priv = dev->dev_private;
-
-       WARN_ON(i915_verify_lists(dev));
        BUG_ON(obj->pin_count == 0);
        BUG_ON(obj->gtt_space == NULL);
 
-       if (--obj->pin_count == 0) {
-               if (!obj->active)
-                       list_move_tail(&obj->mm_list,
-                                      &dev_priv->mm.inactive_list);
+       if (--obj->pin_count == 0)
                obj->pin_mappable = false;
-       }
-       WARN_ON(i915_verify_lists(dev));
 }
 
 int
@@ -3426,12 +3407,10 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
        struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
        struct drm_device *dev = obj->base.dev;
 
-       while (obj->pin_count > 0)
-               i915_gem_object_unpin(obj);
-
        if (obj->phys_obj)
                i915_gem_detach_phys_object(dev, obj);
 
+       obj->pin_count = 0;
        i915_gem_free_object_tail(obj);
 }
 
@@ -3699,7 +3678,6 @@ i915_gem_load(struct drm_device *dev)
        INIT_LIST_HEAD(&dev_priv->mm.active_list);
        INIT_LIST_HEAD(&dev_priv->mm.flushing_list);
        INIT_LIST_HEAD(&dev_priv->mm.inactive_list);
-       INIT_LIST_HEAD(&dev_priv->mm.pinned_list);
        INIT_LIST_HEAD(&dev_priv->mm.fence_list);
        INIT_LIST_HEAD(&dev_priv->mm.deferred_free_list);
        INIT_LIST_HEAD(&dev_priv->mm.gtt_list);
index cc93cac242d650a595d447ecf10a95a2765a7d36..a4f6aaabca99a7d6268e5bf59a7025a6be5b6eae 100644 (file)
@@ -114,22 +114,6 @@ i915_verify_lists(struct drm_device *dev)
                }
        }
 
-       list_for_each_entry(obj, &dev_priv->mm.pinned_list, list) {
-               if (obj->base.dev != dev ||
-                   !atomic_read(&obj->base.refcount.refcount)) {
-                       DRM_ERROR("freed pinned %p\n", obj);
-                       err++;
-                       break;
-               } else if (!obj->pin_count || obj->active ||
-                          (obj->base.write_domain & I915_GEM_GPU_DOMAINS)) {
-                       DRM_ERROR("invalid pinned %p (p %d a %d w %x)\n",
-                                 obj,
-                                 obj->pin_count, obj->active,
-                                 obj->base.write_domain);
-                       err++;
-               }
-       }
-
        return warned = err;
 }
 #endif /* WATCH_INACTIVE */
index 399a3a8946b3895d27487b1cb4afd920adffa1f0..91ebb94d7c8be35b668fe194643d7b7c4c5df058 100644 (file)
@@ -35,6 +35,9 @@
 static bool
 mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind)
 {
+       if (obj->pin_count)
+               return false;
+
        list_add(&obj->exec_list, unwind);
        return drm_mm_scan_add_block(obj->gtt_space);
 }
@@ -90,7 +93,7 @@ i915_gem_evict_something(struct drm_device *dev, int min_size,
        /* Now merge in the soon-to-be-expired objects... */
        list_for_each_entry(obj, &dev_priv->mm.active_list, mm_list) {
                /* Does the object require an outstanding flush? */
-               if (obj->base.write_domain || obj->pin_count)
+               if (obj->base.write_domain)
                        continue;
 
                if (mark_free(obj, &unwind_list))
@@ -99,14 +102,11 @@ i915_gem_evict_something(struct drm_device *dev, int min_size,
 
        /* Finally add anything with a pending flush (in order of retirement) */
        list_for_each_entry(obj, &dev_priv->mm.flushing_list, mm_list) {
-               if (obj->pin_count)
-                       continue;
-
                if (mark_free(obj, &unwind_list))
                        goto found;
        }
        list_for_each_entry(obj, &dev_priv->mm.active_list, mm_list) {
-               if (!obj->base.write_domain || obj->pin_count)
+               if (!obj->base.write_domain)
                        continue;
 
                if (mark_free(obj, &unwind_list))
index 26172eef97876e57bdfd93a0c19f264315b83318..24e1dd25257558fd504042788bc3ff629a241bb5 100644 (file)
@@ -918,37 +918,56 @@ i915_error_state_free(struct drm_device *dev,
        kfree(error->overlay);
        kfree(error);
 }
-
-static u32 capture_bo_list(struct drm_i915_error_buffer *err,
-                          int count,
-                          struct list_head *head)
+static void capture_bo(struct drm_i915_error_buffer *err,
+                      struct drm_i915_gem_object *obj)
+{
+       err->size = obj->base.size;
+       err->name = obj->base.name;
+       err->seqno = obj->last_rendering_seqno;
+       err->gtt_offset = obj->gtt_offset;
+       err->read_domains = obj->base.read_domains;
+       err->write_domain = obj->base.write_domain;
+       err->fence_reg = obj->fence_reg;
+       err->pinned = 0;
+       if (obj->pin_count > 0)
+               err->pinned = 1;
+       if (obj->user_pin_count > 0)
+               err->pinned = -1;
+       err->tiling = obj->tiling_mode;
+       err->dirty = obj->dirty;
+       err->purgeable = obj->madv != I915_MADV_WILLNEED;
+       err->ring = obj->ring ? obj->ring->id : -1;
+       err->cache_level = obj->cache_level;
+}
+
+static u32 capture_active_bo(struct drm_i915_error_buffer *err,
+                            int count, struct list_head *head)
 {
        struct drm_i915_gem_object *obj;
        int i = 0;
 
        list_for_each_entry(obj, head, mm_list) {
-               err->size = obj->base.size;
-               err->name = obj->base.name;
-               err->seqno = obj->last_rendering_seqno;
-               err->gtt_offset = obj->gtt_offset;
-               err->read_domains = obj->base.read_domains;
-               err->write_domain = obj->base.write_domain;
-               err->fence_reg = obj->fence_reg;
-               err->pinned = 0;
-               if (obj->pin_count > 0)
-                       err->pinned = 1;
-               if (obj->user_pin_count > 0)
-                       err->pinned = -1;
-               err->tiling = obj->tiling_mode;
-               err->dirty = obj->dirty;
-               err->purgeable = obj->madv != I915_MADV_WILLNEED;
-               err->ring = obj->ring ? obj->ring->id : -1;
-               err->cache_level = obj->cache_level;
-
+               capture_bo(err++, obj);
                if (++i == count)
                        break;
+       }
+
+       return i;
+}
 
-               err++;
+static u32 capture_pinned_bo(struct drm_i915_error_buffer *err,
+                            int count, struct list_head *head)
+{
+       struct drm_i915_gem_object *obj;
+       int i = 0;
+
+       list_for_each_entry(obj, head, gtt_list) {
+               if (obj->pin_count == 0)
+                       continue;
+
+               capture_bo(err++, obj);
+               if (++i == count)
+                       break;
        }
 
        return i;
@@ -1155,8 +1174,9 @@ static void i915_capture_error_state(struct drm_device *dev)
        list_for_each_entry(obj, &dev_priv->mm.active_list, mm_list)
                i++;
        error->active_bo_count = i;
-       list_for_each_entry(obj, &dev_priv->mm.pinned_list, mm_list)
-               i++;
+       list_for_each_entry(obj, &dev_priv->mm.gtt_list, gtt_list)
+               if (obj->pin_count)
+                       i++;
        error->pinned_bo_count = i - error->active_bo_count;
 
        error->active_bo = NULL;
@@ -1171,15 +1191,15 @@ static void i915_capture_error_state(struct drm_device *dev)
 
        if (error->active_bo)
                error->active_bo_count =
-                       capture_bo_list(error->active_bo,
-                                       error->active_bo_count,
-                                       &dev_priv->mm.active_list);
+                       capture_active_bo(error->active_bo,
+                                         error->active_bo_count,
+                                         &dev_priv->mm.active_list);
 
        if (error->pinned_bo)
                error->pinned_bo_count =
-                       capture_bo_list(error->pinned_bo,
-                                       error->pinned_bo_count,
-                                       &dev_priv->mm.pinned_list);
+                       capture_pinned_bo(error->pinned_bo,
+                                         error->pinned_bo_count,
+                                         &dev_priv->mm.gtt_list);
 
        do_gettimeofday(&error->time);