lkml.org 
[lkml]   [2012]   [Aug]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
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
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48


\
 
 \ /
  Last update: 2012-08-11 22:02    [W:0.056 / U:1.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site