[igt-dev] [RFT v4 4/6] igt/i915: Require GTT mapping to be available when needed.

[Chris Wilson]

Quoting Antonio Argenziano (2019-04-11 19:13:13)
> 
> 
> On 27/03/19 14:19, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2019-03-27 21:05:52)
> >>
> >>
> >> On 25/03/19 16:36, Chris Wilson wrote:
> >>> Quoting Antonio Argenziano (2019-03-25 23:20:41)
> >>>> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> >>>> index a9383000..15c8440f 100644
> >>>> --- a/tests/i915/gem_exec_schedule.c
> >>>> +++ b/tests/i915/gem_exec_schedule.c
> >>>> @@ -1236,6 +1236,7 @@ igt_main
> >>>>                           igt_subtest_f("independent-%s", e->name) {
> >>>>                                   igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags));
> >>>>                                   igt_require(gem_can_store_dword(fd, e->exec_id | e->flags));
> >>>> +                               gem_require_mmap_gtt(fd);
> >>>>                                   independent(fd, e->exec_id | e->flags);
> >>>>                           }
> >>>>                   }
> >>>> @@ -1328,8 +1329,10 @@ igt_main
> >>>>                                   igt_subtest_f("wide-%s", e->name)
> >>>>                                           wide(fd, e->exec_id | e->flags);
> >>>>    
> >>>> -                               igt_subtest_f("reorder-wide-%s", e->name)
> >>>> +                               igt_subtest_f("reorder-wide-%s", e->name) {
> >>>> +                                       gem_require_mmap_gtt(fd);
> >>>>                                           reorder_wide(fd, e->exec_id | e->flags);
> >>>> +                               }
> >>>>    
> >>>>                                   igt_subtest_f("smoketest-%s", e->name)
> >>>>                                           smoketest(fd, e->exec_id | e->flags, 5);
> >>>
> >>> Hmm. I would rather the basic scheduling tests remained functioning.
> >>
> >> Didn't we have a convincing argument against using wc + sync?
> > 
> > Just the sync part iirc. So you just need to come up with an alternative
> > solution. What I've used elsewhere is to write the ring timestamp from
> > each batch and verify they are in the order I expect. That should work
> > just fine with anything... (The most complicated thing there is actually
> > determining the engine->mmio_base, and really we'd be better exporting
> > that from the kernel for simplicity.)
> 
> I think I'm missing something, the issue we had was the async update of 
> the single BO we use for all submissions (at least for reorder-wide) 
> needed to go though gtt mapping, wouldn't that still be there even if we 
> change the contents of the batch?

I mean it should be possible to prove the scheduling order without
relying on modifying the batches. The timestamp is a good indicator of
the order, the only question is in setting up the submission to force a
scheduling decision -- and that usually means a fence to delay HW
submission until after all execbufs.

Losing these tests for future gen should be enough motivation to devise
a new set that exercise the same paths through the scheduler without the
drawable of needing async gtt modifications (if it was just async gtt,
async wc should be fine though -- I've forgotten the problem we ran into!).
-Chris