All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT
@ 2019-11-20 18:57 Zbigniew Kempczyński
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 18:57 UTC (permalink / raw)
  To: igt-dev

This is second version of LMEM enabling in IGT.

v2: Refactoring, addressing issues from previous review.
    Fixing bugs in tests.
v3: Changes according to the review comments.
    Adding gem_mmap__device_coherent(). 

Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

Lukasz Kalamarz (4):
  lib/i915/gem_mman: add mmap_offset support
  lib/i915/intel_memory_region: Add lib to manage memory regions
  tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
  tests/i915/gem_exec_basic: Iterate over all memory regions

 lib/Makefile.sources           |   2 +
 lib/i915/gem_mman.c            | 222 ++++++++++++++++++++-----
 lib/i915/gem_mman.h            |  42 ++++-
 lib/i915/intel_memory_region.c | 291 +++++++++++++++++++++++++++++++++
 lib/i915/intel_memory_region.h | 141 ++++++++++++++++
 lib/ioctl_wrappers.h           |   1 +
 lib/meson.build                |   1 +
 tests/Makefile.sources         |   3 +
 tests/i915/gem_exec_basic.c    | 109 +++++++-----
 tests/i915/gem_mmap_offset.c   | 158 ++++++++++++++++++
 tests/meson.build              |   1 +
 11 files changed, 892 insertions(+), 79 deletions(-)
 create mode 100644 lib/i915/intel_memory_region.c
 create mode 100644 lib/i915/intel_memory_region.h
 create mode 100644 tests/i915/gem_mmap_offset.c

-- 
2.23.0

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support
  2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
@ 2019-11-20 18:57 ` Zbigniew Kempczyński
  2019-11-20 19:41   ` Chris Wilson
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 18:57 UTC (permalink / raw)
  To: igt-dev

From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

With introduction of LMEM concept new IOCTL call were implemented
- gem_mmap_offset. This patch add support in IGT for it.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 lib/i915/gem_mman.c | 222 ++++++++++++++++++++++++++++++++++++--------
 lib/i915/gem_mman.h |  42 ++++++++-
 2 files changed, 224 insertions(+), 40 deletions(-)

diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
index 6256627b..1467b883 100644
--- a/lib/i915/gem_mman.c
+++ b/lib/i915/gem_mman.c
@@ -40,6 +40,39 @@
 #define VG(x) do {} while (0)
 #endif
 
+#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
+
+static bool gem_has_mmap_offset(int fd)
+{
+	int has_mmap_offset = 0;
+	struct drm_i915_getparam gp;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = LOCAL_I915_PARAM_MMAP_OFFSET_VERSION;
+	gp.value = &has_mmap_offset;
+	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+	return has_mmap_offset > 0;
+}
+
+void gem_require_mmap_offset(int i915)
+{
+	igt_require(gem_has_mmap_offset(i915));
+}
+
+static int gem_mmap_gtt_version(int fd)
+{
+	struct drm_i915_getparam gp;
+	int gtt_version = -1;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = I915_PARAM_MMAP_GTT_VERSION;
+	gp.value = &gtt_version;
+	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+	return gtt_version;
+}
+
 /**
  * __gem_mmap__gtt:
  * @fd: open i915 drm file descriptor
@@ -101,46 +134,63 @@ int gem_munmap(void *ptr, uint64_t size)
 	return ret;
 }
 
-bool gem_mmap__has_wc(int fd)
+bool __gem_mmap__has_wc(int fd)
 {
-	static int has_wc = -1;
-
-	if (has_wc == -1) {
-		struct drm_i915_getparam gp;
-		int mmap_version = -1;
-		int gtt_version = -1;
-
-		has_wc = 0;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_MMAP_GTT_VERSION;
-		gp.value = &gtt_version;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_MMAP_VERSION;
-		gp.value = &mmap_version;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-		/* Do we have the new mmap_ioctl with DOMAIN_WC? */
-		if (mmap_version >= 1 && gtt_version >= 2) {
-			struct drm_i915_gem_mmap arg;
-
-			/* Does this device support wc-mmaps ? */
-			memset(&arg, 0, sizeof(arg));
-			arg.handle = gem_create(fd, 4096);
-			arg.offset = 0;
-			arg.size = 4096;
-			arg.flags = I915_MMAP_WC;
-			has_wc = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg) == 0;
-			gem_close(fd, arg.handle);
-		}
-		errno = 0;
+	int has_wc = 0;
+
+	struct drm_i915_getparam gp;
+	int mmap_version = -1;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = I915_PARAM_MMAP_VERSION;
+	gp.value = &mmap_version;
+	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+	/* Do we have the mmap_ioctl with DOMAIN_WC? */
+	if (mmap_version >= 1 && gem_mmap_gtt_version(fd) >= 2) {
+		struct drm_i915_gem_mmap arg;
+
+		/* Does this device support wc-mmaps ? */
+		memset(&arg, 0, sizeof(arg));
+		arg.handle = gem_create(fd, 4096);
+		arg.offset = 0;
+		arg.size = 4096;
+		arg.flags = I915_MMAP_WC;
+		has_wc = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg) == 0;
+		gem_close(fd, arg.handle);
 	}
+	errno = 0;
 
 	return has_wc > 0;
 }
 
+bool __gem_mmap_offset__has_wc(int fd)
+{
+	int has_wc = 0;
+
+	if (gem_has_mmap_offset(fd)) {
+		struct local_i915_gem_mmap_offset arg;
+
+		/* Does this device support wc-mmaps ? */
+		memset(&arg, 0, sizeof(arg));
+		arg.handle = gem_create(fd, 4096);
+		arg.offset = 0;
+		arg.flags = LOCAL_I915_MMAP_OFFSET_WC;
+		has_wc = igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET,
+				   &arg) == 0;
+		gem_close(fd, arg.handle);
+	}
+
+	errno = 0;
+
+	return has_wc > 0;
+}
+
+bool gem_mmap__has_wc(int fd)
+{
+	return __gem_mmap_offset__has_wc(fd) || __gem_mmap__has_wc(fd);
+}
+
 /**
  * __gem_mmap:
  * @fd: open i915 drm file descriptor
@@ -157,8 +207,8 @@ bool gem_mmap__has_wc(int fd)
  *
  * Returns: A pointer to the created memory mapping, NULL on failure.
  */
-static void
-*__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned int prot, uint64_t flags)
+void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size,
+		 unsigned int prot, uint64_t flags)
 {
 	struct drm_i915_gem_mmap arg;
 
@@ -177,6 +227,43 @@ static void
 	return from_user_pointer(arg.addr_ptr);
 }
 
+/**
+ * __gem_mmap_offset:
+ * @fd: open i915 drm file descriptor
+ * @handle: gem buffer object handle
+ * @offset: offset in the gem buffer of the mmap arena
+ * @size: size of the mmap arena
+ * @prot: memory protection bits as used by mmap()
+ * @flags: flags used to determine caching
+ *
+ * Similar to __gem_mmap but use MMAP_OFFSET IOCTL.
+ *
+ * Returns: A pointer to the created memory mapping, NULL on failure.
+ */
+void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size,
+			unsigned int prot, uint64_t flags)
+{
+	struct local_i915_gem_mmap_offset arg;
+	void *ptr;
+
+	memset(&arg, 0, sizeof(arg));
+	arg.handle = handle;
+	arg.offset = offset;
+	arg.flags = flags;
+
+	if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET, &arg))
+		return NULL;
+
+	ptr = mmap64(0, size, prot, MAP_SHARED, fd, arg.offset);
+
+	if (ptr == MAP_FAILED)
+		ptr = NULL;
+	else
+		errno = 0;
+
+	return ptr;
+}
+
 /**
  * __gem_mmap__wc:
  * @fd: open i915 drm file descriptor
@@ -194,7 +281,12 @@ static void
  */
 void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
 {
-	return __gem_mmap(fd, handle, offset, size, prot, I915_MMAP_WC);
+	void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
+				      LOCAL_I915_MMAP_OFFSET_WC);
+	if (!ptr)
+		ptr = __gem_mmap(fd, handle, offset, size, prot, I915_MMAP_WC);
+
+	return ptr;
 }
 
 /**
@@ -205,14 +297,62 @@ void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, un
  * @size: size of the mmap arena
  * @prot: memory protection bits as used by mmap()
  *
- * Like __gem_mmap__wc() except we assert on failure.
+ * Try to __gem_mmap_wc(). Assert on failure.
  *
  * Returns: A pointer to the created memory mapping
  */
 void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
 {
 	void *ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
+
+	igt_assert(ptr);
+	return ptr;
+}
+
+/**
+ * __gem_mmap__device_coherent:
+ * @fd: open i915 drm file descriptor
+ * @handle: gem buffer object handle
+ * @offset: offset in the gem buffer of the mmap arena
+ * @size: size of the mmap arena
+ * @prot: memory protection bits as used by mmap()
+ *
+ * Try to __gem_mmap_offset, then __gem_mmap__wc(), then __gem_mmap__gtt().
+ *
+ * Returns: A pointer to the created memory mapping or NULL.
+ */
+void *__gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
+				  uint64_t size, unsigned prot)
+{
+	void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
+				      LOCAL_I915_MMAP_OFFSET_WC);
+	if (!ptr)
+		ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
+
+	if (!ptr)
+		ptr = __gem_mmap__gtt(fd, handle, size, prot);
+
+	return ptr;
+}
+
+/**
+ * gem_mmap__device_coherent:
+ * @fd: open i915 drm file descriptor
+ * @handle: gem buffer object handle
+ * @offset: offset in the gem buffer of the mmap arena
+ * @size: size of the mmap arena
+ * @prot: memory protection bits as used by mmap()
+ *
+ * Call __gem_mmap__device__coherent(), asserts on fail.
+ *
+ * Returns: A pointer to the created memory mapping.
+ */
+void *gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
+				uint64_t size, unsigned prot)
+{
+	void *ptr = gem_mmap__device_coherent(fd, handle, offset, size, prot);
 	igt_assert(ptr);
+
 	return ptr;
 }
 
@@ -248,7 +388,11 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
  */
 void *gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
 {
-	void *ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
+	void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
+				      LOCAL_I915_MMAP_OFFSET_WB);
+	if (!ptr)
+		ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
+
 	igt_assert(ptr);
 	return ptr;
 }
diff --git a/lib/i915/gem_mman.h b/lib/i915/gem_mman.h
index 096ff592..e774d5c8 100644
--- a/lib/i915/gem_mman.h
+++ b/lib/i915/gem_mman.h
@@ -25,12 +25,46 @@
 #ifndef GEM_MMAN_H
 #define GEM_MMAN_H
 
+#define LOCAL_I915_GEM_MMAP_OFFSET       DRM_I915_GEM_MMAP_GTT
+#define LOCAL_IOCTL_I915_GEM_MMAP_OFFSET         DRM_IOWR(DRM_COMMAND_BASE + \
+	LOCAL_I915_GEM_MMAP_OFFSET, struct local_i915_gem_mmap_offset)
+
+struct local_i915_gem_mmap_offset {
+	/** Handle for the object being mapped. */
+	__u32 handle;
+	__u32 pad;
+	/**
+	 * Fake offset to use for subsequent mmap call
+	 *
+	 * This is a fixed-size type for 32/64 compatibility.
+	 */
+	__u64 offset;
+	/**
+	 * Flags for extended behaviour.
+	 *
+	 * It is mandatory that either one of the _WC/_WB flags
+	 * should be passed here.
+	 */
+	__u64 flags;
+#define LOCAL_I915_MMAP_OFFSET_WC (1 << 0)
+#define LOCAL_I915_MMAP_OFFSET_WB (1 << 1)
+#define LOCAL_I915_MMAP_OFFSET_UC (1 << 2)
+#define LOCAL_I915_MMAP_OFFSET_FLAGS \
+	(LOCAL_I915_MMAP_OFFSET_WC | LOCAL_I915_MMAP_OFFSET_WB | LOCAL_I915_MMAP_OFFSET_UC)
+};
+
+void gem_require_mmap_offset(int i915);
+bool gem_mmap_has_gtt(int fd);
+
 void *gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot);
 void *gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
 
+bool __gem_mmap__has_wc(int fd);
+bool __gem_mmap_offset__has_wc(int fd);
 bool gem_mmap__has_wc(int fd);
 void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
-
+void *gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
+				uint64_t size, unsigned prot);
 #ifndef I915_GEM_DOMAIN_WC
 #define I915_GEM_DOMAIN_WC 0x80
 #endif
@@ -38,9 +72,15 @@ void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsi
 bool gem_has_mappable_ggtt(int i915);
 void gem_require_mappable_ggtt(int i915);
 
+void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size,
+		 unsigned int prot, uint64_t flags);
+void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size,
+			 unsigned int prot, uint64_t flags);
 void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot);
 void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
 void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
