All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number
@ 2019-01-09 17:40 Lukasz Kalamarz
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap Lukasz Kalamarz
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Lukasz Kalamarz @ 2019-01-09 17:40 UTC (permalink / raw)
  To: igt-dev

In lib code there were few functions using param number instead of
defines. This could lead to some issues (i.e. stolen support
pointing on different parameter) and we would like to avoid them.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/ioctl_wrappers.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 9f255508..f71f0e32 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -708,12 +708,12 @@ bool gem_mmap__has_wc(int fd)
 		has_wc = 0;
 
 		memset(&gp, 0, sizeof(gp));
-		gp.param = 40; /* MMAP_GTT_VERSION */
+		gp.param = I915_PARAM_MMAP_GTT_VERSION;
 		gp.value = &gtt_version;
 		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
 		memset(&gp, 0, sizeof(gp));
-		gp.param = 30; /* MMAP_VERSION */
+		gp.param = I915_PARAM_MMAP_VERSION;
 		gp.value = &mmap_version;
 		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
@@ -964,7 +964,7 @@ static int gem_gtt_type(int fd)
 	int val = 0;
 
 	memset(&gp, 0, sizeof(gp));
-	gp.param = 18; /* HAS_ALIASING_PPGTT */
+	gp.param = I915_PARAM_HAS_ALIASING_PPGTT;
 	gp.value = &val;
 
 	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
-- 
2.17.2

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

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

* [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
@ 2019-01-09 17:40 ` Lukasz Kalamarz
  2019-01-09 18:04   ` Daniele Ceraolo Spurio
  2019-01-09 20:40   ` Chris Wilson
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 3/6] lib/igt_dummyload: use gem_mmap__cpu and gem_mmap__wc when applicable Lukasz Kalamarz
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: Lukasz Kalamarz @ 2019-01-09 17:40 UTC (permalink / raw)
  To: igt-dev

Previous implementation of __gem_mmap__cpu and __gem_mmap_wc  only
differ with setting proper flag for caching. This patch implement __gem_mmap
which merge those two functions into one wrapper.
Small documentation modification are add to be in par with new implementation.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/ioctl_wrappers.c | 96 +++++++++++++++++++++++++++++++-------------
 lib/ioctl_wrappers.h |  1 +
 2 files changed, 68 insertions(+), 29 deletions(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index f71f0e32..87744edc 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -736,6 +736,45 @@ bool gem_mmap__has_wc(int fd)
 	return has_wc > 0;
 }
 
+/**
+ * __gem_mmap:
+ * @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()
+ * @wc: flag marking if we want to set up MMAP_WC flag
+ *
+ * This functions wraps up procedure to establish a memory mapping through
+ * direct cpu access, bypassing the gpu (valid for wc == false). For wc == true
+ * it also bypass cpu caches completely and GTT system agent (i.e. there is no
+ * automatic tiling of the mmapping through the fence registers).
+ *
+ * Returns: A pointer to the created memory mapping, NULL on failure.
+ */
+void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc)
+{
+	struct drm_i915_gem_mmap arg;
+
+	if (wc & !gem_mmap__has_wc(fd)) {
+		errno = ENOSYS;
+		return NULL;
+	}
+
+	memset(&arg, 0, sizeof(arg));
+	arg.handle = handle;
+	arg.offset = offset;
+	arg.size = size;
+	arg.flags = wc ? I915_MMAP_WC : 0;
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
+		return NULL;
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
+
+	errno = 0;
+	return from_user_pointer(arg.addr_ptr);
+}
+
 /**
  * __gem_mmap__wc:
  * @fd: open i915 drm file descriptor
@@ -753,25 +792,20 @@ bool gem_mmap__has_wc(int fd)
  */
 void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
 {
-	struct drm_i915_gem_mmap arg;
+       struct drm_i915_gem_mmap mmap_arg;
 
-	if (!gem_mmap__has_wc(fd)) {
-		errno = ENOSYS;
-		return NULL;
-	}
+       memset(&mmap_arg, 0, sizeof(mmap_arg));
+       mmap_arg.handle = handle;
+       mmap_arg.offset = offset;
+       mmap_arg.size = size;
+       mmap_arg.flags = I915_MMAP_WC;
+       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
+               return NULL;
 
-	memset(&arg, 0, sizeof(arg));
-	arg.handle = handle;
-	arg.offset = offset;
-	arg.size = size;
-	arg.flags = I915_MMAP_WC;
-	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
-		return NULL;
+       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
 
-	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
-
-	errno = 0;
-	return from_user_pointer(arg.addr_ptr);
+       errno = 0;
+       return from_user_pointer(mmap_arg.addr_ptr);
 }
 
 /**
@@ -782,13 +816,16 @@ 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.
+ * This functions wraps up procedure to establish a memory mapping through
+ * direct cpu access, bypassing the gpu and cpu caches completely and also
+ * bypassing the GTT system agent (i.e. there is no automatic tiling of
+ * the mmapping through the fence registers).We 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);
+	void *ptr = __gem_mmap(fd, handle, offset, size, prot, true);
 	igt_assert(ptr);
 	return ptr;
 }
@@ -810,17 +847,17 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
 {
 	struct drm_i915_gem_mmap mmap_arg;
 
-	memset(&mmap_arg, 0, sizeof(mmap_arg));
-	mmap_arg.handle = handle;
-	mmap_arg.offset = offset;
-	mmap_arg.size = size;
-	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
-		return NULL;
+       memset(&mmap_arg, 0, sizeof(mmap_arg));
+       mmap_arg.handle = handle;
+       mmap_arg.offset = offset;
+       mmap_arg.size = size;
+       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
+               return NULL;
 
-	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
+       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
 
-	errno = 0;
-	return from_user_pointer(mmap_arg.addr_ptr);
+       errno = 0;
+       return from_user_pointer(mmap_arg.addr_ptr);
 }
 
 /**
@@ -831,13 +868,14 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
  * @size: size of the mmap arena
  * @prot: memory protection bits as used by mmap()
  *
- * Like __gem_mmap__cpu() except we assert on failure.
+ * This functions wraps up procedure to establish a memory mapping through
+ * direct cpu access, bypassing the gpu completely and we assert on failure.
  *
  * Returns: A pointer to the created memory mapping
  */
 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(fd, handle, offset, size, prot, false);
 	igt_assert(ptr);
 	return ptr;
 }
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index b22b36b0..51e66c08 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -97,6 +97,7 @@ void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsi
 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(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc);
 
 int gem_munmap(void *ptr, uint64_t size);
 
-- 
2.17.2

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

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

* [igt-dev] [PATCH i-g-t 3/6] lib/igt_dummyload: use gem_mmap__cpu and gem_mmap__wc when applicable
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap Lukasz Kalamarz
@ 2019-01-09 17:40 ` Lukasz Kalamarz
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code Lukasz Kalamarz
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Lukasz Kalamarz @ 2019-01-09 17:40 UTC (permalink / raw)
  To: igt-dev

We had some duplicates in code that are using direct call to
__gem_mmap__cpu or __gem_mmap__wc and then assert it result, which is what
gem_mmap__cpu and gem_mmap__wc is taking care for us.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/igt_dummyload.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 2027a4b7..982906f2 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -148,14 +148,13 @@ emit_recursive_batch(igt_spin_t *spin,
 
 		if (__gem_set_caching(fd, spin->poll_handle,
 				      I915_CACHING_CACHED) == 0)
-			spin->running = __gem_mmap__cpu(fd, spin->poll_handle,
-							0, 4096,
-							PROT_READ | PROT_WRITE);
+			spin->running = gem_mmap__cpu(fd, spin->poll_handle,
+						      0, 4096,
+						      PROT_READ | PROT_WRITE);
 		else
-			spin->running = __gem_mmap__wc(fd, spin->poll_handle,
-						       0, 4096,
-						       PROT_READ | PROT_WRITE);
-		igt_assert(spin->running);
+			spin->running = gem_mmap__wc(fd, spin->poll_handle,
+						     0, 4096,
+						     PROT_READ | PROT_WRITE);
 		igt_assert_eq(*spin->running, 0);
 
 		*batch++ = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
-- 
2.17.2

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

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

* [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap Lukasz Kalamarz
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 3/6] lib/igt_dummyload: use gem_mmap__cpu and gem_mmap__wc when applicable Lukasz Kalamarz
@ 2019-01-09 17:40 ` Lukasz Kalamarz
  2019-01-09 18:57   ` Daniele Ceraolo Spurio
  2019-01-09 20:45   ` Chris Wilson
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 5/6] lib/ tests/: drop usage of __gem_mmap__wc " Lukasz Kalamarz
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: Lukasz Kalamarz @ 2019-01-09 17:40 UTC (permalink / raw)
  To: igt-dev

With implementation of __gem_mmap we can drop __gem_mmap__cpu
and instead use refactored function.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 benchmarks/gem_exec_reloc.c      | 6 +++---
 tests/i915/gem_exec_big.c        | 2 +-
 tests/i915/gem_exec_lut_handle.c | 6 +++---
 tests/i915/gem_mmap.c            | 4 ++--
 tests/i915/gem_stolen.c          | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/benchmarks/gem_exec_reloc.c b/benchmarks/gem_exec_reloc.c
index f9487d95..40a1d500 100644
--- a/benchmarks/gem_exec_reloc.c
+++ b/benchmarks/gem_exec_reloc.c
@@ -116,13 +116,13 @@ static int run(unsigned batch_size,
 	if (num_relocs) {
 		size = ALIGN(sizeof(*mem_reloc)*num_relocs, 4096);
 		reloc_handle = gem_create(fd, size);
-		reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
+		reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
 		memcpy(reloc, mem_reloc, sizeof(*mem_reloc)*num_relocs);
 		munmap(reloc, size);
 
 		if (flags & FAULT) {
 			igt_disable_prefault();
-			reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
+			reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
 		} else
 			reloc = mem_reloc;
 	}
@@ -163,7 +163,7 @@ static int run(unsigned batch_size,
 			}
 			if (flags & FAULT && reloc) {
 				munmap(reloc, size);
-				reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
+				reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
 				gem_exec[num_objects].relocs_ptr = (uintptr_t)reloc;
 			}
 			gem_execbuf(fd, &execbuf);
diff --git a/tests/i915/gem_exec_big.c b/tests/i915/gem_exec_big.c
index a15672f6..b57560af 100644
--- a/tests/i915/gem_exec_big.c
+++ b/tests/i915/gem_exec_big.c
@@ -220,7 +220,7 @@ igt_simple_main
 		gem_write(fd, handle, 0, batch, sizeof(batch));
 
 		if (!FORCE_PREAD_PWRITE && gem_has_llc(fd))
-			ptr = __gem_mmap__cpu(fd, handle, 0, batch_size, PROT_READ);
+			ptr = __gem_mmap(fd, handle, 0, batch_size, PROT_READ, false);
 		else if (!FORCE_PREAD_PWRITE && gem_mmap__has_wc(fd))
 			ptr = __gem_mmap__wc(fd, handle, 0, batch_size, PROT_READ);
 		else
diff --git a/tests/i915/gem_exec_lut_handle.c b/tests/i915/gem_exec_lut_handle.c
index 98e6ae5a..9929f0c7 100644
--- a/tests/i915/gem_exec_lut_handle.c
+++ b/tests/i915/gem_exec_lut_handle.c
@@ -148,7 +148,7 @@ igt_simple_main
 				struct timeval start, end;
 
 				if (p->flags & FAULT)
-					reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
+					reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
 				else
 					reloc = mem_reloc;
 
@@ -182,7 +182,7 @@ igt_simple_main
 					}
 					if (p->flags & FAULT) {
 						munmap(reloc, size);
-						reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
+						reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
 						gem_exec[MAX_NUM_EXEC].relocs_ptr = to_user_pointer(reloc);
 					}
 					gem_execbuf(fd, &execbuf);
@@ -212,7 +212,7 @@ igt_simple_main
 					}
 					if (p->flags & FAULT) {
 						munmap(reloc, size);
-						reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
+						reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
 						gem_exec[MAX_NUM_EXEC].relocs_ptr = to_user_pointer(reloc);
 					}
 					gem_execbuf(fd, &execbuf);
diff --git a/tests/i915/gem_mmap.c b/tests/i915/gem_mmap.c
index 0ed15878..5cbea456 100644
--- a/tests/i915/gem_mmap.c
+++ b/tests/i915/gem_mmap.c
@@ -80,8 +80,8 @@ test_huge_bo(int huge)
 	bo = gem_create(fd, huge_object_size);
 
 	/* Obtain CPU mapping for the object. */
-	ptr_cpu = __gem_mmap__cpu(fd, bo, 0, huge_object_size,
-				PROT_READ | PROT_WRITE);
+	ptr_cpu = __gem_mmap(fd, bo, 0, huge_object_size,
+			     PROT_READ | PROT_WRITE, false);
 	igt_require(ptr_cpu);
 	gem_set_domain(fd, bo, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
 	gem_close(fd, bo);
diff --git a/tests/i915/gem_stolen.c b/tests/i915/gem_stolen.c
index 1d489976..9b88c440 100644
--- a/tests/i915/gem_stolen.c
+++ b/tests/i915/gem_stolen.c
@@ -392,7 +392,7 @@ stolen_no_mmap(int fd)
 
 	handle = gem_create_stolen(fd, SIZE);
 
-	addr = __gem_mmap__cpu(fd, handle, 0, SIZE, PROT_READ | PROT_WRITE);
+	addr = __gem_mmap(fd, handle, 0, SIZE, PROT_READ | PROT_WRITE, false);
 	igt_assert(addr == NULL);
 
 	gem_close(fd, handle);
-- 
2.17.2

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

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

* [igt-dev] [PATCH i-g-t 5/6] lib/ tests/: drop usage of __gem_mmap__wc in code
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
                   ` (2 preceding siblings ...)
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code Lukasz Kalamarz
@ 2019-01-09 17:40 ` Lukasz Kalamarz
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 6/6] lib/ioctl_wrapper: remove __gem_mmap__cpu and __gem_mmap__wc Lukasz Kalamarz
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Lukasz Kalamarz @ 2019-01-09 17:40 UTC (permalink / raw)
  To: igt-dev

With implementation of __gem_mmap we can drop __gem_mmap__wc
and instead use refactored function.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/igt_dummyload.c           | 4 ++--
 tests/i915/gem_exec_big.c     | 2 +-
 tests/i915/gem_exec_gttfill.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 982906f2..00f39b94 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -114,8 +114,8 @@ emit_recursive_batch(igt_spin_t *spin,
 	memset(relocs, 0, sizeof(relocs));
 
 	obj[BATCH].handle = gem_create(fd, BATCH_SIZE);
-	batch = __gem_mmap__wc(fd, obj[BATCH].handle,
-			       0, BATCH_SIZE, PROT_WRITE);
+	batch = __gem_mmap(fd, obj[BATCH].handle, 0, BATCH_SIZE,
+		           PROT_WRITE, true);
 	if (!batch)
 		batch = gem_mmap__gtt(fd, obj[BATCH].handle,
 				       	BATCH_SIZE, PROT_WRITE);
diff --git a/tests/i915/gem_exec_big.c b/tests/i915/gem_exec_big.c
index b57560af..15b1021d 100644
--- a/tests/i915/gem_exec_big.c
+++ b/tests/i915/gem_exec_big.c
@@ -222,7 +222,7 @@ igt_simple_main
 		if (!FORCE_PREAD_PWRITE && gem_has_llc(fd))
 			ptr = __gem_mmap(fd, handle, 0, batch_size, PROT_READ, false);
 		else if (!FORCE_PREAD_PWRITE && gem_mmap__has_wc(fd))
-			ptr = __gem_mmap__wc(fd, handle, 0, batch_size, PROT_READ);
+			ptr = __gem_mmap(fd, handle, 0, batch_size, PROT_READ, true);
 		else
 			ptr = NULL;
 
diff --git a/tests/i915/gem_exec_gttfill.c b/tests/i915/gem_exec_gttfill.c
index efd612bb..c39d2d15 100644
--- a/tests/i915/gem_exec_gttfill.c
+++ b/tests/i915/gem_exec_gttfill.c
@@ -157,8 +157,8 @@ static void fillgtt(int fd, unsigned ring, int timeout)
 	for (unsigned i = 0; i < count; i++) {
 		batches[i].handle = gem_create(fd, BATCH_SIZE);
 		batches[i].ptr =
-			__gem_mmap__wc(fd, batches[i].handle,
-				       0, BATCH_SIZE, PROT_WRITE);
+			__gem_mmap(fd, batches[i].handle,0, BATCH_SIZE,
+				   PROT_WRITE, true);
 		if (!batches[i].ptr) {
 			batches[i].ptr =
 				__gem_mmap__gtt(fd, batches[i].handle,
-- 
2.17.2

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

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

* [igt-dev] [PATCH i-g-t 6/6] lib/ioctl_wrapper: remove __gem_mmap__cpu and __gem_mmap__wc
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
                   ` (3 preceding siblings ...)
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 5/6] lib/ tests/: drop usage of __gem_mmap__wc " Lukasz Kalamarz
@ 2019-01-09 17:40 ` Lukasz Kalamarz
  2019-01-09 18:14 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Patchwork
  2019-01-09 19:01 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 12+ messages in thread
From: Lukasz Kalamarz @ 2019-01-09 17:40 UTC (permalink / raw)
  To: igt-dev

After fixing all dependencies within IGT, we can remove
obsolted code from lib.

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/ioctl_wrappers.c | 63 --------------------------------------------
 lib/ioctl_wrappers.h |  2 --
 2 files changed, 65 deletions(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 87744edc..1874230d 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -775,39 +775,6 @@ void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsign
 	return from_user_pointer(arg.addr_ptr);
 }
 
-/**
- * __gem_mmap__wc:
- * @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()
- *
- * This functions wraps up procedure to establish a memory mapping through
- * direct cpu access, bypassing the gpu and cpu caches completely and also
- * bypassing the GTT system agent (i.e. there is no automatic tiling of
- * the mmapping through the fence registers).
- *
- * Returns: A pointer to the created memory mapping, NULL on failure.
- */
-void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
-{
-       struct drm_i915_gem_mmap mmap_arg;
-
-       memset(&mmap_arg, 0, sizeof(mmap_arg));
-       mmap_arg.handle = handle;
-       mmap_arg.offset = offset;
-       mmap_arg.size = size;
-       mmap_arg.flags = I915_MMAP_WC;
-       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
-               return NULL;
-
-       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
-
-       errno = 0;
-       return from_user_pointer(mmap_arg.addr_ptr);
-}
-
 /**
  * gem_mmap__wc:
  * @fd: open i915 drm file descriptor
@@ -830,36 +797,6 @@ void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsi
 	return ptr;
 }
 
-/**
- * __gem_mmap__cpu:
- * @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()
- *
- * This functions wraps up procedure to establish a memory mapping through
- * direct cpu access, bypassing the gpu completely.
- *
- * Returns: A pointer to the created memory mapping, NULL on failure.
- */
-void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
-{
-	struct drm_i915_gem_mmap mmap_arg;
-
-       memset(&mmap_arg, 0, sizeof(mmap_arg));
-       mmap_arg.handle = handle;
-       mmap_arg.offset = offset;
-       mmap_arg.size = size;
-       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
-               return NULL;
-
-       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
-
-       errno = 0;
-       return from_user_pointer(mmap_arg.addr_ptr);
-}
-
 /**
  * gem_mmap__cpu:
  * @fd: open i915 drm file descriptor
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 51e66c08..77e69d7f 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -95,8 +95,6 @@ void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsi
 #endif
 
 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(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc);
 
 int gem_munmap(void *ptr, uint64_t size);
-- 
2.17.2

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

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

* Re: [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap Lukasz Kalamarz
@ 2019-01-09 18:04   ` Daniele Ceraolo Spurio
  2019-01-09 20:40   ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-01-09 18:04 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev



On 01/09/2019 09:40 AM, Lukasz Kalamarz wrote:
> Previous implementation of __gem_mmap__cpu and __gem_mmap_wc  only
> differ with setting proper flag for caching. This patch implement __gem_mmap
> which merge those two functions into one wrapper.
> Small documentation modification are add to be in par with new implementation.
> 
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/ioctl_wrappers.c | 96 +++++++++++++++++++++++++++++++-------------
>   lib/ioctl_wrappers.h |  1 +
>   2 files changed, 68 insertions(+), 29 deletions(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index f71f0e32..87744edc 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -736,6 +736,45 @@ bool gem_mmap__has_wc(int fd)
>   	return has_wc > 0;
>   }
>   
> +/**
> + * __gem_mmap:
> + * @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()
> + * @wc: flag marking if we want to set up MMAP_WC flag
> + *
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu (valid for wc == false). For wc == true
> + * it also bypass cpu caches completely and GTT system agent (i.e. there is no
> + * automatic tiling of the mmapping through the fence registers).
> + *
> + * Returns: A pointer to the created memory mapping, NULL on failure.
> + */
> +void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc)

