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:
struct intel_engine_cs {
...
u32 last_seqno_seen;
}
u32 intel_ring_read_seqno(struct intel_engine_cs *engine) {
// First try simple read
u32 seqno = intel_ring_get_seqno(engine);
if (seqno == engine->last_seqno_seen) {
// Do additional flush, then try again
engine->seqno_barrier(engine);
seqno = intel_ring_get_seqno(engine);
}
engine->last_seqno_seen = seqno;
return seqno;
}
Then callers don't have to know anything about coherency; they can just
assume that they will automatically get the latest value.
In the presumably common case where the value *has* been updated, and
the cache has noted the update and invalidated the old local value, the
first read will successfully find the updated seqno and return quickly.
Then we only do the extra work in the case where there would otherwise
appear to be nothing to do (i.e. when we would sleep, or spin, or
otherwise wait, if this function were to return the same value as last
time).
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
