[PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[Chris Wilson]

A purely recursive batch has the downside that it is a severe drain on
system resources (see commit f978cc027cd0 "lib/dummyload: Pad with a few
nops so that we do not completely hog the system") which can result in
the test being starved and failing to make reasonably progress. For more
reliable resets, also include an arbitration point. This should lessen
the efficacy of the hang...

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 lib/igt_gt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 89727d22..2aebad14 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -313,12 +313,12 @@ igt_hang_t igt_hang_ctx(int fd,
 	len = 2;
 	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
 		len++;
-	b[0] = MI_BATCH_BUFFER_START | (len - 2);
-	b[len] = MI_BATCH_BUFFER_END;
-	b[len+1] = MI_NOOP;
-	gem_write(fd, exec.handle, 0, b, sizeof(b));
+	b[0] = 0x5 << 24; /* ARB_CHk */
+	b[1] = MI_BATCH_BUFFER_START | (len - 2);
+	b[len+1] = MI_BATCH_BUFFER_END;
+	gem_write(fd, exec.handle, 4096-sizeof(b), b, sizeof(b));
 
-	reloc.offset = sizeof(uint32_t);
+	reloc.offset = 4096 - sizeof(b) + 2*sizeof(uint32_t);
 	reloc.target_handle = exec.handle;
 	reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
 
-- 
2.15.0.rc2

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

Re: [PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[Ville Syrjälä]

On Fri, Oct 27, 2017 at 01:45:35PM +0100, Chris Wilson wrote:
> A purely recursive batch has the downside that it is a severe drain on
> system resources (see commit f978cc027cd0 "lib/dummyload: Pad with a few
> nops so that we do not completely hog the system") which can result in
> the test being starved and failing to make reasonably progress. For more
> reliable resets, also include an arbitration point. This should lessen
> the efficacy of the hang...
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  lib/igt_gt.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index 89727d22..2aebad14 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -313,12 +313,12 @@ igt_hang_t igt_hang_ctx(int fd,
>  	len = 2;
>  	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>  		len++;
> -	b[0] = MI_BATCH_BUFFER_START | (len - 2);
> -	b[len] = MI_BATCH_BUFFER_END;
> -	b[len+1] = MI_NOOP;
> -	gem_write(fd, exec.handle, 0, b, sizeof(b));
> +	b[0] = 0x5 << 24; /* ARB_CHk */

That seems to be gen4+ only. Also
"This instruction can be placed only in a ring buffer, never in a batch
buffer."

> +	b[1] = MI_BATCH_BUFFER_START | (len - 2);
> +	b[len+1] = MI_BATCH_BUFFER_END;
> +	gem_write(fd, exec.handle, 4096-sizeof(b), b, sizeof(b));
>  
> -	reloc.offset = sizeof(uint32_t);
> +	reloc.offset = 4096 - sizeof(b) + 2*sizeof(uint32_t);
>  	reloc.target_handle = exec.handle;
>  	reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
>  
> -- 
> 2.15.0.rc2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[Chris Wilson]

Quoting Ville Syrjälä (2017-10-27 13:54:49)
> On Fri, Oct 27, 2017 at 01:45:35PM +0100, Chris Wilson wrote:
> > A purely recursive batch has the downside that it is a severe drain on
> > system resources (see commit f978cc027cd0 "lib/dummyload: Pad with a few
> > nops so that we do not completely hog the system") which can result in
> > the test being starved and failing to make reasonably progress. For more
> > reliable resets, also include an arbitration point. This should lessen
> > the efficacy of the hang...
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >  lib/igt_gt.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> > index 89727d22..2aebad14 100644
> > --- a/lib/igt_gt.c
> > +++ b/lib/igt_gt.c
> > @@ -313,12 +313,12 @@ igt_hang_t igt_hang_ctx(int fd,
> >       len = 2;
> >       if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> >               len++;
> > -     b[0] = MI_BATCH_BUFFER_START | (len - 2);
> > -     b[len] = MI_BATCH_BUFFER_END;
> > -     b[len+1] = MI_NOOP;
> > -     gem_write(fd, exec.handle, 0, b, sizeof(b));
> > +     b[0] = 0x5 << 24; /* ARB_CHk */
> 
> That seems to be gen4+ only. Also
> "This instruction can be placed only in a ring buffer, never in a batch
> buffer."

Idea of since/until when? It's definitely listed for batches on gen8+.

The good news being an unrecognised MI cmd it is skipped. Better news if
it causes a GPU hang ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[Chris Wilson]

Quoting Chris Wilson (2017-10-27 13:59:17)
> Quoting Ville Syrjälä (2017-10-27 13:54:49)
> > On Fri, Oct 27, 2017 at 01:45:35PM +0100, Chris Wilson wrote:
> > > A purely recursive batch has the downside that it is a severe drain on
> > > system resources (see commit f978cc027cd0 "lib/dummyload: Pad with a few
> > > nops so that we do not completely hog the system") which can result in
> > > the test being starved and failing to make reasonably progress. For more
> > > reliable resets, also include an arbitration point. This should lessen
> > > the efficacy of the hang...
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > ---
> > >  lib/igt_gt.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> > > index 89727d22..2aebad14 100644
> > > --- a/lib/igt_gt.c
> > > +++ b/lib/igt_gt.c
> > > @@ -313,12 +313,12 @@ igt_hang_t igt_hang_ctx(int fd,
> > >       len = 2;
> > >       if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > >               len++;
> > > -     b[0] = MI_BATCH_BUFFER_START | (len - 2);
> > > -     b[len] = MI_BATCH_BUFFER_END;
> > > -     b[len+1] = MI_NOOP;
> > > -     gem_write(fd, exec.handle, 0, b, sizeof(b));
> > > +     b[0] = 0x5 << 24; /* ARB_CHk */
> > 
> > That seems to be gen4+ only. Also
> > "This instruction can be placed only in a ring buffer, never in a batch
> > buffer."
> 
> Idea of since/until when? It's definitely listed for batches on gen8+.
> 
> The good news being an unrecognised MI cmd it is skipped. Better news if
> it causes a GPU hang ;)

But it certainly doesn't work as an arbitration point on gen6, presuming
that was what we were missing for the reset to function.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for lib/gt: Insert an arbitration point in our hang batch

[Patchwork]

== Series Details ==

Series: lib/gt: Insert an arbitration point in our hang batch
URL   : https://patchwork.freedesktop.org/series/32753/
State : warning

== Summary ==

IGT patchset tested on top of latest successful build
1fc4de1ca390adec9be8bd7cc0c36cab07465959 igt/gem_exec_latency: Wire up an interloper for preemption

with latest DRM-Tip kernel build CI_DRM_3290
795d258e67f4 drm-tip: 2017y-10m-27d-11h-42m-39s UTC integration manifest

No testlist changes.

Test gem_ringfill:
        Subgroup basic-default-hang:
                dmesg-warn -> PASS       (fi-blb-e6850) fdo#101600 +1
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-gdg-551) fdo#102618
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-edid:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-load-detect:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup prune-stale-modes:
                pass       -> SKIP       (fi-ivb-3520m)

fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:450s
fi-blb-e6850     total:289  pass:224  dwarn:0   dfail:0   fail:0   skip:65  time:369s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:524s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:263s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:501s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:498s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:483s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:617s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:422s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:250s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:584s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:482s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:429s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:433s
fi-ivb-3520m     total:289  pass:256  dwarn:0   dfail:0   fail:0   skip:33  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:502s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:570s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:585s
fi-pnv-d510      total:289  pass:223  dwarn:0   dfail:0   fail:0   skip:66  time:541s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:593s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:645s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:461s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:564s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:425s

== Logs ==

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

Re: [PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[Ville Syrjälä]

On Fri, Oct 27, 2017 at 01:59:17PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2017-10-27 13:54:49)
> > On Fri, Oct 27, 2017 at 01:45:35PM +0100, Chris Wilson wrote:
> > > A purely recursive batch has the downside that it is a severe drain on
> > > system resources (see commit f978cc027cd0 "lib/dummyload: Pad with a few
> > > nops so that we do not completely hog the system") which can result in
> > > the test being starved and failing to make reasonably progress. For more
> > > reliable resets, also include an arbitration point. This should lessen
> > > the efficacy of the hang...
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > ---
> > >  lib/igt_gt.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> > > index 89727d22..2aebad14 100644
> > > --- a/lib/igt_gt.c
> > > +++ b/lib/igt_gt.c
> > > @@ -313,12 +313,12 @@ igt_hang_t igt_hang_ctx(int fd,
> > >       len = 2;
> > >       if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > >               len++;
> > > -     b[0] = MI_BATCH_BUFFER_START | (len - 2);
> > > -     b[len] = MI_BATCH_BUFFER_END;
> > > -     b[len+1] = MI_NOOP;
> > > -     gem_write(fd, exec.handle, 0, b, sizeof(b));
> > > +     b[0] = 0x5 << 24; /* ARB_CHk */
> > 
> > That seems to be gen4+ only. Also
> > "This instruction can be placed only in a ring buffer, never in a batch
> > buffer."
> 
> Idea of since/until when? It's definitely listed for batches on gen8+.

If I'm reading things correctly then IVB+ can have it in an RCS batch.
But for the other engines it seems that the "ring only" restriction
still applies, until some *future* gen.

> The good news being an unrecognised MI cmd it is skipped. Better news if
> it causes a GPU hang ;)

Based on the description it could be that it just blindly jumps to UHPTR
and there wouldn't be any way to get back to the batch. Maybe. That's
assuming UHPTR would have the valid bit set in the first place. I guess
if UHPTR isn't valid then nothing should happend whether or not the
parser accepts the command.

I don't actually know how you would get back to the previous point on
the ring either after jumping to UHPTR from the ring. Is the previous
HEAD saved somewhere?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] lib/gt: Insert an arbitration point in our hang batch

[Chris Wilson]

Quoting Ville Syrjälä (2017-10-27 14:24:16)
> On Fri, Oct 27, 2017 at 01:59:17PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2017-10-27 13:54:49)
> > > On Fri, Oct 27, 2017 at 01:45:35PM +0100, Chris Wilson wrote:
> > > > A purely recursive batch has the downside that it is a severe drain on
> > > > system resources (see commit f978cc027cd0 "lib/dummyload: Pad with a few
> > > > nops so that we do not completely hog the system") which can result in
> > > > the test being starved and failing to make reasonably progress. For more
> > > > reliable resets, also include an arbitration point. This should lessen
> > > > the efficacy of the hang...
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > > ---
> > > >  lib/igt_gt.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> > > > index 89727d22..2aebad14 100644
> > > > --- a/lib/igt_gt.c
> > > > +++ b/lib/igt_gt.c
> > > > @@ -313,12 +313,12 @@ igt_hang_t igt_hang_ctx(int fd,
> > > >       len = 2;
> > > >       if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > > >               len++;
> > > > -     b[0] = MI_BATCH_BUFFER_START | (len - 2);
> > > > -     b[len] = MI_BATCH_BUFFER_END;
> > > > -     b[len+1] = MI_NOOP;
> > > > -     gem_write(fd, exec.handle, 0, b, sizeof(b));
> > > > +     b[0] = 0x5 << 24; /* ARB_CHk */
> > > 
> > > That seems to be gen4+ only. Also
> > > "This instruction can be placed only in a ring buffer, never in a batch
> > > buffer."
> > 
> > Idea of since/until when? It's definitely listed for batches on gen8+.
> 
> If I'm reading things correctly then IVB+ can have it in an RCS batch.
> But for the other engines it seems that the "ring only" restriction
> still applies, until some *future* gen.
> 
> > The good news being an unrecognised MI cmd it is skipped. Better news if
> > it causes a GPU hang ;)
> 
> Based on the description it could be that it just blindly jumps to UHPTR
> and there wouldn't be any way to get back to the batch. Maybe. That's
> assuming UHPTR would have the valid bit set in the first place. I guess
> if UHPTR isn't valid then nothing should happend whether or not the
> parser accepts the command.

Fortunately we never set the valid bit for UHPTR. Unfortunately we never
clear it either. (Or set the INSTPM for autoclear). However, because the
gpu often seems to be working, I conclude that bit is rarely randomly
set.
 
> I don't actually know how you would get back to the previous point on
> the ring either after jumping to UHPTR from the ring. Is the previous
> HEAD saved somewhere?

Afaict, it basically bumps RING_HEAD to UHPTR and so continues to
RING_TAIL. (And presumably then jumps back to UHPTR upon hitting that
preemption point unless you manage to clear UHPTR in time.)

But I'm guessing as I haven't successfully used it yet.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx