* [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush
@ 2018-09-04 6:38 Chris Wilson
2018-09-04 6:45 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Chris Wilson @ 2018-09-04 6:38 UTC (permalink / raw)
To: intel-gfx
Continuing the fun of trying to find exactly the delay that is
sufficient to ensure that the page directory is fully loaded between
context switches, move the extra flush added in commit 70b73f9ac113
("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs") to just
after we flush the pd. Entirely based on the empirical data of running
failing tests in a loop until we survive a day (before the mtbf is 10-30
minutes).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107769
References: 70b73f9ac113 ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 40 ++++++++++++++-----------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 86604dd1c5a5..472939f5c18f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1707,9 +1707,29 @@ static int switch_context(struct i915_request *rq)
}
if (ppgtt) {
+ ret = engine->emit_flush(rq, EMIT_INVALIDATE);
+ if (ret)
+ goto err_mm;
+
ret = flush_pd_dir(rq);
if (ret)
goto err_mm;
+
+ /*
+ * Not only do we need a full barrier (post-sync write) after
+ * invalidating the TLBs, but we need to wait a little bit
+ * longer. Whether this is merely delaying us, or the
+ * subsequent flush is a key part of serialising with the
+ * post-sync op, this extra pass appears vital before a
+ * mm switch!
+ */
+ ret = engine->emit_flush(rq, EMIT_INVALIDATE);
+ if (ret)
+ goto err_mm;
+
+ ret = engine->emit_flush(rq, EMIT_FLUSH);
+ if (ret)
+ goto err_mm;
}
if (ctx->remap_slice) {
@@ -1947,7 +1967,7 @@ static void gen6_bsd_submit_request(struct i915_request *request)
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
}
-static int emit_mi_flush_dw(struct i915_request *rq, u32 flags)
+static int mi_flush_dw(struct i915_request *rq, u32 flags)
{
u32 cmd, *cs;
@@ -1985,23 +2005,7 @@ static int emit_mi_flush_dw(struct i915_request *rq, u32 flags)
static int gen6_flush_dw(struct i915_request *rq, u32 mode, u32 invflags)
{
- int err;
-
- /*
- * Not only do we need a full barrier (post-sync write) after
- * invalidating the TLBs, but we need to wait a little bit
- * longer. Whether this is merely delaying us, or the
- * subsequent flush is a key part of serialising with the
- * post-sync op, this extra pass appears vital before a
- * mm switch!
- */
- if (mode & EMIT_INVALIDATE) {
- err = emit_mi_flush_dw(rq, invflags);
- if (err)
- return err;
- }
-
- return emit_mi_flush_dw(rq, 0);
+ return mi_flush_dw(rq, mode & EMIT_INVALIDATE ? invflags : 0);
}
static int gen6_bsd_ring_flush(struct i915_request *rq, u32 mode)
--
2.19.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/ringbuffer: Move double invalidate to after pd flush
2018-09-04 6:38 [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush Chris Wilson
@ 2018-09-04 6:45 ` Patchwork
2018-09-04 7:03 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-09-04 6:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/ringbuffer: Move double invalidate to after pd flush
URL : https://patchwork.freedesktop.org/series/49104/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
fd054884982c drm/i915/ringbuffer: Move double invalidate to after pd flush
-:15: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15:
References: 70b73f9ac113 ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs")
-:15: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 70b73f9ac113 ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs")'
#15:
References: 70b73f9ac113 ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs")
total: 1 errors, 1 warnings, 0 checks, 61 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/ringbuffer: Move double invalidate to after pd flush
2018-09-04 6:38 [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush Chris Wilson
2018-09-04 6:45 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-09-04 7:03 ` Patchwork
2018-09-04 8:21 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-04 13:22 ` [PATCH] " Mika Kuoppala
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-09-04 7:03 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/ringbuffer: Move double invalidate to after pd flush
URL : https://patchwork.freedesktop.org/series/49104/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4761 -> Patchwork_10077 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/49104/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10077 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@amdgpu/amd_basic@userptr:
fi-kbl-8809g: PASS -> INCOMPLETE (fdo#107402)
igt@drv_selftest@live_guc:
fi-skl-guc: NOTRUN -> DMESG-WARN (fdo#107258)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
igt@pm_rpm@basic-pci-d3-state:
fi-glk-dsi: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@pm_rpm@module-reload:
fi-cnl-psr: PASS -> WARN (fdo#107708, fdo#107602)
==== Possible fixes ====
igt@drv_getparams_basic@basic-eu-total:
fi-kbl-7560u: INCOMPLETE -> PASS
igt@gem_exec_suspend@basic-s4-devices:
fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-a:
fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS
igt@kms_psr@primary_page_flip:
fi-cnl-psr: FAIL (fdo#107336) -> PASS
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258
fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
fdo#107708 https://bugs.freedesktop.org/show_bug.cgi?id=107708
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (50 -> 47) ==
Additional (1): fi-skl-guc
Missing (4): fi-ctg-p8600 fi-ilk-m540 fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4761 -> Patchwork_10077
CI_DRM_4761: 0c30a7d8aa491fa8f35dc04e2e1ccb73b9c30ff6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4622: 022be555443eaa3317da6a9a451cf2c9dfcd6ab8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10077: fd054884982cfa47a5295476d0a8addb633e4fd0 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
fd054884982c drm/i915/ringbuffer: Move double invalidate to after pd flush
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10077/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915/ringbuffer: Move double invalidate to after pd flush
2018-09-04 6:38 [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush Chris Wilson
2018-09-04 6:45 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-09-04 7:03 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-09-04 8:21 ` Patchwork
2018-09-04 13:22 ` [PATCH] " Mika Kuoppala
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-09-04 8:21 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/ringbuffer: Move double invalidate to after pd flush
URL : https://patchwork.freedesktop.org/series/49104/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4761_full -> Patchwork_10077_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10077_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10077_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10077_full:
=== IGT changes ===
==== Warnings ====
igt@perf_pmu@rc6:
shard-kbl: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_10077_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_suspend@basic-s3:
shard-kbl: PASS -> DMESG-WARN (fdo#103313)
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023)
igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
shard-hsw: PASS -> FAIL (fdo#105767)
igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-msflip-blt:
shard-glk: PASS -> FAIL (fdo#103167)
igt@perf@blocking:
shard-hsw: PASS -> FAIL (fdo#102252)
==== Possible fixes ====
igt@kms_ccs@pipe-b-bad-pixel-format:
shard-apl: DMESG-WARN (fdo#103558, fdo#105602) -> PASS +1
igt@kms_cursor_crc@cursor-64x64-random:
shard-kbl: FAIL (fdo#103232) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-kbl: FAIL (fdo#105363, fdo#102887) -> PASS
igt@kms_setmode@basic:
shard-kbl: FAIL (fdo#99912) -> PASS
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4761 -> Patchwork_10077
CI_DRM_4761: 0c30a7d8aa491fa8f35dc04e2e1ccb73b9c30ff6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4622: 022be555443eaa3317da6a9a451cf2c9dfcd6ab8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10077: fd054884982cfa47a5295476d0a8addb633e4fd0 @ 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_10077/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush
2018-09-04 6:38 [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush Chris Wilson
` (2 preceding siblings ...)
2018-09-04 8:21 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-09-04 13:22 ` Mika Kuoppala
2018-09-04 13:27 ` Chris Wilson
3 siblings, 1 reply; 6+ messages in thread
From: Mika Kuoppala @ 2018-09-04 13:22 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Continuing the fun of trying to find exactly the delay that is
> sufficient to ensure that the page directory is fully loaded between
> context switches, move the extra flush added in commit 70b73f9ac113
> ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs") to just
> after we flush the pd. Entirely based on the empirical data of running
> failing tests in a loop until we survive a day (before the mtbf is 10-30
> minutes).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107769
> References: 70b73f9ac113 ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 40 ++++++++++++++-----------
> 1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 86604dd1c5a5..472939f5c18f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1707,9 +1707,29 @@ static int switch_context(struct i915_request *rq)
> }
>
> if (ppgtt) {
> + ret = engine->emit_flush(rq, EMIT_INVALIDATE);
> + if (ret)
> + goto err_mm;
> +
> ret = flush_pd_dir(rq);
> if (ret)
> goto err_mm;
> +
> + /*
> + * Not only do we need a full barrier (post-sync write) after
> + * invalidating the TLBs, but we need to wait a little bit
> + * longer. Whether this is merely delaying us, or the
> + * subsequent flush is a key part of serialising with the
> + * post-sync op, this extra pass appears vital before a
> + * mm switch!
> + */
> + ret = engine->emit_flush(rq, EMIT_INVALIDATE);
> + if (ret)
> + goto err_mm;
> +
> + ret = engine->emit_flush(rq, EMIT_FLUSH);
> + if (ret)
> + goto err_mm;
Someone said that proof is in the pudding. Just could
be more fun if someone would show us the recipe.
Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> }
>
> if (ctx->remap_slice) {
> @@ -1947,7 +1967,7 @@ static void gen6_bsd_submit_request(struct i915_request *request)
> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> }
>
> -static int emit_mi_flush_dw(struct i915_request *rq, u32 flags)
> +static int mi_flush_dw(struct i915_request *rq, u32 flags)
> {
> u32 cmd, *cs;
>
> @@ -1985,23 +2005,7 @@ static int emit_mi_flush_dw(struct i915_request *rq, u32 flags)
>
> static int gen6_flush_dw(struct i915_request *rq, u32 mode, u32 invflags)
> {
> - int err;
> -
> - /*
> - * Not only do we need a full barrier (post-sync write) after
> - * invalidating the TLBs, but we need to wait a little bit
> - * longer. Whether this is merely delaying us, or the
> - * subsequent flush is a key part of serialising with the
> - * post-sync op, this extra pass appears vital before a
> - * mm switch!
> - */
> - if (mode & EMIT_INVALIDATE) {
> - err = emit_mi_flush_dw(rq, invflags);
> - if (err)
> - return err;
> - }
> -
> - return emit_mi_flush_dw(rq, 0);
> + return mi_flush_dw(rq, mode & EMIT_INVALIDATE ? invflags : 0);
> }
>
> static int gen6_bsd_ring_flush(struct i915_request *rq, u32 mode)
> --
> 2.19.0.rc1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush
2018-09-04 13:22 ` [PATCH] " Mika Kuoppala
@ 2018-09-04 13:27 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-09-04 13:27 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2018-09-04 14:22:17)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Continuing the fun of trying to find exactly the delay that is
> > sufficient to ensure that the page directory is fully loaded between
> > context switches, move the extra flush added in commit 70b73f9ac113
> > ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs") to just
> > after we flush the pd. Entirely based on the empirical data of running
> > failing tests in a loop until we survive a day (before the mtbf is 10-30
> > minutes).
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107769
> > References: 70b73f9ac113 ("drm/i915/ringbuffer: Delay after invalidating gen6+ xcs")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 40 ++++++++++++++-----------
> > 1 file changed, 22 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 86604dd1c5a5..472939f5c18f 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1707,9 +1707,29 @@ static int switch_context(struct i915_request *rq)
> > }
> >
> > if (ppgtt) {
> > + ret = engine->emit_flush(rq, EMIT_INVALIDATE);
> > + if (ret)
> > + goto err_mm;
> > +
> > ret = flush_pd_dir(rq);
> > if (ret)
> > goto err_mm;
> > +
> > + /*
> > + * Not only do we need a full barrier (post-sync write) after
> > + * invalidating the TLBs, but we need to wait a little bit
> > + * longer. Whether this is merely delaying us, or the
> > + * subsequent flush is a key part of serialising with the
> > + * post-sync op, this extra pass appears vital before a
> > + * mm switch!
> > + */
> > + ret = engine->emit_flush(rq, EMIT_INVALIDATE);
> > + if (ret)
> > + goto err_mm;
> > +
> > + ret = engine->emit_flush(rq, EMIT_FLUSH);
> > + if (ret)
> > + goto err_mm;
>
> Someone said that proof is in the pudding. Just could
> be more fun if someone would show us the recipe.
Hah, in the Coca-Cola Company two people know the secret, we are much
more secure than that!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-04 13:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-04 6:38 [PATCH] drm/i915/ringbuffer: Move double invalidate to after pd flush Chris Wilson
2018-09-04 6:45 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-09-04 7:03 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-04 8:21 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-04 13:22 ` [PATCH] " Mika Kuoppala
2018-09-04 13:27 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox