diff mbox

[1/2] intel-gpu-tools: pass argc/argv to simple main

Message ID 1403878537-29020-2-git-send-email-tim.gore@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

tim.gore@intel.com June 27, 2014, 2:15 p.m. UTC
From: Tim Gore <tim.gore@intel.com>

Quite a few single tests do not use the igt_simple_main
macro because they want access to argc/argv. So change the
igt_simple_main macro to pass these arguments through to the
"__real_mainxxx" function, and change these tests to use
the macro.

Signed-off-by: Tim Gore <tim.gore@intel.com>
---
 lib/igt_core.h                   |  8 ++++----
 tests/gem_ctx_basic.c            |  6 +-----
 tests/gem_exec_blt.c             |  5 +----
 tests/gem_gtt_speed.c            |  5 +----
 tests/gem_hang.c                 |  5 +----
 tests/gem_render_copy.c          |  4 +---
 tests/gem_render_linear_blits.c  |  5 +----
 tests/gem_render_tiled_blits.c   |  5 +----
 tests/gem_seqno_wrap.c           | 11 ++++-------
 tests/gem_stress.c               |  5 +----
 tests/gen3_mixed_blits.c         |  5 +----
 tests/gen3_render_linear_blits.c |  5 +----
 tests/gen3_render_mixed_blits.c  |  5 +----
 tests/gen3_render_tiledx_blits.c |  5 +----
 tests/gen3_render_tiledy_blits.c |  5 +----
 15 files changed, 21 insertions(+), 63 deletions(-)

Comments

Thomas Wood July 1, 2014, 9:19 a.m. UTC | #1
On 27 June 2014 15:15,  <tim.gore@intel.com> wrote:
> From: Tim Gore <tim.gore@intel.com>
>
> Quite a few single tests do not use the igt_simple_main
> macro because they want access to argc/argv. So change the
> igt_simple_main macro to pass these arguments through to the
> "__real_mainxxx" function, and change these tests to use
> the macro.

It might also be worth marking igt_simple_init as an internal function
as after this change it is only used in one of the internal framework
tests.

>
> Signed-off-by: Tim Gore <tim.gore@intel.com>
> ---
>  lib/igt_core.h                   |  8 ++++----
>  tests/gem_ctx_basic.c            |  6 +-----
>  tests/gem_exec_blt.c             |  5 +----
>  tests/gem_gtt_speed.c            |  5 +----
>  tests/gem_hang.c                 |  5 +----
>  tests/gem_render_copy.c          |  4 +---
>  tests/gem_render_linear_blits.c  |  5 +----
>  tests/gem_render_tiled_blits.c   |  5 +----
>  tests/gem_seqno_wrap.c           | 11 ++++-------
>  tests/gem_stress.c               |  5 +----
>  tests/gen3_mixed_blits.c         |  5 +----
>  tests/gen3_render_linear_blits.c |  5 +----
>  tests/gen3_render_mixed_blits.c  |  5 +----
>  tests/gen3_render_tiledx_blits.c |  5 +----
>  tests/gen3_render_tiledy_blits.c |  5 +----
>  15 files changed, 21 insertions(+), 63 deletions(-)
>
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index e252eba..9853e6b 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -176,13 +176,13 @@ void igt_simple_init(void);
>   * the test needs to parse additional cmdline arguments of its own.

This documentation comment needs updating to mention that argc and
argv are available.


>   */
>  #define igt_simple_main \
> -       static void igt_tokencat(__real_main, __LINE__)(void); \
> +       static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv); \
>         int main(int argc, char **argv) { \
>                 igt_simple_init(); \
> -               igt_tokencat(__real_main, __LINE__)(); \
> -               exit(0); \
> +               igt_tokencat(__real_main, __LINE__)(argc, argv); \
> +               igt_exit(); \
>         } \
> -       static void igt_tokencat(__real_main, __LINE__)(void) \
> +       static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv) \
>
>  __attribute__((format(printf, 1, 2)))
>  void igt_skip(const char *f, ...) __attribute__((noreturn));
> diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c
> index 3e9b688..fe770ea 100644
> --- a/tests/gem_ctx_basic.c
> +++ b/tests/gem_ctx_basic.c
> @@ -145,12 +145,10 @@ static void parse(int argc, char *argv[])
>         }
>  }
>
> -int main(int argc, char *argv[])
> +igt_simple_main
>  {
>         int i;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any_render();
>         devid = intel_get_drm_devid(fd);
>
> @@ -173,6 +171,4 @@ int main(int argc, char *argv[])
>
>         free(threads);
>         close(fd);
> -
> -       return 0;
>  }
> diff --git a/tests/gem_exec_blt.c b/tests/gem_exec_blt.c
> index 3bcef18..3d092fe 100644
> --- a/tests/gem_exec_blt.c
> +++ b/tests/gem_exec_blt.c
> @@ -253,12 +253,10 @@ static void run(int object_size)
>         close(fd);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         int i;
>
> -       igt_simple_init();
> -
>         igt_skip_on_simulation();
>
>         if (argc > 1) {
> @@ -270,5 +268,4 @@ int main(int argc, char **argv)
>         } else
>                 run(OBJECT_SIZE);
>
> -       return 0;
>  }
> diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c
> index 385eeb7..fa20de0 100644
> --- a/tests/gem_gtt_speed.c
> +++ b/tests/gem_gtt_speed.c
> @@ -50,7 +50,7 @@ static double elapsed(const struct timeval *start,
>         return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - start->tv_usec))/loop;
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         struct timeval start, end;
>         uint8_t *buf;
> @@ -59,8 +59,6 @@ int main(int argc, char **argv)
>         int loop, i, tiling;
>         int fd;
>
> -       igt_simple_init();
> -
>         igt_skip_on_simulation();
>
>         if (argc > 1)
> @@ -329,5 +327,4 @@ int main(int argc, char **argv)
>         gem_close(fd, handle);
>         close(fd);
>
> -       return 0;


There are a couple of other returns that need to be replaced in this
test and in a few others, since the function igt_simple_main creates
returns void.


