[Mesa-stable] [PATCH] Revert "radeonsi: avoid syncing the driver thread in si_fence_finish"

[Timothy Arceri]

Cc: mesa-stable <mesa-stable at lists.freedesktop.org>

Hopefully this applies cleanly to 18.2. If so it would be good if we can 
include this there.

On 12/9/18 8:50 pm, Timothy Arceri 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);
> +
>   	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. */
>