All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: thomas.hellstrom@linux.intel.com, daniel@ffwll.ch,
	intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH] drm/i915: remove questionable fence optimization during copy
Date: Tue, 21 Dec 2021 15:07:13 +0100	[thread overview]
Message-ID: <20211221140713.367218-1-christian.koenig@amd.com> (raw)

First of all as discussed multiple times now kernel copies *must* always wait
for all fences in a BO before actually doing the copy. This is mandatory.

Additional to that drop the handling when there can't be a shared slot
allocated on the source BO and just properly return an error code. Otherwise
this code path would only be tested under out of memory conditions.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 39 +++++++-------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 80df9f592407..798f310d710c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -250,19 +250,14 @@ static struct dma_fence *i915_deps_to_fence(struct i915_deps *deps,
 }
 
 static int i915_deps_add_resv(struct i915_deps *deps, struct dma_resv *resv,
-			      bool all, const bool no_excl,
 			      const struct ttm_operation_ctx *ctx)
 {
 	struct dma_resv_iter iter;
 	struct dma_fence *fence;
+	int ret;
 
 	dma_resv_assert_held(resv);
-	dma_resv_for_each_fence(&iter, resv, all, fence) {
-		int ret;
-
-		if (no_excl && dma_resv_iter_is_exclusive(&iter))
-			continue;
-
+	dma_resv_for_each_fence(&iter, resv, true, fence) {
 		ret = i915_deps_add_dependency(deps, fence, ctx);
 		if (ret)
 			return ret;
@@ -698,7 +693,7 @@ static struct dma_fence *prev_fence(struct ttm_buffer_object *bo,
 		 * TODO: Only await excl fence here, and shared fences before
 		 * signaling the migration fence.
 		 */
-		ret = i915_deps_add_resv(&deps, bo->base.resv, true, false, ctx);
+		ret = i915_deps_add_resv(&deps, bo->base.resv, ctx);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -828,22 +823,21 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
 	struct i915_refct_sgt *dst_rsgt;
 	struct dma_fence *copy_fence, *dep_fence;
 	struct i915_deps deps;
-	int ret, shared_err;
+	int ret;
 
 	assert_object_held(dst);
 	assert_object_held(src);
 	i915_deps_init(&deps, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
 
-	/*
-	 * We plan to add a shared fence only for the source. If that
-	 * fails, we await all source fences before commencing
-	 * the copy instead of only the exclusive.
-	 */
-	shared_err = dma_resv_reserve_shared(src_bo->base.resv, 1);
-	ret = i915_deps_add_resv(&deps, dst_bo->base.resv, true, false, &ctx);
-	if (!ret)
-		ret = i915_deps_add_resv(&deps, src_bo->base.resv,
-					 !!shared_err, false, &ctx);
+	ret = dma_resv_reserve_shared(src_bo->base.resv, 1);
+	if (ret)
+		return ret;
+
+	ret = i915_deps_add_resv(&deps, dst_bo->base.resv, &ctx);
+	if (ret)
+		return ret;
+
+	ret = i915_deps_add_resv(&deps, src_bo->base.resv, &ctx);
 	if (ret)
 		return ret;
 
@@ -861,12 +855,7 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
 		return PTR_ERR_OR_ZERO(copy_fence);
 
 	dma_resv_add_excl_fence(dst_bo->base.resv, copy_fence);
-
-	/* If we failed to reserve a shared slot, add an exclusive fence */
-	if (shared_err)
-		dma_resv_add_excl_fence(src_bo->base.resv, copy_fence);
-	else
-		dma_resv_add_shared_fence(src_bo->base.resv, copy_fence);
+	dma_resv_add_shared_fence(src_bo->base.resv, copy_fence);
 
 	dma_fence_put(copy_fence);
 
-- 
2.25.1


             reply	other threads:[~2022-01-10 13:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-21 14:07 Christian König [this message]
2021-12-21 15:47 ` [Intel-gfx] [PATCH] drm/i915: remove questionable fence optimization during copy Thomas Hellström
2021-12-22  7:47   ` Christian König
2022-01-10 13:27 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211221140713.367218-1-christian.koenig@amd.com \
    --to=ckoenig.leichtzumerken@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.