Chris Wilson <chris at chris-wilson.co.uk> writes: > On Mon, 17 Dec 2012 18:44:26 +0200, Mika Kuoppala <mika.kuoppala at linux.intel.com> wrote: >> Hardware status page needs to have proper seqno set >> as our initial seqno can be arbitrary. If initial seqno is close >> to wrap boundary on init and i915_seqno_passed() (31bit space) >> refers to hw status page which contains zero, errorneous result >> will be returned. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=58230 >> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com> > > Looks good baring the last chunk. > > >> @@ -1435,8 +1439,18 @@ int intel_ring_handle_seqno_wrap(struct intel_ring_buffer *ring) >> * post-wrap semaphore waits completing immediately. Clear them. */ >> update_mboxes(ring, ring->signal_mbox[0]); >> update_mboxes(ring, ring->signal_mbox[1]); >> + intel_ring_emit(ring, MI_STORE_DWORD_INDEX); >> + intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT); >> + intel_ring_emit(ring, seqno); >> + intel_ring_emit(ring, MI_USER_INTERRUPT); >> intel_ring_advance(ring); >> >> + /* Wait until mboxes have actually cleared before pushing >> + * anything to the rings */ >> + ret = ring_wait_for_space(ring, ring->size - 8); >> + if (ret) >> + return ret; > > I don't this is well justified. Can you clearly explain the situation > where it is required? As the ring_add_request can it self cause seqno wrap due to intel_ring_begin, and the fact that it will update the *other* rings mboxes, we need to wait until all the rings have proceed with clearing the mboxes. > How about if we assert that the ring is idle, and just blitz the > registers and hws rather than go through the ring? > -Chris I have tried this but failed. I think the problem is ring_add_request. It will still inject seqnos before the wrap boundary if intel_ring_begin in itself just wrapped. This is why we need to clear mboxes and set the hws page through rings. I have a patch which allocates seqnos explicitly early in i915_gem_do_execbuffer, gets rid of outstanding_lazy_request and related i915_gem_check_olr completely thus making the wrap handling much more simpler as we don't need to be careful in ring_sync nor ring_add_request as no cross wrap boundary stuff can no longer happen. But I got the impression that you don't like this approach. -Mika > -- > Chris Wilson, Intel Open Source Technology Centre
