[Mesa-dev] [PATCH 00/17] Add support for GL_EXT_semaphore
Andres Rodriguez
andresx7 at gmail.com
Fri Nov 3 18:01:57 UTC 2017
On 2017-11-03 05:15 AM, Nicolai Hähnle wrote:
> Hi Andres,
>
> Thank you for doing this. I haven't read the series in detail yet, but I
> have a bunch of questions and comments.
Thanks for all the feedback.
>
> I find it very unfortunate to have both fence and semaphore objects in
> the Gallium interface. The two even become mixed up in the amdgpu
> winsys. My first thought is that it should be possible to improve this
> by simply turning *all* Gallium fences into semaphores as far as the
> semantics of the Gallium interface are concerned (the underlying
> implementation can of course still vary). All the existing uses of
> fences basically become semaphores that are create and signaled
> immediately by calls to pipe_context::flush.
Agreed with this point. See the discussion on patch 9 for a query on
taking a slightly different approach. But regardless of the outcome I
agree that merging these would be cleaner.
>
> This brings me to the next point: all new related Gallium interfaces
> should really be in the same commit. So the first commit should
> introduce PIPE_CAP_SEMAPHORE and all new pipe_context/pipe_screen
> functions. It should also contain documentation (see context.rst /
> screen.rst). Writing up clean documentation for semaphores should also
> help with resolving the question of the previous paragraph.
>
> Adding the implementations in ddebug, trace, and u_threaded_context can
> (and probably should) be in a separate commit.
>
> transition_resource: since it's currently a no-op, I would just leave it
> out entirely for now. Also, if anything, I think the semantics are
> better served by adding the layout parameter to
> pipe_context::flush_resource and using only that (I believe you're
> always calling both functions together when calling transition_resource,
> which is a pretty strong hint :-)).
I wasn't sure if leaving it out would be considered good form. That is
less rebasing work for me, so I'm happy with that approach :)
>
> As an additional minor point, the sequence of patches should really be:
>
> - Gallium interface
> - Gallium utils
> - mesa patches
> - enabling the extension in mesa/st comes last
> - winsys & radeonsi patches
Will do. This plus your other feedback above on squashing the gallium
interfaces and splitting out the other ddebug/etc changes helps me
understand which of the little details are important. Thanks for taking
the time to point it out.
Regards,
Andres
>
> Not a big deal, but should be possible to fix very easily.
>
> I'm sending some more comments about flushes on individual patches.
>
> Cheers,
> Nicolai
>
>
> On 02.11.2017 04:57, Andres Rodriguez wrote:
>> This series adds radeonsi support for GL_EXT_semaphore.
>>
>> GL_EXT_semaphore is used by steam's vrclient to synchronize access to
>> shared
>> surfaces (vulkan <-> gl interop). It allows our gl vrclients to
>> enqueue their
>> framebuffer surface without waiting for a glFinish().
>>
>> If anyone has any suggestions on improvements for the implicit flush
>> added in
>> patch 9 please let me know. Either a way to remove it, or a way to
>> move it to
>> a codepath that is a bit more specific to radeonsi.
>>
>> Test results for this series (pretty alpha quality auto testing):
>> https://lostgoat.me/gpuci/jenkins/blue/organizations/jenkins/amdgpu-reviews/detail/amdgpu-reviews/184/pipeline
>>
>>
>> Test results for each individual patch can also be found here (see
>> 'Related
>> Changes' section):
>> https://gerrit.lostgoat.me/#/c/454/
>>
>> And here is a baseline for test results for comparison:
>> https://lostgoat.me/gpuci/jenkins/blue/organizations/jenkins/amdgpu-master/detail/amdgpu-master/1269/tests
>>
>>
>> Tests include Vulkan CTS and piglit on Kaveri and Polaris 10.
>>
>> I just kicked off the tests, so they might still show in a queued/in
>> progress
>> state for a few hours. Hopefully my ryzen server and my test slaves
>> survive the
>> unsupervised effort (*fingers crossed it doesn't catch fire while I
>> sleep*).
>>
>> Patches 1-12 add support for semaphore wait/signal/import
>> Patch 13 implements buffer/texture barriers
>> Patches 14-16 implement layout transitions
>> Patch 17 exposes the extension
>>
>> Andres Rodriguez (17):
>> gallium: introduce PIPE_CAP_SEMAPHORE
>> mesa/st: expose EXT_semaphore and EXT_semaphore_fd
>> mesa: add support for semaphore object creation/import/delete
>> mesa: add semaphore parameter stub
>> gallium: introduce semaphore object
>> u_threaded_context: add support for semaphore wait/signal
>> mesa/st: add support for semaphore object create/import/delete
>> mesa: add support for semaphore object signal/wait
>> mesa/st: add support for waiting for semaphore objects
>> mesa: minor tidy up for memory object error strings
>> winsys/amdgpu: add support for syncobj signaling
>> radeonsi: implement semaphore operations
>> mesa: implement buffer/texture barriers for semaphore wait/signal
>> gallium: add transition_resource call
>> mesa/st: hook up resource transitions for semaphore calls
>> radeonsi: implement pipe transition_resource callback
>> radeonsi: advertise support for GL_EXT_semaphore
>>
>> src/gallium/auxiliary/util/u_threaded_context.c | 52 ++++
>> .../auxiliary/util/u_threaded_context_calls.h | 1 +
>> src/gallium/docs/source/screen.rst | 1 +
>> src/gallium/drivers/ddebug/dd_context.c | 23 ++
>> src/gallium/drivers/ddebug/dd_screen.c | 25 ++
>> src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 +
>> src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
>> src/gallium/drivers/i915/i915_screen.c | 1 +
>> src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
>> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
>> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
>> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
>> src/gallium/drivers/r300/r300_screen.c | 1 +
>> src/gallium/drivers/r600/r600_pipe.c | 1 +
>> src/gallium/drivers/radeon/r600_pipe_common.c | 52 ++++
>> src/gallium/drivers/radeon/r600_pipe_common.h | 5 +
>> src/gallium/drivers/radeon/radeon_winsys.h | 12 +
>> src/gallium/drivers/radeonsi/si_blit.c | 11 +
>> src/gallium/drivers/radeonsi/si_pipe.c | 3 +
>> src/gallium/drivers/softpipe/sp_screen.c | 1 +
>> src/gallium/drivers/svga/svga_screen.c | 1 +
>> src/gallium/drivers/swr/swr_screen.cpp | 1 +
>> src/gallium/drivers/trace/tr_context.c | 36 +++
>> src/gallium/drivers/trace/tr_screen.c | 39 +++
>> src/gallium/drivers/vc4/vc4_screen.c | 1 +
>> src/gallium/drivers/virgl/virgl_screen.c | 1 +
>> src/gallium/include/pipe/p_context.h | 36 +++
>> src/gallium/include/pipe/p_defines.h | 12 +
>> src/gallium/include/pipe/p_screen.h | 22 ++
>> src/gallium/include/pipe/p_state.h | 8 +
>> src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 81 +++++-
>> src/gallium/winsys/amdgpu/drm/amdgpu_cs.h | 4 +
>> src/mesa/Makefile.sources | 2 +
>> src/mesa/drivers/common/driverfuncs.c | 3 +
>> src/mesa/main/dd.h | 58 +++++
>> src/mesa/main/extensions_table.h | 2 +
>> src/mesa/main/externalobjects.c | 274
>> +++++++++++++++++++--
>> src/mesa/main/externalobjects.h | 34 ++-
>> src/mesa/main/mtypes.h | 9 +
>> src/mesa/main/shared.c | 17 ++
>> src/mesa/meson.build | 2 +
>> src/mesa/state_tracker/st_cb_semaphoreobjects.c | 164 ++++++++++++
>> src/mesa/state_tracker/st_cb_semaphoreobjects.h | 25 ++
>> src/mesa/state_tracker/st_context.c | 2 +
>> src/mesa/state_tracker/st_extensions.c | 2 +
>> 45 files changed, 1012 insertions(+), 19 deletions(-)
>> create mode 100644 src/mesa/state_tracker/st_cb_semaphoreobjects.c
>> create mode 100644 src/mesa/state_tracker/st_cb_semaphoreobjects.h
>>
>
>
More information about the mesa-dev
mailing list