"Barbalho, Rafael" <rafael.barbalho@xxxxxxxxx> writes: >> -----Original Message----- >> From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >> Sent: Thursday, May 21, 2015 4:08 PM >> To: Mika Kuoppala >> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; miku@xxxxxx; Barbalho, Rafael >> Subject: Re: [PATCH 02/20] drm/i915: Force PD restore on dirty >> ppGTTs >> >> On Thu, May 21, 2015 at 05:37:30PM +0300, Mika Kuoppala wrote: >> > Force page directory reload when ppgtt va->pa >> > mapping has changed. Extend dirty rings mechanism >> > for gen > 7 and use it to force pd restore in execlist >> > mode when vm has been changed. >> > >> > Some parts of execlist context update cleanup based on >> > work by Chris Wilson. >> > >> > v2: Add comment about lite restore (Chris) >> > >> > Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/intel_lrc.c | 65 ++++++++++++++++++++------------- >> ------- >> > 1 file changed, 33 insertions(+), 32 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c >> b/drivers/gpu/drm/i915/intel_lrc.c >> > index 0413b8f..5ee2a8c 100644 >> > --- a/drivers/gpu/drm/i915/intel_lrc.c >> > +++ b/drivers/gpu/drm/i915/intel_lrc.c >> > @@ -264,9 +264,10 @@ u32 intel_execlists_ctx_id(struct >> drm_i915_gem_object *ctx_obj) >> > } >> > >> > static uint64_t execlists_ctx_descriptor(struct intel_engine_cs *ring, >> > - struct drm_i915_gem_object >> *ctx_obj) >> > + struct intel_context *ctx) >> > { >> > struct drm_device *dev = ring->dev; >> > + struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; >> > uint64_t desc; >> > uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj); >> > >> > @@ -284,6 +285,14 @@ static uint64_t execlists_ctx_descriptor(struct >> intel_engine_cs *ring, >> > * signalling between Command Streamers */ >> > /* desc |= GEN8_CTX_FORCE_RESTORE; */ >> > >> > + /* When performing a LiteRestore but with updated PD we need >> > + * to force the GPU to reload the PD >> > + */ >> > + if (intel_ring_flag(ring) & ctx->ppgtt->pd_dirty_rings) { >> > + desc |= GEN8_CTX_FORCE_PD_RESTORE; >> >> Wasn't there a hardware issue which basically meant you are not >> allowed to actually set this bit? >> >> Rafael had some details on that as far as I recall so adding cc... > > Ville is correct, there is a hardware issue in CHV with this bit and it should > not be set. On BDW I am not sure, although you can stop the pre-fetching > & caching in BDW by using 64-bit PPGTT addressing. > > So it's no from me I'm afraid. Thanks for pointing out the details on this. I worked around this with preallocating our top level PDP structure (4 pages with 32bit mode) so that hardware sees immutable top level structure per context. Ville also noticed that initializing scratch structures by copying is not good as it introduces unnecessary read. Better to just fill with pte/pde entries always. This triggered a rebase on rest of the series so I will resend whole series. -Mika > >> >> > + ctx->ppgtt->pd_dirty_rings &= ~intel_ring_flag(ring); >> > + } >> > + > > <Snip> > >> > -- >> > 1.9.1 >> > >> > _______________________________________________ >> > Intel-gfx mailing list >> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Ville Syrjälä >> Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