+void *__gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
+				  uint64_t size, unsigned prot);
 
 int gem_munmap(void *ptr, uint64_t size);
 
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
@ 2019-11-20 18:57 ` Zbigniew Kempczyński
  2019-11-20 19:43   ` Chris Wilson
  2019-11-20 19:45   ` Chris Wilson
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 18:57 UTC (permalink / raw)
  To: igt-dev

From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

LMEM series introduced concept of memory_regions. This patch implement
helper functions that allow user to manage them in more convenient way.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Signed-off-by: Zbigniew Kempczyński <lukasz.kalamarz@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 lib/Makefile.sources           |   2 +
 lib/i915/intel_memory_region.c | 291 +++++++++++++++++++++++++++++++++
 lib/i915/intel_memory_region.h | 141 ++++++++++++++++
 lib/ioctl_wrappers.h           |   1 +
 lib/meson.build                |   1 +
 5 files changed, 436 insertions(+)
 create mode 100644 lib/i915/intel_memory_region.c
 create mode 100644 lib/i915/intel_memory_region.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 9d1a4e06..87b9f146 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -17,6 +17,8 @@ lib_source_list =	 	\
 	i915/gem_mman.h	\
 	i915/gem_vm.c	\
 	i915/gem_vm.h	\
+	i915/intel_memory_region.c	\
+	i915/intel_memory_region.h	\
 	i915_3d.h		\
 	i915_reg.h		\
 	i915_pciids.h		\
diff --git a/lib/i915/intel_memory_region.c b/lib/i915/intel_memory_region.c
new file mode 100644
index 00000000..a6c5f2f9
--- /dev/null
+++ b/lib/i915/intel_memory_region.c
@@ -0,0 +1,291 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include <stdarg.h>
+#include <alloca.h>
+
+#include "intel_reg.h"
+#include "drmtest.h"
+#include "ioctl_wrappers.h"
+#include "igt_dummyload.h"
+#include "igt_gt.h"
+#include "intel_chipset.h"
+
+#include "i915/intel_memory_region.h"
+
+#define SZ_4K (4096)
+#define SZ_64K (65536)
+
+#define i915_query_items(fd, items, n_items) do { \
+		igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
+		errno = 0; \
+	} while (0)
+#define i915_query_items_err(fd, items, n_items, err) do { \
+		igt_assert_eq(__i915_query_items(fd, items, n_items), -err); \
+	} while (0)
+
+static int
+__i915_query(int fd, struct drm_i915_query *q)
+{
+	if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
+		return -errno;
+	return 0;
+}
+
+static int
+__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t n_items)
+{
+	struct drm_i915_query q = {
+		.num_items = n_items,
+		.items_ptr = to_user_pointer(items),
+	};
+	return __i915_query(fd, &q);
+}
+
+bool gem_has_query_support(int fd)
+{
+	struct drm_i915_query query = {};
+
+	return __i915_query(fd, &query) == 0;
+}
+
+const struct intel_memory_region intel_memory_regions[] = {
+	{ "SMEM", LOCAL_I915_SYSTEM_MEMORY, 0 },
+	{ "LMEM", LOCAL_I915_DEVICE_MEMORY, 0 },
+	{ NULL, 0, 0}
+};
+
+/**
+ *  gem_get_batch_size:
+ *  @fd: open i915 drm file descriptor
+ *  @region: region in which we want to create a batch
+ *
+ *  FIXME: Currently function assumes we have 64K on DEVICE and 4K
+ *  on SYSTEM memory. I know Chris is going to kill me for that
+ *  but I'll fix this when patch with memory region page size detection
+ *  will be merged.
+ */
+uint32_t gem_get_batch_size(int fd, uint32_t region)
+{
+	return IS_DEVICE_MEMORY_REGION(region) ? SZ_64K : SZ_4K;
+}
+
+/**
+ * gem_get_query_memory_regions:
+ * @fd: open i915 drm file descriptor
+ *
+ * This function wraps query mechanism for memory regions.
+ *
+ * Returns: Filled struct with available memory regions.
+ */
+struct local_i915_query_memory_region_info *gem_query_memory_regions(int fd)
+{
+	struct drm_i915_query_item item;
+	struct local_i915_query_memory_region_info *query_info;
+
+	memset(&item, 0, sizeof(item));
+	item.query_id = LOCAL_I915_QUERY_MEMREGION_INFO;
+	i915_query_items(fd, &item, 1);
+
+	query_info = calloc(1, item.length);
+
+	item.data_ptr = to_user_pointer(query_info);
+	i915_query_items(fd, &item, 1);
+
+	return query_info;
+}
+
+/**
+ * gem_get_lmem_region_count:
+ * @fd: open i915 drm file descriptor
+ *
+ * Helper function to check how many lmem regions are available on device.
+ *
+ * Returns: Number of found lmem regions.
+ */
+uint8_t gem_get_lmem_region_count(int fd)
+{
+	struct local_i915_query_memory_region_info *query_info;
+	uint8_t num_regions;
+	uint8_t lmem_regions = 0;
+
+	query_info = gem_query_memory_regions(fd);
+	num_regions = query_info->num_regions;
+
+	for (int i = 0; i < num_regions; i++) {
+		if (IS_DEVICE_MEMORY_REGION(query_info->regions[i].id))
+			lmem_regions += 1;
+	}
+	free(query_info);
+
+	return lmem_regions;
+}
+
+/**
+ * gem_has_lmem:
+ * @fd: open i915 drm file descriptor
+ *
+ * Helper function to check if lmem is available on device.
+ *
+ * Returns: True if at least one lmem region was found.
+ */
+bool gem_has_lmem(int fd)
+{
+	return gem_get_lmem_region_count(fd) > 0;
+}
+
+/**
+ * gem_query_has_memory_region:
+ * @query_info: query result of memory regions
+ * @region: region existance to check inside @query_info regions
+ *
+ * This function check existence of region in @query_info
+ *
+ * Returns: true if memory region was found. Otherwise false.
+ */
+bool gem_query_has_memory_region(struct local_i915_query_memory_region_info *query_info,
+				 uint32_t region)
+{
+	for (int i = 0; i < query_info->num_regions; i++)
+		if (query_info->regions[i].id == region)
+			return true;
+
+	return false;
+}
+
+/**
+ * gem_query_require_region:
+ * @query_info: query result of memory regions
+ * @region: region to check inside query
+ *
+ * Function lead to skipping test if @region doesn't exists in @query_info.
+ */
+void gem_query_require_region(struct local_i915_query_memory_region_info *query_info,
+			      uint32_t region)
+{
+	igt_require(gem_query_has_memory_region(query_info, region));
+}
+
+/**
+ * __gem_migrate_to_memory_regions:
+ * @fd: open i915 drm file descriptor
+ * @handle: buffer object handle
+ * @mem_regions: memory regions id array
+ * @size: memory regions array size
+ *
+ * Wrapper function on IOCTL_I915_GEM_OBJECT_SETPARAM. It sets object to be
+ * migrated into one of memory region specified in the array. Array contains
+ * memory regions in requested priority order - if no migration to first
+ * memory region is possible next one is selected and so on.
+ *
+ * Returns: errno
+ */
+static int __gem_migrate_to_memory_regions(int fd, int handle,
+					   uint32_t *mem_regions,
+					   uint32_t size)
+{
+	struct local_i915_gem_object_param obj;
+	int err = 0;
+
+	memset(&obj, 0, sizeof(obj));
+	obj.handle = handle;
+	obj.size = size;
+	obj.param = I915_PARAM_MEMORY_REGION | I915_OBJECT_PARAM;
+	obj.data = to_user_pointer(mem_regions);
+
+	if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_OBJECT_SETPARAM, &obj)) {
+		err = -errno;
+		errno = 0;
+	}
+
+	return err;
+}
+
+/**
+ * gem_migrate_to_memory_region:
+ * @fd: open i915 drm file descriptor
+ * @handle: handle to GEM bo
+ * @type: memory region type
+ * @instance: memory region instance
+ *
+ * This function wraps GEM_OBJECT_SETPARAM into user friendly version. Object
+ * which user pass @handle will be migrated to memory region, specified
+ * by @type and @instance.
+ *
+ * Returns: errno if error occurred.
+ */
+int gem_migrate_to_memory_region(int fd, int handle, uint32_t region)
+{
+	return __gem_migrate_to_memory_regions(fd, handle, &region, 1);
+}
+
+/**
+ * gem_migrate_to_lmem:
+ * @fd: open i915 drm file descriptor
+ * @handle: handle to object that user want to migrate to LMEM
+ *
+ * This function wraps GEM_OBJECT_SETPARAM into user friendly version. Object
+ * which user pass @handle will be migrated to LMEM.
+ *
+ * Returns: errno if error occurred.
+ */
+int gem_migrate_to_lmem(int fd, int handle)
+{
+	return gem_migrate_to_memory_region(fd, handle, REGION_DEVICE_MEMORY(0));
+}
+
+/**
+ * gem_migrate_to_smem:
+ * @fd: open i915 drm file descriptor
+ * @handle: handle to object that user want to migrate to SMEM
+ *
+ * This function wraps GEM_OBJECT_SETPARAM into user friendly version. Object
+ * onto which user pass @handle will be migrated to SMEM.
+ *
+ * Returns: errno if error occurred.
+ */
+int gem_migrate_to_smem(int fd, int handle)
+{
+	return gem_migrate_to_memory_region(fd, handle, REGION_SYSTEM_MEMORY(0));
+}
+
+/* gem_create_in_memory_region_list:
+ * @fd: opened i915 drm file descriptor
+ * @size: requested size of the buffer
+ * @mem_regions: memory regions array (priority list)
+ * @num_regions: @mem_regions length
+ */
+uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
+					  uint32_t *mem_regions,
+					  int num_regions)
+{
+	uint32_t handle = gem_create(fd, size);
+	int ret = __gem_migrate_to_memory_regions(fd, handle,
+						  mem_regions, num_regions);
+	igt_assert_eq(ret, 0);
+
+	return handle;
+}
diff --git a/lib/i915/intel_memory_region.h b/lib/i915/intel_memory_region.h
new file mode 100644
index 00000000..4603cd02
--- /dev/null
+++ b/lib/i915/intel_memory_region.h
@@ -0,0 +1,141 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef INTEL_MEMORY_REGION_H
+#define INTEL_MEMORY_REGION_H
+
+#include <stdint.h>
+#include "igt_aux.h"
+#include "i915_drm.h"
+
+#define INTEL_MEMORY_TYPE_SHIFT 16
+
+#define INTEL_MEMORY_REGION_ID(type, instance) \
+			((BIT((type) + INTEL_MEMORY_TYPE_SHIFT)) | BIT(instance))
+
+#define MEMORY_TYPE_FROM_REGION(r) (igt_fls(r >> INTEL_MEMORY_TYPE_SHIFT) - 1)
+#define MEMORY_INSTANCE_FROM_REGION(r) (igt_fls(r & 0xffff) - 1)
+
+#define IS_DEVICE_MEMORY_REGION(region) \
+	(MEMORY_TYPE_FROM_REGION(region) == LOCAL_I915_DEVICE_MEMORY)
+#define IS_SYSTEM_MEMORY_REGION(region) \
+	(MEMORY_TYPE_FROM_REGION(region) == LOCAL_I915_SYSTEM_MEMORY)
+
+/* Region macros for migration */
+#define REGION_SYSTEM_MEMORY(n) INTEL_MEMORY_REGION_ID(LOCAL_I915_SYSTEM_MEMORY, n)
+#define REGION_DEVICE_MEMORY(n) INTEL_MEMORY_REGION_ID(LOCAL_I915_DEVICE_MEMORY, n)
+
+#define LOCAL_I915_QUERY_MEMREGION_INFO   4
+struct local_i915_memory_region_info {
+
+	/** Base type of a region */
+#define LOCAL_I915_SYSTEM_MEMORY         0
+#define LOCAL_I915_DEVICE_MEMORY         1
+
+	/** The region id is encoded in a layout which makes it possible to
+	 * retrieve the following information:
+	 *
+	 *  Base type: log2(ID >> 16)
+	 */
+	__u32 id;
+
+	/** Reserved field. MBZ */
+	__u32 rsvd0;
+
+	/** Unused for now. MBZ */
+	__u64 flags;
+
+	__u64 size;
+
+	/** Reserved fields must be cleared to zero. */
+	__u64 rsvd1[4];
+};
+
+struct local_i915_query_memory_region_info {
+
+	/** Number of struct drm_i915_memory_region_info structs */
+	__u32 num_regions;
+
+	/** MBZ */
+	__u32 rsvd[3];
+
+	struct local_i915_memory_region_info regions[];
+};
+
+struct local_i915_gem_object_param {
+	/** Handle for the object */
+	__u32 handle;
+
+	__u32 size;
+
+	/** Set the memory region for the object listed in preference order
+	 *  as an array of region ids within data. To force an object
+	 *  to a particular memory region, set the region as the sole entry.
+	 *
+	 *  Valid region ids are derived from the id field of
+	 *  struct drm_i915_memory_region_info.
+	 *  See struct drm_i915_query_memory_region_info.
+	 */
+#define I915_OBJECT_PARAM  (1ull<<32)
+#define I915_PARAM_MEMORY_REGION 0x1
+	__u64 param;
+
+	__u64 data;
+};
+
+#define LOCAL_I915_GEM_OBJECT_SETPARAM	DRM_I915_GEM_CONTEXT_SETPARAM
+#define LOCAL_IOCTL_I915_GEM_OBJECT_SETPARAM	DRM_IOWR(DRM_COMMAND_BASE \
+	+ LOCAL_I915_GEM_OBJECT_SETPARAM, struct local_i915_gem_object_param)
+
+extern const struct intel_memory_region {
+	const char *region_name;
+	uint8_t memory_type;
+	uint8_t memory_instance;
+} intel_memory_regions[];
+
+bool gem_has_query_support(int fd);
+
+uint32_t gem_get_batch_size(int fd, uint32_t region);
+
+struct local_i915_query_memory_region_info *gem_query_memory_regions(int fd);
+
+uint8_t gem_get_lmem_region_count(int fd);
+
+bool gem_has_lmem(int fd);
+bool gem_query_has_memory_region(struct local_i915_query_memory_region_info *query_info,
+				 uint32_t region);
+void gem_query_require_region(struct local_i915_query_memory_region_info *query_info,
+			      uint32_t region);
+int gem_migrate_to_memory_region(int fd, int handle, uint32_t region);
+int gem_migrate_to_lmem(int fd, int handle);
+int gem_migrate_to_smem(int fd, int handle);
+
+uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
+					  uint32_t *mem_regions,
+					  int num_regions);
+#define gem_create_in_memory_regions(fd, size, regions...) ({ \
+	unsigned int arr__[] = { regions }; \
+	gem_create_in_memory_region_list(fd, size, arr__, ARRAY_SIZE(arr__)); \
+})
+
+#endif /* INTEL_MEMORY_REGION_H */
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index f2412d78..885cbb06 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -38,6 +38,7 @@
 
 #include "i915/gem_context.h"
 #include "i915/gem_scheduler.h"
+#include "i915/intel_memory_region.h"
 
 /**
  * igt_ioctl:
diff --git a/lib/meson.build b/lib/meson.build
index 3f908912..7a155ae7 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -7,6 +7,7 @@ lib_sources = [
 	'i915/gem_ring.c',
 	'i915/gem_mman.c',
 	'i915/gem_vm.c',
+	'i915/intel_memory_region.c',
 	'igt_color_encoding.c',
 	'igt_debugfs.c',
 	'igt_device.c',
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
  2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
@ 2019-11-20 18:57 ` Zbigniew Kempczyński
  2019-11-20 19:50   ` Chris Wilson
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions Zbigniew Kempczyński
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 18:57 UTC (permalink / raw)
  To: igt-dev

From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

This test is a copy/paste of few gem_mmap subtests, due to good
coverage in previous one. We also need to be sure that we cover
all available memory regions.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/Makefile.sources       |   3 +
 tests/i915/gem_mmap_offset.c | 158 +++++++++++++++++++++++++++++++++++
 tests/meson.build            |   1 +
 3 files changed, 162 insertions(+)
 create mode 100644 tests/i915/gem_mmap_offset.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 27801c89..9c6c3933 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -320,6 +320,9 @@ gem_mmap_SOURCES = i915/gem_mmap.c
 TESTS_progs += gem_mmap_gtt
 gem_mmap_gtt_SOURCES = i915/gem_mmap_gtt.c
 
+TESTS_progs += gem_mmap_offset
+gem_mmap_offset_SOURCES = i915/gem_mmap_offset.c
+
 TESTS_progs += gem_mmap_offset_exhaustion
 gem_mmap_offset_exhaustion_SOURCES = i915/gem_mmap_offset_exhaustion.c
 
diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
new file mode 100644
index 00000000..5215d82e
--- /dev/null
+++ b/tests/i915/gem_mmap_offset.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include "drm.h"
+
+IGT_TEST_DESCRIPTION("Basic MMAP_OFFSET IOCTL tests for mem regions\n");
+
+static int mmap_offset_ioctl(int fd, struct local_i915_gem_mmap_offset *arg)
+{
+	int err = 0;
+
+	if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET, arg))
+		err = -errno;
+
+	errno = 0;
+	return err;
+}
+
+igt_main
+{
+	uint8_t *addr;
+	uint32_t obj_size, batch_size;
+	uint32_t mem_type, mem_instance;
+	uint32_t region;
+	int fd;
+	const struct intel_memory_region *mr;
+	struct local_i915_query_memory_region_info *query_info;
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_INTEL);
+		gem_require_mmap_offset(fd);
+		igt_require(gem_mmap__has_wc(fd));
+
+		query_info = gem_query_memory_regions(fd);
+		igt_assert(query_info);
+	}
+
+	for (mr = intel_memory_regions; mr->region_name; mr++) {
+		mem_type = mr->memory_type;
+		mem_instance = mr->memory_instance;
+		region = INTEL_MEMORY_REGION_ID(mem_type, mem_instance);
+		batch_size = gem_get_batch_size(fd, region);
+		obj_size = 4 * batch_size;
+
+		igt_subtest_f("bad-object-%s", mr->region_name) {
+			uint32_t real_handle;
+			uint32_t handles[20];
+			int i = 0;
+
+			gem_query_require_region(query_info, region);
+
+			real_handle = gem_create_in_memory_regions(fd, obj_size,
+								   region);
+
+			handles[i++] = 0xdeadbeef;
+			for (int bit = 0; bit < 16; bit++)
+				handles[i++] = real_handle | (1 << (bit + 16));
+			handles[i] = real_handle + 1;
+
+			for (; i >= 0; i--) {
+				struct local_i915_gem_mmap_offset arg = {
+					.handle = handles[i],
+					.flags = LOCAL_I915_MMAP_OFFSET_WC,
+				};
+
+				igt_debug("Trying MMAP IOCTL with handle %x\n",
+					  handles[i]);
+				igt_assert_eq(mmap_offset_ioctl(fd, &arg),
+					      -ENOENT);
+			}
+
+			gem_close(fd, real_handle);
+		}
+
+		igt_subtest_f("basic-%s", mr->region_name) {
+			uint32_t handle;
+			uint64_t flags;
+			uint8_t *expected;
+			uint8_t *buf;
+
+			gem_query_require_region(query_info, region);
+
+			handle = gem_create_in_memory_regions(fd, obj_size,
+							      region);
+			flags = LOCAL_I915_MMAP_OFFSET_WC;
+			addr = __gem_mmap_offset(fd, handle, 0, obj_size,
+						 PROT_READ | PROT_WRITE,
+						 flags);
+			igt_assert(addr);
+
+			igt_debug("Testing contents of newly created object.\n");
+			expected = calloc(obj_size, sizeof(*expected));
+			igt_assert_eq(memcmp(addr, expected, obj_size), 0);
+			free(expected);
+
+			igt_debug("Testing coherency of writes and mmap reads.\n");
+			buf = calloc(obj_size, sizeof(*buf));
+			memset(buf + 1024, 0x01, 1024);
+			gem_write(fd, handle, 0, buf, obj_size);
+			igt_assert_eq(memcmp(buf, addr, obj_size), 0);
+
+			igt_debug("Testing that mapping stays after close\n");
+			gem_close(fd, handle);
+			igt_assert_eq(memcmp(buf, addr, obj_size), 0);
+			free(buf);
+
+			igt_debug("Testing unmapping\n");
+			munmap(addr, obj_size);
+		}
+
+		igt_subtest_f("short-mmap-%s", mr->region_name) {
+			uint32_t handle;
+
+			gem_query_require_region(query_info, region);
+
+			handle = gem_create_in_memory_regions(fd, obj_size,
+							      region);
+			igt_assert(obj_size > batch_size);
+
+			addr = gem_mmap__wc(fd, handle, 0, batch_size,
+					    PROT_WRITE);
+			igt_assert(addr);
+			memset(addr, 0, batch_size);
+			munmap(addr, batch_size);
+
+			gem_close(fd, handle);
+		}
+	}
+
+	igt_fixture {
+		free(query_info);
+		close(fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 755fc9e6..644b5504 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -174,6 +174,7 @@ i915_progs = [
 	'gem_media_vme',
 	'gem_mmap',
 	'gem_mmap_gtt',
+	'gem_mmap_offset',
 	'gem_mmap_offset_exhaustion',
 	'gem_mmap_wc',
 	'gem_partial_pwrite_pread',
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions
  2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
                   ` (2 preceding siblings ...)
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
@ 2019-11-20 18:57 ` Zbigniew Kempczyński
  2019-11-20 19:53   ` Chris Wilson
  2019-11-20 19:26 ` [igt-dev] ✗ GitLab.Pipeline: failure for Basic LMEM support in IGT (rev3) Patchwork
  2019-11-20 19:35 ` [igt-dev] ✗ Fi.CI.BAT: " Patchwork
  5 siblings, 1 reply; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 18:57 UTC (permalink / raw)
  To: igt-dev

