Re: [PATCH] drm/ttm: remove fence_lock

[Maarten Lankhorst <maarten.lankhorst@canonical.com>]

Op 18-10-12 13:02, Thomas Hellstrom schreef:
> On 10/18/2012 10:37 AM, Maarten Lankhorst wrote:
>> Hey,
>>
>> Op 18-10-12 09:59, Thomas Hellstrom schreef:
>>>
>>>
>>> On 10/18/2012 09:28 AM, Thomas Hellstrom wrote:
>>>> Hi, Maarten,
>>>>
>>>> As you know I have been having my doubts about this change.
>>>> To me it seems insane to be forced to read the fence pointer under a
>>>> reserved lock, simply because when you take the reserve lock, another
>>>> process may have it and there is a substantial chance that that process
>>>> will also be waiting for idle while holding the reserve lock.
>> I think it makes perfect sense, the only times you want to read the fence
>> is when you want to change the members protected by the reservation.
>
> No, that's not true. A typical case (or the only case)
> is where you want to do a map with no_wait semantics. You will want
> to be able to access a buffer's results even if the eviction code
> is about to schedule an unbind from the GPU, and have the buffer
> reserved?
Well either block on reserve or return -EBUSY if reserved, presumably the
former..

ttm_bo_vm_fault does the latter already, anyway.

You don't need to hold the reservation while performing the wait itself though,
you could check if ttm_bo_wait(no_wait_gpu = true) returns -EBUSY, and if so
take a ref to the sync_obj member and then wait after unreserving. You won't
reset sync_obj member to NULL in that case, but that should be harmless.
This will allow you to keep the reservations fast and short. Maybe a helper
would be appropriate for this since radeon and nouveau both seem to do this.

The next time someone wants to do a wait it will go through the fastpath and
unset the sync_obj member, since it's already signaled, or it's removed when
ttm_execbuffer_util is used.

~Maarten