diff mbox series

drm/i915/ringbuffer: Reload PDs harder on byt/bcs

Message ID 20180910130808.10809-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915/ringbuffer: Reload PDs harder on byt/bcs | expand

Commit Message

Chris Wilson Sept. 10, 2018, 1:08 p.m. UTC
Baytrail takes a little more convincing that it needs to actually reload
its Page Directoy (ppGTT) before the context switch, so repeat it until
it gets the message. Once again the arbitrary values here are
empirically derived.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861
Testcase: igt/gem_exec_parallel/fds
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Chris Wilson Sept. 11, 2018, 7:43 a.m. UTC | #1
Quoting Chris Wilson (2018-09-10 14:08:08)
> Baytrail takes a little more convincing that it needs to actually reload
> its Page Directoy (ppGTT) before the context switch, so repeat it until
> it gets the message. Once again the arbitrary values here are
> empirically derived.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861
> Testcase: igt/gem_exec_parallel/fds
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

On drm-tip, this testcase fails within 15 iterations (30s). It has now
survived 24 hours. I think it've proved itself against this one testcase
and deserves a shot at the rest.
-Chris
Chris Wilson Sept. 12, 2018, 7:40 a.m. UTC | #2
Quoting Chris Wilson (2018-09-11 08:43:32)
> Quoting Chris Wilson (2018-09-10 14:08:08)
> > Baytrail takes a little more convincing that it needs to actually reload
> > its Page Directoy (ppGTT) before the context switch, so repeat it until
> > it gets the message. Once again the arbitrary values here are
> > empirically derived.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861
> > Testcase: igt/gem_exec_parallel/fds
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> On drm-tip, this testcase fails within 15 iterations (30s). It has now
> survived 24 hours. I think it've proved itself against this one testcase
> and deserves a shot at the rest.

48 hours. Please kindly ack.
-Chris
Chris Wilson Sept. 12, 2018, 9:49 a.m. UTC | #3
Quoting Chris Wilson (2018-09-10 14:08:08)
> Baytrail takes a little more convincing that it needs to actually reload
> its Page Directoy (ppGTT) before the context switch, so repeat it until
> it gets the message. Once again the arbitrary values here are
> empirically derived.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861
> Testcase: igt/gem_exec_parallel/fds
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

From another thread,

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

Ta,
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 472939f5c18f..d0ef50bf930a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1677,9 +1677,26 @@  static int switch_context(struct i915_request *rq)
 	GEM_BUG_ON(HAS_EXECLISTS(rq->i915));
 
 	if (ppgtt) {
-		ret = load_pd_dir(rq, ppgtt);
-		if (ret)
-			goto err;
+		int loops;
+
+		/*
+		 * Baytail takes a little more convincing that it really needs
+		 * to reload the PD between contexts. It is not just a little
+		 * longer, as adding more stalls after the load_pd_dir (i.e.
+		 * adding a long loop around flush_pd_dir) is not as effective
+		 * as reloading the PD umpteen times. 32 is derived from
+		 * experimentation (gem_exec_parallel/fds) and has no good
+		 * explanation.
+		 */
+		loops = 1;
+		if (engine->id == BCS && IS_VALLEYVIEW(engine->i915))
+			loops = 32;
+
+		do {
+			ret = load_pd_dir(rq, ppgtt);
+			if (ret)
+				goto err;
+		} while (--loops);
 
 		if (intel_engine_flag(engine) & ppgtt->pd_dirty_rings) {
 			unwind_mm = intel_engine_flag(engine);