This shouldn't be called from outside this file, so you can just make it 
static.

> +{
> +	struct drm_i915_gem_mmap arg;
> +
> +	if (wc & !gem_mmap__has_wc(fd)) {
> +		errno = ENOSYS;
> +		return NULL;
> +	}
> +
> +	memset(&arg, 0, sizeof(arg));
> +	arg.handle = handle;
> +	arg.offset = offset;
> +	arg.size = size;
> +	arg.flags = wc ? I915_MMAP_WC : 0;
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
> +		return NULL;
> +
> +	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
> +
> +	errno = 0;
> +	return from_user_pointer(arg.addr_ptr);
> +}
> +
>   /**
>    * __gem_mmap__wc:
>    * @fd: open i915 drm file descriptor
> @@ -753,25 +792,20 @@ bool gem_mmap__has_wc(int fd)
>    */
>   void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot)
>   {
> -	struct drm_i915_gem_mmap arg;
> +       struct drm_i915_gem_mmap mmap_arg;
>   

Any reason not to just do:

void *__gem_mmap__wc(...)
{
	return __gem_mmap(..., true);
}

?

> -	if (!gem_mmap__has_wc(fd)) {
> -		errno = ENOSYS;
> -		return NULL;
> -	}
> +       memset(&mmap_arg, 0, sizeof(mmap_arg));
> +       mmap_arg.handle = handle;
> +       mmap_arg.offset = offset;
> +       mmap_arg.size = size;
> +       mmap_arg.flags = I915_MMAP_WC;
> +       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
> +               return NULL;
>   
> -	memset(&arg, 0, sizeof(arg));
> -	arg.handle = handle;
> -	arg.offset = offset;
> -	arg.size = size;
> -	arg.flags = I915_MMAP_WC;
> -	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
> -		return NULL;
> +       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
>   
> -	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
> -
> -	errno = 0;
> -	return from_user_pointer(arg.addr_ptr);
> +       errno = 0;
> +       return from_user_pointer(mmap_arg.addr_ptr);

Looks like you've changed all tabs to spaces here. Can just remove it 
all if you follow the above suggestion.

>   }
>   
>   /**
> @@ -782,13 +816,16 @@ 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.
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu and cpu caches completely and also
> + * bypassing the GTT system agent (i.e. there is no automatic tiling of
> + * the mmapping through the fence registers).We 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);
> +	void *ptr = __gem_mmap(fd, handle, offset, size, prot, true);

If you follow the above suggestion for __gem_mmap__wc then there'll be 
no need to update here.

>   	igt_assert(ptr);
>   	return ptr;
>   }
> @@ -810,17 +847,17 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u

The same suggestion I had for the __wc path apply for __cpu


Daniele

>   {
>   	struct drm_i915_gem_mmap mmap_arg;
>   
> -	memset(&mmap_arg, 0, sizeof(mmap_arg));
> -	mmap_arg.handle = handle;
> -	mmap_arg.offset = offset;
> -	mmap_arg.size = size;
> -	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
> -		return NULL;
> +       memset(&mmap_arg, 0, sizeof(mmap_arg));
> +       mmap_arg.handle = handle;
> +       mmap_arg.offset = offset;
> +       mmap_arg.size = size;
> +       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
> +               return NULL;
>   
> -	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
> +       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
>   
> -	errno = 0;
> -	return from_user_pointer(mmap_arg.addr_ptr);
> +       errno = 0;
> +       return from_user_pointer(mmap_arg.addr_ptr);
>   }
>   
>   /**
> @@ -831,13 +868,14 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
>    * @size: size of the mmap arena
>    * @prot: memory protection bits as used by mmap()
>    *
> - * Like __gem_mmap__cpu() except we assert on failure.
> + * This functions wraps up procedure to establish a memory mapping through
> + * direct cpu access, bypassing the gpu completely and we assert on failure.
>    *
>    * Returns: A pointer to the created memory mapping
>    */
>   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(fd, handle, offset, size, prot, false);
>   	igt_assert(ptr);
>   	return ptr;
>   }
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index b22b36b0..51e66c08 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -97,6 +97,7 @@ void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsi
>   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(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc);
>   
>   int gem_munmap(void *ptr, uint64_t size);
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
                   ` (4 preceding siblings ...)
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 6/6] lib/ioctl_wrapper: remove __gem_mmap__cpu and __gem_mmap__wc Lukasz Kalamarz
@ 2019-01-09 18:14 ` Patchwork
  2019-01-09 19:01 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-01-09 18:14 UTC (permalink / raw)
  To: Lukasz Kalamarz; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number
