op 23-07-14 08:52, Christian KÃnig schreef:
Am 23.07.2014 08:40, schrieb Maarten Lankhorst:It doesn't need to be completely reliable, or finish immediately.
op 22-07-14 17:59, Christian KÃnig schreef: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?
Am 22.07.2014 17:42, schrieb Daniel Vetter: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.
On Tue, Jul 22, 2014 at 5:35 PM, Christian KÃnigIn this case sorry for so much noise. I really haven't looked in so much detail into anything but Maarten's Radeon patches.
<christian.koenig@xxxxxxx> wrote:
Drivers exporting fences need to provide a fence->signaled and a fence->waitWell that's almost what we have right now with the exception that
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.
drivers are allowed (actually must for correctness when updating
fences) the ww_mutexes for dma-bufs (or other buffer objects).
But how does that then work right now? My impression was that it's mandatory for drivers to call fence_signaled()?
This allows you to keep interrupts disabled when you don't need them.
And any time wake_up_all(&rdev->fence_queue) is called all the fences that were enabled will be rechecked.
The lockup handling calls radeon_fence_wait, not the generic fence_wait. It doesn't call the exported wait function that takes the exclusive_lock in read mode.That's actually not such a good idea.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.Agreed that any shared locks are out of the way (especially stuff likeYeah 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).
dev->struct_mutex or other non-strictly driver-private stuff, i915 is
really bad here still).
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 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.
And lockdep should have complained if I screwed that up. :-)
~Maarten