[i-g-t,v3,2/3] tests: Use drm_get_param where applicable

Submitted by Lukasz Kalamarz on Nov. 26, 2018, 3:36 p.m.

Details

Message ID 20181126153624.3690-2-lukasz.kalamarz@intel.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Lukasz Kalamarz Nov. 26, 2018, 3:36 p.m.
Across several tests we check values of a given parameters.
With implementation of drm_get_param we can drop duplicated
lines and use helper function instead.
v2: Fixed errors in argument (fd are named as i915)

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>
---
 tests/i915/gem_busy.c          |  8 ++------
 tests/i915/gem_cs_tlb.c        | 10 ++--------
 tests/i915/gem_ctx_isolation.c |  7 +------
 tests/i915/gem_exec_async.c    |  6 ++----
 tests/i915/gem_exec_capture.c  |  5 +----
 tests/i915/gem_exec_fence.c    | 14 ++------------
 tests/i915/gem_exec_flush.c    |  5 +----
 tests/i915/gem_exec_params.c   | 16 ++++++----------
 tests/i915/gem_exec_parse.c    |  7 ++-----
 tests/i915/gem_exec_suspend.c  |  7 +------
 tests/i915/gem_mmap_gtt.c      |  7 ++-----
 tests/i915/gem_mmap_wc.c       |  7 +------
 tests/i915/hangman.c           |  5 +----
 tests/prime_vgem.c             |  7 ++-----
 14 files changed, 26 insertions(+), 85 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
index 76b44a5d..6cc8e45b 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,14 +402,9 @@  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);
+	val = drm_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
 	errno = 0;
 
 	return val > 0;
diff --git a/tests/i915/gem_cs_tlb.c b/tests/i915/gem_cs_tlb.c
index 51e1c4e1..891cd551 100644
--- a/tests/i915/gem_cs_tlb.c
+++ b/tests/i915/gem_cs_tlb.c
@@ -58,17 +58,11 @@  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;
-
+	val = drm_get_param(fd, 37); /* I915_PARAM_HAS_EXEC_SOFTPIN */
 	errno = 0;
+
 	return (val == 1);
 }
 
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 058cf3ec..27c8429d 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -671,14 +671,9 @@  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;
-
-	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	value = drm_get_param(fd, 50); /* I915_PARAM_HAS_CONTEXT_ISOLATION */
 	errno = 0;
 
 	return value;
diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
index 9a06af7e..6b5a5d25 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,12 +174,9 @@  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);
+	async = drm_get_param(fd, LOCAL_PARAM_HAS_EXEC_ASYNC);
 
 	return async > 0;
 }
diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
index 3e4a4377..72cf8ef4 100644
--- a/tests/i915/gem_exec_capture.c
+++ b/tests/i915/gem_exec_capture.c
@@ -189,12 +189,9 @@  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);
+	async = drm_get_param(fd, LOCAL_PARAM_HAS_EXEC_CAPTURE);
 
 	return async > 0;
 }
diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index ba46595d..ee143a5f 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -803,14 +803,9 @@  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));
+	value = drm_get_param(fd, 0xdeadbeef ^ 51); /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
 	errno = 0;
 
 	return value;
@@ -825,14 +820,9 @@  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));
+	value = drm_get_param(fd, 49); /* I915_PARAM_HAS_EXEC_FENCE_ARRAY */
 	errno = 0;
 
 	return value;
diff --git a/tests/i915/gem_exec_flush.c b/tests/i915/gem_exec_flush.c
index f820b2a8..bdc9257d 100644
--- a/tests/i915/gem_exec_flush.c
+++ b/tests/i915/gem_exec_flush.c
@@ -359,11 +359,8 @@  static void batch(int fd, unsigned ring, int nchild, int timeout,
 
 	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);
+		cmdparser = drm_get_param(fd, I915_PARAM_CMD_PARSER_VERSION);
 		igt_require(cmdparser > 0);
 	}
 
