Discussion:
[Intel-gfx] [PATCH] drm/i915: Flush GPU relocs harder for gen3
Chris Wilson
2018-12-07 13:40:37 UTC
Permalink
Adding an extra MI_STORE_DWORD_IMM to the gpu relocation path for gen3
was good, but still not good enough. To survive 24+ hours under test we
needed to perform not one, not two but three extra store-dw. Doing so
for each GPU relocation was a little unsightly and since we need to
worry about userspace hitting the same issues, we should apply the dummy
store-dw into the EMIT_FLUSH.

Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
References: 7fa28e146994 ("drm/i915: Write GPU relocs harder with gen3")
Testcase: igt/gem_tiled_fence_blits # blb/pnv
Signed-off-by: Chris Wilson <***@chris-wilson.co.uk>
Cc: Joonas Lahtinen <***@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +------
drivers/gpu/drm/i915/intel_ringbuffer.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 10a4afb4f235..786d719e652d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1268,7 +1268,7 @@ relocate_entry(struct i915_vma *vma,
else if (gen >= 4)
len = 4;
else
- len = 6;
+ len = 3;

batch = reloc_gpu(eb, vma, len);
if (IS_ERR(batch))
@@ -1309,11 +1309,6 @@ relocate_entry(struct i915_vma *vma,
*batch++ = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
*batch++ = addr;
*batch++ = target_offset;
-
- /* And again for good measure (blb/pnv) */
- *batch++ = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
- *batch++ = addr;
- *batch++ = target_offset;
}

goto out;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 74a4d587c312..02f6a9b81083 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -69,19 +69,28 @@ unsigned int intel_ring_update_space(struct intel_ring *ring)
static int
gen2_render_ring_flush(struct i915_request *rq, u32 mode)
{
+ unsigned int num_store_dw;
u32 cmd, *cs;

cmd = MI_FLUSH;
-
+ num_store_dw = 0;
if (mode & EMIT_INVALIDATE)
cmd |= MI_READ_FLUSH;
+ if (mode & EMIT_FLUSH)
+ num_store_dw = 4;

- cs = intel_ring_begin(rq, 2);
+ cs = intel_ring_begin(rq, 2 + 3 * num_store_dw);
if (IS_ERR(cs))
return PTR_ERR(cs);

*cs++ = cmd;
- *cs++ = MI_NOOP;
+ while (num_store_dw--) {
+ *cs++ = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
+ *cs++ = i915_scratch_offset(rq->i915);
+ *cs++ = 0;
+ }
+ *cs++ = MI_FLUSH | MI_NO_WRITE_FLUSH;
+
intel_ring_advance(rq, cs);

return 0;
--
2.20.0.rc2
Joonas Lahtinen
2018-12-07 14:08:57 UTC
Permalink
Quoting Chris Wilson (2018-12-07 15:40:37)
Post by Chris Wilson
Adding an extra MI_STORE_DWORD_IMM to the gpu relocation path for gen3
was good, but still not good enough. To survive 24+ hours under test we
needed to perform not one, not two but three extra store-dw. Doing so
for each GPU relocation was a little unsightly and since we need to
worry about userspace hitting the same issues, we should apply the dummy
store-dw into the EMIT_FLUSH.
Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
References: 7fa28e146994 ("drm/i915: Write GPU relocs harder with gen3")
Testcase: igt/gem_tiled_fence_blits # blb/pnv
As we're not going to be adding read-only scratch pages for Gen2 (I
think it's a few generations too old to have it :P), this is:

Reviewed-by: Joonas Lahtinen <***@linux.intel.com>

Regards, Joonas
Post by Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +------
drivers/gpu/drm/i915/intel_ringbuffer.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 10a4afb4f235..786d719e652d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1268,7 +1268,7 @@ relocate_entry(struct i915_vma *vma,
else if (gen >= 4)
len = 4;
else
- len = 6;
+ len = 3;
batch = reloc_gpu(eb, vma, len);
if (IS_ERR(batch))
@@ -1309,11 +1309,6 @@ relocate_entry(struct i915_vma *vma,
*batch++ = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
*batch++ = addr;
*batch++ = target_offset;
-
- /* And again for good measure (blb/pnv) */
- *batch++ = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
- *batch++ = addr;
- *batch++ = target_offset;
}
goto out;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 74a4d587c312..02f6a9b81083 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -69,19 +69,28 @@ unsigned int intel_ring_update_space(struct intel_ring *ring)
static int
gen2_render_ring_flush(struct i915_request *rq, u32 mode)
{
+ unsigned int num_store_dw;
u32 cmd, *cs;
cmd = MI_FLUSH;
-
+ num_store_dw = 0;
if (mode & EMIT_INVALIDATE)
cmd |= MI_READ_FLUSH;
+ if (mode & EMIT_FLUSH)
+ num_store_dw = 4;
- cs = intel_ring_begin(rq, 2);
+ cs = intel_ring_begin(rq, 2 + 3 * num_store_dw);
if (IS_ERR(cs))
return PTR_ERR(cs);
*cs++ = cmd;
- *cs++ = MI_NOOP;
+ while (num_store_dw--) {
+ *cs++ = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
+ *cs++ = i915_scratch_offset(rq->i915);
+ *cs++ = 0;
+ }
+ *cs++ = MI_FLUSH | MI_NO_WRITE_FLUSH;
+
intel_ring_advance(rq, cs);
return 0;
--
2.20.0.rc2
Patchwork
2018-12-07 15:53:20 UTC
Permalink
== Series Details ==

Series: drm/i915: Flush GPU relocs harder for gen3
URL : https://patchwork.freedesktop.org/series/53751/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f17c6fbe1857 drm/i915: Flush GPU relocs harder for gen3
-:14: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 7fa28e146994 ("drm/i915: Write GPU relocs harder with gen3")'
#14:
References: 7fa28e146994 ("drm/i915: Write GPU relocs harder with gen3")

total: 1 errors, 0 warnings, 0 checks, 50 lines checked
Patchwork
2018-12-07 16:11:35 UTC
Permalink
== Series Details ==

Series: drm/i915: Flush GPU relocs harder for gen3
URL : https://patchwork.freedesktop.org/series/53751/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5287 -> Patchwork_11050
====================================================

Summary
-------

**SUCCESS**

No regressions found.

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

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

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

### IGT changes ###

#### Possible regressions ####

* ***@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- {fi-icl-u3}: PASS -> INCOMPLETE


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

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

### IGT changes ###

#### Issues hit ####

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


#### Possible fixes ####

* ***@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: DMESG-WARN [fdo#102614] -> PASS


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

[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718


Participating hosts (51 -> 45)
------------------------------

Additional (1): fi-glk-dsi
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y


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

* Linux: CI_DRM_5287 -> Patchwork_11050

CI_DRM_5287: 1f240056ad2a4b4b4f174f652202dd7d9e7d0fed @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11050: f17c6fbe1857e4d105505c4c0261d0cf2f2e69c3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f17c6fbe1857 drm/i915: Flush GPU relocs harder for gen3

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11050/
Patchwork
2018-12-07 23:40:25 UTC
Permalink
== Series Details ==

Series: drm/i915: Flush GPU relocs harder for gen3
URL : https://patchwork.freedesktop.org/series/53751/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5287_full -> Patchwork_11050_full
====================================================

Summary
-------

**SUCCESS**

No regressions found.



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

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

### IGT changes ###

#### Issues hit ####

* ***@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
- shard-skl: PASS -> FAIL [fdo#107815] / [fdo#108470]

* ***@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#107956] +1

* ***@kms_chv_cursor_fail@pipe-c-128x128-left-edge:
- shard-skl: NOTRUN -> FAIL [fdo#104671]

* ***@kms_cursor_crc@cursor-128x128-onscreen:
- shard-skl: NOTRUN -> FAIL [fdo#103232]

* ***@kms_cursor_crc@cursor-128x128-random:
- shard-glk: PASS -> FAIL [fdo#103232]

* ***@kms_cursor_crc@cursor-128x128-suspend:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#104108]
- {shard-iclb}: NOTRUN -> FAIL [fdo#103232]

* ***@kms_cursor_crc@cursor-256x85-offscreen:
- shard-skl: PASS -> FAIL [fdo#103232]

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

* ***@kms_cursor_crc@cursor-64x64-suspend:
- shard-apl: PASS -> FAIL [fdo#103191] / [fdo#103232]

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

* ***@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-move:
- shard-glk: PASS -> FAIL [fdo#103167]

* ***@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt:
- {shard-iclb}: PASS -> FAIL [fdo#103167] +2

* ***@kms_frontbuffer_tracking@fbcpsr-stridechange:
- shard-skl: NOTRUN -> FAIL [fdo#105683]

* ***@kms_panel_fitting@legacy:
- shard-skl: NOTRUN -> FAIL [fdo#105456]

* ***@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- {shard-iclb}: PASS -> INCOMPLETE [fdo#107713]

* {***@kms_plane@pixel-format-pipe-b-planes-source-clamping}:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#106885]

* ***@kms_plane@pixel-format-pipe-c-planes:
- shard-glk: PASS -> FAIL [fdo#103166]

* {***@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
- shard-apl: PASS -> FAIL [fdo#108948]

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

* ***@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
- shard-skl: NOTRUN -> FAIL [fdo#108145] +1

* ***@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-apl: PASS -> FAIL [fdo#103166]

* {***@kms_rotation_crc@multiplane-rotation-cropping-top}:
- shard-kbl: PASS -> DMESG-FAIL [fdo#108950]

* ***@perf@blocking:
- shard-hsw: PASS -> FAIL [fdo#102252]

* ***@pm_rpm@gem-pread:
- shard-skl: PASS -> INCOMPLETE [fdo#107807]

* ***@pm_rpm@modeset-lpsp-stress:
- {shard-iclb}: PASS -> DMESG-WARN [fdo#108654]

* ***@pm_rpm@modeset-lpsp-stress-no-wait:
- {shard-iclb}: PASS -> DMESG-WARN [fdo#107724]

* {***@runner@aborted}:
- {shard-iclb}: NOTRUN -> FAIL [fdo#108654]


#### Possible fixes ####

* ***@gem_exec_nop@basic-series:
- shard-snb: INCOMPLETE [fdo#105411] -> PASS

* ***@kms_busy@basic-modeset-a:
- shard-apl: DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +8

* ***@kms_cursor_legacy@cursora-vs-flipa-atomic-transitions-varying-size:
- {shard-iclb}: DMESG-WARN [fdo#107724] -> PASS +13

* ***@kms_cursor_legacy@pipe-c-single-move:
- shard-skl: INCOMPLETE -> PASS

* ***@kms_draw_crc@draw-method-rgb565-render-untiled:
- {shard-iclb}: WARN [fdo#108336] -> PASS +3

* ***@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-pgflip-blt:
- {shard-iclb}: DMESG-FAIL [fdo#107720] / [fdo#107724] -> PASS +1

* ***@kms_frontbuffer_tracking@fbc-suspend:
- {shard-iclb}: DMESG-FAIL [fdo#107724] -> PASS +4
- shard-snb: DMESG-WARN [fdo#102365] -> PASS

* ***@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
- shard-apl: FAIL [fdo#108145] -> PASS

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

* ***@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: FAIL [fdo#107815] -> PASS

* ***@kms_plane_multiple@atomic-pipe-a-tiling-y:
- {shard-iclb}: FAIL [fdo#103166] -> PASS +1

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

* ***@kms_rotation_crc@sprite-rotation-180:
- {shard-iclb}: DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +5

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

* ***@pm_rpm@modeset-non-lpsp-stress-no-wait:
- shard-skl: INCOMPLETE [fdo#107807] -> SKIP


#### Warnings ####

* ***@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render:
- {shard-iclb}: DMESG-FAIL [fdo#107724] -> FAIL [fdo#103167]

* ***@kms_plane_multiple@atomic-pipe-b-tiling-x:
- {shard-iclb}: DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103166]


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

[fdo#102252]: https://bugs.freedesktop.org/show_bug.cgi?id=102252
[fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
[fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107720]: https://bugs.freedesktop.org/show_bug.cgi?id=107720
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
[fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
[fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
[fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
[fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

No changes in participating hosts


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

* Linux: CI_DRM_5287 -> Patchwork_11050

CI_DRM_5287: 1f240056ad2a4b4b4f174f652202dd7d9e7d0fed @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11050: f17c6fbe1857e4d105505c4c0261d0cf2f2e69c3 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11050/
Loading...