public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper
@ 2018-12-04 16:25 Lukasz Kalamarz
  2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 2/3] tests: Use i915_get_param where applicable Lukasz Kalamarz
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Lukasz Kalamarz @ 2018-12-04 16:25 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

getparam is used in few places across IGT, but no helper function
is used to reduce code duplication.

v2: Added doc part and changed return value in case of error
v3: Renamed function to i915_get_param, created internal error
checking function and helper function to check if given feature
is enabled.

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>
Cc: Petri Latvala <Petri.latvala@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Ankit K Nautiyal <ankit.k.nautiyal@intel.com>
---
 lib/ioctl_wrappers.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
 lib/ioctl_wrappers.h |  2 ++
 2 files changed, 52 insertions(+)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 9f255508..b48dad5b 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -468,6 +468,56 @@ void gem_sync(int fd, uint32_t handle)
 	errno = 0;
 }
 
+static int
+__i915_get_param(int fd, struct drm_i915_getparam *gp)
+{
+	int err;
+
+	err = 0;
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp))
+		err = -errno;
+
+	errno = 0;
+	return err;
+}
+
+/**
+ * i915_get_param:
+ * @fd: open i915 drm file descriptor
+ * @param: drm parameter we want to read
+ *
+ * Helper function that execute GETPARAM ioctl for a given parameter.
+ *
+ * Return: Read value from GETPARAM
+ */
+int i915_get_param(int fd, uint32_t param)
+{
+	int value;
+	drm_i915_getparam_t gp = {
+		.param = param,
+		.value = &value
+	};
+
+	igt_assert_eq(__i915_get_param(fd, &gp), 0);
+
+	return value;
+}
+
+/**
+ * i915_has_feature:
+ * @fd: open i915 drm file descriptor
+ * @param: drm parameter we want to read
+ *
+ * Helper function that check if given functionality is enabled.
+ * Check is done via getparam IOCTL.
+ *
+ * Return: Value whether feature is enabled or not
+ */
+
+bool i915_has_feature(int fd, uint32_t param)
+{
+	return i915_get_param(fd, param) > 0;
+}
 
 bool gem_create__has_stolen_support(int fd)
 {
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index b22b36b0..c314a070 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -74,6 +74,8 @@ int __gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
 void gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
 int gem_wait(int fd, uint32_t handle, int64_t *timeout_ns);
 void gem_sync(int fd, uint32_t handle);
+int i915_get_param(int fd, uint32_t param);
+bool i915_has_feature(int fd, uint32_t param);
 bool gem_create__has_stolen_support(int fd);
 uint32_t __gem_create_stolen(int fd, uint64_t size);
 uint32_t gem_create_stolen(int fd, 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] 9+ messages in thread

* [igt-dev] [PATCH i-g-t v4 2/3] tests: Use i915_get_param where applicable
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
@ 2018-12-04 16:25 ` Lukasz Kalamarz
  2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 3/3] lib: Use i915_get_param where it is applicable Lukasz Kalamarz
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Lukasz Kalamarz @ 2018-12-04 16:25 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

Across several tests we check values of a given parameters.
With implementation of i915_get_param we can drop duplicated
lines and use helper function instead.

v2: Fixed errors in argument (fd are named as i915)
v3: Used defined parameters names instead of numbers
v4: Used renamed function and helper has_feature 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>
Cc: Petri Latvala <Petri.latvala@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Ankit K Nautiyal <ankit.k.nautiyal@intel.com>
---
 tests/i915/gem_busy.c          | 13 ++-----------
 tests/i915/gem_cs_tlb.c        | 13 +------------
 tests/i915/gem_ctx_isolation.c |  6 +-----
 tests/i915/gem_exec_async.c    | 10 ++--------
 tests/i915/gem_exec_capture.c  |  9 +--------
 tests/i915/gem_exec_fence.c    | 24 ++----------------------
 tests/i915/gem_exec_flush.c    | 11 ++---------
 tests/i915/gem_exec_params.c   | 16 ++--------------
 tests/i915/gem_exec_parse.c    |  7 ++-----
 tests/i915/gem_exec_suspend.c  | 12 +-----------
 tests/i915/gem_mmap_gtt.c      |  9 +++------
 tests/i915/gem_mmap_wc.c       |  7 +------
 tests/i915/hangman.c           |  9 +--------
 tests/prime_vgem.c             |  9 +--------
 14 files changed, 22 insertions(+), 133 deletions(-)

diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
index 76b44a5d..7baf7f1f 100644
--- a/tests/i915/gem_busy.c
+++ b/tests/i915/gem_busy.c
@@ -28,6 +28,7 @@
 #include "igt.h"
 #include "igt_rand.h"
 #include "igt_vgem.h"
+#include "ioctl_wrappers.h"
 #include "i915/gem_ring.h"
 
 #define LOCAL_EXEC_NO_RELOC (1<<11)
@@ -401,17 +402,7 @@ static void close_race(int fd)
 
 static bool has_semaphores(int fd)
 {
-	struct drm_i915_getparam gp;
-	int val = -1;
-
-	memset(&gp, 0, sizeof(gp));
-	gp.param = I915_PARAM_HAS_SEMAPHORES;
-	gp.value = &val;
-
-	drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-	errno = 0;
-
-	return val > 0;
+	return i915_has_feature(fd, I915_PARAM_HAS_SEMAPHORES);
 }
 
 static bool has_extended_busy_ioctl(int fd)
diff --git a/tests/i915/gem_cs_tlb.c b/tests/i915/gem_cs_tlb.c
index 51e1c4e1..f50b7f52 100644
--- a/tests/i915/gem_cs_tlb.c
+++ b/tests/i915/gem_cs_tlb.c
@@ -58,18 +58,7 @@ IGT_TEST_DESCRIPTION("Check whether we correctly invalidate the cs tlb.");
 
 static bool has_softpin(int fd)
 {
-	struct drm_i915_getparam gp;
-	int val = 0;
-
-	memset(&gp, 0, sizeof(gp));
-	gp.param = 37; /* I915_PARAM_HAS_EXEC_SOFTPIN */
-	gp.value = &val;
-
-	if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
-		return 0;
-
-	errno = 0;
-	return (val == 1);
+	return i915_has_feature(fd, I915_PARAM_HAS_EXEC_SOFTPIN);
 }
 
 static void *
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 058cf3ec..8e38a3a9 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -671,14 +671,10 @@ static void preservation(int fd,
 
 static unsigned int __has_context_isolation(int fd)
 {
-	struct drm_i915_getparam gp;
 	int value = 0;
 
-	memset(&gp, 0, sizeof(gp));
-	gp.param = 50; /* I915_PARAM_HAS_CONTEXT_ISOLATION */
-	gp.value = &value;
+	value = i915_get_param(fd, I915_PARAM_HAS_CONTEXT_ISOLATION);
 
-	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 	errno = 0;
 
 	return value;
diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
index 9a06af7e..c65a9f07 100644
--- a/tests/i915/gem_exec_async.c
+++ b/tests/i915/gem_exec_async.c
@@ -22,6 +22,7 @@
  */
 
 #include "igt.h"
+#include "ioctl_wrappers.h"
 
 #define LOCAL_OBJECT_ASYNC (1 << 6)
 #define LOCAL_PARAM_HAS_EXEC_ASYNC 43
@@ -173,14 +174,7 @@ static void one(int fd, unsigned ring, uint32_t flags)
 
 static bool has_async_execbuf(int fd)
 {
-	drm_i915_getparam_t gp;
-	int async = -1;
-
-	gp.param = LOCAL_PARAM_HAS_EXEC_ASYNC;
-	gp.value = &async;
-	drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-	return async > 0;
+	return i915_has_feature(fd, LOCAL_PARAM_HAS_EXEC_ASYNC);
 }
 
 igt_main
diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index f26b4102..93b5dd3f 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -509,14 +509,7 @@ static void userptr(int fd, int dir)
 
 static bool has_capture(int fd)
 {
-	drm_i915_getparam_t gp;
-	int async = -1;
-
-	gp.param = LOCAL_PARAM_HAS_EXEC_CAPTURE;
-	gp.value = &async;
-	drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-
-	return async > 0;
+	return i915_has_feature(fd, LOCAL_PARAM_HAS_EXEC_CAPTURE);
 }
 
 igt_main
diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index ba46595d..a9442e13 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -803,17 +803,7 @@ static void test_fence_flip(int i915)
 
 static bool has_submit_fence(int fd)
 {
-	struct drm_i915_getparam gp;
-	int value = 0;
-
-	memset(&gp, 0, sizeof(gp));
-	gp.param = 0xdeadbeef ^ 51; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
-	gp.value = &value;
-
-	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
-	errno = 0;
-
-	return value;
+	return i915_has_feature(fd, 0xdeadbeef ^ 51); /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
 }
 
 static bool has_syncobj(int fd)
@@ -825,17 +815,7 @@ static bool has_syncobj(int fd)
 
 static bool exec_has_fence_array(int fd)
 {
-	struct drm_i915_getparam gp;
-	int value = 0;
-
-	memset(&gp, 0, sizeof(gp));
-	gp.param = 49; /* I915_PARAM_HAS_EXEC_FENCE_ARRAY */
-	gp.value = &value;
-
-	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
-	errno = 0;
-
-	return value;
+	return i915_has_feature(fd, I915_PARAM_HAS_EXEC_FENCE_ARRAY);
 }
 
 static void test_invalid_fence_array(int fd)
diff --git a/tests/i915/gem_exec_flush.c b/tests/i915/gem_exec_flush.c
index f820b2a8..197fe23a 100644
--- a/tests/i915/gem_exec_flush.c
+++ b/tests/i915/gem_exec_flush.c
@@ -357,15 +357,8 @@ static void batch(int fd, unsigned ring, int nchild, int timeout,
 {
 	const int gen = intel_gen(intel_get_drm_devid(fd));
 
-	if (flags & CMDPARSER) {
-		int cmdparser = -1;
-                drm_i915_getparam_t gp;
-
-		gp.param = I915_PARAM_CMD_PARSER_VERSION;
-		gp.value = &cmdparser;
-		drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-		igt_require(cmdparser > 0);
-	}
+	if (flags & CMDPARSER)
+		igt_require(i915_has_feature(fd, I915_PARAM_CMD_PARSER_VERSION));
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, nchild) {
diff --git a/tests/i915/gem_exec_params.c b/tests/i915/gem_exec_params.c
index 49c56a8d..1ead75bd 100644
--- a/tests/i915/gem_exec_params.c
+++ b/tests/i915/gem_exec_params.c
@@ -78,24 +78,12 @@ static bool has_ring(int fd, unsigned ring_exec_flags)
 
 static bool has_exec_batch_first(int fd)
 {
-	int val = -1;
-	struct drm_i915_getparam gp = {
-		.param = 48,
-		.value = &val,
-	};
-	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-	return val > 0;
+	return i915_has_feature(fd, I915_PARAM_HAS_EXEC_BATCH_FIRST);
 }
 
 static bool has_resource_streamer(int fd)
 {
-	int val = -1;
-	struct drm_i915_getparam gp = {
-		.param = I915_PARAM_HAS_RESOURCE_STREAMER,
-		.value = &val,
-	};
-	ioctl(fd, DRM_IOCTL_I915_GETPARAM , &gp);
-	return val > 0;
+	return i915_has_feature(fd, I915_PARAM_HAS_RESOURCE_STREAMER);
 }
 
 static void test_batch_first(int fd)
diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
index b653b1bd..c6bf8eb8 100644
--- a/tests/i915/gem_exec_parse.c
+++ b/tests/i915/gem_exec_parse.c
@@ -61,12 +61,9 @@ static int parser_version;
 static int command_parser_version(int fd)
 {
 	int version = -1;
-	drm_i915_getparam_t gp;
 
-	gp.param = I915_PARAM_CMD_PARSER_VERSION;
-	gp.value = &version;
-
-	if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) == 0)
+	version = i915_get_param(fd, I915_PARAM_CMD_PARSER_VERSION);
+	if (version >= 1)
 		return version;
 
 	return -1;
diff --git a/tests/i915/gem_exec_suspend.c b/tests/i915/gem_exec_suspend.c
index 43c52d10..2e1be233 100644
--- a/tests/i915/gem_exec_suspend.c
+++ b/tests/i915/gem_exec_suspend.c
@@ -73,17 +73,7 @@ static void test_all(int fd, unsigned flags)
 
 static bool has_semaphores(int fd)
 {
-	struct drm_i915_getparam gp;
-	int val = -1;
-
-	memset(&gp, 0, sizeof(gp));
-	gp.param = I915_PARAM_HAS_SEMAPHORES;
-	gp.value = &val;
-
-	drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-	errno = 0;
-
-	return val > 0;
+	return i915_has_feature(fd, I915_PARAM_HAS_SEMAPHORES);
 }
 
 static void run_test(int fd, unsigned engine, unsigned flags)
diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
index f6353555..3d55cc64 100644
--- a/tests/i915/gem_mmap_gtt.c
+++ b/tests/i915/gem_mmap_gtt.c
@@ -311,13 +311,10 @@ test_write_gtt(int fd)
 
 static bool is_coherent(int i915)
 {
-	int val = 1; /* by default, we assume GTT is coherent, hence the test */
-	struct drm_i915_getparam gp = {
-		gp.param = 52, /* GTT_COHERENT */
-		gp.value = &val,
-	};
+	int val;
+
+	val = i915_get_param(i915, I915_PARAM_MMAP_GTT_COHERENT);
 
-	ioctl(i915, DRM_IOCTL_I915_GETPARAM, &gp);
 	return val;
 }
 
diff --git a/tests/i915/gem_mmap_wc.c b/tests/i915/gem_mmap_wc.c
index 110883eb..6ad979ce 100644
--- a/tests/i915/gem_mmap_wc.c
+++ b/tests/i915/gem_mmap_wc.c
@@ -85,7 +85,6 @@ create_pointer(int fd)
 static void
 test_invalid_flags(int fd)
 {
-	struct drm_i915_getparam gp;
 	struct local_i915_gem_mmap_v2 arg;
 	uint64_t flag = I915_MMAP_WC;
 	int val = -1;
@@ -95,12 +94,8 @@ test_invalid_flags(int fd)
 	arg.offset = 0;
 	arg.size = 4096;
 
-	memset(&gp, 0, sizeof(gp));
-	gp.param = 30; /* MMAP_VERSION */
-	gp.value = &val;
-
 	/* Do we have the new mmap_ioctl? */
-	drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	val = i915_get_param(fd, I915_PARAM_MMAP_VERSION);
 
 	if (val >= 1) {
 		/*
diff --git a/tests/i915/hangman.c b/tests/i915/hangman.c
index 6ddae491..6d7f342b 100644
--- a/tests/i915/hangman.c
+++ b/tests/i915/hangman.c
@@ -111,14 +111,7 @@ static FILE *open_error(void)
 
 static bool uses_cmd_parser(void)
 {
-	int parser_version = 0;
-	drm_i915_getparam_t gp;
-
-	gp.param = I915_PARAM_CMD_PARSER_VERSION;
-	gp.value = &parser_version;
-	drmIoctl(device, DRM_IOCTL_I915_GETPARAM, &gp);
-
-	return parser_version > 0;
+	return i915_get_param(device, I915_PARAM_CMD_PARSER_VERSION);
 }
 
 static void check_error_state(const char *expected_ring_name,
diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index 60bb951c..addc3e90 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -265,14 +265,7 @@ static void test_shrink(int vgem, int i915)
 
 static bool is_coherent(int i915)
 {
-	int val = 1; /* by default, we assume GTT is coherent, hence the test */
-	struct drm_i915_getparam gp = {
-		gp.param = 52, /* GTT_COHERENT */
-		gp.value = &val,
-	};
-
-	ioctl(i915, DRM_IOCTL_I915_GETPARAM, &gp);
-	return val;
+	return i915_get_param(i915, I915_PARAM_MMAP_GTT_COHERENT);
 }
 
 static void test_gtt_interleaved(int vgem, int i915)
-- 
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] 9+ messages in thread

* [igt-dev] [PATCH i-g-t v4 3/3] lib: Use i915_get_param where it is applicable
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
  2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 2/3] tests: Use i915_get_param where applicable Lukasz Kalamarz
@ 2018-12-04 16:25 ` Lukasz Kalamarz
  2018-12-04 16:41   ` Chris Wilson
  2018-12-04 16:39 ` [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Chris Wilson
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Lukasz Kalamarz @ 2018-12-04 16:25 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

With usage of implemented i915_get_param helper function, we can
remove some duplicated code lines across few libs.

v2: Fix typos and missing include
v3: Drop unnecessary getparam structure in one function
v4: Rebased and used renamed 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>
Cc: Petri Latvala <Petri.latvala@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Ankit K Nautiyal <ankit.k.nautiyal@intel.com>
---
 lib/drmtest.c             | 13 ++------
 lib/i915/gem_scheduler.c  |  8 +----
 lib/i915/gem_submission.c | 10 +++---
 lib/igt_gt.c              |  9 ++----
 lib/intel_chipset.c       |  7 ++---
 lib/ioctl_wrappers.c      | 66 ++++++---------------------------------
 lib/meson.build           |  3 +-
 7 files changed, 25 insertions(+), 91 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index d2aa1c19..3006c855 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -107,20 +107,11 @@ bool is_i915_device(int fd)
 
 static bool has_known_intel_chipset(int fd)
 {
-	struct drm_i915_getparam gp;
 	int devid = 0;
 
-	memset(&gp, 0, sizeof(gp));
-	gp.param = I915_PARAM_CHIPSET_ID;
-	gp.value = &devid;
+	devid = i915_get_param(fd, I915_PARAM_CHIPSET_ID);
 
-	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
-		return false;
-
-	if (!intel_gen(devid))
-		return false;
-
-	return true;
+	return devid > 0 && intel_gen(devid);
 }
 
 #define LOCAL_I915_EXEC_VEBOX	(4 << 0)
diff --git a/lib/i915/gem_scheduler.c b/lib/i915/gem_scheduler.c
index ad156306..7d509393 100644
--- a/lib/i915/gem_scheduler.c
+++ b/lib/i915/gem_scheduler.c
@@ -52,14 +52,8 @@ unsigned gem_scheduler_capability(int fd)
 	static int caps = -1;
 
 	if (caps < 0) {
-		struct drm_i915_getparam gp;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = LOCAL_I915_PARAM_HAS_SCHEDULER;
-		gp.value = &caps;
-
 		caps = 0;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
+		caps = i915_get_param(fd, LOCAL_I915_PARAM_HAS_SCHEDULER);
 		errno = 0;
 	}
 
diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 2fd460d5..40938667 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -53,12 +53,12 @@
 static bool has_semaphores(int fd, int dir)
 {
 	int val = 0;
-	struct drm_i915_getparam gp = {
-		gp.param = I915_PARAM_HAS_SEMAPHORES,
-		gp.value = &val,
-	};
-	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) < 0)
+
+	val = i915_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
+
+	if (val < 0)
 		val = igt_sysfs_get_boolean(dir, "semaphores");
+
 	return val;
 }
 
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index a2061924..663f41e8 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -57,14 +57,11 @@ static bool has_gpu_reset(int fd)
 {
 	static int once = -1;
 	if (once < 0) {
-		struct drm_i915_getparam gp;
-		int val = 0;
+		int val = -1;
 
-		memset(&gp, 0, sizeof(gp));
-		gp.param = 35; /* HAS_GPU_RESET */
-		gp.value = &val;
+		val = i915_get_param(fd, I915_PARAM_HAS_GPU_RESET);
 
-		if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
+		if (val < 0)
 			once = intel_gen(intel_get_drm_devid(fd)) >= 5;
 		else
 			once = val > 0;
diff --git a/lib/intel_chipset.c b/lib/intel_chipset.c
index 4748a3fb..a6a80f58 100644
--- a/lib/intel_chipset.c
+++ b/lib/intel_chipset.c
@@ -41,6 +41,7 @@
 #include "drmtest.h"
 #include "intel_chipset.h"
 #include "igt_core.h"
+#include "ioctl_wrappers.h"
 
 /**
  * SECTION:intel_chipset
@@ -125,7 +126,6 @@ intel_get_pci_device(void)
 uint32_t
 intel_get_drm_devid(int fd)
 {
-	struct drm_i915_getparam gp;
 	const char *override;
 	int devid = 0;
 
@@ -135,10 +135,7 @@ intel_get_drm_devid(int fd)
 	if (override)
 		return strtol(override, NULL, 0);
 
-	memset(&gp, 0, sizeof(gp));
-	gp.param = I915_PARAM_CHIPSET_ID;
-	gp.value = &devid;
-	ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
+	devid = i915_get_param(fd, I915_PARAM_CHIPSET_ID);
 
 	return devid;
 }
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index b48dad5b..dfe8ba69 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -522,16 +522,12 @@ bool i915_has_feature(int fd, uint32_t param)
 bool gem_create__has_stolen_support(int fd)
 {
 	static int has_stolen_support = -1;
-	struct drm_i915_getparam gp;
 	int val = -1;
 
 	if (has_stolen_support < 0) {
-		memset(&gp, 0, sizeof(gp));
-		gp.param = 38; /* CREATE_VERSION */
-		gp.value = &val;
+		val = i915_get_param(fd, I915_PARAM_HAS_POOLED_EU);
 
 		/* Do we have the extended gem_create_ioctl? */
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 		has_stolen_support = val >= 2;
 	}
 
@@ -751,21 +747,13 @@ 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 = 40; /* 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.value = &mmap_version;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+		gtt_version = i915_get_param(fd, I915_PARAM_MMAP_GTT_VERSION);
+		mmap_version = i915_get_param(fd, I915_PARAM_MMAP_VERSION);
 
 		/* Do we have the new mmap_ioctl with DOMAIN_WC? */
 		if (mmap_version >= 1 && gtt_version >= 2) {
@@ -1010,17 +998,11 @@ bool gem_bo_busy(int fd, uint32_t handle)
  */
 static int gem_gtt_type(int fd)
 {
-	struct drm_i915_getparam gp;
 	int val = 0;
 
-	memset(&gp, 0, sizeof(gp));
-	gp.param = 18; /* HAS_ALIASING_PPGTT */
-	gp.value = &val;
-
-	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
-		return 0;
-
+	val = i915_get_param(fd, I915_PARAM_HAS_ALIASING_PPGTT);
 	errno = 0;
+
 	return val;
 }
 
@@ -1064,13 +1046,9 @@ bool gem_uses_full_ppgtt(int fd)
  */
 int gem_gpu_reset_type(int fd)
 {
-	struct drm_i915_getparam gp;
 	int gpu_reset_type = -1;
 
-	memset(&gp, 0, sizeof(gp));
-	gp.param = I915_PARAM_HAS_GPU_RESET;
-	gp.value = &gpu_reset_type;
-	drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	gpu_reset_type = i915_get_param(fd, I915_PARAM_HAS_GPU_RESET);
 
 	return gpu_reset_type;
 }
@@ -1118,14 +1096,8 @@ int gem_available_fences(int fd)
 	static int num_fences = -1;
 
 	if (num_fences < 0) {
-		struct drm_i915_getparam gp;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_NUM_FENCES_AVAIL;
-		gp.value = &num_fences;
-
 		num_fences = 0;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
+		num_fences = i915_get_param(fd, I915_PARAM_NUM_FENCES_AVAIL);
 		errno = 0;
 	}
 
@@ -1137,14 +1109,8 @@ bool gem_has_llc(int fd)
 	static int has_llc = -1;
 
 	if (has_llc < 0) {
-		struct drm_i915_getparam gp;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_HAS_LLC;
-		gp.value = &has_llc;
-
 		has_llc = 0;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
+		has_llc = i915_get_param(fd, I915_PARAM_HAS_LLC);
 		errno = 0;
 	}
 
@@ -1394,14 +1360,8 @@ bool gem_has_softpin(int fd)
 	static int has_softpin = -1;
 
 	if (has_softpin < 0) {
-		struct drm_i915_getparam gp;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_HAS_EXEC_SOFTPIN;
-		gp.value = &has_softpin;
-
 		has_softpin = 0;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
+		has_softpin = i915_has_feature(fd, I915_PARAM_HAS_EXEC_SOFTPIN);
 		errno = 0;
 	}
 
@@ -1422,14 +1382,8 @@ bool gem_has_exec_fence(int fd)
 	static int has_exec_fence = -1;
 
 	if (has_exec_fence < 0) {
-		struct drm_i915_getparam gp;
-
-		memset(&gp, 0, sizeof(gp));
-		gp.param = I915_PARAM_HAS_EXEC_FENCE;
-		gp.value = &has_exec_fence;
-
 		has_exec_fence = 0;
-		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
+		has_exec_fence = i915_has_feature(fd, I915_PARAM_HAS_EXEC_FENCE);
 		errno = 0;
 	}
 
diff --git a/lib/meson.build b/lib/meson.build
index a3b24ea0..e987c32b 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -140,7 +140,8 @@ igt_deps = [ lib_igt ] + lib_deps
 lin_igt_chipset_build = static_library('igt_chipset',
                                        ['intel_chipset.c',
                                         'intel_device_info.c'],
-                                       include_directories : inc)
+                                       include_directories : inc,
+				       dependencies: lib_deps)
 
 lib_igt_chipset = declare_dependency(link_with : lin_igt_chipset_build,
                                      include_directories : inc)
-- 
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] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
  2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 2/3] tests: Use i915_get_param where applicable Lukasz Kalamarz
  2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 3/3] lib: Use i915_get_param where it is applicable Lukasz Kalamarz
