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

From: "Christian König" <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
To: Dave Airlie <airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: "Olsak, Marek" <Marek.Olsak-5C7GfCeVMHo@public.gmane.org>,
	dri-devel
	<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	amd-gfx mailing list
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH libdrm] libdrm_amdgpu: add kernel semaphore support
Date: Mon, 17 Jul 2017 19:02:17 +0200	[thread overview]
Message-ID: <db2890bd-36de-ab34-da99-c3dcd691a5e2@vodafone.de> (raw)
In-Reply-To: <CAPM=9twZq965gfWHPhjF-P1W6fAV5=dutH3TRmpGK2mwG7waJg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am 17.07.2017 um 05:36 schrieb Dave Airlie:
>> I can take a look at it, I just won't have time until next week most likely.
> I've taken a look, and it's seemingly more complicated than I'm
> expecting I'd want to land in Mesa before 17.2 ships, I'd really
> prefer to just push the new libdrm_amdgpu api from this patch. If I
> have to port all the current radv code to the new API, I'll most
> definitely get something wrong.
>
> Adding the new API so far looks like
> https://cgit.freedesktop.org/~airlied/drm/log/?h=drm-amdgpu-cs-submit-raw
>
> https://cgit.freedesktop.org/~airlied/drm/commit/?h=drm-amdgpu-cs-submit-raw&id=e7f85d0ca617fa41e72624780c9035df132e23c4
> being the API, and whether it should take a uint32_t context id or
> context handle left as an open question in the last patch in the
> series.

I would stick with the context handle, as far as I can see there isn't 
any value in using the uint32_t for this.

We just want to be able to send arbitrary chunks down into the kernel 
without libdrm_amdgpu involvement and/or the associated overhead of the 
extra loop and the semaphore handling.

So your "amdgpu/cs: add new raw cs submission interface just taking 
chunks" patch looks fine to me as far as I can tell.

As far as I can see the "amdgpu: refactor semaphore handling" patch is 
actually incorrect. We must hole the mutex while sending the CS down to 
the kernel, or otherwise "context->last_seq" won't be accurate.

> However to hook this into radv or radeonsi will take a bit of
> rewriting of a lot of code that is probably a bit more fragile than
> I'd like for this sort of surgery at this point.

Again, I can move over the existing Mesa stuff if you like.

> I'd actually suspect if we do want to proceed with this type of
> interface, we might be better doing it all in common mesa code, and
> maybe bypassing libdrm_amdgpu altogether, which I suppose the API I've
> written here is mostly already doing.

I want to stick with the other interfaces for now. No need to make it 
more complicated than it already is.

Only the CS stuff is the most performance critical and thing we have 
right now.

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