[Intel-gfx] [PATCH 020/190] drm/i915: Remove the lazy_coherency parameter from request-completed?

[Chris Wilson]

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