From patchwork Sat Apr 16 09:17:31 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 712031 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p3G9Irdq008515 for ; Sat, 16 Apr 2011 09:19:13 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 71C819E918 for ; Sat, 16 Apr 2011 02:18:53 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (server109-228-6-236.live-servers.net [109.228.6.236]) by gabe.freedesktop.org (Postfix) with ESMTP id 76E289EB14 for ; Sat, 16 Apr 2011 02:18:04 -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 32233669-1500050 for multiple; Sat, 16 Apr 2011 10:17:50 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Sat, 16 Apr 2011 10:17:31 +0100 Message-Id: <1302945465-32115-8-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1302945465-32115-1-git-send-email-chris@chris-wilson.co.uk> References: <1302945465-32115-1-git-send-email-chris@chris-wilson.co.uk> X-Originating-IP: 78.156.66.37 Cc: Andy Whitcroft , Daniel Vetter Subject: [Intel-gfx] [PATCH 07/21] drm/i915: Track fence setup separately from fenced object lifetime 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.6 (demeter1.kernel.org [140.211.167.41]); Sat, 16 Apr 2011 09:19:13 +0000 (UTC) This fixes a bookkeeping error causing an OOPS whilst waiting for an object to finish using a fence. Now we can simply wait for the fence to be written independent of the objects currently inhabiting it (past, present and future). A large amount of the change is to delay updating the information about the fence on bo until after we successfully write, or queue the write to, the register. This avoids the complication of undoing a partial change should we fail in pipelining the change. Cc: Andy Whitcroft Cc: Daniel Vetter Signed-off-by: Chris Wilson Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 155 ++++++++++++++++++++------------------- 2 files changed, 82 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9b22f11..0296967 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -129,6 +129,7 @@ struct drm_i915_master_private { struct drm_i915_fence_reg { struct list_head lru_list; struct drm_i915_gem_object *obj; + struct intel_ring_buffer *setup_ring; uint32_t setup_seqno; }; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8c835de..b9515ac 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1730,6 +1730,8 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) i915_gem_object_move_off_active(obj); obj->fenced_gpu_access = false; + obj->last_fenced_seqno = 0; + obj->active = 0; obj->pending_gpu_write = false; drm_gem_object_unreference(&obj->base); @@ -1895,7 +1897,6 @@ static void i915_gem_reset_fences(struct drm_device *dev) reg->obj->fence_reg = I915_FENCE_REG_NONE; reg->obj->fenced_gpu_access = false; reg->obj->last_fenced_seqno = 0; - reg->obj->last_fenced_ring = NULL; i915_gem_clear_fence_reg(dev, reg); } } @@ -2525,7 +2526,7 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj, if (obj->fenced_gpu_access) { if (obj->base.write_domain & I915_GEM_GPU_DOMAINS) { - ret = i915_gem_flush_ring(obj->last_fenced_ring, + ret = i915_gem_flush_ring(obj->ring, 0, obj->base.write_domain); if (ret) return ret; @@ -2536,17 +2537,22 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj, obj->fenced_gpu_access = false; } + if (obj->last_fenced_seqno && + ring_passed_seqno(obj->last_fenced_ring, obj->last_fenced_seqno)) + obj->last_fenced_seqno = 0; + if (obj->last_fenced_seqno && pipelined != obj->last_fenced_ring) { - if (!ring_passed_seqno(obj->last_fenced_ring, - obj->last_fenced_seqno)) { - ret = i915_wait_request(obj->last_fenced_ring, - obj->last_fenced_seqno); - if (ret) - return ret; - } + ret = i915_wait_request(obj->last_fenced_ring, + obj->last_fenced_seqno); + if (ret) + return ret; + /* Since last_fence_seqno can retire much earlier than + * last_rendering_seqno, we track that here for efficiency. + * (With a catch-all in move_to_inactive() to prevent very + * old seqno from lying around.) + */ obj->last_fenced_seqno = 0; - obj->last_fenced_ring = NULL; } /* Ensure that all CPU reads are completed before installing a fence @@ -2613,7 +2619,7 @@ i915_find_fence_reg(struct drm_device *dev, first = reg; if (!pipelined || - !reg->obj->last_fenced_ring || + !reg->obj->last_fenced_seqno || reg->obj->last_fenced_ring == pipelined) { avail = reg; break; @@ -2630,7 +2636,6 @@ i915_find_fence_reg(struct drm_device *dev, * i915_gem_object_get_fence - set up a fence reg for an object * @obj: object to map through a fence reg * @pipelined: ring on which to queue the change, or NULL for CPU access - * @interruptible: must we wait uninterruptibly for the register to retire? * * When mapping objects through the GTT, userspace wants to be able to write * to them without having to worry about swizzling if the object is tiled. @@ -2638,6 +2643,10 @@ i915_find_fence_reg(struct drm_device *dev, * This function walks the fence regs looking for a free one for @obj, * stealing one if it can't find any. * + * Note: if two fence registers point to the same or overlapping memory region + * the results are undefined. This is even more fun with asynchronous updates + * via the GPU! + * * It then sets up the reg based on the object's properties: address, pitch * and tiling format. */ @@ -2648,6 +2657,7 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj, struct drm_device *dev = obj->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_fence_reg *reg; + struct drm_i915_gem_object *old = NULL; int regnum; int ret; @@ -2657,45 +2667,21 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj, /* Just update our place in the LRU if our fence is getting reused. */ if (obj->fence_reg != I915_FENCE_REG_NONE) { reg = &dev_priv->fence_regs[obj->fence_reg]; - list_move_tail(®->lru_list, &dev_priv->mm.fence_list); - - if (obj->tiling_changed) { - ret = i915_gem_object_flush_fence(obj, pipelined); - if (ret) - return ret; - - if (!obj->fenced_gpu_access && !obj->last_fenced_seqno) - pipelined = NULL; - - if (pipelined) { - reg->setup_seqno = - i915_gem_next_request_seqno(pipelined); - obj->last_fenced_seqno = reg->setup_seqno; - obj->last_fenced_ring = pipelined; - } + if (obj->tiling_changed) goto update; - } - - if (!pipelined) { - if (reg->setup_seqno) { - if (!ring_passed_seqno(obj->last_fenced_ring, - reg->setup_seqno)) { - ret = i915_wait_request(obj->last_fenced_ring, - reg->setup_seqno); - if (ret) - return ret; - } - reg->setup_seqno = 0; - } - } else if (obj->last_fenced_ring && - obj->last_fenced_ring != pipelined) { - ret = i915_gem_object_flush_fence(obj, pipelined); + if (reg->setup_seqno && pipelined != reg->setup_ring) { + ret = i915_wait_request(reg->setup_ring, + reg->setup_seqno); if (ret) return ret; + + reg->setup_ring = 0; + reg->setup_seqno = 0; } + list_move_tail(®->lru_list, &dev_priv->mm.fence_list); return 0; } @@ -2703,47 +2689,43 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj, if (reg == NULL) return -ENOSPC; - ret = i915_gem_object_flush_fence(obj, pipelined); - if (ret) - return ret; - - if (reg->obj) { - struct drm_i915_gem_object *old = reg->obj; - + if ((old = reg->obj)) { drm_gem_object_reference(&old->base); if (old->tiling_mode) i915_gem_release_mmap(old); - ret = i915_gem_object_flush_fence(old, pipelined); - if (ret) { - drm_gem_object_unreference(&old->base); - return ret; - } + ret = i915_gem_object_flush_fence(old, NULL); //pipelined); + if (ret) + goto err; - if (old->last_fenced_seqno == 0 && obj->last_fenced_seqno == 0) - pipelined = NULL; + /* Mark the fence register as in-use if pipelined */ + reg->setup_ring = old->last_fenced_ring; + reg->setup_seqno = old->last_fenced_seqno; + } - old->fence_reg = I915_FENCE_REG_NONE; - old->last_fenced_ring = pipelined; - old->last_fenced_seqno = - pipelined ? i915_gem_next_request_seqno(pipelined) : 0; +update: + ret = i915_gem_object_flush_fence(obj, pipelined); + if (ret) + goto err; - drm_gem_object_unreference(&old->base); - } else if (obj->last_fenced_seqno == 0) - pipelined = NULL; + if (reg->setup_seqno && pipelined != reg->setup_ring) { + ret = i915_wait_request(reg->setup_ring, + reg->setup_seqno); + if (ret) + goto err; - reg->obj = obj; - list_move_tail(®->lru_list, &dev_priv->mm.fence_list); - obj->fence_reg = reg - dev_priv->fence_regs; - obj->last_fenced_ring = pipelined; + reg->setup_ring = 0; + reg->setup_seqno = 0; + } - reg->setup_seqno = - pipelined ? i915_gem_next_request_seqno(pipelined) : 0; - obj->last_fenced_seqno = reg->setup_seqno; + /* If we had a pipelined request, but there is no pending GPU access or + * update to a fence register for this memory region, we can write + * the new fence register immediately. + */ + if (obj->last_fenced_seqno == 0 && reg->setup_seqno == 0) + pipelined = NULL; -update: - obj->tiling_changed = false; regnum = reg - dev_priv->fence_regs; switch (INTEL_INFO(dev)->gen) { case 6: @@ -2760,7 +2742,31 @@ update: ret = i830_write_fence_reg(obj, pipelined, regnum); break; } + if (ret) + goto err; + + if (pipelined) { + reg->setup_seqno = i915_gem_next_request_seqno(pipelined); + reg->setup_ring = pipelined; + if (old) { + old->last_fenced_ring = pipelined; + old->last_fenced_seqno = reg->setup_seqno; + } + } + + if (old) { + old->fence_reg = I915_FENCE_REG_NONE; + drm_gem_object_unreference(&old->base); + } + + list_move_tail(®->lru_list, &dev_priv->mm.fence_list); + reg->obj = obj; + obj->fence_reg = regnum; + obj->tiling_changed = false; + return 0; +err: + drm_gem_object_unreference(&old->base); return ret; } @@ -2799,6 +2805,7 @@ i915_gem_clear_fence_reg(struct drm_device *dev, list_del_init(®->lru_list); reg->obj = NULL; + reg->setup_ring = 0; reg->setup_seqno = 0; }