[Intel-gfx] [PATCH igt] igt/gem_spin_batch: Skip overloading aliased BSD engines

Chris Wilson chris at chris-wilson.co.uk
Thu Dec 21 11:32:18 UTC 2017


Quoting Tvrtko Ursulin (2017-12-21 11:21:23)
> 
> On 21/12/2017 10:53, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-12-21 10:02:06)
> >>
> >> On 20/12/2017 17:56, Chris Wilson wrote:
> >>> BSD == BSD1 or BSD2. Since we already emit spinners to the explicit BSD
> >>> rins, skip the aliased ring.
> >>>
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104352
> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>> ---
> >>>    tests/gem_spin_batch.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/tests/gem_spin_batch.c b/tests/gem_spin_batch.c
> >>> index 896311304..cccba75a7 100644
> >>> --- a/tests/gem_spin_batch.c
> >>> +++ b/tests/gem_spin_batch.c
> >>> @@ -77,7 +77,7 @@ static void spin_on_all_engines(int fd, unsigned int timeout_sec)
> >>>        unsigned engine;
> >>>    
> >>>        for_each_engine(fd, engine) {
> >>> -             if (engine == 0)
> >>> +             if (engine == 0 || engine == I915_EXEC_BSD)
> >>
> >> You forget the other annoyance of the VCS selection uAPI where explicit
> >> flags can only be used on dual-VCS platforms? :) So I think you need to
> >> skip on engine & I915_EXEC_BSD_RING1, unless I am missing something.
> > 
> > No way, the uapi can't be that stupid. It is.
> 
> Ugh my suggestion was even incorrect. The skipping criteria needs to be 
> branched based on HAS_BSD2.
> 
> if (HAS_BSD2())
>         skip I915_EXEC_BSD
> else
>         skip I915_EXEC_BSD_RING1
> 
> Can I mention again my suggestion of making for_each_engine iterate 
> engines, and not uABI flags?
> 
> That would solve multiple issues with one big swat. Maybe it would add 
> some new ones, like if we miss some ABI testing coverage, but I still 
> think exercising engines and exercising ABI is better separated.
> 
> > Can we do the s/for_each_engine/for_each_ring/ yet?
> 
> Doesn't ring a bell - what is the name change supposed to signal?

I care about uABI coverage. As I recall the^Wmy plan was when we have an
interface that allows for explicit engine selection, we use it. But that
means we first need to convert all existing loops over to
for_each_exec_ring(), and then introduce for_each_engine() converting
and adding extra tests as required depending on whether the tests are
exercising the uABI itself or only care about HW internals.
-Chris


More information about the Intel-gfx mailing list