> -----Original Message----- > From: daniel.vetter@xxxxxxxx [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > Daniel Vetter > Sent: Monday, January 12, 2015 11:26 PM > To: Gore, Tim > Cc: Gordon, David S; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Wood, Thomas > Subject: Re: [PATCH i-g-t] tests/gem_exec_params: change flags > used in invalid-flags test > > On Mon, Jan 12, 2015 at 04:14:03PM +0000, Gore, Tim wrote: > > > > > > > -----Original Message----- > > > From: Gordon, David S > > > Sent: Monday, January 12, 2015 4:04 PM > > > To: Gore, Tim; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: Wood, Thomas > > > Subject: Re: [PATCH i-g-t] tests/gem_exec_params: change > > > flags used in invalid-flags test > > > > > > On 12/01/15 14:09, tim.gore@xxxxxxxxx wrote: > > > > From: Tim Gore <tim.gore@xxxxxxxxx> > > > > > > > > The invalid-flags test in gem_exec_params uses > > > > (I915_EXEC_HANDLE_LUT << 1) as an invalid flag, but this is no > > > > longer invalid for recent android versions, and may not be invalid > > > > in Linux in the future. So I have changed this test to use > (__I915_EXEC_UNKNOWN_FLAGS) instead. > > > > __I915_EXEC_UNKNOWN_FLAGS is defined in i915_drm.h as a mask of > > > > all the undefined flags. > > > > > > > > Signed-off-by: Tim Gore <tim.gore@xxxxxxxxx> > > > > --- > > > > tests/gem_exec_params.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c > > > > index > > > > f63eda9..2a1c544 100644 > > > > --- a/tests/gem_exec_params.c > > > > +++ b/tests/gem_exec_params.c > > > > @@ -179,7 +179,7 @@ igt_main > > > > /* HANDLE_LUT and NO_RELOC are already exercised by > > > > gem_exec_lut_handle */ > > > > > > > > igt_subtest("invalid-flag") { > > > > - execbuf.flags = I915_EXEC_RENDER | > > > (I915_EXEC_HANDLE_LUT << 1); > > > > + execbuf.flags = I915_EXEC_RENDER | > > > (__I915_EXEC_UNKNOWN_FLAGS); > > > > RUN_FAIL(EINVAL); > > > > } > > > > > > > > > > Should we perhaps have a test that iterates over each bit in this > > > mask one at a time (to check that EACH of them is correctly detected > > > and > > > rejected) as well as this one with ALL the unknown flag bits set? > > > > > > .Dave. > > > > Yes, I can do that if people like the idea. > > Well the testcase should still fail if the kernel is accepting any flags - the idea > is very much that every time you add a flag the test fails and will remind you > to add the new testcases for the new flag. So any patch which makes LUT << > 1 no longer fail the tests if it's not rejected is nacked by me. > > Imo you should just carry an igt patch in the android version somewhere to > adapt the testcase to your abi changes. > -Daniel No, the patch uses __I915_EXEC_UNKNOWN_FLAGS, which is set in i915_drm.h, and hopefully Is maintained to be the set of all invalid flags. In the upstream version this is set to -(I915_EXEC_HANDLE_LUT<<1). In the android version it is set to -(I915_EXEC_ENABLE_WATCHDOG<<1) So Using this macro should give you the right test in each case, rather than having a special Android test case that is separately maintained from the actual definition of the flags. Tim > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx