Re: [PATCH libdrm] libdrm_amdgpu: add kernel semaphore support

["Marek Olšák" <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Tue, Jul 11, 2017 at 11:20 AM, Dave Airlie <airlied@gmail.com> wrote:
> On 11 July 2017 at 18:36, Christian König <deathsimple@vodafone.de> wrote:
>> Am 11.07.2017 um 08:49 schrieb Dave Airlie:
>>>
>>> On 7 July 2017 at 19:07, Christian König <deathsimple@vodafone.de> wrote:
>>>>
>>>> Hi Dave,
>>>>
>>>> on first glance that looks rather good to me, but there is one things I
>>>> don't really like and I strongly think Marek will absolutely agree on
>>>> that:
>>>> When we add a new CS function then let's get ride of all this
>>>> abstraction!
>>>>
>>>> The new function should get an amdgpu_device_handle and a list of chunks
>>>> to
>>>> submit, nothing else.
>>>>
>>>> When then provide helper functions to generate the chunks out of the
>>>> existing amdgpu_context_handle and amdgpu_bo_list_handle.
>>>>
>>>> That should be perfectly sufficient and extensible for future additions
>>>> as
>>>> well.
>>>
>>> Sounds tempting, but it a bit messier than it looks once I started
>>> digging into it.
>>>
>>> The main things I ran up against is the context sequence mutex protecting
>>> the
>>> kernel submissions per context which would be tricky to figure out why
>>> that is
>>> required (should we be submitting from different contexts on different
>>> threads?)
>>
>>
>> The sequence lock is just to keep last_seq up to date and last_seq just
>> exists because of amdgpu_cs_signal_semaphore.
>>
>> We want to get ride of that, so you can drop support for this altogether in
>> the new IOCTL.
>>
>>> I'd prefer to land this then refactor a new interface, I do wonder if
>>> maybe Marek
>>> would prefer just doing this all in Mesa and avoiding these APIs a bit
>>> more :-)
>>>
>>> Once I get the syncobjs in I might look at internally refactoring the
>>> code a bit more,
>>> then a new API.
>>
>>
>> Actually I wanted to propose just to remove the old semaphore API, it was
>> never used by Mesa or any other open source user.
>
> radv uses it right now until we have syncobjs.
>
> So it should hang around.

Dave, this may sound outrageous, but think about it: What would happen
if we removed the old semaphore API? Users would have to get a new
RADV after getting new libdrm_amdgpu. Is that really that big of a
deal? It happens with LLVM all the time and we've managed to cope with
that just fine.

Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx