On 04/01/16 13:02, Dave Gordon wrote:
On 04/01/16 11:26, Chris Wilson wrote:
On Mon, Jan 04, 2016 at 11:16:04AM +0000, Dave Gordon wrote:
On 14/12/15 15:11, Chris Wilson wrote:
On Mon, Dec 14, 2015 at 02:59:30PM +0000, Tvrtko Ursulin wrote:
Hi,
On 11/12/15 11:33, Chris Wilson wrote:
Now that we have split out the seqno-barrier from the
engine->get_seqno() callback itself, we can move the users of the
seqno-barrier to the required callsites simplifying the common
code and
making the required workaround handling much more explicit.
What bothers me about this patch, and the one preceding it, is that
I don't see a tangible improvement for the programmer who still has
to know when to read the seqno and when to "read it harder, read for
real".
In earlier patches, I called it irq_barrier.
It's not reading it harder. It's just that there is a ordering issue
with receiving an interrupt and the seqno write being visible.
Barrier in this sense has a relation to the state of things but
somehow feels too low level to me when used from the code. But to be
fair I am not sure how to better define it.
Would ring->get_seqno paired with ring->read_seqno perhaps make
sense? Implementation for ring->read_seqno would just be a flush
followed by ring->get_seqno then. Or maybe keep the barrier and add
ring->read_seqno which would be ring->seqno_barrier +
ring_get_seqno?
No.
-Chris
We could instead put the knowledge about whether and how to read
"for real" inside the read-the-seqno function. For example:
You do appreciate the irony that you are on the reviewer list for patches
that do that?
http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=breadcrumbs&id=34409f2d965001d7d63f21a1c5339b07eed6af34
No, I haven't got as far as that one, since it was posted over a week
after the message at the head of this thread. Anyway, I still can't see
in that patch anything equivalent to what I described above.
Oh, I spotted what you meant, but it's not in /that/ patch (which was a
version of PATCH 15/32 (Slaughter the thundering i915_wait_request herd)
it's in PATCH 19/32 (Check the CPU cached value of seqno after waking
the waiter).
Even so, it's not at the same level of code structure; I was suggesting
pushing it all the way down, because __i915_wait_request() and/or
i915_gem_request_completed() aren't the only functions that use it.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
