Am 22.04.2016 um 19:28 schrieb Nicolai Hähnle: > On 22.04.2016 11:52, Roland Scheidegger wrote: >> Am 22.04.2016 um 18:22 schrieb Nicolai Hähnle: >>> On 22.04.2016 08:56, Roland Scheidegger wrote: >>>> I don't quite understand why this is necessary. >>>> Couldn't you just handle such failures in the driver easily? >>> >>> How? When we need a new query buffer due to command buffer flush, and >>> that buffer allocation fails, there's no easy way out (that still >>> produces correct results). I suppose we could try to stall everything, >>> store results so far off to the side on the CPU, and then try to >>> continue by re-using the already allocated buffer. >> >> But if you couldn't end the query due to this failure, how are you going >> to return correct results anyway? > > We're not going to return correct results, but we tell the application > about it by signaling GL_OUT_OF_MEMORY.
Ah ok. Please mention that in the commit message it's just done so state trackers can return an error. Reviewed-by: Roland Scheidegger <srol...@vmware.com> > >> That's what I don't understand. I >> don't think the gl error will really help an app much. >> There's likely more functions which potentially do allocation as a side >> effect somewhere in the driver, and we don't really care about that >> neither. > > Quite likely, but IMO that's a bad thing. We should follow the spec > unless there are _very_ good reasons not to. I think the problem is that it's literally impossible to tell if a function is going to trigger allocations in a driver, thus would have to add that to just about every function. Roland > Cheers, > Nicolai > >> >> Roland >> >> >> >>> >>> But that seems like an awful lot of work + a lot of code that won't >>> really ever be tested for handling what is an out of memory condition >>> that will soon lead to failures elsewhere anyway. >>> >>> Nicolai >>> >>>> I can't >>>> quite see why informing the state tracker of it really helps. >>>> >>>> Roland >>>> >>>> Am 20.04.2016 um 17:43 schrieb Nicolai Hähnle: >>>>> From: Nicolai Hähnle <nicolai.haeh...@amd.com> >>>>> >>>>> Even when begin_query succeeds, there can still be failures in query >>>>> handling. >>>>> For example for radeon, additional buffers may have to be allocated >>>>> when >>>>> queries span multiple command buffers. >>>>> --- >>>>> src/gallium/drivers/ddebug/dd_context.c | 4 ++-- >>>>> src/gallium/drivers/freedreno/freedreno_query.c | 3 ++- >>>>> src/gallium/drivers/i915/i915_query.c | 3 ++- >>>>> src/gallium/drivers/ilo/ilo_query.c | 6 ++++-- >>>>> src/gallium/drivers/llvmpipe/lp_query.c | 4 +++- >>>>> src/gallium/drivers/noop/noop_pipe.c | 3 ++- >>>>> src/gallium/drivers/nouveau/nv30/nv30_query.c | 3 ++- >>>>> src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++- >>>>> src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 3 ++- >>>>> src/gallium/drivers/r300/r300_query.c | 8 +++++--- >>>>> src/gallium/drivers/radeon/r600_query.c | 3 ++- >>>>> src/gallium/drivers/rbug/rbug_context.c | 9 ++++++--- >>>>> src/gallium/drivers/softpipe/sp_query.c | 3 ++- >>>>> src/gallium/drivers/svga/svga_pipe_query.c | 3 ++- >>>>> src/gallium/drivers/swr/swr_query.cpp | 3 ++- >>>>> src/gallium/drivers/trace/tr_context.c | 6 ++++-- >>>>> src/gallium/drivers/vc4/vc4_query.c | 3 ++- >>>>> src/gallium/drivers/virgl/virgl_query.c | 3 ++- >>>>> src/gallium/include/pipe/p_context.h | 2 +- >>>>> 19 files changed, 49 insertions(+), 26 deletions(-) >>>>> >>>>> diff --git a/src/gallium/drivers/ddebug/dd_context.c >>>>> b/src/gallium/drivers/ddebug/dd_context.c >>>>> index 72a950a..d06efbc 100644 >>>>> --- a/src/gallium/drivers/ddebug/dd_context.c >>>>> +++ b/src/gallium/drivers/ddebug/dd_context.c >>>>> @@ -104,13 +104,13 @@ dd_context_begin_query(struct pipe_context >>>>> *_pipe, struct pipe_query *query) >>>>> return pipe->begin_query(pipe, dd_query_unwrap(query)); >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> dd_context_end_query(struct pipe_context *_pipe, struct pipe_query >>>>> *query) >>>>> { >>>>> struct dd_context *dctx = dd_context(_pipe); >>>>> struct pipe_context *pipe = dctx->pipe; >>>>> >>>>> - pipe->end_query(pipe, dd_query_unwrap(query)); >>>>> + return pipe->end_query(pipe, dd_query_unwrap(query)); >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/freedreno/freedreno_query.c >>>>> b/src/gallium/drivers/freedreno/freedreno_query.c >>>>> index a942705..18e0c79 100644 >>>>> --- a/src/gallium/drivers/freedreno/freedreno_query.c >>>>> +++ b/src/gallium/drivers/freedreno/freedreno_query.c >>>>> @@ -66,11 +66,12 @@ fd_begin_query(struct pipe_context *pctx, struct >>>>> pipe_query *pq) >>>>> return q->funcs->begin_query(fd_context(pctx), q); >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> fd_end_query(struct pipe_context *pctx, struct pipe_query *pq) >>>>> { >>>>> struct fd_query *q = fd_query(pq); >>>>> q->funcs->end_query(fd_context(pctx), q); >>>>> + return true; >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/i915/i915_query.c >>>>> b/src/gallium/drivers/i915/i915_query.c >>>>> index fa1b01d..d6015a6 100644 >>>>> --- a/src/gallium/drivers/i915/i915_query.c >>>>> +++ b/src/gallium/drivers/i915/i915_query.c >>>>> @@ -60,8 +60,9 @@ static boolean i915_begin_query(struct pipe_context >>>>> *ctx, >>>>> return true; >>>>> } >>>>> >>>>> -static void i915_end_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> +static bool i915_end_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> { >>>>> + return true; >>>>> } >>>>> >>>>> static boolean i915_get_query_result(struct pipe_context *ctx, >>>>> diff --git a/src/gallium/drivers/ilo/ilo_query.c >>>>> b/src/gallium/drivers/ilo/ilo_query.c >>>>> index 8a42f58..3088c96 100644 >>>>> --- a/src/gallium/drivers/ilo/ilo_query.c >>>>> +++ b/src/gallium/drivers/ilo/ilo_query.c >>>>> @@ -128,7 +128,7 @@ ilo_begin_query(struct pipe_context *pipe, struct >>>>> pipe_query *query) >>>>> return true; >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> ilo_end_query(struct pipe_context *pipe, struct pipe_query *query) >>>>> { >>>>> struct ilo_query *q = ilo_query(query); >>>>> @@ -136,7 +136,7 @@ ilo_end_query(struct pipe_context *pipe, struct >>>>> pipe_query *query) >>>>> if (!q->active) { >>>>> /* require ilo_begin_query() first */ >>>>> if (q->in_pairs) >>>>> - return; >>>>> + return false; >>>>> >>>>> ilo_begin_query(pipe, query); >>>>> } >>>>> @@ -144,6 +144,8 @@ ilo_end_query(struct pipe_context *pipe, struct >>>>> pipe_query *query) >>>>> q->active = false; >>>>> >>>>> ilo_query_table[q->type].end(ilo_context(pipe), q); >>>>> + >>>>> + return true; >>>>> } >>>>> >>>>> /** >>>>> diff --git a/src/gallium/drivers/llvmpipe/lp_query.c >>>>> b/src/gallium/drivers/llvmpipe/lp_query.c >>>>> index 2fddc90..d5ed656 100644 >>>>> --- a/src/gallium/drivers/llvmpipe/lp_query.c >>>>> +++ b/src/gallium/drivers/llvmpipe/lp_query.c >>>>> @@ -239,7 +239,7 @@ llvmpipe_begin_query(struct pipe_context *pipe, >>>>> struct pipe_query *q) >>>>> } >>>>> >>>>> >>>>> -static void >>>>> +static bool >>>>> llvmpipe_end_query(struct pipe_context *pipe, struct pipe_query *q) >>>>> { >>>>> struct llvmpipe_context *llvmpipe = llvmpipe_context( pipe ); >>>>> @@ -298,6 +298,8 @@ llvmpipe_end_query(struct pipe_context *pipe, >>>>> struct pipe_query *q) >>>>> default: >>>>> break; >>>>> } >>>>> + >>>>> + return true; >>>>> } >>>>> >>>>> boolean >>>>> diff --git a/src/gallium/drivers/noop/noop_pipe.c >>>>> b/src/gallium/drivers/noop/noop_pipe.c >>>>> index 55aca74..99e5f1a 100644 >>>>> --- a/src/gallium/drivers/noop/noop_pipe.c >>>>> +++ b/src/gallium/drivers/noop/noop_pipe.c >>>>> @@ -63,8 +63,9 @@ static boolean noop_begin_query(struct pipe_context >>>>> *ctx, struct pipe_query *que >>>>> return true; >>>>> } >>>>> >>>>> -static void noop_end_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> +static bool noop_end_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> { >>>>> + return true; >>>>> } >>>>> >>>>> static boolean noop_get_query_result(struct pipe_context *ctx, >>>>> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_query.c >>>>> b/src/gallium/drivers/nouveau/nv30/nv30_query.c >>>>> index cb53a36..aa9a12f 100644 >>>>> --- a/src/gallium/drivers/nouveau/nv30/nv30_query.c >>>>> +++ b/src/gallium/drivers/nouveau/nv30/nv30_query.c >>>>> @@ -175,7 +175,7 @@ nv30_query_begin(struct pipe_context *pipe, >>>>> struct pipe_query *pq) >>>>> return true; >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> nv30_query_end(struct pipe_context *pipe, struct pipe_query *pq) >>>>> { >>>>> struct nv30_context *nv30 = nv30_context(pipe); >>>>> @@ -194,6 +194,7 @@ nv30_query_end(struct pipe_context *pipe, struct >>>>> pipe_query *pq) >>>>> PUSH_DATA (push, 0); >>>>> } >>>>> PUSH_KICK (push); >>>>> + return true; >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c >>>>> b/src/gallium/drivers/nouveau/nv50/nv50_query.c >>>>> index fa70fb6..9a1397a 100644 >>>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_query.c >>>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c >>>>> @@ -54,11 +54,12 @@ nv50_begin_query(struct pipe_context *pipe, >>>>> struct pipe_query *pq) >>>>> return q->funcs->begin_query(nv50_context(pipe), q); >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> nv50_end_query(struct pipe_context *pipe, struct pipe_query *pq) >>>>> { >>>>> struct nv50_query *q = nv50_query(pq); >>>>> q->funcs->end_query(nv50_context(pipe), q); >>>>> + return true; >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c >>>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c >>>>> index b34271c..91fb72f 100644 >>>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c >>>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c >>>>> @@ -58,11 +58,12 @@ nvc0_begin_query(struct pipe_context *pipe, >>>>> struct pipe_query *pq) >>>>> return q->funcs->begin_query(nvc0_context(pipe), q); >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> nvc0_end_query(struct pipe_context *pipe, struct pipe_query *pq) >>>>> { >>>>> struct nvc0_query *q = nvc0_query(pq); >>>>> q->funcs->end_query(nvc0_context(pipe), q); >>>>> + return true; >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/r300/r300_query.c >>>>> b/src/gallium/drivers/r300/r300_query.c >>>>> index 7603985..8557eb7 100644 >>>>> --- a/src/gallium/drivers/r300/r300_query.c >>>>> +++ b/src/gallium/drivers/r300/r300_query.c >>>>> @@ -110,7 +110,7 @@ void r300_stop_query(struct r300_context *r300) >>>>> r300->query_current = NULL; >>>>> } >>>>> >>>>> -static void r300_end_query(struct pipe_context* pipe, >>>>> +static bool r300_end_query(struct pipe_context* pipe, >>>>> struct pipe_query* query) >>>>> { >>>>> struct r300_context* r300 = r300_context(pipe); >>>>> @@ -120,16 +120,18 @@ static void r300_end_query(struct pipe_context* >>>>> pipe, >>>>> pb_reference(&q->buf, NULL); >>>>> r300_flush(pipe, RADEON_FLUSH_ASYNC, >>>>> (struct pipe_fence_handle**)&q->buf); >>>>> - return; >>>>> + return true; >>>>> } >>>>> >>>>> if (q != r300->query_current) { >>>>> fprintf(stderr, "r300: end_query: Got invalid query.\n"); >>>>> assert(0); >>>>> - return; >>>>> + return false; >>>>> } >>>>> >>>>> r300_stop_query(r300); >>>>> + >>>>> + return true; >>>>> } >>>>> >>>>> static boolean r300_get_query_result(struct pipe_context* pipe, >>>>> diff --git a/src/gallium/drivers/radeon/r600_query.c >>>>> b/src/gallium/drivers/radeon/r600_query.c >>>>> index de6e37b..813885b 100644 >>>>> --- a/src/gallium/drivers/radeon/r600_query.c >>>>> +++ b/src/gallium/drivers/radeon/r600_query.c >>>>> @@ -725,12 +725,13 @@ boolean r600_query_hw_begin(struct >>>>> r600_common_context *rctx, >>>>> return true; >>>>> } >>>>> >>>>> -static void r600_end_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> +static bool r600_end_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> { >>>>> struct r600_common_context *rctx = (struct r600_common_context >>>>> *)ctx; >>>>> struct r600_query *rquery = (struct r600_query *)query; >>>>> >>>>> rquery->ops->end(rctx, rquery); >>>>> + return true; >>>>> } >>>>> >>>>> void r600_query_hw_end(struct r600_common_context *rctx, >>>>> diff --git a/src/gallium/drivers/rbug/rbug_context.c >>>>> b/src/gallium/drivers/rbug/rbug_context.c >>>>> index 1280c45..38dee74 100644 >>>>> --- a/src/gallium/drivers/rbug/rbug_context.c >>>>> +++ b/src/gallium/drivers/rbug/rbug_context.c >>>>> @@ -178,17 +178,20 @@ rbug_begin_query(struct pipe_context *_pipe, >>>>> return ret; >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> rbug_end_query(struct pipe_context *_pipe, >>>>> struct pipe_query *query) >>>>> { >>>>> struct rbug_context *rb_pipe = rbug_context(_pipe); >>>>> struct pipe_context *pipe = rb_pipe->pipe; >>>>> + bool ret; >>>>> >>>>> pipe_mutex_lock(rb_pipe->call_mutex); >>>>> - pipe->end_query(pipe, >>>>> - query); >>>>> + ret = pipe->end_query(pipe, >>>>> + query); >>>>> pipe_mutex_unlock(rb_pipe->call_mutex); >>>>> + >>>>> + return ret; >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/softpipe/sp_query.c >>>>> b/src/gallium/drivers/softpipe/sp_query.c >>>>> index 81e9710..bec0116 100644 >>>>> --- a/src/gallium/drivers/softpipe/sp_query.c >>>>> +++ b/src/gallium/drivers/softpipe/sp_query.c >>>>> @@ -134,7 +134,7 @@ softpipe_begin_query(struct pipe_context *pipe, >>>>> struct pipe_query *q) >>>>> } >>>>> >>>>> >>>>> -static void >>>>> +static bool >>>>> softpipe_end_query(struct pipe_context *pipe, struct pipe_query *q) >>>>> { >>>>> struct softpipe_context *softpipe = softpipe_context( pipe ); >>>>> @@ -201,6 +201,7 @@ softpipe_end_query(struct pipe_context *pipe, >>>>> struct pipe_query *q) >>>>> break; >>>>> } >>>>> softpipe->dirty |= SP_NEW_QUERY; >>>>> + return true; >>>>> } >>>>> >>>>> >>>>> diff --git a/src/gallium/drivers/svga/svga_pipe_query.c >>>>> b/src/gallium/drivers/svga/svga_pipe_query.c >>>>> index 75bc9ce..05b756a 100644 >>>>> --- a/src/gallium/drivers/svga/svga_pipe_query.c >>>>> +++ b/src/gallium/drivers/svga/svga_pipe_query.c >>>>> @@ -945,7 +945,7 @@ svga_begin_query(struct pipe_context *pipe, >>>>> struct pipe_query *q) >>>>> } >>>>> >>>>> >>>>> -static void >>>>> +static bool >>>>> svga_end_query(struct pipe_context *pipe, struct pipe_query *q) >>>>> { >>>>> struct svga_context *svga = svga_context(pipe); >>>>> @@ -1057,6 +1057,7 @@ svga_end_query(struct pipe_context *pipe, >>>>> struct pipe_query *q) >>>>> assert(!"unexpected query type in svga_end_query()"); >>>>> } >>>>> svga->sq[sq->type] = NULL; >>>>> + return true; >>>>> } >>>>> >>>>> >>>>> diff --git a/src/gallium/drivers/swr/swr_query.cpp >>>>> b/src/gallium/drivers/swr/swr_query.cpp >>>>> index e4b8b68..ff4d9b3 100644 >>>>> --- a/src/gallium/drivers/swr/swr_query.cpp >>>>> +++ b/src/gallium/drivers/swr/swr_query.cpp >>>>> @@ -281,7 +281,7 @@ swr_begin_query(struct pipe_context *pipe, struct >>>>> pipe_query *q) >>>>> return true; >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> swr_end_query(struct pipe_context *pipe, struct pipe_query *q) >>>>> { >>>>> struct swr_context *ctx = swr_context(pipe); >>>>> @@ -295,6 +295,7 @@ swr_end_query(struct pipe_context *pipe, struct >>>>> pipe_query *q) >>>>> pq->result = &pq->end; >>>>> pq->enable_stats = FALSE; >>>>> swr_gather_stats(pipe, pq); >>>>> + return true; >>>>> } >>>>> >>>>> >>>>> diff --git a/src/gallium/drivers/trace/tr_context.c >>>>> b/src/gallium/drivers/trace/tr_context.c >>>>> index b575f2c..7ed4f49 100644 >>>>> --- a/src/gallium/drivers/trace/tr_context.c >>>>> +++ b/src/gallium/drivers/trace/tr_context.c >>>>> @@ -218,12 +218,13 @@ trace_context_begin_query(struct pipe_context >>>>> *_pipe, >>>>> } >>>>> >>>>> >>>>> -static void >>>>> +static bool >>>>> trace_context_end_query(struct pipe_context *_pipe, >>>>> struct pipe_query *query) >>>>> { >>>>> struct trace_context *tr_ctx = trace_context(_pipe); >>>>> struct pipe_context *pipe = tr_ctx->pipe; >>>>> + bool ret; >>>>> >>>>> query = trace_query_unwrap(query); >>>>> >>>>> @@ -232,9 +233,10 @@ trace_context_end_query(struct pipe_context >>>>> *_pipe, >>>>> trace_dump_arg(ptr, pipe); >>>>> trace_dump_arg(ptr, query); >>>>> >>>>> - pipe->end_query(pipe, query); >>>>> + ret = pipe->end_query(pipe, query); >>>>> >>>>> trace_dump_call_end(); >>>>> + return ret; >>>>> } >>>>> >>>>> >>>>> diff --git a/src/gallium/drivers/vc4/vc4_query.c >>>>> b/src/gallium/drivers/vc4/vc4_query.c >>>>> index 17400a3..ddf8f8f 100644 >>>>> --- a/src/gallium/drivers/vc4/vc4_query.c >>>>> +++ b/src/gallium/drivers/vc4/vc4_query.c >>>>> @@ -56,9 +56,10 @@ vc4_begin_query(struct pipe_context *ctx, struct >>>>> pipe_query *query) >>>>> return true; >>>>> } >>>>> >>>>> -static void >>>>> +static bool >>>>> vc4_end_query(struct pipe_context *ctx, struct pipe_query *query) >>>>> { >>>>> + return true; >>>>> } >>>>> >>>>> static boolean >>>>> diff --git a/src/gallium/drivers/virgl/virgl_query.c >>>>> b/src/gallium/drivers/virgl/virgl_query.c >>>>> index 5173bd3..4ddb925 100644 >>>>> --- a/src/gallium/drivers/virgl/virgl_query.c >>>>> +++ b/src/gallium/drivers/virgl/virgl_query.c >>>>> @@ -107,7 +107,7 @@ static boolean virgl_begin_query(struct >>>>> pipe_context *ctx, >>>>> return true; >>>>> } >>>>> >>>>> -static void virgl_end_query(struct pipe_context *ctx, >>>>> +static bool virgl_end_query(struct pipe_context *ctx, >>>>> struct pipe_query *q) >>>>> { >>>>> struct virgl_context *vctx = virgl_context(ctx); >>>>> @@ -121,6 +121,7 @@ static void virgl_end_query(struct pipe_context >>>>> *ctx, >>>>> >>>>> >>>>> virgl_encoder_end_query(vctx, query->handle); >>>>> + return true; >>>>> } >>>>> >>>>> static boolean virgl_get_query_result(struct pipe_context *ctx, >>>>> diff --git a/src/gallium/include/pipe/p_context.h >>>>> b/src/gallium/include/pipe/p_context.h >>>>> index 82efaf5..9d7a8eb 100644 >>>>> --- a/src/gallium/include/pipe/p_context.h >>>>> +++ b/src/gallium/include/pipe/p_context.h >>>>> @@ -140,7 +140,7 @@ struct pipe_context { >>>>> struct pipe_query *q); >>>>> >>>>> boolean (*begin_query)(struct pipe_context *pipe, struct >>>>> pipe_query *q); >>>>> - void (*end_query)(struct pipe_context *pipe, struct pipe_query >>>>> *q); >>>>> + bool (*end_query)(struct pipe_context *pipe, struct pipe_query >>>>> *q); >>>>> >>>>> /** >>>>> * Get results of a query. >>>>> >>>> >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev