[Intel-gfx] [PATCH 1/2] intel-gpu-tools: pass argc/argv to simple main
Gore, Tim
tim.gore at intel.com
Wed Jul 9 15:50:30 CEST 2014
Some inline comments
Tim
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, July 07, 2014 5:12 PM
> To: Gore, Tim
> Cc: intel-gfx at 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 at intel.com wrote:
> > From: Tim Gore <tim.gore at 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 at 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 at 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
More information about the Intel-gfx
mailing list