[igt-dev] [PATCH i-g-t] tests/i915/i915_hangman : Skips test for RCS/CCS on DG2

[John Harrison]

On 1/24/2022 10:12, John Harrison wrote:
> On 1/24/2022 06:42, priyanka.dandamudi at intel.com wrote:
>> From: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
>>
>> Updated code to skip for RCS/CCS engines on DG2 as the
>> platform cant handle shared reset domain.
> Is this based on running the test and seeing it fail? Or just thinking 
> that it is a problem?
>
> As per my update to this test in the internal tree, I do not see it 
> fail on the CCS engine only RCS. That also correlates with the nature 
> of the DG2 workaround - if you hang RCS then CCS cannot switch but if 
> you hang CCS then RCS can still switch quite happily. So if this test 
> is failing on CCS in upstream then we have a bug that needs to be fixed.
>
> John.
PS: Meant to add that making the skip is only a short term hack to get 
the CI results green. The correct fix is to update the test to be aware 
of the implications of the workaround. I.e. still run the hang test on 
all engines but, on DG2, expect more than one engine to be reset as 
appropriate to the workaround.

Lastly, just adding a continue to the loop is not the correct way to 
skip a test. That just makes the test vanish with no record that it is 
being skipped. You should use the igt_skip helper. That way, the test is 
still seen to exist can the skip can be tracked.

John.


>
>>
>> Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
>> Cc: Melkaveri, Arjun <arjun.melkaveri at intel.com>
>> ---
>>   tests/i915/i915_hangman.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/i915/i915_hangman.c b/tests/i915/i915_hangman.c
>> index 23055c27..abbdaed5 100644
>> --- a/tests/i915/i915_hangman.c
>> +++ b/tests/i915/i915_hangman.c
>> @@ -445,6 +445,9 @@ static void do_tests(const char *name, const char 
>> *prefix,
>>       snprintf(buff, sizeof(buff), "%s-error-state-capture", prefix);
>>       igt_subtest_with_dynamic(buff) {
>>           for_each_ctx_engine(device, ctx, e) {
>> +            if (IS_DG2(intel_get_drm_devid(device)) &&
>> +                    (e->class == I915_ENGINE_CLASS_RENDER || 
>> e->class == I915_ENGINE_CLASS_COMPUTE))
>> +                                continue;
>>               igt_dynamic_f("%s", e->name)
>>                   test_error_state_capture(ctx, e);
>>           }
>> @@ -466,7 +469,10 @@ static void do_tests(const char *name, const 
>> char *prefix,
>>           igt_require(has_gpu_reset > 1);
>>             for_each_ctx_engine(device, ctx, e) {
>> -            igt_dynamic_f("%s", e->name)
>> +            if (IS_DG2(intel_get_drm_devid(device)) &&
>> +                    (e->class == I915_ENGINE_CLASS_RENDER || 
>> e->class == I915_ENGINE_CLASS_COMPUTE))
>> +                continue;
>> +                igt_dynamic_f("%s", e->name)
>>                   test_engine_hang(ctx, e, 0);
>>           }
>>       }
>> @@ -486,6 +492,9 @@ static void do_tests(const char *name, const char 
>> *prefix,
>>           igt_require(has_gpu_reset > 1);
>>             for_each_ctx_engine(device, ctx, e) {
>> +            if (IS_DG2(intel_get_drm_devid(device)) &&
>> +                    (e->class == I915_ENGINE_CLASS_RENDER || 
>> e->class == I915_ENGINE_CLASS_COMPUTE))
>> +                continue;
>>               igt_dynamic_f("%s", e->name)
>>                   test_engine_hang(ctx, e, IGT_SPIN_INVALID_CS);
>>           }
>