diff --git a/tests/i915/gem_exec_params.c b/tests/i915/gem_exec_params.c
index 49c56a8d..5f443961 100644
--- a/tests/i915/gem_exec_params.c
+++ b/tests/i915/gem_exec_params.c
@@ -79,22 +79,18 @@  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);
+
+	val = drm_get_param(fd, 48);
+
 	return val > 0;
 }
 
 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);
+
+	val = drm_get_param(fd, I915_PARAM_HAS_RESOURCE_STREAMER);
+
 	return val > 0;
 }
 
diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
index b653b1bd..15315438 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 = drm_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..b44af299 100644
--- a/tests/i915/gem_exec_suspend.c
+++ b/tests/i915/gem_exec_suspend.c
@@ -73,14 +73,9 @@  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);
+	val = drm_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
 	errno = 0;
 
 	return val > 0;
diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
index f6353555..396694e6 100644
--- a/tests/i915/gem_mmap_gtt.c
+++ b/tests/i915/gem_mmap_gtt.c
@@ -312,12 +312,9 @@  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,
-	};
 
-	ioctl(i915, DRM_IOCTL_I915_GETPARAM, &gp);
+	val = drm_get_param(i915, 52); /* GTT_COHERENT */
+
 	return val;
 }
 
diff --git a/tests/i915/gem_mmap_wc.c b/tests/i915/gem_mmap_wc.c
index 110883eb..73b2166b 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 = drm_get_param(fd, 30); /* MMAP_VERSION */
 
 	if (val >= 1) {
 		/*
diff --git a/tests/i915/hangman.c b/tests/i915/hangman.c
index 6ddae491..09316226 100644
--- a/tests/i915/hangman.c
+++ b/tests/i915/hangman.c
@@ -112,11 +112,8 @@  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);
+	parser_version = drm_get_param(device, I915_PARAM_CMD_PARSER_VERSION);
 
 	return parser_version > 0;
 }
diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index 60bb951c..759299da 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -266,12 +266,9 @@  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);
+	val = drm_get_param(i915, 52); /* GTT_COHERENT */
+
 	return val;
 }
 

Comments

Antonio Argenziano Nov. 26, 2018, 9:37 p.m.
On 26/11/18 07:36, Lukasz Kalamarz wrote:
> Across several tests we check values of a given parameters.
> With implementation of drm_get_param we can drop duplicated
> lines and use helper function instead.
> v2: Fixed errors in argument (fd are named as i915)
> 
> 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>
> ---
>   tests/i915/gem_busy.c          |  8 ++------
>   tests/i915/gem_cs_tlb.c        | 10 ++--------
>   tests/i915/gem_ctx_isolation.c |  7 +------
>   tests/i915/gem_exec_async.c    |  6 ++----
>   tests/i915/gem_exec_capture.c  |  5 +----
>   tests/i915/gem_exec_fence.c    | 14 ++------------
>   tests/i915/gem_exec_flush.c    |  5 +----
>   tests/i915/gem_exec_params.c   | 16 ++++++----------
>   tests/i915/gem_exec_parse.c    |  7 ++-----
>   tests/i915/gem_exec_suspend.c  |  7 +------
>   tests/i915/gem_mmap_gtt.c      |  7 ++-----
>   tests/i915/gem_mmap_wc.c       |  7 +------
>   tests/i915/hangman.c           |  5 +----
>   tests/prime_vgem.c             |  7 ++-----
>   14 files changed, 26 insertions(+), 85 deletions(-)
> 
> diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
> index 76b44a5d..6cc8e45b 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,14 +402,9 @@ 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);
> +	val = drm_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
>   	errno = 0;
>   
>   	return val > 0;
> diff --git a/tests/i915/gem_cs_tlb.c b/tests/i915/gem_cs_tlb.c
> index 51e1c4e1..891cd551 100644
> --- a/tests/i915/gem_cs_tlb.c
> +++ b/tests/i915/gem_cs_tlb.c
> @@ -58,17 +58,11 @@ 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;
> -
> +	val = drm_get_param(fd, 37); /* I915_PARAM_HAS_EXEC_SOFTPIN */

Since you are here, this param is now defined in i915_drm.h use the 
macro defined there. There are a few more cases below as well.