@ 2018-12-04 16:39 ` Chris Wilson
  2018-12-04 16:40 ` Chris Wilson
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-12-04 16:39 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev; +Cc: Petri Latvala

Quoting Lukasz Kalamarz (2018-12-04 16:25:24)
> getparam is used in few places across IGT, but no helper function
> is used to reduce code duplication.
> 
> v2: Added doc part and changed return value in case of error
> v3: Renamed function to i915_get_param, created internal error
> checking function and helper function to check if given feature
> is enabled.
> 
> 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>
> Cc: Petri Latvala <Petri.latvala@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Ankit K Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  lib/ioctl_wrappers.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/ioctl_wrappers.h |  2 ++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 9f255508..b48dad5b 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c

Out of the freedom of lib/i915/*, you chose lib/ioctl_wrappers.c for this
i915 specific interface?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
                   ` (2 preceding siblings ...)
  2018-12-04 16:39 ` [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Chris Wilson
@ 2018-12-04 16:40 ` Chris Wilson
  2018-12-04 17:12 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] " Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-12-04 16:40 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev; +Cc: Petri Latvala

Quoting Lukasz Kalamarz (2018-12-04 16:25:24)
> getparam is used in few places across IGT, but no helper function
> is used to reduce code duplication.
> 
> v2: Added doc part and changed return value in case of error
> v3: Renamed function to i915_get_param, created internal error
> checking function and helper function to check if given feature
> is enabled.
> 
> 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>
> Cc: Petri Latvala <Petri.latvala@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Ankit K Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  lib/ioctl_wrappers.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/ioctl_wrappers.h |  2 ++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 9f255508..b48dad5b 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -468,6 +468,56 @@ void gem_sync(int fd, uint32_t handle)
>         errno = 0;
>  }
>  
> +static int
> +__i915_get_param(int fd, struct drm_i915_getparam *gp)
> +{
> +       int err;
> +
> +       err = 0;
> +       if (igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp))
> +               err = -errno;
> +
> +       errno = 0;
> +       return err;
> +}
> +
> +/**
> + * i915_get_param:
> + * @fd: open i915 drm file descriptor
> + * @param: drm parameter we want to read
> + *
> + * Helper function that execute GETPARAM ioctl for a given parameter.
> + *
> + * Return: Read value from GETPARAM
> + */
> +int i915_get_param(int fd, uint32_t param)
> +{
> +       int value;
> +       drm_i915_getparam_t gp = {
> +               .param = param,
> +               .value = &value
> +       };
> +
> +       igt_assert_eq(__i915_get_param(fd, &gp), 0);
> +
> +       return value;
> +}
> +
> +/**
> + * i915_has_feature:
> + * @fd: open i915 drm file descriptor
> + * @param: drm parameter we want to read
> + *
> + * Helper function that check if given functionality is enabled.
> + * Check is done via getparam IOCTL.
> + *
> + * Return: Value whether feature is enabled or not
> + */
> +
> +bool i915_has_feature(int fd, uint32_t param)
> +{
> +       return i915_get_param(fd, param) > 0;

Nope, this should return false not assert if the kernel doesn't
recognise said feature.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v4 3/3] lib: Use i915_get_param where it is applicable
  2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 3/3] lib: Use i915_get_param where it is applicable Lukasz Kalamarz
