Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 29/01/2019 18:57, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-28 13:47:02)
>>
>> On 28/01/2019 11:23, Chris Wilson wrote:
>>> Check that we are allowed to reset the GPU prior to execution.
>>>
>>> v2: Push the require checking up into a subgroup
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    tests/i915/gem_workarounds.c | 29 ++++++++++++++++++++++++++---
>>>    1 file changed, 26 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
>>> index 78478ad2c..44e3dce8a 100644
>>> --- a/tests/i915/gem_workarounds.c
>>> +++ b/tests/i915/gem_workarounds.c
>>> @@ -282,9 +282,32 @@ igt_main
>>>        }
>>>    
>>>        for (op = ops; op->name; op++) {
>>> -             for (m = modes; m->name; m++) {
>>> -                     igt_subtest_f("%s%s", op->name, m->name)
>>> -                             check_workarounds(device, op->op, m->flags);
>>> +             igt_subtest_group {
>>> +                     igt_hang_t hang = {};
>>> +
>>> +                     igt_fixture {
>>> +                             switch (op->op) {
>>> +                             case GPU_RESET:
>>> +                                     hang = igt_allow_hang(device, 0, 0);
>>> +                                     break;
>>> +                             default:
>>> +                                     break;
>>> +                             }
>>> +                     }
>>> +
>>> +                     for (m = modes; m->name; m++)
>>> +                             igt_subtest_f("%s%s", op->name, m->name)
>>> +                                     check_workarounds(device, op->op, m->flags);
>>> +
>>> +                     igt_fixture {
>>> +                             switch (op->op) {
>>> +                             case GPU_RESET:
>>> +                                     igt_disallow_hang(device, hang);
>>> +                                     break;
>>> +                             default:
>>> +                                     break;
>>> +                             }
>>> +                     }
>>>                }
>>>        }
>>>    }
>>>
>>
>> Why the verbose switch and not just:
>>
>>          it (op->op == GPU_RESET)
>>                  hand = igt_allow_hang(...)
> 
> It matched the lower level and I thought would be easier to extend in
> future.

Okay I guess.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev