[Mesa-dev] [PATCH 00/17] Add support for GL_EXT_semaphore
Nicolai Hähnle
nhaehnle at gmail.com
Fri Nov 3 09:15:46 UTC 2017
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.
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.
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 :-)).
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
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
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list