URL   : https://patchwork.freedesktop.org/series/54954/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5383 -> IGTPW_2207
====================================================

Summary
-------

  **WARNING**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54954/revisions/1/

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       SKIP -> PASS +7

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      PASS -> SKIP +36

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       SKIP -> PASS

  
Known issues
------------

  Here are the changes found in IGTPW_2207 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#108473]

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - fi-kbl-7567u:       DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS

  * igt@gem_exec_suspend@basic-s3:
    - fi-kbl-7567u:       DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602] -> PASS

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       DMESG-WARN [fdo#105602] / [fdo#108529] -> PASS +1

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-kbl-7567u:       FAIL [fdo#108767] -> PASS +2

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-skl-6700hq:      DMESG-WARN [fdo#105998] -> PASS +1

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - fi-bsw-n3050:       FAIL [fdo#100368] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-kbl-7567u:       DMESG-FAIL [fdo#105079] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       FAIL [fdo#108800] -> PASS

  * igt@pm_rpm@module-reload:
    - fi-kbl-7567u:       DMESG-WARN [fdo#108529] -> PASS

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#108473]: https://bugs.freedesktop.org/show_bug.cgi?id=108473
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800


Participating hosts (48 -> 42)
------------------------------

  Additional (2): fi-gdg-551 fi-pnv-d510 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-icl-u3 fi-ivb-3520m 


Build changes
-------------

    * IGT: IGT_4757 -> IGTPW_2207

  CI_DRM_5383: f0835c765f5520b13d032a1904a2f90a44297b3b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2207: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2207/
  IGT_4757: 738f43a54d626f08e250c926a5aeec53458fbd3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code Lukasz Kalamarz
@ 2019-01-09 18:57   ` Daniele Ceraolo Spurio
  2019-01-09 20:45   ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-01-09 18:57 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev



On 01/09/2019 09:40 AM, Lukasz Kalamarz wrote:
> With implementation of __gem_mmap we can drop __gem_mmap__cpu
> and instead use refactored function.

Ok now I see why you didn't make it static in patch 2 :)
I'd personally prefer to keep __gem_mmap__cpu and __gem_mmap__wc as 
they're paired with gem_mmap__cpu and gem_mmap__wc and also make it 
explicit what mapping we're going for, but I'm not going to block if the 
majority prefers to get rid of them.

> 
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   benchmarks/gem_exec_reloc.c      | 6 +++---
>   tests/i915/gem_exec_big.c        | 2 +-
>   tests/i915/gem_exec_lut_handle.c | 6 +++---
>   tests/i915/gem_mmap.c            | 4 ++--
>   tests/i915/gem_stolen.c          | 2 +-
>   5 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/benchmarks/gem_exec_reloc.c b/benchmarks/gem_exec_reloc.c
> index f9487d95..40a1d500 100644
> --- a/benchmarks/gem_exec_reloc.c
> +++ b/benchmarks/gem_exec_reloc.c
> @@ -116,13 +116,13 @@ static int run(unsigned batch_size,
>   	if (num_relocs) {
>   		size = ALIGN(sizeof(*mem_reloc)*num_relocs, 4096);
>   		reloc_handle = gem_create(fd, size);
> -		reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
> +		reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);

I'm wondering what the reason is not to use gem_mmap__cpu here and in 
the other cases where we pass the mapping as a reloc. From what I can 
see we do access the pointer (e.g. with the memcpy the line below or 
passing it to execbuf), so I'd expect we'd want to make sure it is valid

Daniele

>   		memcpy(reloc, mem_reloc, sizeof(*mem_reloc)*num_relocs);
>   		munmap(reloc, size);
>   
>   		if (flags & FAULT) {
>   			igt_disable_prefault();
> -			reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
> +			reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
>   		} else
>   			reloc = mem_reloc;
>   	}
> @@ -163,7 +163,7 @@ static int run(unsigned batch_size,
>   			}
>   			if (flags & FAULT && reloc) {
>   				munmap(reloc, size);
> -				reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
> +				reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
>   				gem_exec[num_objects].relocs_ptr = (uintptr_t)reloc;
>   			}
>   			gem_execbuf(fd, &execbuf);
> diff --git a/tests/i915/gem_exec_big.c b/tests/i915/gem_exec_big.c
> index a15672f6..b57560af 100644
> --- a/tests/i915/gem_exec_big.c
> +++ b/tests/i915/gem_exec_big.c
> @@ -220,7 +220,7 @@ igt_simple_main
>   		gem_write(fd, handle, 0, batch, sizeof(batch));
>   
>   		if (!FORCE_PREAD_PWRITE && gem_has_llc(fd))
> -			ptr = __gem_mmap__cpu(fd, handle, 0, batch_size, PROT_READ);
> +			ptr = __gem_mmap(fd, handle, 0, batch_size, PROT_READ, false);
>   		else if (!FORCE_PREAD_PWRITE && gem_mmap__has_wc(fd))
>   			ptr = __gem_mmap__wc(fd, handle, 0, batch_size, PROT_READ);
>   		else
> diff --git a/tests/i915/gem_exec_lut_handle.c b/tests/i915/gem_exec_lut_handle.c
> index 98e6ae5a..9929f0c7 100644
> --- a/tests/i915/gem_exec_lut_handle.c
> +++ b/tests/i915/gem_exec_lut_handle.c
> @@ -148,7 +148,7 @@ igt_simple_main
>   				struct timeval start, end;
>   
>   				if (p->flags & FAULT)
> -					reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
> +					reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
>   				else
>   					reloc = mem_reloc;
>   
> @@ -182,7 +182,7 @@ igt_simple_main
>   					}
>   					if (p->flags & FAULT) {
>   						munmap(reloc, size);
> -						reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
> +						reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
>   						gem_exec[MAX_NUM_EXEC].relocs_ptr = to_user_pointer(reloc);
>   					}
>   					gem_execbuf(fd, &execbuf);
> @@ -212,7 +212,7 @@ igt_simple_main
>   					}
>   					if (p->flags & FAULT) {
>   						munmap(reloc, size);
> -						reloc = __gem_mmap__cpu(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE);
> +						reloc = __gem_mmap(fd, reloc_handle, 0, size, PROT_READ | PROT_WRITE, false);
>   						gem_exec[MAX_NUM_EXEC].relocs_ptr = to_user_pointer(reloc);
>   					}
>   					gem_execbuf(fd, &execbuf);
> diff --git a/tests/i915/gem_mmap.c b/tests/i915/gem_mmap.c
> index 0ed15878..5cbea456 100644
> --- a/tests/i915/gem_mmap.c
> +++ b/tests/i915/gem_mmap.c
> @@ -80,8 +80,8 @@ test_huge_bo(int huge)
>   	bo = gem_create(fd, huge_object_size);
>   
>   	/* Obtain CPU mapping for the object. */
> -	ptr_cpu = __gem_mmap__cpu(fd, bo, 0, huge_object_size,
> -				PROT_READ | PROT_WRITE);
> +	ptr_cpu = __gem_mmap(fd, bo, 0, huge_object_size,
> +			     PROT_READ | PROT_WRITE, false);
>   	igt_require(ptr_cpu);
>   	gem_set_domain(fd, bo, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
>   	gem_close(fd, bo);
> diff --git a/tests/i915/gem_stolen.c b/tests/i915/gem_stolen.c
> index 1d489976..9b88c440 100644
> --- a/tests/i915/gem_stolen.c
> +++ b/tests/i915/gem_stolen.c
> @@ -392,7 +392,7 @@ stolen_no_mmap(int fd)
>   
>   	handle = gem_create_stolen(fd, SIZE);
>   
> -	addr = __gem_mmap__cpu(fd, handle, 0, SIZE, PROT_READ | PROT_WRITE);
> +	addr = __gem_mmap(fd, handle, 0, SIZE, PROT_READ | PROT_WRITE, false);
>   	igt_assert(addr == NULL);
>   
>   	gem_close(fd, handle);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number
  2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
                   ` (5 preceding siblings ...)
  2019-01-09 18:14 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Patchwork
@ 2019-01-09 19:01 ` Patchwork
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-01-09 19:01 UTC (permalink / raw)
  To: Lukasz Kalamarz; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number
URL   : https://patchwork.freedesktop.org/series/54954/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5383_full -> IGTPW_2207_full
====================================================

Summary
-------

  **WARNING**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54954/revisions/1/

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

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

### IGT changes ###

#### Warnings ####

  * igt@perf_pmu@rc6:
    - shard-kbl:          SKIP -> PASS

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-snb:          SKIP -> PASS

  
Known issues
------------

  Here are the changes found in IGTPW_2207_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-snb:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-hsw:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +8

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x64-random:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
    - shard-glk:          PASS -> FAIL [fdo#103184]

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-hsw:          PASS -> FAIL [fdo#102887]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167]
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-glk:          PASS -> FAIL [fdo#108145] +2
    - shard-kbl:          PASS -> FAIL [fdo#108145]
    - shard-apl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1
    - shard-kbl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +4

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          PASS -> DMESG-FAIL [fdo#105763] / [fdo#106538]

  
#### Possible fixes ####

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          FAIL [fdo#105767] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-kbl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-hsw:          INCOMPLETE [fdo#103540] -> SKIP

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-kbl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-glk:          DMESG-WARN [fdo#109244] -> INCOMPLETE [fdo#103359] / [fdo#106886] / [k.org#198133]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          DMESG-WARN [fdo#105604] -> DMESG-FAIL [fdo#108950]

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4757 -> IGTPW_2207
    * Piglit: piglit_4509 -> None

  CI_DRM_5383: f0835c765f5520b13d032a1904a2f90a44297b3b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2207: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2207/
  IGT_4757: 738f43a54d626f08e250c926a5aeec53458fbd3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap Lukasz Kalamarz
  2019-01-09 18:04   ` Daniele Ceraolo Spurio
@ 2019-01-09 20:40   ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-01-09 20:40 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev

Quoting Lukasz Kalamarz (2019-01-09 17:40:53)
> +void *__gem_mmap(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot, bool wc)
> +{
> +       struct drm_i915_gem_mmap arg;
> +
> +       if (wc & !gem_mmap__has_wc(fd)) {

Let's just pass it to the kernel once rather than twice. It only really
made sense if there was an assert later on and we wanted to
differentiate between expected failure and user error.

> +               errno = ENOSYS;
> +               return NULL;
> +       }
> +
> +       memset(&arg, 0, sizeof(arg));
> +       arg.handle = handle;
> +       arg.offset = offset;
> +       arg.size = size;
> +       arg.flags = wc ? I915_MMAP_WC : 0;

Then take _flags_.

> +       if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &arg))
> +               return NULL;
> +
> +       VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
> +
> +       errno = 0;
> +       return from_user_pointer(arg.addr_ptr);
> +}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code
  2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code Lukasz Kalamarz
  2019-01-09 18:57   ` Daniele Ceraolo Spurio
@ 2019-01-09 20:45   ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-01-09 20:45 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev

Quoting Lukasz Kalamarz (2019-01-09 17:40:55)
> With implementation of __gem_mmap we can drop __gem_mmap__cpu
> and instead use refactored function.

No, I would rather keep the mode (__cpu, __wc, __gtt) explicit.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2019-01-09 20:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 17:40 [igt-dev] [PATCH i-g-t 1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Lukasz Kalamarz
2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 2/6] lib/ioctl_wrapper: Implement __gem_mmap Lukasz Kalamarz
2019-01-09 18:04   ` Daniele Ceraolo Spurio
2019-01-09 20:40   ` Chris Wilson
2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 3/6] lib/igt_dummyload: use gem_mmap__cpu and gem_mmap__wc when applicable Lukasz Kalamarz
2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 4/6] benchmarks/ tests/i915/: drop usage of __gem_mmap__cpu in code Lukasz Kalamarz
2019-01-09 18:57   ` Daniele Ceraolo Spurio
2019-01-09 20:45   ` Chris Wilson
2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 5/6] lib/ tests/: drop usage of __gem_mmap__wc " Lukasz Kalamarz
2019-01-09 17:40 ` [igt-dev] [PATCH i-g-t 6/6] lib/ioctl_wrapper: remove __gem_mmap__cpu and __gem_mmap__wc Lukasz Kalamarz
2019-01-09 18:14 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] lib/ioctl_wrapper: use defines for get_param instead of param number Patchwork
2019-01-09 19:01 ` [igt-dev] ✓ Fi.CI.IGT: " 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.