[Intel-gfx] [PATCH igt v2] lib: Skip aliased bsd ABI ring if bsd2 is available

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-02-21 13:35:38)
> 
> On 21/02/2018 13:07, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-02-21 12:55:17)
> >>
> >> On 21/02/2018 12:53, Tvrtko Ursulin wrote:
> >>>
> >>> On 21/02/2018 12:17, Chris Wilson wrote:
> >>>> How much do I want this uABI to rot away? Say "Never again!" to implicit
> >>>> aliasing.
> >>>>
> >>>> In the meantime, we do not need to perform duplicate work on bsd2
> >>>> machines, as especially we do not know which engine bsd relates to.
> >>>>
> >>>> v2: When in doubt, shout!
> >>>>
> >>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>>> ---
> >>>>    lib/ioctl_wrappers.c | 2 +-
> >>>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> >>>> index 8748cfcf..b9b86079 100644
> >>>> --- a/lib/ioctl_wrappers.c
> >>>> +++ b/lib/ioctl_wrappers.c
> >>>> @@ -1460,7 +1460,7 @@ bool gem_has_ring(int fd, unsigned ring)
> >>>>        /* silly ABI, the kernel thinks everyone who has BSD also has
> >>>> BSD2 */
> >>>>        if ((ring & ~(3<<13)) == I915_EXEC_BSD) {
> >>>> -        if (ring & (3 << 13) && !gem_has_bsd2(fd))
> >>>> +        if (!(ring & (3 << 13)) ^ !gem_has_bsd2(fd))
> >>>>                return false;
> >>>>        }
> >>>
> >>> If default BSD (1)
> >>>       and no BSD2 -> 1 ^ 1 = 1 OK
> >>
> >> Oops
> >> 1 ^ 1 = 0 so also bad
> >>
> >>>       and BSD2 -> 1 ^ 0 = 1 BAD
> >>>
> >>> If explicit BSD (0)
> >>>       and no BSD2 -> 0 ^ 1 = 1 BAD
> >>>       has BSD2 -> 0 ^ 0 = 0 = BAD
> >>>
> >>> Maybe I'm confused.. please simplify the statement? :)
> > 
> > return false?
> > 
> > default BSD ^ has-bsd2
> > 0             0       -> 0 OK
> > 0             1       -> 1 SKIP
> > 1             0       -> 1 SKIP
> > 1             1       -> 0 OK
> > 
> > So we only use bsd (implicit) on single BSD engine machines, and only
> > use bsd0,bsd1 (explicit) on dual engine machines.
> 
> Don't get in your notation why is 0 OK and 1 SKIP. 0 is false = no ring 
> = skip.
> 
> default BSD ^   has-bsd2
> 0               0               0 ^ !0 = 1 has engine - WRONG
> 0               1               0 ^ !1 = 0 no engine - WRONG
> 1               0               1 ^ !0 = 0 no engine - WRONG
> 1               1               1 ^ !1 = 1 has engine - WRONG
> 
> 
> default BSD ^   has-bsd2
> 0               0               0 ^ 0 = 0 no engine - OK
> 0               1               0 ^ 1 = 1 has engine - OK
> 1               0               1 ^ 0 = 1 has engine - OK
> 1               1               1 ^ 1 = 0 no engine - OK
> 
> So previous version was OK!?

Now I am confused, v2 is the one that appears to work.

Anyway, I remember why it is like this. The intention is to iterate the
ABI not the physical engines, I keep meaning to do
s/for_each_engine/for_each_uabi_ring/ and then for_each_physical_engine
that skips the aliased default and bsd (and whatever the future may
bring).

Bring on class/instance.
-Chris