[Mesa-dev] [PATCH] st/mesa: implement query pause/resume for meta operations
Marek Olšák
maraeo at gmail.com
Mon Aug 3 17:18:45 PDT 2015
My understanding was that pipe->clear, blit, and resource_copy_region
should not increment query values (except for TIME_ELAPSED).
Currently, all radeon drivers implement this in the same way as this
patch does (it's done more efficiently though), but I've been meaning
to get rid of that and just turn off the global hardware counters
without touching the active queries, which would be the best solution.
If there are drivers which want this to be done in the state tracker,
fine, but please don't enable this code for the rest.
Like Ilia said, a new gallium context state would fix clear_with_quad
for radeon and nouveau at least.
Marek
On Tue, Aug 4, 2015 at 1:00 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> 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
> _______________________________________________
> 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