Re: [Mesa-dev] [PATCH 1/4] gallium: add bool return to pipe_context::end_query

Fri, 22 Apr 2016 10:36:43 -0700 

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.


> 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.


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























					Reply via email to