>  }
> diff --git a/tests/gem_hang.c b/tests/gem_hang.c
> index 6248244..a4f4d10 100644
> --- a/tests/gem_hang.c
> +++ b/tests/gem_hang.c
> @@ -68,12 +68,10 @@ gpu_hang(void)
>         intel_batchbuffer_flush(batch);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         int fd;
>
> -       igt_simple_init();
> -
>         igt_assert_f(argc == 2,
>                      "usage: %s <disabled pipe number>\n",
>                      argv[0]);
> @@ -93,5 +91,4 @@ int main(int argc, char **argv)
>
>         close(fd);
>
> -       return 0;
>  }
> diff --git a/tests/gem_render_copy.c b/tests/gem_render_copy.c
> index fd26b43..12dd90d 100644
> --- a/tests/gem_render_copy.c
> +++ b/tests/gem_render_copy.c
> @@ -117,7 +117,7 @@ scratch_buf_check(data_t *data, struct igt_buf *buf, int x, int y,
>                      color, val, x, y);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         data_t data = {0, };
>         struct intel_batchbuffer *batch = NULL;
> @@ -127,7 +127,6 @@ int main(int argc, char **argv)
>         int opt_dump_png = false;
>         int opt_dump_aub = igt_aub_dump_enabled();
>
> -       igt_simple_init();
>
>         while ((opt = getopt(argc, argv, "d")) != -1) {
>                 switch (opt) {
> @@ -189,5 +188,4 @@ int main(int argc, char **argv)
>                 scratch_buf_check(&data, &dst, WIDTH - 10, HEIGHT - 10, SRC_COLOR);
>         }
>
> -       return 0;
>  }
> diff --git a/tests/gem_render_linear_blits.c b/tests/gem_render_linear_blits.c
> index f847486..7c6081e 100644
> --- a/tests/gem_render_linear_blits.c
> +++ b/tests/gem_render_linear_blits.c
> @@ -81,7 +81,7 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>         }
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         drm_intel_bufmgr *bufmgr;
>         struct intel_batchbuffer *batch;
> @@ -90,8 +90,6 @@ int main(int argc, char **argv)
>         uint32_t start = 0;
>         int i, j, fd, count;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any();
>
>         render_copy = igt_get_render_copyfunc(intel_get_drm_devid(fd));
> @@ -201,5 +199,4 @@ int main(int argc, char **argv)
>         for (i = 0; i < count; i++)
>                 check_bo(fd, bo[i]->handle, start_val[i]);
>
> -       return 0;
>  }
> diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c
> index f63c57e..ec8e8de 100644
> --- a/tests/gem_render_tiled_blits.c
> +++ b/tests/gem_render_tiled_blits.c
> @@ -95,7 +95,7 @@ check_bo(struct intel_batchbuffer *batch, struct igt_buf *buf, uint32_t val)
>                 dri_bo_unmap(linear);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         drm_intel_bufmgr *bufmgr;
>         struct intel_batchbuffer *batch;
> @@ -105,8 +105,6 @@ int main(int argc, char **argv)
>         int i, j, fd, count;
>         uint32_t devid;
>
> -       igt_simple_init();
> -
>         igt_skip_on_simulation();
>
>         fd = drm_open_any();
> @@ -212,5 +210,4 @@ int main(int argc, char **argv)
>         for (i = 0; i < count; i++)
>                 check_bo(batch, &buf[i], start_val[i]);
>
> -       return 0;
>  }
> diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
> index beda28b..8a54c2e 100644
> --- a/tests/gem_seqno_wrap.c
> +++ b/tests/gem_seqno_wrap.c
> @@ -533,12 +533,9 @@ static void parse_options(int argc, char **argv)
>         }
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         int wcount = 0;
> -       int r = -1;
> -
> -       igt_simple_init();
>
>         parse_options(argc, argv);
>
> @@ -563,8 +560,8 @@ int main(int argc, char **argv)
>
>         if (options.rounds == wcount) {
>                 igt_debug("done %d wraps successfully\n", wcount);
> -               return 0;
>         }
> -
> -       return r;
> +       else {
> +           igt_fail(-1);
> +       }
>  }
> diff --git a/tests/gem_stress.c b/tests/gem_stress.c
> index 2ccb6fc..35ed32f 100644
> --- a/tests/gem_stress.c
> +++ b/tests/gem_stress.c
> @@ -860,13 +860,11 @@ static void check_render_copyfunc(void)
>  }
>
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         int i, j;
>         unsigned *current_permutation, *tmp_permutation;
>
> -       igt_simple_init();
> -
>         drm_fd = drm_open_any();
>         devid = intel_get_drm_devid(drm_fd);
>
> @@ -925,5 +923,4 @@ int main(int argc, char **argv)
>
>         igt_stop_signal_helper();
>
> -       return 0;
>  }
> diff --git a/tests/gen3_mixed_blits.c b/tests/gen3_mixed_blits.c
> index 75d61a5..f0a6b64 100644
> --- a/tests/gen3_mixed_blits.c
> +++ b/tests/gen3_mixed_blits.c
> @@ -457,14 +457,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>         munmap(v, WIDTH*HEIGHT*4);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         uint32_t *handle, *tiling, *start_val;
>         uint32_t start = 0;
>         int i, fd, count;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any();
>
>         igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -533,5 +531,4 @@ int main(int argc, char **argv)
>                 check_bo(fd, handle[i], start_val[i]);
>         igt_info("done\n");
>
> -       return 0;
>  }
> diff --git a/tests/gen3_render_linear_blits.c b/tests/gen3_render_linear_blits.c
> index 7fe368d..60c0d0b 100644
> --- a/tests/gen3_render_linear_blits.c
> +++ b/tests/gen3_render_linear_blits.c
> @@ -325,14 +325,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>         }
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         uint32_t *handle, *start_val;
>         uint32_t start = 0;
>         int i, fd, count;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any();
>
>         igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -393,5 +391,4 @@ int main(int argc, char **argv)
>         for (i = 0; i < count; i++)
>                 check_bo(fd, handle[i], start_val[i]);
>
> -       return 0;
>  }
> diff --git a/tests/gen3_render_mixed_blits.c b/tests/gen3_render_mixed_blits.c
> index 77ac0e2..68660a3 100644
> --- a/tests/gen3_render_mixed_blits.c
> +++ b/tests/gen3_render_mixed_blits.c
> @@ -345,14 +345,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>         munmap(v, WIDTH*HEIGHT*4);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         uint32_t *handle, *tiling, *start_val;
>         uint32_t start = 0;
>         int i, fd, count;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any();
>
>         igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -421,5 +419,4 @@ int main(int argc, char **argv)
>                 check_bo(fd, handle[i], start_val[i]);
>         igt_info("done\n");
>
> -       return 0;
>  }
> diff --git a/tests/gen3_render_tiledx_blits.c b/tests/gen3_render_tiledx_blits.c
> index 95c0c96..d54d714 100644
> --- a/tests/gen3_render_tiledx_blits.c
> +++ b/tests/gen3_render_tiledx_blits.c
> @@ -332,14 +332,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>         munmap(v, WIDTH*HEIGHT*4);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         uint32_t *handle, *start_val;
>         uint32_t start = 0;
>         int i, fd, count;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any();
>
>         igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -400,5 +398,4 @@ int main(int argc, char **argv)
>         for (i = 0; i < count; i++)
>                 check_bo(fd, handle[i], start_val[i]);
>
> -       return 0;
>  }
> diff --git a/tests/gen3_render_tiledy_blits.c b/tests/gen3_render_tiledy_blits.c
> index 1b9a419..3ef08c8 100644
> --- a/tests/gen3_render_tiledy_blits.c
> +++ b/tests/gen3_render_tiledy_blits.c
> @@ -332,14 +332,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>         munmap(v, WIDTH*HEIGHT*4);
>  }
>
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>         uint32_t *handle, *start_val;
>         uint32_t start = 0;
>         int i, fd, count;
>
> -       igt_simple_init();
> -
>         fd = drm_open_any();
>
>         igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -407,5 +405,4 @@ int main(int argc, char **argv)
>                 check_bo(fd, handle[i], start_val[i]);
>         igt_info("done\n");
>
> -       return 0;
>  }
> --
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter July 7, 2014, 4:12 p.m. UTC | #2
On Fri, Jun 27, 2014 at 03:15:36PM +0100, tim.gore@intel.com wrote:
> From: Tim Gore <tim.gore@intel.com>
> 
> Quite a few single tests do not use the igt_simple_main
> macro because they want access to argc/argv. So change the
> igt_simple_main macro to pass these arguments through to the
> "__real_mainxxx" function, and change these tests to use
> the macro.
> 
> Signed-off-by: Tim Gore <tim.gore@intel.com>
> ---
>  lib/igt_core.h                   |  8 ++++----
>  tests/gem_ctx_basic.c            |  6 +-----
>  tests/gem_exec_blt.c             |  5 +----
>  tests/gem_gtt_speed.c            |  5 +----
>  tests/gem_hang.c                 |  5 +----
>  tests/gem_render_copy.c          |  4 +---
>  tests/gem_render_linear_blits.c  |  5 +----
>  tests/gem_render_tiled_blits.c   |  5 +----
>  tests/gem_seqno_wrap.c           | 11 ++++-------
>  tests/gem_stress.c               |  5 +----
>  tests/gen3_mixed_blits.c         |  5 +----
>  tests/gen3_render_linear_blits.c |  5 +----
>  tests/gen3_render_mixed_blits.c  |  5 +----
>  tests/gen3_render_tiledx_blits.c |  5 +----
>  tests/gen3_render_tiledy_blits.c |  5 +----
>  15 files changed, 21 insertions(+), 63 deletions(-)
> 
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index e252eba..9853e6b 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -176,13 +176,13 @@ void igt_simple_init(void);
>   * the test needs to parse additional cmdline arguments of its own.
>   */
>  #define igt_simple_main \
> -	static void igt_tokencat(__real_main, __LINE__)(void); \
> +	static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv); \
>  	int main(int argc, char **argv) { \
>  		igt_simple_init(); \
> -		igt_tokencat(__real_main, __LINE__)(); \
> -		exit(0); \
> +		igt_tokencat(__real_main, __LINE__)(argc, argv); \
> +		igt_exit(); \
>  	} \
> -	static void igt_tokencat(__real_main, __LINE__)(void) \
> +	static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv) \

Hm, I'd just add argc/argv to igt_simple_init like you do in the next
patch and update the relevant tests which have their own main. That would
be more in line with subtest-tests which have their own additional
arguments, too.

Also having functions with magic/predefined arguments is a bit too much
magic for my taste. If we keep the signature of real_main as-is we'll
avoid (highly unlikely, but still) accidental name clashes.

And with my suggestion for patch 2 to just have a bare-bones argv parsing
for simple tests we also don't need to call igt_exit.
-Daniel

>  
>  __attribute__((format(printf, 1, 2)))
>  void igt_skip(const char *f, ...) __attribute__((noreturn));
> diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c
> index 3e9b688..fe770ea 100644
> --- a/tests/gem_ctx_basic.c
> +++ b/tests/gem_ctx_basic.c
> @@ -145,12 +145,10 @@ static void parse(int argc, char *argv[])
>  	}
>  }
>  
> -int main(int argc, char *argv[])
> +igt_simple_main
>  {
>  	int i;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any_render();
>  	devid = intel_get_drm_devid(fd);
>  
> @@ -173,6 +171,4 @@ int main(int argc, char *argv[])
>  
>  	free(threads);
>  	close(fd);
> -
> -	return 0;
>  }
> diff --git a/tests/gem_exec_blt.c b/tests/gem_exec_blt.c
> index 3bcef18..3d092fe 100644
> --- a/tests/gem_exec_blt.c
> +++ b/tests/gem_exec_blt.c
> @@ -253,12 +253,10 @@ static void run(int object_size)
>  	close(fd);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	int i;
>  
> -	igt_simple_init();
> -
>  	igt_skip_on_simulation();
>  
>  	if (argc > 1) {
> @@ -270,5 +268,4 @@ int main(int argc, char **argv)
>  	} else
>  		run(OBJECT_SIZE);
>  
> -	return 0;
>  }
> diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c
> index 385eeb7..fa20de0 100644
> --- a/tests/gem_gtt_speed.c
> +++ b/tests/gem_gtt_speed.c
> @@ -50,7 +50,7 @@ static double elapsed(const struct timeval *start,
>  	return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - start->tv_usec))/loop;
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	struct timeval start, end;
>  	uint8_t *buf;
> @@ -59,8 +59,6 @@ int main(int argc, char **argv)
>  	int loop, i, tiling;
>  	int fd;
>  
> -	igt_simple_init();
> -
>  	igt_skip_on_simulation();
>  
>  	if (argc > 1)
> @@ -329,5 +327,4 @@ int main(int argc, char **argv)
>  	gem_close(fd, handle);
>  	close(fd);
>  
> -	return 0;
>  }
> diff --git a/tests/gem_hang.c b/tests/gem_hang.c
> index 6248244..a4f4d10 100644
> --- a/tests/gem_hang.c
> +++ b/tests/gem_hang.c
> @@ -68,12 +68,10 @@ gpu_hang(void)
>  	intel_batchbuffer_flush(batch);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	int fd;
>  
> -	igt_simple_init();
> -
>  	igt_assert_f(argc == 2,
>  		     "usage: %s <disabled pipe number>\n",
>  		     argv[0]);
> @@ -93,5 +91,4 @@ int main(int argc, char **argv)
>  
>  	close(fd);
>  
> -	return 0;
>  }
> diff --git a/tests/gem_render_copy.c b/tests/gem_render_copy.c
> index fd26b43..12dd90d 100644
> --- a/tests/gem_render_copy.c
> +++ b/tests/gem_render_copy.c
> @@ -117,7 +117,7 @@ scratch_buf_check(data_t *data, struct igt_buf *buf, int x, int y,
>  		     color, val, x, y);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	data_t data = {0, };
>  	struct intel_batchbuffer *batch = NULL;
> @@ -127,7 +127,6 @@ int main(int argc, char **argv)
>  	int opt_dump_png = false;
>  	int opt_dump_aub = igt_aub_dump_enabled();
>  
> -	igt_simple_init();
>  
>  	while ((opt = getopt(argc, argv, "d")) != -1) {
>  		switch (opt) {
> @@ -189,5 +188,4 @@ int main(int argc, char **argv)
>  		scratch_buf_check(&data, &dst, WIDTH - 10, HEIGHT - 10, SRC_COLOR);
>  	}
>  
> -	return 0;
>  }
> diff --git a/tests/gem_render_linear_blits.c b/tests/gem_render_linear_blits.c
> index f847486..7c6081e 100644
> --- a/tests/gem_render_linear_blits.c
> +++ b/tests/gem_render_linear_blits.c
> @@ -81,7 +81,7 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>  	}
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	drm_intel_bufmgr *bufmgr;
>  	struct intel_batchbuffer *batch;
> @@ -90,8 +90,6 @@ int main(int argc, char **argv)
>  	uint32_t start = 0;
>  	int i, j, fd, count;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any();
>  
>  	render_copy = igt_get_render_copyfunc(intel_get_drm_devid(fd));
> @@ -201,5 +199,4 @@ int main(int argc, char **argv)
>  	for (i = 0; i < count; i++)
>  		check_bo(fd, bo[i]->handle, start_val[i]);
>  
> -	return 0;
>  }
> diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c
> index f63c57e..ec8e8de 100644
> --- a/tests/gem_render_tiled_blits.c
> +++ b/tests/gem_render_tiled_blits.c
> @@ -95,7 +95,7 @@ check_bo(struct intel_batchbuffer *batch, struct igt_buf *buf, uint32_t val)
>  		dri_bo_unmap(linear);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	drm_intel_bufmgr *bufmgr;
>  	struct intel_batchbuffer *batch;
> @@ -105,8 +105,6 @@ int main(int argc, char **argv)
>  	int i, j, fd, count;
>  	uint32_t devid;
>  
> -	igt_simple_init();
> -
>  	igt_skip_on_simulation();
>  
>  	fd = drm_open_any();
> @@ -212,5 +210,4 @@ int main(int argc, char **argv)
>  	for (i = 0; i < count; i++)
>  		check_bo(batch, &buf[i], start_val[i]);
>  
> -	return 0;
>  }
> diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
> index beda28b..8a54c2e 100644
> --- a/tests/gem_seqno_wrap.c
> +++ b/tests/gem_seqno_wrap.c
> @@ -533,12 +533,9 @@ static void parse_options(int argc, char **argv)
>  	}
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	int wcount = 0;
> -	int r = -1;
> -
> -	igt_simple_init();
>  
>  	parse_options(argc, argv);
>  
> @@ -563,8 +560,8 @@ int main(int argc, char **argv)
>  
>  	if (options.rounds == wcount) {
>  		igt_debug("done %d wraps successfully\n", wcount);
> -		return 0;
>  	}
> -
> -	return r;
> +	else {
> +	    igt_fail(-1);
> +	}
>  }
> diff --git a/tests/gem_stress.c b/tests/gem_stress.c
> index 2ccb6fc..35ed32f 100644
> --- a/tests/gem_stress.c
> +++ b/tests/gem_stress.c
> @@ -860,13 +860,11 @@ static void check_render_copyfunc(void)
>  }
>  
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	int i, j;
>  	unsigned *current_permutation, *tmp_permutation;
>  
> -	igt_simple_init();
> -
>  	drm_fd = drm_open_any();
>  	devid = intel_get_drm_devid(drm_fd);
>  
> @@ -925,5 +923,4 @@ int main(int argc, char **argv)
>  
>  	igt_stop_signal_helper();
>  
> -	return 0;
>  }
> diff --git a/tests/gen3_mixed_blits.c b/tests/gen3_mixed_blits.c
> index 75d61a5..f0a6b64 100644
> --- a/tests/gen3_mixed_blits.c
> +++ b/tests/gen3_mixed_blits.c
> @@ -457,14 +457,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>  	munmap(v, WIDTH*HEIGHT*4);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	uint32_t *handle, *tiling, *start_val;
>  	uint32_t start = 0;
>  	int i, fd, count;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any();
>  
>  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -533,5 +531,4 @@ int main(int argc, char **argv)
>  		check_bo(fd, handle[i], start_val[i]);
>  	igt_info("done\n");
>  
> -	return 0;
>  }
> diff --git a/tests/gen3_render_linear_blits.c b/tests/gen3_render_linear_blits.c
> index 7fe368d..60c0d0b 100644
> --- a/tests/gen3_render_linear_blits.c
> +++ b/tests/gen3_render_linear_blits.c
> @@ -325,14 +325,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>  	}
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	uint32_t *handle, *start_val;
>  	uint32_t start = 0;
>  	int i, fd, count;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any();
>  
>  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -393,5 +391,4 @@ int main(int argc, char **argv)
>  	for (i = 0; i < count; i++)
>  		check_bo(fd, handle[i], start_val[i]);
>  
> -	return 0;
>  }
> diff --git a/tests/gen3_render_mixed_blits.c b/tests/gen3_render_mixed_blits.c
> index 77ac0e2..68660a3 100644
> --- a/tests/gen3_render_mixed_blits.c
> +++ b/tests/gen3_render_mixed_blits.c
> @@ -345,14 +345,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>  	munmap(v, WIDTH*HEIGHT*4);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	uint32_t *handle, *tiling, *start_val;
>  	uint32_t start = 0;
>  	int i, fd, count;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any();
>  
>  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -421,5 +419,4 @@ int main(int argc, char **argv)
>  		check_bo(fd, handle[i], start_val[i]);
>  	igt_info("done\n");
>  
> -	return 0;
>  }
> diff --git a/tests/gen3_render_tiledx_blits.c b/tests/gen3_render_tiledx_blits.c
> index 95c0c96..d54d714 100644
> --- a/tests/gen3_render_tiledx_blits.c
> +++ b/tests/gen3_render_tiledx_blits.c
> @@ -332,14 +332,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>  	munmap(v, WIDTH*HEIGHT*4);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	uint32_t *handle, *start_val;
>  	uint32_t start = 0;
>  	int i, fd, count;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any();
>  
>  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -400,5 +398,4 @@ int main(int argc, char **argv)
>  	for (i = 0; i < count; i++)
>  		check_bo(fd, handle[i], start_val[i]);
>  
> -	return 0;
>  }
> diff --git a/tests/gen3_render_tiledy_blits.c b/tests/gen3_render_tiledy_blits.c
> index 1b9a419..3ef08c8 100644
> --- a/tests/gen3_render_tiledy_blits.c
> +++ b/tests/gen3_render_tiledy_blits.c
> @@ -332,14 +332,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
>  	munmap(v, WIDTH*HEIGHT*4);
>  }
>  
> -int main(int argc, char **argv)
> +igt_simple_main
>  {
>  	uint32_t *handle, *start_val;
>  	uint32_t start = 0;
>  	int i, fd, count;
>  
> -	igt_simple_init();
> -
>  	fd = drm_open_any();
>  
>  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> @@ -407,5 +405,4 @@ int main(int argc, char **argv)
>  		check_bo(fd, handle[i], start_val[i]);
>  	igt_info("done\n");
>  
> -	return 0;
>  }
> -- 
> 1.9.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
tim.gore@intel.com July 9, 2014, 1:50 p.m. UTC | #3
Some inline comments

  Tim

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, July 07, 2014 5:12 PM
> To: Gore, Tim
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/2] intel-gpu-tools: pass argc/argv to simple
> main
> 
> On Fri, Jun 27, 2014 at 03:15:36PM +0100, tim.gore@intel.com wrote:
> > From: Tim Gore <tim.gore@intel.com>
> >
> > Quite a few single tests do not use the igt_simple_main macro because
> > they want access to argc/argv. So change the igt_simple_main macro to
> > pass these arguments through to the "__real_mainxxx" function, and
> > change these tests to use the macro.
> >
> > Signed-off-by: Tim Gore <tim.gore@intel.com>
> > ---
> >  lib/igt_core.h                   |  8 ++++----

 
> >  tests/gem_ctx_basic.c            |  6 +-----
> >  tests/gem_exec_blt.c             |  5 +----
> >  tests/gem_gtt_speed.c            |  5 +----
> >  tests/gem_hang.c                 |  5 +----
> >  tests/gem_render_copy.c          |  4 +---
> >  tests/gem_render_linear_blits.c  |  5 +----
> >  tests/gem_render_tiled_blits.c   |  5 +----
> >  tests/gem_seqno_wrap.c           | 11 ++++-------
> >  tests/gem_stress.c               |  5 +----
> >  tests/gen3_mixed_blits.c         |  5 +----
> >  tests/gen3_render_linear_blits.c |  5 +----
> > tests/gen3_render_mixed_blits.c  |  5 +----
> > tests/gen3_render_tiledx_blits.c |  5 +----
> > tests/gen3_render_tiledy_blits.c |  5 +----
> >  15 files changed, 21 insertions(+), 63 deletions(-)
> >
> > diff --git a/lib/igt_core.h b/lib/igt_core.h index e252eba..9853e6b
> > 100644
> > --- a/lib/igt_core.h
> > +++ b/lib/igt_core.h
> > @@ -176,13 +176,13 @@ void igt_simple_init(void);
> >   * the test needs to parse additional cmdline arguments of its own.
> >   */
> >  #define igt_simple_main \
> > -	static void igt_tokencat(__real_main, __LINE__)(void); \
> > +	static void igt_tokencat(__real_main, __LINE__)(int argc, char
> > +**argv); \
> >  	int main(int argc, char **argv) { \
> >  		igt_simple_init(); \
> > -		igt_tokencat(__real_main, __LINE__)(); \
> > -		exit(0); \
> > +		igt_tokencat(__real_main, __LINE__)(argc, argv); \
> > +		igt_exit(); \
> >  	} \
> > -	static void igt_tokencat(__real_main, __LINE__)(void) \
> > +	static void igt_tokencat(__real_main, __LINE__)(int argc, char
> > +**argv) \
> 
> Hm, I'd just add argc/argv to igt_simple_init like you do in the next patch and
> update the relevant tests which have their own main. That would be more in
> line with subtest-tests which have their own additional arguments, too.
> 
> Also having functions with magic/predefined arguments is a bit too much
> magic for my taste. If we keep the signature of real_main as-is we'll avoid
> (highly unlikely, but still) accidental name clashes.
> 
> And with my suggestion for patch 2 to just have a bare-bones argv parsing for
> simple tests we also don't need to call igt_exit.
> -Daniel

    My motivation here is mainly to reduce the amount of differentiation, so that
    we can move away from having a different methodology for tests that want to
    parse argv. Together with the next patch we move towards all tests having  the
    same setup/initialisation. I believe that this will make the test suite easier to
    maintain and add features to, such as common Doxygen/--help text.
    I agree that the passing argc/v through to real_main is not ideal, but the whole
    macro thing is not to my taste to be honest, for all the usual reasons. If we can
   reduce the number of test "types" (single/multiple , parses argv/or not) it
    should make the macros easier to maintain etc.

       Tim
> 
> >
> >  __attribute__((format(printf, 1, 2)))  void igt_skip(const char *f,
> > ...) __attribute__((noreturn)); diff --git a/tests/gem_ctx_basic.c
> > b/tests/gem_ctx_basic.c index 3e9b688..fe770ea 100644
> > --- a/tests/gem_ctx_basic.c
> > +++ b/tests/gem_ctx_basic.c
> > @@ -145,12 +145,10 @@ static void parse(int argc, char *argv[])
> >  	}
> >  }
> >
> > -int main(int argc, char *argv[])
> > +igt_simple_main
> >  {
> >  	int i;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any_render();
> >  	devid = intel_get_drm_devid(fd);
> >
> > @@ -173,6 +171,4 @@ int main(int argc, char *argv[])
> >
> >  	free(threads);
> >  	close(fd);
> > -
> > -	return 0;
> >  }
> > diff --git a/tests/gem_exec_blt.c b/tests/gem_exec_blt.c index
> > 3bcef18..3d092fe 100644
> > --- a/tests/gem_exec_blt.c
> > +++ b/tests/gem_exec_blt.c
> > @@ -253,12 +253,10 @@ static void run(int object_size)
> >  	close(fd);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	int i;
> >
> > -	igt_simple_init();
> > -
> >  	igt_skip_on_simulation();
> >
> >  	if (argc > 1) {
> > @@ -270,5 +268,4 @@ int main(int argc, char **argv)
> >  	} else
> >  		run(OBJECT_SIZE);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c index
> > 385eeb7..fa20de0 100644
> > --- a/tests/gem_gtt_speed.c
> > +++ b/tests/gem_gtt_speed.c
> > @@ -50,7 +50,7 @@ static double elapsed(const struct timeval *start,
> >  	return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec -
> > start->tv_usec))/loop;  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	struct timeval start, end;
> >  	uint8_t *buf;
> > @@ -59,8 +59,6 @@ int main(int argc, char **argv)
> >  	int loop, i, tiling;
> >  	int fd;
> >
> > -	igt_simple_init();
> > -
> >  	igt_skip_on_simulation();
> >
> >  	if (argc > 1)
> > @@ -329,5 +327,4 @@ int main(int argc, char **argv)
> >  	gem_close(fd, handle);
> >  	close(fd);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gem_hang.c b/tests/gem_hang.c index
> > 6248244..a4f4d10 100644
> > --- a/tests/gem_hang.c
> > +++ b/tests/gem_hang.c
> > @@ -68,12 +68,10 @@ gpu_hang(void)
> >  	intel_batchbuffer_flush(batch);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	int fd;
> >
> > -	igt_simple_init();
> > -
> >  	igt_assert_f(argc == 2,
> >  		     "usage: %s <disabled pipe number>\n",
> >  		     argv[0]);
> > @@ -93,5 +91,4 @@ int main(int argc, char **argv)
> >
> >  	close(fd);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gem_render_copy.c b/tests/gem_render_copy.c index
> > fd26b43..12dd90d 100644
> > --- a/tests/gem_render_copy.c
> > +++ b/tests/gem_render_copy.c
> > @@ -117,7 +117,7 @@ scratch_buf_check(data_t *data, struct igt_buf
> *buf, int x, int y,
> >  		     color, val, x, y);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	data_t data = {0, };
> >  	struct intel_batchbuffer *batch = NULL; @@ -127,7 +127,6 @@ int
> > main(int argc, char **argv)
> >  	int opt_dump_png = false;
> >  	int opt_dump_aub = igt_aub_dump_enabled();
> >
> > -	igt_simple_init();
> >
> >  	while ((opt = getopt(argc, argv, "d")) != -1) {
> >  		switch (opt) {
> > @@ -189,5 +188,4 @@ int main(int argc, char **argv)
> >  		scratch_buf_check(&data, &dst, WIDTH - 10, HEIGHT - 10,
> SRC_COLOR);
> >  	}
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gem_render_linear_blits.c
> > b/tests/gem_render_linear_blits.c index f847486..7c6081e 100644
> > --- a/tests/gem_render_linear_blits.c
> > +++ b/tests/gem_render_linear_blits.c
> > @@ -81,7 +81,7 @@ check_bo(int fd, uint32_t handle, uint32_t val)
> >  	}
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	drm_intel_bufmgr *bufmgr;
> >  	struct intel_batchbuffer *batch;
> > @@ -90,8 +90,6 @@ int main(int argc, char **argv)
> >  	uint32_t start = 0;
> >  	int i, j, fd, count;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any();
> >
> >  	render_copy = igt_get_render_copyfunc(intel_get_drm_devid(fd));
> > @@ -201,5 +199,4 @@ int main(int argc, char **argv)
> >  	for (i = 0; i < count; i++)
> >  		check_bo(fd, bo[i]->handle, start_val[i]);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gem_render_tiled_blits.c
> > b/tests/gem_render_tiled_blits.c index f63c57e..ec8e8de 100644
> > --- a/tests/gem_render_tiled_blits.c
> > +++ b/tests/gem_render_tiled_blits.c
> > @@ -95,7 +95,7 @@ check_bo(struct intel_batchbuffer *batch, struct
> igt_buf *buf, uint32_t val)
> >  		dri_bo_unmap(linear);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	drm_intel_bufmgr *bufmgr;
> >  	struct intel_batchbuffer *batch;
> > @@ -105,8 +105,6 @@ int main(int argc, char **argv)
> >  	int i, j, fd, count;
> >  	uint32_t devid;
> >
> > -	igt_simple_init();
> > -
> >  	igt_skip_on_simulation();
> >
> >  	fd = drm_open_any();
> > @@ -212,5 +210,4 @@ int main(int argc, char **argv)
> >  	for (i = 0; i < count; i++)
> >  		check_bo(batch, &buf[i], start_val[i]);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c index
> > beda28b..8a54c2e 100644
> > --- a/tests/gem_seqno_wrap.c
> > +++ b/tests/gem_seqno_wrap.c
> > @@ -533,12 +533,9 @@ static void parse_options(int argc, char **argv)
> >  	}
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	int wcount = 0;
> > -	int r = -1;
> > -
> > -	igt_simple_init();
> >
> >  	parse_options(argc, argv);
> >
> > @@ -563,8 +560,8 @@ int main(int argc, char **argv)
> >
> >  	if (options.rounds == wcount) {
> >  		igt_debug("done %d wraps successfully\n", wcount);
> > -		return 0;
> >  	}
> > -
> > -	return r;
> > +	else {
> > +	    igt_fail(-1);
> > +	}
> >  }
> > diff --git a/tests/gem_stress.c b/tests/gem_stress.c index
> > 2ccb6fc..35ed32f 100644
> > --- a/tests/gem_stress.c
> > +++ b/tests/gem_stress.c
> > @@ -860,13 +860,11 @@ static void check_render_copyfunc(void)  }
> >
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	int i, j;
> >  	unsigned *current_permutation, *tmp_permutation;
> >
> > -	igt_simple_init();
> > -
> >  	drm_fd = drm_open_any();
> >  	devid = intel_get_drm_devid(drm_fd);
> >
> > @@ -925,5 +923,4 @@ int main(int argc, char **argv)
> >
> >  	igt_stop_signal_helper();
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gen3_mixed_blits.c b/tests/gen3_mixed_blits.c index
> > 75d61a5..f0a6b64 100644
> > --- a/tests/gen3_mixed_blits.c
> > +++ b/tests/gen3_mixed_blits.c
> > @@ -457,14 +457,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
> >  	munmap(v, WIDTH*HEIGHT*4);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	uint32_t *handle, *tiling, *start_val;
> >  	uint32_t start = 0;
> >  	int i, fd, count;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any();
> >
> >  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> > @@ -533,5 +531,4 @@ int main(int argc, char **argv)
> >  		check_bo(fd, handle[i], start_val[i]);
> >  	igt_info("done\n");
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gen3_render_linear_blits.c
> > b/tests/gen3_render_linear_blits.c
> > index 7fe368d..60c0d0b 100644
> > --- a/tests/gen3_render_linear_blits.c
> > +++ b/tests/gen3_render_linear_blits.c
> > @@ -325,14 +325,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
> >  	}
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	uint32_t *handle, *start_val;
> >  	uint32_t start = 0;
> >  	int i, fd, count;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any();
> >
> >  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> > @@ -393,5 +391,4 @@ int main(int argc, char **argv)
> >  	for (i = 0; i < count; i++)
> >  		check_bo(fd, handle[i], start_val[i]);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gen3_render_mixed_blits.c
> > b/tests/gen3_render_mixed_blits.c index 77ac0e2..68660a3 100644
> > --- a/tests/gen3_render_mixed_blits.c
> > +++ b/tests/gen3_render_mixed_blits.c
> > @@ -345,14 +345,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
> >  	munmap(v, WIDTH*HEIGHT*4);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	uint32_t *handle, *tiling, *start_val;
> >  	uint32_t start = 0;
> >  	int i, fd, count;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any();
> >
> >  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> > @@ -421,5 +419,4 @@ int main(int argc, char **argv)
> >  		check_bo(fd, handle[i], start_val[i]);
> >  	igt_info("done\n");
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gen3_render_tiledx_blits.c
> > b/tests/gen3_render_tiledx_blits.c
> > index 95c0c96..d54d714 100644
> > --- a/tests/gen3_render_tiledx_blits.c
> > +++ b/tests/gen3_render_tiledx_blits.c
> > @@ -332,14 +332,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
> >  	munmap(v, WIDTH*HEIGHT*4);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	uint32_t *handle, *start_val;
> >  	uint32_t start = 0;
> >  	int i, fd, count;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any();
> >
> >  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> > @@ -400,5 +398,4 @@ int main(int argc, char **argv)
> >  	for (i = 0; i < count; i++)
> >  		check_bo(fd, handle[i], start_val[i]);
> >
> > -	return 0;
> >  }
> > diff --git a/tests/gen3_render_tiledy_blits.c
> > b/tests/gen3_render_tiledy_blits.c
> > index 1b9a419..3ef08c8 100644
> > --- a/tests/gen3_render_tiledy_blits.c
> > +++ b/tests/gen3_render_tiledy_blits.c
> > @@ -332,14 +332,12 @@ check_bo(int fd, uint32_t handle, uint32_t val)
> >  	munmap(v, WIDTH*HEIGHT*4);
> >  }
> >
> > -int main(int argc, char **argv)
> > +igt_simple_main
> >  {
> >  	uint32_t *handle, *start_val;
> >  	uint32_t start = 0;
> >  	int i, fd, count;
> >
> > -	igt_simple_init();
> > -
> >  	fd = drm_open_any();
> >
> >  	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
> > @@ -407,5 +405,4 @@ int main(int argc, char **argv)
> >  		check_bo(fd, handle[i], start_val[i]);
> >  	igt_info("done\n");
> >
> > -	return 0;
> >  }
> > --
> > 1.9.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Daniel Vetter July 9, 2014, 2:20 p.m. UTC | #4
On Wed, Jul 09, 2014 at 01:50:30PM +0000, Gore, Tim wrote:
> Some inline comments
> 
>   Tim
> 
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Monday, July 07, 2014 5:12 PM
> > To: Gore, Tim
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 1/2] intel-gpu-tools: pass argc/argv to simple
> > main
> > 
> > On Fri, Jun 27, 2014 at 03:15:36PM +0100, tim.gore@intel.com wrote:
> > > From: Tim Gore <tim.gore@intel.com>
> > >
> > > Quite a few single tests do not use the igt_simple_main macro because
> > > they want access to argc/argv. So change the igt_simple_main macro to
> > > pass these arguments through to the "__real_mainxxx" function, and
> > > change these tests to use the macro.
> > >
> > > Signed-off-by: Tim Gore <tim.gore@intel.com>
> > > ---
> > >  lib/igt_core.h                   |  8 ++++----
> 
>  
> > >  tests/gem_ctx_basic.c            |  6 +-----
> > >  tests/gem_exec_blt.c             |  5 +----
> > >  tests/gem_gtt_speed.c            |  5 +----
> > >  tests/gem_hang.c                 |  5 +----
> > >  tests/gem_render_copy.c          |  4 +---
> > >  tests/gem_render_linear_blits.c  |  5 +----
> > >  tests/gem_render_tiled_blits.c   |  5 +----
> > >  tests/gem_seqno_wrap.c           | 11 ++++-------
> > >  tests/gem_stress.c               |  5 +----
> > >  tests/gen3_mixed_blits.c         |  5 +----
> > >  tests/gen3_render_linear_blits.c |  5 +----
> > > tests/gen3_render_mixed_blits.c  |  5 +----
> > > tests/gen3_render_tiledx_blits.c |  5 +----
> > > tests/gen3_render_tiledy_blits.c |  5 +----
> > >  15 files changed, 21 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/lib/igt_core.h b/lib/igt_core.h index e252eba..9853e6b
> > > 100644
> > > --- a/lib/igt_core.h
> > > +++ b/lib/igt_core.h
> > > @@ -176,13 +176,13 @@ void igt_simple_init(void);
> > >   * the test needs to parse additional cmdline arguments of its own.
> > >   */
> > >  #define igt_simple_main \
> > > -	static void igt_tokencat(__real_main, __LINE__)(void); \
> > > +	static void igt_tokencat(__real_main, __LINE__)(int argc, char
> > > +**argv); \
> > >  	int main(int argc, char **argv) { \
> > >  		igt_simple_init(); \
> > > -		igt_tokencat(__real_main, __LINE__)(); \
> > > -		exit(0); \
> > > +		igt_tokencat(__real_main, __LINE__)(argc, argv); \
> > > +		igt_exit(); \
> > >  	} \
> > > -	static void igt_tokencat(__real_main, __LINE__)(void) \
> > > +	static void igt_tokencat(__real_main, __LINE__)(int argc, char
> > > +**argv) \
> > 
> > Hm, I'd just add argc/argv to igt_simple_init like you do in the next patch and
> > update the relevant tests which have their own main. That would be more in
> > line with subtest-tests which have their own additional arguments, too.
> > 
> > Also having functions with magic/predefined arguments is a bit too much
> > magic for my taste. If we keep the signature of real_main as-is we'll avoid
> > (highly unlikely, but still) accidental name clashes.
> > 
> > And with my suggestion for patch 2 to just have a bare-bones argv parsing for
> > simple tests we also don't need to call igt_exit.
> > -Daniel
> 
>     My motivation here is mainly to reduce the amount of differentiation, so that
>     we can move away from having a different methodology for tests that want to
>     parse argv. Together with the next patch we move towards all tests having  the
>     same setup/initialisation. I believe that this will make the test suite easier to
>     maintain and add features to, such as common Doxygen/--help text.
>     I agree that the passing argc/v through to real_main is not ideal, but the whole
>     macro thing is not to my taste to be honest, for all the usual reasons. If we can
>    reduce the number of test "types" (single/multiple , parses argv/or not) it
>     should make the macros easier to maintain etc.