@ 2018-12-04 16:41   ` Chris Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-12-04 16:41 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev; +Cc: Petri Latvala

Quoting Lukasz Kalamarz (2018-12-04 16:25:26)
> With usage of implemented i915_get_param helper function, we can
> remove some duplicated code lines across few libs.
> 
> v2: Fix typos and missing include
> v3: Drop unnecessary getparam structure in one function
> v4: Rebased and used renamed function

So many more unwanted asserts.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] lib/ioctl_wrapper: Add i915_get_param helper
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
                   ` (3 preceding siblings ...)
  2018-12-04 16:40 ` Chris Wilson
@ 2018-12-04 17:12 ` Patchwork
  2018-12-05  0:39 ` [igt-dev] [PATCH i-g-t v4 1/3] " Daniele Ceraolo Spurio
  2018-12-05  2:24 ` [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v4,1/3] " Patchwork
  6 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-12-04 17:12 UTC (permalink / raw)
  To: Lukasz Kalamarz; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v4,1/3] lib/ioctl_wrapper: Add i915_get_param helper
URL   : https://patchwork.freedesktop.org/series/53484/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5258 -> IGTPW_2118
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-ivb-3520m:       PASS -> FAIL [fdo#108880]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       PASS -> DMESG-WARN [fdo#102614]

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         PASS -> FAIL [fdo#104008]
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * {igt@runner@aborted}:
    - {fi-icl-y}:         NOTRUN -> FAIL [fdo#108070]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         DMESG-WARN [fdo#108622] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880


Participating hosts (49 -> 45)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

    * IGT: IGT_4740 -> IGTPW_2118

  CI_DRM_5258: 266e44c58acaeda4bbbee47a04fa61c309615cc4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2118: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2118/
  IGT_4740: dd8de0efa64e50bc06c2882a0028d98ad870e752 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
                   ` (4 preceding siblings ...)
  2018-12-04 17:12 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] " Patchwork
@ 2018-12-05  0:39 ` Daniele Ceraolo Spurio
  2018-12-05  2:24 ` [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v4,1/3] " Patchwork
  6 siblings, 0 replies; 9+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-12-05  0:39 UTC (permalink / raw)
  To: Lukasz Kalamarz, igt-dev; +Cc: Petri Latvala



On 04/12/2018 08:25, Lukasz Kalamarz wrote:
> getparam is used in few places across IGT, but no helper function
> is used to reduce code duplication.
> 
> v2: Added doc part and changed return value in case of error
> v3: Renamed function to i915_get_param, created internal error
> checking function and helper function to check if given feature
> is enabled.
> 
> 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>
> Cc: Petri Latvala <Petri.latvala@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Ankit K Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>   lib/ioctl_wrappers.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
>   lib/ioctl_wrappers.h |  2 ++
>   2 files changed, 52 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 9f255508..b48dad5b 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -468,6 +468,56 @@ void gem_sync(int fd, uint32_t handle)
>   	errno = 0;
>   }
>   
> +static int
> +__i915_get_param(int fd, struct drm_i915_getparam *gp)

This is only called in 1 place. Is it really worth having a separate 
fuction?

> +{
> +	int err;
> +
> +	err = 0;
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp))
> +		err = -errno;
> +
> +	errno = 0;
> +	return err;
> +}
> +
> +/**
> + * i915_get_param:
> + * @fd: open i915 drm file descriptor
> + * @param: drm parameter we want to read
> + *
> + * Helper function that execute GETPARAM ioctl for a given parameter.
> + *
> + * Return: Read value from GETPARAM
> + */
> +int i915_get_param(int fd, uint32_t param)
> +{
> +	int value;
> +	drm_i915_getparam_t gp = {
> +		.param = param,
> +		.value = &value
> +	};
> +
> +	igt_assert_eq(__i915_get_param(fd, &gp), 0);

This assert breaks support for running on older kernels that don't have 
the param flag defined. Most of the current code assumes a default value 
if the flag is not defined (usually feature missing or version 0), but 
with this assert here we'd fail the test instead.
A good example is I915_PARAM_MMAP_GTT_COHERENT, which was only added 
recently and for which the test assumes a default of "true" if the flag 
is unsupported by the kernel.

> +
> +	return value;
> +}
> +
> +/**
> + * i915_has_feature:
> + * @fd: open i915 drm file descriptor
> + * @param: drm parameter we want to read
> + *
> + * Helper function that check if given functionality is enabled.
> + * Check is done via getparam IOCTL.
> + *
> + * Return: Value whether feature is enabled or not
> + */
> +
> +bool i915_has_feature(int fd, uint32_t param)

Just realized that there is already an has_param() wrapper that does 
more or less the same thing, so we can reuse that and/or adapt it to use 
i915_get_param.

Daniele

> +{
> +	return i915_get_param(fd, param) > 0;
> +}
>   
>   bool gem_create__has_stolen_support(int fd)
>   {
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index b22b36b0..c314a070 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -74,6 +74,8 @@ int __gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
>   void gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
>   int gem_wait(int fd, uint32_t handle, int64_t *timeout_ns);
>   void gem_sync(int fd, uint32_t handle);
> +int i915_get_param(int fd, uint32_t param);
> +bool i915_has_feature(int fd, uint32_t param);
>   bool gem_create__has_stolen_support(int fd);
>   uint32_t __gem_create_stolen(int fd, uint64_t size);
>   uint32_t gem_create_stolen(int fd, 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] 9+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v4,1/3] lib/ioctl_wrapper: Add i915_get_param helper
  2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
                   ` (5 preceding siblings ...)
  2018-12-05  0:39 ` [igt-dev] [PATCH i-g-t v4 1/3] " Daniele Ceraolo Spurio
@ 2018-12-05  2:24 ` Patchwork
  6 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-12-05  2:24 UTC (permalink / raw)
  To: Lukasz Kalamarz; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v4,1/3] lib/ioctl_wrapper: Add i915_get_param helper
URL   : https://patchwork.freedesktop.org/series/53484/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5258_full -> IGTPW_2118_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2118_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2118_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/53484/revisions/1/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_busy@extended-semaphore-blt:
    - shard-hsw:          PASS -> FAIL

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-glk:          PASS -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-kbl:          PASS -> TIMEOUT [fdo#108887]

  * igt@i915_suspend@forcewake:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665] +1

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#107956] +1
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_color@pipe-c-ctm-blue-to-red:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103313] / [fdo#105345]

  * igt@kms_color@pipe-c-ctm-max:
    - shard-kbl:          PASS -> FAIL [fdo#108147]
    - shard-apl:          PASS -> FAIL [fdo#108147] +1

  * igt@kms_color@pipe-c-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

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

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

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232] +1

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

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

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

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

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

  
#### Possible fixes ####

  * igt@gem_exec_fence@basic-await-default:
    - shard-hsw:          FAIL [fdo#108888] -> PASS

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-b:
    - shard-apl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +12

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

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-apl:          DMESG-FAIL [fdo#103558] / [fdo#105602] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +2

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

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +5

  * igt@kms_cursor_legacy@pipe-b-forked-bo:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

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

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

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS +4

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-apl:          FAIL -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-glk:          FAIL [fdo#103166] -> PASS

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

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

  
#### Warnings ####

  * igt@kms_content_protection@atomic:
    - shard-apl:          DMESG-FAIL [fdo#103558] / [fdo#105602] -> FAIL [fdo#108597]

  * {igt@kms_rotation_crc@multiplane-rotation-cropping-top}:
    - shard-kbl:          DMESG-FAIL -> DMESG-WARN [fdo#105604]
    - shard-glk:          DMESG-WARN [fdo#105763] / [fdo#106538] -> DMESG-FAIL [fdo#105763] / [fdo#106538]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [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#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105345]: https://bugs.freedesktop.org/show_bug.cgi?id=105345
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108597]: https://bugs.freedesktop.org/show_bug.cgi?id=108597
  [fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
  [fdo#108888]: https://bugs.freedesktop.org/show_bug.cgi?id=108888
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Missing    (2): shard-skl shard-iclb 


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

    * IGT: IGT_4740 -> IGTPW_2118
    * Piglit: piglit_4509 -> None

  CI_DRM_5258: 266e44c58acaeda4bbbee47a04fa61c309615cc4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2118: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2118/
  IGT_4740: dd8de0efa64e50bc06c2882a0028d98ad870e752 @ 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_2118/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2018-12-05  2:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-04 16:25 [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 2/3] tests: Use i915_get_param where applicable Lukasz Kalamarz
2018-12-04 16:25 ` [igt-dev] [PATCH i-g-t v4 3/3] lib: Use i915_get_param where it is applicable Lukasz Kalamarz
2018-12-04 16:41   ` Chris Wilson
2018-12-04 16:39 ` [igt-dev] [PATCH i-g-t v4 1/3] lib/ioctl_wrapper: Add i915_get_param helper Chris Wilson
2018-12-04 16:40 ` Chris Wilson
2018-12-04 17:12 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] " Patchwork
2018-12-05  0:39 ` [igt-dev] [PATCH i-g-t v4 1/3] " Daniele Ceraolo Spurio
2018-12-05  2:24 ` [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v4,1/3] " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox