| Message ID | 20180911115810.8917-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/26] drm/i915/ringbuffer: Reload PDs harder on byt/bcs | expand |
Quoting Chris Wilson (2018-09-11 14:57:45) > 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(-) > > 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); This what they call black magic. Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
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);
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(-)