The problem is that simple tests without subtest do work slightly
differently. One option we could pursue instead is to redefine
igt_simple_main as

igt_main
	igt_subtest("basic")

but that will look a bit ugly in the test listenings. My proposal for this
patch here was to add (argc, argv) parameters to igt_simple_init and then
adjust just that function in patch 2. I don't see the upside of forcefully
converting the tests to use the igt_simple_main macro - we also have
subtest-tests with their open-coded main() function.
-Daniel
diff mbox

Patch

diff --git a/lib/igt_core.h b/lib/igt_core.h
index e252eba..9853e6b 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -176,13 +176,13 @@  void igt_simple_init(void);
  * the test needs to parse additional cmdline arguments of its own.
  */
 #define igt_simple_main \
-	static void igt_tokencat(__real_main, __LINE__)(void); \
+	static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv); \
 	int main(int argc, char **argv) { \
 		igt_simple_init(); \
-		igt_tokencat(__real_main, __LINE__)(); \
-		exit(0); \
+		igt_tokencat(__real_main, __LINE__)(argc, argv); \
+		igt_exit(); \
 	} \
-	static void igt_tokencat(__real_main, __LINE__)(void) \
+	static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv) \
 
 __attribute__((format(printf, 1, 2)))
 void igt_skip(const char *f, ...) __attribute__((noreturn));
diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c
index 3e9b688..fe770ea 100644
--- a/tests/gem_ctx_basic.c
+++ b/tests/gem_ctx_basic.c
@@ -145,12 +145,10 @@  static void parse(int argc, char *argv[])
 	}
 }
 
-int main(int argc, char *argv[])
+igt_simple_main
 {
 	int i;
 
-	igt_simple_init();
-
 	fd = drm_open_any_render();
 	devid = intel_get_drm_devid(fd);
 
@@ -173,6 +171,4 @@  int main(int argc, char *argv[])
 
 	free(threads);
 	close(fd);
-
-	return 0;
 }
diff --git a/tests/gem_exec_blt.c b/tests/gem_exec_blt.c
index 3bcef18..3d092fe 100644
--- a/tests/gem_exec_blt.c
+++ b/tests/gem_exec_blt.c
@@ -253,12 +253,10 @@  static void run(int object_size)
 	close(fd);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	int i;
 
-	igt_simple_init();
-
 	igt_skip_on_simulation();
 
 	if (argc > 1) {
@@ -270,5 +268,4 @@  int main(int argc, char **argv)
 	} else
 		run(OBJECT_SIZE);
 
-	return 0;
 }
diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c
index 385eeb7..fa20de0 100644
--- a/tests/gem_gtt_speed.c
+++ b/tests/gem_gtt_speed.c
@@ -50,7 +50,7 @@  static double elapsed(const struct timeval *start,
 	return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - start->tv_usec))/loop;
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	struct timeval start, end;
 	uint8_t *buf;
