[crucible] add a command line option for selecting the Vulkan device

Submitted by Samuel Pitoiset on April 5, 2019, 10:27 a.m.

Details

Message ID 20190405102714.4492-1-samuel.pitoiset@gmail.com
State Accepted
Headers show
Series "add a command line option for selecting the Vulkan device" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset April 5, 2019, 10:27 a.m.
In a multi-GPU scenario.

Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 doc/crucible-run.1.txt             |  4 ++++
 include/framework/runner/runner.h  |  2 ++
 include/framework/test/test.h      |  1 +
 misc/crucible-completion.bash      |  1 +
 src/cmd/bootstrap.c                |  1 +
 src/cmd/run.c                      | 12 +++++++++++-
 src/framework/runner/runner.c      |  1 +
 src/framework/test/t_phase_setup.c | 13 ++++++++-----
 src/framework/test/test.c          |  1 +
 src/framework/test/test.h          |  3 +++
 10 files changed, 33 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/doc/crucible-run.1.txt b/doc/crucible-run.1.txt
index 59aecd1..195da2c 100644
--- a/doc/crucible-run.1.txt
+++ b/doc/crucible-run.1.txt
@@ -13,6 +13,7 @@  SYNOPSIS
                [--jobs=<jobs> | -j <jobs>] [--[no-]separate-cleanup-threads]
                [--use-spir-v|--no-spir-v] [--isolation=<method> | -I <method>]
                [--junit-xml=<junit-xml-file>]
+               [--device-id=<device-id>]
                [<pattern>...]
 
 DESCRIPTION
@@ -83,6 +84,9 @@  OPTIONS
 --junit-xml=<junit-xml-file>::
     Write JUnit XML to the given file.
 
+--device-id=<device-id>::
+    Select the Vulkan device ID (IDs start from 1).
+
 EXAMPLES
 --------
 * Run all functional tests and stress tests. The following invocations are all
diff --git a/include/framework/runner/runner.h b/include/framework/runner/runner.h
index 01893ef..3015850 100644
--- a/include/framework/runner/runner.h
+++ b/include/framework/runner/runner.h
@@ -51,6 +51,8 @@  struct runner_opts {
 
     /// The runner will write JUnit XML to this path, if not NULL.
     const char *junit_xml_filepath;
+
+    int device_id;
 };
 
 bool runner_init(runner_opts_t *opts);
diff --git a/include/framework/test/test.h b/include/framework/test/test.h
index ca328bb..fc609cb 100644
--- a/include/framework/test/test.h
+++ b/include/framework/test/test.h
@@ -34,6 +34,7 @@  struct test_create_info {
     bool enable_separate_cleanup_thread;
     bool enable_spir_v;
     bool enable_bootstrap;
+    int device_id;
     uint32_t queue_family_index;
 
     uint32_t bootstrap_image_width;
diff --git a/misc/crucible-completion.bash b/misc/crucible-completion.bash
index 8942b14..4377224 100644
--- a/misc/crucible-completion.bash
+++ b/misc/crucible-completion.bash
@@ -35,6 +35,7 @@  __crucible_run()
       --no-cleanup
       --use-spir-v
       --junit-xml
+      --device-id
    "
 
    COMPREPLY=($(compgen -W "$flags $($1 ls-tests)" -- ${COMP_WORDS[COMP_CWORD]}))
diff --git a/src/cmd/bootstrap.c b/src/cmd/bootstrap.c
index bfa2221..f902fe1 100644
--- a/src/cmd/bootstrap.c
+++ b/src/cmd/bootstrap.c
@@ -136,6 +136,7 @@  start(const cru_command_t *cmd, int argc, char **argv)
                        .enable_bootstrap = true,
                        .enable_cleanup_phase = false,
                        .enable_spir_v = true,
+                       .device_id = 1,
                        .bootstrap_image_width = opt_image_width,
                        .bootstrap_image_height = opt_image_height);
     if (!test) {
diff --git a/src/cmd/run.c b/src/cmd/run.c
index 3f06075..078b0db 100644
--- a/src/cmd/run.c
+++ b/src/cmd/run.c
@@ -39,6 +39,7 @@  static int opt_dump = 0;
 static int opt_use_spir_v = 1;
 static int opt_separate_cleanup_thread = 1;
 static char *opt_junit_xml = NULL;
+static int opt_device_id = 1;
 
 // From man:getopt(3) :
 //
@@ -52,12 +53,13 @@  static char *opt_junit_xml = NULL;
 //    above) of optstring is a colon (':'),  then getopt() returns ':' instead
 //    of '?' to indicate a missing option argument.
 //
-static const char *shortopts = "+:hj:I:";
+static const char *shortopts = "+:hj:I:d:";
 
 enum opt_name {
     OPT_NAME_HELP = 'h',
     OPT_NAME_JOBS = 'j',
     OPT_NAME_ISLOATION = 'I',
+    OPT_NAME_DEVICE_ID = 'd',
 
     // Begin long-only options. They begin with the first char value outside
     // the ASCII range.
@@ -77,6 +79,7 @@  static const struct option longopts[] = {
     {"use-spir-v",    no_argument,       &opt_use_spir_v, true},
     {"no-spir-v",     no_argument,       &opt_use_spir_v, false},
     {"junit-xml",     required_argument, NULL,            OPT_NAME_JUNIT_XML},
+    {"device-id",     required_argument, NULL,            OPT_NAME_DEVICE_ID},
 
     {"separate-cleanup-threads",    no_argument, &opt_separate_cleanup_thread, true},
     {"no-separate-cleanup-threads", no_argument, &opt_separate_cleanup_thread, false},
@@ -151,6 +154,12 @@  parse_args(const cru_command_t *cmd, int argc, char **argv)
         case OPT_NAME_JUNIT_XML:
             opt_junit_xml = strdup(optarg);
             break;
+        case OPT_NAME_DEVICE_ID:
+            opt_device_id = strtol(optarg, NULL, 10);
+            if (opt_device_id <= 0) {
+                cru_usage_error(cmd, "--device must be at least 1");
+            }
+            break;
         case ':':
             cru_usage_error(cmd, "%s requires an argument", argv[optind-1]);
             break;
@@ -265,6 +274,7 @@  cmd_start(const cru_command_t *cmd, int argc, char **argv)
         .no_image_dumps = !opt_dump,
         .use_spir_v = opt_use_spir_v,
         .junit_xml_filepath = opt_junit_xml,
+        .device_id = opt_device_id,
     });
 
     if (opt_log_pids)
diff --git a/src/framework/runner/runner.c b/src/framework/runner/runner.c
index aaff68f..eedc2b1 100644
--- a/src/framework/runner/runner.c
+++ b/src/framework/runner/runner.c
@@ -101,6 +101,7 @@  run_test_def(const test_def_t *def, uint32_t queue_family_index)
                        .enable_spir_v = runner_opts.use_spir_v,
                        .enable_separate_cleanup_thread =
                             runner_opts.use_separate_cleanup_threads,
+                       .device_id = runner_opts.device_id,
                        .queue_family_index = queue_family_index);
     if (!test)
         return TEST_RESULT_FAIL;
