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

[Dave Airlie <airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 11 July 2017 at 19:32, Christian König <deathsimple@vodafone.de> wrote:
> Am 11.07.2017 um 11:20 schrieb Dave Airlie:
>>
>> 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.
>
>
> Ah, crap. Ok in this case we can never remove it.
>
> Anyway, the new CS IOCTL shouldn't need any support for this any more.
>
> Just basic submission of chunks and filling in chunks from libdrm objects
> should be enough as far as I can see.
>
> If Marek doesn't have time or doesn't want to take care of it I can send a
> patch if you want.

I can take a look at it, I just won't have time until next week most likely.

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