@@ -59,8 +59,6 @@  int main(int argc, char **argv)
 	int loop, i, tiling;
 	int fd;
 
-	igt_simple_init();
-
 	igt_skip_on_simulation();
 
 	if (argc > 1)
@@ -329,5 +327,4 @@  int main(int argc, char **argv)
 	gem_close(fd, handle);
 	close(fd);
 
-	return 0;
 }
diff --git a/tests/gem_hang.c b/tests/gem_hang.c
index 6248244..a4f4d10 100644
--- a/tests/gem_hang.c
+++ b/tests/gem_hang.c
@@ -68,12 +68,10 @@  gpu_hang(void)
 	intel_batchbuffer_flush(batch);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	int fd;
 
-	igt_simple_init();
-
 	igt_assert_f(argc == 2,
 		     "usage: %s <disabled pipe number>\n",
 		     argv[0]);
@@ -93,5 +91,4 @@  int main(int argc, char **argv)
 
 	close(fd);
 
-	return 0;
 }
diff --git a/tests/gem_render_copy.c b/tests/gem_render_copy.c
index fd26b43..12dd90d 100644
--- a/tests/gem_render_copy.c
+++ b/tests/gem_render_copy.c
@@ -117,7 +117,7 @@  scratch_buf_check(data_t *data, struct igt_buf *buf, int x, int y,
 		     color, val, x, y);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	data_t data = {0, };
 	struct intel_batchbuffer *batch = NULL;
@@ -127,7 +127,6 @@  int main(int argc, char **argv)
 	int opt_dump_png = false;
 	int opt_dump_aub = igt_aub_dump_enabled();
 
-	igt_simple_init();
 
 	while ((opt = getopt(argc, argv, "d")) != -1) {
 		switch (opt) {
@@ -189,5 +188,4 @@  int main(int argc, char **argv)
 		scratch_buf_check(&data, &dst, WIDTH - 10, HEIGHT - 10, SRC_COLOR);
 	}
 
-	return 0;
 }
