[Mesa-dev] [crucible PATCH] add a command line option for selecting the Vulkan device
Samuel Pitoiset
samuel.pitoiset at gmail.com
Fri Apr 5 13:21:56 UTC 2019
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 at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list