Skip to contentSkip to conversation

lib/i915/gem_mman: Add helper routine to check GTT version

PATCH In review Stuart Summers 3 participants 5 messages Nov 17, 2019 – Nov 18, 2019

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 lib/i915/gem_mman: Add helper routine to check GTT version

Add a check to gem_pread and gem_pwrite to only allow GTT subtests when the GTT version is at least 1.

Also add a minor change to gem_mmap_gtt to use the GTT_VERSION IOCTL macro instead of hard coding the value.

---
 lib/i915/gem_mman.c       | 26 +++++++++++++++++++-------
 lib/i915/gem_mman.h       |  2 ++
 tests/i915/gem_mmap_gtt.c |  2 +-
 tests/i915/gem_pread.c    |  3 +++
 tests/i915/gem_pwrite.c   |  3 +++
 5 files changed, 28 insertions(+), 8 deletions(-)
lib/i915/gem_mman.c +19 -7
index 6256627b..cdd37f86 100644
@@ -40,6 +40,24 @@
 #define VG(x) do {} while (0)
 #endif
 
+static int gem_mmap_get_gtt_version(int fd)
+{
+	struct drm_i915_getparam gp;
+	int gtt_version = -1;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.param = I915_PARAM_MMAP_GTT_VERSION;
+	gp.value = &gtt_version;
+	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+	return gtt_version;
+}
+
+bool gem_mmap_has_gtt(int fd)
+{
+	return gem_mmap_get_gtt_version(fd) > 0;
+}
+
 /**
  * __gem_mmap__gtt:
  * @fd: open i915 drm file descriptor
@@ -108,22 +126,16 @@ bool gem_mmap__has_wc(int fd)
 	if (has_wc == -1) {
 		struct drm_i915_getparam gp;
 		int mmap_version = -1;
-		int gtt_version = -1;
 
 		has_wc = 0;
 
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_MMAP_GTT_VERSION;
-		gp.value = &gtt_version;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
 		memset(&gp, 0, sizeof(gp));
 		gp.param = I915_PARAM_MMAP_VERSION;
 		gp.value = &mmap_version;
 		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
 		/* Do we have the new mmap_ioctl with DOMAIN_WC? */