diff --git a/tests/gem_render_linear_blits.c b/tests/gem_render_linear_blits.c
index f847486..7c6081e 100644
--- a/tests/gem_render_linear_blits.c
+++ b/tests/gem_render_linear_blits.c
@@ -81,7 +81,7 @@  check_bo(int fd, uint32_t handle, uint32_t val)
 	}
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	drm_intel_bufmgr *bufmgr;
 	struct intel_batchbuffer *batch;
@@ -90,8 +90,6 @@  int main(int argc, char **argv)
 	uint32_t start = 0;
 	int i, j, fd, count;
 
-	igt_simple_init();
-
 	fd = drm_open_any();
 
 	render_copy = igt_get_render_copyfunc(intel_get_drm_devid(fd));
@@ -201,5 +199,4 @@  int main(int argc, char **argv)
 	for (i = 0; i < count; i++)
 		check_bo(fd, bo[i]->handle, start_val[i]);
 
-	return 0;
 }
diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c
index f63c57e..ec8e8de 100644
--- a/tests/gem_render_tiled_blits.c
+++ b/tests/gem_render_tiled_blits.c
@@ -95,7 +95,7 @@  check_bo(struct intel_batchbuffer *batch, struct igt_buf *buf, uint32_t val)
 		dri_bo_unmap(linear);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	drm_intel_bufmgr *bufmgr;
 	struct intel_batchbuffer *batch;
@@ -105,8 +105,6 @@  int main(int argc, char **argv)
 	int i, j, fd, count;
 	uint32_t devid;
 
-	igt_simple_init();
-
 	igt_skip_on_simulation();
 
 	fd = drm_open_any();
@@ -212,5 +210,4 @@  int main(int argc, char **argv)
 	for (i = 0; i < count; i++)
 		check_bo(batch, &buf[i], start_val[i]);
 
-	return 0;
 }
diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
index beda28b..8a54c2e 100644
--- a/tests/gem_seqno_wrap.c
+++ b/tests/gem_seqno_wrap.c
@@ -533,12 +533,9 @@  static void parse_options(int argc, char **argv)
 	}
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	int wcount = 0;
-	int r = -1;
-
-	igt_simple_init();
 
 	parse_options(argc, argv);
 
@@ -563,8 +560,8 @@  int main(int argc, char **argv)
 
 	if (options.rounds == wcount) {
 		igt_debug("done %d wraps successfully\n", wcount);
-		return 0;
 	}
-
-	return r;
+	else {
+	    igt_fail(-1);
+	}
 }
diff --git a/tests/gem_stress.c b/tests/gem_stress.c
index 2ccb6fc..35ed32f 100644
--- a/tests/gem_stress.c
+++ b/tests/gem_stress.c
@@ -860,13 +860,11 @@  static void check_render_copyfunc(void)
 }
 
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	int i, j;
 	unsigned *current_permutation, *tmp_permutation;
 
-	igt_simple_init();
-
 	drm_fd = drm_open_any();
 	devid = intel_get_drm_devid(drm_fd);
 
@@ -925,5 +923,4 @@  int main(int argc, char **argv)
 
 	igt_stop_signal_helper();
 
-	return 0;
 }
diff --git a/tests/gen3_mixed_blits.c b/tests/gen3_mixed_blits.c
index 75d61a5..f0a6b64 100644
--- a/tests/gen3_mixed_blits.c
+++ b/tests/gen3_mixed_blits.c
@@ -457,14 +457,12 @@  check_bo(int fd, uint32_t handle, uint32_t val)
 	munmap(v, WIDTH*HEIGHT*4);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	uint32_t *handle, *tiling, *start_val;
 	uint32_t start = 0;
 	int i, fd, count;
 
-	igt_simple_init();
-
 	fd = drm_open_any();
 
 	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
@@ -533,5 +531,4 @@  int main(int argc, char **argv)
 		check_bo(fd, handle[i], start_val[i]);
 	igt_info("done\n");
 
-	return 0;
 }
diff --git a/tests/gen3_render_linear_blits.c b/tests/gen3_render_linear_blits.c
index 7fe368d..60c0d0b 100644
--- a/tests/gen3_render_linear_blits.c
+++ b/tests/gen3_render_linear_blits.c
@@ -325,14 +325,12 @@  check_bo(int fd, uint32_t handle, uint32_t val)
 	}
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	uint32_t *handle, *start_val;
 	uint32_t start = 0;
 	int i, fd, count;
 
-	igt_simple_init();
-
 	fd = drm_open_any();
 
 	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
@@ -393,5 +391,4 @@  int main(int argc, char **argv)
 	for (i = 0; i < count; i++)
 		check_bo(fd, handle[i], start_val[i]);
 
-	return 0;
 }
diff --git a/tests/gen3_render_mixed_blits.c b/tests/gen3_render_mixed_blits.c
index 77ac0e2..68660a3 100644
--- a/tests/gen3_render_mixed_blits.c
+++ b/tests/gen3_render_mixed_blits.c
@@ -345,14 +345,12 @@  check_bo(int fd, uint32_t handle, uint32_t val)
 	munmap(v, WIDTH*HEIGHT*4);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	uint32_t *handle, *tiling, *start_val;
 	uint32_t start = 0;
 	int i, fd, count;
 
-	igt_simple_init();
-
 	fd = drm_open_any();
 
 	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
@@ -421,5 +419,4 @@  int main(int argc, char **argv)
 		check_bo(fd, handle[i], start_val[i]);
 	igt_info("done\n");
 
-	return 0;
 }
diff --git a/tests/gen3_render_tiledx_blits.c b/tests/gen3_render_tiledx_blits.c
index 95c0c96..d54d714 100644
--- a/tests/gen3_render_tiledx_blits.c
+++ b/tests/gen3_render_tiledx_blits.c
@@ -332,14 +332,12 @@  check_bo(int fd, uint32_t handle, uint32_t val)
 	munmap(v, WIDTH*HEIGHT*4);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	uint32_t *handle, *start_val;
 	uint32_t start = 0;
 	int i, fd, count;
 
-	igt_simple_init();
-
 	fd = drm_open_any();
 
 	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
@@ -400,5 +398,4 @@  int main(int argc, char **argv)
 	for (i = 0; i < count; i++)
 		check_bo(fd, handle[i], start_val[i]);
 
-	return 0;
 }
diff --git a/tests/gen3_render_tiledy_blits.c b/tests/gen3_render_tiledy_blits.c
index 1b9a419..3ef08c8 100644
--- a/tests/gen3_render_tiledy_blits.c
+++ b/tests/gen3_render_tiledy_blits.c
@@ -332,14 +332,12 @@  check_bo(int fd, uint32_t handle, uint32_t val)
 	munmap(v, WIDTH*HEIGHT*4);
 }
 
-int main(int argc, char **argv)
+igt_simple_main
 {
 	uint32_t *handle, *start_val;
 	uint32_t start = 0;
 	int i, fd, count;
 
-	igt_simple_init();
-
 	fd = drm_open_any();
 
 	igt_require(IS_GEN3(intel_get_drm_devid(fd)));
@@ -407,5 +405,4 @@  int main(int argc, char **argv)
 		check_bo(fd, handle[i], start_val[i]);
 	igt_info("done\n");
 
-	return 0;
 }