public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-27 13:06 Chris Wilson
  2019-01-28 11:03 ` Tvrtko Ursulin
  2019-01-28 11:23 ` Chris Wilson
  0 siblings, 2 replies; 11+ messages in thread
From: Chris Wilson @ 2019-01-27 13:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

Check that we are allowed to reset the GPU prior to execution.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_workarounds.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
index 78478ad2c..0e9ab2386 100644
--- a/tests/i915/gem_workarounds.c
+++ b/tests/i915/gem_workarounds.c
@@ -192,7 +192,11 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 
 	switch (op) {
 	case GPU_RESET:
-		igt_force_gpu_reset(fd);
+		{
+			igt_hang_t hang = igt_allow_hang(fd, ctx, 0);
+			igt_force_gpu_reset(fd);
+			igt_disallow_hang(fd, hang);
+		}
 		break;
 
 	case SUSPEND_RESUME:
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-27 13:06 [PATCH i-g-t] i915/gem_workarounds: Require GPU resets Chris Wilson
@ 2019-01-28 11:03 ` Tvrtko Ursulin
  2019-01-28 11:07   ` Chris Wilson
  2019-01-28 11:23 ` Chris Wilson
  1 sibling, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-01-28 11:03 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 27/01/2019 13:06, Chris Wilson wrote:
> Check that we are allowed to reset the GPU prior to execution.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/i915/gem_workarounds.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> index 78478ad2c..0e9ab2386 100644
> --- a/tests/i915/gem_workarounds.c
> +++ b/tests/i915/gem_workarounds.c
> @@ -192,7 +192,11 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>   
>   	switch (op) {
>   	case GPU_RESET:
> -		igt_force_gpu_reset(fd);
> +		{
> +			igt_hang_t hang = igt_allow_hang(fd, ctx, 0);
> +			igt_force_gpu_reset(fd);
> +			igt_disallow_hang(fd, hang);
> +		}
>   		break;
>   
>   	case SUSPEND_RESUME:
> 

If it doesn't make sense to add the checks into igt_force_gpu_reset (so 
force means force), should we have igt_try_gpu_reset to avoid having to 
wrap it everywhere?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:03 ` Tvrtko Ursulin
@ 2019-01-28 11:07   ` Chris Wilson
  2019-01-28 11:12     ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-01-28 11:07 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-01-28 11:03:30)
> 
> On 27/01/2019 13:06, Chris Wilson wrote:
> > Check that we are allowed to reset the GPU prior to execution.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   tests/i915/gem_workarounds.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> > index 78478ad2c..0e9ab2386 100644
> > --- a/tests/i915/gem_workarounds.c
> > +++ b/tests/i915/gem_workarounds.c
> > @@ -192,7 +192,11 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
> >   
> >       switch (op) {
> >       case GPU_RESET:
> > -             igt_force_gpu_reset(fd);
> > +             {
> > +                     igt_hang_t hang = igt_allow_hang(fd, ctx, 0);
> > +                     igt_force_gpu_reset(fd);
> > +                     igt_disallow_hang(fd, hang);
> > +             }
> >               break;
> >   
> >       case SUSPEND_RESUME:
> > 
> 
> If it doesn't make sense to add the checks into igt_force_gpu_reset (so 
> force means force), should we have igt_try_gpu_reset to avoid having to 
> wrap it everywhere?

Ugh. igt_try_gpu_reset_with_many_many_options_depending_on_test().

I like my requires as high up in the chain as possible, I've been bitten
too many times by hiding them.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:07   ` Chris Wilson
@ 2019-01-28 11:12     ` Chris Wilson
  2019-01-28 13:44       ` Tvrtko Ursulin
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-01-28 11:12 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Chris Wilson (2019-01-28 11:07:40)
> Quoting Tvrtko Ursulin (2019-01-28 11:03:30)
> > 
> > On 27/01/2019 13:06, Chris Wilson wrote:
> > > Check that we are allowed to reset the GPU prior to execution.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >   tests/i915/gem_workarounds.c | 6 +++++-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> > > index 78478ad2c..0e9ab2386 100644
> > > --- a/tests/i915/gem_workarounds.c
> > > +++ b/tests/i915/gem_workarounds.c
> > > @@ -192,7 +192,11 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
> > >   
> > >       switch (op) {
> > >       case GPU_RESET:
> > > -             igt_force_gpu_reset(fd);
> > > +             {
> > > +                     igt_hang_t hang = igt_allow_hang(fd, ctx, 0);
> > > +                     igt_force_gpu_reset(fd);
> > > +                     igt_disallow_hang(fd, hang);
> > > +             }
> > >               break;
> > >   
> > >       case SUSPEND_RESUME:
> > > 
> > 
> > If it doesn't make sense to add the checks into igt_force_gpu_reset (so 
> > force means force), should we have igt_try_gpu_reset to avoid having to 
> > wrap it everywhere?
> 
> Ugh. igt_try_gpu_reset_with_many_many_options_depending_on_test().
> 
> I like my requires as high up in the chain as possible, I've been bitten
> too many times by hiding them.

The key benefit from doing it higher up, is that we should be doing it
as early as possible and should be more descriptive about why.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-27 13:06 [PATCH i-g-t] i915/gem_workarounds: Require GPU resets Chris Wilson
  2019-01-28 11:03 ` Tvrtko Ursulin
@ 2019-01-28 11:23 ` Chris Wilson
  2019-01-28 13:47   ` Tvrtko Ursulin
  1 sibling, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-01-28 11:23 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

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;
+				}
+			}
 		}
 	}
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:12     ` Chris Wilson
@ 2019-01-28 13:44       ` Tvrtko Ursulin
  2019-01-28 13:47         ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-01-28 13:44 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 28/01/2019 11:12, Chris Wilson wrote:
> Quoting Chris Wilson (2019-01-28 11:07:40)
>> Quoting Tvrtko Ursulin (2019-01-28 11:03:30)
>>>
>>> On 27/01/2019 13:06, Chris Wilson wrote:
>>>> Check that we are allowed to reset the GPU prior to execution.
>>>>
>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> ---
>>>>    tests/i915/gem_workarounds.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
>>>> index 78478ad2c..0e9ab2386 100644
>>>> --- a/tests/i915/gem_workarounds.c
>>>> +++ b/tests/i915/gem_workarounds.c
>>>> @@ -192,7 +192,11 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>>>>    
>>>>        switch (op) {
>>>>        case GPU_RESET:
>>>> -             igt_force_gpu_reset(fd);
>>>> +             {
>>>> +                     igt_hang_t hang = igt_allow_hang(fd, ctx, 0);
>>>> +                     igt_force_gpu_reset(fd);
>>>> +                     igt_disallow_hang(fd, hang);
>>>> +             }
>>>>                break;
>>>>    
>>>>        case SUSPEND_RESUME:
>>>>
>>>
>>> If it doesn't make sense to add the checks into igt_force_gpu_reset (so
>>> force means force), should we have igt_try_gpu_reset to avoid having to
>>> wrap it everywhere?
>>
>> Ugh. igt_try_gpu_reset_with_many_many_options_depending_on_test().
>>
>> I like my requires as high up in the chain as possible, I've been bitten
>> too many times by hiding them.
> 
> The key benefit from doing it higher up, is that we should be doing it
> as early as possible and should be more descriptive about why.

Agreed on that, but worry who will notice it in review.

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:23 ` Chris Wilson
@ 2019-01-28 13:47   ` Tvrtko Ursulin
  2019-01-29 18:57     ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-01-28 13:47 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


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(...)


?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 13:44       ` Tvrtko Ursulin
@ 2019-01-28 13:47         ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-01-28 13:47 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-01-28 13:44:47)
> 
> On 28/01/2019 11:12, Chris Wilson wrote:
> Agreed on that, but worry who will notice it in review.

For the next week of struct_mutex removal (haha), CI will tell us.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 13:47   ` Tvrtko Ursulin
@ 2019-01-29 18:57     ` Chris Wilson
  2019-01-30  8:11       ` Tvrtko Ursulin
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-01-29 18:57 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

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.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-29 18:57     ` Chris Wilson
@ 2019-01-30  8:11       ` Tvrtko Ursulin
  2019-01-30  9:49         ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-01-30  8:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


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


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-30  8:11       ` Tvrtko Ursulin
@ 2019-01-30  9:49         ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-01-30  9:49 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-30  9:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-27 13:06 [PATCH i-g-t] i915/gem_workarounds: Require GPU resets Chris Wilson
2019-01-28 11:03 ` Tvrtko Ursulin
2019-01-28 11:07   ` Chris Wilson
2019-01-28 11:12     ` Chris Wilson
2019-01-28 13:44       ` Tvrtko Ursulin
2019-01-28 13:47         ` Chris Wilson
2019-01-28 11:23 ` Chris Wilson
2019-01-28 13:47   ` Tvrtko Ursulin
2019-01-29 18:57     ` Chris Wilson
2019-01-30  8:11       ` Tvrtko Ursulin
2019-01-30  9:49         ` Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox