[Mesa-dev] [PATCH] Revert "radeonsi: avoid syncing the driver thread in si_fence_finish"
Marek Olšák
maraeo at gmail.com
Tue Sep 18 08:15:17 UTC 2018
I'm pretty sure it's correct.
Marek
On Tue, Sep 18, 2018 at 4:09 AM, andrey simiklit
<asimiklit.work at gmail.com> wrote:
> 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
More information about the mesa-dev
mailing list