>   	errno = 0;
> +
>   	return (val == 1);
>   }
>   
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 058cf3ec..27c8429d 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -671,14 +671,9 @@ 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;
> -
> -	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +	value = drm_get_param(fd, 50); /* I915_PARAM_HAS_CONTEXT_ISOLATION */

here^

>   	errno = 0;
>   
>   	return value;
> diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
> index 9a06af7e..6b5a5d25 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,12 +174,9 @@ 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);
> +	async = drm_get_param(fd, LOCAL_PARAM_HAS_EXEC_ASYNC);
>   
>   	return async > 0;
>   }
> diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
> index 3e4a4377..72cf8ef4 100644
> --- a/tests/i915/gem_exec_capture.c
> +++ b/tests/i915/gem_exec_capture.c
> @@ -189,12 +189,9 @@ 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);
> +	async = drm_get_param(fd, LOCAL_PARAM_HAS_EXEC_CAPTURE);
>   
>   	return async > 0;
>   }
> diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
> index ba46595d..ee143a5f 100644
> --- a/tests/i915/gem_exec_fence.c
> +++ b/tests/i915/gem_exec_fence.c
> @@ -803,14 +803,9 @@ 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));
> +	value = drm_get_param(fd, 0xdeadbeef ^ 51); /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
>   	errno = 0;
>   
>   	return value;
> @@ -825,14 +820,9 @@ 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));
> +	value = drm_get_param(fd, 49); /* I915_PARAM_HAS_EXEC_FENCE_ARRAY */

Here^

>   	errno = 0;
>   
>   	return value;
> diff --git a/tests/i915/gem_exec_flush.c b/tests/i915/gem_exec_flush.c
> index f820b2a8..bdc9257d 100644
> --- a/tests/i915/gem_exec_flush.c
> +++ b/tests/i915/gem_exec_flush.c
> @@ -359,11 +359,8 @@ static void batch(int fd, unsigned ring, int nchild, int timeout,
>   
>   	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);
> +		cmdparser = drm_get_param(fd, I915_PARAM_CMD_PARSER_VERSION);
>   		igt_require(cmdparser > 0);
>   	}
>   
> diff --git a/tests/i915/gem_exec_params.c b/tests/i915/gem_exec_params.c
> index 49c56a8d..5f443961 100644
> --- a/tests/i915/gem_exec_params.c
> +++ b/tests/i915/gem_exec_params.c
> @@ -79,22 +79,18 @@ 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);
> +
> +	val = drm_get_param(fd, 48);

Here^

> +
>   	return val > 0;
>   }
>   
>   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);
> +
> +	val = drm_get_param(fd, I915_PARAM_HAS_RESOURCE_STREAMER);
> +
>   	return val > 0;
>   }
>   
> diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> index b653b1bd..15315438 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 = drm_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..b44af299 100644
> --- a/tests/i915/gem_exec_suspend.c
> +++ b/tests/i915/gem_exec_suspend.c
> @@ -73,14 +73,9 @@ 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);
> +	val = drm_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
>   	errno = 0;
>   
>   	return val > 0;
> diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
> index f6353555..396694e6 100644
> --- a/tests/i915/gem_mmap_gtt.c
> +++ b/tests/i915/gem_mmap_gtt.c
> @@ -312,12 +312,9 @@ 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,
> -	};
>   
> -	ioctl(i915, DRM_IOCTL_I915_GETPARAM, &gp);
> +	val = drm_get_param(i915, 52); /* GTT_COHERENT */
> +
>   	return val;
>   }
>   
> diff --git a/tests/i915/gem_mmap_wc.c b/tests/i915/gem_mmap_wc.c
> index 110883eb..73b2166b 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 = drm_get_param(fd, 30); /* MMAP_VERSION */

here^

I think I got all of those that are defined but you should double check :).

Antonio.

