On Mon, Jan 11, 2016 at 03:45:08PM +0000, Dave Gordon wrote: > >@@ -3647,7 +3643,10 @@ static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req) > > * but it is easier and safer to do it every time the waiter > > * is woken. > > */ > >- if (i915_gem_request_completed(req, false)) > >+ if (engine->irq_seqno_barrier) > >+ engine->irq_seqno_barrier(engine); > > I'm still not convinced that this is the right place for the magic, > but at least it's preferable to having a lazy_coherency parameter. > So on the basis that the proper review criterion is "better than > before, and creates no new problems", and not "fixes all known > issues", then I've tried very hard to explain why this must be, and only be, in the interrupt handler bottom-half. The basic premise is that we need some delay between the interrupt and the seqno read (on certain platforms) to compensate for the unordered nature of the MI_STORE_DWORD_INDEX vs MI_USER_INTERRUPT. There are other ways of inserting that delay, mostly by adding the in the ring between the data store and the interrupt, but my preference for only inserting the delay as required on the waiter side rather than after every single batch (based on the current state of affairs where waits should be rare). Even more so if we can convince userspace to switch over to userspace fences, rather than hitting the ioctls everytime. If you are interested, the mesa side looks like http://cgit.freedesktop.org/~ickle/mesa/commit/?h=brw-batch2&id=f2c13b212dd30562db7b405d6ea79c87456ead51 The opposite question is where do you think is the right place to insert the interrupt delay? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
