[Intel-gfx] [PATCH i-g-t 2/4] tests/gem_scheduler: Add gem_scheduler test
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Feb 17 12:37:07 UTC 2016
Hi,
first round of comments inline. A few things will probably have to
change based on the comments on the previous patch in the series so I'll
have a better look at the related logic once the new series is up.
Regards,
Daniele
On 12/02/16 09:38, Derek Morton wrote:
> This is intended to test the scheduler behaviour is correct.
> The subtests are
> <ring>-basic
> Tests that batch buffers of the same priority submitted to a ring
> execute in the order they are submitted.
> <ring>-read
> Submits a batch buffer with a read dependency to a buffer object to
> a ring which is held in the scheduler queue by a long running batch
> buffer. Submit batch buffers to other rings that have a read dependency
> to the same buffer object. Ensure they execute before the batch buffer
> being held up behind the long running batch buffer.
> <ring>-write
> Submits a batch buffer with a write dependency to a buffer object to
> a ring which is held in the scheduler queue by a long running batch
> buffer. Submit batch buffers to other rings that have a write dependency
> to the same buffer object. Submit batch buffers with no interdependencies
> to all rings. Ensure the batch buffers that have write dependencies are
> executed in submission order but the batch buffers without interdependencies
> do not get held up.
>
> Signed-off-by: Derek Morton <derek.j.morton at intel.com>
> ---
> tests/Makefile.sources | 1 +
> tests/gem_scheduler.c | 409 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 410 insertions(+)
> create mode 100644 tests/gem_scheduler.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index df92586..439f62c 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -66,6 +66,7 @@ TESTS_progs_M = \
> gem_request_retire \
> gem_reset_stats \
> gem_ringfill \
> + gem_scheduler \
> gem_set_tiling_vs_blt \
> gem_softpin \
> gem_stolen \
> diff --git a/tests/gem_scheduler.c b/tests/gem_scheduler.c
> new file mode 100644
> index 0000000..4824c13
> --- /dev/null
> +++ b/tests/gem_scheduler.c
> @@ -0,0 +1,409 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Derek Morton <derek.j.morton at intel.com>
> + *
> + */
> +
> +#include "igt.h"
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <time.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +
> +IGT_TEST_DESCRIPTION("Check scheduler behaviour. Basic tests ensure independant "
> + "batch buffers of the same priority are executed in "
> + "submission order. Read-read tests ensure "
> + "batch buffers with a read dependency to the same buffer "
> + "object do not block each other. Write-write dependency "
> + "tests ensure batch buffers with a write dependency to a "
> + "buffer object will be executed in submission order but "
> + "will not block execution of other independant batch "
> + "buffers.");
> +
> +#define SEC_TO_NSEC (1000 * 1000 * 1000)
> +
> +struct ring {
> + const char *name;
> + int id;
> +} rings[] = {
> + { "render", I915_EXEC_RENDER },
> + { "bsd", I915_EXEC_BSD },
Is BSD1/BSD2 difference of any importance for the tests?
> + { "blt", I915_EXEC_BLT },
> + { "vebox", I915_EXEC_VEBOX },
> +};
This is a slight duplication of intel_execution_engines. However, I'm
not sure that intel_execution_engines is suitable here considering that
it doesn't seem you want to check both the vanilla I915_EXEC_BSD case
and the BSD1/BSD2 case
> +
> +#define NBR_RINGS (sizeof(rings)/sizeof(struct ring))
> +
> +/* Basic test. Check batch buffers of the same priority and with no dependencies
> + * are executed in the order they are submitted.
> + */
> +#define NBR_BASIC_FDs (3)
> +static void run_test_basic(int in_flight, int ringid)
> +{
> + int fd[NBR_BASIC_FDs];
> + int loop;
> + drm_intel_bufmgr *bufmgr[NBR_BASIC_FDs];
> + uint32_t *delay_buf, *ts1_buf, *ts2_buf;
> + struct intel_batchbuffer *ts1_bb, *ts2_bb;
> + struct intel_batchbuffer **in_flight_bbs;
> + uint32_t calibrated_1s;
> + drm_intel_bo *delay_bo, *ts1_bo, *ts2_bo;
> +
> + in_flight_bbs = malloc(in_flight * sizeof(struct intel_batchbuffer *));
> + igt_assert(in_flight_bbs);
> +
> + /* Need multiple i915 fd's. Scheduler will not change execution order of
> + * batch buffers from the same context.
> + */
> + for(loop=0; loop < NBR_BASIC_FDs; loop++) {
> + struct intel_batchbuffer *noop_bb;
> + fd[loop] = drm_open_driver(DRIVER_INTEL);
> + igt_assert(fd[loop] >= 0);
> + bufmgr[loop] = drm_intel_bufmgr_gem_init(fd[loop], BATCH_SZ);
> + igt_assert(bufmgr[loop]);
> + drm_intel_bufmgr_gem_enable_reuse(bufmgr[loop]);
> + /* Send a noop batch buffer to force any deferred initialisation */
> + noop_bb = igt_create_noop_bb(fd[loop], bufmgr[loop], ringid, 5);
> + intel_batchbuffer_flush_on_ring(noop_bb, ringid);
> + intel_batchbuffer_free(noop_bb);
> + }
> +
> + /* Create buffer objects */
> + delay_bo = drm_intel_bo_alloc(bufmgr[0], "delay bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(delay_bo);
> + ts1_bo = drm_intel_bo_alloc(bufmgr[1], "ts1 bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(ts1_bo);
> + ts2_bo = drm_intel_bo_alloc(bufmgr[2], "ts2 bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(ts2_bo);
> +
> + /* Put some non zero values in the delay bo */
> + drm_intel_bo_map(delay_bo, 1);
As I already mentioned on the other patch in some cases you could use a
subdata/get_subdata call instead of the map/unmap dance
> + delay_buf = delay_bo->virtual;
> + delay_buf[0] = 0xff;
> + drm_intel_bo_unmap(delay_bo);
> +
> + calibrated_1s = igt_calibrate_delay_bb(fd[0], bufmgr[0], ringid);
> +
> + /* Batch buffers to fill the ring */
"fill the ring" feels a bit unclear to me, because you're not filling
the ringbuffer but the in-flight queue of the scheduler. Maybe use
something like "reach the maximum number of in flight batches"?
> + in_flight_bbs[0] = igt_create_delay_bb(fd[0], bufmgr[0], ringid, calibrated_1s, delay_bo);
> + for(loop = 1; loop < in_flight; loop++)
> + in_flight_bbs[loop] = igt_create_noop_bb(fd[0], bufmgr[0], ringid, 5);
> +
> + /* Extra batch buffers in the scheduler queue */
> + ts1_bb = igt_create_timestamp_bb(fd[1], bufmgr[1], ringid, ts1_bo, NULL, false);
> + ts2_bb = igt_create_timestamp_bb(fd[2], bufmgr[2], ringid, ts2_bo, NULL, false);
> +
> + /* Flush batchbuffers */
> + for(loop = 0; loop < in_flight; loop++)
> + intel_batchbuffer_flush_on_ring(in_flight_bbs[loop], ringid);
> + intel_batchbuffer_flush_on_ring(ts1_bb, ringid);
> + intel_batchbuffer_flush_on_ring(ts2_bb, ringid);
> +
> + /* This will not return until the bo has finished executing */
> + drm_intel_bo_map(delay_bo, 0);
> + drm_intel_bo_map(ts1_bo, 0);
> + drm_intel_bo_map(ts2_bo, 0);
> +
> + delay_buf = delay_bo->virtual;
> + ts1_buf = ts1_bo->virtual;
> + ts2_buf = ts2_bo->virtual;
> +
> + igt_debug("Delay Timestamp = 0x%08" PRIx32 "\n", delay_buf[2]);
> + igt_debug("TS1 Timestamp = 0x%08" PRIx32 "\n", ts1_buf[0]);
> + igt_debug("TS2 Timestamp = 0x%08" PRIx32 "\n", ts2_buf[0]);
> +
> + /* buf[0] in the target buffer should be 0 if the batch buffer completed */
> + igt_assert_f(delay_buf[0] == 0,
> + "delay_buf[0] expected 0x0, got 0x%" PRIx32 "\n", delay_buf[0]);
> +
> + igt_assert_f(igt_compare_timestamps(delay_buf[2], ts1_buf[0]),
> + "Delay ts (0x%08" PRIx32 ") > TS1 ts (0x%08" PRIx32 ")\n",
> + delay_buf[2], ts1_buf[0]);
> + igt_assert_f(igt_compare_timestamps(ts1_buf[0], ts2_buf[0]),
> + "TS1 ts (0x%08" PRIx32 ") > TS2 ts (0x%08" PRIx32 ")\n",
> + ts1_buf[0], ts2_buf[0]);
> +
> + /* Cleanup */
> + for(loop = 0; loop < in_flight; loop++)
> + intel_batchbuffer_free(in_flight_bbs[loop]);
> + intel_batchbuffer_free(ts1_bb);
> + intel_batchbuffer_free(ts2_bb);
> +
> + drm_intel_bo_unreference(delay_bo);
> + drm_intel_bo_unreference(ts1_bo);
> + drm_intel_bo_unreference(ts2_bo);
> + for(loop = 0; loop < 3; loop++) {
3 -> NBR_BASIC_FDs
> + drm_intel_bufmgr_destroy(bufmgr[loop]);
> + close(fd[loop]);
> + }
> + free(in_flight_bbs);
> +}
> +
> +/* Dependency test.
> + * write=0, Submit batch buffers with read dependencies to all rings. Delay one
> + * with a long executing batch buffer. Check the others are not held up.
> + * write=1, Submit batch buffers with write dependencies to all rings. Delay one
> + * with a long executing batch buffer. Also submit batch buffers with no
> + * dependencies to all rings. Batch buffers with write dependencies should be
> + * executed in submission order. The batch buffers with no dependencies should
> + * not be held up.
> + */
> +static void run_test_dependency(int in_flight, int ring, bool write)
This function contains several setup/cleanup loops that make the actual
test logic itself a bit difficult to isolate. Maybe those could be moved
to helper function to make the distinction clearer
> +{
> + int fd[NBR_RINGS], fd2[NBR_RINGS];
> + int loop;
> + int prime_fd;
> + uint32_t *delay_buf, *ts_buf[NBR_RINGS], *ts2_buf[NBR_RINGS], *shared_buf;
> + uint32_t calibrated_1s;
> + drm_intel_bufmgr *bufmgr[NBR_RINGS], *bufmgr2[NBR_RINGS];
> + struct intel_batchbuffer *ts_bb[NBR_RINGS], *ts2_bb[NBR_RINGS], **in_flight_bbs;
> + drm_intel_bo *delay_bo, *ts_bo[NBR_RINGS], *ts2_bo[NBR_RINGS], *shared_bo[NBR_RINGS];
> +
> + in_flight_bbs = malloc(in_flight * sizeof(struct intel_batchbuffer *));
> + igt_assert(in_flight_bbs);
> +
> + /* Need multiple i915 fd's. Scheduler will not change execution order of
> + * batch buffers from the same context.
> + */
> + for(loop=0; loop < NBR_RINGS; loop++) {
> + struct intel_batchbuffer *noop_bb;
> + fd[loop] = drm_open_driver(DRIVER_INTEL);
> + igt_assert(fd[loop] >= 0);
> + bufmgr[loop] = drm_intel_bufmgr_gem_init(fd[loop], BATCH_SZ);
> + igt_assert(bufmgr[loop]);
> + drm_intel_bufmgr_gem_enable_reuse(bufmgr[loop]);
> + /* Send a noop batch buffer to force any deferred initialisation */
> + noop_bb = igt_create_noop_bb(fd[loop], bufmgr[loop], rings[loop].id, 5);
> + intel_batchbuffer_flush_on_ring(noop_bb, rings[loop].id);
> + intel_batchbuffer_free(noop_bb);
The fd opening code is duplicated several times in this file. Could use
a static function to reduce duplication
> + if(write) {
> + struct intel_batchbuffer *noop_bb2;
> + fd2[loop] = drm_open_driver(DRIVER_INTEL);
> + igt_assert(fd2[loop] >= 0);
> + bufmgr2[loop] = drm_intel_bufmgr_gem_init(fd2[loop], BATCH_SZ);
> + igt_assert(bufmgr2[loop]);
> + drm_intel_bufmgr_gem_enable_reuse(bufmgr2[loop]);
> + /* Send a noop batch buffer to force any deferred initialisation */
> + noop_bb2 = igt_create_noop_bb(fd2[loop], bufmgr2[loop], rings[loop].id, 5);
> + intel_batchbuffer_flush_on_ring(noop_bb2, rings[loop].id);
> + intel_batchbuffer_free(noop_bb2);
> + }
> + }
> +
> + /* Create buffer objects */
> + delay_bo = drm_intel_bo_alloc(bufmgr[ring], "delay bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(delay_bo);
> + for(loop = 0; loop < NBR_RINGS; loop++) {
> + ts_bo[loop] = drm_intel_bo_alloc(bufmgr[loop], "ts bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(ts_bo[loop]);
> + if(write) {
> + ts2_bo[loop] = drm_intel_bo_alloc(bufmgr2[loop], "ts bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(ts2_bo[loop]);
> + }
> + }
> +
> + /* Create shared buffer object */
> + shared_bo[0] = drm_intel_bo_alloc(bufmgr[0], "shared bo", BATCH_SZ, BATCH_SZ);
> + igt_assert(shared_bo[0]);
> +
> + drm_intel_bo_gem_export_to_prime(shared_bo[0], &prime_fd);
> + for(loop = 1; loop < NBR_RINGS; loop++) {
> + shared_bo[loop] = drm_intel_bo_gem_create_from_prime(bufmgr[loop],
> + prime_fd, BATCH_SZ);
> + igt_assert(shared_bo[loop]);
> + }
> + close(prime_fd);
> +
> + /* Put some non zero values in the delay and shared bo */
> + drm_intel_bo_map(delay_bo, 1);
> + delay_buf = delay_bo->virtual;
> + delay_buf[0] = 0xff;
> + drm_intel_bo_unmap(delay_bo);
> + drm_intel_bo_map(shared_bo[0], 1);
> + shared_buf = shared_bo[0]->virtual;
> + shared_buf[0] = 0xff00ff00;
> + drm_intel_bo_unmap(shared_bo[0]);
> +
> + calibrated_1s = igt_calibrate_delay_bb(fd[ring], bufmgr[ring], rings[ring].id);
> +
> + /* Batch buffers to fill the ring */
> + in_flight_bbs[0] = igt_create_delay_bb(fd[ring], bufmgr[ring],
> + rings[ring].id, calibrated_1s, delay_bo);
> + for(loop = 1; loop < in_flight; loop++)
> + in_flight_bbs[loop] = igt_create_noop_bb(fd[ring], bufmgr[ring],
> + rings[ring].id, 5);
> +
> + for(loop = 0; loop < NBR_RINGS; loop++) {
> + ts_bb[loop] = igt_create_timestamp_bb(fd[loop], bufmgr[loop],
> + rings[loop].id, ts_bo[loop], shared_bo[loop], write);
> + if(write)
> + ts2_bb[loop] = igt_create_timestamp_bb(fd2[loop], bufmgr2[loop],
> + rings[loop].id, ts2_bo[loop], NULL, false);
> + }
> +
> + /* Flush batchbuffers */
> + for(loop = 0; loop < in_flight; loop++)
> + intel_batchbuffer_flush_on_ring(in_flight_bbs[loop], rings[ring].id);
> +
> + intel_batchbuffer_flush_on_ring(ts_bb[ring], rings[ring].id);
> + for(loop = 0; loop < NBR_RINGS; loop++)
> + if(loop != ring)
> + intel_batchbuffer_flush_on_ring(ts_bb[loop], rings[loop].id);
> +
> + if(write) {
> + intel_batchbuffer_flush_on_ring(ts2_bb[ring], rings[ring].id);
> + for(loop = 0; loop < NBR_RINGS; loop++)
> + if(loop != ring)
> + intel_batchbuffer_flush_on_ring(ts2_bb[loop], rings[loop].id);
> + }
> +
> + /* This will not return until the bo has finished executing */
> + drm_intel_bo_map(delay_bo, 0);
> + delay_buf = delay_bo->virtual;
> + for(loop = 0; loop < NBR_RINGS; loop++) {
> + drm_intel_bo_map(ts_bo[loop], 0);
> + ts_buf[loop] = ts_bo[loop]->virtual;
> + if(write) {
> + drm_intel_bo_map(ts2_bo[loop], 0);
> + ts2_buf[loop] = ts2_bo[loop]->virtual;
> + }
> + }
> +
> + /* buf[0] in the target buffer should be 0 if the batch buffer completed */
> + igt_assert_f(delay_buf[0] == 0,
> + "delay_buf[0] expected 0x0, got 0x%" PRIx32 "\n",
> + delay_buf[0]);
> +
> + igt_debug("%6s delay timestamp = 0x%08" PRIx32 "\n",
> + rings[ring].name, delay_buf[2]);
> + for(loop = 0; loop < NBR_RINGS; loop++)
> + igt_debug("%6s batch timestamp = 0x%08" PRIx32 "\n",
> + rings[loop].name, ts_buf[loop][0]);
> +
> + if(write) {
> + igt_debug("Independant batch buffers\n");
> + for(loop = 0; loop < NBR_RINGS; loop++)
> + igt_debug("%6s batch timestamp = 0x%08" PRIx32 "\n",
> + rings[loop].name, ts2_buf[loop][0]);
> + }
> +
> + for(loop = 0; loop < NBR_RINGS; loop++) {
> + if(loop != ring) {
> + if(write) {
> + /* Write dependency, delayed ring should run first */
> + igt_assert_f(igt_compare_timestamps(ts_buf[ring][0], ts_buf[loop][0]),
> + "%s ran before %s - 0x%08" PRIx32 " vs 0x%08" PRIx32 "\n",
> + rings[loop].name, rings[ring].name,
> + ts_buf[loop][0], ts_buf[ring][0]);
> + /* Second bb without dependency should run first */
> + igt_assert_f(igt_compare_timestamps(ts2_buf[loop][0], ts_buf[loop][0]),
> + "(%s) independant bb was held up - 0x%08" PRIx32 " vs 0x%08" PRIx32 "\n",
> + rings[loop].name, ts_buf[loop][0], ts2_buf[loop][0]);
> + }
> + else
> + /* Read dependency, delayed ring should run last */
> + igt_assert_f(igt_compare_timestamps(ts_buf[loop][0], ts_buf[ring][0]),
> + "%s ran after %s - 0x%08" PRIx32 " vs 0x%08" PRIx32 "\n",
> + rings[loop].name, rings[ring].name,
> + ts_buf[loop][0], ts_buf[ring][0]);
> + }
> + }
> +
> + /* Cleanup */
> + for(loop = 0; loop < in_flight; loop++)
> + intel_batchbuffer_free(in_flight_bbs[loop]);
> +
> + for(loop = 0; loop < NBR_RINGS; loop++) {
> + intel_batchbuffer_free(ts_bb[loop]);
> + drm_intel_bo_unreference(ts_bo[loop]);
> + drm_intel_bo_unreference(shared_bo[loop]);
> + if(write) {
> + intel_batchbuffer_free(ts2_bb[loop]);
> + drm_intel_bo_unreference(ts2_bo[loop]);
> + }
> + }
> +
> + drm_intel_bo_unreference(delay_bo);
> +
> + for(loop = 0; loop < NBR_RINGS; loop++) {
> + drm_intel_bufmgr_destroy(bufmgr[loop]);
> + close(fd[loop]);
> + if(write) {
> + drm_intel_bufmgr_destroy(bufmgr2[loop]);
> + close(fd2[loop]);
> + }
> + }
> +
> + free(in_flight_bbs);
> +}
> +
> +igt_main
> +{
> + int loop;
> + int in_flight;
> +
> + igt_fixture {
> + int debug_fd;
> + int l;
> + char buf[6];
> + /* Get nbr of batch buffers that the scheduler will queue in the
> + * HW. If this debugfs file does not exist there is no scheduler
> + * so skip the test.
> + */
> + debug_fd = igt_debugfs_open("i915_scheduler_min_flying", O_RDONLY);
> + igt_skip_on(debug_fd == -1);
> + l = read(debug_fd, buf, sizeof(buf)-1);
> + igt_assert(l > 0);
> + igt_assert(l < sizeof(buf));
> + buf[l] = '\0';
> + /* May be a decimal or hex number depending on sheduler version */
> + if(sscanf(buf, "0x%2x", &in_flight) != 1)
> + igt_assert_f(sscanf(buf, "%2d", &in_flight) == 1,
> + "Error reading from i915_scheduler_min_flying\n");
This will probably stabilize before the scheduler is merged, so the
final version should probably have only 1 sscanf call
> + close(debug_fd);
> + igt_debug("in flight = %d\n", in_flight);
> + }
> +
> + for (loop=0; loop < NBR_RINGS; loop++)
> + igt_subtest_f("%s-basic", rings[loop].name) {
> + run_test_basic(in_flight, rings[loop].id);
> + }
> +
> + for (loop=0; loop < NBR_RINGS; loop++)
> + igt_subtest_f("%s-read", rings[loop].name) {
> + run_test_dependency(in_flight, loop, false);
> + }
> +
> + for (loop=0; loop < NBR_RINGS; loop++)
> + igt_subtest_f("%s-write", rings[loop].name) {
> + run_test_dependency(in_flight, loop, true);
> + }
> +
> +}
More information about the Intel-gfx
mailing list