From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

As a part of local memory effort we need to make sure, that basic
scenarios are covered for every available memory region. This patch is
an attempt for this problem. If it will be accepted it will be
replicated on each test that can benefit from it.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/i915/gem_exec_basic.c | 109 +++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 39 deletions(-)

diff --git a/tests/i915/gem_exec_basic.c b/tests/i915/gem_exec_basic.c
index 1287860b..4a444916 100644
--- a/tests/i915/gem_exec_basic.c
+++ b/tests/i915/gem_exec_basic.c
@@ -25,12 +25,12 @@
 
 IGT_TEST_DESCRIPTION("Basic sanity check of execbuf-ioctl rings.");
 
-static uint32_t batch_create(int fd)
+static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
 {
 	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	uint32_t handle;
 
-	handle = gem_create(fd, 4096);
+	handle = gem_create_in_memory_regions(fd, batch_size, region);
 	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
 
 	return handle;
@@ -42,7 +42,7 @@ static void batch_fini(int fd, uint32_t handle)
 	gem_close(fd, handle);
 }
 
-static void noop(int fd, uint64_t flags)
+static void noop(int fd, uint64_t flags, uint32_t batch_size, uint32_t region)
 {
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_exec_object2 exec;
@@ -50,8 +50,7 @@ static void noop(int fd, uint64_t flags)
 	gem_require_ring(fd, flags);
 
 	memset(&exec, 0, sizeof(exec));
-
-	exec.handle = batch_create(fd);
+	exec.handle = batch_create(fd, batch_size, region);
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(&exec);
@@ -62,7 +61,8 @@ static void noop(int fd, uint64_t flags)
 	batch_fini(fd, exec.handle);
 }
 
