From: "Marek Olšák" <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Dave Airlie <airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: "Christian König"
<deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>,
"amd-gfx mailing list"
<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
"Olsak, Marek" <Marek.Olsak-5C7GfCeVMHo@public.gmane.org>,
dri-devel
<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH libdrm] libdrm_amdgpu: add kernel semaphore support
Date: Tue, 18 Jul 2017 10:45:32 -0400 [thread overview]
Message-ID: <CAAxE2A545=WVwdc8izaGMNFj=3x5UXAjs1+QLQDKpMgPBbtb3A@mail.gmail.com> (raw)
In-Reply-To: <CAPM=9tx46kYToSvirxL_TTv5dONe8UmM0MCDmAj=YQpCVhyWtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Dave,
If you just add "get" functions for what you need from amdgpu objects,
that should be fine.
Marek
On Mon, Jul 17, 2017 at 11:00 PM, Dave Airlie <airlied@gmail.com> wrote:
> On 18 July 2017 at 03:02, Christian König <deathsimple@vodafone.de> wrote:
>> 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.
>
> As I suspected this plan is full of traps.
>
> So with the raw cs api I posted (using amdgpu_bo_list_handle instead), I ran
> into two places the abstraction cuts me.
>
> CC winsys/amdgpu/radv_amdgpu_cs.lo
> winsys/amdgpu/radv_amdgpu_cs.c: In function ‘radv_amdgpu_cs_submit’:
> winsys/amdgpu/radv_amdgpu_cs.c:1173:63: error: dereferencing pointer
> to incomplete type ‘struct amdgpu_bo’
> chunk_data[i].fence_data.handle = request->fence_info.handle->handle;
> ^~
> winsys/amdgpu/radv_amdgpu_cs.c:1193:31: error: dereferencing pointer
> to incomplete type ‘struct amdgpu_context’
> dep->ctx_id = info->context->id;
>
> In order to do user fence chunk I need the actual bo handle not the
> amdgpu wrapped one, we don't have an accessor method for that.
>
> In order to do the dependencies chunks, I need a context id.
>
> Now I suppose I can add chunk creation helpers to libdrm, but it does
> seems like it breaks the future proof interface if we can't access the
> details of a bunch of objects we want to pass through to the kernel
> API.
>
> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2017-07-18 14:45 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-06 1:17 [PATCH libdrm] libdrm_amdgpu: add kernel semaphore support Dave Airlie
2017-07-06 22:19 ` Dave Airlie
2017-07-07 9:07 ` Christian König
[not found] ` <8a5bb44b-c749-43e1-c58a-6c340bbf5474-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-11 6:49 ` Dave Airlie
2017-07-11 8:36 ` Christian König
[not found] ` <e8320749-8afb-5d64-93e4-7c64192735c5-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-11 9:20 ` Dave Airlie
2017-07-11 9:32 ` Christian König
[not found] ` <c8c18534-e831-4e48-603a-f72e0234ba6e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-12 8:43 ` Dave Airlie
[not found] ` <CAPM=9tzweASCX=Jf2S6-YEqbCnff94WkDN563TDN55v+F-0sQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-17 3:36 ` Dave Airlie
[not found] ` <CAPM=9twZq965gfWHPhjF-P1W6fAV5=dutH3TRmpGK2mwG7waJg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-17 17:02 ` Christian König
[not found] ` <db2890bd-36de-ab34-da99-c3dcd691a5e2-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-18 3:00 ` Dave Airlie
[not found] ` <CAPM=9tx46kYToSvirxL_TTv5dONe8UmM0MCDmAj=YQpCVhyWtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-18 14:45 ` Marek Olšák [this message]
2017-07-17 17:22 ` Marek Olšák
[not found] ` <CAAxE2A6_-UvYJn81-cHwZrdWsULJZ=xH7ShzhRf5o4xg7YQa2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-17 17:35 ` Christian König
[not found] ` <24c27ce4-34ec-41a0-b8af-b63d9fd89ee3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-18 2:29 ` zhoucm1
[not found] ` <596D7282.2020502-5C7GfCeVMHo@public.gmane.org>
2017-07-18 13:57 ` Christian König
2017-07-19 1:55 ` zhoucm1
[not found] ` <596EBC0B.5050905-5C7GfCeVMHo@public.gmane.org>
2017-07-19 10:12 ` Michel Dänzer
[not found] ` <CAPM=9tw2jGVmJW=VcE=t1WuOXVdj61Qj4jocZQUVGwmHe4DFGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-11 17:47 ` Marek Olšák
[not found] ` <CAAxE2A5KB7WqP7xFMOxRy=eWCRQvkkY_2dp5SENODU3YGAdz5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-12 8:42 ` Dave Airlie
[not found] ` <20170706011749.4217-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-01 15:36 ` Marek Olšák
[not found] ` <CAAxE2A6Fco40WfJy9kt5usntnv2GP92PjRiw9mcUfPrs+jr+Xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-04 19:23 ` Marek Olšák
2017-09-04 21:19 ` Dave Airlie
2017-09-04 21:15 ` Dave Airlie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAxE2A545=WVwdc8izaGMNFj=3x5UXAjs1+QLQDKpMgPBbtb3A@mail.gmail.com' \
--to=maraeo-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=Marek.Olsak-5C7GfCeVMHo@public.gmane.org \
--cc=airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org \
--cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).