* [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
@ 2015-08-06 21:33 Daniel Vetter
2015-08-07 12:53 ` David Weinehall
2015-08-07 13:04 ` Paulo Zanoni
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Vetter @ 2015-08-06 21:33 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
The point of testing for LAST_FLAG + 1 is to catch abi extensions -
despite our best efforts we really suck at properly reviewing for test
coverage when extending ABI.
The real bug here is that David Weinhall hasn't submitted updated igts
for the NO_ZEROMAP feature yet. Imo the right course of action is to
revert that feature if the testcase don't show up within a few days.
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
tests/gem_ctx_param_basic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index 5ff3b13f4c7a..b44b37cf0538 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -98,7 +98,7 @@ igt_main
ctx_param.size = 0;
}
- ctx_param.param = -1;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
igt_subtest("invalid-param-get") {
ctx_param.context = ctx;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
2015-08-06 21:33 [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params" Daniel Vetter
@ 2015-08-07 12:53 ` David Weinehall
2015-08-07 13:13 ` Daniel Vetter
2015-08-21 13:26 ` Ander Conselvan De Oliveira
2015-08-07 13:04 ` Paulo Zanoni
1 sibling, 2 replies; 7+ messages in thread
From: David Weinehall @ 2015-08-07 12:53 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
>
> The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> despite our best efforts we really suck at properly reviewing for test
> coverage when extending ABI.
>
> The real bug here is that David Weinhall hasn't submitted updated igts
> for the NO_ZEROMAP feature yet. Imo the right course of action is to
> revert that feature if the testcase don't show up within a few days.
The reason I never submitted it was probably because of Chris's strong
opposition to the feature in the first place; I've had the testcase
laying around on my computer for quite a while.
Anyhow, here's a slightly modified version of that test -- hopefully
not breaking anything.
Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index bc5d4bd827cf..f4deca6bd79e 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
uint32_t size;
uint64_t param;
#define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
+#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
uint64_t value;
};
void gem_context_require_ban_period(int fd);
diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index b44b37cf0538..1e7e8ff40703 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -57,7 +57,7 @@ igt_main
ctx = gem_context_create(fd);
}
- ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
igt_subtest("basic") {
ctx_param.context = ctx;
@@ -98,21 +98,31 @@ igt_main
ctx_param.size = 0;
}
- ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
- igt_subtest("invalid-param-get") {
- ctx_param.context = ctx;
- TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
+ igt_subtest("non-root-set") {
+ igt_fork(child, 1) {
+ igt_drop_root();
+
+ ctx_param.context = ctx;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+ ctx_param.value--;
+ TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
+ }
+
+ igt_waitchildren();
}
- igt_subtest("invalid-param-set") {
+ igt_subtest("root-set") {
ctx_param.context = ctx;
- TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+ ctx_param.value--;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
}
- ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
- igt_subtest("non-root-set") {
+ igt_subtest("non-root-set-no-zeromap") {
igt_fork(child, 1) {
igt_drop_root();
@@ -125,13 +135,32 @@ igt_main
igt_waitchildren();
}
- igt_subtest("root-set") {
+ igt_subtest("root-set-no-zeromap-enabled") {
ctx_param.context = ctx;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
- ctx_param.value--;
+ ctx_param.value = 1;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
+ }
+
+ igt_subtest("root-set-no-zeromap-disabled") {
+ ctx_param.context = ctx;
+ TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+ ctx_param.value = 0;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
}
+ ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP + 1;
+
+ igt_subtest("invalid-param-get") {
+ ctx_param.context = ctx;
+ TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
+ }
+
+ igt_subtest("invalid-param-set") {
+ ctx_param.context = ctx;
+ TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
+ }
+
igt_fixture
close(fd);
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
2015-08-06 21:33 [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params" Daniel Vetter
2015-08-07 12:53 ` David Weinehall
@ 2015-08-07 13:04 ` Paulo Zanoni
2015-08-10 8:31 ` David Weinehall
1 sibling, 1 reply; 7+ messages in thread
From: Paulo Zanoni @ 2015-08-07 13:04 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
2015-08-06 18:33 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
>
> The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> despite our best efforts we really suck at properly reviewing for test
> coverage when extending ABI.
>
> The real bug here is that David Weinhall hasn't submitted updated igts
> for the NO_ZEROMAP feature yet. Imo the right course of action is to
> revert that feature if the testcase don't show up within a few days.
>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> tests/gem_ctx_param_basic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index 5ff3b13f4c7a..b44b37cf0538 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -98,7 +98,7 @@ igt_main
> ctx_param.size = 0;
> }
>
> - ctx_param.param = -1;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
How about adding a comment somewhere "If this breaks it's because we
extended the number of params without updating IGT. Please add the
proper tests for the new param"? That will help preventing us from
making the same error again next year.
>
> igt_subtest("invalid-param-get") {
> ctx_param.context = ctx;
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
2015-08-07 12:53 ` David Weinehall
@ 2015-08-07 13:13 ` Daniel Vetter
2015-08-21 13:26 ` Ander Conselvan De Oliveira
1 sibling, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2015-08-07 13:13 UTC (permalink / raw)
To: Daniel Vetter, Intel Graphics Development, Jesse Barnes, Daniel Vetter
On Fri, Aug 07, 2015 at 03:53:57PM +0300, David Weinehall wrote:
> On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
> >
> > The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> > despite our best efforts we really suck at properly reviewing for test
> > coverage when extending ABI.
> >
> > The real bug here is that David Weinhall hasn't submitted updated igts
> > for the NO_ZEROMAP feature yet. Imo the right course of action is to
> > revert that feature if the testcase don't show up within a few days.
>
> The reason I never submitted it was probably because of Chris's strong
> opposition to the feature in the first place; I've had the testcase
> laying around on my computer for quite a while.
>
> Anyhow, here's a slightly modified version of that test -- hopefully
> not breaking anything.
>
>
> Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90808
btw this is first priority for bug scrub, finding bugs. Also this is a
regression, which actually makes it even more important than just basic
bug scrubbing. Note that bug fixing itself is only about 3rd tier
priority, i.e. something you can do when you have free time by accident.
Oh and the bug is a regression, but not correctly marked as such. That
means QA fail or bug scrub fail, either way we need to figure out what
went wrong here.
Please discuss this with Christophe Prigent as our permanent bug scrub
leader, figure out what needs to be fixed in bkms and present the solution
in next week's bug scrub coordination meeting on Thu.
Thanks, Daniel
>
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index bc5d4bd827cf..f4deca6bd79e 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
> uint32_t size;
> uint64_t param;
> #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
> +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
> uint64_t value;
> };
> void gem_context_require_ban_period(int fd);
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index b44b37cf0538..1e7e8ff40703 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -57,7 +57,7 @@ igt_main
> ctx = gem_context_create(fd);
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
>
> igt_subtest("basic") {
> ctx_param.context = ctx;
> @@ -98,21 +98,31 @@ igt_main
> ctx_param.size = 0;
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
>
> - igt_subtest("invalid-param-get") {
> - ctx_param.context = ctx;
> - TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
> + igt_subtest("non-root-set") {
> + igt_fork(child, 1) {
> + igt_drop_root();
> +
> + ctx_param.context = ctx;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> + ctx_param.value--;
> + TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
> + }
> +
> + igt_waitchildren();
> }
>
> - igt_subtest("invalid-param-set") {
> + igt_subtest("root-set") {
> ctx_param.context = ctx;
> - TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> + ctx_param.value--;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
>
> - igt_subtest("non-root-set") {
> + igt_subtest("non-root-set-no-zeromap") {
> igt_fork(child, 1) {
> igt_drop_root();
>
> @@ -125,13 +135,32 @@ igt_main
> igt_waitchildren();
> }
>
> - igt_subtest("root-set") {
> + igt_subtest("root-set-no-zeromap-enabled") {
> ctx_param.context = ctx;
> TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> - ctx_param.value--;
> + ctx_param.value = 1;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> + }
> +
> + igt_subtest("root-set-no-zeromap-disabled") {
> + ctx_param.context = ctx;
> + TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> + ctx_param.value = 0;
> TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> }
>
> + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP + 1;
> +
> + igt_subtest("invalid-param-get") {
> + ctx_param.context = ctx;
> + TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, EINVAL);
> + }
> +
> + igt_subtest("invalid-param-set") {
> + ctx_param.context = ctx;
> + TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
> + }
> +
> igt_fixture
> close(fd);
> }
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
2015-08-07 13:04 ` Paulo Zanoni
@ 2015-08-10 8:31 ` David Weinehall
0 siblings, 0 replies; 7+ messages in thread
From: David Weinehall @ 2015-08-10 8:31 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
On Fri, Aug 07, 2015 at 10:04:47AM -0300, Paulo Zanoni wrote:
> 2015-08-06 18:33 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
> >
> > The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> > despite our best efforts we really suck at properly reviewing for test
> > coverage when extending ABI.
> >
> > The real bug here is that David Weinhall hasn't submitted updated igts
> > for the NO_ZEROMAP feature yet. Imo the right course of action is to
> > revert that feature if the testcase don't show up within a few days.
> >
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > tests/gem_ctx_param_basic.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> > index 5ff3b13f4c7a..b44b37cf0538 100644
> > --- a/tests/gem_ctx_param_basic.c
> > +++ b/tests/gem_ctx_param_basic.c
> > @@ -98,7 +98,7 @@ igt_main
> > ctx_param.size = 0;
> > }
> >
> > - ctx_param.param = -1;
> > + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
>
> How about adding a comment somewhere "If this breaks it's because we
> extended the number of params without updating IGT. Please add the
> proper tests for the new param"? That will help preventing us from
> making the same error again next year.
Good idea!
Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
2015-08-07 12:53 ` David Weinehall
2015-08-07 13:13 ` Daniel Vetter
@ 2015-08-21 13:26 ` Ander Conselvan De Oliveira
2015-08-24 7:39 ` Ander Conselvan De Oliveira
1 sibling, 1 reply; 7+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-08-21 13:26 UTC (permalink / raw)
To: David Weinehall, Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, 2015-08-07 at 15:53 +0300, David Weinehall wrote:
> On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
> >
> > The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> > despite our best efforts we really suck at properly reviewing for test
> > coverage when extending ABI.
> >
> > The real bug here is that David Weinhall hasn't submitted updated igts
> > for the NO_ZEROMAP feature yet. Imo the right course of action is to
> > revert that feature if the testcase don't show up within a few days.
>
> The reason I never submitted it was probably because of Chris's strong
> opposition to the feature in the first place; I've had the testcase
> laying around on my computer for quite a while.
>
> Anyhow, here's a slightly modified version of that test -- hopefully
> not breaking anything.
>
>
> Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
>
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index bc5d4bd827cf..f4deca6bd79e 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
> uint32_t size;
> uint64_t param;
> #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
> +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
> uint64_t value;
> };
> void gem_context_require_ban_period(int fd);
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index b44b37cf0538..1e7e8ff40703 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -57,7 +57,7 @@ igt_main
> ctx = gem_context_create(fd);
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
>
> igt_subtest("basic") {
> ctx_param.context = ctx;
> @@ -98,21 +98,31 @@ igt_main
[...]
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
>
> - igt_subtest("non-root-set") {
> + igt_subtest("non-root-set-no-zeromap") {
> igt_fork(child, 1) {
> igt_drop_root();
ctx_param.context = ctx;
TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
ctx_param.value--;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
(I've added the code missing from the context)
The code in i915_gem_context_setparam_ioctl() that handles CONTEXT_PARAM_NO_ZEROMAP never returns
EPERM, so this test always fails.
Cheers,
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
2015-08-21 13:26 ` Ander Conselvan De Oliveira
@ 2015-08-24 7:39 ` Ander Conselvan De Oliveira
0 siblings, 0 replies; 7+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-08-24 7:39 UTC (permalink / raw)
To: David Weinehall, Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, 2015-08-21 at 16:26 +0300, Ander Conselvan De Oliveira wrote:
> On Fri, 2015-08-07 at 15:53 +0300, David Weinehall wrote:
> > On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> > > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
> > >
> > > The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> > > despite our best efforts we really suck at properly reviewing for test
> > > coverage when extending ABI.
> > >
> > > The real bug here is that David Weinhall hasn't submitted updated igts
> > > for the NO_ZEROMAP feature yet. Imo the right course of action is to
> > > revert that feature if the testcase don't show up within a few days.
> >
> > The reason I never submitted it was probably because of Chris's strong
> > opposition to the feature in the first place; I've had the testcase
> > laying around on my computer for quite a while.
> >
> > Anyhow, here's a slightly modified version of that test -- hopefully
> > not breaking anything.
> >
> >
> > Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
> >
> > diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> > index bc5d4bd827cf..f4deca6bd79e 100644
> > --- a/lib/ioctl_wrappers.h
> > +++ b/lib/ioctl_wrappers.h
> > @@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
> > uint32_t size;
> > uint64_t param;
> > #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
> > +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
> > uint64_t value;
> > };
> > void gem_context_require_ban_period(int fd);
> > diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> > index b44b37cf0538..1e7e8ff40703 100644
> > --- a/tests/gem_ctx_param_basic.c
> > +++ b/tests/gem_ctx_param_basic.c
> > @@ -57,7 +57,7 @@ igt_main
> > ctx = gem_context_create(fd);
> > }
> >
> > - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> > + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> >
> > igt_subtest("basic") {
> > ctx_param.context = ctx;
> > @@ -98,21 +98,31 @@ igt_main
>
> [...]
>
> > - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> > + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
> >
> > - igt_subtest("non-root-set") {
> > + igt_subtest("non-root-set-no-zeromap") {
> > igt_fork(child, 1) {
> > igt_drop_root();
>
> ctx_param.context = ctx;
> TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
> TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> ctx_param.value--;
> TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
>
> (I've added the code missing from the context)
Except I added the wrong code. Here's what is in i-g-t now:
ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
igt_subtest("non-root-set-no-zeromap") {
igt_fork(child, 1) {
igt_drop_root();
ctx_param.context = ctx;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
ctx_param.value--;
TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
}
igt_waitchildren();
}
> The code in i915_gem_context_setparam_ioctl() that handles CONTEXT_PARAM_NO_ZEROMAP never returns
> EPERM, so this test always fails.
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-24 7:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-06 21:33 [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params" Daniel Vetter
2015-08-07 12:53 ` David Weinehall
2015-08-07 13:13 ` Daniel Vetter
2015-08-21 13:26 ` Ander Conselvan De Oliveira
2015-08-24 7:39 ` Ander Conselvan De Oliveira
2015-08-07 13:04 ` Paulo Zanoni
2015-08-10 8:31 ` David Weinehall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.