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

Chris Wilson
2019-01-30T09:49:21Z

Quoting Tvrtko Ursulin (2019-01-30 08:11:05)
>
> 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.

More accurately I was too lazy to implement a require vfunc with a data
struct.
-Chris