Re: [rfc] drm sync objects (search for spock)

2017-05-09 Thread Jason Ekstrand
On Wed, Apr 26, 2017 at 7:57 AM, Christian König 
wrote:

> Am 26.04.2017 um 11:57 schrieb Dave Airlie:
>
>> On 26 April 2017 at 18:45, Christian König 
>> wrote:
>>
>>> Am 26.04.2017 um 05:28 schrieb Dave Airlie:
>>>
 Okay I've gone around the sun with these a few times, and
 pretty much implemented what I said last week.

 This is pretty much a complete revamp.

 1. sync objects are self contained drm objects, they
 have a file reference so can be passed between processes.

 2. Added a sync object wait interface modelled on the vulkan
 fence waiting API.

 3. sync_file interaction is explicitly different than
 opaque fd passing, you import a sync file state into an
 existing syncobj, or create a new sync_file from an
 existing syncobj. This means no touching the sync file code
 at all. \o/

>>>
I've done a quick scan over the patches and I like the API.  It almost
looks as if Santa read my wish list. :-)

That said, it was a "quick scan" so don't take this as any sort of actual
code review.  It'll probably be a little while before I get a chance to sit
down and look at i915 again but things seem reasonable.


> Doesn't that break the Vulkan requirement that if a sync_obj is exported to
>>> an fd and imported on the other side we get the same sync_obj again?
>>>
>>> In other words the fd is exported and imported only once and the
>>> expectation
>>> is that we fence containing it will change on each signaling operation.
>>>
>>> As far as I can see with the current implementation you get two
>>> sync_objects
>>> on in the exporting process and one in the importing one.
>>>
>> Are you talking about using sync_file interop for vulkan, then yes
>> that would be wrong.
>>
>> But that isn't how this works, see 1. above the sync obj has a 1:1
>> mapping with an
>> opaque fd (non-sync_file) that is only used for interprocess passing
>> of the syncobj.
>>
>
> Ah, ok that makes some more sense.
>
> You can interoperate with sync_files by importing/exporting the
>> syncobj fence into
>> and out of them but that aren't meant for passing the fds around, more
>> for where you
>> get a sync_file fd from somewhere else and you want to use it and
>> vice-versa.
>>
>
> I would prefer dealing only with one type of fd in userspace, but that
> approach should work as well.
>
> I'd like to move any drm APIs away from sync_file to avoid the fd wastages,
>>
>
> That sounds like it make sense, but I would still rather vote for
> extending the sync_file interface than deprecating it with another one.
>
> I'd also like to move the driver specific fences to syncobj where I can.
>>
>
> I'm pretty sure that is not a good idea.
>
> Beating the sequence based fence values we currently use for CPU sync in
> performance would be pretty hard. E.g. at least on amdgpu we can even check
> those by doing a simple 64bit read from a memory address, without IOCTL or
> any other overhead involved.
>
> Regards,
> Christian.
>
>
>
>> Dave.
>>
>
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [rfc] drm sync objects (search for spock)

2017-04-26 Thread Christian König

Am 26.04.2017 um 11:57 schrieb Dave Airlie:

On 26 April 2017 at 18:45, Christian König  wrote:

Am 26.04.2017 um 05:28 schrieb Dave Airlie:

Okay I've gone around the sun with these a few times, and
pretty much implemented what I said last week.

This is pretty much a complete revamp.

1. sync objects are self contained drm objects, they
have a file reference so can be passed between processes.

2. Added a sync object wait interface modelled on the vulkan
fence waiting API.

3. sync_file interaction is explicitly different than
opaque fd passing, you import a sync file state into an
existing syncobj, or create a new sync_file from an
existing syncobj. This means no touching the sync file code
at all. \o/


Doesn't that break the Vulkan requirement that if a sync_obj is exported to
an fd and imported on the other side we get the same sync_obj again?

In other words the fd is exported and imported only once and the expectation
is that we fence containing it will change on each signaling operation.

As far as I can see with the current implementation you get two sync_objects
on in the exporting process and one in the importing one.

Are you talking about using sync_file interop for vulkan, then yes
that would be wrong.

But that isn't how this works, see 1. above the sync obj has a 1:1
mapping with an
opaque fd (non-sync_file) that is only used for interprocess passing
of the syncobj.


Ah, ok that makes some more sense.


You can interoperate with sync_files by importing/exporting the
syncobj fence into
and out of them but that aren't meant for passing the fds around, more
for where you
get a sync_file fd from somewhere else and you want to use it and vice-versa.


I would prefer dealing only with one type of fd in userspace, but that 
approach should work as well.



I'd like to move any drm APIs away from sync_file to avoid the fd wastages,


That sounds like it make sense, but I would still rather vote for 
extending the sync_file interface than deprecating it with another one.



I'd also like to move the driver specific fences to syncobj where I can.


I'm pretty sure that is not a good idea.

Beating the sequence based fence values we currently use for CPU sync in 
performance would be pretty hard. E.g. at least on amdgpu we can even 
check those by doing a simple 64bit read from a memory address, without 
IOCTL or any other overhead involved.


Regards,
Christian.



Dave.



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [rfc] drm sync objects (search for spock)

2017-04-26 Thread Dave Airlie
On 26 April 2017 at 18:45, Christian König  wrote:
> Am 26.04.2017 um 05:28 schrieb Dave Airlie:
>>
>> Okay I've gone around the sun with these a few times, and
>> pretty much implemented what I said last week.
>>
>> This is pretty much a complete revamp.
>>
>> 1. sync objects are self contained drm objects, they
>> have a file reference so can be passed between processes.
>>
>> 2. Added a sync object wait interface modelled on the vulkan
>> fence waiting API.
>>
>> 3. sync_file interaction is explicitly different than
>> opaque fd passing, you import a sync file state into an
>> existing syncobj, or create a new sync_file from an
>> existing syncobj. This means no touching the sync file code
>> at all. \o/
>
>
> Doesn't that break the Vulkan requirement that if a sync_obj is exported to
> an fd and imported on the other side we get the same sync_obj again?
>
> In other words the fd is exported and imported only once and the expectation
> is that we fence containing it will change on each signaling operation.
>
> As far as I can see with the current implementation you get two sync_objects
> on in the exporting process and one in the importing one.

Are you talking about using sync_file interop for vulkan, then yes
that would be wrong.

But that isn't how this works, see 1. above the sync obj has a 1:1
mapping with an
opaque fd (non-sync_file) that is only used for interprocess passing
of the syncobj.

You can interoperate with sync_files by importing/exporting the
syncobj fence into
and out of them but that aren't meant for passing the fds around, more
for where you
get a sync_file fd from somewhere else and you want to use it and vice-versa.

I'd like to move any drm APIs away from sync_file to avoid the fd wastages, I'd
also like to move the driver specific fences to syncobj where I can.

Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [rfc] drm sync objects (search for spock)

2017-04-26 Thread Christian König

Am 26.04.2017 um 05:28 schrieb Dave Airlie:

Okay I've gone around the sun with these a few times, and
pretty much implemented what I said last week.

This is pretty much a complete revamp.

1. sync objects are self contained drm objects, they
have a file reference so can be passed between processes.

2. Added a sync object wait interface modelled on the vulkan
fence waiting API.

3. sync_file interaction is explicitly different than
opaque fd passing, you import a sync file state into an
existing syncobj, or create a new sync_file from an
existing syncobj. This means no touching the sync file code
at all. \o/


Doesn't that break the Vulkan requirement that if a sync_obj is exported 
to an fd and imported on the other side we get the same sync_obj again?


In other words the fd is exported and imported only once and the 
expectation is that we fence containing it will change on each signaling 
operation.


As far as I can see with the current implementation you get two 
sync_objects on in the exporting process and one in the importing one.


Regards,
Christian.



I haven't used rcu anywhere here, I've used xchg to swap
fence pointers in the hope that's safe. If this does need
rcu'ing I suggest we do it in a follow on patch to minimise
the review pain.

Dave.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel