| Message ID | 1426521492-4315-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 16, 2015 at 05:58:12PM +0200, Mika Kuoppala wrote: > Sometimes when first batch is run with blitter ring, > @@ -931,7 +931,6 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, > intel_ring_emit(ring, RING_PP_DIR_BASE(ring)); > intel_ring_emit(ring, get_pd_offset(ppgtt)); > intel_ring_emit(ring, MI_NOOP); > - intel_ring_advance(ring); > > /* XXX: RCS is the only one to auto invalidate the TLBs? */ > if (ring->id != RCS) { > @@ -940,6 +939,8 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, > return ret; > } > > + __intel_ring_advance(ring); > + I really would like a comment before each of these, something like /* XXX This papers over a bug loading PD prior to batch execution */ This last chunk is buggy. The earlier unadvanced return will break the ringbuffer state tracking. Do you have equal success just with the plain s/intel_ring_advance/__intel_ring_advance/ ? -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Mon, Mar 16, 2015 at 05:58:12PM +0200, Mika Kuoppala wrote: >> Sometimes when first batch is run with blitter ring, >> @@ -931,7 +931,6 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, >> intel_ring_emit(ring, RING_PP_DIR_BASE(ring)); >> intel_ring_emit(ring, get_pd_offset(ppgtt)); >> intel_ring_emit(ring, MI_NOOP); >> - intel_ring_advance(ring); >> >> /* XXX: RCS is the only one to auto invalidate the TLBs? */ >> if (ring->id != RCS) { >> @@ -940,6 +939,8 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, >> return ret; >> } >> >> + __intel_ring_advance(ring); >> + > > I really would like a comment before each of these, something like > /* XXX This papers over a bug loading PD prior to batch execution */ > > This last chunk is buggy. The earlier unadvanced return will break the > ringbuffer state tracking. Do you have equal success just with the plain > s/intel_ring_advance/__intel_ring_advance/ ? I have managed to create a series that doesn't need this tail update trickery at all. Please ignore this series. -Mika
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 2034f7c..bbfcbb6 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -390,7 +390,7 @@ static int gen8_write_pdp(struct intel_engine_cs *ring, unsigned entry, intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, GEN8_RING_PDP_LDW(ring, entry)); intel_ring_emit(ring, (u32)(val)); - intel_ring_advance(ring); + __intel_ring_advance(ring); return 0; } @@ -896,7 +896,7 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, intel_ring_emit(ring, RING_PP_DIR_BASE(ring)); intel_ring_emit(ring, get_pd_offset(ppgtt)); intel_ring_emit(ring, MI_NOOP); - intel_ring_advance(ring); + __intel_ring_advance(ring); return 0; } @@ -931,7 +931,6 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, intel_ring_emit(ring, RING_PP_DIR_BASE(ring)); intel_ring_emit(ring, get_pd_offset(ppgtt)); intel_ring_emit(ring, MI_NOOP); - intel_ring_advance(ring); /* XXX: RCS is the only one to auto invalidate the TLBs? */ if (ring->id != RCS) { @@ -940,6 +939,8 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, return ret; } + __intel_ring_advance(ring); + return 0; }
Sometimes when first batch is run with blitter ring, and even tho its PP_BASE has been properly set, the ring seems to be very confused about its memory view. The ring ACHTD keeps progressing past batch boundary, towards the end off address space. Eventually after quite amount of time, hangcheck will declare ring as hanged. But the proceeding reset can run into the same problem with the ring init. If we update the tail immediately after mm switch, the render ring will have a proper pd loaded, before the blitter ring is initialized. This fixes, or papers over, the blitter 'ACTHD proceeding through address space without progress' problem that has been quite a while in ppgtt=2 ring init. Also with Chris Wilson's patch to report GPU faults, I got page faults with unloaded PD's from address zero when running, igt/gem_ppgtt. This patch fixes also those. Testcase: igt/gem_ppgtt Testcase: igt/gem_reset_stats --r ban-blt Cc: Michel Thierry <michel.thierry@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)