Re: [PATCH] drm/ttm/nouveau: consolidate slowpath reserve

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 28.07.20 um 08:24 schrieb Dave Airlie:
From: Dave Airlie <airlied@xxxxxxxxxx>

The WARN_ON in the non-underscore path is off questionable value
(can we drop it from the non-slowpath?). At least for nouveau
where it's just looked up the gem object we know the ttm object
has a reference always so we can skip the check.

Yeah, agreed. Wanted to look into removing that for quite some time as well.

It's probably nouveau could use execbut utils here at some point
but for now align the code between them to always call the __
versions, and remove the non underscored version.

Can we do it the other way around and remove all uses of the __ versions of the functions instead and then merge the __ version into the normal one without the WARN_ON()?

Christian.


Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
---
  drivers/gpu/drm/nouveau/nouveau_gem.c  |  6 ++---
  drivers/gpu/drm/ttm/ttm_execbuf_util.c | 10 +--------
  include/drm/ttm/ttm_bo_driver.h        | 31 +++++++++++---------------
  3 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 81f111ad3f4f..d2d535d2ece1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -422,15 +422,15 @@ validate_init(struct nouveau_channel *chan, struct drm_file *file_priv,
  			break;
  		}
- ret = ttm_bo_reserve(&nvbo->bo, true, false, &op->ticket);
+		ret = __ttm_bo_reserve(&nvbo->bo, true, false, &op->ticket);
  		if (ret) {
  			list_splice_tail_init(&vram_list, &op->list);
  			list_splice_tail_init(&gart_list, &op->list);
  			list_splice_tail_init(&both_list, &op->list);
  			validate_fini_no_ticket(op, chan, NULL, NULL);
  			if (unlikely(ret == -EDEADLK)) {
-				ret = ttm_bo_reserve_slowpath(&nvbo->bo, true,
-							      &op->ticket);
+				ret = __ttm_bo_reserve_slowpath(&nvbo->bo, true,
+								&op->ticket);
  				if (!ret)
  					res_bo = nvbo;
  			}
diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 1797f04c0534..a24f13bc90fb 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -119,13 +119,7 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
  		ttm_eu_backoff_reservation_reverse(list, entry);
if (ret == -EDEADLK) {
-			if (intr) {
-				ret = dma_resv_lock_slow_interruptible(bo->base.resv,
-										 ticket);
-			} else {
-				dma_resv_lock_slow(bo->base.resv, ticket);
-				ret = 0;
-			}
+			ret = __ttm_bo_reserve_slowpath(bo, intr, ticket);
  		}
if (!ret && entry->num_shared)
@@ -133,8 +127,6 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
  								entry->num_shared);
if (unlikely(ret != 0)) {
-			if (ret == -EINTR)
-				ret = -ERESTARTSYS;
  			if (ticket) {
  				ww_acquire_done(ticket);
  				ww_acquire_fini(ticket);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 5a37f1cc057e..563b970949a1 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -695,7 +695,7 @@ static inline int ttm_bo_reserve(struct ttm_buffer_object *bo,
  }
/**
- * ttm_bo_reserve_slowpath:
+ * __ttm_bo_reserve_slowpath:
   * @bo: A pointer to a struct ttm_buffer_object.
   * @interruptible: Sleep interruptible if waiting.
   * @sequence: Set (@bo)->sequence to this value after lock
@@ -704,24 +704,19 @@ static inline int ttm_bo_reserve(struct ttm_buffer_object *bo,
   * from all our other reservations. Because there are no other reservations
   * held by us, this function cannot deadlock any more.
   */
-static inline int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo,
-					  bool interruptible,
-					  struct ww_acquire_ctx *ticket)
+static inline int __ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo,
+					    bool interruptible,
+					    struct ww_acquire_ctx *ticket)
  {
-	int ret = 0;
-
-	WARN_ON(!kref_read(&bo->kref));
-
-	if (interruptible)
-		ret = dma_resv_lock_slow_interruptible(bo->base.resv,
-								 ticket);
-	else
-		dma_resv_lock_slow(bo->base.resv, ticket);
-
-	if (ret == -EINTR)
-		ret = -ERESTARTSYS;
-
-	return ret;
+	if (interruptible) {
+		int ret = dma_resv_lock_slow_interruptible(bo->base.resv,
+							   ticket);
+		if (ret == -EINTR)
+			ret = -ERESTARTSYS;
+		return ret;
+	}
+	dma_resv_lock_slow(bo->base.resv, ticket);
+	return 0;
  }
/**

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux