On Tue, Jul 11, 2017 at 11:20 AM, Dave Airlie <airlied at gmail.com> wrote: > On 11 July 2017 at 18:36, Christian König <deathsimple at vodafone.de> wrote: >> Am 11.07.2017 um 08:49 schrieb Dave Airlie: >>> >>> On 7 July 2017 at 19:07, Christian König <deathsimple at 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