>   
>   	if (val >= 1) {
>   		/*
> diff --git a/tests/i915/hangman.c b/tests/i915/hangman.c
> index 6ddae491..09316226 100644
> --- a/tests/i915/hangman.c
> +++ b/tests/i915/hangman.c
> @@ -112,11 +112,8 @@ 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);
> +	parser_version = drm_get_param(device, I915_PARAM_CMD_PARSER_VERSION);
>   
>   	return parser_version > 0;
>   }
> diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
> index 60bb951c..759299da 100644
> --- a/tests/prime_vgem.c
> +++ b/tests/prime_vgem.c
> @@ -266,12 +266,9 @@ 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);
> +	val = drm_get_param(i915, 52); /* GTT_COHERENT */
> +
>   	return val;
>   }
>   
>
Daniele Ceraolo Spurio Nov. 28, 2018, 10:06 p.m.
On 26/11/2018 07:36, Lukasz Kalamarz wrote:
> Across several tests we check values of a given parameters.
> With implementation of drm_get_param we can drop duplicated
> lines and use helper function instead.
> v2: Fixed errors in argument (fd are named as i915)
> 
> 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>
> ---
>   tests/i915/gem_busy.c          |  8 ++------
>   tests/i915/gem_cs_tlb.c        | 10 ++--------
>   tests/i915/gem_ctx_isolation.c |  7 +------
>   tests/i915/gem_exec_async.c    |  6 ++----
>   tests/i915/gem_exec_capture.c  |  5 +----
>   tests/i915/gem_exec_fence.c    | 14 ++------------
>   tests/i915/gem_exec_flush.c    |  5 +----
>   tests/i915/gem_exec_params.c   | 16 ++++++----------
>   tests/i915/gem_exec_parse.c    |  7 ++-----
>   tests/i915/gem_exec_suspend.c  |  7 +------
>   tests/i915/gem_mmap_gtt.c      |  7 ++-----
>   tests/i915/gem_mmap_wc.c       |  7 +------
>   tests/i915/hangman.c           |  5 +----
>   tests/prime_vgem.c             |  7 ++-----
>   14 files changed, 26 insertions(+), 85 deletions(-)
> 
> diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
> index 76b44a5d..6cc8e45b 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,14 +402,9 @@ 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);
> +	val = drm_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
>   	errno = 0;
>   
>   	return val > 0;
> diff --git a/tests/i915/gem_cs_tlb.c b/tests/i915/gem_cs_tlb.c
> index 51e1c4e1..891cd551 100644
> --- a/tests/i915/gem_cs_tlb.c
> +++ b/tests/i915/gem_cs_tlb.c
> @@ -58,17 +58,11 @@ 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;
> -
> +	val = drm_get_param(fd, 37); /* I915_PARAM_HAS_EXEC_SOFTPIN */
>   	errno = 0;
> +
>   	return (val == 1);
>   }
>   
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 058cf3ec..27c8429d 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -671,14 +671,9 @@ 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;
> -
> -	igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +	value = drm_get_param(fd, 50); /* I915_PARAM_HAS_CONTEXT_ISOLATION */

The code that uses this function does an igt_require() on the its 
return, so it will now pass even if value < 0. value was not modified 
before for non existing params but it is now negative with your change.

>   	errno = 0;
>   
>   	return value;
> diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
> index 9a06af7e..6b5a5d25 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,12 +174,9 @@ 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);
> +	async = drm_get_param(fd, LOCAL_PARAM_HAS_EXEC_ASYNC);
>   
>   	return async > 0;
>   }
> diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
> index 3e4a4377..72cf8ef4 100644
> --- a/tests/i915/gem_exec_capture.c
> +++ b/tests/i915/gem_exec_capture.c
> @@ -189,12 +189,9 @@ 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);
> +	async = drm_get_param(fd, LOCAL_PARAM_HAS_EXEC_CAPTURE);
>   
>   	return async > 0;
>   }
> diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
> index ba46595d..ee143a5f 100644
> --- a/tests/i915/gem_exec_fence.c
> +++ b/tests/i915/gem_exec_fence.c
> @@ -803,14 +803,9 @@ 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));
> +	value = drm_get_param(fd, 0xdeadbeef ^ 51); /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
>   	errno = 0;
>   
>   	return value;

The return is converted to bool, so even if value is negative it'll 
become true. return value > 0?

