* [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT
@ 2019-11-22 8:13 Zbigniew Kempczyński
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI Zbigniew Kempczyński
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 8:13 UTC (permalink / raw)
To: igt-dev
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().
v4: If memory region query is not supported use probe()
as a fallback to system and device memory regions.
v5: Quickfix - removing override which enforces probe()
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 (3):
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
Zbigniew Kempczyński (1):
include/drm-uapi/i915_drm: Add local defs for memory region uAPI
include/drm-uapi/i915_drm.h | 92 ++++++++
lib/Makefile.sources | 2 +
lib/i915/gem_mman.c | 225 ++++++++++++++++----
lib/i915/gem_mman.h | 18 +-
lib/i915/intel_memory_region.c | 369 +++++++++++++++++++++++++++++++++
lib/i915/intel_memory_region.h | 88 ++++++++
lib/ioctl_wrappers.h | 1 +
lib/meson.build | 1 +
tests/Makefile.sources | 3 +
tests/i915/gem_mmap_offset.c | 163 +++++++++++++++
tests/meson.build | 1 +
11 files changed, 923 insertions(+), 40 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] 15+ messages in thread
* [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI
2019-11-22 8:13 [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
@ 2019-11-22 8:13 ` Zbigniew Kempczyński
2019-11-22 8:56 ` Chris Wilson
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 8:13 UTC (permalink / raw)
To: igt-dev
Contains local definitions and structures for memory region uAPI
until upstream changes will be merged.
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
include/drm-uapi/i915_drm.h | 92 +++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index ab899abb..3accb290 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -2245,6 +2245,98 @@ struct drm_i915_query_perf_config {
__u8 data[];
};
+/*
+ * Local structures and definitions to be removed on upstream merge
+ */
+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_GTT 0
+#define LOCAL_I915_MMAP_OFFSET_WC 1
+#define LOCAL_I915_MMAP_OFFSET_WB 2
+#define LOCAL_I915_MMAP_OFFSET_UC 3
+
+ __u64 extensions;
+};
+
+#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
+#define LOCAL_I915_STOLEN_MEMORY 2
+
+ /** 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)
+
#if defined(__cplusplus)
}
#endif
--
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] 15+ messages in thread
* [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support
2019-11-22 8:13 [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI Zbigniew Kempczyński
@ 2019-11-22 8:13 ` Zbigniew Kempczyński
2019-11-22 8:59 ` Chris Wilson
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 8:13 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 | 225 ++++++++++++++++++++++++++++++++++++--------
lib/i915/gem_mman.h | 18 +++-
2 files changed, 203 insertions(+), 40 deletions(-)
diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
index 6256627b..8d6db0d1 100644
--- a/lib/i915/gem_mman.c
+++ b/lib/i915/gem_mman.c
@@ -40,6 +40,33 @@
#define VG(x) do {} while (0)
#endif
+#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
+
+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 = >t_version;
+ ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+ return gtt_version;
+}
+
+static bool gem_has_mmap_offset(int fd)
+{
+ int gtt_version = gem_mmap_gtt_version(fd);
+
+ return gtt_version >= 4;
+}
+
+void gem_require_mmap_offset(int i915)
+{
+ igt_require(gem_has_mmap_offset(i915));
+}
+
/**
* __gem_mmap__gtt:
* @fd: open i915 drm file descriptor
@@ -101,46 +128,60 @@ 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 = >t_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;
+ 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,11 +198,13 @@ 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;
+ igt_assert(offset == 0);
+
memset(&arg, 0, sizeof(arg));
arg.handle = handle;
arg.offset = offset;
@@ -177,6 +220,47 @@ 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
+ *
+ * Mmap the gem buffer memory on offset returned in GEM_MMAP_OFFSET ioctl.
+ * Offset argument passed in function call must be 0. In the future
+ * when driver will allow slice mapping of buffer object this restriction
+ * will be removed.
+ *
+ * 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;
+
+ igt_assert(offset == 0);
+
+ memset(&arg, 0, sizeof(arg));
+ arg.handle = handle;
+ 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 +278,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 +294,68 @@ 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()
+ *
+ * Returns: A pointer to a block of linear device memory mapped into the
+ * process with WC semantics. When no WC is available try to mmap using GGTT.
+ */
+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.
+ * Offset argument passed in function call must be 0. In the future
+ * when driver will allow slice mapping of buffer object this restriction
+ * will be removed.
+ *
+ * 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;
+
+ igt_assert(offset == 0);
+
+ ptr = gem_mmap__device_coherent(fd, handle, offset, size, prot);
igt_assert(ptr);
+
return ptr;
}
@@ -248,7 +391,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..fe1299c0 100644
--- a/lib/i915/gem_mman.h
+++ b/lib/i915/gem_mman.h
@@ -25,12 +25,22 @@
#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)
+
+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 +48,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] 15+ messages in thread
* [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions
2019-11-22 8:13 [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI Zbigniew Kempczyński
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
@ 2019-11-22 8:13 ` Zbigniew Kempczyński
2019-11-22 9:11 ` Chris Wilson
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 4/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
2019-11-22 8:54 ` [igt-dev] ✗ Fi.CI.BAT: failure for Basic LMEM support in IGT (rev5) Patchwork
4 siblings, 1 reply; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 8:13 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.
v2: when driver doesn't support i915_query supports memory region
switch to fallback and probe system and device memory regions.
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 | 369 +++++++++++++++++++++++++++++++++
lib/i915/intel_memory_region.h | 88 ++++++++
lib/ioctl_wrappers.h | 1 +
lib/meson.build | 1 +
5 files changed, 461 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..56f6c527
--- /dev/null
+++ b/lib/i915/intel_memory_region.c
@@ -0,0 +1,369 @@
+/*
+ * 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)
+
+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_item(int fd, struct drm_i915_query_item *item)
+{
+ struct drm_i915_query q = {
+ .num_items = 1,
+ .items_ptr = to_user_pointer(item),
+ };
+
+ 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 },
+ { "STLN", LOCAL_I915_STOLEN_MEMORY, 0 },
+ { NULL, 0, 0 },
+};
+
+const char *memory_region_name(uint32_t region) {
+ uint32_t type;
+
+ type = MEMORY_TYPE_FROM_REGION(region);
+
+ return intel_memory_regions[type].region_name;
+}
+/**
+ * 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. To be fixed after memory region page size detection
+ * patch will be merged.
+ */
+uint32_t gem_get_batch_size(int fd, uint32_t region)
+{
+ return IS_DEVICE_MEMORY_REGION(region) ? SZ_64K : SZ_4K;
+}
+
+static bool __try_gem_create_in_region(int fd, uint32_t region)
+{
+ uint32_t size, handle;
+ int ret;
+
+ size = gem_get_batch_size(fd, region);
+ handle = gem_create(fd, size);
+ ret = gem_migrate_to_memory_region(fd, handle, region);
+
+ gem_close(fd, handle);
+
+ return !ret;
+}
+
+static struct local_i915_query_memory_region_info *__probe_regions(int fd)
+{
+ struct local_i915_query_memory_region_info *query_info;
+ uint32_t length = sizeof(*query_info);
+ uint32_t *regions = NULL;
+ int max_regions = INTEL_MEMORY_TYPE_SHIFT;
+ int num_regions = 0;
+
+ for (int i = 0; intel_memory_regions[i].region_name; i++) {
+ uint8_t type;
+
+ /* We're not interested of other memory region types */
+ type = intel_memory_regions[i].memory_type;
+ if (type != LOCAL_I915_SYSTEM_MEMORY &&
+ type != LOCAL_I915_DEVICE_MEMORY)
+ continue;
+
+ for (int instance = 0; instance <= max_regions; instance++) {
+ uint32_t region;
+ int exists;
+
+ region = INTEL_MEMORY_REGION_ID(type, instance);
+ igt_debug("Probing memory region: %08x\n", region);
+ exists = __try_gem_create_in_region(fd, region);
+ if (!exists)
+ break;
+
+ igt_debug("Region %08x exists\n", region);
+ regions = realloc(regions, (num_regions + 1) *
+ sizeof(uint32_t));
+ igt_assert(regions);
+ regions[num_regions++] = region;
+ }
+ }
+
+ if (!num_regions)
+ return NULL;
+
+ length += sizeof(struct local_i915_memory_region_info) * num_regions;
+ query_info = calloc(1, length);
+ igt_assert(query_info);
+
+ /* Prepare query_info structure with regions id filled */
+ for (int i = 0; i < num_regions; i++) {
+ query_info->regions[i].id = regions[i];
+ query_info->regions[i].size = 0;
+ }
+ free(regions);
+ query_info->num_regions = num_regions;
+
+ return query_info;
+}
+
+/**
+ * gem_query_memory_regions:
+ * @fd: open i915 drm file descriptor
+ *
+ * This function wraps query mechanism for memory regions. If there's
+ * no i915_query probe memory regions and return same structure as from query.
+ *
+ * Returns: Filled struct with available memory regions. Allocates structure
+ * which must by freed by caller.
+ */
+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;
+ int ret;
+
+ memset(&item, 0, sizeof(item));
+ item.query_id = LOCAL_I915_QUERY_MEMREGION_INFO;
+
+ ret = __i915_query_item(fd, &item);
+ if (!ret) {
+ query_info = calloc(1, item.length);
+ igt_assert(query_info);
+
+ item.data_ptr = to_user_pointer(query_info);
+ __i915_query_item(fd, &item);
+
+ return query_info;
+ }
+
+ /* Fallback, there's no memregion i915_query supported. Try probe(). */
+ query_info = __probe_regions(fd);
+ igt_assert(query_info);
+
+ 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, uint32_t 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, uint32_t handle, uint32_t region)
+{
+ return __gem_migrate_to_memory_regions(fd, handle, ®ion, 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, uint32_t 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, uint32_t 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..5e453a13
--- /dev/null
+++ b/lib/i915/intel_memory_region.h
@@ -0,0 +1,88 @@
+/*
+ * 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)
+#define IS_STOLEN_MEMORY_REGION(region) \
+ (MEMORY_TYPE_FROM_REGION(region) == LOCAL_I915_STOLEN_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)
+
+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);
+const char *memory_region_name(uint32_t region);
+
+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, uint32_t handle, uint32_t region);
+int gem_migrate_to_lmem(int fd, uint32_t handle);
+int gem_migrate_to_smem(int fd, uint32_t 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__)); \
+})
+
+#define for_each_memory_region(query_info__, region__) \
+ for (int __r = 0; \
+ region__ = &(query_info__->regions[__r]), \
+ __r < query_info__->num_regions; \
+ __r++)
+
+#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] 15+ messages in thread
* [igt-dev] [PATCH i-g-t v5 4/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
2019-11-22 8:13 [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
` (2 preceding siblings ...)
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
@ 2019-11-22 8:13 ` Zbigniew Kempczyński
2019-11-22 9:16 ` Chris Wilson
2019-11-22 8:54 ` [igt-dev] ✗ Fi.CI.BAT: failure for Basic LMEM support in IGT (rev5) Patchwork
4 siblings, 1 reply; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 8:13 UTC (permalink / raw)
To: igt-dev
From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Few simple tests which tries to create / mmap buffer objects
in system / device 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 | 163 +++++++++++++++++++++++++++++++++++
tests/meson.build | 1 +
3 files changed, 167 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..a4432f93
--- /dev/null
+++ b/tests/i915/gem_mmap_offset.c
@@ -0,0 +1,163 @@
+/*
+ * 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;
+}
+
+static void
+mmap_offset_bad_object(int fd, struct local_i915_query_memory_region_info *regions)
+{
+ const char *region_name;
+ struct local_i915_memory_region_info *mem_region;
+ uint32_t region, obj_size, batch_size, real_handle;
+ uint32_t handles[20];
+
+ for_each_memory_region(regions, mem_region) {
+ int i = 0;
+
+ region = mem_region->id;
+ region_name = memory_region_name(region);
+ if (IS_STOLEN_MEMORY_REGION(region)) {
+ continue;
+ }
+ igt_debug("mmap-offset-bad-object: region name: %s, "
+ "region: %08x\n", region_name, region);
+
+ batch_size = gem_get_batch_size(fd, region);
+ obj_size = 4 * batch_size;
+ 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);
+ }
+}
+
+static void
+mmap_offset_basic(int fd, struct local_i915_query_memory_region_info *regions)
+{
+ const char *region_name;
+ struct local_i915_memory_region_info *mem_region;
+ uint64_t flags;
+ uint32_t region, handle, obj_size;
+ uint8_t *expected, *buf, *addr;
+
+ for_each_memory_region(regions, mem_region) {
+ region = mem_region->id;
+ region_name = memory_region_name(region);
+ if (IS_STOLEN_MEMORY_REGION(region)) {
+ continue;
+ }
+ igt_debug("mmap-offset-basic: region name: %s, region: %08x\n",
+ region_name, region);
+
+ obj_size = 4 * gem_get_batch_size(fd, 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_main
+{
+ int fd;
+ struct local_i915_query_memory_region_info *regions;
+
+ igt_fixture {
+ fd = drm_open_driver(DRIVER_INTEL);
+ gem_require_mmap_offset(fd);
+ igt_require(gem_mmap__has_wc(fd));
+
+ regions = gem_query_memory_regions(fd);
+ igt_assert(regions);
+ }
+
+ igt_describe("Verify mapping to invalid gem objects won't be created"
+ " in different memory regions");
+ igt_subtest_f("mmap-offset-bad-object")
+ mmap_offset_bad_object(fd, regions);
+
+ igt_describe("Check buffer object mapping is coherent with "
+ "data written by gem_write in all memory regions");
+ igt_subtest_f("mmap-offset-basic")
+ mmap_offset_basic(fd, regions);
+
+ igt_fixture {
+ free(regions);
+ 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] 15+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for Basic LMEM support in IGT (rev5)
2019-11-22 8:13 [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
` (3 preceding siblings ...)
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 4/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
@ 2019-11-22 8:54 ` Patchwork
4 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-11-22 8:54 UTC (permalink / raw)
To: Lukasz Kalamarz; +Cc: igt-dev
== Series Details ==
Series: Basic LMEM support in IGT (rev5)
URL : https://patchwork.freedesktop.org/series/65171/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7402 -> IGTPW_3748
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_3748 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_3748, 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_3748/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_3748:
### IGT changes ###
#### Possible regressions ####
* igt@gem_exec_gttfill@basic:
- fi-bsw-kefka: [PASS][1] -> [TIMEOUT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bsw-kefka/igt@gem_exec_gttfill@basic.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bsw-kefka/igt@gem_exec_gttfill@basic.html
- fi-hsw-4770: [PASS][3] -> [DMESG-FAIL][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-hsw-4770/igt@gem_exec_gttfill@basic.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-hsw-4770/igt@gem_exec_gttfill@basic.html
- fi-byt-n2820: [PASS][5] -> [TIMEOUT][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-byt-n2820/igt@gem_exec_gttfill@basic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-byt-n2820/igt@gem_exec_gttfill@basic.html
- fi-snb-2600: [PASS][7] -> [DMESG-FAIL][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-snb-2600/igt@gem_exec_gttfill@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-snb-2600/igt@gem_exec_gttfill@basic.html
- fi-ilk-650: [PASS][9] -> [DMESG-FAIL][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-ilk-650/igt@gem_exec_gttfill@basic.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-ilk-650/igt@gem_exec_gttfill@basic.html
* igt@gem_mmap_gtt@basic-small-bo-tiledx:
- fi-blb-e6850: [PASS][11] -> [FAIL][12] +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-blb-e6850/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-blb-e6850/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
- fi-snb-2520m: [PASS][13] -> [FAIL][14] +1 similar issue
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-snb-2520m/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-snb-2520m/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
* igt@gem_mmap_gtt@basic-small-bo-tiledy:
- fi-bwr-2160: [PASS][15] -> [FAIL][16] +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bwr-2160/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bwr-2160/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-kbl-guc: [PASS][17] -> [FAIL][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-bsw-nick: [PASS][19] -> [FAIL][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bsw-nick/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bsw-nick/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-kbl-r: [PASS][21] -> [FAIL][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-r/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-r/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-kbl-soraka: [PASS][23] -> [FAIL][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-soraka/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-soraka/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-icl-u3: [PASS][25] -> [FAIL][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-skl-6770hq: [PASS][27] -> [FAIL][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-6770hq/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-6770hq/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-cfl-guc: [PASS][29] -> [FAIL][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cfl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cfl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-skl-lmem: NOTRUN -> [FAIL][31]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-lmem/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-byt-j1900: [PASS][32] -> [FAIL][33]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-byt-j1900/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-byt-j1900/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-skl-6700k2: [PASS][34] -> [FAIL][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-6700k2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-6700k2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-cfl-8700k: [PASS][36] -> [FAIL][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cfl-8700k/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cfl-8700k/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-apl-guc: [PASS][38] -> [FAIL][39]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-apl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-apl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-icl-dsi: [PASS][40] -> [FAIL][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-bdw-5557u: [PASS][42] -> [FAIL][43]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bdw-5557u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bdw-5557u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-pnv-d510: [PASS][44] -> [FAIL][45]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-pnv-d510/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-pnv-d510/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-skl-6600u: [PASS][46] -> [FAIL][47]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-6600u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-6600u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-skl-guc: [PASS][48] -> [FAIL][49]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-guc/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-icl-y: [PASS][50] -> [FAIL][51]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-y/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-y/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-hsw-4770r: [PASS][52] -> [FAIL][53] +1 similar issue
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-hsw-4770r/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-hsw-4770r/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-kbl-7500u: [PASS][54] -> [FAIL][55]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-7500u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-7500u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-hsw-peppy: [PASS][56] -> [FAIL][57]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-hsw-peppy/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-hsw-peppy/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-kbl-8809g: [PASS][58] -> [FAIL][59]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-8809g/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-8809g/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-glk-dsi: [PASS][60] -> [FAIL][61]
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-glk-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-glk-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-kbl-x1275: [PASS][62] -> [FAIL][63]
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-x1275/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-x1275/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-bsw-n3050: NOTRUN -> [FAIL][64]
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bsw-n3050/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-ivb-3770: [PASS][65] -> [FAIL][66]
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-ivb-3770/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-ivb-3770/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-cml-u2: [PASS][67] -> [FAIL][68]
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cml-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cml-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-icl-u2: [PASS][69] -> [FAIL][70]
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-u2/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-bxt-dsi: [PASS][71] -> [FAIL][72]
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bxt-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bxt-dsi/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
- fi-whl-u: [PASS][73] -> [FAIL][74]
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-whl-u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-whl-u/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
* igt@gem_workarounds@basic-read:
- fi-icl-dsi: [PASS][75] -> [TIMEOUT][76]
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-dsi/igt@gem_workarounds@basic-read.html
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-dsi/igt@gem_workarounds@basic-read.html
- fi-cml-u2: [PASS][77] -> [TIMEOUT][78]
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cml-u2/igt@gem_workarounds@basic-read.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cml-u2/igt@gem_workarounds@basic-read.html
- fi-icl-y: [PASS][79] -> [TIMEOUT][80]
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-y/igt@gem_workarounds@basic-read.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-y/igt@gem_workarounds@basic-read.html
- fi-icl-u3: [PASS][81] -> [TIMEOUT][82]
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-u3/igt@gem_workarounds@basic-read.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-u3/igt@gem_workarounds@basic-read.html
- fi-glk-dsi: [PASS][83] -> [CRASH][84] +1 similar issue
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-glk-dsi/igt@gem_workarounds@basic-read.html
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-glk-dsi/igt@gem_workarounds@basic-read.html
- fi-icl-u2: [PASS][85] -> [TIMEOUT][86]
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-u2/igt@gem_workarounds@basic-read.html
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-u2/igt@gem_workarounds@basic-read.html
- fi-bsw-nick: [PASS][87] -> [CRASH][88]
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bsw-nick/igt@gem_workarounds@basic-read.html
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bsw-nick/igt@gem_workarounds@basic-read.html
- fi-bdw-5557u: [PASS][89] -> [TIMEOUT][90]
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bdw-5557u/igt@gem_workarounds@basic-read.html
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bdw-5557u/igt@gem_workarounds@basic-read.html
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u3: [PASS][91] -> [CRASH][92]
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
- fi-bsw-n3050: NOTRUN -> [CRASH][93] +1 similar issue
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bsw-n3050/igt@kms_frontbuffer_tracking@basic.html
- fi-kbl-soraka: [PASS][94] -> [CRASH][95]
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-soraka/igt@kms_frontbuffer_tracking@basic.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-soraka/igt@kms_frontbuffer_tracking@basic.html
- fi-bxt-dsi: [PASS][96] -> [CRASH][97] +1 similar issue
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bxt-dsi/igt@kms_frontbuffer_tracking@basic.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bxt-dsi/igt@kms_frontbuffer_tracking@basic.html
- fi-ivb-3770: [PASS][98] -> [CRASH][99]
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-ivb-3770/igt@kms_frontbuffer_tracking@basic.html
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-ivb-3770/igt@kms_frontbuffer_tracking@basic.html
- fi-skl-6700k2: [PASS][100] -> [CRASH][101]
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-6700k2/igt@kms_frontbuffer_tracking@basic.html
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-6700k2/igt@kms_frontbuffer_tracking@basic.html
- fi-cml-u2: [PASS][102] -> [CRASH][103]
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
- fi-apl-guc: [PASS][104] -> [CRASH][105] +1 similar issue
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-apl-guc/igt@kms_frontbuffer_tracking@basic.html
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-apl-guc/igt@kms_frontbuffer_tracking@basic.html
- fi-byt-j1900: [PASS][106] -> [CRASH][107]
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-byt-j1900/igt@kms_frontbuffer_tracking@basic.html
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-byt-j1900/igt@kms_frontbuffer_tracking@basic.html
- fi-skl-6600u: [PASS][108] -> [CRASH][109]
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-6600u/igt@kms_frontbuffer_tracking@basic.html
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-6600u/igt@kms_frontbuffer_tracking@basic.html
- fi-icl-dsi: [PASS][110] -> [CRASH][111]
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
- fi-bdw-5557u: [PASS][112] -> [CRASH][113]
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-bdw-5557u/igt@kms_frontbuffer_tracking@basic.html
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-bdw-5557u/igt@kms_frontbuffer_tracking@basic.html
- fi-kbl-r: [PASS][114] -> [CRASH][115]
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-r/igt@kms_frontbuffer_tracking@basic.html
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-r/igt@kms_frontbuffer_tracking@basic.html
- fi-icl-y: [PASS][116] -> [CRASH][117]
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-icl-y/igt@kms_frontbuffer_tracking@basic.html
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-icl-y/igt@kms_frontbuffer_tracking@basic.html
- fi-hsw-4770r: [PASS][118] -> [CRASH][119]
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-hsw-4770r/igt@kms_frontbuffer_tracking@basic.html
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-hsw-4770r/igt@kms_frontbuffer_tracking@basic.html
- fi-hsw-peppy: [PASS][120] -> [CRASH][121]
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
- fi-skl-6770hq: [PASS][122] -> [CRASH][123]
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-skl-6770hq/igt@kms_frontbuffer_tracking@basic.html
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-6770hq/igt@kms_frontbuffer_tracking@basic.html
- fi-cfl-guc: [PASS][124] -> [CRASH][125]
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cfl-guc/igt@kms_frontbuffer_tracking@basic.html
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cfl-guc/igt@kms_frontbuffer_tracking@basic.html
- fi-skl-lmem: NOTRUN -> [CRASH][126]
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-skl-lmem/igt@kms_frontbuffer_tracking@basic.html
- fi-snb-2520m: [PASS][127] -> [CRASH][128]
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-snb-2520m/igt@kms_frontbuffer_tracking@basic.html
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-snb-2520m/igt@kms_frontbuffer_tracking@basic.html
- fi-whl-u: [PASS][129] -> [CRASH][130]
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-whl-u/igt@kms_frontbuffer_tracking@basic.html
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-whl-u/igt@kms_frontbuffer_tracking@basic.html
- fi-cfl-8700k: [PASS][131] -> [CRASH][132]
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-cfl-8700k/igt@kms_frontbuffer_tracking@basic.html
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-cfl-8700k/igt@kms_frontbuffer_tracking@basic.html
- fi-kbl-7500u: [PASS][133] -> [CRASH][134]
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7402/fi-kbl-7500u/igt@kms_frontbuffer_tracking@basic.html
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/fi-kbl-7500u/igt@kms_frontbuffer_tracking@basic.html
- fi-skl-guc: [PASS][135] -> [CRASH][1
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3748/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI Zbigniew Kempczyński
@ 2019-11-22 8:56 ` Chris Wilson
0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2019-11-22 8:56 UTC (permalink / raw)
To: Zbigniew Kempczyński, igt-dev
Quoting Zbigniew Kempczyński (2019-11-22 08:13:54)
> Contains local definitions and structures for memory region uAPI
> until upstream changes will be merged.
Just use the expected names in i915_drm.h (ideally this would be a copy
of the kernel headers from the combined series) so we have less fixing to
do later.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
@ 2019-11-22 8:59 ` Chris Wilson
2019-11-22 9:12 ` Zbigniew Kempczyński
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-11-22 8:59 UTC (permalink / raw)
To: Zbigniew Kempczyński, igt-dev
Quoting Zbigniew Kempczyński (2019-11-22 08:13:55)
> 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 | 225 ++++++++++++++++++++++++++++++++++++--------
> lib/i915/gem_mman.h | 18 +++-
> 2 files changed, 203 insertions(+), 40 deletions(-)
>
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 6256627b..8d6db0d1 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -40,6 +40,33 @@
> #define VG(x) do {} while (0)
> #endif
>
> +#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
Leftover.
> +
> +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 = >t_version;
> + ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +
> + return gtt_version;
> +}
> +
> +static bool gem_has_mmap_offset(int fd)
> +{
> + int gtt_version = gem_mmap_gtt_version(fd);
> +
> + return gtt_version >= 4;
> +}
> +
> +void gem_require_mmap_offset(int i915)
> +{
> + igt_require(gem_has_mmap_offset(i915));
> +}
> +
> /**
> * __gem_mmap__gtt:
> * @fd: open i915 drm file descriptor
> @@ -101,46 +128,60 @@ 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 = >t_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;
> + 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,11 +198,13 @@ 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;
>
> + igt_assert(offset == 0);
> +
> memset(&arg, 0, sizeof(arg));
> arg.handle = handle;
> arg.offset = offset;
> @@ -177,6 +220,47 @@ 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
> + *
> + * Mmap the gem buffer memory on offset returned in GEM_MMAP_OFFSET ioctl.
> + * Offset argument passed in function call must be 0. In the future
> + * when driver will allow slice mapping of buffer object this restriction
> + * will be removed.
> + *
> + * 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;
> +
> + igt_assert(offset == 0);
> +
> + memset(&arg, 0, sizeof(arg));
> + arg.handle = handle;
> + 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);
I am tempted to say arg.offset + offset, but that is premature until an
actual slice API is decided upon.
> +
> + if (ptr == MAP_FAILED)
> + ptr = NULL;
> + else
> + errno = 0;
> +
> + return ptr;
> +}
> +
> /**
> * __gem_mmap__wc:
> * @fd: open i915 drm file descriptor
> @@ -194,7 +278,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 +294,68 @@ 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()
> + *
> + * Returns: A pointer to a block of linear device memory mapped into the
> + * process with WC semantics. When no WC is available try to mmap using GGTT.
> + */
> +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.
> + * Offset argument passed in function call must be 0. In the future
> + * when driver will allow slice mapping of buffer object this restriction
> + * will be removed.
> + *
> + * 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;
> +
> + igt_assert(offset == 0);
> +
> + ptr = gem_mmap__device_coherent(fd, handle, offset, size, prot);
> igt_assert(ptr);
> +
> return ptr;
> }
>
> @@ -248,7 +391,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..fe1299c0 100644
> --- a/lib/i915/gem_mman.h
> +++ b/lib/i915/gem_mman.h
> @@ -25,12 +25,22 @@
> #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)
> +
> +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 +48,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);
This looks to be a reasonable step forward that should at least cover us
for the transition period.
local_* notwithstanding,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
@ 2019-11-22 9:11 ` Chris Wilson
2019-11-22 9:33 ` Zbigniew Kempczyński
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-11-22 9:11 UTC (permalink / raw)
To: Zbigniew Kempczyński, igt-dev
Quoting Zbigniew Kempczyński (2019-11-22 08:13:56)
> 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.
>
> v2: when driver doesn't support i915_query supports memory region
> switch to fallback and probe system and device memory regions.
>
> 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 | 369 +++++++++++++++++++++++++++++++++
> lib/i915/intel_memory_region.h | 88 ++++++++
> lib/ioctl_wrappers.h | 1 +
> lib/meson.build | 1 +
> 5 files changed, 461 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..56f6c527
> --- /dev/null
> +++ b/lib/i915/intel_memory_region.c
> @@ -0,0 +1,369 @@
> +/*
> + * 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)
> +
> +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_item(int fd, struct drm_i915_query_item *item)
> +{
> + struct drm_i915_query q = {
> + .num_items = 1,
> + .items_ptr = to_user_pointer(item),
> + };
> +
> + 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 },
I love how the friendly names are so terse and do not follow the api /s
> + { "LMEM", LOCAL_I915_DEVICE_MEMORY, 0 },
> + { "STLN", LOCAL_I915_STOLEN_MEMORY, 0 },
> + { NULL, 0, 0 },
> +};
> +
> +const char *memory_region_name(uint32_t region) {
> + uint32_t type;
> +
> + type = MEMORY_TYPE_FROM_REGION(region);
We will never end up with garbage here, but we are so polite in our
testing.
> +
> + return intel_memory_regions[type].region_name;
> +}
> +/**
> + * 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. To be fixed after memory region page size detection
> + * patch will be merged.
> + */
> +uint32_t gem_get_batch_size(int fd, uint32_t region)
> +{
> + return IS_DEVICE_MEMORY_REGION(region) ? SZ_64K : SZ_4K;
> +}
> +
> +static bool __try_gem_create_in_region(int fd, uint32_t region)
> +{
> + uint32_t size, handle;
> + int ret;
> +
> + size = gem_get_batch_size(fd, region);
size=1 will always be correct, that has now been enshrined in the ABI.
> + handle = gem_create(fd, size);
> + ret = gem_migrate_to_memory_region(fd, handle, region);
Migrate is definitely not supported in the first wave of landings;
memory placement will be a construction time property with possible
later extension to runtime.
> +
> + gem_close(fd, handle);
> +
> + return !ret;
> +}
> +
> +static struct local_i915_query_memory_region_info *__probe_regions(int fd)
> +{
> + struct local_i915_query_memory_region_info *query_info;
> + uint32_t length = sizeof(*query_info);
> + uint32_t *regions = NULL;
> + int max_regions = INTEL_MEMORY_TYPE_SHIFT;
> + int num_regions = 0;
> +
> + for (int i = 0; intel_memory_regions[i].region_name; i++) {
> + uint8_t type;
> +
> + /* We're not interested of other memory region types */
> + type = intel_memory_regions[i].memory_type;
> + if (type != LOCAL_I915_SYSTEM_MEMORY &&
> + type != LOCAL_I915_DEVICE_MEMORY)
> + continue;
> +
> + for (int instance = 0; instance <= max_regions; instance++) {
> + uint32_t region;
> + int exists;
> +
> + region = INTEL_MEMORY_REGION_ID(type, instance);
> + igt_debug("Probing memory region: %08x\n", region);
> + exists = __try_gem_create_in_region(fd, region);
> + if (!exists)
> + break;
> +
> + igt_debug("Region %08x exists\n", region);
> + regions = realloc(regions, (num_regions + 1) *
> + sizeof(uint32_t));
> + igt_assert(regions);
> + regions[num_regions++] = region;
> + }
> + }
> +
> + if (!num_regions)
> + return NULL;
> +
> + length += sizeof(struct local_i915_memory_region_info) * num_regions;
> + query_info = calloc(1, length);
> + igt_assert(query_info);
> +
> + /* Prepare query_info structure with regions id filled */
> + for (int i = 0; i < num_regions; i++) {
> + query_info->regions[i].id = regions[i];
> + query_info->regions[i].size = 0;
> + }
> + free(regions);
> + query_info->num_regions = num_regions;
> +
> + return query_info;
> +}
> +
> +/**
> + * gem_query_memory_regions:
> + * @fd: open i915 drm file descriptor
> + *
> + * This function wraps query mechanism for memory regions. If there's
> + * no i915_query probe memory regions and return same structure as from query.
> + *
> + * Returns: Filled struct with available memory regions. Allocates structure
> + * which must by freed by caller.
> + */
> +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;
> + int ret;
> +
> + memset(&item, 0, sizeof(item));
> + item.query_id = LOCAL_I915_QUERY_MEMREGION_INFO;
> +
> + ret = __i915_query_item(fd, &item);
> + if (!ret) {
> + query_info = calloc(1, item.length);
> + igt_assert(query_info);
> +
> + item.data_ptr = to_user_pointer(query_info);
> + __i915_query_item(fd, &item);
> +
> + return query_info;
> + }
> +
> + /* Fallback, there's no memregion i915_query supported. Try probe(). */
> + query_info = __probe_regions(fd);
Oh, we will not have memory regions without a query. If anything we will
get memory regions described before you can make a choice.
> + igt_assert(query_info);
> +
> + 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);
Meh. Didn't need the heap.
> + 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;
That seems even more of a waste of heap, as above.
> +}
> +
> +/**
> + * 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,
Remind me to have words with the naming committee.
The rest is for an interface that has yet to be seen and discussed, so
shelving for later.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support
2019-11-22 8:59 ` Chris Wilson
@ 2019-11-22 9:12 ` Zbigniew Kempczyński
0 siblings, 0 replies; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 9:12 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
On Fri, Nov 22, 2019 at 08:59:10AM +0000, Chris Wilson wrote:
> > +#define LOCAL_I915_PARAM_MMAP_OFFSET_VERSION 54
>
> Leftover.
You're right.
> > +/**
> > + * __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
> > + *
> > + * Mmap the gem buffer memory on offset returned in GEM_MMAP_OFFSET ioctl.
> > + * Offset argument passed in function call must be 0. In the future
> > + * when driver will allow slice mapping of buffer object this restriction
> > + * will be removed.
> > + *
> > + * 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;
> > +
> > + igt_assert(offset == 0);
> > +
> > + memset(&arg, 0, sizeof(arg));
> > + arg.handle = handle;
> > + 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);
>
> I am tempted to say arg.offset + offset, but that is premature until an
> actual slice API is decided upon.
As I'm going to remove LOCAL_* define above so I will add this to the call.
>
> This looks to be a reasonable step forward that should at least cover us
> for the transition period.
>
> local_* notwithstanding,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
Thanks. I'm fixing above and add R-B.
Zbigniew
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 4/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 4/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
@ 2019-11-22 9:16 ` Chris Wilson
0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2019-11-22 9:16 UTC (permalink / raw)
To: Zbigniew Kempczyński, igt-dev
Quoting Zbigniew Kempczyński (2019-11-22 08:13:57)
> From: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
>
> Few simple tests which tries to create / mmap buffer objects
> in system / device 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>
Fwiw, this is not the gem_mmap_offset I need to land the kernel patches
to implement gem_mmap_offset that are pretty much ready and postdue.
I pretty much need the union of gem_mmap_wc and gem_mmap_gtt.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions
2019-11-22 9:11 ` Chris Wilson
@ 2019-11-22 9:33 ` Zbigniew Kempczyński
2019-11-22 9:46 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 9:33 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
On Fri, Nov 22, 2019 at 09:11:46AM +0000, Chris Wilson wrote:
> > +const struct intel_memory_region intel_memory_regions[] = {
> > + { "SMEM", LOCAL_I915_SYSTEM_MEMORY, 0 },
>
> I love how the friendly names are so terse and do not follow the api /s
>
> > + { "LMEM", LOCAL_I915_DEVICE_MEMORY, 0 },
> > + { "STLN", LOCAL_I915_STOLEN_MEMORY, 0 },
> > + { NULL, 0, 0 },
> > +};
> > +
> > +const char *memory_region_name(uint32_t region) {
> > + uint32_t type;
> > +
> > + type = MEMORY_TYPE_FROM_REGION(region);
>
> We will never end up with garbage here, but we are so polite in our
> testing.
If you have better idea I'll fix that.
>
> > +
> > + return intel_memory_regions[type].region_name;
> > +}
> > +/**
> > + * 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. To be fixed after memory region page size detection
> > + * patch will be merged.
> > + */
> > +uint32_t gem_get_batch_size(int fd, uint32_t region)
> > +{
> > + return IS_DEVICE_MEMORY_REGION(region) ? SZ_64K : SZ_4K;
> > +}
> > +
> > +static bool __try_gem_create_in_region(int fd, uint32_t region)
> > +{
> > + uint32_t size, handle;
> > + int ret;
> > +
> > + size = gem_get_batch_size(fd, region);
>
> size=1 will always be correct, that has now been enshrined in the ABI.
Should I change it or region "page size" is acceptable?
>
> > + handle = gem_create(fd, size);
> > + ret = gem_migrate_to_memory_region(fd, handle, region);
>
> Migrate is definitely not supported in the first wave of landings;
> memory placement will be a construction time property with possible
> later extension to runtime.
If I would have gem_create_ext() I would definitely use it. Unfortunately
only mechanism I know at the moment to place BO in memory region is
to migrate it. Is anything I missed? Will we have gem_create_ext() with
memory region as an argument?
>
> > +
> > + gem_close(fd, handle);
> > +
> > + return !ret;
> > +}
> > +
> > +static struct local_i915_query_memory_region_info *__probe_regions(int fd)
> > +{
> > + struct local_i915_query_memory_region_info *query_info;
> > + uint32_t length = sizeof(*query_info);
> > + uint32_t *regions = NULL;
> > + int max_regions = INTEL_MEMORY_TYPE_SHIFT;
> > + int num_regions = 0;
> > +
> > + for (int i = 0; intel_memory_regions[i].region_name; i++) {
> > + uint8_t type;
> > +
> > + /* We're not interested of other memory region types */
> > + type = intel_memory_regions[i].memory_type;
> > + if (type != LOCAL_I915_SYSTEM_MEMORY &&
> > + type != LOCAL_I915_DEVICE_MEMORY)
> > + continue;
> > +
> > + for (int instance = 0; instance <= max_regions; instance++) {
> > + uint32_t region;
> > + int exists;
> > +
> > + region = INTEL_MEMORY_REGION_ID(type, instance);
> > + igt_debug("Probing memory region: %08x\n", region);
> > + exists = __try_gem_create_in_region(fd, region);
> > + if (!exists)
> > + break;
> > +
> > + igt_debug("Region %08x exists\n", region);
> > + regions = realloc(regions, (num_regions + 1) *
> > + sizeof(uint32_t));
> > + igt_assert(regions);
> > + regions[num_regions++] = region;
> > + }
> > + }
> > +
> > + if (!num_regions)
> > + return NULL;
> > +
> > + length += sizeof(struct local_i915_memory_region_info) * num_regions;
> > + query_info = calloc(1, length);
> > + igt_assert(query_info);
> > +
> > + /* Prepare query_info structure with regions id filled */
> > + for (int i = 0; i < num_regions; i++) {
> > + query_info->regions[i].id = regions[i];
> > + query_info->regions[i].size = 0;
> > + }
> > + free(regions);
> > + query_info->num_regions = num_regions;
> > +
> > + return query_info;
> > +}
> > +
> > +/**
> > + * gem_query_memory_regions:
> > + * @fd: open i915 drm file descriptor
> > + *
> > + * This function wraps query mechanism for memory regions. If there's
> > + * no i915_query probe memory regions and return same structure as from query.
> > + *
> > + * Returns: Filled struct with available memory regions. Allocates structure
> > + * which must by freed by caller.
> > + */
> > +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;
> > + int ret;
> > +
> > + memset(&item, 0, sizeof(item));
> > + item.query_id = LOCAL_I915_QUERY_MEMREGION_INFO;
> > +
> > + ret = __i915_query_item(fd, &item);
> > + if (!ret) {
> > + query_info = calloc(1, item.length);
> > + igt_assert(query_info);
> > +
> > + item.data_ptr = to_user_pointer(query_info);
> > + __i915_query_item(fd, &item);
> > +
> > + return query_info;
> > + }
> > +
> > + /* Fallback, there's no memregion i915_query supported. Try probe(). */
> > + query_info = __probe_regions(fd);
>
> Oh, we will not have memory regions without a query. If anything we will
> get memory regions described before you can make a choice.
Unfortunately after your comment on previous series I concluded after first
memory region patches landing we would support them without query (current
patches doesn't support them, thus this fallback).
>
> > + igt_assert(query_info);
> > +
> > + 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);
>
> Meh. Didn't need the heap.
You're suggesting doing allocation in lmem regions until it fails?
A lot of ioctl() will be called in this case.
>
> > + 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;
>
> That seems even more of a waste of heap, as above.
>
> > +}
> > +
> > +/**
> > + * 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,
>
> Remind me to have words with the naming committee.
>
> The rest is for an interface that has yet to be seen and discussed, so
> shelving for later.
> -Chris
To be ownest I don't know what's wrong with this function name.
If you have better candidate you're welcome.
Zbigniew
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions
2019-11-22 9:33 ` Zbigniew Kempczyński
@ 2019-11-22 9:46 ` Chris Wilson
2019-11-22 14:40 ` Zbigniew Kempczyński
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-11-22 9:46 UTC (permalink / raw)
To: Zbigniew Kempczyński; +Cc: igt-dev
Quoting Zbigniew Kempczyński (2019-11-22 09:33:07)
> On Fri, Nov 22, 2019 at 09:11:46AM +0000, Chris Wilson wrote:
>
> > > +const struct intel_memory_region intel_memory_regions[] = {
> > > + { "SMEM", LOCAL_I915_SYSTEM_MEMORY, 0 },
> >
> > I love how the friendly names are so terse and do not follow the api /s
> >
> > > + { "LMEM", LOCAL_I915_DEVICE_MEMORY, 0 },
> > > + { "STLN", LOCAL_I915_STOLEN_MEMORY, 0 },
> > > + { NULL, 0, 0 },
> > > +};
> > > +
> > > +const char *memory_region_name(uint32_t region) {
> > > + uint32_t type;
> > > +
> > > + type = MEMORY_TYPE_FROM_REGION(region);
> >
> > We will never end up with garbage here, but we are so polite in our
> > testing.
>
> If you have better idea I'll fix that.
All I mean here is that type < ARRAY_SIZE(intel_memory_regions) else
report "unknown" or something.
> > > + return intel_memory_regions[type].region_name;
> > > +}
> > > +/**
> > > + * 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. To be fixed after memory region page size detection
> > > + * patch will be merged.
> > > + */
> > > +uint32_t gem_get_batch_size(int fd, uint32_t region)
> > > +{
> > > + return IS_DEVICE_MEMORY_REGION(region) ? SZ_64K : SZ_4K;
> > > +}
> > > +
> > > +static bool __try_gem_create_in_region(int fd, uint32_t region)
> > > +{
> > > + uint32_t size, handle;
> > > + int ret;
> > > +
> > > + size = gem_get_batch_size(fd, region);
> >
> > size=1 will always be correct, that has now been enshrined in the ABI.
>
> Should I change it or region "page size" is acceptable?
I don't mind; just suggesting it was redundant. So not even worth the
rant :)
> >
> > > + handle = gem_create(fd, size);
> > > + ret = gem_migrate_to_memory_region(fd, handle, region);
> >
> > Migrate is definitely not supported in the first wave of landings;
> > memory placement will be a construction time property with possible
> > later extension to runtime.
>
> If I would have gem_create_ext() I would definitely use it. Unfortunately
> only mechanism I know at the moment to place BO in memory region is
> to migrate it. Is anything I missed? Will we have gem_create_ext() with
> memory region as an argument?
I mean that this needs to be raised as an issue in the kernel
implementation as I have not seen a single suggestion for handling
gem_create with multiple memory regions since about 5 years ago.
And yet it is absolutely crucial...
> > > + /* Fallback, there's no memregion i915_query supported. Try probe(). */
> > > + query_info = __probe_regions(fd);
> >
> > Oh, we will not have memory regions without a query. If anything we will
> > get memory regions described before you can make a choice.
>
> Unfortunately after your comment on previous series I concluded after first
> memory region patches landing we would support them without query (current
> patches doesn't support them, thus this fallback).
No, I mean we are landing GEM_MMAP_OFFSET_IOCTL shortly before we have
memory regions, as that ioctl simply extends existing API and can be
used currently.
The query and memory regions are part of the same API bundle that comes
later.
> > > + igt_assert(query_info);
> > > +
> > > + 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);
> >
> > Meh. Didn't need the heap.
>
> You're suggesting doing allocation in lmem regions until it fails?
> A lot of ioctl() will be called in this case.
Nope. Look at the ioctl interface again; for most of the trivial
alloc+free patterns you can supply a stack buffer and do a single ioctl
with no allocations.
> > > +bool gem_query_has_memory_region(struct local_i915_query_memory_region_info *query_info,
> >
> > Remind me to have words with the naming committee.
> >
> > The rest is for an interface that has yet to be seen and discussed, so
> > shelving for later.
> > -Chris
>
> To be ownest I don't know what's wrong with this function name.
> If you have better candidate you're welcome.
It's the uAPI type name that has gone a little overboard with its Hungarian
inheritance. I'm just making a note to complain to its author.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions
2019-11-22 9:46 ` Chris Wilson
@ 2019-11-22 14:40 ` Zbigniew Kempczyński
2019-11-22 16:50 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Zbigniew Kempczyński @ 2019-11-22 14:40 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev
On Fri, Nov 22, 2019 at 09:46:11AM +0000, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2019-11-22 09:33:07)
> > On Fri, Nov 22, 2019 at 09:11:46AM +0000, Chris Wilson wrote:
> >
> > > > +const struct intel_memory_region intel_memory_regions[] = {
> > > > + { "SMEM", LOCAL_I915_SYSTEM_MEMORY, 0 },
> > >
> > > I love how the friendly names are so terse and do not follow the api /s
> > >
> > > > + { "LMEM", LOCAL_I915_DEVICE_MEMORY, 0 },
> > > > + { "STLN", LOCAL_I915_STOLEN_MEMORY, 0 },
> > > > + { NULL, 0, 0 },
> > > > +};
> > > > +
> > > > +const char *memory_region_name(uint32_t region) {
> > > > + uint32_t type;
> > > > +
> > > > + type = MEMORY_TYPE_FROM_REGION(region);
> > >
> > > We will never end up with garbage here, but we are so polite in our
> > > testing.
> >
> > If you have better idea I'll fix that.
>
> All I mean here is that type < ARRAY_SIZE(intel_memory_regions) else
> report "unknown" or something.
You're right. Argument checking should be definitely added here.
>
> > > > + return intel_memory_regions[type].region_name;
> > > > +}
> > > > +/**
> > > > + * 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. To be fixed after memory region page size detection
> > > > + * patch will be merged.
> > > > + */
> > > > +uint32_t gem_get_batch_size(int fd, uint32_t region)
> > > > +{
> > > > + return IS_DEVICE_MEMORY_REGION(region) ? SZ_64K : SZ_4K;
> > > > +}
> > > > +
> > > > +static bool __try_gem_create_in_region(int fd, uint32_t region)
> > > > +{
> > > > + uint32_t size, handle;
> > > > + int ret;
> > > > +
> > > > + size = gem_get_batch_size(fd, region);
> > >
> > > size=1 will always be correct, that has now been enshrined in the ABI.
> >
> > Should I change it or region "page size" is acceptable?
>
> I don't mind; just suggesting it was redundant. So not even worth the
> rant :)
>
> > >
> > > > + handle = gem_create(fd, size);
> > > > + ret = gem_migrate_to_memory_region(fd, handle, region);
> > >
> > > Migrate is definitely not supported in the first wave of landings;
> > > memory placement will be a construction time property with possible
> > > later extension to runtime.
> >
> > If I would have gem_create_ext() I would definitely use it. Unfortunately
> > only mechanism I know at the moment to place BO in memory region is
> > to migrate it. Is anything I missed? Will we have gem_create_ext() with
> > memory region as an argument?
>
> I mean that this needs to be raised as an issue in the kernel
> implementation as I have not seen a single suggestion for handling
> gem_create with multiple memory regions since about 5 years ago.
> And yet it is absolutely crucial...
>
> > > > + /* Fallback, there's no memregion i915_query supported. Try probe(). */
> > > > + query_info = __probe_regions(fd);
> > >
> > > Oh, we will not have memory regions without a query. If anything we will
> > > get memory regions described before you can make a choice.
> >
> > Unfortunately after your comment on previous series I concluded after first
> > memory region patches landing we would support them without query (current
> > patches doesn't support them, thus this fallback).
>
> No, I mean we are landing GEM_MMAP_OFFSET_IOCTL shortly before we have
> memory regions, as that ioctl simply extends existing API and can be
> used currently.
>
> The query and memory regions are part of the same API bundle that comes
> later
That means that this patch series should be splitted, gem_mman first
(you've already added R-B there). Will you merge this patch (gem_mman)
or should I send it as separate with R-B?
> > > > + igt_assert(query_info);
> > > > +
> > > > + 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);
> > >
> > > Meh. Didn't need the heap.
> >
> > You're suggesting doing allocation in lmem regions until it fails?
> > A lot of ioctl() will be called in this case.
>
> Nope. Look at the ioctl interface again; for most of the trivial
> alloc+free patterns you can supply a stack buffer and do a single ioctl
> with no allocations.
To be sure I will support allocation on-stack (via alloca for example
or variable length array C99 extention or hardcoded size surely enough)
I should multiply possible number of memory regions (currently 16) by
number or instances (using current split also 16) by sizeof(*..region_info)
+ sizeof(...query_info). From my calculation it gives 16B + 14336B so this
require around 16KB on the stack. Have I missed something?
>
> > > > +bool gem_query_has_memory_region(struct local_i915_query_memory_region_info *query_info,
> > >
> > > Remind me to have words with the naming committee.
> > >
> > > The rest is for an interface that has yet to be seen and discussed, so
> > > shelving for later.
> > > -Chris
> >
> > To be ownest I don't know what's wrong with this function name.
> > If you have better candidate you're welcome.
>
> It's the uAPI type name that has gone a little overboard with its Hungarian
> inheritance. I'm just making a note to complain to its author.
> -Chris
I don't know what Hungarian inheritance means. I know why I likely left
this function - previously interface allows having different memory region
query views depending on fd_setparam settings. I'll remove it.
Zbigniew
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions
2019-11-22 14:40 ` Zbigniew Kempczyński
@ 2019-11-22 16:50 ` Chris Wilson
0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2019-11-22 16:50 UTC (permalink / raw)
To: Zbigniew Kempczyński; +Cc: igt-dev
Quoting Zbigniew Kempczyński (2019-11-22 14:40:09)
> On Fri, Nov 22, 2019 at 09:46:11AM +0000, Chris Wilson wrote:
> > Quoting Zbigniew Kempczyński (2019-11-22 09:33:07)
> > > > > + /* Fallback, there's no memregion i915_query supported. Try probe(). */
> > > > > + query_info = __probe_regions(fd);
> > > >
> > > > Oh, we will not have memory regions without a query. If anything we will
> > > > get memory regions described before you can make a choice.
> > >
> > > Unfortunately after your comment on previous series I concluded after first
> > > memory region patches landing we would support them without query (current
> > > patches doesn't support them, thus this fallback).
> >
> > No, I mean we are landing GEM_MMAP_OFFSET_IOCTL shortly before we have
> > memory regions, as that ioctl simply extends existing API and can be
> > used currently.
> >
> > The query and memory regions are part of the same API bundle that comes
> > later
>
> That means that this patch series should be splitted, gem_mman first
> (you've already added R-B there). Will you merge this patch (gem_mman)
> or should I send it as separate with R-B?
Sure, the first patch can go in after we land mmap_offset and do a check
before breaking everything.
>
>
> > > > > + igt_assert(query_info);
> > > > > +
> > > > > + 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);
> > > >
> > > > Meh. Didn't need the heap.
> > >
> > > You're suggesting doing allocation in lmem regions until it fails?
> > > A lot of ioctl() will be called in this case.
> >
> > Nope. Look at the ioctl interface again; for most of the trivial
> > alloc+free patterns you can supply a stack buffer and do a single ioctl
> > with no allocations.
>
> To be sure I will support allocation on-stack (via alloca for example
> or variable length array C99 extention or hardcoded size surely enough)
> I should multiply possible number of memory regions (currently 16) by
> number or instances (using current split also 16) by sizeof(*..region_info)
> + sizeof(...query_info). From my calculation it gives 16B + 14336B so this
> require around 16KB on the stack. Have I missed something?
Just pick a limit that is reasonable for the quick queries, and leave
full queries to spill to heap. If we can't do a quick query, it would be
tempting to raise it as an API issue.
Or if there's no such possibility, ignore me; I'm used to being able to
do most things in 256B.
>
> >
> > > > > +bool gem_query_has_memory_region(struct local_i915_query_memory_region_info *query_info,
> > > >
> > > > Remind me to have words with the naming committee.
> > > >
> > > > The rest is for an interface that has yet to be seen and discussed, so
> > > > shelving for later.
> > > > -Chris
> > >
> > > To be ownest I don't know what's wrong with this function name.
> > > If you have better candidate you're welcome.
> >
> > It's the uAPI type name that has gone a little overboard with its Hungarian
> > inheritance. I'm just making a note to complain to its author.
>
> I don't know what Hungarian inheritance means. I know why I likely left
> this function - previously interface allows having different memory region
> query views depending on fd_setparam settings. I'll remove it.
I have no complaints about the function name; it's the uAPI type name
that I think is less than ideal.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-11-22 16:50 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22 8:13 [igt-dev] [PATCH i-g-t v5 0/4] Basic LMEM support in IGT Zbigniew Kempczyński
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 1/4] include/drm-uapi/i915_drm: Add local defs for memory region uAPI Zbigniew Kempczyński
2019-11-22 8:56 ` Chris Wilson
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 2/4] lib/i915/gem_mman: add mmap_offset support Zbigniew Kempczyński
2019-11-22 8:59 ` Chris Wilson
2019-11-22 9:12 ` Zbigniew Kempczyński
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 3/4] lib/i915/intel_memory_region: Add lib to manage memory regions Zbigniew Kempczyński
2019-11-22 9:11 ` Chris Wilson
2019-11-22 9:33 ` Zbigniew Kempczyński
2019-11-22 9:46 ` Chris Wilson
2019-11-22 14:40 ` Zbigniew Kempczyński
2019-11-22 16:50 ` Chris Wilson
2019-11-22 8:13 ` [igt-dev] [PATCH i-g-t v5 4/4] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset Zbigniew Kempczyński
2019-11-22 9:16 ` Chris Wilson
2019-11-22 8:54 ` [igt-dev] ✗ Fi.CI.BAT: failure for Basic LMEM support in IGT (rev5) 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.