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

[Tvrtko Ursulin]

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++) {
+               unsigned int flags = eb_ring(e);
+
                igt_subtest_group {
                        igt_fixture {
-                               gem_require_ring(i915, e->flags);
+                               gem_require_ring(i915, flags);
                                gem_require_contexts(i915);
                        }
 
                        igt_subtest_f("legacy-%s-persistence", e->name)
-                               test_persistence(i915, e->flags);
+                               test_persistence(i915, flags);
 
                        igt_subtest_f("legacy-%s-cleanup", e->name)
-                               test_nonpersistent_cleanup(i915, e->flags);
+                               test_nonpersistent_cleanup(i915, flags);
 
                        igt_subtest_f("legacy-%s-queued", e->name)
-                               test_nonpersistent_queued(i915, e->flags);
+                               test_nonpersistent_queued(i915, flags);
 
                        igt_subtest_f("legacy-%s-mixed", e->name)
-                               test_nonpersistent_mixed(i915, e->flags);
+                               test_nonpersistent_mixed(i915, flags);
 
                        igt_subtest_f("legacy-%s-mixed-process", e->name)
-                               test_process_mixed(i915, e->flags);
+                               test_process_mixed(i915, flags);
 
                        igt_subtest_f("legacy-%s-hostile", e->name)
-                               test_nonpersistent_hostile(i915, e->flags);
+                               test_nonpersistent_hostile(i915, flags);
 
                        igt_subtest_f("legacy-%s-hostile-preempt", e->name)
                                test_nonpersistent_hostile_preempt(i915,
-                                                                  e->flags);
+                                                                  flags);
                }
        }
 
-        __for_each_physical_engine(i915, e) {
+        __for_each_physical_engine(i915, e2) {
                 igt_subtest_group {
                         igt_fixture
                                 gem_require_contexts(i915);
 
-                       igt_subtest_f("%s-persistence", e->name)
-                               test_persistence(i915, e->flags);
+                       igt_subtest_f("%s-persistence", e2->name)
+                               test_persistence(i915, e2->flags);
 
-                       igt_subtest_f("%s-cleanup", e->name)
-                               test_nonpersistent_cleanup(i915, e->flags);
+                       igt_subtest_f("%s-cleanup", e2->name)
+                               test_nonpersistent_cleanup(i915, e2->flags);
 
-                       igt_subtest_f("%s-queued", e->name)
-                               test_nonpersistent_queued(i915, e->flags);
+                       igt_subtest_f("%s-queued", e2->name)
+                               test_nonpersistent_queued(i915, e2->flags);
 
-                       igt_subtest_f("%s-mixed", e->name)
-                               test_nonpersistent_mixed(i915, e->flags);
+                       igt_subtest_f("%s-mixed", e2->name)
+                               test_nonpersistent_mixed(i915, e2->flags);
 
-                       igt_subtest_f("%s-mixed-process", e->name)
-                               test_process_mixed(i915, e->flags);
+                       igt_subtest_f("%s-mixed-process", e2->name)
+                               test_process_mixed(i915, e2->flags);
 
-                       igt_subtest_f("%s-hostile", e->name)
-                               test_nonpersistent_hostile(i915, e->flags);
+                       igt_subtest_f("%s-hostile", e2->name)
+                               test_nonpersistent_hostile(i915, e2->flags);
 
-                       igt_subtest_f("%s-hostile-preempt", e->name)
+                       igt_subtest_f("%s-hostile-preempt", e2->name)
                                test_nonpersistent_hostile_preempt(i915,
-                                                                  e->flags);
+                                                                  e2->flags);
 
-                       igt_subtest_f("%s-hang", e->name)
-                               test_nonpersistent_hang(i915, e->flags);
+                       igt_subtest_f("%s-hang", e2->name)
+                               test_nonpersistent_hang(i915, e2->flags);
                }
        }
 


Regards,

Tvrtko