diff --git a/src/framework/test/t_phase_setup.c b/src/framework/test/t_phase_setup.c
index a92a5d7..4c5cdce 100644
--- a/src/framework/test/t_phase_setup.c
+++ b/src/framework/test/t_phase_setup.c
@@ -24,6 +24,9 @@ 
 #include "test.h"
 #include "t_phase_setup.h"
 
+/* Maximum supported physical devs. */
+#define MAX_PHYSICAL_DEVS 4
+
 static void *
 test_vk_alloc(void *pUserData, size_t size, size_t alignment,
               VkSystemAllocationScope scope)
@@ -70,16 +73,16 @@  t_setup_phys_dev(void)
     ASSERT_TEST_IN_SETUP_PHASE;
     GET_CURRENT_TEST(t);
 
-    // Crucible uses only the first physical device.
-    // FINISHME: Add a command-line option to use non-default physical device.
+    VkPhysicalDevice physical_devs[MAX_PHYSICAL_DEVS];
 
     uint32_t count = 0;
     qoEnumeratePhysicalDevices(t->vk.instance, &count, NULL);
     t_assertf(count > 0, "failed to enumerate any physical devices");
+    t_assertf(count <= MAX_PHYSICAL_DEVS, "reached the maximum supported physical devices");
+    t_assertf(t->opt.device_id <= count, "requested device id not found");
 
-    count = 1;
-    qoEnumeratePhysicalDevices(t->vk.instance, &count, &t->vk.physical_dev);
-    t_assertf(count == 1, "enumerated %u physical devices, expected 1", count);
+    qoEnumeratePhysicalDevices(t->vk.instance, &count, physical_devs);
+    t->vk.physical_dev = physical_devs[t->opt.device_id - 1];
 
     qoGetPhysicalDeviceProperties(t->vk.physical_dev, &t->vk.physical_dev_props);
 }
diff --git a/src/framework/test/test.c b/src/framework/test/test.c
index 80281b9..85e6310 100644
--- a/src/framework/test/test.c
+++ b/src/framework/test/test.c
@@ -152,6 +152,7 @@  test_create_s(const test_create_info_t *info)
     t->opt.use_spir_v = info->enable_spir_v;
     t->opt.bootstrap = info->enable_bootstrap;
     t->opt.queue_family_index = info->queue_family_index;