-static void readonly(int fd, uint64_t flags)
+static void readonly(int fd, uint64_t flags, uint32_t batch_size,
+		     uint32_t region)
 {
 	struct drm_i915_gem_execbuffer2 *execbuf;
 	struct drm_i915_gem_exec_object2 exec;
@@ -70,39 +70,41 @@ static void readonly(int fd, uint64_t flags)
 	gem_require_ring(fd, flags);
 
 	memset(&exec, 0, sizeof(exec));
-	exec.handle = batch_create(fd);
+	exec.handle = batch_create(fd, batch_size, region);
 
-	execbuf = mmap(NULL, 4096, PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+	execbuf = mmap(NULL, batch_size, PROT_WRITE,
+		       MAP_ANON | MAP_PRIVATE, -1, 0);
 	igt_assert(execbuf != NULL);
 
 	execbuf->buffers_ptr = to_user_pointer(&exec);
 	execbuf->buffer_count = 1;
 	execbuf->flags = flags;
-	igt_assert(mprotect(execbuf, 4096, PROT_READ) == 0);
+	igt_assert(mprotect(execbuf, batch_size, PROT_READ) == 0);
 
 	gem_execbuf(fd, execbuf);
 
-	munmap(execbuf, 4096);
-
+	munmap(execbuf, batch_size);
 	batch_fini(fd, exec.handle);
 }
 
-static void gtt(int fd, uint64_t flags)
+static void gtt(int fd, uint64_t flags, uint32_t batch_size, uint32_t region)
 {
 	struct drm_i915_gem_execbuffer2 *execbuf;
 	struct drm_i915_gem_exec_object2 *exec;
 	uint32_t handle;
 
 	gem_require_ring(fd, flags);
+	gem_require_mappable_ggtt(fd);
+	igt_require(IS_SYSTEM_MEMORY_REGION(region));
 
-	handle = gem_create(fd, 4096);
-
+	handle = gem_create_in_memory_regions(fd, handle, region);
 	gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-	execbuf = gem_mmap__gtt(fd, handle, 4096, PROT_WRITE);
+
+	execbuf = gem_mmap__gtt(fd, handle, batch_size, PROT_WRITE);
 	exec = (struct drm_i915_gem_exec_object2 *)(execbuf + 1);
 	gem_close(fd, handle);
 
-	exec->handle = batch_create(fd);
+	exec->handle = batch_create(fd, batch_size, region);
 
 	execbuf->buffers_ptr = to_user_pointer(exec);
 	execbuf->buffer_count = 1;
@@ -111,36 +113,41 @@ static void gtt(int fd, uint64_t flags)
 	gem_execbuf(fd, execbuf);
 
 	batch_fini(fd, exec->handle);
-	munmap(execbuf, 4096);
+	munmap(execbuf, batch_size);
 }
 
-static void all(int i915)
+static void all(int i915, uint32_t batch_size, uint32_t region)
 {
 	const struct intel_execution_engine2 *e;
 
 	__for_each_physical_engine(i915, e)
-		noop(i915, e->flags);
+		noop(i915, e->flags, batch_size, region);
 }
 
-static void readonly_all(int i915)
+static void readonly_all(int i915, uint32_t batch_size, uint32_t region)
 {
 	const struct intel_execution_engine2 *e;
 
 	__for_each_physical_engine(i915, e)
-		readonly(i915, e->flags);
+		readonly(i915, e->flags, batch_size, region);
 }
 
-static void gtt_all(int i915)
+static void gtt_all(int i915, uint32_t batch_size, uint32_t region)
 {
 	const struct intel_execution_engine2 *e;
 
 	__for_each_physical_engine(i915, e)
-		gtt(i915, e->flags);
+		gtt(i915, e->flags, batch_size, region);
 }
 
 igt_main
 {
 	const struct intel_execution_engine2 *e;
+	const struct intel_memory_region *mr;
+	struct local_i915_query_memory_region_info *query_info;
+	uint32_t mem_type, mem_instance;
+	uint32_t batch_size;
+	uint32_t region;
 	int fd = -1;
 
 	igt_fixture {
@@ -148,27 +155,51 @@ igt_main
 		igt_require_gem(fd);
 
 		igt_fork_hang_detector(fd);
-	}
-
-	igt_subtest("basic-all")
-		all(fd);
 
-	igt_subtest("readonly-all")
-		readonly_all(fd);
-
-	igt_subtest("gtt-all")
-		gtt_all(fd);
+		query_info = gem_query_memory_regions(fd);
+		igt_assert(query_info);
+	}
 
-	__for_each_physical_engine(fd, e) {
-		igt_subtest_f("basic-%s", e->name)
-			noop(fd, e->flags);
-		igt_subtest_f("readonly-%s", e->name)
-			readonly(fd, e->flags);
-		igt_subtest_f("gtt-%s", e->name)
-			gtt(fd, e->flags);
+	for (mr = intel_memory_regions; mr->region_name; mr++) {
+		mem_type = mr->memory_type;
+		mem_instance = mr->memory_instance;
+		region = INTEL_MEMORY_REGION_ID(mem_type, mem_instance);
+
+		batch_size = gem_get_batch_size(fd, region);
+
+		igt_subtest_f("basic-%s-all", mr->region_name) {
+			gem_query_require_region(query_info, region);
+			all(fd, batch_size, region);
+		}
+
+		igt_subtest_f("readonly-%s-all", mr->region_name) {
+			gem_query_require_region(query_info, region);
+			readonly_all(fd, batch_size, region);
+		}
+
+		igt_subtest_f("gtt-%s-all", mr->region_name) {
+			gem_query_require_region(query_info, region);
+			gtt_all(fd, batch_size, region);
+		}
+
+		__for_each_physical_engine(fd, e) {
+			igt_subtest_f("basic-%s-%s", mr->region_name, e->name) {
+				gem_query_require_region(query_info, region);
+				noop(fd, e->flags, batch_size, region);
+			}
+			igt_subtest_f("readonly-%s-%s", mr->region_name, e->name) {
+				gem_query_require_region(query_info, region);
+				readonly(fd, e->flags, batch_size, region);
+			}
+			igt_subtest_f("gtt-%s-%s", mr->region_name, e->name) {
+				gem_query_require_region(query_info, region);
+				gtt(fd, e->flags, batch_size, region);
+			}
+		}
 	}
 
 	igt_fixture {
+		free(query_info);
 		igt_stop_hang_detector();
 		close(fd);
 	}
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] ✗ GitLab.Pipeline: failure for Basic LMEM support in IGT (rev3)
  2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
                   ` (3 preceding siblings ...)
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions Zbigniew Kempczyński
@ 2019-11-20 19:26 ` Patchwork
  2019-11-20 19:35 ` [igt-dev] ✗ Fi.CI.BAT: " Patchwork
  5 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-11-20 19:26 UTC (permalink / raw)
  To: Lukasz Kalamarz; +Cc: igt-dev

== Series Details ==

Series: Basic LMEM support in IGT (rev3)
URL   : https://patchwork.freedesktop.org/series/65171/
State : failure

== Summary ==

ERROR! This series introduces new undocumented tests:

gem_exec_basic@basic-LMEM-all
gem_exec_basic@basic-LMEM-bcs0
gem_exec_basic@basic-LMEM-rcs0
gem_exec_basic@basic-LMEM-vcs0
gem_exec_basic@basic-LMEM-vcs1
gem_exec_basic@basic-LMEM-vcs2
gem_exec_basic@basic-LMEM-vecs0
gem_exec_basic@basic-SMEM-all
gem_exec_basic@basic-SMEM-bcs0
gem_exec_basic@basic-SMEM-rcs0
gem_exec_basic@basic-SMEM-vcs0
gem_exec_basic@basic-SMEM-vcs1
gem_exec_basic@basic-SMEM-vcs2
gem_exec_basic@basic-SMEM-vecs0
gem_exec_basic@gtt-LMEM-all
gem_exec_basic@gtt-LMEM-bcs0
gem_exec_basic@gtt-LMEM-rcs0
gem_exec_basic@gtt-LMEM-vcs0
gem_exec_basic@gtt-LMEM-vcs1
gem_exec_basic@gtt-LMEM-vcs2
gem_exec_basic@gtt-LMEM-vecs0
gem_exec_basic@gtt-SMEM-all
gem_exec_basic@gtt-SMEM-bcs0
gem_exec_basic@gtt-SMEM-rcs0
gem_exec_basic@gtt-SMEM-vcs0
gem_exec_basic@gtt-SMEM-vcs1
gem_exec_basic@gtt-SMEM-vcs2
gem_exec_basic@gtt-SMEM-vecs0
gem_exec_basic@readonly-LMEM-all
gem_exec_basic@readonly-LMEM-bcs0
gem_exec_basic@readonly-LMEM-rcs0
gem_exec_basic@readonly-LMEM-vcs0
gem_exec_basic@readonly-LMEM-vcs1
gem_exec_basic@readonly-LMEM-vcs2
gem_exec_basic@readonly-LMEM-vecs0
gem_exec_basic@readonly-SMEM-all
gem_exec_basic@readonly-SMEM-bcs0
gem_exec_basic@readonly-SMEM-rcs0
gem_exec_basic@readonly-SMEM-vcs0
gem_exec_basic@readonly-SMEM-vcs1
gem_exec_basic@readonly-SMEM-vcs2
gem_exec_basic@readonly-SMEM-vecs0
gem_mmap_offset@bad-object-LMEM
gem_mmap_offset@bad-object-SMEM
gem_mmap_offset@basic-LMEM
gem_mmap_offset@basic-SMEM
gem_mmap_offset@short-mmap-LMEM
gem_mmap_offset@short-mmap-SMEM

Can you document them as per the requirement in the [CONTRIBUTING.md]?

[Documentation] has more details on how to do this.

Here are few examples:
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/0316695d03aa46108296b27f3982ec93200c7a6e
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/443cc658e1e6b492ee17bf4f4d891029eb7a205d

Thanks in advance!

[CONTRIBUTING.md]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/CONTRIBUTING.md#L19
[Documentation]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe

Other than that, pipeline status: SUCCESS.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/81050 for the overview.

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/81050
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for Basic LMEM support in IGT (rev3)
  2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
                   ` (4 preceding siblings ...)
  2019-11-20 19:26 ` [igt-dev] ✗ GitLab.Pipeline: failure for Basic LMEM support in IGT (rev3) Patchwork
@ 2019-11-20 19:35 ` Patchwork
  5 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-11-20 19:35 UTC (permalink / raw)
  To: Lukasz Kalamarz; +Cc: igt-dev

== Series Details ==

Series: Basic LMEM support in IGT (rev3)
URL   : https://patchwork.freedesktop.org/series/65171/
State : failure

== Summary ==

CI Bug Log - changes from IGT_5299 -> IGTPW_3737
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_3737 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3737, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_3737:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_basic@basic-all:
    - fi-icl-u2:          [PASS][1] -> [SKIP][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u2/igt@gem_exec_basic@basic-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u2/igt@gem_exec_basic@basic-all.html
    - fi-cml-u2:          [PASS][3] -> [SKIP][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cml-u2/igt@gem_exec_basic@basic-all.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cml-u2/igt@gem_exec_basic@basic-all.html
    - fi-icl-u3:          [PASS][5] -> [SKIP][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u3/igt@gem_exec_basic@basic-all.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u3/igt@gem_exec_basic@basic-all.html
    - fi-cml-s:           [PASS][7] -> [SKIP][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cml-s/igt@gem_exec_basic@basic-all.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cml-s/igt@gem_exec_basic@basic-all.html
    - fi-icl-y:           [PASS][9] -> [SKIP][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-y/igt@gem_exec_basic@basic-all.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-y/igt@gem_exec_basic@basic-all.html
    - fi-icl-dsi:         [PASS][11] -> [SKIP][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-dsi/igt@gem_exec_basic@basic-all.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-dsi/igt@gem_exec_basic@basic-all.html
    - fi-icl-guc:         [PASS][13] -> [SKIP][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-guc/igt@gem_exec_basic@basic-all.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-guc/igt@gem_exec_basic@basic-all.html

  * igt@gem_exec_gttfill@basic:
    - fi-bsw-kefka:       [PASS][15] -> [TIMEOUT][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bsw-kefka/igt@gem_exec_gttfill@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bsw-kefka/igt@gem_exec_gttfill@basic.html
    - fi-ivb-3770:        [PASS][17] -> [DMESG-FAIL][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-ivb-3770/igt@gem_exec_gttfill@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-ivb-3770/igt@gem_exec_gttfill@basic.html
    - fi-bsw-n3050:       [PASS][19] -> [TIMEOUT][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bsw-n3050/igt@gem_exec_gttfill@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bsw-n3050/igt@gem_exec_gttfill@basic.html
    - fi-snb-2600:        [PASS][21] -> [DMESG-FAIL][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-snb-2600/igt@gem_exec_gttfill@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-snb-2600/igt@gem_exec_gttfill@basic.html
    - fi-hsw-4770r:       [PASS][23] -> [TIMEOUT][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-hsw-4770r/igt@gem_exec_gttfill@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-hsw-4770r/igt@gem_exec_gttfill@basic.html
    - fi-ilk-650:         [PASS][25] -> [DMESG-FAIL][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-ilk-650/igt@gem_exec_gttfill@basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-ilk-650/igt@gem_exec_gttfill@basic.html

  * igt@gem_mmap_gtt@basic-small-bo-tiledx:
    - fi-hsw-4770:        [PASS][27] -> [FAIL][28] +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-hsw-4770/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-hsw-4770/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
    - fi-blb-e6850:       [PASS][29] -> [FAIL][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-blb-e6850/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-blb-e6850/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
    - fi-snb-2520m:       [PASS][31] -> [FAIL][32] +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-snb-2520m/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-snb-2520m/igt@gem_mmap_gtt@basic-small-bo-tiledx.html

  * igt@gem_mmap_gtt@basic-small-bo-tiledy:
    - fi-bwr-2160:        [PASS][33] -> [FAIL][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bwr-2160/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bwr-2160/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-kbl-guc:         [PASS][35] -> [FAIL][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-bsw-nick:        [PASS][37] -> [FAIL][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bsw-nick/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bsw-nick/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-kbl-r:           [PASS][39] -> [FAIL][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-r/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-r/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-kbl-soraka:      [PASS][41] -> [FAIL][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-soraka/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-soraka/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-icl-u3:          [PASS][43] -> [FAIL][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-skl-6770hq:      [PASS][45] -> [FAIL][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-skl-6770hq/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-skl-6770hq/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-cfl-guc:         [PASS][47] -> [FAIL][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cfl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cfl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-byt-j1900:       [PASS][49] -> [FAIL][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-byt-j1900/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-byt-j1900/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-skl-6700k2:      [PASS][51] -> [FAIL][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-skl-6700k2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-skl-6700k2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-byt-n2820:       [PASS][53] -> [FAIL][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-byt-n2820/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-byt-n2820/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-cfl-8700k:       [PASS][55] -> [FAIL][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cfl-8700k/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cfl-8700k/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-apl-guc:         [PASS][57] -> [FAIL][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-apl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-apl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-icl-dsi:         [PASS][59] -> [FAIL][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-bdw-5557u:       [PASS][61] -> [FAIL][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bdw-5557u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bdw-5557u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-pnv-d510:        [PASS][63] -> [FAIL][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-pnv-d510/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-pnv-d510/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-skl-6600u:       [PASS][65] -> [FAIL][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-skl-6600u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-skl-6600u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-skl-guc:         [PASS][67] -> [FAIL][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-skl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-skl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-icl-y:           [PASS][69] -> [FAIL][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-y/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-y/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-kbl-7500u:       [PASS][71] -> [FAIL][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-7500u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-7500u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-hsw-peppy:       [PASS][73] -> [FAIL][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-hsw-peppy/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-hsw-peppy/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-kbl-8809g:       [PASS][75] -> [FAIL][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-8809g/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-8809g/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-glk-dsi:         [PASS][77] -> [FAIL][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-glk-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-glk-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-kbl-x1275:       [PASS][79] -> [FAIL][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-x1275/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-x1275/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-cml-u2:          [PASS][81] -> [FAIL][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cml-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cml-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-icl-u2:          [PASS][83] -> [FAIL][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-icl-guc:         [PASS][85] -> [FAIL][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-bxt-dsi:         [PASS][87] -> [FAIL][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bxt-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bxt-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
    - fi-whl-u:           [PASS][89] -> [FAIL][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-whl-u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-whl-u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html

  * igt@gem_workarounds@basic-read:
    - fi-icl-dsi:         [PASS][91] -> [TIMEOUT][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-dsi/igt@gem_workarounds@basic-read.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-dsi/igt@gem_workarounds@basic-read.html
    - fi-cml-u2:          [PASS][93] -> [TIMEOUT][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cml-u2/igt@gem_workarounds@basic-read.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cml-u2/igt@gem_workarounds@basic-read.html
    - fi-icl-y:           [PASS][95] -> [TIMEOUT][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-y/igt@gem_workarounds@basic-read.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-y/igt@gem_workarounds@basic-read.html
    - fi-icl-u3:          [PASS][97] -> [TIMEOUT][98]
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u3/igt@gem_workarounds@basic-read.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u3/igt@gem_workarounds@basic-read.html
    - fi-glk-dsi:         [PASS][99] -> [CRASH][100] +1 similar issue
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-glk-dsi/igt@gem_workarounds@basic-read.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-glk-dsi/igt@gem_workarounds@basic-read.html
    - fi-icl-guc:         [PASS][101] -> [TIMEOUT][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-guc/igt@gem_workarounds@basic-read.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-guc/igt@gem_workarounds@basic-read.html
    - fi-icl-u2:          [PASS][103] -> [TIMEOUT][104]
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u2/igt@gem_workarounds@basic-read.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u2/igt@gem_workarounds@basic-read.html
    - fi-bsw-nick:        [PASS][105] -> [CRASH][106]
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bsw-nick/igt@gem_workarounds@basic-read.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bsw-nick/igt@gem_workarounds@basic-read.html
    - fi-bdw-5557u:       [PASS][107] -> [TIMEOUT][108]
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bdw-5557u/igt@gem_workarounds@basic-read.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bdw-5557u/igt@gem_workarounds@basic-read.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          [PASS][109] -> [CRASH][110]
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
    - fi-kbl-soraka:      [PASS][111] -> [CRASH][112]
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-soraka/igt@kms_frontbuffer_tracking@basic.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-soraka/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-guc:         [PASS][113] -> [CRASH][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html
    - fi-bxt-dsi:         [PASS][115] -> [CRASH][116] +1 similar issue
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bxt-dsi/igt@kms_frontbuffer_tracking@basic.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bxt-dsi/igt@kms_frontbuffer_tracking@basic.html
    - fi-skl-6700k2:      [PASS][117] -> [CRASH][118]
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-skl-6700k2/igt@kms_frontbuffer_tracking@basic.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-skl-6700k2/igt@kms_frontbuffer_tracking@basic.html
    - fi-cml-u2:          [PASS][119] -> [CRASH][120]
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
    - fi-apl-guc:         [PASS][121] -> [CRASH][122] +1 similar issue
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-apl-guc/igt@kms_frontbuffer_tracking@basic.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-apl-guc/igt@kms_frontbuffer_tracking@basic.html
    - fi-byt-j1900:       [PASS][123] -> [CRASH][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-byt-j1900/igt@kms_frontbuffer_tracking@basic.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-byt-j1900/igt@kms_frontbuffer_tracking@basic.html
    - fi-hsw-4770:        [PASS][125] -> [CRASH][126]
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-hsw-4770/igt@kms_frontbuffer_tracking@basic.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-hsw-4770/igt@kms_frontbuffer_tracking@basic.html
    - fi-skl-6600u:       [PASS][127] -> [CRASH][128]
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-skl-6600u/igt@kms_frontbuffer_tracking@basic.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-skl-6600u/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-dsi:         [PASS][129] -> [CRASH][130]
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
    - fi-bdw-5557u:       [PASS][131] -> [CRASH][132]
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-bdw-5557u/igt@kms_frontbuffer_tracking@basic.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-bdw-5557u/igt@kms_frontbuffer_tracking@basic.html
    - fi-kbl-r:           [PASS][133] -> [CRASH][134]
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-kbl-r/igt@kms_frontbuffer_tracking@basic.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-kbl-r/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-y:           [PASS][135] -> [CRASH][136]
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-icl-y/igt@kms_frontbuffer_tracking@basic.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/fi-icl-y/igt@kms_frontbuffer_tracking@basic.html
    - fi-hsw-peppy:       [PASS][137] -> [CRASH][138]
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5299/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [138]: https://intel-g

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3737/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
@ 2019-11-20 19:41   ` Chris Wilson
  2019-11-21  7:25     ` Zbigniew Kempczyński
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-11-20 19:41 UTC (permalink / raw)
  To: Zbigniew Kempczyński, igt-dev

Quoting Zbigniew Kempczyński (2019-11-20 18:57:36)
> From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> 
> With introduction of LMEM concept new IOCTL call were implemented
> - gem_mmap_offset. This patch add support in IGT for it.
> 
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> ---
>  lib/i915/gem_mman.c | 222 ++++++++++++++++++++++++++++++++++++--------
>  lib/i915/gem_mman.h |  42 ++++++++-
>  2 files changed, 224 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 6256627b..1467b883 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -40,6 +40,39 @@
>  #define VG(x) do {} while (0)
>  #endif
>  
> +#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
> +
> +static bool gem_has_mmap_offset(int fd)
> +{
> +       int has_mmap_offset = 0;
> +       struct drm_i915_getparam gp;
> +
> +       memset(&gp, 0, sizeof(gp));
> +       gp.param = LOCAL_I915_PARAM_MMAP_OFFSET_VERSION;
> +       gp.value = &has_mmap_offset;
> +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);

* Blinks.

Oh no it isn't.

It's just I915_PARAM_MMAP_GTT_VERSION >= 4

> +
> +       return has_mmap_offset > 0;
> +}
> +
> +void gem_require_mmap_offset(int i915)
> +{
> +       igt_require(gem_has_mmap_offset(i915));
> +}
> +
> +static int gem_mmap_gtt_version(int fd)
> +{
> +       struct drm_i915_getparam gp;
> +       int gtt_version = -1;
> +
> +       memset(&gp, 0, sizeof(gp));
> +       gp.param = I915_PARAM_MMAP_GTT_VERSION;
> +       gp.value = &gtt_version;
> +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +
> +       return gtt_version;
> +}
> +
>  /**
>   * __gem_mmap__gtt:
>   * @fd: open i915 drm file descriptor
> @@ -101,46 +134,63 @@ int gem_munmap(void *ptr, uint64_t size)
>         return ret;
>  }
>  
> -bool gem_mmap__has_wc(int fd)
> +bool __gem_mmap__has_wc(int fd)
>  {
> -       static int has_wc = -1;
> -
> -       if (has_wc == -1) {
> -               struct drm_i915_getparam gp;
> -               int mmap_version = -1;
> -               int gtt_version = -1;
> -
> -               has_wc = 0;
> -
> -               memset(&gp, 0, sizeof(gp));
> -               gp.param = I915_PARAM_MMAP_GTT_VERSION;
> -               gp.value = &gtt_version;
> -               ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> -
> -               memset(&gp, 0, sizeof(gp));
> -               gp.param = I915_PARAM_MMAP_VERSION;
> -               gp.value = &mmap_version;
> -               ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> -
> -               /* Do we have the new mmap_ioctl with DOMAIN_WC? */
> -               if (mmap_version >= 1 && gtt_version >= 2) {
> -                       struct drm_i915_gem_mmap arg;
> -
> -                       /* Does this device support wc-mmaps ? */
> -                       memset(&arg, 0, sizeof(arg));
> -                       arg.handle = gem_create(fd, 4096);
> -                       arg.offset = 0;
> -                       arg.size = 4096;
> -                       arg.flags = I915_MMAP_WC;
> -                       has_wc = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg) == 0;
> -                       gem_close(fd, arg.handle);
> -               }
> -               errno = 0;
> +       int has_wc = 0;
> +
> +       struct drm_i915_getparam gp;
> +       int mmap_version = -1;
> +
> +       memset(&gp, 0, sizeof(gp));
> +       gp.param = I915_PARAM_MMAP_VERSION;
> +       gp.value = &mmap_version;
> +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +
> +       /* Do we have the mmap_ioctl with DOMAIN_WC? */
> +       if (mmap_version >= 1 && gem_mmap_gtt_version(fd) >= 2) {
> +               struct drm_i915_gem_mmap arg;
> +
> +               /* Does this device support wc-mmaps ? */
> +               memset(&arg, 0, sizeof(arg));
> +               arg.handle = gem_create(fd, 4096);
> +               arg.offset = 0;
> +               arg.size = 4096;
> +               arg.flags = I915_MMAP_WC;
> +               has_wc = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg) == 0;
> +               gem_close(fd, arg.handle);
>         }
> +       errno = 0;

Ok.

>  
>         return has_wc > 0;
>  }
>  
> +bool __gem_mmap_offset__has_wc(int fd)
> +{
> +       int has_wc = 0;
> +
> +       if (gem_has_mmap_offset(fd)) {

Don't need the double ioctl... Hmm, oh wait. That's what you get for not
checking arg.pad. Gah.

> +               struct local_i915_gem_mmap_offset arg;
> +
> +               /* Does this device support wc-mmaps ? */
> +               memset(&arg, 0, sizeof(arg));
> +               arg.handle = gem_create(fd, 4096);
> +               arg.offset = 0;
> +               arg.flags = LOCAL_I915_MMAP_OFFSET_WC;
> +               has_wc = igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET,
> +                                  &arg) == 0;
> +               gem_close(fd, arg.handle);
> +       }

Ok.

> +
> +       errno = 0;
> +
> +       return has_wc > 0;
> +}
> +
> +bool gem_mmap__has_wc(int fd)
> +{
> +       return __gem_mmap_offset__has_wc(fd) || __gem_mmap__has_wc(fd);
> +}
> +
>  /**
>   * __gem_mmap:
>   * @fd: open i915 drm file descriptor
> @@ -157,8 +207,8 @@ bool gem_mmap__has_wc(int fd)
>   *
>   * Returns: A pointer to the created memory mapping, NULL on failure.
>   */
> -static void
> -*__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned int prot, uint64_t flags)
> +void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size,
> +                unsigned int prot, uint64_t flags)
>  {
>         struct drm_i915_gem_mmap arg;
>  
> @@ -177,6 +227,43 @@ static void
>         return from_user_pointer(arg.addr_ptr);
>  }
>  
> +/**
> + * __gem_mmap_offset:
> + * @fd: open i915 drm file descriptor
> + * @handle: gem buffer object handle
> + * @offset: offset in the gem buffer of the mmap arena
> + * @size: size of the mmap arena
> + * @prot: memory protection bits as used by mmap()
> + * @flags: flags used to determine caching
> + *
> + * Similar to __gem_mmap but use MMAP_OFFSET IOCTL.
> + *
> + * Returns: A pointer to the created memory mapping, NULL on failure.
> + */
> +void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size,
> +                       unsigned int prot, uint64_t flags)
> +{
> +       struct local_i915_gem_mmap_offset arg;
> +       void *ptr;
> +
> +       memset(&arg, 0, sizeof(arg));
> +       arg.handle = handle;
> +       arg.offset = offset;

That's an interesting extension :-p

Yeah, being able to mmap a slice of the object is one of the extensions
I had in mind.

> +       arg.flags = flags;
> +
> +       if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET, &arg))
> +               return NULL;
> +
> +       ptr = mmap64(0, size, prot, MAP_SHARED, fd, arg.offset);
> +
> +       if (ptr == MAP_FAILED)
> +               ptr = NULL;
> +       else
> +               errno = 0;
> +
> +       return ptr;
> +}
> +
>  /**
>   * __gem_mmap__wc:
>   * @fd: open i915 drm file descriptor
> @@ -194,7 +281,12 @@ static void
>   */
>  void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
>  {
> -       return __gem_mmap(fd, handle, offset, size, prot, I915_MMAP_WC);
> +       void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
> +                                     LOCAL_I915_MMAP_OFFSET_WC);
> +       if (!ptr)
> +               ptr = __gem_mmap(fd, handle, offset, size, prot, I915_MMAP_WC);

Ok.

> +
> +       return ptr;
>  }
>  
>  /**
> @@ -205,14 +297,62 @@ void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, un
>   * @size: size of the mmap arena
>   * @prot: memory protection bits as used by mmap()
>   *
> - * Like __gem_mmap__wc() except we assert on failure.
> + * Try to __gem_mmap_wc(). Assert on failure.

It was better when it was __wc, at least then it matches the function
name ;)

>   *
>   * Returns: A pointer to the created memory mapping
>   */
>  void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
>  {
>         void *ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
> +
> +       igt_assert(ptr);
> +       return ptr;
> +}
> +
> +/**
> + * __gem_mmap__device_coherent:
> + * @fd: open i915 drm file descriptor
> + * @handle: gem buffer object handle
> + * @offset: offset in the gem buffer of the mmap arena
> + * @size: size of the mmap arena
> + * @prot: memory protection bits as used by mmap()
> + *
> + * Try to __gem_mmap_offset, then __gem_mmap__wc(), then __gem_mmap__gtt().

Don't repeat the code, we can read that! You need to explain the
semantics.

Returns a pointer to a block of linear device memory mapped into the
process with WC semantics.

That's a slight lie if we fallback to __gtt as it may end up being
tiled, so note the exception.

> + *
> + * Returns: A pointer to the created memory mapping or NULL.
> + */
> +void *__gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
> +                                 uint64_t size, unsigned prot)
> +{
> +       void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
> +                                     LOCAL_I915_MMAP_OFFSET_WC);
> +       if (!ptr)
> +               ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
> +
> +       if (!ptr)
> +               ptr = __gem_mmap__gtt(fd, handle, size, prot);
> +
> +       return ptr;
> +}
> +
> +/**
> + * gem_mmap__device_coherent:
> + * @fd: open i915 drm file descriptor
> + * @handle: gem buffer object handle
> + * @offset: offset in the gem buffer of the mmap arena
> + * @size: size of the mmap arena
> + * @prot: memory protection bits as used by mmap()
> + *
> + * Call __gem_mmap__device__coherent(), asserts on fail.
> + *
> + * Returns: A pointer to the created memory mapping.
> + */
> +void *gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
> +                               uint64_t size, unsigned prot)
> +{
> +       void *ptr = gem_mmap__device_coherent(fd, handle, offset, size, prot);
>         igt_assert(ptr);

Might as well abide by kernel coding style, blank lines after variables.

And if we are writing a library function, lets be nice and include debug
info in the assert (handle, size, protection etc).

You never know, we might start a trend of being helpful.

> +
>         return ptr;
>  }
>  
> @@ -248,7 +388,11 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
>   */
>  void *gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
>  {
> -       void *ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> +       void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
> +                                     LOCAL_I915_MMAP_OFFSET_WB);
> +       if (!ptr)
> +               ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);

Ok.

> +
>         igt_assert(ptr);
>         return ptr;
>  }
> diff --git a/lib/i915/gem_mman.h b/lib/i915/gem_mman.h
> index 096ff592..e774d5c8 100644
> --- a/lib/i915/gem_mman.h
> +++ b/lib/i915/gem_mman.h
> @@ -25,12 +25,46 @@
>  #ifndef GEM_MMAN_H
>  #define GEM_MMAN_H
>  
> +#define LOCAL_I915_GEM_MMAP_OFFSET       DRM_I915_GEM_MMAP_GTT
> +#define LOCAL_IOCTL_I915_GEM_MMAP_OFFSET         DRM_IOWR(DRM_COMMAND_BASE + \
> +       LOCAL_I915_GEM_MMAP_OFFSET, struct local_i915_gem_mmap_offset)
> +
> +struct local_i915_gem_mmap_offset {
> +       /** Handle for the object being mapped. */
> +       __u32 handle;
> +       __u32 pad;
> +       /**
> +        * Fake offset to use for subsequent mmap call
> +        *
> +        * This is a fixed-size type for 32/64 compatibility.
> +        */
> +       __u64 offset;
> +       /**
> +        * Flags for extended behaviour.
> +        *
> +        * It is mandatory that either one of the _WC/_WB flags
> +        * should be passed here.
> +        */
> +       __u64 flags;
> +#define LOCAL_I915_MMAP_OFFSET_WC (1 << 0)
> +#define LOCAL_I915_MMAP_OFFSET_WB (1 << 1)
> +#define LOCAL_I915_MMAP_OFFSET_UC (1 << 2)
> +#define LOCAL_I915_MMAP_OFFSET_FLAGS \
> +       (LOCAL_I915_MMAP_OFFSET_WC | LOCAL_I915_MMAP_OFFSET_WB | LOCAL_I915_MMAP_OFFSET_UC)
> +};

I suggest just having a dummy commit to update i915_drm.h until the real
commit lands.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
@ 2019-11-20 19:43   ` Chris Wilson
  2019-11-20 19:58     ` Zbigniew Kempczyński
  2019-11-20 19:45   ` Chris Wilson
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-11-20 19:43 UTC (permalink / raw)
  To: Zbigniew Kempczyński, igt-dev

Quoting Zbigniew Kempczyński (2019-11-20 18:57:37)
> +/**
> + *  gem_get_batch_size:
> + *  @fd: open i915 drm file descriptor
> + *  @region: region in which we want to create a batch
> + *
> + *  FIXME: Currently function assumes we have 64K on DEVICE and 4K
> + *  on SYSTEM memory. I know Chris is going to kill me for that
> + *  but I'll fix this when patch with memory region page size detection
> + *  will be merged.

How am I supposed to rant now?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
  2019-11-20 19:43   ` Chris Wilson
@ 2019-11-20 19:45   ` Chris Wilson
  2019-11-20 20:06     ` Zbigniew Kempczyński
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-11-20 19:45 UTC (permalink / raw)
  To: Zbigniew Kempczyński, igt-dev

Quoting Zbigniew Kempczyński (2019-11-20 18:57:37)
> +/* gem_create_in_memory_region_list:
> + * @fd: opened i915 drm file descriptor
> + * @size: requested size of the buffer
> + * @mem_regions: memory regions array (priority list)
> + * @num_regions: @mem_regions length
> + */
> +uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
> +                                         uint32_t *mem_regions,
> +                                         int num_regions)
> +{
> +       uint32_t handle = gem_create(fd, size);
> +       int ret = __gem_migrate_to_memory_regions(fd, handle,
> +                                                 mem_regions, num_regions);
> +       igt_assert_eq(ret, 0);

No gem_create_ext() where we can pass OBJECT_PARAM in at create time? So
Joonas can later say these are construct time only properties?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
@ 2019-11-20 19:50   ` Chris Wilson
  2019-11-21 17:07     ` Zbigniew Kempczyński
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-11-20 19:50 UTC (permalink / raw)
  To: Zbigniew Kempczyński, igt-dev

Quoting Zbigniew Kempczyński (2019-11-20 18:57:38)
> From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> 
> This test is a copy/paste of few gem_mmap subtests, due to good
> coverage in previous one. We also need to be sure that we cover
> all available memory regions.
> 
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> ---
>  tests/Makefile.sources       |   3 +
>  tests/i915/gem_mmap_offset.c | 158 +++++++++++++++++++++++++++++++++++
>  tests/meson.build            |   1 +
>  3 files changed, 162 insertions(+)
>  create mode 100644 tests/i915/gem_mmap_offset.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 27801c89..9c6c3933 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -320,6 +320,9 @@ gem_mmap_SOURCES = i915/gem_mmap.c
>  TESTS_progs += gem_mmap_gtt
>  gem_mmap_gtt_SOURCES = i915/gem_mmap_gtt.c
>  
> +TESTS_progs += gem_mmap_offset
> +gem_mmap_offset_SOURCES = i915/gem_mmap_offset.c
> +
>  TESTS_progs += gem_mmap_offset_exhaustion
>  gem_mmap_offset_exhaustion_SOURCES = i915/gem_mmap_offset_exhaustion.c
>  
> diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
> new file mode 100644
> index 00000000..5215d82e
> --- /dev/null
> +++ b/tests/i915/gem_mmap_offset.c
> @@ -0,0 +1,158 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "igt.h"
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include "drm.h"
> +
> +IGT_TEST_DESCRIPTION("Basic MMAP_OFFSET IOCTL tests for mem regions\n");
> +
> +static int mmap_offset_ioctl(int fd, struct local_i915_gem_mmap_offset *arg)
> +{
> +       int err = 0;
> +
> +       if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET, arg))
> +               err = -errno;
> +
> +       errno = 0;
> +       return err;
> +}
> +
> +igt_main
> +{
> +       uint8_t *addr;
> +       uint32_t obj_size, batch_size;
> +       uint32_t mem_type, mem_instance;
> +       uint32_t region;
> +       int fd;
> +       const struct intel_memory_region *mr;
> +       struct local_i915_query_memory_region_info *query_info;
> +
> +       igt_fixture {
> +               fd = drm_open_driver(DRIVER_INTEL);
> +               gem_require_mmap_offset(fd);
> +               igt_require(gem_mmap__has_wc(fd));
> +
> +               query_info = gem_query_memory_regions(fd);
> +               igt_assert(query_info);

This ioctl will be available before memory regions uAPI, and we will be
expecting coverage...

gem_mmap_offset_regions
or
gem_memory_regions_mmap

or just igt_subtest_group {}

And do we have a plan for switching to something like i915_gem_ instead?

> +       }
> +
> +       for (mr = intel_memory_regions; mr->region_name; mr++) {

See igt_subtest_with_dynamic

> +               mem_type = mr->memory_type;
> +               mem_instance = mr->memory_instance;
> +               region = INTEL_MEMORY_REGION_ID(mem_type, mem_instance);
> +               batch_size = gem_get_batch_size(fd, region);
> +               obj_size = 4 * batch_size;
> +
> +               igt_subtest_f("bad-object-%s", mr->region_name) {
> +                       uint32_t real_handle;
> +                       uint32_t handles[20];
> +                       int i = 0;
> +
> +                       gem_query_require_region(query_info, region);
> +
> +                       real_handle = gem_create_in_memory_regions(fd, obj_size,
> +                                                                  region);
> +
> +                       handles[i++] = 0xdeadbeef;
> +                       for (int bit = 0; bit < 16; bit++)
> +                               handles[i++] = real_handle | (1 << (bit + 16));
> +                       handles[i] = real_handle + 1;
> +
> +                       for (; i >= 0; i--) {
> +                               struct local_i915_gem_mmap_offset arg = {
> +                                       .handle = handles[i],
> +                                       .flags = LOCAL_I915_MMAP_OFFSET_WC,
> +                               };
> +
> +                               igt_debug("Trying MMAP IOCTL with handle %x\n",
> +                                         handles[i]);
> +                               igt_assert_eq(mmap_offset_ioctl(fd, &arg),
> +                                             -ENOENT);
> +                       }
> +
> +                       gem_close(fd, real_handle);
> +               }
> +
> +               igt_subtest_f("basic-%s", mr->region_name) {
> +                       uint32_t handle;
> +                       uint64_t flags;
> +                       uint8_t *expected;
> +                       uint8_t *buf;
> +
> +                       gem_query_require_region(query_info, region);
> +
> +                       handle = gem_create_in_memory_regions(fd, obj_size,
> +                                                             region);
> +                       flags = LOCAL_I915_MMAP_OFFSET_WC;
> +                       addr = __gem_mmap_offset(fd, handle, 0, obj_size,
> +                                                PROT_READ | PROT_WRITE,
> +                                                flags);
> +                       igt_assert(addr);
> +
> +                       igt_debug("Testing contents of newly created object.\n");
> +                       expected = calloc(obj_size, sizeof(*expected));
> +                       igt_assert_eq(memcmp(addr, expected, obj_size), 0);
> +                       free(expected);
> +
> +                       igt_debug("Testing coherency of writes and mmap reads.\n");
> +                       buf = calloc(obj_size, sizeof(*buf));
> +                       memset(buf + 1024, 0x01, 1024);
> +                       gem_write(fd, handle, 0, buf, obj_size);
> +                       igt_assert_eq(memcmp(buf, addr, obj_size), 0);
> +
> +                       igt_debug("Testing that mapping stays after close\n");
> +                       gem_close(fd, handle);
> +                       igt_assert_eq(memcmp(buf, addr, obj_size), 0);
> +                       free(buf);
> +
> +                       igt_debug("Testing unmapping\n");
> +                       munmap(addr, obj_size);
> +               }
> +
> +               igt_subtest_f("short-mmap-%s", mr->region_name) {
> +                       uint32_t handle;

Not supported in vanilla ioctl, coming to an extension near you :)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions
  2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions Zbigniew Kempczyński
@ 2019-11-20 19:53   ` Chris Wilson
  2019-11-21  8:40     ` Zbigniew Kempczyński
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-11-20 19:53 UTC (permalink / raw)
  To: Zbigniew Kempczyński, igt-dev

Quoting Zbigniew Kempczyński (2019-11-20 18:57:39)
> From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> 
> As a part of local memory effort we need to make sure, that basic
> scenarios are covered for every available memory region. This patch is
> an attempt for this problem. If it will be accepted it will be
> replicated on each test that can benefit from it.

gem_exec_basic just exercises the execbuf ioctl... which does not
mention memory region. You are thinking of gem_exec_nop and
gem_exec_store for this first level of basic testing.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 19:43   ` Chris Wilson
@ 2019-11-20 19:58     ` Zbigniew Kempczyński
  0 siblings, 0 replies; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 19:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Nov 20, 2019 at 07:43:08PM +0000, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2019-11-20 18:57:37)
> > +/**
> > + *  gem_get_batch_size:
> > + *  @fd: open i915 drm file descriptor
> > + *  @region: region in which we want to create a batch
> > + *
> > + *  FIXME: Currently function assumes we have 64K on DEVICE and 4K
> > + *  on SYSTEM memory. I know Chris is going to kill me for that
> > + *  but I'll fix this when patch with memory region page size detection
> > + *  will be merged.
> 
> How am I supposed to rant now?
> -Chris

I've seen your comment on Łukasz patch, so I guessed you'll be 
angry I'm trying to put same/similar code. I'm sorry for that,
I'm going to fix that later when Janusz patch will land in IGT.
This shortcoming can be then easily fixed using his code then,
so please forgive me hard joke. But you started first on irc 
frightening me with leopard ;-)

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 19:45   ` Chris Wilson
@ 2019-11-20 20:06     ` Zbigniew Kempczyński
  2019-11-20 20:15       ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-20 20:06 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Nov 20, 2019 at 07:45:16PM +0000, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2019-11-20 18:57:37)
> > +/* gem_create_in_memory_region_list:
> > + * @fd: opened i915 drm file descriptor
> > + * @size: requested size of the buffer
> > + * @mem_regions: memory regions array (priority list)
> > + * @num_regions: @mem_regions length
> > + */
> > +uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
> > +                                         uint32_t *mem_regions,
> > +                                         int num_regions)
> > +{
> > +       uint32_t handle = gem_create(fd, size);
> > +       int ret = __gem_migrate_to_memory_regions(fd, handle,
> > +                                                 mem_regions, num_regions);
> > +       igt_assert_eq(ret, 0);
> 
> No gem_create_ext() where we can pass OBJECT_PARAM in at create time? So
> Joonas can later say these are construct time only properties?
> -Chris

You mean more generic wrapper than above? No problem to add one
but this one was for migrate bo to requested memory regions, nothing else.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 20:06     ` Zbigniew Kempczyński
@ 2019-11-20 20:15       ` Chris Wilson
  2019-11-21  8:44         ` Zbigniew Kempczyński
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-11-20 20:15 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev

Quoting Zbigniew Kempczyński (2019-11-20 20:06:40)
> On Wed, Nov 20, 2019 at 07:45:16PM +0000, Chris Wilson wrote:
> > Quoting Zbigniew Kempczyński (2019-11-20 18:57:37)
> > > +/* gem_create_in_memory_region_list:
> > > + * @fd: opened i915 drm file descriptor
> > > + * @size: requested size of the buffer
> > > + * @mem_regions: memory regions array (priority list)
> > > + * @num_regions: @mem_regions length
> > > + */
> > > +uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
> > > +                                         uint32_t *mem_regions,
> > > +                                         int num_regions)
> > > +{
> > > +       uint32_t handle = gem_create(fd, size);
> > > +       int ret = __gem_migrate_to_memory_regions(fd, handle,
> > > +                                                 mem_regions, num_regions);
> > > +       igt_assert_eq(ret, 0);
> > 
> > No gem_create_ext() where we can pass OBJECT_PARAM in at create time? So
> > Joonas can later say these are construct time only properties?
> > -Chris
> 
> You mean more generic wrapper than above? No problem to add one
> but this one was for migrate bo to requested memory regions, nothing else.

No, I mean we need to extend the GEM_CREATE_IOCTL.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support
  2019-11-20 19:41   ` Chris Wilson
@ 2019-11-21  7:25     ` Zbigniew Kempczyński
  2019-11-21  7:30       ` Chris Wilson
  2019-11-21  7:34       ` Chris Wilson
  0 siblings, 2 replies; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-21  7:25 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Nov 20, 2019 at 07:41:45PM +0000, Chris Wilson wrote:
> > +#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
> > +
> > +static bool gem_has_mmap_offset(int fd)
> > +{
> > +       int has_mmap_offset = 0;
> > +       struct drm_i915_getparam gp;
> > +
> > +       memset(&gp, 0, sizeof(gp));
> > +       gp.param = LOCAL_I915_PARAM_MMAP_OFFSET_VERSION;
> > +       gp.value = &has_mmap_offset;
> > +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> 
> * Blinks.
> 
> Oh no it isn't.
> 
> It's just I915_PARAM_MMAP_GTT_VERSION >= 4

I've double checked:

i915_getparam:
	case I915_PARAM_MMAP_GTT_VERSION:
		 -> i915_gem_mmap_gtt_version()
                 	-> return 3

	case I915_PARAM_MMAP_OFFSET_VERSION:
		-> return 1

Where's the magic 4 in GTT_VERSION? I don't see it.
Is some non merged patch containing it?

> 
> > +
> > +       return has_mmap_offset > 0;
> > +}
> > +
> > +void gem_require_mmap_offset(int i915)
> > +{
> > +       igt_require(gem_has_mmap_offset(i915));
> > +}
> > +
> > +static int gem_mmap_gtt_version(int fd)
> > +{
> > +       struct drm_i915_getparam gp;
> > +       int gtt_version = -1;
> > +
> > +       memset(&gp, 0, sizeof(gp));
> > +       gp.param = I915_PARAM_MMAP_GTT_VERSION;
> > +       gp.value = &gtt_version;
> > +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > +
> > +       return gtt_version;
> > +}
> > +
> >  /**
> >   * __gem_mmap__gtt:
> >   * @fd: open i915 drm file descriptor
> > @@ -101,46 +134,63 @@ int gem_munmap(void *ptr, uint64_t size)
> >         return ret;
> >  }
> >  
> > -bool gem_mmap__has_wc(int fd)
> > +bool __gem_mmap__has_wc(int fd)
> >  {
> > -       static int has_wc = -1;
> > -
> > -       if (has_wc == -1) {
> > -               struct drm_i915_getparam gp;
> > -               int mmap_version = -1;
> > -               int gtt_version = -1;
> > -
> > -               has_wc = 0;
> > -
> > -               memset(&gp, 0, sizeof(gp));
> > -               gp.param = I915_PARAM_MMAP_GTT_VERSION;
> > -               gp.value = &gtt_version;
> > -               ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > -
> > -               memset(&gp, 0, sizeof(gp));
> > -               gp.param = I915_PARAM_MMAP_VERSION;
> > -               gp.value = &mmap_version;
> > -               ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > -
> > -               /* Do we have the new mmap_ioctl with DOMAIN_WC? */
> > -               if (mmap_version >= 1 && gtt_version >= 2) {
> > -                       struct drm_i915_gem_mmap arg;
> > -
> > -                       /* Does this device support wc-mmaps ? */
> > -                       memset(&arg, 0, sizeof(arg));
> > -                       arg.handle = gem_create(fd, 4096);
> > -                       arg.offset = 0;
> > -                       arg.size = 4096;
> > -                       arg.flags = I915_MMAP_WC;
> > -                       has_wc = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg) == 0;
> > -                       gem_close(fd, arg.handle);
> > -               }
> > -               errno = 0;
> > +       int has_wc = 0;
> > +
> > +       struct drm_i915_getparam gp;
> > +       int mmap_version = -1;
> > +
> > +       memset(&gp, 0, sizeof(gp));
> > +       gp.param = I915_PARAM_MMAP_VERSION;
> > +       gp.value = &mmap_version;
> > +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > +
> > +       /* Do we have the mmap_ioctl with DOMAIN_WC? */
> > +       if (mmap_version >= 1 && gem_mmap_gtt_version(fd) >= 2) {
> > +               struct drm_i915_gem_mmap arg;
> > +
> > +               /* Does this device support wc-mmaps ? */
> > +               memset(&arg, 0, sizeof(arg));
> > +               arg.handle = gem_create(fd, 4096);
> > +               arg.offset = 0;
> > +               arg.size = 4096;
> > +               arg.flags = I915_MMAP_WC;
> > +               has_wc = igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg) == 0;
> > +               gem_close(fd, arg.handle);
> >         }
> > +       errno = 0;
> 
> Ok.
> 
> >  
> >         return has_wc > 0;
> >  }
> >  
> > +bool __gem_mmap_offset__has_wc(int fd)
> > +{
> > +       int has_wc = 0;
> > +
> > +       if (gem_has_mmap_offset(fd)) {
> 
> Don't need the double ioctl... Hmm, oh wait. That's what you get for not
> checking arg.pad. Gah.

You suggest we can skip gem_has_mmap_offset() call by trial by fire 
I915_GEM_MMAP_OFFSET ioctl()?

> 
> > +               struct local_i915_gem_mmap_offset arg;
> > +
> > +               /* Does this device support wc-mmaps ? */
> > +               memset(&arg, 0, sizeof(arg));
> > +               arg.handle = gem_create(fd, 4096);
> > +               arg.offset = 0;
> > +               arg.flags = LOCAL_I915_MMAP_OFFSET_WC;
> > +               has_wc = igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET,
> > +                                  &arg) == 0;
> > +               gem_close(fd, arg.handle);
> > +       }
> 
> Ok.
> 
> > +
> > +       errno = 0;
> > +
> > +       return has_wc > 0;
> > +}
> > +
> > +bool gem_mmap__has_wc(int fd)
> > +{
> > +       return __gem_mmap_offset__has_wc(fd) || __gem_mmap__has_wc(fd);
> > +}
> > +
> >  /**
> >   * __gem_mmap:
> >   * @fd: open i915 drm file descriptor
> > @@ -157,8 +207,8 @@ bool gem_mmap__has_wc(int fd)
> >   *
> >   * Returns: A pointer to the created memory mapping, NULL on failure.
> >   */
> > -static void
> > -*__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned int prot, uint64_t flags)
> > +void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size,
> > +                unsigned int prot, uint64_t flags)
> >  {
> >         struct drm_i915_gem_mmap arg;
> >  
> > @@ -177,6 +227,43 @@ static void
> >         return from_user_pointer(arg.addr_ptr);
> >  }
> >  
> > +/**
> > + * __gem_mmap_offset:
> > + * @fd: open i915 drm file descriptor
> > + * @handle: gem buffer object handle
> > + * @offset: offset in the gem buffer of the mmap arena
> > + * @size: size of the mmap arena
> > + * @prot: memory protection bits as used by mmap()
> > + * @flags: flags used to determine caching
> > + *
> > + * Similar to __gem_mmap but use MMAP_OFFSET IOCTL.
> > + *
> > + * Returns: A pointer to the created memory mapping, NULL on failure.
> > + */
> > +void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size,
> > +                       unsigned int prot, uint64_t flags)
> > +{
> > +       struct local_i915_gem_mmap_offset arg;
> > +       void *ptr;
> > +
> > +       memset(&arg, 0, sizeof(arg));
> > +       arg.handle = handle;
> > +       arg.offset = offset;
> 
> That's an interesting extension :-p
> 
> Yeah, being able to mmap a slice of the object is one of the extensions
> I had in mind.

Doesn't it work as I expect? Isn't memory range verified in do_mmap()
and after syscall our implementation does the real job then?

> 
> > +       arg.flags = flags;
> > +
> > +       if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_OFFSET, &arg))
> > +               return NULL;
> > +
> > +       ptr = mmap64(0, size, prot, MAP_SHARED, fd, arg.offset);
> > +
> > +       if (ptr == MAP_FAILED)
> > +               ptr = NULL;
> > +       else
> > +               errno = 0;
> > +
> > +       return ptr;
> > +}
> > +
> >  /**
> >   * __gem_mmap__wc:
> >   * @fd: open i915 drm file descriptor
> > @@ -194,7 +281,12 @@ static void
> >   */
> >  void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> >  {
> > -       return __gem_mmap(fd, handle, offset, size, prot, I915_MMAP_WC);
> > +       void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
> > +                                     LOCAL_I915_MMAP_OFFSET_WC);
> > +       if (!ptr)
> > +               ptr = __gem_mmap(fd, handle, offset, size, prot, I915_MMAP_WC);
> 
> Ok.
> 
> > +
> > +       return ptr;
> >  }
> >  
> >  /**
> > @@ -205,14 +297,62 @@ void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, un
> >   * @size: size of the mmap arena
> >   * @prot: memory protection bits as used by mmap()
> >   *
> > - * Like __gem_mmap__wc() except we assert on failure.
> > + * Try to __gem_mmap_wc(). Assert on failure.
> 
> It was better when it was __wc, at least then it matches the function
> name ;)

Ok, I'll fix that.

> 
> >   *
> >   * Returns: A pointer to the created memory mapping
> >   */
> >  void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> >  {
> >         void *ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
> > +
> > +       igt_assert(ptr);
> > +       return ptr;
> > +}
> > +
> > +/**
> > + * __gem_mmap__device_coherent:
> > + * @fd: open i915 drm file descriptor
> > + * @handle: gem buffer object handle
> > + * @offset: offset in the gem buffer of the mmap arena
> > + * @size: size of the mmap arena
> > + * @prot: memory protection bits as used by mmap()
> > + *
> > + * Try to __gem_mmap_offset, then __gem_mmap__wc(), then __gem_mmap__gtt().
> 
> Don't repeat the code, we can read that! You need to explain the
> semantics.
> 
> Returns a pointer to a block of linear device memory mapped into the
> process with WC semantics.
> 
> That's a slight lie if we fallback to __gtt as it may end up being
> tiled, so note the exception.

Ok, I'll fix that.

> 
> > + *
> > + * Returns: A pointer to the created memory mapping or NULL.
> > + */
> > +void *__gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
> > +                                 uint64_t size, unsigned prot)
> > +{
> > +       void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
> > +                                     LOCAL_I915_MMAP_OFFSET_WC);
> > +       if (!ptr)
> > +               ptr = __gem_mmap__wc(fd, handle, offset, size, prot);
> > +
> > +       if (!ptr)
> > +               ptr = __gem_mmap__gtt(fd, handle, size, prot);
> > +
> > +       return ptr;
> > +}
> > +
> > +/**
> > + * gem_mmap__device_coherent:
> > + * @fd: open i915 drm file descriptor
> > + * @handle: gem buffer object handle
> > + * @offset: offset in the gem buffer of the mmap arena
> > + * @size: size of the mmap arena
> > + * @prot: memory protection bits as used by mmap()
> > + *
> > + * Call __gem_mmap__device__coherent(), asserts on fail.
> > + *
> > + * Returns: A pointer to the created memory mapping.
> > + */
> > +void *gem_mmap__device_coherent(int fd, uint32_t handle, uint64_t offset,
> > +                               uint64_t size, unsigned prot)
> > +{
> > +       void *ptr = gem_mmap__device_coherent(fd, handle, offset, size, prot);
> >         igt_assert(ptr);
> 
> Might as well abide by kernel coding style, blank lines after variables.
> 
> And if we are writing a library function, lets be nice and include debug
> info in the assert (handle, size, protection etc).
> 
> You never know, we might start a trend of being helpful.

Ok. 
> 
> > +
> >         return ptr;
> >  }
> >  
> > @@ -248,7 +388,11 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
> >   */
> >  void *gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
> >  {
> > -       void *ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> > +       void *ptr = __gem_mmap_offset(fd, handle, offset, size, prot,
> > +                                     LOCAL_I915_MMAP_OFFSET_WB);
> > +       if (!ptr)
> > +               ptr = __gem_mmap__cpu(fd, handle, offset, size, prot);
> 
> Ok.
> 
> > +
> >         igt_assert(ptr);
> >         return ptr;
> >  }
> > diff --git a/lib/i915/gem_mman.h b/lib/i915/gem_mman.h
> > index 096ff592..e774d5c8 100644
> > --- a/lib/i915/gem_mman.h
> > +++ b/lib/i915/gem_mman.h
> > @@ -25,12 +25,46 @@
> >  #ifndef GEM_MMAN_H
> >  #define GEM_MMAN_H
> >  
> > +#define LOCAL_I915_GEM_MMAP_OFFSET       DRM_I915_GEM_MMAP_GTT
> > +#define LOCAL_IOCTL_I915_GEM_MMAP_OFFSET         DRM_IOWR(DRM_COMMAND_BASE + \
> > +       LOCAL_I915_GEM_MMAP_OFFSET, struct local_i915_gem_mmap_offset)
> > +
> > +struct local_i915_gem_mmap_offset {
> > +       /** Handle for the object being mapped. */
> > +       __u32 handle;
> > +       __u32 pad;
> > +       /**
> > +        * Fake offset to use for subsequent mmap call
> > +        *
> > +        * This is a fixed-size type for 32/64 compatibility.
> > +        */
> > +       __u64 offset;
> > +       /**
> > +        * Flags for extended behaviour.
> > +        *
> > +        * It is mandatory that either one of the _WC/_WB flags
> > +        * should be passed here.
> > +        */
> > +       __u64 flags;
> > +#define LOCAL_I915_MMAP_OFFSET_WC (1 << 0)
> > +#define LOCAL_I915_MMAP_OFFSET_WB (1 << 1)
> > +#define LOCAL_I915_MMAP_OFFSET_UC (1 << 2)
> > +#define LOCAL_I915_MMAP_OFFSET_FLAGS \
> > +       (LOCAL_I915_MMAP_OFFSET_WC | LOCAL_I915_MMAP_OFFSET_WB | LOCAL_I915_MMAP_OFFSET_UC)
> > +};
> 
> I suggest just having a dummy commit to update i915_drm.h until the real
> commit lands.
> -Chris

Ok. I'll move structures and definitions there.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support
  2019-11-21  7:25     ` Zbigniew Kempczyński
@ 2019-11-21  7:30       ` Chris Wilson
  2019-11-21  7:34       ` Chris Wilson
  1 sibling, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-11-21  7:30 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev

Quoting Zbigniew Kempczyński (2019-11-21 07:25:20)
> On Wed, Nov 20, 2019 at 07:41:45PM +0000, Chris Wilson wrote:
> > > +#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
> > > +
> > > +static bool gem_has_mmap_offset(int fd)
> > > +{
> > > +       int has_mmap_offset = 0;
> > > +       struct drm_i915_getparam gp;
> > > +
> > > +       memset(&gp, 0, sizeof(gp));
> > > +       gp.param = LOCAL_I915_PARAM_MMAP_OFFSET_VERSION;
> > > +       gp.value = &has_mmap_offset;
> > > +       ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > 
> > * Blinks.
> > 
> > Oh no it isn't.
> > 
> > It's just I915_PARAM_MMAP_GTT_VERSION >= 4
> 
> I've double checked:
> 
> i915_getparam:
>         case I915_PARAM_MMAP_GTT_VERSION:
>                  -> i915_gem_mmap_gtt_version()
>                         -> return 3
> 
>         case I915_PARAM_MMAP_OFFSET_VERSION:
>                 -> return 1
> 
> Where's the magic 4 in GTT_VERSION? I don't see it.
> Is some non merged patch containing it?

You are looking at the unmerged garbage pile.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support
  2019-11-21  7:25     ` Zbigniew Kempczyński
  2019-11-21  7:30       ` Chris Wilson
@ 2019-11-21  7:34       ` Chris Wilson
  1 sibling, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-11-21  7:34 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev

Quoting Zbigniew Kempczyński (2019-11-21 07:25:20)
> On Wed, Nov 20, 2019 at 07:41:45PM +0000, Chris Wilson wrote:
> > > +bool __gem_mmap_offset__has_wc(int fd)
> > > +{
> > > +       int has_wc = 0;
> > > +
> > > +       if (gem_has_mmap_offset(fd)) {
> > 
> > Don't need the double ioctl... Hmm, oh wait. That's what you get for not
> > checking arg.pad. Gah.
> 
> You suggest we can skip gem_has_mmap_offset() call by trial by fire 
> I915_GEM_MMAP_OFFSET ioctl()?

That's what I would have done, but alas the inherited ioctl does not
allow it and we need to use the extra GETPARAM to verify MMAP_OFFSET
exists.

> > > +/**
> > > + * __gem_mmap_offset:
> > > + * @fd: open i915 drm file descriptor
> > > + * @handle: gem buffer object handle
> > > + * @offset: offset in the gem buffer of the mmap arena
> > > + * @size: size of the mmap arena
> > > + * @prot: memory protection bits as used by mmap()
> > > + * @flags: flags used to determine caching
> > > + *
> > > + * Similar to __gem_mmap but use MMAP_OFFSET IOCTL.
> > > + *
> > > + * Returns: A pointer to the created memory mapping, NULL on failure.
> > > + */
> > > +void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size,
> > > +                       unsigned int prot, uint64_t flags)
> > > +{
> > > +       struct local_i915_gem_mmap_offset arg;
> > > +       void *ptr;
> > > +
> > > +       memset(&arg, 0, sizeof(arg));
> > > +       arg.handle = handle;
> > > +       arg.offset = offset;
> > 
> > That's an interesting extension :-p
> > 
> > Yeah, being able to mmap a slice of the object is one of the extensions
> > I had in mind.
> 
> Doesn't it work as I expect? Isn't memory range verified in do_mmap()
> and after syscall our implementation does the real job then?

Nope. The mmap_offset only returns an offset valid for the whole buffer.
Technically you could mmap() a slice of that, but that's rejected in the
fault handler.

The arg.offset is an outparam only.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions
  2019-11-20 19:53   ` Chris Wilson
@ 2019-11-21  8:40     ` Zbigniew Kempczyński
  2019-11-21  8:53       ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-21  8:40 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Nov 20, 2019 at 07:53:02PM +0000, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2019-11-20 18:57:39)
> > From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > 
> > As a part of local memory effort we need to make sure, that basic
> > scenarios are covered for every available memory region. This patch is
> > an attempt for this problem. If it will be accepted it will be
> > replicated on each test that can benefit from it.
> 
> gem_exec_basic just exercises the execbuf ioctl... which does not
> mention memory region. You are thinking of gem_exec_nop and
> gem_exec_store for this first level of basic testing.
> -Chris

Drop it?

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions
  2019-11-20 20:15       ` Chris Wilson
@ 2019-11-21  8:44         ` Zbigniew Kempczyński
  0 siblings, 0 replies; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-21  8:44 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Nov 20, 2019 at 08:15:51PM +0000, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2019-11-20 20:06:40)
> > On Wed, Nov 20, 2019 at 07:45:16PM +0000, Chris Wilson wrote:
> > > Quoting Zbigniew Kempczyński (2019-11-20 18:57:37)
> > > > +/* gem_create_in_memory_region_list:
> > > > + * @fd: opened i915 drm file descriptor
> > > > + * @size: requested size of the buffer
> > > > + * @mem_regions: memory regions array (priority list)
> > > > + * @num_regions: @mem_regions length
> > > > + */
> > > > +uint32_t gem_create_in_memory_region_list(int fd, uint64_t size,
> > > > +                                         uint32_t *mem_regions,
> > > > +                                         int num_regions)
> > > > +{
> > > > +       uint32_t handle = gem_create(fd, size);
> > > > +       int ret = __gem_migrate_to_memory_regions(fd, handle,
> > > > +                                                 mem_regions, num_regions);
> > > > +       igt_assert_eq(ret, 0);
> > > 
> > > No gem_create_ext() where we can pass OBJECT_PARAM in at create time? So
> > > Joonas can later say these are construct time only properties?
> > > -Chris
> > 
> > You mean more generic wrapper than above? No problem to add one
> > but this one was for migrate bo to requested memory regions, nothing else.
> 
> No, I mean we need to extend the GEM_CREATE_IOCTL.
> -Chris

That was one of my first questions to Daniel, why we don't use .pad field
in drm_i915_gem_create. So I can only live with current API.
Do you think should I change above function or it can stay as it is?

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions
  2019-11-21  8:40     ` Zbigniew Kempczyński
@ 2019-11-21  8:53       ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-11-21  8:53 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev

Quoting Zbigniew Kempczyński (2019-11-21 08:40:11)
> On Wed, Nov 20, 2019 at 07:53:02PM +0000, Chris Wilson wrote:
> > Quoting Zbigniew Kempczyński (2019-11-20 18:57:39)
> > > From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > > 
> > > As a part of local memory effort we need to make sure, that basic
> > > scenarios are covered for every available memory region. This patch is
> > > an attempt for this problem. If it will be accepted it will be
> > > replicated on each test that can benefit from it.
> > 
> > gem_exec_basic just exercises the execbuf ioctl... which does not
> > mention memory region. You are thinking of gem_exec_nop and
> > gem_exec_store for this first level of basic testing.
> > -Chris
> 
> Drop it?

Yeah, I see gem_exec_basic as only doing the sanitychecks for the
execbuf2 ioctl itself, which basically only needs to answer the question
of whether the ioctl exists, and is usable by other tests. It such a
broad ioctl interfacing with all parts of the HW that we do need to
break it down into parts, and asking whether it is usable with all
memory regions accessible from the system, goes beyond the simple
question of whether the tests who do not care about that (and just use
the defaults handed to them) will work.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
  2019-11-20 19:50   ` Chris Wilson
@ 2019-11-21 17:07     ` Zbigniew Kempczyński
  0 siblings, 0 replies; 22+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-21 17:07 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Nov 20, 2019 at 07:50:55PM +0000, Chris Wilson wrote:
> > +igt_main
> > +{
> > +       uint8_t *addr;
> > +       uint32_t obj_size, batch_size;
> > +       uint32_t mem_type, mem_instance;
> > +       uint32_t region;
> > +       int fd;
> > +       const struct intel_memory_region *mr;
> > +       struct local_i915_query_memory_region_info *query_info;
> > +
> > +       igt_fixture {
> > +               fd = drm_open_driver(DRIVER_INTEL);
> > +               gem_require_mmap_offset(fd);
> > +               igt_require(gem_mmap__has_wc(fd));
> > +
> > +               query_info = gem_query_memory_regions(fd);
> > +               igt_assert(query_info);
> 
> This ioctl will be available before memory regions uAPI, and we will be
> expecting coverage...

You mean i915_query.c test should be extended to do similar?

> 
> gem_mmap_offset_regions
> or
> gem_memory_regions_mmap
> 
> or just igt_subtest_group {}
> 
> And do we have a plan for switching to something like i915_gem_ instead?

I tried to understand and I'm sorry I don't get what you mean.

> 
> > +       }
> > +
> > +       for (mr = intel_memory_regions; mr->region_name; mr++) {
> 
> See igt_subtest_with_dynamic

I wondered to provide macro similar to existing - for_each_memory_region.

I'm taking a look how to use dynamic test here. 

> > +               mem_type = mr->memory_type;
> > +               mem_instance = mr->memory_instance;
> > +               region = INTEL_MEMORY_REGION_ID(mem_type, mem_instance);
> > +               batch_size = gem_get_batch_size(fd, region);
> > +               obj_size = 4 * batch_size;
> > +
> > +               igt_subtest_f("bad-object-%s", mr->region_name) {
> > +                       uint32_t real_handle;
> > +                       uint32_t handles[20];
> > +                       int i = 0;
> > +
> > +                       gem_query_require_region(query_info, region);
> > +
> > +                       real_handle = gem_create_in_memory_regions(fd, obj_size,
> > +                                                                  region);
> > +
> > +                       handles[i++] = 0xdeadbeef;
> > +                       for (int bit = 0; bit < 16; bit++)
> > +                               handles[i++] = real_handle | (1 << (bit + 16));
> > +                       handles[i] = real_handle + 1;
> > +
> > +                       for (; i >= 0; i--) {
> > +                               struct local_i915_gem_mmap_offset arg = {
> > +                                       .handle = handles[i],
> > +                                       .flags = LOCAL_I915_MMAP_OFFSET_WC,
> > +                               };
> > +
> > +                               igt_debug("Trying MMAP IOCTL with handle %x\n",
> > +                                         handles[i]);
> > +                               igt_assert_eq(mmap_offset_ioctl(fd, &arg),
> > +                                             -ENOENT);
> > +                       }
> > +
> > +                       gem_close(fd, real_handle);
> > +               }
> > +
> > +               igt_subtest_f("basic-%s", mr->region_name) {
> > +                       uint32_t handle;
> > +                       uint64_t flags;
> > +                       uint8_t *expected;
> > +                       uint8_t *buf;
> > +
> > +                       gem_query_require_region(query_info, region);
> > +
> > +                       handle = gem_create_in_memory_regions(fd, obj_size,
> > +                                                             region);
> > +                       flags = LOCAL_I915_MMAP_OFFSET_WC;
> > +                       addr = __gem_mmap_offset(fd, handle, 0, obj_size,
> > +                                                PROT_READ | PROT_WRITE,
> > +                                                flags);
> > +                       igt_assert(addr);
> > +
> > +                       igt_debug("Testing contents of newly created object.\n");
> > +                       expected = calloc(obj_size, sizeof(*expected));
> > +                       igt_assert_eq(memcmp(addr, expected, obj_size), 0);
> > +                       free(expected);
> > +
> > +                       igt_debug("Testing coherency of writes and mmap reads.\n");
> > +                       buf = calloc(obj_size, sizeof(*buf));
> > +                       memset(buf + 1024, 0x01, 1024);
> > +                       gem_write(fd, handle, 0, buf, obj_size);
> > +                       igt_assert_eq(memcmp(buf, addr, obj_size), 0);
> > +
> > +                       igt_debug("Testing that mapping stays after close\n");
> > +                       gem_close(fd, handle);
> > +                       igt_assert_eq(memcmp(buf, addr, obj_size), 0);
> > +                       free(buf);
> > +
> > +                       igt_debug("Testing unmapping\n");
> > +                       munmap(addr, obj_size);
> > +               }
> > +
> > +               igt_subtest_f("short-mmap-%s", mr->region_name) {
> > +                       uint32_t handle;
> 
> Not supported in vanilla ioctl, coming to an extension near you :)
> -Chris
Hmm, still not get it :/

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-11-21 17:07 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 18:57 [igt-dev] [PATCH i-g-t v3 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
2019-11-20 19:41   ` Chris Wilson
2019-11-21  7:25     ` Zbigniew Kempczyński
2019-11-21  7:30       ` Chris Wilson
2019-11-21  7:34       ` Chris Wilson
2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 2/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
2019-11-20 19:43   ` Chris Wilson
2019-11-20 19:58     ` Zbigniew Kempczyński
2019-11-20 19:45   ` Chris Wilson
2019-11-20 20:06     ` Zbigniew Kempczyński
2019-11-20 20:15       ` Chris Wilson
2019-11-21  8:44         ` Zbigniew Kempczyński
2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 3/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
2019-11-20 19:50   ` Chris Wilson
2019-11-21 17:07     ` Zbigniew Kempczyński
2019-11-20 18:57 ` [igt-dev] [PATCH i-g-t v3 4/4] tests/i915/gem_exec_basic: Iterate over all memory regions Zbigniew Kempczyński
2019-11-20 19:53   ` Chris Wilson
2019-11-21  8:40     ` Zbigniew Kempczyński
2019-11-21  8:53       ` Chris Wilson
2019-11-20 19:26 ` [igt-dev] ✗ GitLab.Pipeline: failure for Basic LMEM support in IGT (rev3) Patchwork
2019-11-20 19:35 ` [igt-dev] ✗ Fi.CI.BAT: " Patchwork

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.