| Message ID | 20170223155757.10316-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris Wilson <chris@chris-wilson.co.uk> writes: > We are required to reload the TLBs around ppgtt switches. However, we > already do an unconditional TLB invalidate before every batch and a flush > afterwards, so this condition is already satisfied without extra flushes > around the LRI instructions. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ----------------- > 1 file changed, 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 057239ab3f70..999f15455f48 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1431,13 +1431,8 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, > { > struct intel_engine_cs *engine = req->engine; > u32 *cs; > - int ret; > > /* NB: TLBs must be flushed and invalidated before a switch */ > - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); The pattern slightly changes as you dont get the flush on the first switch. But what is there to flush on first switch anyways. Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> > - if (ret) > - return ret; > - > cs = intel_ring_begin(req, 6); > if (IS_ERR(cs)) > return PTR_ERR(cs); > @@ -1458,13 +1453,8 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, > { > struct intel_engine_cs *engine = req->engine; > u32 *cs; > - int ret; > > /* NB: TLBs must be flushed and invalidated before a switch */ > - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); > - if (ret) > - return ret; > - > cs = intel_ring_begin(req, 6); > if (IS_ERR(cs)) > return PTR_ERR(cs); > @@ -1477,13 +1467,6 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, > *cs++ = MI_NOOP; > intel_ring_advance(req, cs); > > - /* XXX: RCS is the only one to auto invalidate the TLBs? */ > - if (engine->id != RCS) { > - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); > - if (ret) > - return ret; > - } > - > return 0; > } > > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Feb 24, 2017 at 11:47:56AM +0200, Mika Kuoppala wrote: > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > We are required to reload the TLBs around ppgtt switches. However, we > > already do an unconditional TLB invalidate before every batch and a flush > > afterwards, so this condition is already satisfied without extra flushes > > around the LRI instructions. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ----------------- > > 1 file changed, 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 057239ab3f70..999f15455f48 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -1431,13 +1431,8 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, > > { > > struct intel_engine_cs *engine = req->engine; > > u32 *cs; > > - int ret; > > > > /* NB: TLBs must be flushed and invalidated before a switch */ > > - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); > > The pattern slightly changes as you dont get the flush on the first > switch. > > But what is there to flush on first switch anyways. Those flushes onto the empty ring have always left me questioning my sanity ;) -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 057239ab3f70..999f15455f48 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1431,13 +1431,8 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, { struct intel_engine_cs *engine = req->engine; u32 *cs; - int ret; /* NB: TLBs must be flushed and invalidated before a switch */ - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); - if (ret) - return ret; - cs = intel_ring_begin(req, 6); if (IS_ERR(cs)) return PTR_ERR(cs); @@ -1458,13 +1453,8 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, { struct intel_engine_cs *engine = req->engine; u32 *cs; - int ret; /* NB: TLBs must be flushed and invalidated before a switch */ - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); - if (ret) - return ret; - cs = intel_ring_begin(req, 6); if (IS_ERR(cs)) return PTR_ERR(cs); @@ -1477,13 +1467,6 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, *cs++ = MI_NOOP; intel_ring_advance(req, cs); - /* XXX: RCS is the only one to auto invalidate the TLBs? */ - if (engine->id != RCS) { - ret = engine->emit_flush(req, EMIT_INVALIDATE | EMIT_FLUSH); - if (ret) - return ret; - } - return 0; }
We are required to reload the TLBs around ppgtt switches. However, we already do an unconditional TLB invalidate before every batch and a flush afterwards, so this condition is already satisfied without extra flushes around the LRI instructions. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ----------------- 1 file changed, 17 deletions(-)