| Message ID | 1456942245-24749-3-git-send-email-derek.j.morton@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 02, 2016 at 06:10:40PM +0000, Derek Morton wrote: > Some platforms have ring BSD available but no BSD2. > Because of the current verification, tests involving ring BSD1 > will be skipped if no BSD2 is available. The code does what exactly what we want. We run BSD or BSD, BSD1, BSD2 (because in this case we have no idea what BSD actually is). -Chris
I need to control exactly which ring each batchbuffer is submitted to if there is more than 1 BSD ring as I need to fill the queues in a known manner. My understanding is that if I submit to BSD when there is BSD1 and BSD2 the kernel will choose which ring it gets submitted to. If this logic breaks other tests I will remove this patch and put the logic in the scheduler test. //Derek -----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Wednesday, March 2, 2016 6:42 PM To: Morton, Derek J <derek.j.morton@intel.com> Cc: intel-gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch Subject: Re: [Intel-gfx] [PATCH i-g-t v2 2/7] lib/ioctl_wrappers: Separate ring BSD1 from BSD2 checks On Wed, Mar 02, 2016 at 06:10:40PM +0000, Derek Morton wrote: > Some platforms have ring BSD available but no BSD2. > Because of the current verification, tests involving ring BSD1 will be > skipped if no BSD2 is available. The code does what exactly what we want. We run BSD or BSD, BSD1, BSD2 (because in this case we have no idea what BSD actually is). -Chris -- Chris Wilson, Intel Open Source Technology Centre
On 02/03/16 18:10, Derek Morton wrote: > Some platforms have ring BSD available but no BSD2. > Because of the current verification, tests involving ring BSD1 > will be skipped if no BSD2 is available. > > Decoupling the checks will allow running the BSD1 specific tests > on these platforms. > > Based on a patch originally submitted by Gabriel Feceoru to > patchwork-hook-test branch. > > Signed-off-by: Derek Morton <derek.j.morton@intel.com> > --- > lib/ioctl_wrappers.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c > index f42e2c9..a4c6aa4 100644 > --- a/lib/ioctl_wrappers.c > +++ b/lib/ioctl_wrappers.c > @@ -1432,8 +1432,10 @@ void gem_require_ring(int fd, int ring_id) > > /* silly ABI, the kernel thinks everyone who has BSD also has BSD2 */ > if ((ring_id & ~(3<<13)) == I915_EXEC_BSD) { > - if (ring_id & (3 << 13)) > + if (ring_id & (2 << 13)) > igt_require(gem_has_bsd2(fd)); > + if (ring_id & (1 << 13)) > + igt_require(gem_has_bsd(fd)); > } > } > Isn't the first (unchanged) line rather unclear? Why not test for (ring_id & I915_EXEC_BSD)? And then you only need to additionally test bit 14. if (ring_id & I915_EXEC_BSD) { /* require at least one BSD ring */ igt_require(gem_has_bsd(fd)); if (ring_id & (2 << 13)) { /* specifically require BSD2 */ igt_require(gem_has_bsd2(fd)); } } .Dave.
On 07/03/16 11:56, Dave Gordon wrote: > On 02/03/16 18:10, Derek Morton wrote: >> Some platforms have ring BSD available but no BSD2. >> Because of the current verification, tests involving ring BSD1 >> will be skipped if no BSD2 is available. >> >> Decoupling the checks will allow running the BSD1 specific tests >> on these platforms. >> >> Based on a patch originally submitted by Gabriel Feceoru to >> patchwork-hook-test branch. >> >> Signed-off-by: Derek Morton <derek.j.morton@intel.com> >> --- >> lib/ioctl_wrappers.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c >> index f42e2c9..a4c6aa4 100644 >> --- a/lib/ioctl_wrappers.c >> +++ b/lib/ioctl_wrappers.c >> @@ -1432,8 +1432,10 @@ void gem_require_ring(int fd, int ring_id) >> >> /* silly ABI, the kernel thinks everyone who has BSD also has >> BSD2 */ >> if ((ring_id & ~(3<<13)) == I915_EXEC_BSD) { >> - if (ring_id & (3 << 13)) >> + if (ring_id & (2 << 13)) >> igt_require(gem_has_bsd2(fd)); >> + if (ring_id & (1 << 13)) >> + igt_require(gem_has_bsd(fd)); >> } >> } >> > > Isn't the first (unchanged) line rather unclear? > Why not test for (ring_id & I915_EXEC_BSD)? > And then you only need to additionally test bit 14. > > if (ring_id & I915_EXEC_BSD) { Oops, no, this should be: if ((ring_id & I915_EXEC_RING_MASK) == I915_EXEC_BSD) { > /* require at least one BSD ring */ > igt_require(gem_has_bsd(fd)); > > if (ring_id & (2 << 13)) { Or better (and symbolically): if ((ring_id & I915_EXEC_BSD_MASK) == I915_EXEC_BSD_RING2) { > /* specifically require BSD2 */ > igt_require(gem_has_bsd2(fd)); > } > } > > .Dave.
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index f42e2c9..a4c6aa4 100644 --- a/lib/ioctl_wrappers.c +++ b/lib/ioctl_wrappers.c @@ -1432,8 +1432,10 @@ void gem_require_ring(int fd, int ring_id) /* silly ABI, the kernel thinks everyone who has BSD also has BSD2 */ if ((ring_id & ~(3<<13)) == I915_EXEC_BSD) { - if (ring_id & (3 << 13)) + if (ring_id & (2 << 13)) igt_require(gem_has_bsd2(fd)); + if (ring_id & (1 << 13)) + igt_require(gem_has_bsd(fd)); } }
Some platforms have ring BSD available but no BSD2. Because of the current verification, tests involving ring BSD1 will be skipped if no BSD2 is available. Decoupling the checks will allow running the BSD1 specific tests on these platforms. Based on a patch originally submitted by Gabriel Feceoru to patchwork-hook-test branch. Signed-off-by: Derek Morton <derek.j.morton@intel.com> --- lib/ioctl_wrappers.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)