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 v12 12/31] lib/intel_batchbuffer: Add tracking intel_buf to intel_bb
Date: Tue,  5 Jan 2021 09:10:29 +0100	[thread overview]
Message-ID: <20210105081048.14389-13-zbigniew.kempczynski@intel.com> (raw)
In-Reply-To: <20210105081048.14389-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 a4e90dfc6..cb489f305 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -1312,6 +1312,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;
@@ -1442,6 +1444,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
@@ -1458,6 +1469,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);
@@ -1809,24 +1821,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;
@@ -1862,7 +1863,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) {
@@ -1883,10 +1889,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;
 }
 
@@ -1905,16 +1922,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)
 {
@@ -2312,7 +2353,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
@@ -2351,6 +2392,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);
@@ -2452,7 +2496,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 b9a4ee7e8..82df991bc 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"
@@ -480,6 +481,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
@@ -592,6 +596,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 c0d2ea4fb..5833c3f68 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 54480bff6..1a3d86925 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:[~2021-01-05  8:11 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-05  8:10 [igt-dev] [PATCH i-g-t v12 00/31] Introduce IGT allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 01/31] lib/igt_list: igt_hlist implementation Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 02/31] lib/igt_map: Introduce igt_map Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 03/31] lib/igt_core: Track child process pid and tid Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 04/31] lib/intel_allocator_simple: Add simple allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 05/31] lib/intel_allocator_random: Add random allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 06/31] lib/intel_allocator: Add intel_allocator core Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 07/31] lib/intel_allocator: Try to stop smoothly instead of deinit Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 08/31] lib/intel_allocator_msgchannel: Scale to 4k of parallel clients Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 09/31] lib/intel_bufops: Removes handle from allocator, change size Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 10/31] lib/intel_bufops: Add init with handle and size function Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 11/31] lib/intel_batchbuffer: Integrate intel_bb with allocator Zbigniew Kempczyński
2021-01-05  8:10 ` Zbigniew Kempczyński [this message]
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 13/31] lib/intel_aux_pgtable: Get addresses for aux table from an allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 14/31] lib/igt_fb: Initialize intel_buf with same size as fb Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 15/31] tests/api_intel_bb: Modify test to verify intel_bb with allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 16/31] tests/api_intel_bb: Add compressed->compressed copy Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 17/31] tests/api_intel_allocator: Simple allocator test suite Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 18/31] tests/gem|kms: Remove intel_bb from fixture Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 19/31] tests/gem_mmap_offset: Use intel_buf wrapper code instead direct Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 20/31] tests/gem_ppgtt: Adopt test to use intel_bb with allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 21/31] tests/gem_render_copy_redux: Adopt to use with intel_bb and allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 22/31] tests/perf.c: Remove buffer from batch Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 23/31] tests/gem_linear_blits: Use intel allocator Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 24/31] lib/ioctl_wrappers: Add gem_has_relocations() check Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 25/31] lib/intel_batchbuffer: Use relocations in intel-bb up to gen12 Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 26/31] tests/api_intel_*: Adopt to use relocations as default " Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 27/31] tests/gem_ppgtt: Migrate memory check out of render blits Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 28/31] tests/api_intel_bb: Remove check-canonical test Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 29/31] tests/api_intel_bb: Use allocator in delta-check test Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 30/31] lib/intel_allocator: Separate allocator multiprocess start Zbigniew Kempczyński
2021-01-05  8:10 ` [igt-dev] [PATCH i-g-t v12 31/31] tests/api_intel_allocator: Prepare to run with sanitizer Zbigniew Kempczyński
2021-01-05  9:10 ` [igt-dev] ✓ Fi.CI.BAT: success for Introduce IGT allocator (rev13) Patchwork
2021-01-05 10:10 ` [igt-dev] ✓ Fi.CI.IGT: " 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=20210105081048.14389-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.