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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.