+    t->opt.device_id = info->device_id;
 
     if (info->enable_bootstrap) {
         if (info->enable_cleanup_phase) {
diff --git a/src/framework/test/test.h b/src/framework/test/test.h
index cafcc7e..4fafcc2 100644
--- a/src/framework/test/test.h
+++ b/src/framework/test/test.h
@@ -121,6 +121,9 @@  struct test {
         /// thread.
         bool no_separate_cleanup_thread;
 
+        /// The Vulkan device ID.
+        int device_id;
+
         uint32_t queue_family_index;
     } opt;
 

Comments

On Friday, 2019-04-05 12:27:14 +0200, Samuel Pitoiset wrote:
> In a multi-GPU scenario.
> 
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>

FYI, crucible now uses merge requests on gitlab :)
https://gitlab.freedesktop.org/mesa/crucible/merge_requests

> ---
>  doc/crucible-run.1.txt             |  4 ++++
>  include/framework/runner/runner.h  |  2 ++
>  include/framework/test/test.h      |  1 +
>  misc/crucible-completion.bash      |  1 +
>  src/cmd/bootstrap.c                |  1 +
>  src/cmd/run.c                      | 12 +++++++++++-
>  src/framework/runner/runner.c      |  1 +
>  src/framework/test/t_phase_setup.c | 13 ++++++++-----
>  src/framework/test/test.c          |  1 +
>  src/framework/test/test.h          |  3 +++
>  10 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/crucible-run.1.txt b/doc/crucible-run.1.txt
> index 59aecd1..195da2c 100644
> --- a/doc/crucible-run.1.txt
> +++ b/doc/crucible-run.1.txt
> @@ -13,6 +13,7 @@ SYNOPSIS
>                 [--jobs=<jobs> | -j <jobs>] [--[no-]separate-cleanup-threads]
>                 [--use-spir-v|--no-spir-v] [--isolation=<method> | -I <method>]
>                 [--junit-xml=<junit-xml-file>]
> +               [--device-id=<device-id>]
>                 [<pattern>...]
>  
>  DESCRIPTION
> @@ -83,6 +84,9 @@ OPTIONS
>  --junit-xml=<junit-xml-file>::
>      Write JUnit XML to the given file.
>  
> +--device-id=<device-id>::
> +    Select the Vulkan device ID (IDs start from 1).

Maybe s/id/index/, since that's _not_ the device's deviceID?
More on that below.

> +
>  EXAMPLES
>  --------
>  * Run all functional tests and stress tests. The following invocations are all
> diff --git a/include/framework/runner/runner.h b/include/framework/runner/runner.h
> index 01893ef..3015850 100644
> --- a/include/framework/runner/runner.h
> +++ b/include/framework/runner/runner.h
> @@ -51,6 +51,8 @@ struct runner_opts {
>  
>      /// The runner will write JUnit XML to this path, if not NULL.
>      const char *junit_xml_filepath;
> +
> +    int device_id;
>  };
>  
>  bool runner_init(runner_opts_t *opts);
> diff --git a/include/framework/test/test.h b/include/framework/test/test.h
> index ca328bb..fc609cb 100644
> --- a/include/framework/test/test.h
> +++ b/include/framework/test/test.h
> @@ -34,6 +34,7 @@ struct test_create_info {
>      bool enable_separate_cleanup_thread;
>      bool enable_spir_v;
>      bool enable_bootstrap;
> +    int device_id;
>      uint32_t queue_family_index;
>  
>      uint32_t bootstrap_image_width;
> diff --git a/misc/crucible-completion.bash b/misc/crucible-completion.bash
> index 8942b14..4377224 100644
> --- a/misc/crucible-completion.bash
> +++ b/misc/crucible-completion.bash
> @@ -35,6 +35,7 @@ __crucible_run()
>        --no-cleanup
>        --use-spir-v
>        --junit-xml
> +      --device-id
>     "
>  
>     COMPREPLY=($(compgen -W "$flags $($1 ls-tests)" -- ${COMP_WORDS[COMP_CWORD]}))
> diff --git a/src/cmd/bootstrap.c b/src/cmd/bootstrap.c
> index bfa2221..f902fe1 100644
> --- a/src/cmd/bootstrap.c
> +++ b/src/cmd/bootstrap.c
> @@ -136,6 +136,7 @@ start(const cru_command_t *cmd, int argc, char **argv)
>                         .enable_bootstrap = true,
>                         .enable_cleanup_phase = false,
>                         .enable_spir_v = true,
> +                       .device_id = 1,
>                         .bootstrap_image_width = opt_image_width,
>                         .bootstrap_image_height = opt_image_height);
>      if (!test) {
> diff --git a/src/cmd/run.c b/src/cmd/run.c
> index 3f06075..078b0db 100644
> --- a/src/cmd/run.c
> +++ b/src/cmd/run.c
> @@ -39,6 +39,7 @@ static int opt_dump = 0;
>  static int opt_use_spir_v = 1;
>  static int opt_separate_cleanup_thread = 1;
>  static char *opt_junit_xml = NULL;
> +static int opt_device_id = 1;
>  
>  // From man:getopt(3) :
>  //
> @@ -52,12 +53,13 @@ static char *opt_junit_xml = NULL;
>  //    above) of optstring is a colon (':'),  then getopt() returns ':' instead
>  //    of '?' to indicate a missing option argument.
>  //
> -static const char *shortopts = "+:hj:I:";
> +static const char *shortopts = "+:hj:I:d:";
>  
>  enum opt_name {
>      OPT_NAME_HELP = 'h',
>      OPT_NAME_JOBS = 'j',
>      OPT_NAME_ISLOATION = 'I',
> +    OPT_NAME_DEVICE_ID = 'd',
>  
>      // Begin long-only options. They begin with the first char value outside
>      // the ASCII range.
> @@ -77,6 +79,7 @@ static const struct option longopts[] = {
>      {"use-spir-v",    no_argument,       &opt_use_spir_v, true},
>      {"no-spir-v",     no_argument,       &opt_use_spir_v, false},
>      {"junit-xml",     required_argument, NULL,            OPT_NAME_JUNIT_XML},
> +    {"device-id",     required_argument, NULL,            OPT_NAME_DEVICE_ID},
>  
>      {"separate-cleanup-threads",    no_argument, &opt_separate_cleanup_thread, true},
>      {"no-separate-cleanup-threads", no_argument, &opt_separate_cleanup_thread, false},
> @@ -151,6 +154,12 @@ parse_args(const cru_command_t *cmd, int argc, char **argv)
>          case OPT_NAME_JUNIT_XML:
>              opt_junit_xml = strdup(optarg);
>              break;
> +        case OPT_NAME_DEVICE_ID:
> +            opt_device_id = strtol(optarg, NULL, 10);
> +            if (opt_device_id <= 0) {
> +                cru_usage_error(cmd, "--device must be at least 1");
> +            }
> +            break;
>          case ':':
>              cru_usage_error(cmd, "%s requires an argument", argv[optind-1]);
>              break;
> @@ -265,6 +274,7 @@ cmd_start(const cru_command_t *cmd, int argc, char **argv)
>          .no_image_dumps = !opt_dump,
>          .use_spir_v = opt_use_spir_v,
>          .junit_xml_filepath = opt_junit_xml,
> +        .device_id = opt_device_id,
>      });
>  
>      if (opt_log_pids)
> diff --git a/src/framework/runner/runner.c b/src/framework/runner/runner.c
> index aaff68f..eedc2b1 100644
> --- a/src/framework/runner/runner.c
> +++ b/src/framework/runner/runner.c
> @@ -101,6 +101,7 @@ run_test_def(const test_def_t *def, uint32_t queue_family_index)
>                         .enable_spir_v = runner_opts.use_spir_v,
>                         .enable_separate_cleanup_thread =
>                              runner_opts.use_separate_cleanup_threads,
> +                       .device_id = runner_opts.device_id,
>                         .queue_family_index = queue_family_index);
>      if (!test)
>          return TEST_RESULT_FAIL;
> diff --git a/src/framework/test/t_phase_setup.c b/src/framework/test/t_phase_setup.c
> index a92a5d7..4c5cdce 100644
> --- a/src/framework/test/t_phase_setup.c
> +++ b/src/framework/test/t_phase_setup.c
> @@ -24,6 +24,9 @@
>  #include "test.h"
>  #include "t_phase_setup.h"
>  
> +/* Maximum supported physical devs. */
> +#define MAX_PHYSICAL_DEVS 4
> +
>  static void *
>  test_vk_alloc(void *pUserData, size_t size, size_t alignment,
>                VkSystemAllocationScope scope)
> @@ -70,16 +73,16 @@ t_setup_phys_dev(void)
>      ASSERT_TEST_IN_SETUP_PHASE;
>      GET_CURRENT_TEST(t);
>  
> -    // Crucible uses only the first physical device.
> -    // FINISHME: Add a command-line option to use non-default physical device.
> +    VkPhysicalDevice physical_devs[MAX_PHYSICAL_DEVS];
>  
>      uint32_t count = 0;
>      qoEnumeratePhysicalDevices(t->vk.instance, &count, NULL);
>      t_assertf(count > 0, "failed to enumerate any physical devices");
> +    t_assertf(count <= MAX_PHYSICAL_DEVS, "reached the maximum supported physical devices");
> +    t_assertf(t->opt.device_id <= count, "requested device id not found");
>  
> -    count = 1;
> -    qoEnumeratePhysicalDevices(t->vk.instance, &count, &t->vk.physical_dev);
> -    t_assertf(count == 1, "enumerated %u physical devices, expected 1", count);
> +    qoEnumeratePhysicalDevices(t->vk.instance, &count, physical_devs);
> +    t->vk.physical_dev = physical_devs[t->opt.device_id - 1];

vkEnumeratePhysicalDevices() has no guarantees of ordering, so this
might be rather fragile.

Perhaps passing in VkPhysicalDeviceProperties::vendorID and
VkPhysicalDeviceProperties::deviceID to crucible and matching devices
based on these two would be a better option?

>  
>      qoGetPhysicalDeviceProperties(t->vk.physical_dev, &t->vk.physical_dev_props);
>  }
> diff --git a/src/framework/test/test.c b/src/framework/test/test.c
> index 80281b9..85e6310 100644
> --- a/src/framework/test/test.c
> +++ b/src/framework/test/test.c
> @@ -152,6 +152,7 @@ test_create_s(const test_create_info_t *info)
>      t->opt.use_spir_v = info->enable_spir_v;
>      t->opt.bootstrap = info->enable_bootstrap;
>      t->opt.queue_family_index = info->queue_family_index;
> +    t->opt.device_id = info->device_id;
>  
>      if (info->enable_bootstrap) {
>          if (info->enable_cleanup_phase) {
> diff --git a/src/framework/test/test.h b/src/framework/test/test.h
> index cafcc7e..4fafcc2 100644
> --- a/src/framework/test/test.h
> +++ b/src/framework/test/test.h
> @@ -121,6 +121,9 @@ struct test {
>          /// thread.
>          bool no_separate_cleanup_thread;
>  
> +        /// The Vulkan device ID.
> +        int device_id;
> +
>          uint32_t queue_family_index;
>      } opt;
>  
> -- 
> 2.21.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 4/5/19 2:59 PM, Eric Engestrom wrote:
> On Friday, 2019-04-05 12:27:14 +0200, Samuel Pitoiset wrote:
>> In a multi-GPU scenario.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> FYI, crucible now uses merge requests on gitlab :)
> https://gitlab.freedesktop.org/mesa/crucible/merge_requests
>
>> ---
>>   doc/crucible-run.1.txt             |  4 ++++
>>   include/framework/runner/runner.h  |  2 ++
>>   include/framework/test/test.h      |  1 +
>>   misc/crucible-completion.bash      |  1 +
>>   src/cmd/bootstrap.c                |  1 +
>>   src/cmd/run.c                      | 12 +++++++++++-
>>   src/framework/runner/runner.c      |  1 +
>>   src/framework/test/t_phase_setup.c | 13 ++++++++-----
>>   src/framework/test/test.c          |  1 +
>>   src/framework/test/test.h          |  3 +++
>>   10 files changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/doc/crucible-run.1.txt b/doc/crucible-run.1.txt
>> index 59aecd1..195da2c 100644
>> --- a/doc/crucible-run.1.txt
>> +++ b/doc/crucible-run.1.txt
>> @@ -13,6 +13,7 @@ SYNOPSIS
>>                  [--jobs=<jobs> | -j <jobs>] [--[no-]separate-cleanup-threads]
>>                  [--use-spir-v|--no-spir-v] [--isolation=<method> | -I <method>]
>>                  [--junit-xml=<junit-xml-file>]
>> +               [--device-id=<device-id>]
>>                  [<pattern>...]
>>   
>>   DESCRIPTION
>> @@ -83,6 +84,9 @@ OPTIONS
>>   --junit-xml=<junit-xml-file>::
>>       Write JUnit XML to the given file.
>>   
>> +--device-id=<device-id>::
>> +    Select the Vulkan device ID (IDs start from 1).
> Maybe s/id/index/, since that's _not_ the device's deviceID?
> More on that below.
VK-GL-CTS calls it device-id too.
>
>> +
>>   EXAMPLES
>>   --------
>>   * Run all functional tests and stress tests. The following invocations are all
>> diff --git a/include/framework/runner/runner.h b/include/framework/runner/runner.h
>> index 01893ef..3015850 100644
>> --- a/include/framework/runner/runner.h
>> +++ b/include/framework/runner/runner.h
>> @@ -51,6 +51,8 @@ struct runner_opts {
>>   
>>       /// The runner will write JUnit XML to this path, if not NULL.
>>       const char *junit_xml_filepath;
>> +
>> +    int device_id;
>>   };
>>   
>>   bool runner_init(runner_opts_t *opts);
>> diff --git a/include/framework/test/test.h b/include/framework/test/test.h
>> index ca328bb..fc609cb 100644
>> --- a/include/framework/test/test.h
>> +++ b/include/framework/test/test.h
>> @@ -34,6 +34,7 @@ struct test_create_info {
>>       bool enable_separate_cleanup_thread;
>>       bool enable_spir_v;
>>       bool enable_bootstrap;
>> +    int device_id;
>>       uint32_t queue_family_index;
>>   
>>       uint32_t bootstrap_image_width;
>> diff --git a/misc/crucible-completion.bash b/misc/crucible-completion.bash
>> index 8942b14..4377224 100644
>> --- a/misc/crucible-completion.bash
>> +++ b/misc/crucible-completion.bash
>> @@ -35,6 +35,7 @@ __crucible_run()
>>         --no-cleanup
>>         --use-spir-v
>>         --junit-xml
>> +      --device-id
>>      "
>>   
>>      COMPREPLY=($(compgen -W "$flags $($1 ls-tests)" -- ${COMP_WORDS[COMP_CWORD]}))
>> diff --git a/src/cmd/bootstrap.c b/src/cmd/bootstrap.c
>> index bfa2221..f902fe1 100644
>> --- a/src/cmd/bootstrap.c
>> +++ b/src/cmd/bootstrap.c
>> @@ -136,6 +136,7 @@ start(const cru_command_t *cmd, int argc, char **argv)
>>                          .enable_bootstrap = true,
>>                          .enable_cleanup_phase = false,
>>                          .enable_spir_v = true,
>> +                       .device_id = 1,
>>                          .bootstrap_image_width = opt_image_width,
>>                          .bootstrap_image_height = opt_image_height);
>>       if (!test) {
>> diff --git a/src/cmd/run.c b/src/cmd/run.c
>> index 3f06075..078b0db 100644
>> --- a/src/cmd/run.c
>> +++ b/src/cmd/run.c
>> @@ -39,6 +39,7 @@ static int opt_dump = 0;
>>   static int opt_use_spir_v = 1;
>>   static int opt_separate_cleanup_thread = 1;
>>   static char *opt_junit_xml = NULL;
>> +static int opt_device_id = 1;
>>   
>>   // From man:getopt(3) :
>>   //
>> @@ -52,12 +53,13 @@ static char *opt_junit_xml = NULL;
>>   //    above) of optstring is a colon (':'),  then getopt() returns ':' instead
>>   //    of '?' to indicate a missing option argument.
>>   //
>> -static const char *shortopts = "+:hj:I:";
>> +static const char *shortopts = "+:hj:I:d:";
>>   
>>   enum opt_name {
>>       OPT_NAME_HELP = 'h',
>>       OPT_NAME_JOBS = 'j',
>>       OPT_NAME_ISLOATION = 'I',
>> +    OPT_NAME_DEVICE_ID = 'd',
>>   
>>       // Begin long-only options. They begin with the first char value outside
>>       // the ASCII range.
>> @@ -77,6 +79,7 @@ static const struct option longopts[] = {
>>       {"use-spir-v",    no_argument,       &opt_use_spir_v, true},
>>       {"no-spir-v",     no_argument,       &opt_use_spir_v, false},
>>       {"junit-xml",     required_argument, NULL,            OPT_NAME_JUNIT_XML},
>> +    {"device-id",     required_argument, NULL,            OPT_NAME_DEVICE_ID},
>>   
>>       {"separate-cleanup-threads",    no_argument, &opt_separate_cleanup_thread, true},
>>       {"no-separate-cleanup-threads", no_argument, &opt_separate_cleanup_thread, false},
>> @@ -151,6 +154,12 @@ parse_args(const cru_command_t *cmd, int argc, char **argv)
>>           case OPT_NAME_JUNIT_XML:
>>               opt_junit_xml = strdup(optarg);
>>               break;
>> +        case OPT_NAME_DEVICE_ID:
>> +            opt_device_id = strtol(optarg, NULL, 10);
>> +            if (opt_device_id <= 0) {
>> +                cru_usage_error(cmd, "--device must be at least 1");
>> +            }
>> +            break;
>>           case ':':
>>               cru_usage_error(cmd, "%s requires an argument", argv[optind-1]);
>>               break;
>> @@ -265,6 +274,7 @@ cmd_start(const cru_command_t *cmd, int argc, char **argv)
>>           .no_image_dumps = !opt_dump,
>>           .use_spir_v = opt_use_spir_v,
>>           .junit_xml_filepath = opt_junit_xml,
>> +        .device_id = opt_device_id,
>>       });
>>   
>>       if (opt_log_pids)
>> diff --git a/src/framework/runner/runner.c b/src/framework/runner/runner.c
>> index aaff68f..eedc2b1 100644
>> --- a/src/framework/runner/runner.c
>> +++ b/src/framework/runner/runner.c
>> @@ -101,6 +101,7 @@ run_test_def(const test_def_t *def, uint32_t queue_family_index)
>>                          .enable_spir_v = runner_opts.use_spir_v,
>>                          .enable_separate_cleanup_thread =
>>                               runner_opts.use_separate_cleanup_threads,
>> +                       .device_id = runner_opts.device_id,
>>                          .queue_family_index = queue_family_index);
>>       if (!test)
>>           return TEST_RESULT_FAIL;
>> diff --git a/src/framework/test/t_phase_setup.c b/src/framework/test/t_phase_setup.c
>> index a92a5d7..4c5cdce 100644
>> --- a/src/framework/test/t_phase_setup.c
>> +++ b/src/framework/test/t_phase_setup.c
>> @@ -24,6 +24,9 @@
>>   #include "test.h"
>>   #include "t_phase_setup.h"
>>   
>> +/* Maximum supported physical devs. */
>> +#define MAX_PHYSICAL_DEVS 4
>> +
>>   static void *
>>   test_vk_alloc(void *pUserData, size_t size, size_t alignment,
>>                 VkSystemAllocationScope scope)
>> @@ -70,16 +73,16 @@ t_setup_phys_dev(void)
>>       ASSERT_TEST_IN_SETUP_PHASE;
>>       GET_CURRENT_TEST(t);
>>   
>> -    // Crucible uses only the first physical device.
>> -    // FINISHME: Add a command-line option to use non-default physical device.
>> +    VkPhysicalDevice physical_devs[MAX_PHYSICAL_DEVS];
>>   
>>       uint32_t count = 0;
>>       qoEnumeratePhysicalDevices(t->vk.instance, &count, NULL);
>>       t_assertf(count > 0, "failed to enumerate any physical devices");
>> +    t_assertf(count <= MAX_PHYSICAL_DEVS, "reached the maximum supported physical devices");
>> +    t_assertf(t->opt.device_id <= count, "requested device id not found");
>>   
>> -    count = 1;
>> -    qoEnumeratePhysicalDevices(t->vk.instance, &count, &t->vk.physical_dev);
>> -    t_assertf(count == 1, "enumerated %u physical devices, expected 1", count);
>> +    qoEnumeratePhysicalDevices(t->vk.instance, &count, physical_devs);
>> +    t->vk.physical_dev = physical_devs[t->opt.device_id - 1];
> vkEnumeratePhysicalDevices() has no guarantees of ordering, so this
> might be rather fragile.
>
> Perhaps passing in VkPhysicalDeviceProperties::vendorID and
> VkPhysicalDeviceProperties::deviceID to crucible and matching devices
> based on these two would be a better option?
Well, VK-GL-CTS does the same thing as well. Ideally we should use 
vendorID and deviceID but I don't think this multi-GPU scenario is 
really used. For my needs, it's enough to just pass --device-id=1 or 
--device-id=2 and then check manually that it targets the expected GPU.
>
>>   
>>       qoGetPhysicalDeviceProperties(t->vk.physical_dev, &t->vk.physical_dev_props);
>>   }
>> diff --git a/src/framework/test/test.c b/src/framework/test/test.c
>> index 80281b9..85e6310 100644
>> --- a/src/framework/test/test.c
>> +++ b/src/framework/test/test.c
>> @@ -152,6 +152,7 @@ test_create_s(const test_create_info_t *info)
>>       t->opt.use_spir_v = info->enable_spir_v;
>>       t->opt.bootstrap = info->enable_bootstrap;
>>       t->opt.queue_family_index = info->queue_family_index;
>> +    t->opt.device_id = info->device_id;
>>   
>>       if (info->enable_bootstrap) {
>>           if (info->enable_cleanup_phase) {
>> diff --git a/src/framework/test/test.h b/src/framework/test/test.h
>> index cafcc7e..4fafcc2 100644
>> --- a/src/framework/test/test.h
>> +++ b/src/framework/test/test.h
>> @@ -121,6 +121,9 @@ struct test {
>>           /// thread.
>>           bool no_separate_cleanup_thread;
>>   
>> +        /// The Vulkan device ID.
>> +        int device_id;
>> +
>>           uint32_t queue_family_index;
>>       } opt;
>>   
>> -- 
>> 2.21.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
ping

On 4/5/19 12:27 PM, Samuel Pitoiset wrote:
> In a multi-GPU scenario.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>   doc/crucible-run.1.txt             |  4 ++++
>   include/framework/runner/runner.h  |  2 ++
>   include/framework/test/test.h      |  1 +
>   misc/crucible-completion.bash      |  1 +
>   src/cmd/bootstrap.c                |  1 +
>   src/cmd/run.c                      | 12 +++++++++++-
>   src/framework/runner/runner.c      |  1 +
>   src/framework/test/t_phase_setup.c | 13 ++++++++-----
>   src/framework/test/test.c          |  1 +
>   src/framework/test/test.h          |  3 +++
>   10 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/doc/crucible-run.1.txt b/doc/crucible-run.1.txt
> index 59aecd1..195da2c 100644
> --- a/doc/crucible-run.1.txt
> +++ b/doc/crucible-run.1.txt
> @@ -13,6 +13,7 @@ SYNOPSIS
>                  [--jobs=<jobs> | -j <jobs>] [--[no-]separate-cleanup-threads]
>                  [--use-spir-v|--no-spir-v] [--isolation=<method> | -I <method>]
>                  [--junit-xml=<junit-xml-file>]
> +               [--device-id=<device-id>]
>                  [<pattern>...]
>   
>   DESCRIPTION
> @@ -83,6 +84,9 @@ OPTIONS
>   --junit-xml=<junit-xml-file>::
>       Write JUnit XML to the given file.
>   
> +--device-id=<device-id>::
> +    Select the Vulkan device ID (IDs start from 1).
> +
>   EXAMPLES
>   --------
>   * Run all functional tests and stress tests. The following invocations are all
> diff --git a/include/framework/runner/runner.h b/include/framework/runner/runner.h
> index 01893ef..3015850 100644
> --- a/include/framework/runner/runner.h
> +++ b/include/framework/runner/runner.h
> @@ -51,6 +51,8 @@ struct runner_opts {
>   
>       /// The runner will write JUnit XML to this path, if not NULL.
>       const char *junit_xml_filepath;
> +
> +    int device_id;
>   };
>   
>   bool runner_init(runner_opts_t *opts);
> diff --git a/include/framework/test/test.h b/include/framework/test/test.h
> index ca328bb..fc609cb 100644
> --- a/include/framework/test/test.h
> +++ b/include/framework/test/test.h
> @@ -34,6 +34,7 @@ struct test_create_info {
>       bool enable_separate_cleanup_thread;
>       bool enable_spir_v;
>       bool enable_bootstrap;
> +    int device_id;
>       uint32_t queue_family_index;
>   
>       uint32_t bootstrap_image_width;
> diff --git a/misc/crucible-completion.bash b/misc/crucible-completion.bash
> index 8942b14..4377224 100644
> --- a/misc/crucible-completion.bash
> +++ b/misc/crucible-completion.bash
> @@ -35,6 +35,7 @@ __crucible_run()
>         --no-cleanup
>         --use-spir-v
>         --junit-xml
> +      --device-id
>      "
>   
>      COMPREPLY=($(compgen -W "$flags $($1 ls-tests)" -- ${COMP_WORDS[COMP_CWORD]}))
> diff --git a/src/cmd/bootstrap.c b/src/cmd/bootstrap.c
> index bfa2221..f902fe1 100644
> --- a/src/cmd/bootstrap.c
> +++ b/src/cmd/bootstrap.c
> @@ -136,6 +136,7 @@ start(const cru_command_t *cmd, int argc, char **argv)
>                          .enable_bootstrap = true,
>                          .enable_cleanup_phase = false,
>                          .enable_spir_v = true,
> +                       .device_id = 1,
>                          .bootstrap_image_width = opt_image_width,
>                          .bootstrap_image_height = opt_image_height);
>       if (!test) {
> diff --git a/src/cmd/run.c b/src/cmd/run.c
> index 3f06075..078b0db 100644
> --- a/src/cmd/run.c
> +++ b/src/cmd/run.c
> @@ -39,6 +39,7 @@ static int opt_dump = 0;
>   static int opt_use_spir_v = 1;
>   static int opt_separate_cleanup_thread = 1;
>   static char *opt_junit_xml = NULL;
> +static int opt_device_id = 1;
>   
>   // From man:getopt(3) :
>   //
> @@ -52,12 +53,13 @@ static char *opt_junit_xml = NULL;
>   //    above) of optstring is a colon (':'),  then getopt() returns ':' instead
>   //    of '?' to indicate a missing option argument.
>   //
> -static const char *shortopts = "+:hj:I:";
> +static const char *shortopts = "+:hj:I:d:";
>   
>   enum opt_name {
>       OPT_NAME_HELP = 'h',
>       OPT_NAME_JOBS = 'j',
>       OPT_NAME_ISLOATION = 'I',
> +    OPT_NAME_DEVICE_ID = 'd',
>   
>       // Begin long-only options. They begin with the first char value outside
>       // the ASCII range.
> @@ -77,6 +79,7 @@ static const struct option longopts[] = {
>       {"use-spir-v",    no_argument,       &opt_use_spir_v, true},
>       {"no-spir-v",     no_argument,       &opt_use_spir_v, false},
>       {"junit-xml",     required_argument, NULL,            OPT_NAME_JUNIT_XML},
> +    {"device-id",     required_argument, NULL,            OPT_NAME_DEVICE_ID},
>   
>       {"separate-cleanup-threads",    no_argument, &opt_separate_cleanup_thread, true},
>       {"no-separate-cleanup-threads", no_argument, &opt_separate_cleanup_thread, false},
> @@ -151,6 +154,12 @@ parse_args(const cru_command_t *cmd, int argc, char **argv)
>           case OPT_NAME_JUNIT_XML:
>               opt_junit_xml = strdup(optarg);
>               break;
> +        case OPT_NAME_DEVICE_ID:
> +            opt_device_id = strtol(optarg, NULL, 10);
> +            if (opt_device_id <= 0) {
> +                cru_usage_error(cmd, "--device must be at least 1");
> +            }
> +            break;
>           case ':':
>               cru_usage_error(cmd, "%s requires an argument", argv[optind-1]);
>               break;
> @@ -265,6 +274,7 @@ cmd_start(const cru_command_t *cmd, int argc, char **argv)
>           .no_image_dumps = !opt_dump,
>           .use_spir_v = opt_use_spir_v,
>           .junit_xml_filepath = opt_junit_xml,
> +        .device_id = opt_device_id,
>       });
>   
>       if (opt_log_pids)
> diff --git a/src/framework/runner/runner.c b/src/framework/runner/runner.c
> index aaff68f..eedc2b1 100644
> --- a/src/framework/runner/runner.c
> +++ b/src/framework/runner/runner.c
> @@ -101,6 +101,7 @@ run_test_def(const test_def_t *def, uint32_t queue_family_index)
>                          .enable_spir_v = runner_opts.use_spir_v,
>                          .enable_separate_cleanup_thread =
>                               runner_opts.use_separate_cleanup_threads,
> +                       .device_id = runner_opts.device_id,
>                          .queue_family_index = queue_family_index);
>       if (!test)
>           return TEST_RESULT_FAIL;
> diff --git a/src/framework/test/t_phase_setup.c b/src/framework/test/t_phase_setup.c
> index a92a5d7..4c5cdce 100644
> --- a/src/framework/test/t_phase_setup.c
> +++ b/src/framework/test/t_phase_setup.c
> @@ -24,6 +24,9 @@
>   #include "test.h"
>   #include "t_phase_setup.h"
>   
> +/* Maximum supported physical devs. */
> +#define MAX_PHYSICAL_DEVS 4
> +
>   static void *
>   test_vk_alloc(void *pUserData, size_t size, size_t alignment,
>                 VkSystemAllocationScope scope)
> @@ -70,16 +73,16 @@ t_setup_phys_dev(void)
>       ASSERT_TEST_IN_SETUP_PHASE;
>       GET_CURRENT_TEST(t);
>   
> -    // Crucible uses only the first physical device.
> -    // FINISHME: Add a command-line option to use non-default physical device.
> +    VkPhysicalDevice physical_devs[MAX_PHYSICAL_DEVS];
>   
>       uint32_t count = 0;
>       qoEnumeratePhysicalDevices(t->vk.instance, &count, NULL);
>       t_assertf(count > 0, "failed to enumerate any physical devices");
> +    t_assertf(count <= MAX_PHYSICAL_DEVS, "reached the maximum supported physical devices");
> +    t_assertf(t->opt.device_id <= count, "requested device id not found");
>   
> -    count = 1;
> -    qoEnumeratePhysicalDevices(t->vk.instance, &count, &t->vk.physical_dev);
> -    t_assertf(count == 1, "enumerated %u physical devices, expected 1", count);
> +    qoEnumeratePhysicalDevices(t->vk.instance, &count, physical_devs);
> +    t->vk.physical_dev = physical_devs[t->opt.device_id - 1];
>   
>       qoGetPhysicalDeviceProperties(t->vk.physical_dev, &t->vk.physical_dev_props);
>   }
> diff --git a/src/framework/test/test.c b/src/framework/test/test.c
> index 80281b9..85e6310 100644
> --- a/src/framework/test/test.c
> +++ b/src/framework/test/test.c
> @@ -152,6 +152,7 @@ test_create_s(const test_create_info_t *info)
>       t->opt.use_spir_v = info->enable_spir_v;
>       t->opt.bootstrap = info->enable_bootstrap;
>       t->opt.queue_family_index = info->queue_family_index;
> +    t->opt.device_id = info->device_id;
>   
>       if (info->enable_bootstrap) {
>           if (info->enable_cleanup_phase) {
> diff --git a/src/framework/test/test.h b/src/framework/test/test.h
> index cafcc7e..4fafcc2 100644
> --- a/src/framework/test/test.h
> +++ b/src/framework/test/test.h
> @@ -121,6 +121,9 @@ struct test {
>           /// thread.
>           bool no_separate_cleanup_thread;
>   
> +        /// The Vulkan device ID.
> +        int device_id;
> +
>           uint32_t queue_family_index;
>       } opt;
>