[Mesa-dev] [PATCH] r600g: remove one pointless flush

Vadim Girlin vadimgirlin at gmail.com
Fri Oct 28 13:52:16 PDT 2011


On Fri, 2011-10-28 at 19:47 +0200, Marek Olšák wrote:
> I've got no idea what the flush was good for, but it's useless from
> the looks of it. The rest of the patch is just a cleanup resulting
> from some of the variables being no longer used for anything useful.
> 
> There are no piglit regressions.

It was intended to handle multiple interleaved query and conditional
render calls with single query object (in this case in theory we may
have multiple outstanding queries in current CS and separate data block
in the buffer for each query, with possible buffer overflow). I wasn't
sure if this is possible to do in one CS, so I used this additional
check to be on the safe side. Probably such situation is not possible
currently due to the flushes from the other paths, but might be possible
in the future after removing all unnecessary flushes. If you think it
won't be possible at all, I'm OK with this patch. Though I don't like
existing solution in any case, so even if we'll need this check later,
probably it should be done in some better way. 

IIRR this case isn't covered by the piglit. Probably I'll try to
create/modify some test for this.

Vadim

> ---
>  src/gallium/drivers/r600/r600.h            |    6 +-----
>  src/gallium/drivers/r600/r600_blit.c       |    2 +-
>  src/gallium/drivers/r600/r600_hw_context.c |   21 ++-------------------
>  3 files changed, 4 insertions(+), 25 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/r600.h b/src/gallium/drivers/r600/r600.h
> index f58549a..9367651 100644
> --- a/src/gallium/drivers/r600/r600.h
> +++ b/src/gallium/drivers/r600/r600.h
> @@ -176,10 +176,6 @@ struct r600_query {
>  	unsigned				results_end;
>  	/* Size of the result */
>  	unsigned				result_size;
> -	/* Count of new queries started in one stream without flushing */
> -	unsigned				queries_emitted;
> -	/* State flags */
> -	boolean					flushed;
>  	/* The buffer where query results are stored. It's used as a ring,
>  	 * data blocks for current query are stored sequentially from
>  	 * results_start to results_end, with wrapping on the buffer end */
> @@ -258,7 +254,7 @@ boolean r600_context_query_result(struct r600_context *ctx,
>  void r600_query_begin(struct r600_context *ctx, struct r600_query *query);
>  void r600_query_end(struct r600_context *ctx, struct r600_query *query);
>  void r600_context_queries_suspend(struct r600_context *ctx);
> -void r600_context_queries_resume(struct r600_context *ctx, boolean flushed);
> +void r600_context_queries_resume(struct r600_context *ctx);
>  void r600_query_predication(struct r600_context *ctx, struct r600_query *query, int operation,
>  			    int flag_wait);
>  void r600_context_emit_fence(struct r600_context *ctx, struct r600_resource *fence,
> diff --git a/src/gallium/drivers/r600/r600_blit.c b/src/gallium/drivers/r600/r600_blit.c
> index 3eba0ad..9326dc6 100644
> --- a/src/gallium/drivers/r600/r600_blit.c
> +++ b/src/gallium/drivers/r600/r600_blit.c
> @@ -96,7 +96,7 @@ static void r600_blitter_end(struct pipe_context *ctx)
>  					       rctx->saved_render_cond_mode);
>  		rctx->saved_render_cond = NULL;
>  	}
> -	r600_context_queries_resume(&rctx->ctx, FALSE);
> +	r600_context_queries_resume(&rctx->ctx);
>  }
>  
>  static unsigned u_num_layers(struct pipe_resource *r, unsigned level)
> diff --git a/src/gallium/drivers/r600/r600_hw_context.c b/src/gallium/drivers/r600/r600_hw_context.c
> index a7d7ce6..1332748 100644
> --- a/src/gallium/drivers/r600/r600_hw_context.c
> +++ b/src/gallium/drivers/r600/r600_hw_context.c
> @@ -1521,7 +1521,7 @@ void r600_context_flush(struct r600_context *ctx, unsigned flags)
>  	r600_init_cs(ctx);
>  
>  	/* resume queries */
> -	r600_context_queries_resume(ctx, TRUE);
> +	r600_context_queries_resume(ctx);
>  
>  	/* set all valid group as dirty so they get reemited on
>  	 * next draw command
> @@ -1619,18 +1619,6 @@ void r600_query_begin(struct r600_context *ctx, struct r600_query *query)
>  		r600_context_flush(ctx, RADEON_FLUSH_ASYNC);
>  	}
>  
> -	if (query->type == PIPE_QUERY_OCCLUSION_COUNTER) {
> -		/* Count queries emitted without flushes, and flush if more than
> -		 * half of buffer used, to avoid overwriting results which may be
> -		 * still in use. */
> -		if (query->flushed) {
> -			query->queries_emitted = 1;
> -		} else {
> -			if (++query->queries_emitted > query->buffer->b.b.b.width0 / query->result_size / 2)
> -				r600_context_flush(ctx, RADEON_FLUSH_ASYNC);
> -		}
> -	}
> -
>  	new_results_end = query->results_end + query->result_size;
>  	if (new_results_end >= query->buffer->b.b.b.width0)
>  		new_results_end = 0;
> @@ -1713,8 +1701,6 @@ void r600_query_end(struct r600_context *ctx, struct r600_query *query)
>  	if (query->results_end >= query->buffer->b.b.b.width0)
>  		query->results_end = 0;
>  
> -	query->flushed = FALSE;
> -
>  	ctx->num_query_running--;
>  }
>  
> @@ -1842,14 +1828,11 @@ void r600_context_queries_suspend(struct r600_context *ctx)
>  	}
>  }
>  
> -void r600_context_queries_resume(struct r600_context *ctx, boolean flushed)
> +void r600_context_queries_resume(struct r600_context *ctx)
>  {
>  	struct r600_query *query;
>  
>  	LIST_FOR_EACH_ENTRY(query, &ctx->active_query_list, list) {
> -		if (flushed)
> -			query->flushed = TRUE;
> -
>  		r600_query_begin(ctx, query);
>  	}
>  }





More information about the mesa-dev mailing list