On Thu, Nov 24, 2016 at 01:47:46PM +0000, Robert Bragg wrote:
> This further generalises the description passed to test_lri so we only
> need one loop over the entries with test_lri deducing the exected errno
> and value based on whether the register is marked as whitelisted and
> depending on the current command parser version.
>
> Each tested register LRI now test gets its own subtest like:
> igt_subtest_f("test-lri-%s", reg_name)
>
> The test_lri helper now also double checks that the initial
> intel_register_write() takes before issuing the LRI.
>
> In case of a failure the test_lri helper now uses igt_debug to log the
> register name, address and value being tested.
That looks like it should help immensely if one of these should ever
fire. Thanks.
> Signed-off-by: Robert Bragg <robert@xxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> tests/gem_exec_parse.c | 102 +++++++++++++++++++++++++------------------------
> 1 file changed, 52 insertions(+), 50 deletions(-)
>
> diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
> index cc2103a..0cd7053 100644
> --- a/tests/gem_exec_parse.c
> +++ b/tests/gem_exec_parse.c
> @@ -258,12 +258,17 @@ static void exec_batch_chained(int fd, uint32_t cmd_bo, uint32_t *cmds,
> * from...
> */
> struct test_lri {
> - uint32_t reg, read_mask, init_val, test_val;
> + const char *name; /* register name for debug info */
> + uint32_t reg; /* address to test */
I would use offset, but we often mix the two anyway.
> + uint32_t read_mask; /* ignore things like HW status bits */
> + uint32_t init_val; /* initial identifiable value to set without LRI */
> + uint32_t test_val; /* value to attempt loading via LRI command */
> + bool whitelisted; /* expect to become NOOP / fail if not whitelisted */
> + int min_ver; /* required command parser version to test */
> };
>
> static void
> -test_lri(int fd, uint32_t handle,
> - struct test_lri *test, int expected_errno, uint32_t expect)
> +test_lri(int fd, uint32_t handle, struct test_lri *test)
> {
> uint32_t lri[] = {
> MI_LOAD_REGISTER_IMM,
> @@ -271,9 +276,20 @@ test_lri(int fd, uint32_t handle,
> test->test_val,
> MI_BATCH_BUFFER_END,
> };
> + int bad_lri_errno = parser_version >= 8 ? 0 : -EINVAL;
> + int expected_errno = test->whitelisted ? 0 : bad_lri_errno;
> + uint32_t expect = test->whitelisted ? test->test_val : test->init_val;
> +
> + igt_debug("Testing %s LRI: addr=%x, val=%x, expected errno=%d, expected val=%x\n",
> + test->name, test->reg, test->test_val,
> + expected_errno, expect);
>
> intel_register_write(test->reg, test->init_val);
>
> + igt_assert_eq_u32((intel_register_read(test->reg) &
> + test->read_mask),
> + test->init_val);
> +
> exec_batch(fd, handle,
> lri, sizeof(lri),
> I915_EXEC_RENDER,
I would still be verbose in the assert, it is the first thing you read
on seeing the error before digging through the debug log.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
