Re: [PATCH i-g-t 6/8] tests: convert simple tests to use igt_simple_init_parse_opts

Gore, Tim
2014-07-24T10:17:22Z

There is a problem here with the -r option in gem_seqno_wrap, as this conflicts
with the --run-subtest option in common_init. This is a general issue with this
method of splitting the responsibility for parsing the command line. As the
common command line options defined in common_init (--run-subtest etc)
do not support short options, a simple solution might be to use some non
printing characters for their getopt return codes, eg

{"list-subtests", 0, 0, '\x0c'}, ....

This way, individual tests are unlikely to clash with these. (I chose '\x0c' since
'l' is the twelfth character ?) I tried this locally and it seemed to work.

Tim

> -----Original Message-----
> From: Thomas Wood [mailto:thomas.wood@intel.com]
> Sent: Wednesday, July 23, 2014 11:58 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Gore, Tim; daniel.vetter@ffwll.ch
> Subject: [PATCH i-g-t 6/8] tests: convert simple tests to use
> igt_simple_init_parse_opts
>
> Convert simple tests to use igt_simple_init_parse_opts if they require extra
> options.
>
> Signed-off-by: Thomas Wood <thomas.wood@intel.com>
> ---
> lib/igt_core.h | 1 +
> tests/gem_ctx_basic.c | 20 +++------
> tests/gem_render_copy.c | 25 +++++------- tests/gem_seqno_wrap.c | 82
> +++++++++++++++----------------------
> tests/gem_stress.c | 105 ++++++++++++++++++++++------------------------
> --
> 5 files changed, 98 insertions(+), 135 deletions(-)
>
> diff --git a/lib/igt_core.h b/lib/igt_core.h index 408cf3a..b19a897 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -36,6 +36,7 @@
> #include <string.h>
> #include <sys/types.h>
> #include <stdarg.h>
> +#include <getopt.h>
>
> /**
> * IGT_EXIT_TIMEOUT:
> diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c index
> a2464fd..a0bec60 100644
> --- a/tests/gem_ctx_basic.c
> +++ b/tests/gem_ctx_basic.c
> @@ -39,7 +39,6 @@
> #include <errno.h>
> #include <sys/stat.h>
> #include <sys/time.h>
> -#include <getopt.h>
> #include "drm.h"
> #include "ioctl_wrappers.h"
> #include "drmtest.h"
> @@ -119,11 +118,9 @@ static void *work(void *arg)
> pthread_exit(NULL);
> }
>
> -static void parse(int argc, char *argv[])
> +static int opt_handler(int opt, int opt_index)
> {
> - int opt;
> - while ((opt = getopt(argc, argv, "i:c:n:muh?")) != -1) {
> - switch (opt) {
> + switch (opt) {
> case 'i':
> iter = atoi(optarg);
> break;
> @@ -136,20 +133,17 @@ static void parse(int argc, char *argv[])
> case 'u':
> uncontexted = 1;
> break;
> - case 'h':
> - case '?':
> - default:
> - igt_success();
> - break;
> - }
> }
> +
> + return 0;
> }
>
> int main(int argc, char *argv[])
> {
> int i;
>
> - igt_simple_init(argc, argv);
> + igt_simple_init_parse_opts(argc, argv, "i:c:n:mu", NULL, NULL,
> + opt_handler);
>
> fd = drm_open_any_render();
> devid = intel_get_drm_devid(fd);
> @@ -159,8 +153,6 @@ int main(int argc, char *argv[])
> iter = 4;
> }
>
> - parse(argc, argv);
> -
> threads = calloc(num_contexts, sizeof(*threads));
>
> for (i = 0; i < num_contexts; i++)
> diff --git a/tests/gem_render_copy.c b/tests/gem_render_copy.c index
> 76ba40e..6ff0c77 100644
> --- a/tests/gem_render_copy.c
> +++ b/tests/gem_render_copy.c
> @@ -41,7 +41,6 @@
> #include <errno.h>
> #include <sys/stat.h>
> #include <sys/time.h>
> -#include <getopt.h>
>
> #include <drm.h>
>
> @@ -67,6 +66,7 @@ typedef struct {
> drm_intel_bufmgr *bufmgr;
> uint32_t linear[WIDTH * HEIGHT];
> } data_t;
> +static int opt_dump_png = false;
>
> static void scratch_buf_write_to_png(struct igt_buf *buf, const char
> *filename) { @@ -117,27 +117,24 @@ scratch_buf_check(data_t *data,
> struct igt_buf *buf, int x, int y,
> color, val, x, y);
> }
>
> +static int opt_handler(int opt, int opt_index) {
> + if (opt == 'd') {
> + opt_dump_png = true;
> + }
> +
> + return 0;
> +}
> +
> int main(int argc, char **argv)
> {
> data_t data = {0, };
> struct intel_batchbuffer *batch = NULL;
> struct igt_buf src, dst;
> igt_render_copyfunc_t render_copy = NULL;
> - int opt;
> - int opt_dump_png = false;
> int opt_dump_aub = igt_aub_dump_enabled();
>
> - igt_simple_init(argc, argv);
> -
> - while ((opt = getopt(argc, argv, "d")) != -1) {
> - switch (opt) {
> - case 'd':
> - opt_dump_png = true;
> - break;
> - default:
> - break;
> - }
> - }
> + igt_simple_init_parse_opts(argc, argv, "d", NULL, NULL,
> opt_handler);
>
> igt_fixture {
> data.drm_fd = drm_open_any_render();
> diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c index
> 0fa722d..3a40860 100644
> --- a/tests/gem_seqno_wrap.c
> +++ b/tests/gem_seqno_wrap.c
> @@ -38,7 +38,6 @@
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <limits.h>
> -#include <getopt.h>
> #include <signal.h>
> #include <errno.h>
>
> @@ -452,45 +451,9 @@ static void background_run_once(void)
> sleep(3);
> }
>
> -static void print_usage(const char *s)
> +static int parse_options(int opt, int opt_index)
> {
> - igt_info("%s: [OPTION]...\n", s);
> - igt_info(" where options are:\n");
> - igt_info(" -b --background run in background inducing wraps\n");
> - igt_info(" -n --rounds=num run num times across wrap
> boundary, 0 == forever\n");
> - igt_info(" -t --timeout=sec set timeout to wait for testrun to sec
> seconds\n");
> - igt_info(" -d --dontwrap don't wrap just run the test\n");
> - igt_info(" -p --prewrap=n set seqno to WRAP - n for each
> testrun\n");
> - igt_info(" -r --norandom dont randomize prewrap space\n");
> - igt_info(" -i --buffers number of buffers to copy\n");
> - igt_fail(-1);
> -}
> -
> -static void parse_options(int argc, char **argv) -{
> - int c;
> - int option_index = 0;
> - static struct option long_options[] = {
> - {"rounds", required_argument, 0, 'n'},
> - {"background", no_argument, 0, 'b'},
> - {"timeout", required_argument, 0, 't'},
> - {"dontwrap", no_argument, 0, 'd'},
> - {"prewrap", required_argument, 0, 'p'},
> - {"norandom", no_argument, 0, 'r'},
> - {"buffers", required_argument, 0, 'i'},
> - };
> -
> - options.rounds = SLOW_QUICK(50, 2);
> - options.background = 0;
> - options.dontwrap = 0;
> - options.timeout = 20;
> - options.random = 1;
> - options.prewrap_space = 21;
> - options.buffers = 10;
> -
> - while((c = getopt_long(argc, argv, "n:bvt:dp:ri:",
> - long_options, &option_index)) != -1) {
> - switch(c) {
> + switch(opt) {
> case 'b':
> options.background = 1;
> igt_info("running in background inducing wraps\n");
> @@ -520,17 +483,9 @@ static void parse_options(int argc, char **argv)
> options.prewrap_space = atoi(optarg);
> igt_info("prewrap set to %d (0x%x)\n",
> options.prewrap_space, UINT32_MAX - options.prewrap_space);
> break;
> - default:
> - igt_info("unkown command options\n");
> - print_usage(argv[0]);
> - break;
> - }
> }
>
> - if (optind < argc) {
> - igt_info("unkown command options\n");
> - print_usage(argv[0]);
> - }
> + return 0;
> }
>
> int main(int argc, char **argv)
> @@ -538,9 +493,36 @@ int main(int argc, char **argv)
> int wcount = 0;
> int r = -1;
>
> - igt_simple_init(argc, argv);
> + static struct option long_options[] = {
> + {"rounds", required_argument, 0, 'n'},
> + {"background", no_argument, 0, 'b'},
> + {"timeout", required_argument, 0, 't'},
> + {"dontwrap", no_argument, 0, 'd'},
> + {"prewrap", required_argument, 0, 'p'},
> + {"norandom", no_argument, 0, 'r'},
> + {"buffers", required_argument, 0, 'i'},
> + { 0, 0, 0, 0 }
> + };
> +
> + const char *help =
> + " -b --background run in background inducing wraps\n"
> + " -n --rounds=num run num times across wrap boundary,
> 0 == forever\n"
> + " -t --timeout=sec set timeout to wait for testrun to sec
> seconds\n"
> + " -d --dontwrap don't wrap just run the test\n"
> + " -p --prewrap=n set seqno to WRAP - n for each
> testrun\n"
> + " -r --norandom dont randomize prewrap space\n"
> + " -i --buffers number of buffers to copy\n";
> +
> + options.rounds = SLOW_QUICK(50, 2);
> + options.background = 0;
> + options.dontwrap = 0;
> + options.timeout = 20;
> + options.random = 1;
> + options.prewrap_space = 21;
> + options.buffers = 10;
>
> - parse_options(argc, argv);
> + igt_simple_init_parse_opts(argc, argv, "n:bvt:dp:ri:", long_options,
> + help, parse_options);
>
> card_index = drm_get_card();
>
> diff --git a/tests/gem_stress.c b/tests/gem_stress.c index c8d7393..3bbe487
> 100644
> --- a/tests/gem_stress.c
> +++ b/tests/gem_stress.c
> @@ -58,7 +58,6 @@
> #include <errno.h>
> #include <sys/stat.h>
> #include <sys/time.h>
> -#include <getopt.h>
>
> #include <drm.h>
>
> @@ -72,6 +71,10 @@
>
> #define CMD_POLY_STIPPLE_OFFSET 0x7906
>
> +#define DUCTAPE 0xdead0001
> +#define TILESZ 0xdead0002
> +#define CHCK_RENDER 0xdead0003
> +
> /** TODO:
> * - beat on relaxed fencing (i.e. mappable/fenceable tracking in the kernel)
> * - render copy (to check fence tracking and cache coherency management
> by the @@ -624,54 +627,11 @@ static void sanitize_tiles_per_buf(void)
> options.tiles_per_buf = options.scratch_buf_size /
> TILE_BYTES(options.tile_size); }
>
> -static void parse_options(int argc, char **argv)
> +static int parse_options(int opt, int opt_index)
> {
> - int c, tmp;
> - int option_index = 0;
> - static struct option long_options[] = {
> - {"no-hw", 0, 0, 'd'},
> - {"buf-size", 1, 0, 's'},
> - {"gpu-busy-load", 1, 0, 'g'},
> - {"no-signals", 0, 0, 'S'},
> - {"buffer-count", 1, 0, 'c'},
> - {"trace-tile", 1, 0, 't'},
> - {"disable-blt", 0, 0, 'b'},
> - {"disable-render", 0, 0, 'r'},
> - {"untiled", 0, 0, 'u'},
> - {"x-tiled", 0, 0, 'x'},
> - {"use-cpu-maps", 0, 0, 'm'},
> - {"rounds", 1, 0, 'o'},
> - {"no-fail", 0, 0, 'f'},
> - {"tiles-per-buf", 0, 0, 'p'},
> -#define DUCTAPE 0xdead0001
> - {"remove-duct-tape", 0, 0, DUCTAPE},
> -#define TILESZ 0xdead0002
> - {"tile-size", 1, 0, TILESZ},
> -#define CHCK_RENDER 0xdead0003
> - {"check-render-cpyfn", 0, 0, CHCK_RENDER},
> - {NULL, 0, 0, 0},
> - };
> -
> - options.scratch_buf_size = 256*4096;
> - options.no_hw = 0;
> - options.use_signal_helper = 1;
> - options.gpu_busy_load = 0;
> - options.num_buffers = 0;
> - options.trace_tile = -1;
> - options.use_render = 1;
> - options.use_blt = 1;
> - options.forced_tiling = -1;
> - options.use_cpu_maps = 0;
> - options.total_rounds = 512;
> - options.fail = 1;
> - options.ducttape = 1;
> - options.tile_size = 16;
> - options.tiles_per_buf = options.scratch_buf_size /
> TILE_BYTES(options.tile_size);
> - options.check_render_cpyfn = 0;
> + int tmp;
>
> - while((c = getopt_long(argc, argv, "ds:g:c:t:rbuxmo:fp:",
> - long_options, &option_index)) != -1) {
> - switch(c) {
> + switch(opt) {
> case 'd':
> options.no_hw = 1;
> igt_info("no-hw debug mode\n");
> @@ -759,15 +719,8 @@ static void parse_options(int argc, char **argv)
> options.check_render_cpyfn = 1;
> igt_info("checking render copy function\n");
> break;
> - default:
> - igt_info("unkown command options\n");
> - break;
> - }
> }
>
> - if (optind < argc)
> - igt_info("unkown command options\n");
> -
> /* actually 32767, according to docs, but that kills our nice pot
> calculations. */
> options.max_dimension = 16*1024;
> if (options.use_render) {
> @@ -777,6 +730,8 @@ static void parse_options(int argc, char **argv)
> options.max_dimension = 8192;
> }
> igt_info("Limiting buffer to %dx%d\n", options.max_dimension,
> options.max_dimension);
> +
> + return 0;
> }
>
> static void init(void)
> @@ -864,14 +819,50 @@ int main(int argc, char **argv) {
> int i, j;
> unsigned *current_permutation, *tmp_permutation;
> + static struct option long_options[] = {
> + {"no-hw", 0, 0, 'd'},
> + {"buf-size", 1, 0, 's'},
> + {"gpu-busy-load", 1, 0, 'g'},
> + {"no-signals", 0, 0, 'S'},
> + {"buffer-count", 1, 0, 'c'},
> + {"trace-tile", 1, 0, 't'},
> + {"disable-blt", 0, 0, 'b'},
> + {"disable-render", 0, 0, 'r'},
> + {"untiled", 0, 0, 'u'},
> + {"x-tiled", 0, 0, 'x'},
> + {"use-cpu-maps", 0, 0, 'm'},
> + {"rounds", 1, 0, 'o'},
> + {"no-fail", 0, 0, 'f'},
> + {"tiles-per-buf", 0, 0, 'p'},
> + {"remove-duct-tape", 0, 0, DUCTAPE},
> + {"tile-size", 1, 0, TILESZ},
> + {"check-render-cpyfn", 0, 0, CHCK_RENDER},
> + {NULL, 0, 0, 0},
> + };
> +
> + options.scratch_buf_size = 256*4096;
> + options.no_hw = 0;
> + options.use_signal_helper = 1;
> + options.gpu_busy_load = 0;
> + options.num_buffers = 0;
> + options.trace_tile = -1;
> + options.use_render = 1;
> + options.use_blt = 1;
> + options.forced_tiling = -1;
> + options.use_cpu_maps = 0;
> + options.total_rounds = 512;
> + options.fail = 1;
> + options.ducttape = 1;
> + options.tile_size = 16;
> + options.tiles_per_buf = options.scratch_buf_size /
> TILE_BYTES(options.tile_size);
> + options.check_render_cpyfn = 0;
>
> - igt_simple_init(argc, argv);
> + igt_simple_init_parse_opts(argc, argv,"ds:g:c:t:rbuxmo:fp:",
> + long_options, NULL, parse_options);
>
> drm_fd = drm_open_any();
> devid = intel_get_drm_devid(drm_fd);
>
> - parse_options(argc, argv);
> -
> /* start our little helper early before too may allocations occur */
> if (options.use_signal_helper)
> igt_fork_signal_helper();
> --
> 1.9.3