From: Antonio Argenziano <antonio.argenziano@intel.com>
To: Lukasz Kalamarz <lukasz.kalamarz@intel.com>,
igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t v3 2/3] tests: Use drm_get_param where applicable
Date: Mon, 26 Nov 2018 13:37:38 -0800 [thread overview]
Message-ID: <c021c381-7485-227a-c34e-d1f7b535d262@intel.com> (raw)
In-Reply-To: <20181126153624.3690-2-lukasz.kalamarz@intel.com>
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;
> }
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
next prev parent reply other threads:[~2018-11-26 21:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-26 15:36 [igt-dev] [PATCH i-g-t v3 1/3] lib/ioctl_wrapper: Add i915_get_param helper Lukasz Kalamarz
2018-11-26 15:36 ` [igt-dev] [PATCH i-g-t v3 2/3] tests: Use drm_get_param where applicable Lukasz Kalamarz
2018-11-26 21:37 ` Antonio Argenziano [this message]
2018-11-28 22:06 ` Daniele Ceraolo Spurio
2018-11-26 15:36 ` [igt-dev] [PATCH i-g-t v3 3/3] lib: Use drm_get_param where it is applicable Lukasz Kalamarz
2018-11-28 23:01 ` Daniele Ceraolo Spurio
2018-11-26 15:45 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v3,1/3] lib/ioctl_wrapper: Add i915_get_param helper Patchwork
2018-11-26 20:49 ` [igt-dev] [PATCH i-g-t v3 1/3] " Eric Anholt
2018-11-27 6:42 ` Nautiyal, Ankit K
2018-11-27 11:46 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v3,1/3] lib/ioctl_wrapper: Add i915_get_param helper (rev2) Patchwork
2018-11-28 21:34 ` [igt-dev] [PATCH i-g-t v3 1/3] lib/ioctl_wrapper: Add i915_get_param helper Daniele Ceraolo Spurio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c021c381-7485-227a-c34e-d1f7b535d262@intel.com \
--to=antonio.argenziano@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=lukasz.kalamarz@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox