From patchwork Wed Aug 4 13:09:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 117035 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter.kernel.org (8.14.4/8.14.3) with ESMTP id o74DAr3X009094 for ; Wed, 4 Aug 2010 13:11:28 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B97909EF5E for ; Wed, 4 Aug 2010 06:10:52 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (server109-228-4-14.live-servers.net [109.228.4.14]) by gabe.freedesktop.org (Postfix) with ESMTP id 7B7A49E747 for ; Wed, 4 Aug 2010 06:10:43 -0700 (PDT) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.66.37; Received: from arrandale.alporthouse.com (unverified [78.156.66.37]) by fireflyinternet.com (Firefly Internet SMTP) with ESMTP id 1759972-1500048 for multiple; Wed, 04 Aug 2010 14:12:22 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Wed, 4 Aug 2010 14:09:45 +0100 Message-Id: <1280927385-7184-1-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 1.7.1 X-Originating-IP: 78.156.66.37 Subject: [Intel-gfx] [PATCH] drm/i915: Kill the active list spinlock X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter.kernel.org [140.211.167.41]); Wed, 04 Aug 2010 13:11:28 +0000 (UTC) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9214119..08f279b 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -71,12 +71,10 @@ 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; 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,11 +87,12 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data) break; default: DRM_INFO("Ooops, unexpected list\n"); - return 0; + return -EINVAL; } - if (lock) - spin_lock(lock); + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + list_for_each_entry(obj_priv, head, list) { seq_printf(m, " %p: %s %8zd %08x %08x %d%s%s", @@ -116,8 +115,8 @@ 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; } @@ -128,6 +127,9 @@ static int i915_gem_request_info(struct seq_file *m, void *data) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_request *gem_request; + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + seq_printf(m, "Request:\n"); list_for_each_entry(gem_request, &dev_priv->render_ring.request_list, list) { @@ -135,6 +137,9 @@ 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; } @@ -144,6 +149,9 @@ static int i915_gem_seqno_info(struct seq_file *m, void *data) struct drm_device *dev = node->minor->dev; drm_i915_private_t *dev_priv = dev->dev_private; + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + if (dev_priv->render_ring.status_page.page_addr != NULL) { seq_printf(m, "Current sequence: %d\n", i915_get_gem_seqno(dev, &dev_priv->render_ring)); @@ -153,6 +161,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; } @@ -216,6 +227,9 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data) drm_i915_private_t *dev_priv = dev->dev_private; int i; + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + seq_printf(m, "Reserved fences = %d\n", dev_priv->fence_reg_start); seq_printf(m, "Total fences = %d\n", dev_priv->num_fence_regs); for (i = 0; i < dev_priv->num_fence_regs; i++) { @@ -241,6 +255,8 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data) } } + mutex_unlock(&dev->struct_mutex); + return 0; } @@ -284,9 +300,10 @@ static int i915_batchbuffer_info(struct seq_file *m, void *data) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_gem_object *obj; struct drm_i915_gem_object *obj_priv; - int ret; + int ret = 0; - spin_lock(&dev_priv->mm.active_list_lock); + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; list_for_each_entry(obj_priv, &dev_priv->render_ring.active_list, list) { @@ -295,8 +312,7 @@ static int i915_batchbuffer_info(struct seq_file *m, void *data) 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); - return ret; + break; } seq_printf(m, "--- gtt_offset = 0x%08x\n", obj_priv->gtt_offset); @@ -306,9 +322,9 @@ 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; + return ret; } static int i915_ringbuffer_data(struct seq_file *m, void *data) @@ -324,6 +340,9 @@ static int i915_ringbuffer_data(struct seq_file *m, void *data) return 0; } + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + virt = dev_priv->render_ring.virtual_start; for (off = 0; off < dev_priv->render_ring.size; off += 4) { @@ -331,6 +350,8 @@ static int i915_ringbuffer_data(struct seq_file *m, void *data) seq_printf(m, "%08x : %08x\n", off, *ptr); } + mutex_unlock(&dev->struct_mutex); + return 0; } @@ -585,6 +606,9 @@ static int i915_fbc_status(struct seq_file *m, void *unused) return 0; } + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + if (intel_fbc_enabled(dev)) { seq_printf(m, "FBC enabled\n"); } else { @@ -613,6 +637,9 @@ static int i915_fbc_status(struct seq_file *m, void *unused) } seq_printf(m, "\n"); } + + mutex_unlock(&dev->struct_mutex); + return 0; } @@ -643,10 +670,15 @@ static int i915_emon_status(struct seq_file *m, void *unused) drm_i915_private_t *dev_priv = dev->dev_private; unsigned long temp, chipset, gfx; + if (!mutex_trylock(&dev->struct_mutex)) + return -EBUSY; + 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); seq_printf(m, "GFX power: %ld\n", gfx); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 844d10e..ad8dab5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -518,8 +518,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 36d7e93..0f0c387 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1463,8 +1463,6 @@ static void i915_gem_object_move_to_active(struct drm_gem_object *obj, uint32_t seqno, 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); BUG_ON(ring == NULL); obj_priv->ring = ring; @@ -1475,9 +1473,7 @@ i915_gem_object_move_to_active(struct drm_gem_object *obj, uint32_t seqno, obj_priv->active = 1; } /* 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; } @@ -1648,14 +1644,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; @@ -1670,7 +1663,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", @@ -1679,22 +1672,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); } /** @@ -1934,7 +1914,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; @@ -1991,10 +1970,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); @@ -2012,13 +1989,10 @@ i915_gpu_idle(struct drm_device *dev) uint32_t seqno1, seqno2; 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; @@ -4532,10 +4506,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)); @@ -4588,7 +4560,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); @@ -4843,12 +4814,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 4e5af69..6804b34 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -180,14 +180,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; @@ -202,13 +199,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;