| Message ID | 1426513596-11338-2-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 16, 2015 at 03:46:34PM +0200, Mika Kuoppala wrote: > We are freeing the batch that is just pushed to ring. Kill this. It is not correct. > Further, other ring inits should assume that the render > context with the workarounds are pushed before their rings > execute anything. > > This fixes (or papers over) a problem where with full ppgtt > the blitter ring init sometimes fails, where the blt ring > ACTHD runs wild through the address space until eventually > hangcheck kills it. Interesting. We don't do anything inside the golden render state to be of relevance to the blitter engine. So I think this is pure paper and will not ultimately fix the problem. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Mon, Mar 16, 2015 at 03:46:34PM +0200, Mika Kuoppala wrote: >> We are freeing the batch that is just pushed to ring. > > Kill this. It is not correct. > >> Further, other ring inits should assume that the render >> context with the workarounds are pushed before their rings >> execute anything. >> >> This fixes (or papers over) a problem where with full ppgtt >> the blitter ring init sometimes fails, where the blt ring >> ACTHD runs wild through the address space until eventually >> hangcheck kills it. > > Interesting. We don't do anything inside the golden render state to be > of relevance to the blitter engine. So I think this is pure paper and > will not ultimately fix the problem. > -Chris > Perhaps not in the golden state, but do we push any workarounds through render ring init, that are global in scope and required by the other rings? -Mika
On Mon, Mar 16, 2015 at 04:20:46PM +0200, Mika Kuoppala wrote: > Perhaps not in the golden state, but do we push any workarounds through > render ring init, that are global in scope and required by the other rings? No. Afaict the w/a only apply to render. Or else we have been horribly misinformed. I still suspect it is something due to the ordering of pd loads between rings. Serialising the render context init should not fix the similar hangs we see at runtime. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Mon, Mar 16, 2015 at 04:20:46PM +0200, Mika Kuoppala wrote: >> Perhaps not in the golden state, but do we push any workarounds through >> render ring init, that are global in scope and required by the other rings? > > No. Afaict the w/a only apply to render. Or else we have been horribly > misinformed. I still suspect it is something due to the ordering of pd > loads between rings. Serialising the render context init should not fix > the similar hangs we see at runtime. Ok please ignore this patch then. I will send another that does the immediate tail updates on ring->mm_switch. -Mika
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index 521548a..9484c64 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -175,6 +175,10 @@ int i915_gem_render_state_init(struct intel_engine_cs *ring) ret = __i915_add_request(ring, NULL, so.obj); /* __i915_add_request moves object to inactive if it fails */ + if (ret) + goto out; + + ret = intel_ring_idle(ring); out: i915_gem_render_state_fini(&so); return ret;
We are freeing the batch that is just pushed to ring. Further, other ring inits should assume that the render context with the workarounds are pushed before their rings execute anything. This fixes (or papers over) a problem where with full ppgtt the blitter ring init sometimes fails, where the blt ring ACTHD runs wild through the address space until eventually hangcheck kills it. With dynamic page table series this problem became more easily reproduced by just normal boot failing to init blt ring, instead of torturing init with gem_reset_stats. Testcase: igt/gem_reset_stats --r ban-blt Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem_render_state.c | 4 ++++ 1 file changed, 4 insertions(+)