Re: [PATCH 2/5] drm/nouveau: add reservation to nouveau_gem_ioctl_cpu_prep

[Thomas Hellstrom <thomas@shipmail.org>]

On 01/21/2014 04:29 PM, Maarten Lankhorst wrote:
> Hey,
>
> op 21-01-14 16:17, Thomas Hellstrom schreef:
>> Maarten, for this and the other patches in this series,
>>
>> I seem to recall we have this discussion before?
>> IIRC I stated that reservation was a too heavy-weight lock to hold to
>> determine whether a buffer was idle? It's a pretty nasty thing to
>> build in.
>>
> I've sent this patch after determining that this already didn't end up
> being heavyweight.
> Most places were already using the fence_lock and reservation, I just
> fixed up the few
> places that didn't hold a reservation while waiting. Converting the
> few places that didn't
> ended up being trivial, so I thought I'd submit it.

Actually the only *valid* reason for holding a reservation when waiting
for idle is
1) You want to block further command submission on the buffer.
2) You want to switch GPU engine and don't have access to gpu semaphores
/ barriers.

Reservation has the nasty side effect that it blocks command submission
and pins the buffer (in addition now makes the evict list traversals
skip the buffer) which in general is *not* necessary for most wait
cases, so we should instead actually convert the wait cases that don't
fulfill 1) and 2) above in the other direction if we have performance
and latency-reduction in mind. I can't see how a spinlock protecting a
fence pointer or fence list is stopping you from using RW fences as long
as the spinlock is held while manipulating the fence list?

/Thomas