[igt-dev] [PATCH i-g-t v6 1/1] tests/i915/gem_ctx_persistence: Set context with supported engines

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-01-30 11:08:47)
> 
> On 30/01/2020 10:55, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2020-01-30 10:52:26)
> >> On 30/01/2020 10:31, Tvrtko Ursulin wrote:
> >>> On 29/01/2020 21:09, Chris Wilson wrote:
> >>>> Quoting Bommu Krishnaiah (2020-01-29 18:10:02)
> >>>>> Update the context with supported engines on the platform with set_property
> >>>>> I915_CONTEXT_PARAM_ENGINES to make sure the work load is submitted to
> >>>>> the available engines only.
> >>>>>
> >>>>> v2: addressed the review comments.
> >>>>> v3: addressed v2 review comments.
> >>>>> V4: Re-introduced the gem_context_set_all_engines() API,test_process_mixed()
> >>>>> test creating new forked process, to set engine map to new forked process
> >>>>> I added gem_context_set_all_engines() API.
> >>>>> v5: addressed v4 review comments.
> >>>>> v6: addressed chris comments.
> >>>>
> >>>> This is totally useless as a recording of the history. Explain why you
> >>>> had to make the changes, for posterity.
> >>>>
> >>>>> -       __for_each_static_engine(e) {
> >>>>> +       for_each_engine(e, i915) {
> >>>>
> >>>> Incorrect change that leads to compiler warnings and prevents the legacy
> >>>> tests being listed by CI.
> >>>>
> >>>> I've given up and fixed it.
> >>
> >> Where's your version Chris?
> > 
> > 21f4d0f72c0a3519270712565b2fe688157fc18b
> 
> I suggest the below delta to a) not add more dependencies on the static e2 list and b) also cover I915_EXEC_BSD. I'll send a patch if you agree.
> 
> diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
> index 8b9633b214ff..4d4b735d4936 100644
> --- a/tests/i915/gem_ctx_persistence.c
> +++ b/tests/i915/gem_ctx_persistence.c
> @@ -759,7 +759,8 @@ static void smoketest(int i915)
>  
>  igt_main
>  {
> -       const struct intel_execution_engine2 *e;
> +       const struct intel_execution_engine *e;
> +       const struct intel_execution_engine2 *e2;
>         int i915;
>  
>         igt_fixture {
> @@ -792,66 +793,68 @@ igt_main
>         igt_subtest("hang")
>                 test_nohangcheck_hang(i915);
>  
> -       __for_each_static_engine(e) {
> +       for (e = intel_execution_engines; e->name; e++) {

I don't mind too much, I just expected the __for_each_static_engine() to
save on typing. But yeah, eb_ring().

But much yuck on e and e2. Can we move them to separate functions to
hide the eyesore?
-Chris