Re: [Mesa-dev] [PATCH v2 2/2] i965: Use the predicate enable bit for conditional rendering without stalling

2015-01-09 Thread Neil Roberts
Daniel Vetter dan...@ffwll.ch writes:

 Oh, I guess my earlier mail was too late. One issue still is picking
 the numbers, since you seem to assume here that ver = 2 means the
 stuff actually works. But like Ken said the cmd parser in upstream
 isn't really enabled yet.

The patch only enables the predicate enable bit if the ver = 2 *and* it
can do register writes. The register write test already exists to check
whether we can do some transform feedback extensions and indirect
rendering. Therefore I think the patch is safe to land. It will
currently work on IvyBridge with master of Linus' kernel tree and
presumably it will magically start working on Haswell+ if we disable the
hardware validator there too. In the meantime I think it will safely
detect that the feature isn't available and fallback to the old blocking
path.

Considering that, would anyone be able to review the patch? It'd be a
shame to forget about this feature.

- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 2/2] i965: Use the predicate enable bit for conditional rendering without stalling

2014-11-09 Thread Daniel Vetter
On Fri, Nov 07, 2014 at 06:53:00PM +, Neil Roberts wrote:
 Previously whenever a primitive is drawn the driver would call
 _mesa_check_conditional_render which blocks waiting for the result of the
 query to determine whether to render. On Gen7+ there is a bit in the
 3DPRIMITIVE command which can be used to disable the primitive based on the
 value of a state bit. This state bit can be set based on whether two registers
 have different values using the MI_PREDICATE command. We can load these two
 registers with the pixel count values stored in the query begin and end to
 implement conditional rendering without stalling.
 
 Unfortunately these two source registers are not in the whitelist of available
 registers in the kernel driver so this needs a kernel patch to work. This
 patch uses the command parser version from intel_screen to detect whether to
 attempt to set the predicate data registers.
 
 The predicate enable bit is currently only used for drawing 3D primitives. For
 blits, clears, bitmaps, copypixels and drawpixels it still causes a stall. For
 most of these it would probably just work to call the new
 brw_check_conditional_render function instead of
 _mesa_check_conditional_render because they already work in terms of rendering
 primitives. However it's a bit trickier for blits because it can use the BLT
 ring or the blorp codepath. I think these operations are less useful for
 conditional rendering than rendering primitives so it might be best to leave
 it for a later patch.
 
 v2: Use the command parser version to detect whether we can write to the
 predicate data registers instead of trying to execute a register load
 command.
 ---
 
 Glenn Kennard suggested that instead of trying to send a load register
 command to detect whether the predicate source registers can be set we
 could just increase the command parser version in the kernel driver
 and query that. That seems nicer to me so here is a second version of
 the patch to do that. I will post a v2 of the kernel patch too.

Oh, I guess my earlier mail was too late. One issue still is picking the
numbers, since you seem to assume here that ver = 2 means the stuff
actually works. But like Ken said the cmd parser in upstream isn't
really enabled yet.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 2/2] i965: Use the predicate enable bit for conditional rendering without stalling

2014-11-07 Thread Neil Roberts
Previously whenever a primitive is drawn the driver would call
_mesa_check_conditional_render which blocks waiting for the result of the
query to determine whether to render. On Gen7+ there is a bit in the
3DPRIMITIVE command which can be used to disable the primitive based on the
value of a state bit. This state bit can be set based on whether two registers
have different values using the MI_PREDICATE command. We can load these two
registers with the pixel count values stored in the query begin and end to
implement conditional rendering without stalling.

Unfortunately these two source registers are not in the whitelist of available
registers in the kernel driver so this needs a kernel patch to work. This
patch uses the command parser version from intel_screen to detect whether to
attempt to set the predicate data registers.

The predicate enable bit is currently only used for drawing 3D primitives. For
blits, clears, bitmaps, copypixels and drawpixels it still causes a stall. For
most of these it would probably just work to call the new
brw_check_conditional_render function instead of
_mesa_check_conditional_render because they already work in terms of rendering
primitives. However it's a bit trickier for blits because it can use the BLT
ring or the blorp codepath. I think these operations are less useful for
conditional rendering than rendering primitives so it might be best to leave
it for a later patch.

v2: Use the command parser version to detect whether we can write to the
predicate data registers instead of trying to execute a register load
command.
---

Glenn Kennard suggested that instead of trying to send a load register
command to detect whether the predicate source registers can be set we
could just increase the command parser version in the kernel driver
and query that. That seems nicer to me so here is a second version of
the patch to do that. I will post a v2 of the kernel patch too.

 src/mesa/drivers/dri/i965/Makefile.sources |   1 +
 src/mesa/drivers/dri/i965/brw_conditional_render.c | 167 +
 src/mesa/drivers/dri/i965/brw_context.c|   4 +
 src/mesa/drivers/dri/i965/brw_context.h|  21 +++
 src/mesa/drivers/dri/i965/brw_defines.h|   1 +
 src/mesa/drivers/dri/i965/brw_draw.c   |  16 +-
 src/mesa/drivers/dri/i965/brw_queryobj.c   |  17 ++-
 src/mesa/drivers/dri/i965/intel_extensions.c   |   5 +
 src/mesa/drivers/dri/i965/intel_reg.h  |  23 +++
 9 files changed, 243 insertions(+), 12 deletions(-)
 create mode 100644 src/mesa/drivers/dri/i965/brw_conditional_render.c

diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
b/src/mesa/drivers/dri/i965/Makefile.sources
index 711aabe..0adaf4d 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -38,6 +38,7 @@ i965_FILES = \
brw_clip_tri.c \
brw_clip_unfilled.c \
brw_clip_util.c \
+   brw_conditional_render.c \
brw_context.c \
brw_cubemap_normalize.cpp \
brw_curbe.c \
diff --git a/src/mesa/drivers/dri/i965/brw_conditional_render.c 
b/src/mesa/drivers/dri/i965/brw_conditional_render.c
new file mode 100644
index 000..e676aa0
--- /dev/null
+++ b/src/mesa/drivers/dri/i965/brw_conditional_render.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Neil Roberts n...@linux.intel.com
+ */
+
+/** @file brw_conditional_render.c
+ *
+ * Support for conditional rendering based on query objects
+ * (GL_NV_conditional_render, GL_ARB_conditional_render_inverted) on Gen7+.
+ */
+
+#include main/imports.h
+#include main/condrender.h
+
+#include brw_context.h
+#include brw_defines.h
+#include intel_batchbuffer.h
+
+static void
+set_predicate_enable(struct brw_context *brw,
+ bool value)
+{
+