From patchwork Fri Oct 8 13:35:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 12545361 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3B485C433EF for ; Fri, 8 Oct 2021 13:36:01 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F3CEE60F46 for ; Fri, 8 Oct 2021 13:36:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F3CEE60F46 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 413C46F501; Fri, 8 Oct 2021 13:35:49 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 990EB6E069; Fri, 8 Oct 2021 13:35:45 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10130"; a="287388443" X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="287388443" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:45 -0700 X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="522983703" Received: from lenovo-x280.ger.corp.intel.com (HELO thellstr-mobl1.intel.com) ([10.249.254.98]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:43 -0700 From: =?utf-8?q?Thomas_Hellstr=C3=B6m?= To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com, =?utf-8?q?Tho?= =?utf-8?q?mas_Hellstr=C3=B6m?= Date: Fri, 8 Oct 2021 15:35:25 +0200 Message-Id: <20211008133530.664509-2-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> References: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 1/6] drm/i915: Update dma_fence_work X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Move the release callback to after fence signaling to align with what's done for upcoming VM_BIND user-fence signaling. Finally call the work callback regardless of whether we have a fence error or not and update the existing callbacks accordingly. We will need this to intercept the error for failsafe migration. Signed-off-by: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 5 +++ drivers/gpu/drm/i915/i915_sw_fence_work.c | 36 ++++++++++----------- drivers/gpu/drm/i915/i915_sw_fence_work.h | 1 + drivers/gpu/drm/i915/i915_vma.c | 12 +++++-- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c index f0435c6feb68..2143ebaf5b6f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c @@ -28,6 +28,11 @@ static void clflush_work(struct dma_fence_work *base) { struct clflush *clflush = container_of(base, typeof(*clflush), base); + if (base->error) { + dma_fence_set_error(&base->dma, base->error); + return; + } + __do_clflush(clflush->obj); } diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c index 5b33ef23d54c..5b55cddafc9b 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c @@ -6,21 +6,24 @@ #include "i915_sw_fence_work.h" -static void fence_complete(struct dma_fence_work *f) +static void dma_fence_work_complete(struct dma_fence_work *f) { + dma_fence_signal(&f->dma); + if (f->ops->release) f->ops->release(f); - dma_fence_signal(&f->dma); + + dma_fence_put(&f->dma); } -static void fence_work(struct work_struct *work) +static void dma_fence_work_work(struct work_struct *work) { struct dma_fence_work *f = container_of(work, typeof(*f), work); - f->ops->work(f); + if (f->ops->work) + f->ops->work(f); - fence_complete(f); - dma_fence_put(&f->dma); + dma_fence_work_complete(f); } static int __i915_sw_fence_call @@ -31,17 +34,13 @@ fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) switch (state) { case FENCE_COMPLETE: if (fence->error) - dma_fence_set_error(&f->dma, fence->error); - - if (!f->dma.error) { - dma_fence_get(&f->dma); - if (test_bit(DMA_FENCE_WORK_IMM, &f->dma.flags)) - fence_work(&f->work); - else - queue_work(system_unbound_wq, &f->work); - } else { - fence_complete(f); - } + cmpxchg(&f->error, 0, fence->error); + + dma_fence_get(&f->dma); + if (test_bit(DMA_FENCE_WORK_IMM, &f->dma.flags)) + dma_fence_work_work(&f->work); + else + queue_work(system_unbound_wq, &f->work); break; case FENCE_FREE: @@ -84,10 +83,11 @@ void dma_fence_work_init(struct dma_fence_work *f, const struct dma_fence_work_ops *ops) { f->ops = ops; + f->error = 0; spin_lock_init(&f->lock); dma_fence_init(&f->dma, &fence_ops, &f->lock, 0, 0); i915_sw_fence_init(&f->chain, fence_notify); - INIT_WORK(&f->work, fence_work); + INIT_WORK(&f->work, dma_fence_work_work); } int dma_fence_work_chain(struct dma_fence_work *f, struct dma_fence *signal) diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.h b/drivers/gpu/drm/i915/i915_sw_fence_work.h index d56806918d13..caa59fb5252b 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.h +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.h @@ -24,6 +24,7 @@ struct dma_fence_work_ops { struct dma_fence_work { struct dma_fence dma; spinlock_t lock; + int error; struct i915_sw_fence chain; struct i915_sw_dma_fence_cb cb; diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 4b7fc4647e46..5123ac28ad9a 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -301,6 +301,11 @@ static void __vma_bind(struct dma_fence_work *work) struct i915_vma_work *vw = container_of(work, typeof(*vw), base); struct i915_vma *vma = vw->vma; + if (work->error) { + dma_fence_set_error(&work->dma, work->error); + return; + } + vma->ops->bind_vma(vw->vm, &vw->stash, vma, vw->cache_level, vw->flags); } @@ -333,7 +338,7 @@ struct i915_vma_work *i915_vma_work(void) return NULL; dma_fence_work_init(&vw->base, &bind_ops); - vw->base.dma.error = -EAGAIN; /* disable the worker by default */ + vw->base.error = -EAGAIN; /* disable the worker by default */ return vw; } @@ -416,6 +421,9 @@ int i915_vma_bind(struct i915_vma *vma, * part of the obj->resv->excl_fence as it only affects * execution and not content or object's backing store lifetime. */ + + work->base.error = 0; /* enable the queue_work() */ + prev = i915_active_set_exclusive(&vma->active, &work->base.dma); if (prev) { __i915_sw_fence_await_dma_fence(&work->base.chain, @@ -424,8 +432,6 @@ int i915_vma_bind(struct i915_vma *vma, dma_fence_put(prev); } - work->base.dma.error = 0; /* enable the queue_work() */ - if (vma->obj) { __i915_gem_object_pin_pages(vma->obj); work->pinned = i915_gem_object_get(vma->obj); From patchwork Fri Oct 8 13:35:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 12545363 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CAB8C433F5 for ; Fri, 8 Oct 2021 13:36:05 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E83E561042 for ; Fri, 8 Oct 2021 13:36:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E83E561042 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 44C796F509; Fri, 8 Oct 2021 13:35:51 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id E1B1F6F509; Fri, 8 Oct 2021 13:35:47 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10130"; a="287388450" X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="287388450" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:47 -0700 X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="522983712" Received: from lenovo-x280.ger.corp.intel.com (HELO thellstr-mobl1.intel.com) ([10.249.254.98]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:45 -0700 From: =?utf-8?q?Thomas_Hellstr=C3=B6m?= To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com, =?utf-8?q?Tho?= =?utf-8?q?mas_Hellstr=C3=B6m?= Date: Fri, 8 Oct 2021 15:35:26 +0200 Message-Id: <20211008133530.664509-3-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> References: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 2/6] drm/i915: Introduce refcounted sg-tables X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" As we start to introduce asynchronous failsafe object migration, where we update the object state and then submit asynchronous commands we need to record what memory resources are actually used by various part of the command stream. Initially for three purposes: 1) Error capture. 2) Asynchronous migration error recovery. 3) Asynchronous vma bind. At the time where these happens, the object state may have been updated to be several migrations ahead and object sg-tables discarded. In order to make it possible to keep sg-tables with memory resource information for these operations, introduce refcounted sg-tables that aren't freed until the last user is done with them. The alternative would be to reference information sitting on the corresponding ttm_resources which typically have the same lifetime as these refcountes sg_tables, but that leads to other awkward constructs: Due to the design direction chosen for ttm resource managers that would lead to diamond-style inheritance, the LMEM resources may sometimes be prematurely freed, and finally the subclassed struct ttm_resource would have to bleed into the asynchronous vma bind code. Signed-off-by: Thomas Hellström --- .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 159 +++++++++++------- drivers/gpu/drm/i915/i915_scatterlist.c | 62 +++++-- drivers/gpu/drm/i915/i915_scatterlist.h | 76 ++++++++- drivers/gpu/drm/i915/intel_region_ttm.c | 15 +- drivers/gpu/drm/i915/intel_region_ttm.h | 5 +- drivers/gpu/drm/i915/selftests/mock_region.c | 12 +- 7 files changed, 238 insertions(+), 94 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 7c3da4e3e737..d600cf7ceb35 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -485,6 +485,7 @@ struct drm_i915_gem_object { */ struct list_head region_link; + struct i915_refct_sgt *rsgt; struct sg_table *pages; void *mapping; @@ -538,7 +539,7 @@ struct drm_i915_gem_object { } mm; struct { - struct sg_table *cached_io_st; + struct i915_refct_sgt *cached_io_rsgt; struct i915_gem_object_page_iter get_io_page; struct drm_i915_gem_object *backup; bool created:1; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 74a1ffd0d7dd..4b4d7457bef9 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -34,7 +34,7 @@ * struct i915_ttm_tt - TTM page vector with additional private information * @ttm: The base TTM page vector. * @dev: The struct device used for dma mapping and unmapping. - * @cached_st: The cached scatter-gather table. + * @cached_rsgt: The cached scatter-gather table. * * Note that DMA may be going on right up to the point where the page- * vector is unpopulated in delayed destroy. Hence keep the @@ -45,7 +45,7 @@ struct i915_ttm_tt { struct ttm_tt ttm; struct device *dev; - struct sg_table *cached_st; + struct i915_refct_sgt cached_rsgt; }; static const struct ttm_place sys_placement_flags = { @@ -179,6 +179,21 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, placement->busy_placement = busy; } +static void i915_ttm_tt_release(struct kref *ref) +{ + struct i915_ttm_tt *i915_tt = + container_of(ref, typeof(*i915_tt), cached_rsgt.kref); + struct sg_table *st = &i915_tt->cached_rsgt.table; + + GEM_WARN_ON(st->sgl); + + kfree(i915_tt); +} + +static const struct i915_refct_sgt_ops tt_rsgt_ops = { + .release = i915_ttm_tt_release +}; + static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo, uint32_t page_flags) { @@ -203,6 +218,8 @@ static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo, return NULL; } + i915_refct_sgt_init_ops(&i915_tt->cached_rsgt, bo->base.size, + &tt_rsgt_ops); i915_tt->dev = obj->base.dev->dev; return &i915_tt->ttm; @@ -211,13 +228,13 @@ static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo, static void i915_ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm) { struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm); + struct sg_table *st = &i915_tt->cached_rsgt.table; + + GEM_WARN_ON(kref_read(&i915_tt->cached_rsgt.kref) != 1); - if (i915_tt->cached_st) { - dma_unmap_sgtable(i915_tt->dev, i915_tt->cached_st, - DMA_BIDIRECTIONAL, 0); - sg_free_table(i915_tt->cached_st); - kfree(i915_tt->cached_st); - i915_tt->cached_st = NULL; + if (st->sgl) { + dma_unmap_sgtable(i915_tt->dev, st, DMA_BIDIRECTIONAL, 0); + sg_free_table(st); } ttm_pool_free(&bdev->pool, ttm); } @@ -226,8 +243,10 @@ static void i915_ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *ttm) { struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm); + GEM_WARN_ON(kref_read(&i915_tt->cached_rsgt.kref) != 1); + ttm_tt_fini(ttm); - kfree(i915_tt); + i915_refct_sgt_put(&i915_tt->cached_rsgt); } static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo, @@ -261,12 +280,12 @@ static int i915_ttm_move_notify(struct ttm_buffer_object *bo) return 0; } -static void i915_ttm_free_cached_io_st(struct drm_i915_gem_object *obj) +static void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj) { struct radix_tree_iter iter; void __rcu **slot; - if (!obj->ttm.cached_io_st) + if (!obj->ttm.cached_io_rsgt) return; rcu_read_lock(); @@ -274,9 +293,8 @@ static void i915_ttm_free_cached_io_st(struct drm_i915_gem_object *obj) radix_tree_delete(&obj->ttm.get_io_page.radix, iter.index); rcu_read_unlock(); - sg_free_table(obj->ttm.cached_io_st); - kfree(obj->ttm.cached_io_st); - obj->ttm.cached_io_st = NULL; + i915_refct_sgt_put(obj->ttm.cached_io_rsgt); + obj->ttm.cached_io_rsgt = NULL; } static void @@ -347,7 +365,7 @@ static void i915_ttm_purge(struct drm_i915_gem_object *obj) obj->write_domain = 0; obj->read_domains = 0; i915_ttm_adjust_gem_after_move(obj); - i915_ttm_free_cached_io_st(obj); + i915_ttm_free_cached_io_rsgt(obj); obj->mm.madv = __I915_MADV_PURGED; } } @@ -358,7 +376,7 @@ static void i915_ttm_swap_notify(struct ttm_buffer_object *bo) int ret = i915_ttm_move_notify(bo); GEM_WARN_ON(ret); - GEM_WARN_ON(obj->ttm.cached_io_st); + GEM_WARN_ON(obj->ttm.cached_io_rsgt); if (!ret && obj->mm.madv != I915_MADV_WILLNEED) i915_ttm_purge(obj); } @@ -369,7 +387,7 @@ static void i915_ttm_delete_mem_notify(struct ttm_buffer_object *bo) if (likely(obj)) { __i915_gem_object_pages_fini(obj); - i915_ttm_free_cached_io_st(obj); + i915_ttm_free_cached_io_rsgt(obj); } } @@ -389,40 +407,35 @@ i915_ttm_region(struct ttm_device *bdev, int ttm_mem_type) ttm_mem_type - I915_PL_LMEM0); } -static struct sg_table *i915_ttm_tt_get_st(struct ttm_tt *ttm) +static struct i915_refct_sgt *i915_ttm_tt_get_st(struct ttm_tt *ttm) { struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm); struct sg_table *st; int ret; - if (i915_tt->cached_st) - return i915_tt->cached_st; - - st = kzalloc(sizeof(*st), GFP_KERNEL); - if (!st) - return ERR_PTR(-ENOMEM); + if (i915_tt->cached_rsgt.table.sgl) + return i915_refct_sgt_get(&i915_tt->cached_rsgt); + st = &i915_tt->cached_rsgt.table; ret = sg_alloc_table_from_pages_segment(st, ttm->pages, ttm->num_pages, 0, (unsigned long)ttm->num_pages << PAGE_SHIFT, i915_sg_segment_size(), GFP_KERNEL); if (ret) { - kfree(st); + st->sgl = NULL; return ERR_PTR(ret); } ret = dma_map_sgtable(i915_tt->dev, st, DMA_BIDIRECTIONAL, 0); if (ret) { sg_free_table(st); - kfree(st); return ERR_PTR(ret); } - i915_tt->cached_st = st; - return st; + return i915_refct_sgt_get(&i915_tt->cached_rsgt); } -static struct sg_table * +static struct i915_refct_sgt * i915_ttm_resource_get_st(struct drm_i915_gem_object *obj, struct ttm_resource *res) { @@ -436,7 +449,21 @@ i915_ttm_resource_get_st(struct drm_i915_gem_object *obj, * the resulting st. Might make sense for GGTT. */ GEM_WARN_ON(!cpu_maps_iomem(res)); - return intel_region_ttm_resource_to_st(obj->mm.region, res); + if (bo->resource == res) { + if (!obj->ttm.cached_io_rsgt) { + struct i915_refct_sgt *rsgt; + + rsgt = intel_region_ttm_resource_to_rsgt(obj->mm.region, + res); + if (IS_ERR(rsgt)) + return rsgt; + + obj->ttm.cached_io_rsgt = rsgt; + } + return i915_refct_sgt_get(obj->ttm.cached_io_rsgt); + } + + return intel_region_ttm_resource_to_rsgt(obj->mm.region, res); } static int i915_ttm_accel_move(struct ttm_buffer_object *bo, @@ -447,10 +474,7 @@ static int i915_ttm_accel_move(struct ttm_buffer_object *bo, { struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915), bdev); - struct ttm_resource_manager *src_man = - ttm_manager_type(bo->bdev, bo->resource->mem_type); struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo); - struct sg_table *src_st; struct i915_request *rq; struct ttm_tt *src_ttm = bo->ttm; enum i915_cache_level src_level, dst_level; @@ -476,17 +500,22 @@ static int i915_ttm_accel_move(struct ttm_buffer_object *bo, } intel_engine_pm_put(i915->gt.migrate.context->engine); } else { - src_st = src_man->use_tt ? i915_ttm_tt_get_st(src_ttm) : - obj->ttm.cached_io_st; + struct i915_refct_sgt *src_rsgt = + i915_ttm_resource_get_st(obj, bo->resource); + + if (IS_ERR(src_rsgt)) + return PTR_ERR(src_rsgt); src_level = i915_ttm_cache_level(i915, bo->resource, src_ttm); intel_engine_pm_get(i915->gt.migrate.context->engine); ret = intel_context_migrate_copy(i915->gt.migrate.context, - NULL, src_st->sgl, src_level, + NULL, src_rsgt->table.sgl, + src_level, gpu_binds_iomem(bo->resource), dst_st->sgl, dst_level, gpu_binds_iomem(dst_mem), &rq); + i915_refct_sgt_put(src_rsgt); if (!ret && rq) { i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT); i915_request_put(rq); @@ -500,13 +529,14 @@ static int i915_ttm_accel_move(struct ttm_buffer_object *bo, static void __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, struct ttm_resource *dst_mem, struct ttm_tt *dst_ttm, - struct sg_table *dst_st, + struct i915_refct_sgt *dst_rsgt, bool allow_accel) { int ret = -EINVAL; if (allow_accel) - ret = i915_ttm_accel_move(bo, clear, dst_mem, dst_ttm, dst_st); + ret = i915_ttm_accel_move(bo, clear, dst_mem, dst_ttm, + &dst_rsgt->table); if (ret) { struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo); struct intel_memory_region *dst_reg, *src_reg; @@ -523,12 +553,13 @@ static void __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, dst_iter = !cpu_maps_iomem(dst_mem) ? ttm_kmap_iter_tt_init(&_dst_iter.tt, dst_ttm) : ttm_kmap_iter_iomap_init(&_dst_iter.io, &dst_reg->iomap, - dst_st, dst_reg->region.start); + &dst_rsgt->table, + dst_reg->region.start); src_iter = !cpu_maps_iomem(bo->resource) ? ttm_kmap_iter_tt_init(&_src_iter.tt, bo->ttm) : ttm_kmap_iter_iomap_init(&_src_iter.io, &src_reg->iomap, - obj->ttm.cached_io_st, + &obj->ttm.cached_io_rsgt->table, src_reg->region.start); ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter); @@ -544,7 +575,7 @@ static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource_manager *dst_man = ttm_manager_type(bo->bdev, dst_mem->mem_type); struct ttm_tt *ttm = bo->ttm; - struct sg_table *dst_st; + struct i915_refct_sgt *dst_rsgt; bool clear; int ret; @@ -570,22 +601,24 @@ static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, return ret; } - dst_st = i915_ttm_resource_get_st(obj, dst_mem); - if (IS_ERR(dst_st)) - return PTR_ERR(dst_st); + dst_rsgt = i915_ttm_resource_get_st(obj, dst_mem); + if (IS_ERR(dst_rsgt)) + return PTR_ERR(dst_rsgt); clear = !cpu_maps_iomem(bo->resource) && (!ttm || !ttm_tt_is_populated(ttm)); if (!(clear && ttm && !(ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC))) - __i915_ttm_move(bo, clear, dst_mem, bo->ttm, dst_st, true); + __i915_ttm_move(bo, clear, dst_mem, bo->ttm, dst_rsgt, true); ttm_bo_move_sync_cleanup(bo, dst_mem); i915_ttm_adjust_domains_after_move(obj); - i915_ttm_free_cached_io_st(obj); + i915_ttm_free_cached_io_rsgt(obj); if (gpu_binds_iomem(dst_mem) || cpu_maps_iomem(dst_mem)) { - obj->ttm.cached_io_st = dst_st; - obj->ttm.get_io_page.sg_pos = dst_st->sgl; + obj->ttm.cached_io_rsgt = dst_rsgt; + obj->ttm.get_io_page.sg_pos = dst_rsgt->table.sgl; obj->ttm.get_io_page.sg_idx = 0; + } else { + i915_refct_sgt_put(dst_rsgt); } i915_ttm_adjust_gem_after_move(obj); @@ -649,7 +682,6 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj, .interruptible = true, .no_wait_gpu = false, }; - struct sg_table *st; int real_num_busy; int ret; @@ -687,12 +719,16 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj, } if (!i915_gem_object_has_pages(obj)) { - /* Object either has a page vector or is an iomem object */ - st = bo->ttm ? i915_ttm_tt_get_st(bo->ttm) : obj->ttm.cached_io_st; - if (IS_ERR(st)) - return PTR_ERR(st); + struct i915_refct_sgt *rsgt = + i915_ttm_resource_get_st(obj, bo->resource); + + if (IS_ERR(rsgt)) + return PTR_ERR(rsgt); - __i915_gem_object_set_pages(obj, st, i915_sg_dma_sizes(st->sgl)); + GEM_BUG_ON(obj->mm.rsgt); + obj->mm.rsgt = rsgt; + __i915_gem_object_set_pages(obj, &rsgt->table, + i915_sg_dma_sizes(rsgt->table.sgl)); } return ret; @@ -766,6 +802,11 @@ static void i915_ttm_put_pages(struct drm_i915_gem_object *obj, * and shrinkers will move it out if needed. */ + if (obj->mm.rsgt) { + i915_refct_sgt_put(obj->mm.rsgt); + obj->mm.rsgt = NULL; + } + i915_ttm_adjust_lru(obj); } @@ -1023,7 +1064,7 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst, struct ttm_operation_ctx ctx = { .interruptible = intr, }; - struct sg_table *dst_st; + struct i915_refct_sgt *dst_rsgt; int ret; assert_object_held(dst); @@ -1038,11 +1079,11 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst, if (ret) return ret; - dst_st = gpu_binds_iomem(dst_bo->resource) ? - dst->ttm.cached_io_st : i915_ttm_tt_get_st(dst_bo->ttm); - + dst_rsgt = i915_ttm_resource_get_st(dst, dst_bo->resource); __i915_ttm_move(src_bo, false, dst_bo->resource, dst_bo->ttm, - dst_st, allow_accel); + dst_rsgt, allow_accel); + + i915_refct_sgt_put(dst_rsgt); return 0; } diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c b/drivers/gpu/drm/i915/i915_scatterlist.c index 4a6712dca838..8a510ee5d1ad 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.c +++ b/drivers/gpu/drm/i915/i915_scatterlist.c @@ -41,8 +41,32 @@ bool i915_sg_trim(struct sg_table *orig_st) return true; } +static void i915_refct_sgt_release(struct kref *ref) +{ + struct i915_refct_sgt *rsgt = + container_of(ref, typeof(*rsgt), kref); + + sg_free_table(&rsgt->table); + kfree(rsgt); +} + +static const struct i915_refct_sgt_ops rsgt_ops = { + .release = i915_refct_sgt_release +}; + +/** + * i915_refct_sgt_init - Initialize a struct i915_refct_sgt with default ops + * @rsgt: The struct i915_refct_sgt to initialize. + * size: The size of the underlying memory buffer. + */ +void i915_refct_sgt_init(struct i915_refct_sgt *rsgt, size_t size) +{ + i915_refct_sgt_init_ops(rsgt, size, &rsgt_ops); +} + /** - * i915_sg_from_mm_node - Create an sg_table from a struct drm_mm_node + * i915_rsgt_from_mm_node - Create a refcounted sg_table from a struct + * drm_mm_node * @node: The drm_mm_node. * @region_start: An offset to add to the dma addresses of the sg list. * @@ -50,25 +74,28 @@ bool i915_sg_trim(struct sg_table *orig_st) * taking a maximum segment length into account, splitting into segments * if necessary. * - * Return: A pointer to a kmalloced struct sg_table on success, negative + * Return: A pointer to a kmalloced struct i915_refct_sgt on success, negative * error code cast to an error pointer on failure. */ -struct sg_table *i915_sg_from_mm_node(const struct drm_mm_node *node, - u64 region_start) +struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node, + u64 region_start) { const u64 max_segment = SZ_1G; /* Do we have a limit on this? */ u64 segment_pages = max_segment >> PAGE_SHIFT; u64 block_size, offset, prev_end; + struct i915_refct_sgt *rsgt; struct sg_table *st; struct scatterlist *sg; - st = kmalloc(sizeof(*st), GFP_KERNEL); - if (!st) + rsgt = kmalloc(sizeof(*rsgt), GFP_KERNEL); + if (!rsgt) return ERR_PTR(-ENOMEM); + i915_refct_sgt_init(rsgt, node->size << PAGE_SHIFT); + st = &rsgt->table; if (sg_alloc_table(st, DIV_ROUND_UP(node->size, segment_pages), GFP_KERNEL)) { - kfree(st); + i915_refct_sgt_put(rsgt); return ERR_PTR(-ENOMEM); } @@ -104,11 +131,11 @@ struct sg_table *i915_sg_from_mm_node(const struct drm_mm_node *node, sg_mark_end(sg); i915_sg_trim(st); - return st; + return rsgt; } /** - * i915_sg_from_buddy_resource - Create an sg_table from a struct + * i915_rsgt_from_buddy_resource - Create a refcounted sg_table from a struct * i915_buddy_block list * @res: The struct i915_ttm_buddy_resource. * @region_start: An offset to add to the dma addresses of the sg list. @@ -117,11 +144,11 @@ struct sg_table *i915_sg_from_mm_node(const struct drm_mm_node *node, * taking a maximum segment length into account, splitting into segments * if necessary. * - * Return: A pointer to a kmalloced struct sg_table on success, negative + * Return: A pointer to a kmalloced struct i915_refct_sgts on success, negative * error code cast to an error pointer on failure. */ -struct sg_table *i915_sg_from_buddy_resource(struct ttm_resource *res, - u64 region_start) +struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res, + u64 region_start) { struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res); const u64 size = res->num_pages << PAGE_SHIFT; @@ -129,18 +156,21 @@ struct sg_table *i915_sg_from_buddy_resource(struct ttm_resource *res, struct i915_buddy_mm *mm = bman_res->mm; struct list_head *blocks = &bman_res->blocks; struct i915_buddy_block *block; + struct i915_refct_sgt *rsgt; struct scatterlist *sg; struct sg_table *st; resource_size_t prev_end; GEM_BUG_ON(list_empty(blocks)); - st = kmalloc(sizeof(*st), GFP_KERNEL); - if (!st) + rsgt = kmalloc(sizeof(*rsgt), GFP_KERNEL); + if (!rsgt) return ERR_PTR(-ENOMEM); + i915_refct_sgt_init(rsgt, size); + st = &rsgt->table; if (sg_alloc_table(st, res->num_pages, GFP_KERNEL)) { - kfree(st); + i915_refct_sgt_put(rsgt); return ERR_PTR(-ENOMEM); } @@ -181,7 +211,7 @@ struct sg_table *i915_sg_from_buddy_resource(struct ttm_resource *res, sg_mark_end(sg); i915_sg_trim(st); - return st; + return rsgt; } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h index b8bd5925b03f..321fd4a9f777 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -144,10 +144,78 @@ static inline unsigned int i915_sg_segment_size(void) bool i915_sg_trim(struct sg_table *orig_st); -struct sg_table *i915_sg_from_mm_node(const struct drm_mm_node *node, - u64 region_start); +/** + * struct i915_refct_sgt_ops - Operations structure for struct i915_refct_sgt + */ +struct i915_refct_sgt_ops { + /** + * release() - Free the memory of the struct i915_refct_sgt + * @ref: struct kref that is embedded in the struct i915_refct_sgt + */ + void (*release)(struct kref *ref); +}; + +/** + * struct i915_refct_sgt - A refcounted scatter-gather table + * @kref: struct kref for refcounting + * @table: struct sg_table holding the scatter-gather table itself. Note that + * @table->sgl = NULL can be used to determine whether a scatter-gather table + * is present or not. + * @size: The size in bytes of the underlying memory buffer + * @ops: The operations structure. + */ +struct i915_refct_sgt { + struct kref kref; + struct sg_table table; + size_t size; + const struct i915_refct_sgt_ops *ops; +}; + +/** + * i915_refct_sgt_put - Put a refcounted sg-table + * @rsgt the struct i915_refct_sgt to put. + */ +static inline void i915_refct_sgt_put(struct i915_refct_sgt *rsgt) +{ + if (rsgt) + kref_put(&rsgt->kref, rsgt->ops->release); +} + +/** + * i915_refct_sgt_get - Get a refcounted sg-table + * @rsgt the struct i915_refct_sgt to get. + */ +static inline struct i915_refct_sgt * +i915_refct_sgt_get(struct i915_refct_sgt *rsgt) +{ + kref_get(&rsgt->kref); + return rsgt; +} + +/** + * i915_refct_sgt_init_ops - Initialize a refcounted sg-list with a custom + * operations structure + * @rsgt The struct i915_refct_sgt to initialize. + * @size: Size in bytes of the underlying memory buffer. + * @ops: A customized operations structure in case the refcounted sg-list + * is embedded into another structure. + */ +static inline void i915_refct_sgt_init_ops(struct i915_refct_sgt *rsgt, + size_t size, + const struct i915_refct_sgt_ops *ops) +{ + kref_init(&rsgt->kref); + rsgt->table.sgl = NULL; + rsgt->size = size; + rsgt->ops = ops; +} + +void i915_refct_sgt_init(struct i915_refct_sgt *rsgt, size_t size); + +struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node, + u64 region_start); -struct sg_table *i915_sg_from_buddy_resource(struct ttm_resource *res, - u64 region_start); +struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res, + u64 region_start); #endif diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c index 98c7339bf8ba..2e901a27e259 100644 --- a/drivers/gpu/drm/i915/intel_region_ttm.c +++ b/drivers/gpu/drm/i915/intel_region_ttm.c @@ -115,8 +115,8 @@ void intel_region_ttm_fini(struct intel_memory_region *mem) } /** - * intel_region_ttm_resource_to_st - Convert an opaque TTM resource manager resource - * to an sg_table. + * intel_region_ttm_resource_to_rsgt - + * Convert an opaque TTM resource manager resource to a refcounted sg_table. * @mem: The memory region. * @res: The resource manager resource obtained from the TTM resource manager. * @@ -126,17 +126,18 @@ void intel_region_ttm_fini(struct intel_memory_region *mem) * * Return: A malloced sg_table on success, an error pointer on failure. */ -struct sg_table *intel_region_ttm_resource_to_st(struct intel_memory_region *mem, - struct ttm_resource *res) +struct i915_refct_sgt * +intel_region_ttm_resource_to_rsgt(struct intel_memory_region *mem, + struct ttm_resource *res) { if (mem->is_range_manager) { struct ttm_range_mgr_node *range_node = to_ttm_range_mgr_node(res); - return i915_sg_from_mm_node(&range_node->mm_nodes[0], - mem->region.start); + return i915_rsgt_from_mm_node(&range_node->mm_nodes[0], + mem->region.start); } else { - return i915_sg_from_buddy_resource(res, mem->region.start); + return i915_rsgt_from_buddy_resource(res, mem->region.start); } } diff --git a/drivers/gpu/drm/i915/intel_region_ttm.h b/drivers/gpu/drm/i915/intel_region_ttm.h index 6f44075920f2..7bbe2b46b504 100644 --- a/drivers/gpu/drm/i915/intel_region_ttm.h +++ b/drivers/gpu/drm/i915/intel_region_ttm.h @@ -22,8 +22,9 @@ int intel_region_ttm_init(struct intel_memory_region *mem); void intel_region_ttm_fini(struct intel_memory_region *mem); -struct sg_table *intel_region_ttm_resource_to_st(struct intel_memory_region *mem, - struct ttm_resource *res); +struct i915_refct_sgt * +intel_region_ttm_resource_to_rsgt(struct intel_memory_region *mem, + struct ttm_resource *res); void intel_region_ttm_resource_free(struct intel_memory_region *mem, struct ttm_resource *res); diff --git a/drivers/gpu/drm/i915/selftests/mock_region.c b/drivers/gpu/drm/i915/selftests/mock_region.c index efa86dffe3c6..2752b5b98f60 100644 --- a/drivers/gpu/drm/i915/selftests/mock_region.c +++ b/drivers/gpu/drm/i915/selftests/mock_region.c @@ -17,9 +17,9 @@ static void mock_region_put_pages(struct drm_i915_gem_object *obj, struct sg_table *pages) { + i915_refct_sgt_put(obj->mm.rsgt); + obj->mm.rsgt = NULL; intel_region_ttm_resource_free(obj->mm.region, obj->mm.res); - sg_free_table(pages); - kfree(pages); } static int mock_region_get_pages(struct drm_i915_gem_object *obj) @@ -38,12 +38,14 @@ static int mock_region_get_pages(struct drm_i915_gem_object *obj) if (IS_ERR(obj->mm.res)) return PTR_ERR(obj->mm.res); - pages = intel_region_ttm_resource_to_st(obj->mm.region, obj->mm.res); - if (IS_ERR(pages)) { - err = PTR_ERR(pages); + obj->mm.rsgt = intel_region_ttm_resource_to_rsgt(obj->mm.region, + obj->mm.res); + if (IS_ERR(obj->mm.rsgt)) { + err = PTR_ERR(obj->mm.rsgt); goto err_free_resource; } + pages = &obj->mm.rsgt->table; __i915_gem_object_set_pages(obj, pages, i915_sg_dma_sizes(pages->sgl)); return 0; From patchwork Fri Oct 8 13:35:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 12545365 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 59345C433EF for ; Fri, 8 Oct 2021 13:36:08 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3001261042 for ; Fri, 8 Oct 2021 13:36:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3001261042 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 30EF66F50C; Fri, 8 Oct 2021 13:35:53 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id AF2D16F509; Fri, 8 Oct 2021 13:35:49 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10130"; a="287388457" X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="287388457" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:49 -0700 X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="522983721" Received: from lenovo-x280.ger.corp.intel.com (HELO thellstr-mobl1.intel.com) ([10.249.254.98]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:47 -0700 From: =?utf-8?q?Thomas_Hellstr=C3=B6m?= To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com, =?utf-8?q?Tho?= =?utf-8?q?mas_Hellstr=C3=B6m?= Date: Fri, 8 Oct 2021 15:35:27 +0200 Message-Id: <20211008133530.664509-4-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> References: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 3/6] drm/i915/ttm: Failsafe migration blits X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" If the initial fill blit or copy blit of an object fails, the old content of the data might be exposed and read as soon as either CPU- or GPU PTEs are set up to point at the pages. Intercept the blit fence with an async dma_fence_work that checks the blit fence for errors and if there are errors performs an async cpu blit instead. If there is a failure to allocate the async dma_fence_work, allocate it on the stack and sync wait for the blit to complete. Add selftests that simulate gpu blit failures and failure to allocate the async dma_fence_work. Signed-off-by: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 268 ++++++++++++++---- drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 4 + .../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +- 3 files changed, 240 insertions(+), 56 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 4b4d7457bef9..79d4d50aa4e5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -7,6 +7,7 @@ #include #include "i915_drv.h" +#include "i915_sw_fence_work.h" #include "intel_memory_region.h" #include "intel_region_ttm.h" @@ -25,6 +26,18 @@ #define I915_TTM_PRIO_NO_PAGES 1 #define I915_TTM_PRIO_HAS_PAGES 2 +I915_SELFTEST_DECLARE(static bool fail_gpu_migration;) +I915_SELFTEST_DECLARE(static bool fail_work_allocation;) + +#ifdef CONFIG_DRM_I915_SELFTEST +void i915_ttm_migrate_set_failure_modes(bool gpu_migration, + bool work_allocation) +{ + fail_gpu_migration = gpu_migration; + fail_work_allocation = work_allocation; +} +#endif + /* * Size of struct ttm_place vector in on-stack struct ttm_placement allocs */ @@ -466,11 +479,11 @@ i915_ttm_resource_get_st(struct drm_i915_gem_object *obj, return intel_region_ttm_resource_to_rsgt(obj->mm.region, res); } -static int i915_ttm_accel_move(struct ttm_buffer_object *bo, - bool clear, - struct ttm_resource *dst_mem, - struct ttm_tt *dst_ttm, - struct sg_table *dst_st) +static struct dma_fence *i915_ttm_accel_move(struct ttm_buffer_object *bo, + bool clear, + struct ttm_resource *dst_mem, + struct ttm_tt *dst_ttm, + struct sg_table *dst_st) { struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915), bdev); @@ -481,30 +494,29 @@ static int i915_ttm_accel_move(struct ttm_buffer_object *bo, int ret; if (!i915->gt.migrate.context || intel_gt_is_wedged(&i915->gt)) - return -EINVAL; + return ERR_PTR(-EINVAL); + + /* With fail_gpu_migration, we always perform a GPU clear. */ + if (I915_SELFTEST_ONLY(fail_gpu_migration)) + clear = true; dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm); if (clear) { - if (bo->type == ttm_bo_type_kernel) - return -EINVAL; + if (bo->type == ttm_bo_type_kernel && + !I915_SELFTEST_ONLY(fail_gpu_migration)) + return ERR_PTR(-EINVAL); intel_engine_pm_get(i915->gt.migrate.context->engine); ret = intel_context_migrate_clear(i915->gt.migrate.context, NULL, dst_st->sgl, dst_level, gpu_binds_iomem(dst_mem), 0, &rq); - - if (!ret && rq) { - i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT); - i915_request_put(rq); - } - intel_engine_pm_put(i915->gt.migrate.context->engine); } else { struct i915_refct_sgt *src_rsgt = i915_ttm_resource_get_st(obj, bo->resource); if (IS_ERR(src_rsgt)) - return PTR_ERR(src_rsgt); + return ERR_CAST(src_rsgt); src_level = i915_ttm_cache_level(i915, bo->resource, src_ttm); intel_engine_pm_get(i915->gt.migrate.context->engine); @@ -515,55 +527,201 @@ static int i915_ttm_accel_move(struct ttm_buffer_object *bo, dst_st->sgl, dst_level, gpu_binds_iomem(dst_mem), &rq); + i915_refct_sgt_put(src_rsgt); - if (!ret && rq) { - i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT); - i915_request_put(rq); - } - intel_engine_pm_put(i915->gt.migrate.context->engine); } - return ret; + intel_engine_pm_put(i915->gt.migrate.context->engine); + + if (ret && rq) { + i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT); + i915_request_put(rq); + } + + return ret ? ERR_PTR(ret) : &rq->fence; +} + +/** + * struct i915_ttm_memcpy_work - memcpy work item under a dma-fence + * @base: The struct dma_fence_work we subclass. + * @_dst_iter: Storage space for the destination kmap iterator. + * @_src_iter: Storage space for the source kmap iterator. + * @dst_iter: Pointer to the destination kmap iterator. + * @src_iter: Pointer to the source kmap iterator. + * @clear: Whether to clear instead of copy. + * @num_pages: Number of pages in the copy. + * @src_rsgt: Refcounted scatter-gather list of source memory. + * @dst_rsgt: Refcounted scatter-gather list of destination memory. + */ +struct i915_ttm_memcpy_work { + struct dma_fence_work base; + union { + struct ttm_kmap_iter_tt tt; + struct ttm_kmap_iter_iomap io; + } _dst_iter, + _src_iter; + struct ttm_kmap_iter *dst_iter; + struct ttm_kmap_iter *src_iter; + unsigned long num_pages; + bool clear; + struct i915_refct_sgt *src_rsgt; + struct i915_refct_sgt *dst_rsgt; +}; + +static void __memcpy_work(struct dma_fence_work *work) +{ + struct i915_ttm_memcpy_work *copy_work = + container_of(work, typeof(*copy_work), base); + + if (I915_SELFTEST_ONLY(fail_gpu_migration)) + cmpxchg(&work->error, 0, -EINVAL); + + /* If there was an error in the gpu copy operation, run memcpy. */ + if (work->error) + ttm_move_memcpy(copy_work->clear, copy_work->num_pages, + copy_work->dst_iter, copy_work->src_iter); + + /* + * Can't signal before we unref the rsgts, because then + * ttms might be unpopulated before we unref these and we'll hit + * a GEM_WARN_ON() in i915_ttm_tt_unpopulate. Not a real problem, + * but good to keep the GEM_WARN_ON to check that we don't leak rsgts. + */ + i915_refct_sgt_put(copy_work->src_rsgt); + i915_refct_sgt_put(copy_work->dst_rsgt); +} + +static const struct dma_fence_work_ops i915_ttm_memcpy_ops = { + .work = __memcpy_work, +}; + +static void i915_ttm_memcpy_work_init(struct i915_ttm_memcpy_work *copy_work, + struct ttm_buffer_object *bo, bool clear, + struct ttm_resource *dst_mem, + struct ttm_tt *dst_ttm, + struct i915_refct_sgt *dst_rsgt) +{ + struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo); + struct intel_memory_region *dst_reg, *src_reg; + + dst_reg = i915_ttm_region(bo->bdev, dst_mem->mem_type); + src_reg = i915_ttm_region(bo->bdev, bo->resource->mem_type); + GEM_BUG_ON(!dst_reg || !src_reg); + + /* + * We could consider populating only parts of this structure + * (like avoiding the iterators) until it's actually + * determined that we need it. But initializing the iterators + * shouldn't be that costly really. + */ + + copy_work->dst_iter = !cpu_maps_iomem(dst_mem) ? + ttm_kmap_iter_tt_init(©_work->_dst_iter.tt, dst_ttm) : + ttm_kmap_iter_iomap_init(©_work->_dst_iter.io, &dst_reg->iomap, + &dst_rsgt->table, dst_reg->region.start); + + copy_work->src_iter = !cpu_maps_iomem(bo->resource) ? + ttm_kmap_iter_tt_init(©_work->_src_iter.tt, bo->ttm) : + ttm_kmap_iter_iomap_init(©_work->_src_iter.io, &src_reg->iomap, + &obj->ttm.cached_io_rsgt->table, + src_reg->region.start); + copy_work->clear = clear; + copy_work->num_pages = bo->base.size >> PAGE_SHIFT; + + copy_work->dst_rsgt = i915_refct_sgt_get(dst_rsgt); + copy_work->src_rsgt = clear ? NULL : + i915_ttm_resource_get_st(obj, bo->resource); } -static void __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, - struct ttm_resource *dst_mem, - struct ttm_tt *dst_ttm, - struct i915_refct_sgt *dst_rsgt, - bool allow_accel) +/* + * This is only used as a last fallback if the copy_work + * memory allocation fails, prohibiting async moves. + */ +static void __i915_ttm_move_fallback(struct ttm_buffer_object *bo, bool clear, + struct ttm_resource *dst_mem, + struct ttm_tt *dst_ttm, + struct i915_refct_sgt *dst_rsgt, + bool allow_accel) { int ret = -EINVAL; - if (allow_accel) - ret = i915_ttm_accel_move(bo, clear, dst_mem, dst_ttm, - &dst_rsgt->table); - if (ret) { - struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo); - struct intel_memory_region *dst_reg, *src_reg; - union { - struct ttm_kmap_iter_tt tt; - struct ttm_kmap_iter_iomap io; - } _dst_iter, _src_iter; - struct ttm_kmap_iter *dst_iter, *src_iter; - - dst_reg = i915_ttm_region(bo->bdev, dst_mem->mem_type); - src_reg = i915_ttm_region(bo->bdev, bo->resource->mem_type); - GEM_BUG_ON(!dst_reg || !src_reg); - - dst_iter = !cpu_maps_iomem(dst_mem) ? - ttm_kmap_iter_tt_init(&_dst_iter.tt, dst_ttm) : - ttm_kmap_iter_iomap_init(&_dst_iter.io, &dst_reg->iomap, - &dst_rsgt->table, - dst_reg->region.start); - - src_iter = !cpu_maps_iomem(bo->resource) ? - ttm_kmap_iter_tt_init(&_src_iter.tt, bo->ttm) : - ttm_kmap_iter_iomap_init(&_src_iter.io, &src_reg->iomap, - &obj->ttm.cached_io_rsgt->table, - src_reg->region.start); - - ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter); + if (allow_accel) { + struct dma_fence *fence; + + fence = i915_ttm_accel_move(bo, clear, dst_mem, dst_ttm, + &dst_rsgt->table); + if (IS_ERR(fence)) { + ret = PTR_ERR(fence); + } else { + ret = dma_fence_wait(fence, false); + if (!ret) + ret = fence->error; + dma_fence_put(fence); + } + } + + if (ret || I915_SELFTEST_ONLY(fail_gpu_migration)) { + struct i915_ttm_memcpy_work copy_work; + + i915_ttm_memcpy_work_init(©_work, bo, clear, dst_mem, + dst_ttm, dst_rsgt); + + /* Trigger a copy by setting an error value */ + copy_work.base.dma.error = -EINVAL; + __memcpy_work(©_work.base); + } +} + +static int __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, + struct ttm_resource *dst_mem, struct ttm_tt *dst_ttm, + struct i915_refct_sgt *dst_rsgt, bool allow_accel) +{ + struct i915_ttm_memcpy_work *copy_work; + struct dma_fence *fence; + int ret; + + if (!I915_SELFTEST_ONLY(fail_work_allocation)) + copy_work = kzalloc(sizeof(*copy_work), GFP_KERNEL); + else + copy_work = NULL; + + if (!copy_work) { + /* Don't fail with -ENOMEM. Move sync instead. */ + __i915_ttm_move_fallback(bo, clear, dst_mem, dst_ttm, dst_rsgt, + allow_accel); + return 0; + } + + dma_fence_work_init(©_work->base, &i915_ttm_memcpy_ops); + if (allow_accel) { + fence = i915_ttm_accel_move(bo, clear, dst_mem, dst_ttm, + &dst_rsgt->table); + if (IS_ERR(fence)) { + i915_sw_fence_set_error_once(©_work->base.chain, + PTR_ERR(fence)); + } else { + ret = dma_fence_work_chain(©_work->base, fence); + dma_fence_put(fence); + GEM_WARN_ON(ret < 0); + } + } else { + i915_sw_fence_set_error_once(©_work->base.chain, -EINVAL); } + + /* Setup async memcpy */ + i915_ttm_memcpy_work_init(copy_work, bo, clear, dst_mem, dst_ttm, + dst_rsgt); + fence = dma_fence_get(©_work->base.dma); + dma_fence_work_commit_imm(©_work->base); + + /* + * We're synchronizing here for now. For async moves, return the + * fence. + */ + dma_fence_wait(fence, false); + dma_fence_put(fence); + + return ret; } static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.h b/drivers/gpu/drm/i915/gem/i915_gem_ttm.h index 0b7291dd897c..c5bf8863446d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.h @@ -51,6 +51,10 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst, struct drm_i915_gem_object *src, bool allow_accel, bool intr); +I915_SELFTEST_DECLARE +(void i915_ttm_migrate_set_failure_modes(bool gpu_migration, + bool work_allocation);) + /* Internal I915 TTM declarations and definitions below. */ #define I915_PL_LMEM0 TTM_PL_PRIV diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c index 28a700f08b49..a2122bdcc1cb 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c @@ -4,6 +4,7 @@ */ #include "gt/intel_migrate.h" +#include "gem/i915_gem_ttm.h" static int igt_fill_check_buffer(struct drm_i915_gem_object *obj, bool fill) @@ -227,13 +228,34 @@ static int igt_lmem_pages_migrate(void *arg) return err; } +static int igt_lmem_pages_failsafe_migrate(void *arg) +{ + int fail_gpu, fail_alloc, ret; + + for (fail_gpu = 0; fail_gpu < 2; ++fail_gpu) { + for (fail_alloc = 0; fail_alloc < 2; ++fail_alloc) { + pr_info("Simulated failure modes: gpu: %d, alloc: %d\n", + fail_gpu, fail_alloc); + i915_ttm_migrate_set_failure_modes(fail_gpu, + fail_alloc); + ret = igt_lmem_pages_migrate(arg); + if (ret) + goto out_err; + } + } + +out_err: + i915_ttm_migrate_set_failure_modes(false, false); + return ret; +} + int i915_gem_migrate_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { SUBTEST(igt_smem_create_migrate), SUBTEST(igt_lmem_create_migrate), SUBTEST(igt_same_create_migrate), - SUBTEST(igt_lmem_pages_migrate), + SUBTEST(igt_lmem_pages_failsafe_migrate), }; if (!HAS_LMEM(i915)) From patchwork Fri Oct 8 13:35:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 12545367 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5A268C433F5 for ; Fri, 8 Oct 2021 13:36:09 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 27D0B61042 for ; Fri, 8 Oct 2021 13:36:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 27D0B61042 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C6C0F6F511; Fri, 8 Oct 2021 13:35:53 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7BFE06F50C; Fri, 8 Oct 2021 13:35:51 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10130"; a="287388463" X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="287388463" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:51 -0700 X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="522983724" Received: from lenovo-x280.ger.corp.intel.com (HELO thellstr-mobl1.intel.com) ([10.249.254.98]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:49 -0700 From: =?utf-8?q?Thomas_Hellstr=C3=B6m?= To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com, =?utf-8?q?Tho?= =?utf-8?q?mas_Hellstr=C3=B6m?= Date: Fri, 8 Oct 2021 15:35:28 +0200 Message-Id: <20211008133530.664509-5-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> References: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 4/6] drm/i915: Add a struct dma_fence_work timeline X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" The TTM managers and, possibly, the gtt address space managers will need to be able to order fences for async operation. Using dma_fence_is_later() for this will require that the fences we hand them are from a single fence context and ordered. Introduce a struct dma_fence_work_timeline, and a function to attach struct dma_fence_work to such a timeline in a way that all previous fences attached to the timeline will be signaled when the latest attached struct dma_fence_work signals. Signed-off-by: Thomas Hellström --- drivers/gpu/drm/i915/i915_sw_fence_work.c | 89 ++++++++++++++++++++++- drivers/gpu/drm/i915/i915_sw_fence_work.h | 58 +++++++++++++++ 2 files changed, 145 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c index 5b55cddafc9b..87cdb3158042 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c @@ -5,6 +5,66 @@ */ #include "i915_sw_fence_work.h" +#include "i915_utils.h" + +/** + * dma_fence_work_timeline_attach - Attach a struct dma_fence_work to a + * timeline. + * @tl: The timeline to attach to. + * @f: The struct dma_fence_work. + * @tl_cb: The i915_sw_dma_fence_cb needed to attach to the + * timeline. This is typically embedded into the structure that also + * embeds the struct dma_fence_work. + * + * This function takes a timeline reference and associates it with the + * struct dma_fence_work. That reference is given up when the fence + * signals. Furthermore it assigns a fence context and a seqno to the + * dma-fence, and then chains upon the previous fence of the timeline + * if any, to make sure that the fence signals after that fence. The + * @tl_cb callback structure is needed for that chaining. Finally + * the registered last fence of the timeline is replaced by this fence, and + * the timeline takes a reference on the fence, which is released when + * the fence signals. + */ +void dma_fence_work_timeline_attach(struct dma_fence_work_timeline *tl, + struct dma_fence_work *f, + struct i915_sw_dma_fence_cb *tl_cb) +{ + struct dma_fence *await; + + if (tl->ops->get) + tl->ops->get(tl); + + spin_lock(&tl->lock); + await = tl->last_fence; + tl->last_fence = dma_fence_get(&f->dma); + f->dma.seqno = tl->seqno++; + f->dma.context = tl->context; + f->tl = tl; + spin_unlock(&tl->lock); + + if (await) { + __i915_sw_fence_await_dma_fence(&f->chain, await, tl_cb); + dma_fence_put(await); + } +} + +static void dma_fence_work_timeline_detach(struct dma_fence_work *f) +{ + struct dma_fence_work_timeline *tl = f->tl; + bool put = false; + + spin_lock(&tl->lock); + if (tl->last_fence == &f->dma) { + put = true; + tl->last_fence = NULL; + } + spin_unlock(&tl->lock); + if (tl->ops->put) + tl->ops->put(tl); + if (put) + dma_fence_put(&f->dma); +} static void dma_fence_work_complete(struct dma_fence_work *f) { @@ -13,6 +73,9 @@ static void dma_fence_work_complete(struct dma_fence_work *f) if (f->ops->release) f->ops->release(f); + if (f->tl) + dma_fence_work_timeline_detach(f); + dma_fence_put(&f->dma); } @@ -53,14 +116,17 @@ fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) static const char *get_driver_name(struct dma_fence *fence) { - return "dma-fence"; + struct dma_fence_work *f = container_of(fence, typeof(*f), dma); + + return (f->tl && f->tl->ops->name) ? f->tl->ops->name : "dma-fence"; } static const char *get_timeline_name(struct dma_fence *fence) { struct dma_fence_work *f = container_of(fence, typeof(*f), dma); - return f->ops->name ?: "work"; + return (f->tl && f->tl->name) ? f->tl->name : + f->ops->name ?: "work"; } static void fence_release(struct dma_fence *fence) @@ -84,6 +150,7 @@ void dma_fence_work_init(struct dma_fence_work *f, { f->ops = ops; f->error = 0; + f->tl = NULL; spin_lock_init(&f->lock); dma_fence_init(&f->dma, &fence_ops, &f->lock, 0, 0); i915_sw_fence_init(&f->chain, fence_notify); @@ -97,3 +164,21 @@ int dma_fence_work_chain(struct dma_fence_work *f, struct dma_fence *signal) return __i915_sw_fence_await_dma_fence(&f->chain, signal, &f->cb); } + +/** + * dma_fence_work_timeline_init - Initialize a dma_fence_work timeline + * @tl: The timeline to initialize, + * @name: The name of the timeline, + * @ops: The timeline operations. + */ +void dma_fence_work_timeline_init(struct dma_fence_work_timeline *tl, + const char *name, + const struct dma_fence_work_timeline_ops *ops) +{ + tl->name = name; + spin_lock_init(&tl->lock); + tl->context = dma_fence_context_alloc(1); + tl->seqno = 0; + tl->last_fence = NULL; + tl->ops = ops; +} diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.h b/drivers/gpu/drm/i915/i915_sw_fence_work.h index caa59fb5252b..6f41ee360133 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.h +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.h @@ -14,6 +14,53 @@ #include "i915_sw_fence.h" struct dma_fence_work; +struct dma_fence_work_timeline; + +/** + * struct dma_fence_work_timeline_ops - Timeline operations struct + * @name: Timeline ops name. This field is used if the timeline itself has + * a NULL name. Can be set to NULL in which case a default name is used. + * + * The struct dma_fence_work_timeline is intended to be embeddable. + * We use the ops to get and put the parent structure. + */ +struct dma_fence_work_timeline_ops { + /** + * Timeline ops name. Used if the timeline itself has no name. + */ + const char *name; + + /** + * put() - Put the structure embedding the timeline + * @tl: The timeline + */ + void (*put)(struct dma_fence_work_timeline *tl); + + /** + * get() - Get the structure embedding the timeline + * @tl: The timeline + */ + void (*get)(struct dma_fence_work_timeline *tl); +}; + +/** + * struct dma_fence_work_timeline - Simple timeline struct for dma_fence_work + * @name: The name of the timeline. May be set to NULL. Immutable + * @lock: Protects mutable members of the structure. + * @context: The timeline fence context. Immutable. + * @seqno: The previous seqno used. Protected by @lock. + * @last_fence : The previous fence of the timeline. Protected by @lock. + * @ops: The timeline operations struct. Immutable. + */ +struct dma_fence_work_timeline { + const char *name; + /** Protects mutable members of the structure */ + spinlock_t lock; + u64 context; + u64 seqno; + struct dma_fence *last_fence; + const struct dma_fence_work_timeline_ops *ops; +}; struct dma_fence_work_ops { const char *name; @@ -30,6 +77,9 @@ struct dma_fence_work { struct i915_sw_dma_fence_cb cb; struct work_struct work; + + struct dma_fence_work_timeline *tl; + const struct dma_fence_work_ops *ops; }; @@ -65,4 +115,12 @@ static inline void dma_fence_work_commit_imm(struct dma_fence_work *f) dma_fence_work_commit(f); } +void dma_fence_work_timeline_attach(struct dma_fence_work_timeline *tl, + struct dma_fence_work *f, + struct i915_sw_dma_fence_cb *tl_cb); + +void dma_fence_work_timeline_init(struct dma_fence_work_timeline *tl, + const char *name, + const struct dma_fence_work_timeline_ops *ops); + #endif /* I915_SW_FENCE_WORK_H */ From patchwork Fri Oct 8 13:35:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 12545369 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABC67C433EF for ; Fri, 8 Oct 2021 13:36:13 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 73E5561042 for ; Fri, 8 Oct 2021 13:36:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 73E5561042 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A64396F518; Fri, 8 Oct 2021 13:35:56 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8D1196F50D; Fri, 8 Oct 2021 13:35:53 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10130"; a="287388467" X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="287388467" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:53 -0700 X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="522983736" Received: from lenovo-x280.ger.corp.intel.com (HELO thellstr-mobl1.intel.com) ([10.249.254.98]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:51 -0700 From: =?utf-8?q?Thomas_Hellstr=C3=B6m?= To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com, =?utf-8?q?Tho?= =?utf-8?q?mas_Hellstr=C3=B6m?= Date: Fri, 8 Oct 2021 15:35:29 +0200 Message-Id: <20211008133530.664509-6-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> References: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 5/6] drm/i915/ttm: Attach the migration fence to a region timeline on eviction X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On eviction, TTM requires that migration fences from the same region are ordered using dma_fence_is_later(). For request-based fences we therefore need to use the same context for the migration, but now that we use a dma_fence_work for error recovery, and, in addition, might need to coalesce the migration fence with async unbind fences, Create a coalesce fence for this. Chain the coalesce fence on the migration fence and attach it to a region timeline. Signed-off-by: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 84 ++++++++++++++++++---- drivers/gpu/drm/i915/intel_memory_region.c | 43 +++++++++++ drivers/gpu/drm/i915/intel_memory_region.h | 7 ++ 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 79d4d50aa4e5..625ce52e8662 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -672,9 +672,10 @@ static void __i915_ttm_move_fallback(struct ttm_buffer_object *bo, bool clear, } } -static int __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, - struct ttm_resource *dst_mem, struct ttm_tt *dst_ttm, - struct i915_refct_sgt *dst_rsgt, bool allow_accel) +static struct dma_fence * +__i915_ttm_move(struct ttm_buffer_object *bo, bool clear, + struct ttm_resource *dst_mem, struct ttm_tt *dst_ttm, + struct i915_refct_sgt *dst_rsgt, bool allow_accel) { struct i915_ttm_memcpy_work *copy_work; struct dma_fence *fence; @@ -689,7 +690,7 @@ static int __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, /* Don't fail with -ENOMEM. Move sync instead. */ __i915_ttm_move_fallback(bo, clear, dst_mem, dst_ttm, dst_rsgt, allow_accel); - return 0; + return NULL; } dma_fence_work_init(©_work->base, &i915_ttm_memcpy_ops); @@ -714,14 +715,45 @@ static int __i915_ttm_move(struct ttm_buffer_object *bo, bool clear, fence = dma_fence_get(©_work->base.dma); dma_fence_work_commit_imm(©_work->base); - /* - * We're synchronizing here for now. For async moves, return the - * fence. - */ - dma_fence_wait(fence, false); - dma_fence_put(fence); + return fence; +} - return ret; +/** + * struct i915_coalesce_fence - A dma-fence used to coalesce multiple fences + * similar to struct dm_fence_array, and at the same time being timeline- + * attached. + * @base: struct dma_fence_work base. + * @cb: Callback for timeline attachment. + */ +struct i915_coalesce_fence { + struct dma_fence_work base; + struct i915_sw_dma_fence_cb cb; +}; + +/* No .work or .release callback. Just coalescing. */ +static const struct dma_fence_work_ops i915_coalesce_fence_ops = { + .name = "Coalesce fence", +}; + +static struct dma_fence * +i915_ttm_coalesce_fence(struct dma_fence *fence, struct intel_memory_region *mr) +{ + struct i915_coalesce_fence *coalesce = + kmalloc(sizeof(*coalesce), GFP_KERNEL); + + if (!coalesce) { + dma_fence_wait(fence, false); + dma_fence_put(fence); + return NULL; + } + + dma_fence_work_init(&coalesce->base, &i915_coalesce_fence_ops); + dma_fence_work_chain(&coalesce->base, fence); + dma_fence_work_timeline_attach(&mr->tl, &coalesce->base, &coalesce->cb); + dma_fence_get(&coalesce->base.dma); + dma_fence_work_commit_imm(&coalesce->base); + dma_fence_put(fence); + return &coalesce->base.dma; } static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, @@ -734,6 +766,7 @@ static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, ttm_manager_type(bo->bdev, dst_mem->mem_type); struct ttm_tt *ttm = bo->ttm; struct i915_refct_sgt *dst_rsgt; + struct dma_fence *fence = NULL; bool clear; int ret; @@ -765,7 +798,23 @@ static int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, clear = !cpu_maps_iomem(bo->resource) && (!ttm || !ttm_tt_is_populated(ttm)); if (!(clear && ttm && !(ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC))) - __i915_ttm_move(bo, clear, dst_mem, bo->ttm, dst_rsgt, true); + fence = __i915_ttm_move(bo, clear, dst_mem, bo->ttm, dst_rsgt, true); + if (fence && evict) { + struct intel_memory_region *mr = + i915_ttm_region(bo->bdev, bo->resource->mem_type); + + /* + * Attach to the region timeline and for future async unbind, + * which requires a timeline. Also future async unbind fences + * can be attached here. + */ + fence = i915_ttm_coalesce_fence(fence, mr); + } + + if (fence) { + dma_fence_wait(fence, false); + dma_fence_put(fence); + } ttm_bo_move_sync_cleanup(bo, dst_mem); i915_ttm_adjust_domains_after_move(obj); @@ -1223,6 +1272,7 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst, .interruptible = intr, }; struct i915_refct_sgt *dst_rsgt; + struct dma_fence *fence; int ret; assert_object_held(dst); @@ -1238,10 +1288,14 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst, return ret; dst_rsgt = i915_ttm_resource_get_st(dst, dst_bo->resource); - __i915_ttm_move(src_bo, false, dst_bo->resource, dst_bo->ttm, - dst_rsgt, allow_accel); - + fence = __i915_ttm_move(src_bo, false, dst_bo->resource, dst_bo->ttm, + dst_rsgt, allow_accel); i915_refct_sgt_put(dst_rsgt); + if (fence) { + dma_fence_wait(fence, false); + dma_fence_put(fence); + } + return 0; } diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index e7f7e6627750..aa1733e840f7 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -7,6 +7,9 @@ #include "i915_drv.h" #include "i915_ttm_buddy_manager.h" +static const struct dma_fence_work_timeline_ops tl_ops; +static void intel_region_timeline_release_work(struct work_struct *work); + static const struct { u16 class; u16 instance; @@ -127,6 +130,10 @@ intel_memory_region_create(struct drm_i915_private *i915, } kref_init(&mem->kref); + + INIT_WORK(&mem->tl_put_work, intel_region_timeline_release_work); + dma_fence_work_timeline_init(&mem->tl, NULL, &tl_ops); + return mem; err_free: @@ -238,6 +245,42 @@ void intel_memory_regions_driver_release(struct drm_i915_private *i915) } } +static void intel_region_timeline_get(struct dma_fence_work_timeline *tl) +{ + struct intel_memory_region *mr = container_of(tl, typeof(*mr), tl); + + intel_memory_region_get(mr); +} + +static void intel_region_timeline_release_work(struct work_struct *work) +{ + struct intel_memory_region *mr = + container_of(work, typeof(*mr), tl_put_work); + + __intel_memory_region_destroy(&mr->kref); +} + +static void intel_region_timeline_release(struct kref *ref) +{ + struct intel_memory_region *mr = container_of(ref, typeof(*mr), kref); + + /* May be called from hardirq context, so queue the final release. */ + queue_work(system_unbound_wq, &mr->tl_put_work); +} + +static void intel_region_timeline_put(struct dma_fence_work_timeline *tl) +{ + struct intel_memory_region *mr = container_of(tl, typeof(*mr), tl); + + kref_put(&mr->kref, intel_region_timeline_release); +} + +static const struct dma_fence_work_timeline_ops tl_ops = { + .name = "Region timeline", + .get = intel_region_timeline_get, + .put = intel_region_timeline_put, +}; + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/intel_memory_region.c" #include "selftests/mock_region.c" diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h index 3feae3353d33..928819e2edba 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.h +++ b/drivers/gpu/drm/i915/intel_memory_region.h @@ -13,6 +13,8 @@ #include #include +#include "i915_sw_fence_work.h" + struct drm_i915_private; struct drm_i915_gem_object; struct drm_printer; @@ -94,6 +96,11 @@ struct intel_memory_region { bool is_range_manager; void *region_private; + + /** Timeline for TTM eviction fences */ + struct dma_fence_work_timeline tl; + /** Work struct for _region_put() from atomic / irq context */ + struct work_struct tl_put_work; }; struct intel_memory_region * From patchwork Fri Oct 8 13:35:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 12545371 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04735C433EF for ; Fri, 8 Oct 2021 13:36:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CAD7F60F46 for ; Fri, 8 Oct 2021 13:36:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CAD7F60F46 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 386C96F519; Fri, 8 Oct 2021 13:36:01 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 47F716F512; Fri, 8 Oct 2021 13:35:55 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10130"; a="287388473" X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="287388473" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:55 -0700 X-IronPort-AV: E=Sophos;i="5.85,357,1624345200"; d="scan'208";a="522983749" Received: from lenovo-x280.ger.corp.intel.com (HELO thellstr-mobl1.intel.com) ([10.249.254.98]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 06:35:53 -0700 From: =?utf-8?q?Thomas_Hellstr=C3=B6m?= To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: maarten.lankhorst@linux.intel.com, matthew.auld@intel.com, =?utf-8?q?Tho?= =?utf-8?q?mas_Hellstr=C3=B6m?= Date: Fri, 8 Oct 2021 15:35:30 +0200 Message-Id: <20211008133530.664509-7-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> References: <20211008133530.664509-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 6/6] drm/i915: Use irq work for coalescing-only dma-fence-work X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" We are using a timeline-attached struct dma_fence_work to coalesce dma-fences on eviction. In this mode we will not have a work callback attached. Similar to how the dma-fence-chain and dma-fence-array containers do this, use irq work to signal to reduce latency. Signed-off-by: Thomas Hellström --- drivers/gpu/drm/i915/i915_sw_fence_work.c | 36 ++++++++++++++++++----- drivers/gpu/drm/i915/i915_sw_fence_work.h | 2 ++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c index 87cdb3158042..4573f537ada4 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.c +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c @@ -32,16 +32,17 @@ void dma_fence_work_timeline_attach(struct dma_fence_work_timeline *tl, { struct dma_fence *await; + might_sleep(); if (tl->ops->get) tl->ops->get(tl); - spin_lock(&tl->lock); + spin_lock_irq(&tl->lock); await = tl->last_fence; tl->last_fence = dma_fence_get(&f->dma); f->dma.seqno = tl->seqno++; f->dma.context = tl->context; f->tl = tl; - spin_unlock(&tl->lock); + spin_unlock_irq(&tl->lock); if (await) { __i915_sw_fence_await_dma_fence(&f->chain, await, tl_cb); @@ -53,13 +54,14 @@ static void dma_fence_work_timeline_detach(struct dma_fence_work *f) { struct dma_fence_work_timeline *tl = f->tl; bool put = false; + unsigned long irq_flags; - spin_lock(&tl->lock); + spin_lock_irqsave(&tl->lock, irq_flags); if (tl->last_fence == &f->dma) { put = true; tl->last_fence = NULL; } - spin_unlock(&tl->lock); + spin_unlock_irqrestore(&tl->lock, irq_flags); if (tl->ops->put) tl->ops->put(tl); if (put) @@ -68,8 +70,6 @@ static void dma_fence_work_timeline_detach(struct dma_fence_work *f) static void dma_fence_work_complete(struct dma_fence_work *f) { - dma_fence_signal(&f->dma); - if (f->ops->release) f->ops->release(f); @@ -79,13 +79,32 @@ static void dma_fence_work_complete(struct dma_fence_work *f) dma_fence_put(&f->dma); } +static void dma_fence_work_irq_work(struct irq_work *irq_work) +{ + struct dma_fence_work *f = container_of(irq_work, typeof(*f), irq_work); + + dma_fence_signal(&f->dma); + if (f->ops->release) + /* Note we take the signaled path in dma_fence_work_work() */ + queue_work(system_unbound_wq, &f->work); + else + dma_fence_work_complete(f); +} + static void dma_fence_work_work(struct work_struct *work) { struct dma_fence_work *f = container_of(work, typeof(*f), work); + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &f->dma.flags)) { + dma_fence_work_complete(f); + return; + } + if (f->ops->work) f->ops->work(f); + dma_fence_signal(&f->dma); + dma_fence_work_complete(f); } @@ -102,8 +121,10 @@ fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) dma_fence_get(&f->dma); if (test_bit(DMA_FENCE_WORK_IMM, &f->dma.flags)) dma_fence_work_work(&f->work); - else + else if (f->ops->work) queue_work(system_unbound_wq, &f->work); + else + irq_work_queue(&f->irq_work); break; case FENCE_FREE: @@ -155,6 +176,7 @@ void dma_fence_work_init(struct dma_fence_work *f, dma_fence_init(&f->dma, &fence_ops, &f->lock, 0, 0); i915_sw_fence_init(&f->chain, fence_notify); INIT_WORK(&f->work, dma_fence_work_work); + init_irq_work(&f->irq_work, dma_fence_work_irq_work); } int dma_fence_work_chain(struct dma_fence_work *f, struct dma_fence *signal) diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.h b/drivers/gpu/drm/i915/i915_sw_fence_work.h index 6f41ee360133..c412bb4cb288 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.h +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.h @@ -8,6 +8,7 @@ #define I915_SW_FENCE_WORK_H #include +#include #include #include @@ -77,6 +78,7 @@ struct dma_fence_work { struct i915_sw_dma_fence_cb cb; struct work_struct work; + struct irq_work irq_work; struct dma_fence_work_timeline *tl;