-		if (mmap_version >= 1 && gtt_version >= 2) {
+		if (mmap_version >= 1 && gem_mmap_get_gtt_version(fd) >= 2) {
 			struct drm_i915_gem_mmap arg;
 
 			/* Does this device support wc-mmaps ? */
lib/i915/gem_mman.h +2 -0
index 096ff592..65d1a886 100644
@@ -25,6 +25,8 @@
 #ifndef GEM_MMAN_H
 #define GEM_MMAN_H
 
+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);
 
tests/i915/gem_mmap_gtt.c +1 -1
index a5f9d934..5e9ae9e5 100644
@@ -294,7 +294,7 @@ static int mmap_gtt_version(int i915)
 {
 	int val = 0;
 	struct drm_i915_getparam gp = {
-		gp.param = 40, /* MMAP_GTT_VERSION */
+		gp.param = I915_PARAM_MMAP_GTT_VERSION,
 		gp.value = &val,
 	};
 
tests/i915/gem_pread.c +3 -0
index 5b926ab0..f6e02c72 100644
@@ -47,6 +47,9 @@ static void *wrap_gem_mmap__gtt(int i915, uint32_t handle,
 				uint64_t offset, uint64_t length,
 				unsigned int prot)
 {
+	if (!gem_mmap_has_gtt(i915))
+		return NULL;
+
 	return gem_mmap__gtt(i915, handle, length, prot);
 }
 
tests/i915/gem_pwrite.c +3 -0
index 69f823d5..cde4d6ce 100644
@@ -47,6 +47,9 @@ static void *wrap_gem_mmap__gtt(int i915, uint32_t handle,
 				uint64_t offset, uint64_t length,
 				unsigned int prot)
 {
+	if (!gem_mmap_has_gtt(i915))
+		return NULL;
+
 	return gem_mmap__gtt(i915, handle, length, prot);
 }
 
Signature · 6 lines
2.22.0

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev
#2 lore raw
Quoted 5 lines · Chris Wilson

Quoting Stuart Summers (2019-11-17 14:39:16)

Quoted 2 lines

Add a check to gem_pread and gem_pwrite to only allow GTT subtests when the GTT version is at least 1.

No, it must also run on kernels before ioctl versioning. Why?

Hm.. yeah that makes sense. I'll have to come up with something else :) Looking for a way to determine whether we can mmap to the GTT. I'll rethink and repost.

Thanks,
Stuart

Quoted 1 line

-Chris

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#3 lore raw

Quoting Summers, Stuart (2019-11-17 22:12:32)

Quoted 10 lines · Chris Wilson
Quoted 5 lines

Quoting Stuart Summers (2019-11-17 14:39:16)

Quoted 2 lines

Add a check to gem_pread and gem_pwrite to only allow GTT subtests when the GTT version is at least 1.

No, it must also run on kernels before ioctl versioning. Why?

Hm.. yeah that makes sense. I'll have to come up with something else :) Looking for a way to determine whether we can mmap to the GTT. I'll rethink and repost.

It's the same way as detecting whether or not we can use WC, UC or WB page protections: trial-and-error at present.

It's not the worst way... If I was suggesting API, I would suggest it would be a part of the memory discovery i915_query.
-Chris
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#4 lore raw

[igt-dev] ✗ Fi.CI.BAT: failure for lib/i915/gem_mman: Add helper routine to check GTT version

In reply to Stuart Summers
== Series Details ==

== Series Details ==

Series: lib/i915/gem_mman: Add helper routine to check GTT version
URL : https://patchwork.freedesktop.org/series/69590/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7358 -> IGTPW_3718
====================================================

Summary
-------

**FAILURE**

Serious unknown changes coming with IGTPW_3718 absolutely need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in IGTPW_3718, 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_3718/index.html

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

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

### IGT changes ###

#### Possible regressions ####

* igt@gem_exec_suspend@basic-s0:
- fi-icl-u2: [PASS][1] -> [DMESG-WARN][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7358/fi-icl-u2/igt@[email protected]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/fi-icl-u2/igt@[email protected]

* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6770hq: [PASS][3] -> [FAIL][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7358/fi-skl-6770hq/igt@[email protected]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/fi-skl-6770hq/igt@[email protected]

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

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

### IGT changes ###

#### Issues hit ####

* igt@kms_busy@basic-flip-pipe-b:
- fi-skl-6770hq: [PASS][5] -> [DMESG-WARN][6] ([fdo#105541])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7358/fi-skl-6770hq/igt@[email protected]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/fi-skl-6770hq/igt@[email protected]

* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-skl-6770hq: [PASS][7] -> [FAIL][8] ([fdo#109495])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7358/fi-skl-6770hq/igt@[email protected]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/fi-skl-6770hq/igt@[email protected]

#### Possible fixes ####

* igt@i915_pm_rpm@module-reload:
- fi-skl-lmem: [DMESG-WARN][9] ([fdo#112261]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7358/fi-skl-lmem/igt@[email protected]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/fi-skl-lmem/igt@[email protected]

* igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
- fi-skl-6770hq: [FAIL][11] ([fdo#109495]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7358/fi-skl-6770hq/igt@[email protected]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/fi-skl-6770hq/igt@[email protected]

[fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
[fdo#109495]: https://bugs.freedesktop.org/show_bug.cgi?id=109495
[fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261

Participating hosts (50 -> 43)
------------------------------

Additional (2): fi-kbl-7500u fi-bsw-n3050
Missing (9): fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-elk-e7500 fi-byt-clapper fi-bdw-samus

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

* CI: CI-20190529 -> None
* IGT: IGT_5290 -> IGTPW_3718

CI-20190529: 20190529
CI_DRM_7358: 3ff71899c56c5ebe182c84edf0567280863e553e @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3718: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3718/index.html
IGT_5290: 14d19371610fabafa0ee5a21160373b90ada0a30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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