Am 23.07.2014 08:40, schrieb Maarten Lankhorst:
op 22-07-14 17:59, Christian König schreef:
Am 22.07.2014 17:42, schrieb Daniel Vetter:
On Tue, Jul 22, 2014 at 5:35 PM, Christian König
<christian.koe...@amd.com> wrote:
Drivers exporting fences need to provide a fence->signaled and a fence->wait
function, everything else like fence->enable_signaling or calling
fence_signaled() from the driver is optional.
Drivers wanting to use exported fences don't call fence->signaled or
fence->wait in atomic or interrupt context, and not with holding any global
locking primitives (like mmap_sem etc...). Holding locking primitives local
to the driver is ok, as long as they don't conflict with anything possible
used by their own fence implementation.
Well that's almost what we have right now with the exception that
drivers are allowed (actually must for correctness when updating
fences) the ww_mutexes for dma-bufs (or other buffer objects).
In this case sorry for so much noise. I really haven't looked in so much detail
into anything but Maarten's Radeon patches.
But how does that then work right now? My impression was that it's mandatory
for drivers to call fence_signaled()?
It's only mandatory to call fence_signal() if the .enable_signaling callback
has been called, else you can get away with never calling signaling a fence at
all before dropping the last refcount to it.
This allows you to keep interrupts disabled when you don't need them.
Can we somehow avoid the need to call fence_signal() at all? The
interrupts at least on radeon are way to unreliable for such a thing.
Can enable_signalling fail? What's the reason for fence_signaled() in
the first place?
Agreed that any shared locks are out of the way (especially stuff like
dev->struct_mutex or other non-strictly driver-private stuff, i915 is
really bad here still).
Yeah that's also an point I've wanted to note on Maartens patch. Radeon grabs
the read side of it's exclusive semaphore while waiting for fences (because it
assumes that the fence it waits for is a Radeon fence).
Assuming that we need to wait in both directions with Prime (e.g. Intel driver
needs to wait for Radeon to finish rendering and Radeon needs to wait for Intel
to finish displaying), this might become a perfect example of locking inversion.
In the preliminary patches where I can sync radeon with other GPU's I've been
very careful in all the places that call into fences, to make sure that radeon
wouldn't try to handle lockups for a different (possibly also radeon) card.
That's actually not such a good idea.
In case of a lockup we need to handle the lockup cause otherwise it
could happen that radeon waits for the lockup to be resolved and the
lockup handling needs to wait for a fence that's never signaled because
of the lockup.
Christian.
This is also why fence_is_signaled should never block, and why it trylocks the
exclusive_lock. :-) I think lockdep would complain if I grabbed exclusive_lock
while blocking in is_signaled.
So from the core fence framework I think we already have exactly this,
and we only need to adjust the radeon implementation a bit to make it
less risky and invasive to the radeon driver logic.
Agree. Well the biggest problem I see is that exclusive semaphore I need to take
when anything calls into the driver. For the fence code I need to move that down
into the fence->signaled handler, cause that now can be called from outside the
driver.
Maarten solved this by telling the driver in the lockup handler (where we grab the
write side of the exclusive lock) that all interrupts are already enabled, so that
fence->signaled hopefully wouldn't mess with the hardware at all. While this
probably works, it just leaves me with a feeling that we are doing something wrong
here.
There is unfortunately no global mechanism to say 'this card is locked up,
please don't call into any of my fences', and I don't associate fences with
devices, and radeon doesn't keep a global list of fences.
If all of that existed, it would complicate the interface and its callers a
lot, while I like to keep things simple.
So I did the best I could, and simply prevented the fence calls from fiddling
with the hardware. Fortunately gpu lockup is not a common operation. :-)
~Maarten
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
