On Sat, Aug 11, 2012 at 10:14:40AM -0500, Rob Clark wrote:
On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
if (!ret) {cb->base.flags = 0;cb->base.func = __dma_fence_wake_func;cb->base.private = priv;cb->fence = fence;cb->func = func;__add_wait_queue(&fence->event_queue, &cb->base);}spin_unlock_irqrestore(&fence->event_queue.lock, flags);return ret;+} +EXPORT_SYMBOL_GPL(dma_fence_add_callback);
I think for api completenes we should also have a dma_fence_remove_callback function.
We did originally but Maarten found it was difficult to deal with properly when the gpu's hang. I think his alternative was just to require the hung driver to signal the fence. I had kicked around the idea of a dma_fence_cancel() alternative to signal that could pass an error thru to the waiting driver.. although not sure if the other driver could really do anything differently at that point.
Well, the idea is not to cancel all callbacks, but just a single one, in case a driver wants to somehow abort the wait. E.g. when the own gpu dies I guess we should clear all these fence callbacks so that they can't clobber the hw state after the reset.
+/**
- dma_fence_wait - wait for a fence to be signaled
- @fence: [in] The fence to wait on
- @intr: [in] if true, do an interruptible wait
- @timeout: [in] absolute time for timeout, in jiffies.
I don't quite like this, I think we should keep the styl of all other wait_*_timeout functions and pass the arg as timeout in jiffies (and also the same return semantics). Otherwise well have funny code that needs to handle return values differently depending upon whether it waits upon a dma_fence or a native object (where it would us the wait_*_timeout functions directly).
We did start out this way, but there was an ugly jiffies roll-over problem that was difficult to deal with properly. Using an absolute time avoided the problem.
Well, as-is the api works differently than all the other _timeout apis I've seen in the kernel, which makes it confusing. Also, I don't quite see what jiffies wraparound issue there is?
Also, I think we should add the non-_timeout variants, too, just for completeness.
This request here has the same reasons, essentially: If we offer a dma_fence wait api that matches the usual wait apis closely, it's harder to get their usage wrong. I know that i915 has some major cludge for a wait_seqno interface internally, but that's no reason to copy that approach ;-)
Cheers, Daniel