All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-27 13:06 ` Chris Wilson
  0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-27 13:06 ` Chris Wilson
  0 siblings, 0 replies; 26+ 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

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for i915/gem_workarounds: Require GPU resets
  2019-01-27 13:06 ` [igt-dev] " Chris Wilson
  (?)
@ 2019-01-27 13:50 ` Patchwork
  -1 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-01-27 13:50 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915/gem_workarounds: Require GPU resets
URL   : https://patchwork.freedesktop.org/series/55794/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5490 -> IGTPW_2299
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55794/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2299 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       PASS -> DMESG-WARN [fdo#108965]

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#108767]

  
#### Possible fixes ####

  * igt@gem_exec_reloc@basic-gtt-noreloc:
    - fi-skl-guc:         {SKIP} [fdo#109271] -> PASS +81

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (43 -> 36)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-glk-j4005 fi-gdg-551 fi-icl-y 


Build changes
-------------

    * IGT: IGT_4790 -> IGTPW_2299

  CI_DRM_5490: 310d38b4b51e06ef7096716430e2ef262c3e45fe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2299: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2299/
  IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2299/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for i915/gem_workarounds: Require GPU resets
  2019-01-27 13:06 ` [igt-dev] " Chris Wilson
  (?)
  (?)
@ 2019-01-27 15:46 ` Patchwork
  -1 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-01-27 15:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915/gem_workarounds: Require GPU resets
URL   : https://patchwork.freedesktop.org/series/55794/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5490_full -> IGTPW_2299_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2299_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2299_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55794/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2299_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_eio@in-flight-immediate:
    - shard-kbl:          NOTRUN -> DMESG-FAIL

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@runner@aborted}:
    - shard-kbl:          ( 4 FAIL ) -> ( 5 FAIL )

  
Known issues
------------

  Here are the changes found in IGTPW_2299_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-blt:
    - shard-glk:          NOTRUN -> FAIL [fdo#103158]
    - shard-apl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_exec_schedule@pi-ringfull-bsd2:
    - shard-kbl:          NOTRUN -> FAIL [fdo#103158] +1

  * igt@i915_suspend@shrink:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#109244]
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#109244]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956] +1
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_color@pipe-b-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]
    - shard-kbl:          PASS -> FAIL [fdo#108147]

  * igt@kms_color@pipe-c-legacy-gamma:
    - shard-kbl:          PASS -> FAIL [fdo#104782]
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          PASS -> FAIL [fdo#103232] +3
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-kbl:          NOTRUN -> INCOMPLETE [fdo#103665]

  * igt@kms_cursor_crc@cursor-64x64-random:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          NOTRUN -> FAIL [fdo#105454] / [fdo#106509]

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled:
    - shard-glk:          PASS -> FAIL [fdo#108222]

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-kbl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-glk:          PASS -> FAIL [fdo#103166] +7

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@i915_selftest@live_requests:
    - shard-apl:          DMESG-FAIL -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_color@pipe-b-degamma:
    - shard-kbl:          FAIL [fdo#104782] -> PASS
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-onscreen:
    - shard-glk:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-kbl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +4

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_setmode@basic:
    - shard-hsw:          FAIL [fdo#99912] -> PASS

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-kbl:          FAIL [fdo#105010] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105454]: https://bugs.freedesktop.org/show_bug.cgi?id=105454
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108222]: https://bugs.freedesktop.org/show_bug.cgi?id=108222
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4790 -> IGTPW_2299
    * Piglit: piglit_4509 -> None

  CI_DRM_5490: 310d38b4b51e06ef7096716430e2ef262c3e45fe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2299: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2299/
  IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2299/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-27 13:06 ` [igt-dev] " Chris Wilson
@ 2019-01-28 11:03   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 11:03   ` Tvrtko Ursulin
  0 siblings, 0 replies; 26+ 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:03   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-01-28 11:07     ` Chris Wilson
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 11:07     ` Chris Wilson
  0 siblings, 0 replies; 26+ 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:07     ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-01-28 11:12       ` Chris Wilson
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 11:12       ` Chris Wilson
  0 siblings, 0 replies; 26+ 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-27 13:06 ` [igt-dev] " Chris Wilson
@ 2019-01-28 11:23   ` Chris Wilson
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 11:23   ` Chris Wilson
  0 siblings, 0 replies; 26+ 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

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for i915/gem_workarounds: Require GPU resets (rev2)
  2019-01-27 13:06 ` [igt-dev] " Chris Wilson
                   ` (4 preceding siblings ...)
  (?)
@ 2019-01-28 11:58 ` Patchwork
  -1 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-01-28 11:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915/gem_workarounds: Require GPU resets (rev2)
URL   : https://patchwork.freedesktop.org/series/55794/
State : success

== Summary ==

CI Bug Log - changes from IGT_4792 -> IGTPW_2303
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55794/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2303 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@amdgpu/amd_prime@amd-to-i915:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#107341]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_busy@basic-flip-c:
    - fi-kbl-7500u:       {SKIP} [fdo#109271] / [fdo#109278] -> PASS +2

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-kbl-7500u:       DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105602] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (43 -> 41)
------------------------------

  Additional (2): fi-icl-y fi-glk-j4005 
  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-------------

    * IGT: IGT_4792 -> IGTPW_2303

  CI_DRM_5492: bc2fcdf82a5b94f015126dfa9db03f88feb9ae17 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2303: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2303/
  IGT_4792: e061af7bc37bfc77893dae8dab93d712d39d18df @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2303/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:12       ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-01-28 13:44         ` Tvrtko Ursulin
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 13:44         ` Tvrtko Ursulin
  0 siblings, 0 replies; 26+ 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

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

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 11:23   ` [igt-dev] " Chris Wilson
@ 2019-01-28 13:47     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 13:47     ` Tvrtko Ursulin
  0 siblings, 0 replies; 26+ 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 13:44         ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-01-28 13:47           ` Chris Wilson
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-28 13:47           ` Chris Wilson
  0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for i915/gem_workarounds: Require GPU resets (rev2)
  2019-01-27 13:06 ` [igt-dev] " Chris Wilson
                   ` (5 preceding siblings ...)
  (?)
@ 2019-01-28 15:21 ` Patchwork
  -1 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-01-28 15:21 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915/gem_workarounds: Require GPU resets (rev2)
URL   : https://patchwork.freedesktop.org/series/55794/
State : success

== Summary ==

CI Bug Log - changes from IGT_4792_full -> IGTPW_2303_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55794/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2303_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-bsd1:
    - shard-kbl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_wait@write-busy-blt:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-hsw:          NOTRUN -> DMESG-WARN [fdo#107956]
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]
    - shard-kbl:          PASS -> FAIL [fdo#107725] / [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_cursor_crc@cursor-64x21-onscreen:
    - shard-apl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          NOTRUN -> FAIL [fdo#105454] / [fdo#106509]

  * igt@kms_flip@blocking-absolute-wf_vblank:
    - shard-kbl:          NOTRUN -> INCOMPLETE [fdo#103665]

  * igt@kms_invalid_dotclock:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#109373]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590] +1

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@kms_busy@extended-pageflip-hang-newfb-render-a:
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS +3
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +4

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS +1

  * igt@kms_universal_plane@universal-plane-pipe-c-functional:
    - shard-glk:          FAIL [fdo#103166] -> PASS +4

  * igt@perf@oa-exponents:
    - shard-glk:          FAIL [fdo#105483] -> PASS

  * igt@perf_pmu@busy-check-all-rcs0:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          {SKIP} [fdo#109271] -> PASS

  * igt@prime_busy@hang-bsd:
    - shard-hsw:          FAIL [fdo#108807] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105454]: https://bugs.freedesktop.org/show_bug.cgi?id=105454
  [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108807]: https://bugs.freedesktop.org/show_bug.cgi?id=108807
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109467]: https://bugs.freedesktop.org/show_bug.cgi?id=109467
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4792 -> IGTPW_2303

  CI_DRM_5492: bc2fcdf82a5b94f015126dfa9db03f88feb9ae17 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2303: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2303/
  IGT_4792: e061af7bc37bfc77893dae8dab93d712d39d18df @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2303/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-28 13:47     ` [igt-dev] " Tvrtko Ursulin
@ 2019-01-29 18:57       ` Chris Wilson
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-29 18:57       ` Chris Wilson
  0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-29 18:57       ` [Intel-gfx] " Chris Wilson
@ 2019-01-30  8:11         ` Tvrtko Ursulin
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-30  8:11         ` Tvrtko Ursulin
  0 siblings, 0 replies; 26+ 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


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

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
  2019-01-30  8:11         ` [igt-dev] " Tvrtko Ursulin
@ 2019-01-30  9:49           ` Chris Wilson
  -1 siblings, 0 replies; 26+ 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] 26+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Require GPU resets
@ 2019-01-30  9:49           ` Chris Wilson
  0 siblings, 0 replies; 26+ 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-01-30  9:49 UTC | newest]

Thread overview: 26+ 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-27 13:06 ` [igt-dev] " Chris Wilson
2019-01-27 13:50 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-01-27 15:46 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2019-01-28 11:03 ` [PATCH i-g-t] " Tvrtko Ursulin
2019-01-28 11:03   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-01-28 11:07   ` Chris Wilson
2019-01-28 11:07     ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-01-28 11:12     ` Chris Wilson
2019-01-28 11:12       ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-01-28 13:44       ` Tvrtko Ursulin
2019-01-28 13:44         ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-01-28 13:47         ` Chris Wilson
2019-01-28 13:47           ` [Intel-gfx] " Chris Wilson
2019-01-28 11:23 ` Chris Wilson
2019-01-28 11:23   ` [igt-dev] " Chris Wilson
2019-01-28 13:47   ` Tvrtko Ursulin
2019-01-28 13:47     ` [igt-dev] " Tvrtko Ursulin
2019-01-29 18:57     ` Chris Wilson
2019-01-29 18:57       ` [Intel-gfx] " Chris Wilson
2019-01-30  8:11       ` Tvrtko Ursulin
2019-01-30  8:11         ` [igt-dev] " Tvrtko Ursulin
2019-01-30  9:49         ` Chris Wilson
2019-01-30  9:49           ` [igt-dev] " Chris Wilson
2019-01-28 11:58 ` [igt-dev] ✓ Fi.CI.BAT: success for i915/gem_workarounds: Require GPU resets (rev2) Patchwork
2019-01-28 15:21 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.