All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
To: igt-dev@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Subject: [igt-dev] [PATCH i-g-t v3 12/23] lib/intel_batchbuffer: Add tracking intel_buf to intel_bb
Date: Fri, 23 Oct 2020 14:42:01 +0200	[thread overview]
Message-ID: <20201023124212.26428-13-zbigniew.kempczynski@intel.com> (raw)
In-Reply-To: <20201023124212.26428-1-zbigniew.kempczynski@intel.com>

From now on intel_bb starts tracking added/removed intel_bufs.
We're safe now regardless order of intel_buf close/destroy
or intel_bb destroy paths.

When intel_buf is closed/destroyed first and it was previously added
to intel_bb it calls the code which removes itself from intel_bb.

When intel_bb is destroyed first it goes over all tracked intel_bufs
and clears tracking information and clears previous buffer offset
(it sets to INTEL_BUF_INVALID_ADDRESS).

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/intel_batchbuffer.c | 82 +++++++++++++++++++++++++++++++----------
 lib/intel_batchbuffer.h |  5 +++
 lib/intel_bufops.c      | 15 ++++++--
 lib/intel_bufops.h      |  6 +++
 4 files changed, 85 insertions(+), 23 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 22d27376..da52014c 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -1308,6 +1308,8 @@ __intel_bb_create(int i915, uint32_t ctx, uint32_t size, bool do_relocs,
 				     false);
 	ibb->batch_offset = object->offset;
 
+	IGT_INIT_LIST_HEAD(&ibb->intel_bufs);
+
 	ibb->refcount = 1;
 
 	return ibb;
@@ -1438,6 +1440,15 @@ static void __intel_bb_destroy_cache(struct intel_bb *ibb)
 	ibb->root = NULL;
 }
 
+static void __intel_bb_remove_intel_bufs(struct intel_bb *ibb)
+{
+	struct intel_buf *entry, *tmp;
+
+	igt_list_for_each_entry_safe(entry, tmp, &ibb->intel_bufs, link) {
+		intel_bb_remove_intel_buf(ibb, entry);
+	}
+}
+
 /**
  * intel_bb_destroy:
  * @ibb: pointer to intel_bb
@@ -1454,6 +1465,7 @@ void intel_bb_destroy(struct intel_bb *ibb)
 	__intel_bb_destroy_relocations(ibb);
 	__intel_bb_destroy_objects(ibb);
 	__intel_bb_destroy_cache(ibb);
+	__intel_bb_remove_intel_bufs(ibb);
 
 	if (ibb->allocator_type != INTEL_ALLOCATOR_NONE) {
 		intel_allocator_free(ibb->allocator_handle, ibb->handle);
@@ -1801,24 +1813,13 @@ intel_bb_add_object(struct intel_bb *ibb, uint32_t handle, uint64_t size,
 
 	object->offset = offset;
 
-	/* Limit current offset to gtt size */
-	if (offset != INTEL_BUF_INVALID_ADDRESS) {
-		object->offset = gen8_canonical_addr(offset & (ibb->gtt_size - 1));
-
-
-	} else {
-		object->offset = __intel_bb_get_offset(ibb,
-						       handle, size,
-						       object->alignment);
-	}
-
 	if (write)
 		object->flags |= EXEC_OBJECT_WRITE;
 
 	if (ibb->supports_48b_address)
 		object->flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
 
-	if (ibb->uses_full_ppgtt && ibb->allocator_type == INTEL_ALLOCATOR_SIMPLE)
+	if (ibb->uses_full_ppgtt && !ibb->enforce_relocs)
 		object->flags |= EXEC_OBJECT_PINNED;
 
 	return object;
@@ -1854,7 +1855,12 @@ __intel_bb_add_intel_buf(struct intel_bb *ibb, struct intel_buf *buf,
 			 uint64_t alignment, bool write)
 {
 	struct drm_i915_gem_exec_object2 *obj;
+	struct intel_buf *entry;
+	bool found = false;
 
+	igt_assert(ibb);
+	igt_assert(buf);
+	igt_assert(!buf->ibb || buf->ibb == ibb);
 	igt_assert(ALIGN(alignment, 4096) == alignment);
 
 	if (!alignment) {
@@ -1875,10 +1881,21 @@ __intel_bb_add_intel_buf(struct intel_bb *ibb, struct intel_buf *buf,
 	obj = intel_bb_add_object(ibb, buf->handle, intel_buf_bo_size(buf),
 				  buf->addr.offset, alignment, write);
 	buf->addr.offset = obj->offset;
+	buf->ibb = ibb;
 
 	if (!ibb->enforce_relocs)
 		obj->alignment = alignment;
 
+	igt_list_for_each_entry(entry, &ibb->intel_bufs, link) {
+		if (buf->handle == entry->handle) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		igt_list_add_tail(&buf->link, &ibb->intel_bufs);
+
 	return obj;
 }
 
@@ -1897,16 +1914,40 @@ intel_bb_add_intel_buf_with_alignment(struct intel_bb *ibb, struct intel_buf *bu
 
 bool intel_bb_remove_intel_buf(struct intel_bb *ibb, struct intel_buf *buf)
 {
-	bool removed = intel_bb_remove_object(ibb, buf->handle,
-					      buf->addr.offset,
-					      intel_buf_bo_size(buf));
+	struct intel_buf *entry, *tmp;
+	bool removed;
 
-	if (removed)
-		buf->addr.offset = INTEL_BUF_INVALID_ADDRESS;
+	igt_assert(ibb);
+	igt_assert(buf);
+	igt_assert(!buf->ibb || buf->ibb == ibb);
+
+	removed = intel_bb_remove_object(ibb, buf->handle,
+					 buf->addr.offset,
+					 intel_buf_bo_size(buf));
+
+	igt_list_for_each_entry_safe(entry, tmp, &ibb->intel_bufs, link) {
+		if (entry->handle == buf->handle) {
+			buf->addr.offset = INTEL_BUF_INVALID_ADDRESS;
+			buf->ibb = NULL;
+			igt_list_del(&entry->link);
+			break;
+		}
+	}
 
 	return removed;
 }
 
+void intel_bb_intel_buf_list(struct intel_bb *ibb)
+{
+	struct intel_buf *entry;
+
+	igt_list_for_each_entry(entry, &ibb->intel_bufs, link) {
+		igt_info("handle: %u, ibb: %p, offset: %lx\n",
+			 entry->handle, entry->ibb,
+			 (long) entry->addr.offset);
+	}
+}
+
 struct drm_i915_gem_exec_object2 *
 intel_bb_find_object(struct intel_bb *ibb, uint32_t handle)
 {
@@ -2302,7 +2343,7 @@ static void update_offsets(struct intel_bb *ibb,
 
 #define LINELEN 76
 /*
- * @__intel_bb_exec:
+ * __intel_bb_exec:
  * @ibb: pointer to intel_bb
  * @end_offset: offset of the last instruction in the bb
  * @flags: flags passed directly to execbuf
@@ -2341,6 +2382,9 @@ static int __intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
 	if (ibb->dump_base64)
 		intel_bb_dump_base64(ibb, LINELEN);
 
+	/* For debugging on CI, remove in final series */
+	intel_bb_dump_execbuf(ibb, &execbuf);
+
 	ret = __gem_execbuf_wr(ibb->i915, &execbuf);
 	if (ret) {
 		intel_bb_dump_execbuf(ibb, &execbuf);
@@ -2442,7 +2486,7 @@ bool intel_bb_object_offset_to_buf(struct intel_bb *ibb, struct intel_buf *buf)
 		return false;
 	}
 
-	buf->addr.offset = (*found)->offset & (ibb->gtt_size - 1);
+	buf->addr.offset = (*found)->offset;
 	buf->addr.ctx = ibb->ctx;
 
 	return true;
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index 029b7e23..497ca551 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -6,6 +6,7 @@
 #include <i915_drm.h>
 
 #include "igt_core.h"
+#include "igt_list.h"
 #include "intel_reg.h"
 #include "drmtest.h"
 #include "intel_allocator.h"
@@ -474,6 +475,9 @@ struct intel_bb {
 	uint32_t num_relocs;
 	uint32_t allocated_relocs;
 
+	/* Tracked intel_bufs */
+	struct igt_list_head intel_bufs;
+
 	/*
 	 * BO recreate in reset path only when refcount == 0
 	 * Currently we don't need to use atomics because intel_bb
@@ -586,6 +590,7 @@ struct drm_i915_gem_exec_object2 *
 intel_bb_add_intel_buf_with_alignment(struct intel_bb *ibb, struct intel_buf *buf,
 				      uint64_t alignment, bool write);
 bool intel_bb_remove_intel_buf(struct intel_bb *ibb, struct intel_buf *buf);
+void intel_bb_intel_buf_list(struct intel_bb *ibb);
 struct drm_i915_gem_exec_object2 *
 intel_bb_find_object(struct intel_bb *ibb, uint32_t handle);
 
diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
index c0d2ea4f..5833c3f6 100644
--- a/lib/intel_bufops.c
+++ b/lib/intel_bufops.c
@@ -820,17 +820,24 @@ void intel_buf_init(struct buf_ops *bops,
  *
  * Function closes gem BO inside intel_buf if bo is owned by intel_buf.
  * For handle passed from the caller intel_buf doesn't take ownership and
- * doesn't close it in close()/destroy() paths.
+ * doesn't close it in close()/destroy() paths. When intel_buf was previously
+ * added to intel_bb (intel_bb_add_intel_buf() call) it is tracked there and
+ * must be removed from its internal structures.
  */
 void intel_buf_close(struct buf_ops *bops, struct intel_buf *buf)
 {
 	igt_assert(bops);
 	igt_assert(buf);
 
-	if (buf->is_owner) {
-		intel_allocator_remove_handle(bops->fd, buf->handle);
-		gem_close(bops->fd, buf->handle);
+	/* If buf is tracked by some intel_bb ensure it will be removed there */
+	if (buf->ibb) {
+		intel_bb_remove_intel_buf(buf->ibb, buf);
+		buf->addr.offset = INTEL_BUF_INVALID_ADDRESS;
+		buf->ibb = NULL;
 	}
+
+	if (buf->is_owner)
+		gem_close(bops->fd, buf->handle);
 }
 
 /**
diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
index 54480bff..1a3d8692 100644
--- a/lib/intel_bufops.h
+++ b/lib/intel_bufops.h
@@ -2,6 +2,7 @@
 #define __INTEL_BUFOPS_H__
 
 #include <stdint.h>
+#include "igt_list.h"
 #include "igt_aux.h"
 #include "intel_batchbuffer.h"
 
@@ -13,6 +14,7 @@ struct buf_ops;
 
 struct intel_buf {
 	struct buf_ops *bops;
+
 	bool is_owner;
 	uint32_t handle;
 	uint64_t size;
@@ -40,6 +42,10 @@ struct intel_buf {
 		uint32_t ctx;
 	} addr;
 
+	/* Tracking */
+	struct intel_bb *ibb;
+	struct igt_list_head link;
+
 	/* CPU mapping */
 	uint32_t *ptr;
 	bool cpu_write;
-- 
2.26.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  parent reply	other threads:[~2020-10-23 12:42 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-23 12:41 [igt-dev] [PATCH i-g-t v3 00/23] Introduce IGT allocator Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 01/23] lib/igt_list: igt_hlist implementation Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 02/23] lib/igt_map: Introduce igt_map Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 03/23] lib/igt_core: Track child process pid and tid Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 04/23] lib/intel_allocator_simple: Add simple allocator Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 05/23] lib/intel_allocator_random: Add random allocator Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 06/23] lib/intel_allocator: Add intel_allocator core Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 07/23] lib/intel_allocator: Try to stop smoothly instead of deinit Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 08/23] lib/intel_allocator_msgchannel: Scale to 4k of parallel clients Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 09/23] lib/intel_bufops: Removes handle from allocator, change size Zbigniew Kempczyński
2020-10-23 12:41 ` [igt-dev] [PATCH i-g-t v3 10/23] lib/intel_bufops: Add init with handle and size function Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 11/23] lib/intel_batchbuffer: Integrate intel_bb with allocator Zbigniew Kempczyński
2020-10-23 12:42 ` Zbigniew Kempczyński [this message]
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 13/23] lib/intel_aux_pgtable: Get addresses for aux table from an allocator Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 14/23] lib/igt_fb: Initialize intel_buf with same size as fb Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 15/23] tests/api_intel_bb: Modify test to verify intel_bb with allocator Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 16/23] tests/api_intel_bb: Add compressed->compressed copy Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 17/23] tests/api_intel_allocator: Simple allocator test suite Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 18/23] tests/gem|kms: Remove intel_bb from fixture Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 19/23] tests/gem_mmap_offset: Use intel_buf wrapper code instead direct Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 20/23] tests/gem_ppgtt: Adopt test to use intel_bb with allocator Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 21/23] tests/gem_render_copy_redux: Adopt to use with intel_bb and allocator Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 22/23] tests/perf.c: Remove buffer from batch Zbigniew Kempczyński
2020-10-23 12:42 ` [igt-dev] [PATCH i-g-t v3 23/23] tests/gem_linear_blits: Use intel allocator Zbigniew Kempczyński
2020-10-23 13:10 ` [igt-dev] ✓ Fi.CI.BAT: success for Introduce IGT allocator (rev3) Patchwork
2020-10-23 15:29 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2020-10-26  5:30   ` Zbigniew Kempczyński
2020-10-26  5:43 ` [igt-dev] ✓ Fi.CI.BAT: success for Introduce IGT allocator (rev4) Patchwork
2020-10-26  6:44 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2020-10-26  8:09   ` Zbigniew Kempczyński

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=20201023124212.26428-13-zbigniew.kempczynski@intel.com \
    --to=zbigniew.kempczynski@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    /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.