[Mesa-dev] [PATCH] Revert "radeonsi: avoid syncing the driver thread in si_fence_finish"
andrey simiklit
asimiklit.work at gmail.com
Tue Sep 18 08:09:14 UTC 2018
Hi,
Please find my comments below:
Regards,
Andrii.
On Tue, Sep 18, 2018 at 4:49 AM Marek Olšák <maraeo at gmail.com> wrote:
> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>
> Marek
>
> On Wed, Sep 12, 2018 at 6:50 AM, Timothy Arceri <tarceri at itsqueeze.com>
> wrote:
> > This reverts commit bc65dcab3bc48673ff6180afb036561a4b8b1119.
> >
> > This was manually reverted. Reverting stops the menu hanging in
> > some id tech games such as RAGE and Wolfenstein The New Order.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107891
> > ---
> > .../auxiliary/util/u_threaded_context.h | 8 --
> > src/gallium/drivers/radeonsi/si_fence.c | 82 +++++++++----------
> > src/gallium/drivers/radeonsi/si_gfx_cs.c | 2 -
> > 3 files changed, 40 insertions(+), 52 deletions(-)
> >
> > diff --git a/src/gallium/auxiliary/util/u_threaded_context.h
> b/src/gallium/auxiliary/util/u_threaded_context.h
> > index be6933d05a4..a32f893592a 100644
> > --- a/src/gallium/auxiliary/util/u_threaded_context.h
> > +++ b/src/gallium/auxiliary/util/u_threaded_context.h
> > @@ -408,14 +408,6 @@ threaded_transfer(struct pipe_transfer *transfer)
> > return (struct threaded_transfer*)transfer;
> > }
> >
> > -static inline struct pipe_context *
> > -threaded_context_unwrap_unsync(struct pipe_context *pipe)
> > -{
> > - if (!pipe || !pipe->priv)
> > - return pipe;
> > - return (struct pipe_context*)pipe->priv;
> > -}
> > -
> > static inline void
> > tc_unflushed_batch_token_reference(struct tc_unflushed_batch_token
> **dst,
> > struct tc_unflushed_batch_token *src)
> > diff --git a/src/gallium/drivers/radeonsi/si_fence.c
> b/src/gallium/drivers/radeonsi/si_fence.c
> > index 186a785437d..abb7057f299 100644
> > --- a/src/gallium/drivers/radeonsi/si_fence.c
> > +++ b/src/gallium/drivers/radeonsi/si_fence.c
> > @@ -291,8 +291,12 @@ static boolean si_fence_finish(struct pipe_screen
> *screen,
> > {
> > struct radeon_winsys *rws = ((struct si_screen*)screen)->ws;
> > struct si_multi_fence *rfence = (struct si_multi_fence *)fence;
> > + struct si_context *sctx;
> > int64_t abs_timeout = os_time_get_absolute_timeout(timeout);
> >
> > + ctx = threaded_context_unwrap_sync(ctx);
> > + sctx = (struct si_context*)(ctx ? ctx : NULL);
>
Looks like it should be like:
sctx = (struct si_context*)ctx;
or maybe you meant:
sctx = (struct si_context*)(ctx ? ctx->****** : NULL);
> +
> > if (!util_queue_fence_is_signalled(&rfence->ready)) {
> > if (rfence->tc_token) {
> > /* Ensure that si_flush_from_st will be called
> for
> > @@ -345,49 +349,43 @@ static boolean si_fence_finish(struct pipe_screen
> *screen,
> > }
> >
> > /* Flush the gfx IB if it hasn't been flushed yet. */
> > - if (ctx && rfence->gfx_unflushed.ctx) {
> > - struct si_context *sctx;
> > -
> > - sctx = (struct si_context
> *)threaded_context_unwrap_unsync(ctx);
> > - if (rfence->gfx_unflushed.ctx == sctx &&
> > - rfence->gfx_unflushed.ib_index ==
> sctx->num_gfx_cs_flushes) {
> > - /* Section 4.1.2 (Signaling) of the OpenGL 4.6
> (Core profile)
> > - * spec says:
> > - *
> > - * "If the sync object being blocked upon
> will not be
> > - * signaled in finite time (for example, by
> an associated
> > - * fence command issued previously, but not
> yet flushed to
> > - * the graphics pipeline), then
> ClientWaitSync may hang
> > - * forever. To help prevent this behavior, if
> > - * ClientWaitSync is called and all of the
> following are
> > - * true:
> > - *
> > - * * the SYNC_FLUSH_COMMANDS_BIT bit is set
> in flags,
> > - * * sync is unsignaled when ClientWaitSync
> is called,
> > - * * and the calls to ClientWaitSync and
> FenceSync were
> > - * issued from the same context,
> > - *
> > - * then the GL will behave as if the
> equivalent of Flush
> > - * were inserted immediately after the
> creation of sync."
> > - *
> > - * This means we need to flush for such fences
> even when we're
> > - * not going to wait.
> > - */
> > - threaded_context_unwrap_sync(ctx);
> > - si_flush_gfx_cs(sctx,
> > - (timeout ? 0 : PIPE_FLUSH_ASYNC)
> |
> > -
> RADEON_FLUSH_START_NEXT_GFX_IB_NOW,
> > - NULL);
> > - rfence->gfx_unflushed.ctx = NULL;
> > -
> > - if (!timeout)
> > - return false;
> > + if (sctx && rfence->gfx_unflushed.ctx == sctx &&
> > + rfence->gfx_unflushed.ib_index == sctx->num_gfx_cs_flushes) {
> > + /* Section 4.1.2 (Signaling) of the OpenGL 4.6 (Core
> profile)
> > + * spec says:
> > + *
> > + * "If the sync object being blocked upon will not be
> > + * signaled in finite time (for example, by an
> associated
> > + * fence command issued previously, but not yet
> flushed to
> > + * the graphics pipeline), then ClientWaitSync may
> hang
> > + * forever. To help prevent this behavior, if
> > + * ClientWaitSync is called and all of the following
> are
> > + * true:
> > + *
> > + * * the SYNC_FLUSH_COMMANDS_BIT bit is set in flags,
> > + * * sync is unsignaled when ClientWaitSync is
> called,
> > + * * and the calls to ClientWaitSync and FenceSync
> were
> > + * issued from the same context,
> > + *
> > + * then the GL will behave as if the equivalent of
> Flush
> > + * were inserted immediately after the creation of
> sync."
> > + *
> > + * This means we need to flush for such fences even when
> we're
> > + * not going to wait.
> > + */
> > + si_flush_gfx_cs(sctx,
> > + (timeout ? 0 : PIPE_FLUSH_ASYNC) |
> > + RADEON_FLUSH_START_NEXT_GFX_IB_NOW,
> > + NULL);
> > + rfence->gfx_unflushed.ctx = NULL;
> >
> > - /* Recompute the timeout after all that. */
> > - if (timeout && timeout != PIPE_TIMEOUT_INFINITE)
> {
> > - int64_t time = os_time_get_nano();
> > - timeout = abs_timeout > time ?
> abs_timeout - time : 0;
> > - }
> > + if (!timeout)
> > + return false;
> > +
> > + /* Recompute the timeout after all that. */
> > + if (timeout && timeout != PIPE_TIMEOUT_INFINITE) {
> > + int64_t time = os_time_get_nano();
> > + timeout = abs_timeout > time ? abs_timeout -
> time : 0;
> > }
> > }
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_gfx_cs.c
> b/src/gallium/drivers/radeonsi/si_gfx_cs.c
> > index 38b85ce6243..bdb576f7e5c 100644
> > --- a/src/gallium/drivers/radeonsi/si_gfx_cs.c
> > +++ b/src/gallium/drivers/radeonsi/si_gfx_cs.c
> > @@ -147,8 +147,6 @@ void si_flush_gfx_cs(struct si_context *ctx,
> unsigned flags,
> > if (fence)
> > ws->fence_reference(fence, ctx->last_gfx_fence);
> >
> > - /* This must be after cs_flush returns, since the context's API
> > - * thread can concurrently read this value in si_fence_finish. */
> > ctx->num_gfx_cs_flushes++;
> >
> > /* Check VM faults if needed. */
> > --
> > 2.17.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180918/ae09bf9e/attachment-0001.html>
More information about the mesa-dev
mailing list