> @@ -825,14 +820,9 @@ 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));
> +	value = drm_get_param(fd, 49); /* I915_PARAM_HAS_EXEC_FENCE_ARRAY */
>   	errno = 0;
>   
>   	return value;

Same as above

> diff --git a/tests/i915/gem_exec_flush.c b/tests/i915/gem_exec_flush.c
> index f820b2a8..bdc9257d 100644
> --- a/tests/i915/gem_exec_flush.c
> +++ b/tests/i915/gem_exec_flush.c
> @@ -359,11 +359,8 @@ static void batch(int fd, unsigned ring, int nchild, int timeout,
>   
>   	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);
> +		cmdparser = drm_get_param(fd, I915_PARAM_CMD_PARSER_VERSION);
>   		igt_require(cmdparser > 0);
>   	}
>   
> diff --git a/tests/i915/gem_exec_params.c b/tests/i915/gem_exec_params.c
> index 49c56a8d..5f443961 100644
> --- a/tests/i915/gem_exec_params.c
> +++ b/tests/i915/gem_exec_params.c
> @@ -79,22 +79,18 @@ 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);
> +
> +	val = drm_get_param(fd, 48);
> +
>   	return val > 0;
>   }
>   
>   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);
> +
> +	val = drm_get_param(fd, I915_PARAM_HAS_RESOURCE_STREAMER);
> +
>   	return val > 0;
>   }
>   
> diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> index b653b1bd..15315438 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 = drm_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..b44af299 100644
> --- a/tests/i915/gem_exec_suspend.c
> +++ b/tests/i915/gem_exec_suspend.c
> @@ -73,14 +73,9 @@ 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);
> +	val = drm_get_param(fd, I915_PARAM_HAS_SEMAPHORES);
>   	errno = 0;
>   
>   	return val > 0;
> diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
> index f6353555..396694e6 100644
> --- a/tests/i915/gem_mmap_gtt.c
> +++ b/tests/i915/gem_mmap_gtt.c
> @@ -312,12 +312,9 @@ test_write_gtt(int fd)
>   static bool is_coherent(int i915)
>   {
>   	int val = 1; /* by default, we assume GTT is coherent, hence the test */

you're breaking the assumption that val is not modified by the getparam

> -	struct drm_i915_getparam gp = {
> -		gp.param = 52, /* GTT_COHERENT */
> -		gp.value = &val,
> -	};
>   
> -	ioctl(i915, DRM_IOCTL_I915_GETPARAM, &gp);
> +	val = drm_get_param(i915, 52); /* GTT_COHERENT */
> +
>   	return val;

Again cast to bool

>   }
>   
> diff --git a/tests/i915/gem_mmap_wc.c b/tests/i915/gem_mmap_wc.c
> index 110883eb..73b2166b 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 = drm_get_param(fd, 30); /* MMAP_VERSION */
>   
>   	if (val >= 1) {
>   		/*
> diff --git a/tests/i915/hangman.c b/tests/i915/hangman.c
> index 6ddae491..09316226 100644
> --- a/tests/i915/hangman.c
> +++ b/tests/i915/hangman.c
> @@ -112,11 +112,8 @@ 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);
> +	parser_version = drm_get_param(device, I915_PARAM_CMD_PARSER_VERSION);
>   
>   	return parser_version > 0;
>   }
> diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
> index 60bb951c..759299da 100644
> --- a/tests/prime_vgem.c
> +++ b/tests/prime_vgem.c
> @@ -266,12 +266,9 @@ static void test_shrink(int vgem, int i915)
>   static bool is_coherent(int i915)
>   {

Same issues as the other is_coherent() function. BTW, maybe it'd be 
worth moving it to a common place.


>   	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);
> +	val = drm_get_param(i915, 52); /* GTT_COHERENT */
> +
>   	return val;
>   }
>   
> 

A lot of the params are booleans (i.e. the feature is either there or 
not, no version), so we can maybe have a second level wrapper. something 
like:

bool i915_has_feature(int fd, uint32_t param)
{
	return drm_get_param(fd, param) > 0;
}

Daniele