[Mesa-dev] [PATCH] st/mesa: implement query pause/resume for meta operations
Ilia Mirkin
imirkin at alum.mit.edu
Mon Aug 3 16:00:17 PDT 2015
I think I brought this up recently (perhaps this is even in response
to my comments about this)... my suggestion was to introduce a draw
flag that says whether the draw should be counted.
mipmap (and blit) should be using pipe->blit, which should never get
counted anyways, so all that's left are clears. A flag in
pipe_draw_info would solve it nicely. (Or another solution is to
introduce caps to teach ->clear about all the various cases, and not
worrying about impls that fall back to the clear_with_quad mechanism.)
Either way, I don't think it's necessary to pause/resume around pipe->blit ops.
-ilia
On Mon, Aug 3, 2015 at 6:53 PM, Brian Paul <brianp at vmware.com> wrote:
> glClear, blitting, mipmap generation, etc. implemented with quad drawing
> should not effect queries like occlusion count. To accomplish that, this
> patch implements a simple pause/resume mechanism for active queries.
>
> We now keep a list of active queries. When we "pause" queries we actually
> end them. When we resume we get the query result and save it, then re-begin
> the query. When the user requests the query result we add the pre-paused
> value.
>
> As mentioned in the code, this is not a perfect/ideal solution but it fixes
> Piglit's occlusion_query_meta_no_fragments and occlusion_query_meta_save
> tests. In practice, it's pretty unusual to do meta operations between
> glBeginQuery/EndQuery() pairs so these are corner cases.
> ---
> src/mesa/state_tracker/st_cb_blit.c | 7 ++
> src/mesa/state_tracker/st_cb_clear.c | 5 +
> src/mesa/state_tracker/st_cb_queryobj.c | 199 ++++++++++++++++++++++++++++++++
> src/mesa/state_tracker/st_cb_queryobj.h | 10 ++
> src/mesa/state_tracker/st_context.c | 3 +
> src/mesa/state_tracker/st_context.h | 11 +-
> src/mesa/state_tracker/st_gen_mipmap.c | 5 +
> 7 files changed, 239 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_blit.c b/src/mesa/state_tracker/st_cb_blit.c
> index 1396906..77fd908 100644
> --- a/src/mesa/state_tracker/st_cb_blit.c
> +++ b/src/mesa/state_tracker/st_cb_blit.c
> @@ -39,6 +39,7 @@
> #include "st_cb_bitmap.h"
> #include "st_cb_blit.h"
> #include "st_cb_fbo.h"
> +#include "st_cb_queryobj.h"
> #include "st_manager.h"
>
> #include "util/u_format.h"
> @@ -193,6 +194,8 @@ st_BlitFramebuffer(struct gl_context *ctx,
> blit.filter = pFilter;
> blit.render_condition_enable = TRUE;
>
> + st_pause_queries(st);
> +
> if (mask & GL_COLOR_BUFFER_BIT) {
> struct gl_renderbuffer_attachment *srcAtt =
> &readFB->Attachment[readFB->_ColorReadBufferIndex];
> @@ -204,6 +207,7 @@ st_BlitFramebuffer(struct gl_context *ctx,
> GLuint i;
>
> if (!srcObj || !srcObj->pt) {
> + st_resume_queries(st);
> return;
> }
>
> @@ -239,6 +243,7 @@ st_BlitFramebuffer(struct gl_context *ctx,
> GLuint i;
>
> if (!srcRb || !srcRb->surface) {
> + st_resume_queries(st);
> return;
> }
>
> @@ -345,6 +350,8 @@ st_BlitFramebuffer(struct gl_context *ctx,
> }
> }
> }
> +
> + st_resume_queries(st);
> }
>
>
> diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
> index 137fac8..fae28ee 100644
> --- a/src/mesa/state_tracker/st_cb_clear.c
> +++ b/src/mesa/state_tracker/st_cb_clear.c
> @@ -43,6 +43,7 @@
> #include "st_atom.h"
> #include "st_cb_clear.h"
> #include "st_cb_fbo.h"
> +#include "st_cb_queryobj.h"
> #include "st_format.h"
> #include "st_program.h"
>
> @@ -255,6 +256,8 @@ clear_with_quad(struct gl_context *ctx, unsigned clear_buffers)
> x1, y1);
> */
>
> + st_pause_queries(st);
> +
> cso_save_blend(st->cso_context);
> cso_save_stencil_ref(st->cso_context);
> cso_save_depth_stencil_alpha(st->cso_context);
> @@ -381,6 +384,8 @@ clear_with_quad(struct gl_context *ctx, unsigned clear_buffers)
> cso_restore_vertex_elements(st->cso_context);
> cso_restore_aux_vertex_buffer_slot(st->cso_context);
> cso_restore_stream_outputs(st->cso_context);
> +
> + st_resume_queries(st);
> }
>
>
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c b/src/mesa/state_tracker/st_cb_queryobj.c
> index 71222e8..080e18d 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.c
> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> @@ -33,6 +33,8 @@
> */
>
>
> +#include "util/u_debug.h"
> +
> #include "main/imports.h"
> #include "main/context.h"
>
> @@ -80,6 +82,51 @@ st_DeleteQuery(struct gl_context *ctx, struct gl_query_object *q)
> }
>
>
> +/**
> + * Add a query to the active list.
> + */
> +static void
> +save_active_query(struct st_context *st, struct st_query_object *stq)
> +{
> + /* We don't care about pausing/resuming time queries */
> + if (stq->base.Target != GL_TIME_ELAPSED &&
> + stq->base.Target != GL_TIMESTAMP) {
> + struct st_query_list_node *node = CALLOC_STRUCT(st_query_list_node);
> +
> + if (node) {
> + node->query = stq;
> + insert_at_tail(&st->active_queries.list, &node->list);
> + }
> + }
> +}
> +
> +
> +/**
> + * Remove a query from the active list.
> + */
> +static void
> +remove_active_query(struct st_context *st, struct st_query_object *stq)
> +{
> + /* We don't care about pausing/resuming time queries */
> + if (stq->base.Target != GL_TIME_ELAPSED &&
> + stq->base.Target != GL_TIMESTAMP) {
> + struct simple_node *n;
> +
> + foreach (n, &st->active_queries.list) {
> + struct st_query_list_node *node = (struct st_query_list_node *) n;
> +
> + if (node->query == stq) {
> + /* found it, remove it */
> + remove_from_list(&node->list);
> + free(node);
> + return;
> + }
> + }
> + assert(!"query not found in remove_active_query()");
> + }
> +}
> +
> +
> static void
> st_BeginQuery(struct gl_context *ctx, struct gl_query_object *q)
> {
> @@ -163,6 +210,8 @@ st_BeginQuery(struct gl_context *ctx, struct gl_query_object *q)
> }
> }
> assert(stq->type == type);
> +
> + save_active_query(st_context(ctx), stq);
> }
>
>
> @@ -183,6 +232,68 @@ st_EndQuery(struct gl_context *ctx, struct gl_query_object *q)
>
> if (stq->pq)
> pipe->end_query(pipe, stq->pq);
> +
> + remove_active_query(st_context(ctx), stq);
> +}
> +
> +
> +/**
> + * Basically compute orig += new for query results.
> + */
> +static void
> +combine_query_results(union pipe_query_result *orig,
> + const union pipe_query_result *new,
> + unsigned query_target)
> +{
> + switch (query_target) {
> + case PIPE_QUERY_OCCLUSION_PREDICATE:
> + case PIPE_QUERY_SO_OVERFLOW_PREDICATE:
> + case PIPE_QUERY_GPU_FINISHED:
> + orig->b |= new->b;
> + break;
> + case PIPE_QUERY_OCCLUSION_COUNTER:
> + case PIPE_QUERY_TIMESTAMP:
> + case PIPE_QUERY_TIME_ELAPSED:
> + case PIPE_QUERY_PRIMITIVES_GENERATED:
> + case PIPE_QUERY_PRIMITIVES_EMITTED:
> + orig->u64 += new->u64;
> + break;
> + case PIPE_QUERY_SO_STATISTICS:
> + orig->so_statistics.num_primitives_written
> + += new->so_statistics.num_primitives_written;
> + orig->so_statistics.primitives_storage_needed
> + += new->so_statistics.primitives_storage_needed;
> + break;
> + case PIPE_QUERY_TIMESTAMP_DISJOINT:
> + /* XXX is this right? */
> + orig->timestamp_disjoint.frequency = new->timestamp_disjoint.frequency;
> + orig->timestamp_disjoint.disjoint = TRUE;
> + break;
> + case PIPE_QUERY_PIPELINE_STATISTICS:
> + orig->pipeline_statistics.ia_vertices
> + += new->pipeline_statistics.ia_vertices;
> + orig->pipeline_statistics.vs_invocations
> + += new->pipeline_statistics.vs_invocations;
> + orig->pipeline_statistics.hs_invocations
> + += new->pipeline_statistics.hs_invocations;
> + orig->pipeline_statistics.ds_invocations
> + += new->pipeline_statistics.ds_invocations;
> + orig->pipeline_statistics.gs_invocations
> + += new->pipeline_statistics.gs_invocations;
> + orig->pipeline_statistics.gs_primitives
> + += new->pipeline_statistics.gs_primitives;
> + orig->pipeline_statistics.ps_invocations
> + += new->pipeline_statistics.ps_invocations;
> + orig->pipeline_statistics.cs_invocations
> + += new->pipeline_statistics.cs_invocations;
> + orig->pipeline_statistics.c_invocations
> + += new->pipeline_statistics.c_invocations;
> + orig->pipeline_statistics.c_primitives
> + += new->pipeline_statistics.c_primitives;
> + break;
> + default:
> + assert(!"Unexpected query type in combine_query_results()");
> + }
> }
>
>
> @@ -203,6 +314,9 @@ get_query_result(struct pipe_context *pipe,
> if (!pipe->get_query_result(pipe, stq->pq, wait, &data))
> return FALSE;
>
> + /* accumulate any results from being paused with this result */
> + combine_query_results(&data, &stq->pre_paused_result, stq->type);
> +
> switch (stq->base.Target) {
> case GL_VERTICES_SUBMITTED_ARB:
> stq->base.Result = data.pipeline_statistics.ia_vertices;
> @@ -305,3 +419,88 @@ void st_init_query_functions(struct dd_function_table *functions)
> functions->CheckQuery = st_CheckQuery;
> functions->GetTimestamp = st_GetTimestamp;
> }
> +
> +
> +/**
> + * This is used to temporarily suspend any active queries (occlusion, xfb,
> + * etc) when we're about to do a "meta" operation, such as doing a scissored
> + * glClear with polygon drawing.
> + * We basically implement the pause/resume by stopping/re-beginning the query
> + * and saving the pre-pause query result. The pre-pause result is added to
> + * the later query when its result is returned.
> + *
> + * Note: this scheme is not perfect:
> + * 1. It doesn't work correctly with predicated rendering. But it would be
> + * pretty unusual to do a glClear/Blit/GenerateMipmap between the
> + * glBeginQuery/EndQuery() pair for the occlusion test phase.
> + * 2. If we implement GL_ARB_query_buffer_object, we'd have to stuff the
> + * accumulated query result into the buffer, negating the performance
> + * benefit of that feature. Again, it's quite unusual to have meta
> + * operations inside glBeginQuery/EndQuery().
> + */
> +void
> +st_pause_queries(struct st_context *st)
> +{
> + struct simple_node *n;
> +
> + assert(!st->queries_paused);
> + st->queries_paused = TRUE;
> +
> + /* Loop over active queries */
> + foreach (n, &st->active_queries.list) {
> + struct st_query_list_node *qln = (struct st_query_list_node *) n;
> +
> + assert(qln->query->pq);
> + if (!qln->query->pq)
> + continue;
> +
> + /* end the query */
> + st->pipe->end_query(st->pipe, qln->query->pq);
> +
> + /* Note: we don't get the query result here. We postpone that until
> + * st_resume_queries() to give the graphics pipe a little more time
> + * to possibly finish the query to avoid blocking.
> + */
> + }
> +}
> +
> +
> +/**
> + * Counterpart to st_pause_queries(), called after finishing meta operations.
> + * We "resume" the active queries by beginning new queries.
> + */
> +void
> +st_resume_queries(struct st_context *st)
> +{
> + struct simple_node *n;
> +
> + assert(st->queries_paused);
> + st->queries_paused = FALSE;
> +
> + /* Loop over active queries */
> + foreach (n, &st->active_queries.list) {
> + struct st_query_list_node *qln = (struct st_query_list_node *) n;
> + union pipe_query_result result;
> +
> + assert(qln->query->pq);
> + if (!qln->query->pq)
> + continue;
> +
> + /* get the result for the query which we ended in st_pause_queries() */
> + if (!st->pipe->get_query_result(st->pipe, qln->query->pq,
> + TRUE, &result)) {
> + debug_printf("get query result failed\n");
> + continue;
> + }
> +
> + /* Accumulate this result with previously paused result.
> + * Note that we could pause/resume a query many times so we have to
> + * accumulate here.
> + */
> + combine_query_results(&qln->query->pre_paused_result, &result,
> + qln->query->type);
> +
> + /* re-begin the query */
> + st->pipe->begin_query(st->pipe, qln->query->pq);
> + }
> +}
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.h b/src/mesa/state_tracker/st_cb_queryobj.h
> index 2406321..6de3e9e 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.h
> +++ b/src/mesa/state_tracker/st_cb_queryobj.h
> @@ -42,6 +42,11 @@ struct st_query_object
> /* Begin TIMESTAMP query for GL_TIME_ELAPSED_EXT queries */
> struct pipe_query *pq_begin;
>
> + /* Used for pausing/resuming queries. When a query result is requested,
> + * we have to add this to the returned result.
> + */
> + union pipe_query_result pre_paused_result;
> +
> unsigned type; /**< PIPE_QUERY_x */
> };
>
> @@ -59,5 +64,10 @@ st_query_object(struct gl_query_object *q)
> extern void
> st_init_query_functions(struct dd_function_table *functions);
>
> +extern void
> +st_pause_queries(struct st_context *st);
> +
> +extern void
> +st_resume_queries(struct st_context *st);
>
> #endif
> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
> index 72c23ca..224bd0d 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -305,6 +305,9 @@ st_create_context_priv( struct gl_context *ctx, struct pipe_context *pipe,
> _mesa_initialize_dispatch_tables(ctx);
> _mesa_initialize_vbo_vtxfmt(ctx);
>
> + /* Init query object list */
> + make_empty_list(&st->active_queries.list);
> +
> return st;
> }
>
> diff --git a/src/mesa/state_tracker/st_context.h b/src/mesa/state_tracker/st_context.h
> index 81d5480..ce9815c 100644
> --- a/src/mesa/state_tracker/st_context.h
> +++ b/src/mesa/state_tracker/st_context.h
> @@ -28,6 +28,7 @@
> #ifndef ST_CONTEXT_H
> #define ST_CONTEXT_H
>
> +#include "util/simple_list.h"
> #include "main/mtypes.h"
> #include "pipe/p_state.h"
> #include "state_tracker/st_api.h"
> @@ -74,7 +75,11 @@ struct st_tracked_state {
> void (*update)( struct st_context *st );
> };
>
> -
> +struct st_query_list_node
> +{
> + struct simple_node list;
> + struct st_query_object *query;
> +};
>
> struct st_context
> {
> @@ -215,6 +220,10 @@ struct st_context
> int32_t read_stamp;
>
> struct st_config_options options;
> +
> + /** List of active queries */
> + struct st_query_list_node active_queries;
> + boolean queries_paused;
> };
>
>
> diff --git a/src/mesa/state_tracker/st_gen_mipmap.c b/src/mesa/state_tracker/st_gen_mipmap.c
> index 26e1c21..956794d 100644
> --- a/src/mesa/state_tracker/st_gen_mipmap.c
> +++ b/src/mesa/state_tracker/st_gen_mipmap.c
> @@ -40,6 +40,7 @@
> #include "st_context.h"
> #include "st_texture.h"
> #include "st_gen_mipmap.h"
> +#include "st_cb_queryobj.h"
> #include "st_cb_texture.h"
>
>
> @@ -145,6 +146,8 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
> last_layer = util_max_layer(pt, baseLevel);
> }
>
> + st_pause_queries(st);
> +
> /* Try to generate the mipmap by rendering/texturing. If that fails,
> * use the software fallback.
> */
> @@ -153,6 +156,8 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
> _mesa_generate_mipmap(ctx, target, texObj);
> }
>
> + st_resume_queries(st);
> +
> /* Fill in the Mesa gl_texture_image fields */
> for (dstLevel = baseLevel + 1; dstLevel <= lastLevel; dstLevel++) {
> const uint srcLevel = dstLevel - 1;
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list