On 11 July 2017 at 19:32, Christian König <deathsimple at vodafone.de> wrote: > Am 11.07.2017 um 11:20 schrieb Dave Airlie: >> >> 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. > > > 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.
