From patchwork Tue Apr 12 20:31:57 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 702151 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 p3CKlo6U007075 for ; Tue, 12 Apr 2011 20:48:10 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AB7409EFEF for ; Tue, 12 Apr 2011 13:47:50 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id E13939EFE3 for ; Tue, 12 Apr 2011 13:33:01 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 12 Apr 2011 13:33:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.64,199,1301900400"; d="scan'208";a="627582345" Received: from unknown (HELO cantiga.alporthouse.com) ([10.255.16.172]) by orsmga002.jf.intel.com with ESMTP; 12 Apr 2011 13:33:00 -0700 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 12 Apr 2011 21:31:57 +0100 Message-Id: <1302640318-23165-30-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1302640318-23165-1-git-send-email-chris@chris-wilson.co.uk> References: <1302640318-23165-1-git-send-email-chris@chris-wilson.co.uk> Cc: Andy Whitcroft Subject: [Intel-gfx] [PATCH 29/30] 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]); Tue, 12 Apr 2011 20:48:10 +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 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 c837e10..d1fadb8 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 ca14a86..1949048 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1731,6 +1731,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); @@ -1896,7 +1898,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); } } @@ -2497,7 +2498,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; @@ -2508,17 +2509,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 @@ -2585,7 +2591,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; @@ -2602,7 +2608,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. @@ -2610,6 +2615,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. */ @@ -2620,6 +2629,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; @@ -2629,45 +2639,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; } @@ -2675,47 +2661,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: @@ -2732,7 +2714,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; } @@ -2771,6 +2777,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; }