Re: [Mesa-dev] [PATCH V2] glsl: remove element_type() helper
Reviewed-by: Matt Turner matts...@gmail.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 15/20] i915: Enable intel_render path for points
On Fri, May 15, 2015 at 12:18:11PM -0700, Ian Romanick wrote:
There are some really twitchy tests in ES1 (and possibly ES2)
conformance related to this. Do any of those tests change with this commit?
I did run some ES1 conformnce tests, but the branches in the repo
were not very clear so I'm not sure if I ran the right thing (looks
like I used the gles1 branch and managed to build something that
at least runs).
No changes in the results on 855 or PNV between my branch and the
baseline AFAICS.
I'll see if I can get the ES2 tests built as well, and run them on
PNV.
On 03/23/2015 05:47 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
The sub-pixel adjustment for points was killed off in
commit 60d762aa625095a8c1f9597d8530bb5a6fa61b4c
Author: Xiang, Haihao haihao.xi...@intel.com
Date: Wed Jan 2 11:38:51 2008 +0800
i915: Needn't adjust pixel centers. fix #12944
so if we don't need it in intel_tris.c we don't need it in
intel_render.c either, which means we can allow
intel_render.c to render points.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
src/mesa/drivers/dri/i915/intel_render.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/mesa/drivers/dri/i915/intel_render.c
b/src/mesa/drivers/dri/i915/intel_render.c
index 65ecd05..ef1c718 100644
--- a/src/mesa/drivers/dri/i915/intel_render.c
+++ b/src/mesa/drivers/dri/i915/intel_render.c
@@ -54,9 +54,7 @@
* dma buffers. Use strip/fan hardware primitives where possible.
* Try to simulate missing primitives with indexed vertices.
*/
-#define HAVE_POINTS 0 /* Has it, but can't use because subpixel
has to
- * be adjusted for points on the
INTEL/I845G
- */
+#define HAVE_POINTS 1
#define HAVE_LINES 1
#define HAVE_LINE_STRIPS 1
#define HAVE_TRIANGLES 1
@@ -70,7 +68,7 @@
#define HAVE_ELTS0
static const uint32_t hw_prim[GL_POLYGON + 1] = {
- [GL_POINTS] = 0,
+ [GL_POINTS] = PRIM3D_POINTLIST,
[GL_LINES ] = PRIM3D_LINELIST,
[GL_LINE_LOOP] = PRIM3D_LINESTRIP,
[GL_LINE_STRIP] = PRIM3D_LINESTRIP,
@@ -96,7 +94,7 @@ static const GLenum reduced_prim[GL_POLYGON + 1] = {
};
static const int scale_prim[GL_POLYGON + 1] = {
- [GL_POINTS] = 0, /* fallback case */
+ [GL_POINTS] = 1,
[GL_LINES] = 1,
[GL_LINE_LOOP] = 2,
[GL_LINE_STRIP] = 2,
--
Ville Syrjälä
Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] glsl: set the binding value regardless explicit_binding
On Thu, 2015-05-21 at 12:22 +0200, Alejandro Piñeiro wrote: On 20/05/15 23:39, Timothy Arceri wrote: On Thu, 2015-05-14 at 22:49 +0200, Alejandro Piñeiro wrote: On 14/05/15 20:38, Ian Romanick wrote: I think I see what's happening. I don't think I understood atomic.buffer_index well enough when I removed it. :( In binding=3 case, what value does glGetActiveAtomicCounterBufferiv(prog, 0, GL_ATOMIC_COUNTER_BUFFER_BINDING, param); give? My guess is that it gives 0 instead of the binding specified in the shader. With the test I uploaded today, so with binding points 3 and 6, and using index 0 and 1, calling glGetActiveAtomicCounterBufferiv properly returns 3 and 6. Both with and without the patch of this thread. The correct binding is stored in the gl_active_atomic_buffer struct which queried by glGetActiveAtomicCounterBufferiv() Ok. This would be a good addition to the test. Ok. The index and the binding are different. The index is which atomic is this in the shader, and the binding is which binding point is used to attach a buffer to this atomic. Thanks to c0cd5b, I think somewhere we're using one value when we actually want the other... probably the last hunk: --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -2294,7 +2294,7 @@ vec4_visitor::visit_atomic_counter_intrinsic(ir_call *ir) ir-actual_parameters.get_head()); ir_variable *location = deref-variable_referenced(); unsigned surf_index = (prog_data-base.binding_table.abo_start + - location-data.atomic.buffer_index); + location-data.binding); /* Calculate the surface offset */ src_reg offset(this, glsl_type::uint_type); Maybe try adding a utility function that will search gl_shader_program::UniformStorage for location-name and use the atomic_buffer_index stored there for use in the i965 driver? Taking into account that glGetActiveAtomicCounterBufferiv is already working, do you think that this is still needed? For clarity its probably a good idea to implement Ian's suggestion. Changing the binding value to the atomic buffer index (even though it seems to be unused elsewhere after this point) is a bit confusing. This email conversation is a good enough example of the confusion doing this can cause. If you implement Ian's suggestion you can probably remove the if statement too as the binding value is not used after this point. Probably I'm missing a global view of the situation, but taking into account that one of the conclusions of this email conversation is that the index and the binding are different, and removing one of the variables from ir_variable::data.atomic is causing some confusion (and the regression), shouldn't it easier to just revert the change? Commit c0cd5b explains that the removal helped to reduce memory consumption, but as far as I understand, it was done because it was concluded that was not needed. It was part of a memory reduction series, so I'm not sure it should be reverted. I'm sure Ian would have suggested doing so if he thought that was the better option: http://lists.freedesktop.org/archives/mesa-dev/2014-July/063303.html Although I did notice this in the glsl to nir nir code: /* XXX Get rid of buffer_index */ var-data.atomic.buffer_index = ir-data.binding; So this may be a problem too.. Well, if the change is reverted, it is just about removing that comment (assuming that the comment was added for the same reason commit c0cd5b was done). If the change was to be reverted this would probably need to be changed to var-data.atomic.buffer_index = ir-data.atomic.buffer_index; as nir didn't exist when the change was made. BR ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] New stable-branch 10.5 candidate pushed
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello list, The candidate for the Mesa 10.5.6 is now available. The current patch queue is as follows: - 30 queued - 8 nominated (outstanding) - and 1 rejected (obsolete) patches The queue covers nearly every aspect of mesa - core mesa fixes, driver specific patches (i965, nouveau, freedreno), GLX and EGL loader patches, OpenCL (clover) and even Darwin build fixes. Take a look at section Mesa stable queue for more information. Testing - --- The following results are against piglit 305ecc3ac89. Changes - classic i965(snb) - --- None. Changes - swrast classic - None. Changes - gallium softpipe - -- None. Fixes: - spec + arb_texture_buffer_object + formats (vs, 3.1 core) + GL_RG32F pass fail The above pass was introduced with Mesa 10.5.5 and the result is intermetent. Changes - gallium llvmpipe (LLVM 3.6) - - None. Testing reports/general approval - Any testing reports (or general approval of the state of the branch) will be greatly appreciated. Trivial merge conflicts - --- None. The plan is to have 10.5.6 this Friday(22th of May). If you have any questions or comments that you would like to share before the release, please go ahead. Cheers, Emil Mesa stable queue - - Nominated (8) = Alejandro Piñeiro (1): glsl: set the binding value regardless explicit_binding Boyan Ding (1): i915: Add XRGB format to intel_screen_make_configs Brian Paul (1): configure: don't try to build gallium DRI drivers if --disable-dri is set Tapani Pälli (2): glsl: Allow dynamic sampler array indexing with GLSL ES 3.00 glsl: validate sampler array indexing for 'constant-index-expression' Tom Stellard (3): clover: Call clBuildProgram() notification function when build completes v2 gallium/drivers: Add threadsafe wrappers for pipe_context and pipe_screen clover: Use threadsafe wrappers for pipe_screen and pipe_context Rejected/Obsolete (1) = Alejandro Piñeiro (1): glsl: properly setting var-data.binding if explicit_binding is true Queued (30) === Alex Deucher (1): radeonsi: add new bonaire pci id Axel Davy (2): egl/wayland: properly destroy wayland objects glx/dri3: Add additional check for gpu offloading case Emil Velikov (3): docs: Add sha256 sums for the 10.5.5 release egl/main: fix EGL_KHR_get_all_proc_addresses targets/osmesa: drop the -module tag from LDFLAGS Francisco Jerez (4): clover: Refactor event::trigger and ::abort to prevent deadlock and reentrancy issues. clover: Wrap event::_status in a method to prevent unlocked access. clover: Implement locking of the wait_count, _chain and _status members of event. i965: Fix PBO cache coherency issue after _mesa_meta_pbo_GetTexSubImage(). Fredrik Höglund (2): main: Require that the texture exists in framebuffer_texture mesa: Generate GL_INVALID_VALUE in framebuffer_texture when layer 0 Ilia Mirkin (7): nv50/ir: only propagate saturate up if some actual folding took place nv50: keep track of PGRAPH state in nv50_screen nvc0: keep track of PGRAPH state in nvc0_screen nvc0: reset the instanced elements state when doing blit using 3d engine nv50/ir: only enable mul saturate on G200+ st/mesa: make sure to create a clean bool when doing i2b nvc0: switch mechanism for shader eviction to be a while loop Jeremy Huddleston Sequoia (2): swrast: Build fix for darwin darwin: Fix install name of libOSMesa Laura Ekstrand (2): main: Fix an error generated by FramebufferTexture main: Complete error conditions for glInvalidate*Framebuffer. Marta Lofstedt (1): main: glGetIntegeri_v fails for GL_VERTEX_BINDING_STRIDE Rob Clark (2): freedreno: enable a306 freedreno: fix bug in tile/slot calculation Roland Scheidegger (1): draw: (trivial) fix out-of-bounds vector initialization Tim Rowley (1): mesa: fix shininess check for ffvertex_prog v2 Tom Stellard (2): clover: Add a mutex to guard queue::queued_events clover: Fix a bug with multi-threaded events v2 -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQIcBAEBAgAGBQJVXeHmAAoJEO2uN7As60kN/gAP/0cW9KCcAm09rfQNHYPYDHfL A+ZKmW47phUpV4qZ6bTRZAiUeMo9C3uma+PN5UT6hgqOodIPQJrmbedf8BoELFOM x/XvlOD5nnTb6FiB3PFihuDH7TBxP6y/4Py9Uqg2UtQ8VU/MCMNxTXAVwGoafTZ1 BzS9uQoV4lbe80QagC+YIVXXrcRXpd1htXZot1sDGwixcaYXXM6WHT76/V51wBct kYPFFCezOtSS8Ji0J1wtuEv8JSMfFviY0I9/nyyLjNM4ywj/7J3GuQatF0grIE1d yzjZunhe9iMKaM3ZZbLKDe66VJZGSzEhUe1xfRjQD5rMG0Yofms78NKQE4XhT+jN TfPQaBWyaTPIXBtElfeRYUTfEe2R+pI0E2bCSxf7ngSJYrP/eOw5qftDfJem3Z2g
[Mesa-dev] [v3 PATCH 08/10] i965: ensure execution of fragment shader when fragment shader has atomic buffer access
From: Kevin Rogovin kevin.rogo...@intel.com
Ensure that the GPU spawns the fragment shader thread for those
fragment shaders with atomic buffer access.
v1 - v2
No change.
v2 - v3
Use utility function _mesa_active_fragment_shader_has_atomic_ops().
Reviewed-by: Tapani Pälli tapani.palli at intel.com (v1)
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/drivers/dri/i965/gen7_wm_state.c | 4
src/mesa/drivers/dri/i965/gen8_ps_state.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c
b/src/mesa/drivers/dri/i965/gen7_wm_state.c
index 1c47076..63092cf 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
@@ -77,6 +77,10 @@ upload_wm_state(struct brw_context *brw)
dw1 |= GEN7_WM_KILL_ENABLE;
}
+ if (_mesa_active_fragment_shader_has_atomic_ops(brw-ctx) ) {
+ dw1 |= GEN7_WM_DISPATCH_ENABLE;
+ }
+
/* _NEW_BUFFERS | _NEW_COLOR */
if (brw_color_buffer_write_enabled(brw) || writes_depth ||
dw1 GEN7_WM_KILL_ENABLE) {
diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c
b/src/mesa/drivers/dri/i965/gen8_ps_state.c
index 85ad3b6..3dee8b6 100644
--- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
@@ -58,6 +58,9 @@ gen8_upload_ps_extra(struct brw_context *brw,
if (prog_data-uses_omask)
dw1 |= GEN8_PSX_OMASK_TO_RENDER_TARGET;
+ if (_mesa_active_fragment_shader_has_atomic_ops(brw-ctx) )
+ dw1 |= GEN8_PSX_SHADER_HAS_UAV;
+
BEGIN_BATCH(2);
OUT_BATCH(_3DSTATE_PS_EXTRA 16 | (2 - 2));
OUT_BATCH(dw1);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v3 PATCH 07/10] mesa: add helper function for testing if current fragment shader has atomics
From: Kevin Rogovin kevin.rogo...@intel.com
Add helper function that checks if current fragment shader active
of gl_context has atomic buffer access.
v1 - v3
Added in v3 of patch series.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/mtypes.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 7e1f0e0..b88b10a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4464,7 +4464,12 @@ enum _debug
DEBUG_INCOMPLETE_FBO = (1 3)
};
-
+static inline bool
+_mesa_active_fragment_shader_has_atomic_ops(const struct gl_context *ctx)
+{
+ return ctx-Shader._CurrentFragmentProgram!=NULL
+ ctx-Shader._CurrentFragmentProgram-NumAtomicBuffers 0;
+}
#ifdef __cplusplus
}
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
From: Kevin Rogovin kevin.rogo...@intel.com
Define the infrastructure for the extension GL_ARB_framebuffer_no_attachments:
- extension table
- additions to gl_framebuffer
v1 - v2
Spacing and trailing spaces fixes.
v2 - v3
mtypes.h: Correct comment on _HasAttachments.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/extensions.c | 1 +
src/mesa/main/fbobject.c| 1 +
src/mesa/main/framebuffer.c | 1 +
src/mesa/main/mtypes.h | 50 -
4 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index c82416a..3256b2c 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -117,6 +117,7 @@ static const struct extension extension_table[] = {
{ GL_ARB_fragment_program,o(ARB_fragment_program),
GLL,2002 },
{ GL_ARB_fragment_program_shadow,
o(ARB_fragment_program_shadow), GLL,2003 },
{ GL_ARB_fragment_shader, o(ARB_fragment_shader),
GL, 2002 },
+ { GL_ARB_framebuffer_no_attachments,
o(ARB_framebuffer_no_attachments), GL, 2012 },
{ GL_ARB_framebuffer_object, o(ARB_framebuffer_object),
GL, 2005 },
{ GL_ARB_framebuffer_sRGB,o(EXT_framebuffer_sRGB),
GL, 1998 },
{ GL_ARB_get_program_binary, o(dummy_true),
GL, 2010 },
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 1859c27..8fea7f8 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -957,6 +957,7 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
fb-Height = 0;
fb-_AllColorBuffersFixedPoint = GL_TRUE;
fb-_HasSNormOrFloatColorBuffer = GL_FALSE;
+ fb-_HasAttachments = GL_TRUE;
/* Start at -2 to more easily loop over all attachment points.
* -2: depth buffer
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index 665a5ba..c2cfb92 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -157,6 +157,7 @@ _mesa_initialize_window_framebuffer(struct gl_framebuffer
*fb,
fb-_Status = GL_FRAMEBUFFER_COMPLETE_EXT;
fb-_AllColorBuffersFixedPoint = !visual-floatMode;
fb-_HasSNormOrFloatColorBuffer = visual-floatMode;
+ fb-_HasAttachments = GL_TRUE;
compute_depth_max(fb);
}
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 8342517..1a37aa6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3146,12 +3146,29 @@ struct gl_framebuffer
*/
struct gl_config Visual;
- GLuint Width, Height; /** size of frame buffer in pixels */
+ /**
+* size of frame buffer in pixels,
+* no attachments has these values as 0
+*/
+ GLuint Width, Height;
+
+ /**
+* In the case that the framebuffer has no attachment (i.e.
+* GL_ARB_framebuffer_no_attachments) then the geometry of
+* the framebuffer is specified by the default values.
+*/
+ struct {
+ GLuint Width, Height, Layers, NumSamples;
+ GLboolean FixedSampleLocations;
+ } DefaultGeometry;
- /** \name Drawing bounds (Intersection of buffer size and scissor box) */
+ /** \name Drawing bounds (Intersection of buffer size and scissor box)
+* The drawing region is given by [_Xmin, _Xmax) x [_Ymin, _Ymax),
+* (inclusive for _Xmin and _Ymin while exclusive for _Xmax and _Ymax)
+*/
/*@{*/
- GLint _Xmin, _Xmax; /** inclusive */
- GLint _Ymin, _Ymax; /** exclusive */
+ GLint _Xmin, _Xmax;
+ GLint _Ymin, _Ymax;
/*@}*/
/** \name Derived Z buffer stuff */
@@ -3164,6 +3181,18 @@ struct gl_framebuffer
/** One of the GL_FRAMEBUFFER_(IN)COMPLETE_* tokens */
GLenum _Status;
+ /** True if one of Attachment has Type != GL_NONE
+* NOTE: the values for Width and Height are set to 0 in
+* case of no attachments, a backend driver supporting
+* GL_ARB_framebuffer_no_attachments must check for the
+* flag _HasAttachments and if GL_FALSE, must then use
+* the values in DefaultGeometry to initialize its
+* viewport, scissor and so on (in particular _Xmin,
+* _Xmax, _Ymin and _Ymax do NOT take into account
+* _HasAttachments being false)
+*/
+ GLboolean _HasAttachments;
+
/** Integer color values */
GLboolean _IntegerColor;
@@ -3174,7 +3203,9 @@ struct gl_framebuffer
/**
* The maximum number of layers in the framebuffer, or 0 if the framebuffer
* is not layered. For cube maps and cube map arrays, each cube face
-* counts as a layer.
+* counts as a layer. As the case for Width, Height a backend driver
+* supporting GL_ARB_framebuffer_no_attachments must use DefaultGeometry
+* in the case that
[Mesa-dev] [v3 PATCH 06/10] i965: Use _mesa_geometry_ functions appropriately
From: Kevin Rogovin kevin.rogo...@intel.com
Change references to gl_framebuffer::Width, Height, MaxNumLayers
and Visual::samples to use the _mesa_geometry_ convenience functions
for those places where the geometry of the gl_framebuffer is needed
(in contrast to the geometry of the intersection of the attachments
of the gl_framebuffer).
This patch is to pave the way to enable GL_ARB_framebuffer_no_attachments
on Gen7 and higher in i965.
v1 - v2
Remove changes that would only be active in Gen4/5.
Type and casting changes for consistency and readability.
v2 - v3
Updates for rebase against master
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/drivers/dri/i965/brw_clip_state.c | 9 ++---
src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +++---
src/mesa/drivers/dri/i965/brw_sf_state.c | 8
src/mesa/drivers/dri/i965/brw_state_upload.c | 6 --
src/mesa/drivers/dri/i965/brw_wm.c | 7 ---
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +++-
src/mesa/drivers/dri/i965/gen6_clip_state.c| 10 +++---
src/mesa/drivers/dri/i965/gen6_multisample_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen6_scissor_state.c | 13 ++---
src/mesa/drivers/dri/i965/gen6_sf_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen6_viewport_state.c| 5 +++--
src/mesa/drivers/dri/i965/gen6_wm_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen7_sf_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen7_viewport_state.c| 5 +++--
src/mesa/drivers/dri/i965/gen7_wm_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen8_viewport_state.c| 8 +---
16 files changed, 74 insertions(+), 34 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clip_state.c
b/src/mesa/drivers/dri/i965/brw_clip_state.c
index 3223834..dee74db 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_state.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_state.c
@@ -32,6 +32,7 @@
#include brw_context.h
#include brw_state.h
#include brw_defines.h
+#include main/framebuffer.h
static void
upload_clip_vp(struct brw_context *brw)
@@ -59,7 +60,9 @@ brw_upload_clip_unit(struct brw_context *brw)
struct brw_clip_unit_state *clip;
/* _NEW_BUFFERS */
- struct gl_framebuffer *fb = ctx-DrawBuffer;
+ const struct gl_framebuffer *fb = ctx-DrawBuffer;
+ const float fb_width = (float)_mesa_geometric_width(fb);
+ const float fb_height = (float)_mesa_geometric_height(fb);
upload_clip_vp(brw);
@@ -127,8 +130,8 @@ brw_upload_clip_unit(struct brw_context *brw)
/* enable guardband clipping if we can */
if (ctx-ViewportArray[0].X == 0
ctx-ViewportArray[0].Y == 0
- ctx-ViewportArray[0].Width == (float) fb-Width
- ctx-ViewportArray[0].Height == (float) fb-Height)
+ ctx-ViewportArray[0].Width == fb_width
+ ctx-ViewportArray[0].Height == fb_height)
{
clip-clip5.guard_band_enable = 1;
clip-clip6.clipper_viewport_state_ptr =
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 67a693b..1672786 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -39,6 +39,7 @@
#include brw_state.h
#include brw_defines.h
+#include main/framebuffer.h
#include main/fbobject.h
#include main/glformats.h
@@ -46,12 +47,15 @@
static void upload_drawing_rect(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
+ const struct gl_framebuffer *fb = ctx-DrawBuffer;
+ const unsigned int fb_width = _mesa_geometric_width(fb);
+ const unsigned int fb_height = _mesa_geometric_height(fb);
BEGIN_BATCH(4);
OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE 16 | (4 - 2));
OUT_BATCH(0); /* xmin, ymin */
- OUT_BATCH(((ctx-DrawBuffer-Width - 1) 0x) |
- ((ctx-DrawBuffer-Height - 1) 16));
+ OUT_BATCH(((fb_width - 1) 0x) |
+ ((fb_height - 1) 16));
OUT_BATCH(0);
ADVANCE_BATCH();
}
@@ -767,7 +771,7 @@ static void upload_polygon_stipple_offset(struct
brw_context *brw)
* works just fine, and there's no window system to worry about.
*/
if (_mesa_is_winsys_fbo(ctx-DrawBuffer))
- OUT_BATCH((32 - (ctx-DrawBuffer-Height 31)) 31);
+ OUT_BATCH((32 - (_mesa_geometric_height(ctx-DrawBuffer) 31)) 31);
else
OUT_BATCH(0);
ADVANCE_BATCH();
diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c
b/src/mesa/drivers/dri/i965/brw_sf_state.c
index 014b434..6f9397f 100644
--- a/src/mesa/drivers/dri/i965/brw_sf_state.c
+++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
@@ -52,6 +52,14 @@ static void upload_sf_vp(struct brw_context *brw)
sizeof(*sfv), 32, brw-sf.vp_offset);
memset(sfv, 0, sizeof(*sfv));
+ /* Accessing the fields Width and Height of
+* gl_framebuffer to produce the values to
+* program the viewport and scissor is fine
+* as long as the
[Mesa-dev] [v3 PATCH 04/10] mesa: add helper convenience functions for fetching geometry of gl_framebuffer
From: Kevin Rogovin kevin.rogo...@intel.com
Add convenience helper functions for fetching geometry of gl_framebuffer
that return the geometry of the gl_framebuffer instead of the geometry of
the buffers of the gl_framebuffer when then the gl_framebuffer has no
attachments.
v1 - v2
Split from patch mesa:helper-conveniance functions for drivers to implement
ARB_framebuffer_no_attachment.
v2 - v3
Add error check for functions of extension.
Implement DSA functions dependent on extension.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/framebuffer.h | 29 +
src/mesa/main/mtypes.h | 8 +++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
index d02b86f..c402ce7 100644
--- a/src/mesa/main/framebuffer.h
+++ b/src/mesa/main/framebuffer.h
@@ -76,6 +76,35 @@ _mesa_scissor_bounding_box(const struct gl_context *ctx,
const struct gl_framebuffer *buffer,
unsigned idx, int *bbox);
+static inline GLuint
+_mesa_geometric_width(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-Width : buffer-DefaultGeometry.Width;
+}
+
+
+static inline GLuint
+_mesa_geometric_height(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-Height : buffer-DefaultGeometry.Height;
+}
+
+static inline GLuint
+_mesa_geometric_samples(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-Visual.samples : buffer-DefaultGeometry.NumSamples;
+}
+
+static inline GLuint
+_mesa_geometric_layers(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-MaxNumLayers : buffer-DefaultGeometry.Layers;
+}
+
extern void
_mesa_update_draw_buffer_bounds(struct gl_context *ctx,
struct gl_framebuffer *drawFb);
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 1a37aa6..7e1f0e0 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3189,7 +3189,13 @@ struct gl_framebuffer
* the values in DefaultGeometry to initialize its
* viewport, scissor and so on (in particular _Xmin,
* _Xmax, _Ymin and _Ymax do NOT take into account
-* _HasAttachments being false)
+* _HasAttachments being false). To get the geometry
+* of the framebuffer, the helper functions
+* _mesa_geometric_width(),
+* _mesa_geometric_height(),
+* _mesa_geometric_samples(),
+* _mesa_geometric_layers()
+* are available that check _HasAttachments.
*/
GLboolean _HasAttachments;
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v3 PATCH 09/10] i965: enable ARB_framebuffer_no_attachment extension for Gen7 and later
From: Kevin Rogovin kevin.rogo...@intel.com
Enable GL_ARB_framebuffer_no_attachments in i965 for Gen7 and higher.
v1 - v2
No changes.
v2 - v3
intel_extensions.c: Alphabetize insertion.
Reviewed-by: Ian Romanick ian.d.romanick at intel.com (v2)
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/drivers/dri/i965/brw_context.c | 6 ++
src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
2 files changed, 7 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index ea56859..9401a4a 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -613,6 +613,12 @@ brw_initialize_context_constants(struct brw_context *brw)
/* ARB_gpu_shader5 */
if (brw-gen = 7)
ctx-Const.MaxVertexStreams = MIN2(4, MAX_VERTEX_STREAMS);
+
+ /* ARB_framebuffer_no_attachments */
+ ctx-Const.MaxFramebufferWidth = ctx-Const.MaxViewportWidth;
+ ctx-Const.MaxFramebufferHeight = ctx-Const.MaxViewportHeight;
+ ctx-Const.MaxFramebufferLayers = ctx-Const.MaxArrayTextureLayers;
+ ctx-Const.MaxFramebufferSamples = max_samples;
}
static void
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 18b69a0..1f38b5a 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -326,6 +326,7 @@ intelInitExtensions(struct gl_context *ctx)
if (brw-gen = 7) {
ctx-Extensions.ARB_conservative_depth = true;
ctx-Extensions.ARB_derivative_control = true;
+ ctx-Extensions.ARB_framebuffer_no_attachments = true;
ctx-Extensions.ARB_gpu_shader5 = true;
ctx-Extensions.ARB_shader_atomic_counters = true;
ctx-Extensions.ARB_texture_compression_bptc = true;
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v3 PATCH 05/10] mesa: add helper convenience functions for computing box intersected against scissors of gl_framebuffer
From: Kevin Rogovin kevin.rogo...@intel.com
Add helper convenience function that intersects the scissor values
against a passed bounding box. In addition, to avoid replicated code,
make the function _mesa_scissor_bounding_box() use this new function.
v1 - v2
Split from patch mesa:helper-conveniance functions for drivers to implement
ARB_framebuffer_no_attachment.
White space and long line fixes.
v2 - v3
No changes.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/framebuffer.c | 63 +++--
src/mesa/main/framebuffer.h | 4 +++
2 files changed, 47 insertions(+), 20 deletions(-)
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index c2cfb92..dd9e4bc 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -357,42 +357,38 @@ update_framebuffer_size(struct gl_context *ctx, struct
gl_framebuffer *fb)
}
+
/**
- * Calculate the inclusive bounding box for the scissor of a specific viewport
+ * Given a bounding box, intersect the bounding box with the scissor of
+ * a specified vieport.
*
* \param ctx GL context.
- * \param buffer Framebuffer to be checked against
* \param idx Index of the desired viewport
* \param bboxBounding box for the scissored viewport. Stored as xmin,
*xmax, ymin, ymax.
- *
- * \warning This function assumes that the framebuffer dimensions are up to
- * date (e.g., update_framebuffer_size has been recently called on \c buffer).
- *
- * \sa _mesa_clip_to_region
*/
-void
-_mesa_scissor_bounding_box(const struct gl_context *ctx,
- const struct gl_framebuffer *buffer,
- unsigned idx, int *bbox)
+extern void
+_mesa_intersect_scissor_bounding_box(const struct gl_context *ctx,
+ unsigned idx, int *bbox)
{
- bbox[0] = 0;
- bbox[2] = 0;
- bbox[1] = buffer-Width;
- bbox[3] = buffer-Height;
-
if (ctx-Scissor.EnableFlags (1u idx)) {
+ int xmax, ymax;
+
+ xmax = ctx-Scissor.ScissorArray[idx].X
++ ctx-Scissor.ScissorArray[idx].Width;
+ ymax = ctx-Scissor.ScissorArray[idx].Y
++ ctx-Scissor.ScissorArray[idx].Height;
if (ctx-Scissor.ScissorArray[idx].X bbox[0]) {
bbox[0] = ctx-Scissor.ScissorArray[idx].X;
}
if (ctx-Scissor.ScissorArray[idx].Y bbox[2]) {
bbox[2] = ctx-Scissor.ScissorArray[idx].Y;
}
- if (ctx-Scissor.ScissorArray[idx].X +
ctx-Scissor.ScissorArray[idx].Width bbox[1]) {
- bbox[1] = ctx-Scissor.ScissorArray[idx].X +
ctx-Scissor.ScissorArray[idx].Width;
+ if (xmax bbox[1]) {
+ bbox[1] = xmax;
}
- if (ctx-Scissor.ScissorArray[idx].Y +
ctx-Scissor.ScissorArray[idx].Height bbox[3]) {
- bbox[3] = ctx-Scissor.ScissorArray[idx].Y +
ctx-Scissor.ScissorArray[idx].Height;
+ if (ymax bbox[3]) {
+bbox[3] = ymax;
}
/* finally, check for empty region */
if (bbox[0] bbox[1]) {
@@ -402,6 +398,33 @@ _mesa_scissor_bounding_box(const struct gl_context *ctx,
bbox[2] = bbox[3];
}
}
+}
+
+/**
+ * Calculate the inclusive bounding box for the scissor of a specific viewport
+ *
+ * \param ctx GL context.
+ * \param buffer Framebuffer to be checked against
+ * \param idx Index of the desired viewport
+ * \param bboxBounding box for the scissored viewport. Stored as xmin,
+ *xmax, ymin, ymax.
+ *
+ * \warning This function assumes that the framebuffer dimensions are up to
+ * date (e.g., update_framebuffer_size has been recently called on \c buffer).
+ *
+ * \sa _mesa_clip_to_region
+ */
+void
+_mesa_scissor_bounding_box(const struct gl_context *ctx,
+ const struct gl_framebuffer *buffer,
+ unsigned idx, int *bbox)
+{
+ bbox[0] = 0;
+ bbox[2] = 0;
+ bbox[1] = buffer-Width;
+ bbox[3] = buffer-Height;
+
+ _mesa_intersect_scissor_bounding_box(ctx, idx, bbox);
assert(bbox[0] = bbox[1]);
assert(bbox[2] = bbox[3]);
diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
index c402ce7..bb1f2ea 100644
--- a/src/mesa/main/framebuffer.h
+++ b/src/mesa/main/framebuffer.h
@@ -76,6 +76,10 @@ _mesa_scissor_bounding_box(const struct gl_context *ctx,
const struct gl_framebuffer *buffer,
unsigned idx, int *bbox);
+extern void
+_mesa_intersect_scissor_bounding_box(const struct gl_context *ctx,
+ unsigned idx, int *bbox);
+
static inline GLuint
_mesa_geometric_width(const struct gl_framebuffer *buffer)
{
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v3 PATCH 00/10] Implement extension ARB_framebuffer_no_attachments
From: Kevin Rogovin kevin.rogo...@intel.com This patch series implements: - the needed functionality in Mesa-core for ARB_framebuffer_no_attachments - implements and enables the extension i965 Kevin Rogovin (10): mesa:Define extension ARB_framebuffer_no_attachments to gl_framebuffer for extension ARB_framebuffer_no_attachments mesa:Define constants and functions for ARB_framebuffer_no_attachment extension mesa: Complete implementation for ARB_framebuffer_no_attachment in Mesa core mesa: add helper convenience functions for fetching geometry of gl_framebuffer mesa: add helper convenience functions for computing box intersected against scissors of gl_framebuffer i965: Use _mesa_geometry_ functions appropriately mesa: add helper function for testing if current fragment shader has atomics i965: ensure execution of fragment shader when fragment shader has atomic buffer access i965: enable ARB_framebuffer_no_attachment extension for Gen7 and later mark GL_ARB_framebuffer_no_attachments as done for i965 docs/GL3.txt | 4 +- docs/relnotes/10.6.0.html | 1 + .../glapi/gen/ARB_framebuffer_no_attachments.xml | 32 +++ src/mapi/glapi/gen/Makefile.am | 1 + src/mapi/glapi/gen/gl_API.xml | 4 +- src/mesa/drivers/dri/i965/brw_clip_state.c | 9 +- src/mesa/drivers/dri/i965/brw_context.c| 6 + src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +- src/mesa/drivers/dri/i965/brw_sf_state.c | 8 + src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +- src/mesa/drivers/dri/i965/brw_wm.c | 7 +- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +- src/mesa/drivers/dri/i965/gen6_clip_state.c| 10 +- src/mesa/drivers/dri/i965/gen6_multisample_state.c | 3 +- src/mesa/drivers/dri/i965/gen6_scissor_state.c | 13 +- src/mesa/drivers/dri/i965/gen6_sf_state.c | 3 +- src/mesa/drivers/dri/i965/gen6_viewport_state.c| 5 +- src/mesa/drivers/dri/i965/gen6_wm_state.c | 3 +- src/mesa/drivers/dri/i965/gen7_sf_state.c | 3 +- src/mesa/drivers/dri/i965/gen7_viewport_state.c| 5 +- src/mesa/drivers/dri/i965/gen7_wm_state.c | 7 +- src/mesa/drivers/dri/i965/gen8_ps_state.c | 3 + src/mesa/drivers/dri/i965/gen8_viewport_state.c| 8 +- src/mesa/drivers/dri/i965/intel_extensions.c | 1 + src/mesa/main/extensions.c | 1 + src/mesa/main/fbobject.c | 218 +++-- src/mesa/main/fbobject.h | 7 + src/mesa/main/framebuffer.c| 64 -- src/mesa/main/framebuffer.h| 33 src/mesa/main/get.c| 3 + src/mesa/main/get_hash_params.py | 38 src/mesa/main/mtypes.h | 63 +- src/mesa/main/tests/dispatch_sanity.cpp| 4 +- 33 files changed, 511 insertions(+), 84 deletions(-) create mode 100644 src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v3 PATCH 03/10] mesa: Complete implementation for ARB_framebuffer_no_attachments in Mesa core
From: Kevin Rogovin kevin.rogo...@intel.com
Implement GL_ARB_framebuffer_no_attachments in Mesa core
- changes to conditions for framebuffer completenss
- implement set/get functions for framebuffers for
new functions in GL_ARB_framebuffer_no_attachments
v1 - v2
Spacing and exceed 80 characters per line fixes.
v2 - v3
Implement DSA functions of extension.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/fbobject.c | 217 ---
1 file changed, 184 insertions(+), 33 deletions(-)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 4ac3f20..18def71 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1156,14 +1156,49 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
} else if (att_layer_count max_layer_count) {
max_layer_count = att_layer_count;
}
+
+ /**
+ * The extension GL_ARB_framebuffer_no_attachments places the additional
+ * requirement on each attachment that
+ *
+ * The width and height of image are greater than zero and less than or
+ * equal to the values of the implementation-dependent limits
+ * MAX_FRAMEBUFFER_WIDTH and MAX_FRAMEBUFFER_HEIGHT, respectively.
+ *
+ * If image is a three-dimensional texture or a one- or
two-dimensional
+ * array texture and the attachment is layered, the depth or layer count
+ * of the texture is less than or equal to the implementation-dependent
+ * limit MAX_FRAMEBUFFER_LAYERS.
+ *
+ * If image has multiple samples, its sample count is less than or equal
+ * to the value of the implementation-dependent limit MAX_FRAMEBUFFER_-
+ * SAMPLES .
+ *
+ * The same requirements are also in place for GL 4.5,
+ * Section 9.4.1 Framebuffer Attachment Completeness, pg 310-311
+ *
+ * However, this is a tighter restriction than previous version of GL.
+ * In interest of better compatibility, we will not enforce these
+ * restrictions.
+ */
}
fb-MaxNumLayers = max_layer_count;
if (numImages == 0) {
- fb-_Status = GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT_EXT;
- fbo_incomplete(ctx, no attachments, -1);
- return;
+ fb-_HasAttachments = GL_FALSE;
+
+ if (!ctx-Extensions.ARB_framebuffer_no_attachments) {
+ fb-_Status = GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT_EXT;
+ fbo_incomplete(ctx, no attachments, -1);
+ return;
+ }
+
+ if (fb-DefaultGeometry.Width == 0 || fb-DefaultGeometry.Height == 0) {
+ fb-_Status = GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT_EXT;
+ fbo_incomplete(ctx, no attachments and default width or height is
0, -1);
+ return;
+ }
}
if (_mesa_is_desktop_gl(ctx) !ctx-Extensions.ARB_ES2_compatibility) {
@@ -1228,8 +1263,10 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
* renderbuffers/textures are different sizes, the framebuffer
* width/height will be set to the smallest width/height.
*/
- fb-Width = minWidth;
- fb-Height = minHeight;
+ if (numImages != 0) {
+ fb-Width = minWidth;
+ fb-Height = minHeight;
+ }
/* finally, update the visual info for the framebuffer */
_mesa_update_framebuffer_visual(ctx, fb);
@@ -1335,32 +1372,127 @@ _mesa_BindRenderbufferEXT(GLenum target, GLuint
renderbuffer)
bind_renderbuffer(target, renderbuffer, true);
}
-extern void GLAPIENTRY
+static void
+framebuffer_parameteri(struct gl_context *ctx, struct gl_framebuffer *fb,
+ GLenum pname, GLint param, const char *func)
+{
+ switch (pname) {
+ case GL_FRAMEBUFFER_DEFAULT_WIDTH:
+ if (param 0 || param ctx-Const.MaxFramebufferWidth)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+ fb-DefaultGeometry.Width = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_HEIGHT:
+ if (param 0 || param ctx-Const.MaxFramebufferHeight)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+ fb-DefaultGeometry.Height = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_LAYERS:
+ if (param 0 || param ctx-Const.MaxFramebufferLayers)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+ fb-DefaultGeometry.Layers = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
+ if (param 0 || param ctx-Const.MaxFramebufferSamples)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+fb-DefaultGeometry.NumSamples = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS:
+ fb-DefaultGeometry.FixedSampleLocations = param;
+ break;
+ default:
+ _mesa_error(ctx, GL_INVALID_ENUM,
+ %s(pname=0x%x), func, pname);
+ }
+}
+
+void GLAPIENTRY
_mesa_FramebufferParameteri(GLenum target, GLenum
[Mesa-dev] [v3 PATCH 02/10] mesa:Define constants and functions for ARB_framebuffer_no_attachment extension
From: Kevin Rogovin kevin.rogo...@intel.com
Define the enumeration constants, function entry points and
glGet for the GL_ARB_framebuffer_no_attachments.
v1 - v2
Add output=true for GetFramebufferParameteriv parameter params.
Alphabetical insertion.
v2 - v3
Implement _mesa_GetFramebufferParameteriv and _mesa_FramebufferParameteri
as always error.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
.../glapi/gen/ARB_framebuffer_no_attachments.xml | 32 ++
src/mapi/glapi/gen/Makefile.am | 1 +
src/mapi/glapi/gen/gl_API.xml | 4 ++-
src/mesa/main/fbobject.c | 28
src/mesa/main/fbobject.h | 7
src/mesa/main/get.c| 3 ++
src/mesa/main/get_hash_params.py | 38 ++
src/mesa/main/tests/dispatch_sanity.cpp| 4 +--
8 files changed, 114 insertions(+), 3 deletions(-)
create mode 100644 src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
diff --git a/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
b/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
new file mode 100644
index 000..10bdebc
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
@@ -0,0 +1,32 @@
+?xml version=1.0?
+!DOCTYPE OpenGLAPI SYSTEM gl_API.dtd
+
+OpenGLAPI
+
+category name=GL_ARB_framebuffer_no_attachments number=130
+
+enum name=FRAMEBUFFER_DEFAULT_WIDTH value=0x9310 /
+enum name=FRAMEBUFFER_DEFAULT_HEIGHT value=0x9311 /
+enum name=FRAMEBUFFER_DEFAULT_LAYERS value=0x9312 /
+enum name=FRAMEBUFFER_DEFAULT_SAMPLES value=0x9313 /
+enum name=FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS value=0x9314 /
+enum name=MAX_FRAMEBUFFER_WIDTH value=0x9315 /
+enum name=MAX_FRAMEBUFFER_HEIGHT value=0x9316 /
+enum name=MAX_FRAMEBUFFER_LAYERS value=0x9317 /
+enum name=MAX_FRAMEBUFFER_SAMPLES value=0x9318 /
+
+function name=FramebufferParameteri offset=assign
+param name=target type=GLenum /
+param name=pname type=GLenum /
+param name=param type=GLint /
+/function
+
+function name=GetFramebufferParameteriv offset=assign
+param name=target type=GLenum /
+param name=pname type=GLenum /
+param name=params type=GLint * output=true /
+/function
+
+/category
+
+/OpenGLAPI
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index adebd5c..5099f12 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -130,6 +130,7 @@ API_XML = \
ARB_draw_instanced.xml \
ARB_ES2_compatibility.xml \
ARB_ES3_compatibility.xml \
+ ARB_framebuffer_no_attachments.xml \
ARB_framebuffer_object.xml \
ARB_geometry_shader4.xml \
ARB_get_program_binary.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index 3090b9f..5079d30 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8188,7 +8188,9 @@
!-- No new functions, types, enums. --
/category
-!-- ARB extensions #130..#131 --
+xi:include href=ARB_framebuffer_no_attachments.xml
xmlns:xi=http://www.w3.org/2001/XInclude/
+
+!-- ARB extensions #131 --
category name=GL_ARB_explicit_uniform_location number=128
enum name=MAX_UNIFORM_LOCATIONS count=1 value=0x826E
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 8fea7f8..4ac3f20 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1335,6 +1335,34 @@ _mesa_BindRenderbufferEXT(GLenum target, GLuint
renderbuffer)
bind_renderbuffer(target, renderbuffer, true);
}
+extern void GLAPIENTRY
+_mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ (void) target;
+ (void) pname;
+ (void) param;
+
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ glFramebufferParameteri not supported
+ (ARB_framebuffer_no_attachments not implemented));
+}
+
+extern void GLAPIENTRY
+_mesa_GetFramebufferParameteriv(GLenum target, GLenum pname, GLint *params)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ (void) target;
+ (void) pname;
+ (void) param;
+
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ glGetNamedFramebufferParameteriv not supported
+ (ARB_framebuffer_no_attachments not implemented));
+}
+
/**
* Remove the specified renderbuffer or texture from any attachment point in
diff --git a/src/mesa/main/fbobject.h b/src/mesa/main/fbobject.h
index 9f570db..21f5b12 100644
--- a/src/mesa/main/fbobject.h
+++ b/src/mesa/main/fbobject.h
@@ -288,4 +288,11 @@ extern void GLAPIENTRY
_mesa_DiscardFramebufferEXT(GLenum target, GLsizei numAttachments,
const GLenum *attachments);
+
+extern void GLAPIENTRY
+_mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param);
+
+extern void GLAPIENTRY
+_mesa_GetFramebufferParameteriv(GLenum target, GLenum pname, GLint *params);
+
#endif /*
[Mesa-dev] [v3 PATCH 10/10] mark GL_ARB_framebuffer_no_attachments as done for i965
From: Kevin Rogovin kevin.rogo...@intel.com Mark GL_ARB_framebuffer_no_attachments as done for i965. v1 - v2 File added to patch series v2 - v3 docs/GL3.txt : add done mark under GLES3.1 docs/relnotes/10.6.0.html : maintain alphabetical order Reviewed-by: Ian Romanick ian.d.romanick at intel.com (v2) Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com --- docs/GL3.txt | 4 ++-- docs/relnotes/10.6.0.html | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/GL3.txt b/docs/GL3.txt index 9d56ee5..fae8253 100644 --- a/docs/GL3.txt +++ b/docs/GL3.txt @@ -157,7 +157,7 @@ GL 4.3, GLSL 4.30: GL_KHR_debug DONE (all drivers) GL_ARB_explicit_uniform_location DONE (all drivers that support GLSL) GL_ARB_fragment_layer_viewport DONE (nv50, nvc0, r600, llvmpipe) - GL_ARB_framebuffer_no_attachmentsnot started + GL_ARB_framebuffer_no_attachmentsDONE (i965) GL_ARB_internalformat_query2 not started GL_ARB_invalidate_subdataDONE (all drivers) GL_ARB_multi_draw_indirect DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe) @@ -216,7 +216,7 @@ GLES3.1, GLSL ES 3.1 GL_ARB_compute_shaderin progress (jljusten) GL_ARB_draw_indirect DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe) GL_ARB_explicit_uniform_location DONE (all drivers that support GLSL) - GL_ARB_framebuffer_no_attachmentsnot started + GL_ARB_framebuffer_no_attachmentsDONE (i965) GL_ARB_program_interface_query DONE (all drivers) GL_ARB_shader_atomic_countersDONE (i965) GL_ARB_shader_image_load_store in progress (curro) diff --git a/docs/relnotes/10.6.0.html b/docs/relnotes/10.6.0.html index 474a2c7..bc0cf8e 100644 --- a/docs/relnotes/10.6.0.html +++ b/docs/relnotes/10.6.0.html @@ -51,6 +51,7 @@ Note: some of the new features are only available with certain drivers. liGL_ARB_direct_state_access on all drivers that support GL 2.0+/li liGL_ARB_draw_indirect, GL_ARB_multi_draw_indirect on r600/li liGL_ARB_draw_instanced on freedreno/li +liGL_ARB_framebuffer_no_attachments on i965/li liGL_ARB_gpu_shader_fp64 on nvc0, softpipe/li liGL_ARB_gpu_shader5 on i965/gen8+/li liGL_ARB_instanced_arrays on freedreno/li -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 03/10] mesa: Complete implementation for ARB_framebuffer_no_attachments in Mesa core
On Thu, May 21, 2015 at 5:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
If you set your email name to that, you won't get this funny mail, and
your name will appear properly in the actual email sent.
This is what I have in my .gitconfig:
[user]
email = imir...@alum.mit.edu
name = Ilia Mirkin
Adjust to taste :)
Implement GL_ARB_framebuffer_no_attachments in Mesa core
- changes to conditions for framebuffer completenss
- implement set/get functions for framebuffers for
new functions in GL_ARB_framebuffer_no_attachments
v1 - v2
Spacing and exceed 80 characters per line fixes.
v2 - v3
Implement DSA functions of extension.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/fbobject.c | 217
---
1 file changed, 184 insertions(+), 33 deletions(-)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 4ac3f20..18def71 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1156,14 +1156,49 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
} else if (att_layer_count max_layer_count) {
max_layer_count = att_layer_count;
}
+
+ /**
+ * The extension GL_ARB_framebuffer_no_attachments places the
additional
+ * requirement on each attachment that
+ *
+ * The width and height of image are greater than zero and less than
or
+ * equal to the values of the implementation-dependent limits
+ * MAX_FRAMEBUFFER_WIDTH and MAX_FRAMEBUFFER_HEIGHT, respectively.
+ *
+ * If image is a three-dimensional texture or a one- or
two-dimensional
+ * array texture and the attachment is layered, the depth or layer
count
+ * of the texture is less than or equal to the
implementation-dependent
+ * limit MAX_FRAMEBUFFER_LAYERS.
+ *
+ * If image has multiple samples, its sample count is less than or
equal
+ * to the value of the implementation-dependent limit
MAX_FRAMEBUFFER_-
+ * SAMPLES .
+ *
+ * The same requirements are also in place for GL 4.5,
+ * Section 9.4.1 Framebuffer Attachment Completeness, pg 310-311
+ *
+ * However, this is a tighter restriction than previous version of GL.
+ * In interest of better compatibility, we will not enforce these
+ * restrictions.
+ */
}
fb-MaxNumLayers = max_layer_count;
if (numImages == 0) {
- fb-_Status = GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT_EXT;
- fbo_incomplete(ctx, no attachments, -1);
- return;
+ fb-_HasAttachments = GL_FALSE;
+
+ if (!ctx-Extensions.ARB_framebuffer_no_attachments) {
+ fb-_Status = GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT_EXT;
+ fbo_incomplete(ctx, no attachments, -1);
+ return;
+ }
+
+ if (fb-DefaultGeometry.Width == 0 || fb-DefaultGeometry.Height == 0)
{
+ fb-_Status = GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT_EXT;
+ fbo_incomplete(ctx, no attachments and default width or height is
0, -1);
+ return;
+ }
}
if (_mesa_is_desktop_gl(ctx) !ctx-Extensions.ARB_ES2_compatibility) {
@@ -1228,8 +1263,10 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
* renderbuffers/textures are different sizes, the framebuffer
* width/height will be set to the smallest width/height.
*/
- fb-Width = minWidth;
- fb-Height = minHeight;
+ if (numImages != 0) {
+ fb-Width = minWidth;
+ fb-Height = minHeight;
+ }
/* finally, update the visual info for the framebuffer */
_mesa_update_framebuffer_visual(ctx, fb);
@@ -1335,32 +1372,127 @@ _mesa_BindRenderbufferEXT(GLenum target, GLuint
renderbuffer)
bind_renderbuffer(target, renderbuffer, true);
}
-extern void GLAPIENTRY
+static void
+framebuffer_parameteri(struct gl_context *ctx, struct gl_framebuffer *fb,
+ GLenum pname, GLint param, const char *func)
+{
+ switch (pname) {
+ case GL_FRAMEBUFFER_DEFAULT_WIDTH:
+ if (param 0 || param ctx-Const.MaxFramebufferWidth)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+ fb-DefaultGeometry.Width = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_HEIGHT:
+ if (param 0 || param ctx-Const.MaxFramebufferHeight)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+ fb-DefaultGeometry.Height = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_LAYERS:
+ if (param 0 || param ctx-Const.MaxFramebufferLayers)
+_mesa_error(ctx, GL_INVALID_VALUE, %s, func);
+ else
+ fb-DefaultGeometry.Layers = param;
+ break;
+ case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
+ if (param 0 || param
Re: [Mesa-dev] [PATCH V2 17/22] i965/gen9: Add XY_FAST_COPY_BLT support to intelEmitCopyBlit()
On Thu, May 21, 2015 at 12:26 PM, Ben Widawsky b...@bwidawsk.net wrote:
A lot of opinion stuff is below, feel free to ignore them if you don't think
there are improvements.
On Fri, Apr 17, 2015 at 04:51:38PM -0700, Anuj Phogat wrote:
This patch enables using XY_FAST_COPY_BLT only for Yf/Ys tiled buffers.
Later It can be turned on for other tiling patterns (X,Y).
Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
src/mesa/drivers/dri/i965/intel_blit.c | 292
+++
src/mesa/drivers/dri/i965/intel_blit.h | 3 +
src/mesa/drivers/dri/i965/intel_copy_image.c | 3 +
src/mesa/drivers/dri/i965/intel_reg.h| 33 +++
4 files changed, 291 insertions(+), 40 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 9500bd7..36746c4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -43,6 +43,23 @@
#define FILE_DEBUG_FLAG DEBUG_BLIT
+#define SET_TILING_XY_FAST_COPY_BLT(tiling, tr_mode, type) \
+({ \
+ switch (tiling) { \
+ case I915_TILING_X: \
+ CMD |= type ## _TILED_X; \
+ break; \
+ case I915_TILING_Y: \
assert(tr_mode != INTEL_MIPTREE_TRMODE_YF); ?
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YS)\
+ CMD |= type ## _TILED_64K; \
+ else \
+ CMD |= type ## _TILED_Y;\
+ break; \
+ default: \
+ unreachable(not reached);\
+ } \
+})
+
static void
intel_miptree_set_alpha_to_one(struct brw_context *brw,
struct intel_mipmap_tree *mt,
@@ -75,6 +92,12 @@ static uint32_t
br13_for_cpp(int cpp)
{
switch (cpp) {
+ case 16:
+ return BR13_32323232;
+ break;
+ case 8:
+ return BR13_16161616;
+ break;
case 4:
return BR13_;
break;
No need for break after return.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 14.1/15] glapi: Store exec table version info outside the XML
On Tuesday 19 May 2015, Ian Romanick wrote:
From: Ian Romanick ian.d.roman...@intel.com
Currently on the functions that are exclusive to core-profile are
implemented. The remainder continue to live in the XML. Additional
functions can be moved later.
The functions for GL_ARB_draw_indirect and GL_ARB_multi_draw_indirect
are put in the dispatch table inside the VBO module, so they do not need
to be moved over.
The diff of src/mesa/main/api_exec.c before and after this patch is as
expected. All of the functions listed in apiexec.py moved out of a 'if
(_mesa_is_desktop(ctx))' block into a new 'if (ctx-API ==
API_OPENGL_CORE)' block.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Cc: Dave Airlie airl...@redhat.com
Cc: Ilia Mirkin imir...@alum.mit.edu
Cc: Dylan Baker baker.dyla...@gmail.com
Cc: 10.6 mesa-sta...@lists.freedesktop.org
---
src/mapi/glapi/gen/Makefile.am | 3 +-
src/mapi/glapi/gen/apiexec.py| 142
+++
src/mapi/glapi/gen/gl_genexec.py | 54 ---
3 files changed, 187 insertions(+), 12 deletions(-)
create mode 100644 src/mapi/glapi/gen/apiexec.py
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index adebd5c..64bc2ff 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -61,6 +61,7 @@ EXTRA_DIST= \
$(MESA_GLAPI_DIR)/glapi_x86-64.S \
$(MESA_GLAPI_DIR)/glapi_sparc.S \
$(COMMON_GLX) \
+ apiexec.py \
gl_apitemp.py \
gl_enums.py \
gl_genexec.py \
@@ -267,7 +268,7 @@ $(MESA_GLAPI_DIR)/glapi_sparc.S: gl_SPARC_asm.py $(COMMON)
$(MESA_DIR)/main/enums.c: gl_enums.py $(COMMON)
$(PYTHON_GEN) $ -f $(srcdir)/gl_and_es_API.xml $@
-$(MESA_DIR)/main/api_exec.c: gl_genexec.py $(COMMON)
+$(MESA_DIR)/main/api_exec.c: gl_genexec.py apiexec.py $(COMMON)
$(PYTHON_GEN) $ -f $(srcdir)/gl_and_es_API.xml $@
$(MESA_DIR)/main/dispatch.h: gl_table.py $(COMMON)
diff --git a/src/mapi/glapi/gen/apiexec.py b/src/mapi/glapi/gen/apiexec.py
new file mode 100644
index 000..1a9f042
--- /dev/null
+++ b/src/mapi/glapi/gen/apiexec.py
@@ -0,0 +1,142 @@
+#!/usr/bin/env python2
+
+# Copyright (C) 2015 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.
+
+class exec_info():
+Information relating GL APIs to a function.
+
+Each of the four attributes of this class, compatibility, core, es1, and
+es2, specify the minimum API version where a function can possibly exist
+in Mesa. The version is specified as an integer of (real GL version *
+10). For example, glCreateProgram was added in OpenGL 2.0, so
+compatibility=20 and core=31.
+
+If the attribute is None, then it cannot be supported by that
+API. For example, glNewList was removed from core profiles, so
+compatibility=10 and core=None.
+
+Each of the attributes that is not None must have a valid value. The
+valid ranges are:
+
+compatiblity: [10, 30]
+core: [31, )
+es1: [10, 11]
+es2: [20, )
+
+These ranges are enforced by the constructor.
+
+def __init__(self, compatibility=None, core=None, es1=None, es2=None):
+if compatibility is not None:
+assert isinstance(compatibility, int)
+assert compatibility = 10
+assert compatibility = 30
+
+if core is not None:
+assert isinstance(core, int)
+assert core = 31
+
+if es1 is not None:
+assert isinstance(es1, int)
+assert es1 == 10 or es1 == 11
+
+if es2 is not None:
+assert isinstance(es2, int)
+assert es2 = 20
+
+self.compatibility = compatibility
+self.core = core
+self.es1 = es1
+self.es2 = es2
+
+functions = {
+#
[Mesa-dev] [PATCH 1/2] nv50: use a fence for 64 bits queries, based on nvc0
A sequence number is written for 32-bits queries to make sure they are
ready, but not for 64-bits queries. Instead, we have to use a fence.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index 6690aa2..a3c8841 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -46,6 +46,7 @@ struct nv50_query {
boolean flushed;
boolean is64bit;
struct nouveau_mm_allocation *mm;
+ struct nouveau_fence *fence;
};
#define NV50_QUERY_ALLOC_SPACE 256
@@ -92,6 +93,7 @@ static void
nv50_query_destroy(struct pipe_context *pipe, struct pipe_query *pq)
{
nv50_query_allocate(nv50_context(pipe), nv50_query(pq), 0);
+ nouveau_fence_ref(NULL, nv50_query(pq)-fence);
FREE(nv50_query(pq));
}
@@ -260,12 +262,22 @@ nv50_query_end(struct pipe_context *pipe, struct
pipe_query *pq)
break;
}
q-ready = q-flushed = FALSE;
+
+ if (q-is64bit)
+ nouveau_fence_ref(nv50-screen-base.fence.current, q-fence);
}
static INLINE boolean
nv50_query_ready(struct nv50_query *q)
{
- return q-ready || (!q-is64bit (q-data[0] == q-sequence));
+ if (q-is64bit) {
+ if (nouveau_fence_signalled(q-fence))
+ return TRUE;
+ } else {
+ if (q-data[0] == q-sequence)
+ return TRUE;
+ }
+ return FALSE;
}
static boolean
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] nv50: make pipeline statistics queries use 64-bits, based on nvc0
Tested on NVA8. No regression for ARB_pipeline_statistics piglit tests.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index a3c8841..da41209 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -114,7 +114,8 @@ nv50_query_create(struct pipe_context *pipe, unsigned type,
unsigned index)
q-is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
type == PIPE_QUERY_PRIMITIVES_EMITTED ||
- type == PIPE_QUERY_SO_STATISTICS);
+ type == PIPE_QUERY_SO_STATISTICS ||
+ type == PIPE_QUERY_PIPELINE_STATISTICS);
q-type = type;
if (q-type == PIPE_QUERY_OCCLUSION_COUNTER) {
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] glsl: set the binding value regardless explicit_binding
On 20/05/15 23:39, Timothy Arceri wrote: On Thu, 2015-05-14 at 22:49 +0200, Alejandro Piñeiro wrote: On 14/05/15 20:38, Ian Romanick wrote: I think I see what's happening. I don't think I understood atomic.buffer_index well enough when I removed it. :( In binding=3 case, what value does glGetActiveAtomicCounterBufferiv(prog, 0, GL_ATOMIC_COUNTER_BUFFER_BINDING, param); give? My guess is that it gives 0 instead of the binding specified in the shader. With the test I uploaded today, so with binding points 3 and 6, and using index 0 and 1, calling glGetActiveAtomicCounterBufferiv properly returns 3 and 6. Both with and without the patch of this thread. The correct binding is stored in the gl_active_atomic_buffer struct which queried by glGetActiveAtomicCounterBufferiv() Ok. This would be a good addition to the test. Ok. The index and the binding are different. The index is which atomic is this in the shader, and the binding is which binding point is used to attach a buffer to this atomic. Thanks to c0cd5b, I think somewhere we're using one value when we actually want the other... probably the last hunk: --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -2294,7 +2294,7 @@ vec4_visitor::visit_atomic_counter_intrinsic(ir_call *ir) ir-actual_parameters.get_head()); ir_variable *location = deref-variable_referenced(); unsigned surf_index = (prog_data-base.binding_table.abo_start + - location-data.atomic.buffer_index); + location-data.binding); /* Calculate the surface offset */ src_reg offset(this, glsl_type::uint_type); Maybe try adding a utility function that will search gl_shader_program::UniformStorage for location-name and use the atomic_buffer_index stored there for use in the i965 driver? Taking into account that glGetActiveAtomicCounterBufferiv is already working, do you think that this is still needed? For clarity its probably a good idea to implement Ian's suggestion. Changing the binding value to the atomic buffer index (even though it seems to be unused elsewhere after this point) is a bit confusing. This email conversation is a good enough example of the confusion doing this can cause. If you implement Ian's suggestion you can probably remove the if statement too as the binding value is not used after this point. Probably I'm missing a global view of the situation, but taking into account that one of the conclusions of this email conversation is that the index and the binding are different, and removing one of the variables from ir_variable::data.atomic is causing some confusion (and the regression), shouldn't it easier to just revert the change? Commit c0cd5b explains that the removal helped to reduce memory consumption, but as far as I understand, it was done because it was concluded that was not needed. Although I did notice this in the glsl to nir nir code: /* XXX Get rid of buffer_index */ var-data.atomic.buffer_index = ir-data.binding; So this may be a problem too.. Well, if the change is reverted, it is just about removing that comment (assuming that the comment was added for the same reason commit c0cd5b was done). BR -- Alejandro Piñeiro (apinhe...@igalia.com) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] i915: Add XRGB8888 format to intel_screen_make_configs
On 05/16/2015 01:55 AM, Emil Velikov wrote: On 09/04/15 16:05, Emil Velikov wrote: Hi Boyan, On 9 April 2015 at 16:08, Boyan Ding boyan.j.d...@gmail.com wrote: For the same reason as the i965 one, as suggested by Chih-Wei Huang. Cc: 10.4 10.5 mesa-sta...@lists.freedesktop.org Signed-off-by: Boyan Ding boyan.j.d...@gmail.com --- Did you run this and the i965 patch through piglit ? Considering how quiet things are having the information that if fixes/breaks test X and Y would be great to have. Boyan Ding, Chih-Wei, Can you help out with the piglit testing ? Would be nice to avoid breaking things. -Emil Hi, This patch does the same thing as the i965 one, which is already merged. I don't have such hardware to test this one. Regards, Boyan Ding ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] u_math: uses assert, include assert.h
On 05/20/2015 06:56 PM, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com this fixes a build problem found on RHEL s390. not sure what configure options caused it, I couldn't get it on x86 here. Cc: 10.6 mesa-sta...@lists.freedesktop.org Signed-off-by: Dave Airlie airl...@redhat.com --- src/gallium/auxiliary/util/u_math.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index 3d27a59..58070a9 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -42,6 +42,7 @@ #include pipe/p_compiler.h #include c99_math.h +#include assert.h #include float.h #include stdarg.h Reviewed-by: Brian Paul bri...@vmware.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 79706] [TRACKER] Mesa regression tracker
https://bugs.freedesktop.org/show_bug.cgi?id=79706 --- Comment #2 from Emil Velikov emil.l.veli...@gmail.com --- Actually I was contemplating on bringing back the per release regression tracker idea which brought us here in the first place. It will give us a clearer picture of the regressions when planning/doing the releasing. How would people feel on the topic ? One small catch is that the regressions reported by the Intel QA might cause spam as they're lacking the bisected regression keywords and the Product Version is unspecified. Matt any ideas if we can have these with the original report - who can we speak to about this ? -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 25898] glEvalPoint causes glEnd to throw GL_INVALID_OPERATION
https://bugs.freedesktop.org/show_bug.cgi?id=25898 --- Comment #5 from marius predut marius.pre...@intel.com --- Created attachment 115947 -- https://bugs.freedesktop.org/attachment.cgi?id=115947action=edit app outputs no matter if defines or not. -- You are receiving this mail because: You are the QA Contact for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 25898] glEvalPoint causes glEnd to throw GL_INVALID_OPERATION
https://bugs.freedesktop.org/show_bug.cgi?id=25898 --- Comment #6 from marius predut marius.pre...@intel.com --- I confirm that this app don't generate any errors. With or without proposed defines the outputs are identical - see above comments. -- You are receiving this mail because: You are the QA Contact for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
Subject: mesa:Define extension ARB_framebuffer_no_attachments
Space after mesa:
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Define the infrastructure for the extension GL_ARB_framebuffer_no_attachments:
- extension table
- additions to gl_framebuffer
v1 - v2
Spacing and trailing spaces fixes.
v2 - v3
mtypes.h: Correct comment on _HasAttachments.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/extensions.c | 1 +
src/mesa/main/fbobject.c| 1 +
src/mesa/main/framebuffer.c | 1 +
src/mesa/main/mtypes.h | 50
-
4 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index c82416a..3256b2c 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -117,6 +117,7 @@ static const struct extension extension_table[] = {
{ GL_ARB_fragment_program,o(ARB_fragment_program),
GLL,2002 },
{ GL_ARB_fragment_program_shadow,
o(ARB_fragment_program_shadow), GLL,2003 },
{ GL_ARB_fragment_shader, o(ARB_fragment_shader),
GL, 2002 },
+ { GL_ARB_framebuffer_no_attachments,
o(ARB_framebuffer_no_attachments), GL, 2012 },
{ GL_ARB_framebuffer_object,
o(ARB_framebuffer_object), GL, 2005 },
{ GL_ARB_framebuffer_sRGB,o(EXT_framebuffer_sRGB),
GL, 1998 },
{ GL_ARB_get_program_binary, o(dummy_true),
GL, 2010 },
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 1859c27..8fea7f8 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -957,6 +957,7 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
fb-Height = 0;
fb-_AllColorBuffersFixedPoint = GL_TRUE;
fb-_HasSNormOrFloatColorBuffer = GL_FALSE;
+ fb-_HasAttachments = GL_TRUE;
/* Start at -2 to more easily loop over all attachment points.
* -2: depth buffer
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index 665a5ba..c2cfb92 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -157,6 +157,7 @@ _mesa_initialize_window_framebuffer(struct gl_framebuffer
*fb,
fb-_Status = GL_FRAMEBUFFER_COMPLETE_EXT;
fb-_AllColorBuffersFixedPoint = !visual-floatMode;
fb-_HasSNormOrFloatColorBuffer = visual-floatMode;
+ fb-_HasAttachments = GL_TRUE;
compute_depth_max(fb);
}
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 8342517..1a37aa6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3146,12 +3146,29 @@ struct gl_framebuffer
*/
struct gl_config Visual;
- GLuint Width, Height; /** size of frame buffer in pixels */
+ /**
+* size of frame buffer in pixels,
+* no attachments has these values as 0
What does this mean? That if there are no attachments, these are 0?
Maybe capitalize something. All lower-case in a multiline comment
looks sloppy.
+*/
+ GLuint Width, Height;
+
+ /**
+* In the case that the framebuffer has no attachment (i.e.
+* GL_ARB_framebuffer_no_attachments) then the geometry of
+* the framebuffer is specified by the default values.
+*/
+ struct {
+ GLuint Width, Height, Layers, NumSamples;
+ GLboolean FixedSampleLocations;
+ } DefaultGeometry;
- /** \name Drawing bounds (Intersection of buffer size and scissor box) */
+ /** \name Drawing bounds (Intersection of buffer size and scissor box)
+* The drawing region is given by [_Xmin, _Xmax) x [_Ymin, _Ymax),
+* (inclusive for _Xmin and _Ymin while exclusive for _Xmax and _Ymax)
+*/
/*@{*/
- GLint _Xmin, _Xmax; /** inclusive */
- GLint _Ymin, _Ymax; /** exclusive */
+ GLint _Xmin, _Xmax;
+ GLint _Ymin, _Ymax;
/*@}*/
/** \name Derived Z buffer stuff */
@@ -3164,6 +3181,18 @@ struct gl_framebuffer
/** One of the GL_FRAMEBUFFER_(IN)COMPLETE_* tokens */
GLenum _Status;
+ /** True if one of Attachment has Type != GL_NONE
Whether instead of True if -- I know lots of other places say
True if but it sounds silly.
+* NOTE: the values for Width and Height are set to 0 in
+* case of no attachments, a backend driver supporting
+* GL_ARB_framebuffer_no_attachments must check for the
+* flag _HasAttachments and if GL_FALSE, must then use
+* the values in DefaultGeometry to initialize its
+* viewport, scissor and so on (in particular _Xmin,
+* _Xmax, _Ymin and _Ymax do NOT take into account
+* _HasAttachments being false)
This is terribly
Re: [Mesa-dev] [v3 PATCH 02/10] mesa:Define constants and functions for ARB_framebuffer_no_attachment extension
Again, space after mesa: in the subject.
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Define the enumeration constants, function entry points and
glGet for the GL_ARB_framebuffer_no_attachments.
v1 - v2
Add output=true for GetFramebufferParameteriv parameter params.
Alphabetical insertion.
v2 - v3
Implement _mesa_GetFramebufferParameteriv and _mesa_FramebufferParameteri
as always error.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
.../glapi/gen/ARB_framebuffer_no_attachments.xml | 32 ++
src/mapi/glapi/gen/Makefile.am | 1 +
src/mapi/glapi/gen/gl_API.xml | 4 ++-
src/mesa/main/fbobject.c | 28
src/mesa/main/fbobject.h | 7
src/mesa/main/get.c| 3 ++
src/mesa/main/get_hash_params.py | 38
++
src/mesa/main/tests/dispatch_sanity.cpp| 4 +--
8 files changed, 114 insertions(+), 3 deletions(-)
create mode 100644 src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
diff --git a/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
b/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
new file mode 100644
index 000..10bdebc
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
@@ -0,0 +1,32 @@
+?xml version=1.0?
+!DOCTYPE OpenGLAPI SYSTEM gl_API.dtd
+
+OpenGLAPI
+
+category name=GL_ARB_framebuffer_no_attachments number=130
+
+enum name=FRAMEBUFFER_DEFAULT_WIDTH value=0x9310 /
+enum name=FRAMEBUFFER_DEFAULT_HEIGHT value=0x9311 /
+enum name=FRAMEBUFFER_DEFAULT_LAYERS value=0x9312 /
+enum name=FRAMEBUFFER_DEFAULT_SAMPLES value=0x9313 /
+enum name=FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS value=0x9314 /
+enum name=MAX_FRAMEBUFFER_WIDTH value=0x9315 /
+enum name=MAX_FRAMEBUFFER_HEIGHT value=0x9316 /
+enum name=MAX_FRAMEBUFFER_LAYERS value=0x9317 /
+enum name=MAX_FRAMEBUFFER_SAMPLES value=0x9318 /
+
+function name=FramebufferParameteri offset=assign
+param name=target type=GLenum /
+param name=pname type=GLenum /
+param name=param type=GLint /
+/function
+
+function name=GetFramebufferParameteriv offset=assign
+param name=target type=GLenum /
+param name=pname type=GLenum /
+param name=params type=GLint * output=true /
+/function
+
+/category
+
+/OpenGLAPI
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index adebd5c..5099f12 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -130,6 +130,7 @@ API_XML = \
ARB_draw_instanced.xml \
ARB_ES2_compatibility.xml \
ARB_ES3_compatibility.xml \
+ ARB_framebuffer_no_attachments.xml \
ARB_framebuffer_object.xml \
ARB_geometry_shader4.xml \
ARB_get_program_binary.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index 3090b9f..5079d30 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8188,7 +8188,9 @@
!-- No new functions, types, enums. --
/category
-!-- ARB extensions #130..#131 --
+xi:include href=ARB_framebuffer_no_attachments.xml
xmlns:xi=http://www.w3.org/2001/XInclude/
+
+!-- ARB extensions #131 --
category name=GL_ARB_explicit_uniform_location number=128
enum name=MAX_UNIFORM_LOCATIONS count=1 value=0x826E
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 8fea7f8..4ac3f20 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1335,6 +1335,34 @@ _mesa_BindRenderbufferEXT(GLenum target, GLuint
renderbuffer)
bind_renderbuffer(target, renderbuffer, true);
}
+extern void GLAPIENTRY
+_mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ (void) target;
+ (void) pname;
+ (void) param;
+
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ glFramebufferParameteri not supported
+ (ARB_framebuffer_no_attachments not implemented));
+}
+
+extern void GLAPIENTRY
+_mesa_GetFramebufferParameteriv(GLenum target, GLenum pname, GLint *params)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ (void) target;
+ (void) pname;
+ (void) param;
+
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ glGetNamedFramebufferParameteriv not supported
+ (ARB_framebuffer_no_attachments not implemented));
+}
+
/**
* Remove the specified renderbuffer or texture from any attachment point in
diff --git a/src/mesa/main/fbobject.h b/src/mesa/main/fbobject.h
index 9f570db..21f5b12 100644
--- a/src/mesa/main/fbobject.h
+++ b/src/mesa/main/fbobject.h
@@ -288,4 +288,11 @@ extern void GLAPIENTRY
_mesa_DiscardFramebufferEXT(GLenum target, GLsizei numAttachments,
const
Re: [Mesa-dev] [v3 PATCH 02/10] mesa:Define constants and functions for ARB_framebuffer_no_attachment extension
I'm happy to see new documentation, so thanks for writing it up! But let's separate this from the functional changes related to implementing the extension. (Didn't I give this comment last time?) If you did, I missed it. Unless there are objections, I'll remove this from the series and make a tiny patch later that is just the documentation. -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 04/10] mesa: add helper convenience functions for fetching geometry of gl_framebuffer
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Add convenience helper functions for fetching geometry of gl_framebuffer
that return the geometry of the gl_framebuffer instead of the geometry of
the buffers of the gl_framebuffer when then the gl_framebuffer has no
attachments.
v1 - v2
Split from patch mesa:helper-conveniance functions for drivers to implement
ARB_framebuffer_no_attachment.
v2 - v3
Add error check for functions of extension.
Implement DSA functions dependent on extension.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/framebuffer.h | 29 +
src/mesa/main/mtypes.h | 8 +++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
index d02b86f..c402ce7 100644
--- a/src/mesa/main/framebuffer.h
+++ b/src/mesa/main/framebuffer.h
@@ -76,6 +76,35 @@ _mesa_scissor_bounding_box(const struct gl_context *ctx,
const struct gl_framebuffer *buffer,
unsigned idx, int *bbox);
+static inline GLuint
+_mesa_geometric_width(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-Width : buffer-DefaultGeometry.Width;
+}
+
+
Extra new line.
+static inline GLuint
+_mesa_geometric_height(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-Height : buffer-DefaultGeometry.Height;
+}
+
+static inline GLuint
+_mesa_geometric_samples(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-Visual.samples : buffer-DefaultGeometry.NumSamples;
+}
+
+static inline GLuint
+_mesa_geometric_layers(const struct gl_framebuffer *buffer)
+{
+ return buffer-_HasAttachments ?
+ buffer-MaxNumLayers : buffer-DefaultGeometry.Layers;
+}
+
extern void
_mesa_update_draw_buffer_bounds(struct gl_context *ctx,
struct gl_framebuffer *drawFb);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 03/10] mesa: Complete implementation for ARB_framebuffer_no_attachments in Mesa core
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Implement GL_ARB_framebuffer_no_attachments in Mesa core
- changes to conditions for framebuffer completenss
- implement set/get functions for framebuffers for
new functions in GL_ARB_framebuffer_no_attachments
v1 - v2
Spacing and exceed 80 characters per line fixes.
v2 - v3
Implement DSA functions of extension.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/fbobject.c | 217
---
1 file changed, 184 insertions(+), 33 deletions(-)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 4ac3f20..18def71 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1156,14 +1156,49 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
} else if (att_layer_count max_layer_count) {
max_layer_count = att_layer_count;
}
+
+ /**
+ * The extension GL_ARB_framebuffer_no_attachments places the
additional
+ * requirement on each attachment that
+ *
+ * The width and height of image are greater than zero and less than
or
+ * equal to the values of the implementation-dependent limits
+ * MAX_FRAMEBUFFER_WIDTH and MAX_FRAMEBUFFER_HEIGHT, respectively.
Extra space between . and
+ *
+ * If image is a three-dimensional texture or a one- or
two-dimensional
+ * array texture and the attachment is layered, the depth or layer
count
+ * of the texture is less than or equal to the
implementation-dependent
+ * limit MAX_FRAMEBUFFER_LAYERS.
+ *
+ * If image has multiple samples, its sample count is less than or
equal
+ * to the value of the implementation-dependent limit
MAX_FRAMEBUFFER_-
+ * SAMPLES .
Extra space before .
Also, if anyone is ever grepping for MAX_FRAMEBUFFER_SAMPLES, they
won't find this. I'd move the whole word to the next line.
+ *
+ * The same requirements are also in place for GL 4.5,
+ * Section 9.4.1 Framebuffer Attachment Completeness, pg 310-311
+ *
+ * However, this is a tighter restriction than previous version of GL.
+ * In interest of better compatibility, we will not enforce these
+ * restrictions.
+ */
Comment at the end of a block like this looks strange. I'm not sure
what it's commenting on.
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 21/22] i965/gen9: Plugin the code for selecting YF/YS tiling on skl+
On Fri, Apr 17, 2015 at 04:51:42PM -0700, Anuj Phogat wrote:
Note: Yf/Ys tiling stays disabled to avoid any piglit regressions. I'm
working on fixing the remaining piglit failures.
We need to do some benchmarking to come up with conditions to choose
Ys (64 KB) over Yf (4 KB). Any thoughts on how big a texture should
be so that 64 KB tiling is preferred over 4KB?
Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 81
++
1 file changed, 81 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index 28927e9..b2a2dac 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -684,6 +684,65 @@ intel_miptree_total_width_height(struct brw_context *brw,
}
}
+static bool
+intel_miptree_choose_tr_mode(struct brw_context *brw,
Either rename this function, or make it actually choose the tr_mode.
+ mesa_format format,
+ uint32_t width0,
+ uint32_t num_samples,
+ enum intel_miptree_tiling_mode requested,
+ struct intel_mipmap_tree *mt,
+ uint32_t tr_mode)
As I state above and below, I don't think tr_mode should be passed in here. This
function is supposed to choose the mode.
+{
+ const unsigned bpp = mt-cpp * 8;
+
+ /* bpp must be power of 2. */
+ if (!mt-compressed
+ _mesa_is_format_color_format(mt-format)
I am not finding the reason for the color format restriction. I believe it, just
not seeing it...
+ (requested == INTEL_MIPTREE_TILING_Y ||
+requested == INTEL_MIPTREE_TILING_ANY)
+ (bpp (bpp (bpp - 1)) == 0)) {
+
is_power_of_2
How about something like...
/* Low bits are the highest priority modes */
modes = INTEL_MIPTREE_TRMODE_YS | INTEL_MIPTREE_TRMODE_YF;
while ((modes = 1) 1) {
... try to allocate
}
+ mt-tr_mode = tr_mode;
you're leaking tr_mode state here if you fail below.
+ mt-align_w = intel_horizontal_texture_alignment_unit(brw, mt);
+ mt-align_h = intel_vertical_texture_alignment_unit(brw, mt);
+
+ intel_miptree_total_width_height(brw, mt);
+
+ /* pitch == 0 || height == 0 indicates the null texture */
What is the null texture?
+ if (!mt || !mt-total_width || !mt-total_height) {
+ intel_miptree_release(mt);
+ return false;
+ }
+
+ mt-tiling = intel_miptree_choose_tiling(brw, format, width0,
+ num_samples,
+ requested, mt);
+
+ if (mt-tiling == I915_TILING_Y ||
+ mt-tiling == (I915_TILING_Y | I915_TILING_X)) {
+
+ /* Don't allow YS tiling at the moment. Using 64KB tiling for small
+ * textures might result in to wastage of memory.
+ * FIXME: Revisit this condition when we have more information about
+ * the specific cases where using YS over YF will be useful.
+ */
+ if (mt-tr_mode == INTEL_MIPTREE_TRMODE_YF)
+return true;
+ }
+
+ /* Can't use requested tr_mode. Free up the memory allocated for
+ * miptree levels in intel_miptree_total_width_height().
+ */
+ unsigned level;
+ for (level = mt-first_level; level = mt-last_level; level++) {
+ free(mt-level[level].slice);
+ mt-level[level].slice = NULL;
+ }
intel_miptree_release(mt); ? Either that or remove it above. Seems like
returning false should always do the same thing.
+ }
+
+ return false;
+}
+
void
brw_miptree_layout(struct brw_context *brw,
mesa_format format,
@@ -695,6 +754,28 @@ brw_miptree_layout(struct brw_context *brw,
{
bool gen6_hiz_or_stencil = false;
+ /* Check in advance if we can do Y tiling with Yf or Ys tiled resource
+* modes. Fall back to using INTEL_MIPTREE_TRMODE_NONE.
+* FIXME: Buffers with Yf/Ys tiling end up using meta upload/download
+* paths or the blitter for cases where they used tiled_memcpy paths in
+* case of Y tiling. This has exposed some bugs and missing support of
+* few formats in meta path. To avoid piglit regressions disable the
+* Yf/Ys tiling at the moment.
+*/
+ if (brw-gen = 9 !for_bo false /* disable Yf/Ys */) {
+ /* Prefer YF over YS at the moment. */
+ if (intel_miptree_choose_tr_mode(brw, format, width0, num_samples,
+ requested, mt,
+ INTEL_MIPTREE_TRMODE_YF) ||
+ intel_miptree_choose_tr_mode(brw, format, width0, num_samples,
+ requested, mt,
+ INTEL_MIPTREE_TRMODE_YS))
+
Re: [Mesa-dev] [v3 PATCH 03/10] mesa: Complete implementation for ARB_framebuffer_no_attachments in Mesa core
Extra space between . and Fix I will Extra space before . Fix I will Also, if anyone is ever grepping for MAX_FRAMEBUFFER_SAMPLES, they won't find this. I'd move the whole word to the next line. Ok. + * + * The same requirements are also in place for GL 4.5, + * Section 9.4.1 Framebuffer Attachment Completeness, pg 310-311 + * + * However, this is a tighter restriction than previous version of GL. + * In interest of better compatibility, we will not enforce these + * restrictions. + */ Comment at the end of a block like this looks strange. I'm not sure what it's commenting on. The purpose of the comment block is to list what the extension and GL4.5 add to the requirements for FBO happiness. The last sentence is writing out *now* that these added requirements, if enforced, risk breaking older applications and stating that the extra requirements will not be enforced. Would it be helpful at the top of the comment block to say GL4.5/GL_ARB_framebuffer_no_attachments added these requirements? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 22/22] i965/gen9: Disable Mip Tail for YF/YS tiled surfaces
On Fri, Apr 17, 2015 at 04:51:43PM -0700, Anuj Phogat wrote:
This fixed the buffer corruption happening in a FBO which use YF/YS
tiled renderbuffer or texture as color attachment.
Spec recommends disabling mip tails for non-mip-mapped surfaces.
But, with this enabled I couldn't get correct data out of YF/YS
tiled surface. I get the expected data with this disabled.
The docs do say to disable to disable the Mip Tail this field must be set to a
mip that larger than those present in the surface So I don't see why you said,
But. The docs also say you must set this field when trmode isn't None.
In other words, if you don't want to use mip tails, you are using the correct
mechanism to disable it.
I haven't spent any time trying to understand miptails. So, I'm
not sure if this patch is the right thing to do. But, It helps
move things forward at the moment.
I don't understand them either, but I think your code is fine. Maybe add an
assert that max level is 15?
So with some explanation in the commit message that this disables mip tails,
this is:
Reviewed-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
src/mesa/drivers/dri/i965/brw_defines.h| 3 +++
src/mesa/drivers/dri/i965/gen8_surface_state.c | 10 --
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index c62c09b..1c50172 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -594,6 +594,9 @@
#define GEN9_SURFACE_TRMODE_TILEYF 1
#define GEN9_SURFACE_TRMODE_TILEYS 2
+#define GEN9_SURFACE_MIP_TAIL_START_LOD_SHIFT 8
+#define GEN9_SURFACE_MIP_TAIL_START_LOD_MASK INTEL_MASK(11, 8)
+
/* Surface state DW6 */
#define GEN7_SURFACE_MCS_ENABLE (1 0)
#define GEN7_SURFACE_MCS_PITCH_SHIFT3
diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index 189f1db..155563f 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -258,8 +258,11 @@ gen8_update_texture_surface(struct gl_context *ctx,
GEN7_SURFACE_MIN_LOD) |
(intelObj-_MaxLevel - tObj-BaseLevel); /* mip count */
- if (brw-gen = 9)
+ if (brw-gen = 9) {
surf[5] |= SET_FIELD(tr_mode, GEN9_SURFACE_TRMODE);
+ /* Disable Mip Tail by setting a large value. */
+ surf[5] |= SET_FIELD(15, GEN9_SURFACE_MIP_TAIL_START_LOD);
+ }
if (aux_mt) {
surf[6] = SET_FIELD(mt-qpitch / 4, GEN8_SURFACE_AUX_QPITCH) |
@@ -446,8 +449,11 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
surf[5] = irb-mt_level - irb-mt-first_level;
- if (brw-gen = 9)
+ if (brw-gen = 9) {
surf[5] |= SET_FIELD(tr_mode, GEN9_SURFACE_TRMODE);
+ /* Disable Mip Tail by setting a large value. */
+ surf[5] |= SET_FIELD(15, GEN9_SURFACE_MIP_TAIL_START_LOD);
+ }
if (aux_mt) {
surf[6] = SET_FIELD(mt-qpitch / 4, GEN8_SURFACE_AUX_QPITCH) |
--
2.3.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir/remove_phis: handle trivial back-edges
Some loops may have phi nodes that look like:
foo = ...
loop {
bar = phi(foo, bar)
...
}
in which case we can remove the phi node and replace all uses of 'bar'
with 'foo'. In particular, there are some L4D2 vertex shaders with loops
that, after optimization, look like:
/* succs: block_1 */
loop {
block block_1:
/* preds: block_0 block_4 */
vec1 ssa_2195 = phi block_0: ssa_2136, block_4: ssa_994
vec1 ssa_7321 = phi block_0: ssa_8195, block_4: ssa_7321
vec1 ssa_7324 = phi block_0: ssa_8198, block_4: ssa_7324
vec1 ssa_7327 = phi block_0: ssa_8174, block_4: ssa_7327
vec1 ssa_8139 = intrinsic load_uniform () () (232)
vec1 ssa_588 = ige ssa_2195, ssa_8139
/* succs: block_2 block_3 */
if ssa_588 {
block block_2:
/* preds: block_1 */
break
/* succs: block_5 */
} else {
block block_3:
/* preds: block_1 */
/* succs: block_4 */
}
block block_4:
/* preds: block_3 */
vec1 ssa_994 = iadd ssa_2195, ssa_2150
/* succs: block_1 */
}
where after removing the second, third, and fourth phi nodes, the loop
becomes entirely dead, and this patch combined with my nir-dead-cf-v4
branch will cause the loop to be deleted entirely.
No piglit regressions.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir_opt_remove_phis.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/glsl/nir/nir_opt_remove_phis.c
b/src/glsl/nir/nir_opt_remove_phis.c
index 7896584..3660413 100644
--- a/src/glsl/nir/nir_opt_remove_phis.c
+++ b/src/glsl/nir/nir_opt_remove_phis.c
@@ -60,6 +60,21 @@ remove_phis_block(nir_block *block, void *state)
nir_foreach_phi_src(phi, src) {
assert(src-src.is_ssa);
+
+ /* For phi nodes at the beginning of loops, we may encounter some
+ * sources from backedges that point back to the destination of the
+ * same phi, i.e. something like:
+ *
+ * a = phi(a, b, ...)
+ *
+ * We can safely ignore these sources, since if all of the normal
+ * sources point to the same definition, then that definition must
+ * still dominate the phi node, and the phi will still always take
+ * the value of that definition.
+ */
+
+ if (src-src.ssa == phi-dest.ssa)
+continue;
if (def == NULL) {
def = src-src.ssa;
@@ -74,6 +89,11 @@ remove_phis_block(nir_block *block, void *state)
if (!srcs_same)
continue;
+ /* We must have found at least one definition, since there must be at
+ * least one forward edge.
+ */
+ assert(def != NULL);
+
assert(phi-dest.is_ssa);
nir_ssa_def_rewrite_uses(phi-dest.ssa, nir_src_for_ssa(def),
mem_ctx);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 10/10] mark GL_ARB_framebuffer_no_attachments as done for i965
On Thu, May 21, 2015 at 5:30 PM, kevin.rogo...@intel.com wrote: From: Kevin Rogovin kevin.rogo...@intel.com Mark GL_ARB_framebuffer_no_attachments as done for i965. v1 - v2 File added to patch series v2 - v3 docs/GL3.txt : add done mark under GLES3.1 docs/relnotes/10.6.0.html : maintain alphabetical order Reviewed-by: Ian Romanick ian.d.romanick at intel.com (v2) Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com --- docs/GL3.txt | 4 ++-- docs/relnotes/10.6.0.html | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/GL3.txt b/docs/GL3.txt index 9d56ee5..fae8253 100644 --- a/docs/GL3.txt +++ b/docs/GL3.txt @@ -157,7 +157,7 @@ GL 4.3, GLSL 4.30: GL_KHR_debug DONE (all drivers) GL_ARB_explicit_uniform_location DONE (all drivers that support GLSL) GL_ARB_fragment_layer_viewport DONE (nv50, nvc0, r600, llvmpipe) - GL_ARB_framebuffer_no_attachmentsnot started + GL_ARB_framebuffer_no_attachmentsDONE (i965) GL_ARB_internalformat_query2 not started GL_ARB_invalidate_subdataDONE (all drivers) GL_ARB_multi_draw_indirect DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe) @@ -216,7 +216,7 @@ GLES3.1, GLSL ES 3.1 GL_ARB_compute_shaderin progress (jljusten) GL_ARB_draw_indirect DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe) GL_ARB_explicit_uniform_location DONE (all drivers that support GLSL) - GL_ARB_framebuffer_no_attachmentsnot started + GL_ARB_framebuffer_no_attachmentsDONE (i965) GL_ARB_program_interface_query DONE (all drivers) GL_ARB_shader_atomic_countersDONE (i965) GL_ARB_shader_image_load_store in progress (curro) diff --git a/docs/relnotes/10.6.0.html b/docs/relnotes/10.6.0.html index 474a2c7..bc0cf8e 100644 --- a/docs/relnotes/10.6.0.html +++ b/docs/relnotes/10.6.0.html 10.7.0, at this point. @@ -51,6 +51,7 @@ Note: some of the new features are only available with certain drivers. liGL_ARB_direct_state_access on all drivers that support GL 2.0+/li liGL_ARB_draw_indirect, GL_ARB_multi_draw_indirect on r600/li liGL_ARB_draw_instanced on freedreno/li +liGL_ARB_framebuffer_no_attachments on i965/li liGL_ARB_gpu_shader_fp64 on nvc0, softpipe/li liGL_ARB_gpu_shader5 on i965/gen8+/li liGL_ARB_instanced_arrays on freedreno/li -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 10/10] mark GL_ARB_framebuffer_no_attachments as done for i965
On Fri, 2015-05-22 at 00:30 +0300, kevin.rogo...@intel.com wrote: From: Kevin Rogovin kevin.rogo...@intel.com Mark GL_ARB_framebuffer_no_attachments as done for i965. v1 - v2 File added to patch series v2 - v3 docs/GL3.txt : add done mark under GLES3.1 docs/relnotes/10.6.0.html : maintain alphabetical order Reviewed-by: Ian Romanick ian.d.romanick at intel.com (v2) ian.d.roman...@intel.com I've done this before too, copying the Reviewed-by string from the mailing list archive. I noticed the same thing in another patch too. Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com --- docs/GL3.txt | 4 ++-- docs/relnotes/10.6.0.html | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/GL3.txt b/docs/GL3.txt index 9d56ee5..fae8253 100644 --- a/docs/GL3.txt +++ b/docs/GL3.txt @@ -157,7 +157,7 @@ GL 4.3, GLSL 4.30: GL_KHR_debug DONE (all drivers) GL_ARB_explicit_uniform_location DONE (all drivers that support GLSL) GL_ARB_fragment_layer_viewport DONE (nv50, nvc0, r600, llvmpipe) - GL_ARB_framebuffer_no_attachmentsnot started + GL_ARB_framebuffer_no_attachmentsDONE (i965) GL_ARB_internalformat_query2 not started GL_ARB_invalidate_subdataDONE (all drivers) GL_ARB_multi_draw_indirect DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe) @@ -216,7 +216,7 @@ GLES3.1, GLSL ES 3.1 GL_ARB_compute_shaderin progress (jljusten) GL_ARB_draw_indirect DONE (i965, nvc0, r600, radeonsi, llvmpipe, softpipe) GL_ARB_explicit_uniform_location DONE (all drivers that support GLSL) - GL_ARB_framebuffer_no_attachmentsnot started + GL_ARB_framebuffer_no_attachmentsDONE (i965) GL_ARB_program_interface_query DONE (all drivers) GL_ARB_shader_atomic_countersDONE (i965) GL_ARB_shader_image_load_store in progress (curro) diff --git a/docs/relnotes/10.6.0.html b/docs/relnotes/10.6.0.html index 474a2c7..bc0cf8e 100644 --- a/docs/relnotes/10.6.0.html +++ b/docs/relnotes/10.6.0.html @@ -51,6 +51,7 @@ Note: some of the new features are only available with certain drivers. liGL_ARB_direct_state_access on all drivers that support GL 2.0+/li liGL_ARB_draw_indirect, GL_ARB_multi_draw_indirect on r600/li liGL_ARB_draw_instanced on freedreno/li +liGL_ARB_framebuffer_no_attachments on i965/li liGL_ARB_gpu_shader_fp64 on nvc0, softpipe/li liGL_ARB_gpu_shader5 on i965/gen8+/li liGL_ARB_instanced_arrays on freedreno/li ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 06/10] i965: Use _mesa_geometry_ functions appropriately
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Change references to gl_framebuffer::Width, Height, MaxNumLayers
and Visual::samples to use the _mesa_geometry_ convenience functions
for those places where the geometry of the gl_framebuffer is needed
(in contrast to the geometry of the intersection of the attachments
of the gl_framebuffer).
This patch is to pave the way to enable GL_ARB_framebuffer_no_attachments
on Gen7 and higher in i965.
v1 - v2
Remove changes that would only be active in Gen4/5.
Type and casting changes for consistency and readability.
v2 - v3
Updates for rebase against master
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/drivers/dri/i965/brw_clip_state.c | 9 ++---
This is gen4/5 only, isn't it?
src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +++---
src/mesa/drivers/dri/i965/brw_sf_state.c | 8
As is this?
src/mesa/drivers/dri/i965/brw_state_upload.c | 6 --
src/mesa/drivers/dri/i965/brw_wm.c | 7 ---
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +++-
src/mesa/drivers/dri/i965/gen6_clip_state.c| 10 +++---
src/mesa/drivers/dri/i965/gen6_multisample_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen6_scissor_state.c | 13 ++---
src/mesa/drivers/dri/i965/gen6_sf_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen6_viewport_state.c| 5 +++--
src/mesa/drivers/dri/i965/gen6_wm_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen7_sf_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen7_viewport_state.c| 5 +++--
src/mesa/drivers/dri/i965/gen7_wm_state.c | 3 ++-
src/mesa/drivers/dri/i965/gen8_viewport_state.c| 8 +---
16 files changed, 74 insertions(+), 34 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clip_state.c
b/src/mesa/drivers/dri/i965/brw_clip_state.c
index 3223834..dee74db 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_state.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_state.c
@@ -32,6 +32,7 @@
#include brw_context.h
#include brw_state.h
#include brw_defines.h
+#include main/framebuffer.h
static void
upload_clip_vp(struct brw_context *brw)
@@ -59,7 +60,9 @@ brw_upload_clip_unit(struct brw_context *brw)
struct brw_clip_unit_state *clip;
/* _NEW_BUFFERS */
- struct gl_framebuffer *fb = ctx-DrawBuffer;
+ const struct gl_framebuffer *fb = ctx-DrawBuffer;
+ const float fb_width = (float)_mesa_geometric_width(fb);
+ const float fb_height = (float)_mesa_geometric_height(fb);
upload_clip_vp(brw);
@@ -127,8 +130,8 @@ brw_upload_clip_unit(struct brw_context *brw)
/* enable guardband clipping if we can */
if (ctx-ViewportArray[0].X == 0
ctx-ViewportArray[0].Y == 0
- ctx-ViewportArray[0].Width == (float) fb-Width
- ctx-ViewportArray[0].Height == (float) fb-Height)
+ ctx-ViewportArray[0].Width == fb_width
+ ctx-ViewportArray[0].Height == fb_height)
{
clip-clip5.guard_band_enable = 1;
clip-clip6.clipper_viewport_state_ptr =
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 67a693b..1672786 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -39,6 +39,7 @@
#include brw_state.h
#include brw_defines.h
+#include main/framebuffer.h
#include main/fbobject.h
#include main/glformats.h
@@ -46,12 +47,15 @@
static void upload_drawing_rect(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
+ const struct gl_framebuffer *fb = ctx-DrawBuffer;
+ const unsigned int fb_width = _mesa_geometric_width(fb);
+ const unsigned int fb_height = _mesa_geometric_height(fb);
BEGIN_BATCH(4);
OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE 16 | (4 - 2));
OUT_BATCH(0); /* xmin, ymin */
- OUT_BATCH(((ctx-DrawBuffer-Width - 1) 0x) |
- ((ctx-DrawBuffer-Height - 1) 16));
+ OUT_BATCH(((fb_width - 1) 0x) |
+ ((fb_height - 1) 16));
Remove the tab on this line and indent it properly while we're changing it.
OUT_BATCH(0);
ADVANCE_BATCH();
}
@@ -767,7 +771,7 @@ static void upload_polygon_stipple_offset(struct
brw_context *brw)
* works just fine, and there's no window system to worry about.
*/
if (_mesa_is_winsys_fbo(ctx-DrawBuffer))
- OUT_BATCH((32 - (ctx-DrawBuffer-Height 31)) 31);
+ OUT_BATCH((32 - (_mesa_geometric_height(ctx-DrawBuffer) 31)) 31);
else
OUT_BATCH(0);
ADVANCE_BATCH();
diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c
b/src/mesa/drivers/dri/i965/brw_sf_state.c
index 014b434..6f9397f 100644
--- a/src/mesa/drivers/dri/i965/brw_sf_state.c
+++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
@@ -52,6 +52,14 @@ static void
Re: [Mesa-dev] [v3 PATCH 08/10] i965: ensure execution of fragment shader when fragment shader has atomic buffer access
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Ensure that the GPU spawns the fragment shader thread for those
fragment shaders with atomic buffer access.
v1 - v2
No change.
v2 - v3
Use utility function _mesa_active_fragment_shader_has_atomic_ops().
Reviewed-by: Tapani Pälli tapani.palli at intel.com (v1)
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/drivers/dri/i965/gen7_wm_state.c | 4
src/mesa/drivers/dri/i965/gen8_ps_state.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c
b/src/mesa/drivers/dri/i965/gen7_wm_state.c
index 1c47076..63092cf 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
@@ -77,6 +77,10 @@ upload_wm_state(struct brw_context *brw)
dw1 |= GEN7_WM_KILL_ENABLE;
}
+ if (_mesa_active_fragment_shader_has_atomic_ops(brw-ctx) ) {
Extra space between ) and )
+ dw1 |= GEN7_WM_DISPATCH_ENABLE;
+ }
+
/* _NEW_BUFFERS | _NEW_COLOR */
if (brw_color_buffer_write_enabled(brw) || writes_depth ||
dw1 GEN7_WM_KILL_ENABLE) {
diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c
b/src/mesa/drivers/dri/i965/gen8_ps_state.c
index 85ad3b6..3dee8b6 100644
--- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
@@ -58,6 +58,9 @@ gen8_upload_ps_extra(struct brw_context *brw,
if (prog_data-uses_omask)
dw1 |= GEN8_PSX_OMASK_TO_RENDER_TARGET;
+ if (_mesa_active_fragment_shader_has_atomic_ops(brw-ctx) )
Extra space between ) and )
+ dw1 |= GEN8_PSX_SHADER_HAS_UAV;
+
BEGIN_BATCH(2);
OUT_BATCH(_3DSTATE_PS_EXTRA 16 | (2 - 2));
OUT_BATCH(dw1);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
Some commit message nits... :)
On 2015-05-21 14:30:48, wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Define the infrastructure for the extension GL_ARB_framebuffer_no_attachments:
This line is too long. (It will not fit in 80 columns in git log since
git log adds some spaces before each commit message line.)
- extension table
- additions to gl_framebuffer
v1 - v2
Spacing and trailing spaces fixes.
This looks odd to me. I think you only need 'v2:' here. So, either
v2: Spacing and trailing spaces fixes
or,
v2:
* Spacing and trailing spaces fixes
-Jordan
v2 - v3
mtypes.h: Correct comment on _HasAttachments.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/extensions.c | 1 +
src/mesa/main/fbobject.c| 1 +
src/mesa/main/framebuffer.c | 1 +
src/mesa/main/mtypes.h | 50
-
4 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index c82416a..3256b2c 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -117,6 +117,7 @@ static const struct extension extension_table[] = {
{ GL_ARB_fragment_program,o(ARB_fragment_program),
GLL,2002 },
{ GL_ARB_fragment_program_shadow,
o(ARB_fragment_program_shadow), GLL,2003 },
{ GL_ARB_fragment_shader, o(ARB_fragment_shader),
GL, 2002 },
+ { GL_ARB_framebuffer_no_attachments,
o(ARB_framebuffer_no_attachments), GL, 2012 },
{ GL_ARB_framebuffer_object,
o(ARB_framebuffer_object), GL, 2005 },
{ GL_ARB_framebuffer_sRGB,o(EXT_framebuffer_sRGB),
GL, 1998 },
{ GL_ARB_get_program_binary, o(dummy_true),
GL, 2010 },
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 1859c27..8fea7f8 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -957,6 +957,7 @@ _mesa_test_framebuffer_completeness(struct gl_context
*ctx,
fb-Height = 0;
fb-_AllColorBuffersFixedPoint = GL_TRUE;
fb-_HasSNormOrFloatColorBuffer = GL_FALSE;
+ fb-_HasAttachments = GL_TRUE;
/* Start at -2 to more easily loop over all attachment points.
* -2: depth buffer
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index 665a5ba..c2cfb92 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -157,6 +157,7 @@ _mesa_initialize_window_framebuffer(struct gl_framebuffer
*fb,
fb-_Status = GL_FRAMEBUFFER_COMPLETE_EXT;
fb-_AllColorBuffersFixedPoint = !visual-floatMode;
fb-_HasSNormOrFloatColorBuffer = visual-floatMode;
+ fb-_HasAttachments = GL_TRUE;
compute_depth_max(fb);
}
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 8342517..1a37aa6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3146,12 +3146,29 @@ struct gl_framebuffer
*/
struct gl_config Visual;
- GLuint Width, Height; /** size of frame buffer in pixels */
+ /**
+* size of frame buffer in pixels,
+* no attachments has these values as 0
+*/
+ GLuint Width, Height;
+
+ /**
+* In the case that the framebuffer has no attachment (i.e.
+* GL_ARB_framebuffer_no_attachments) then the geometry of
+* the framebuffer is specified by the default values.
+*/
+ struct {
+ GLuint Width, Height, Layers, NumSamples;
+ GLboolean FixedSampleLocations;
+ } DefaultGeometry;
- /** \name Drawing bounds (Intersection of buffer size and scissor box) */
+ /** \name Drawing bounds (Intersection of buffer size and scissor box)
+* The drawing region is given by [_Xmin, _Xmax) x [_Ymin, _Ymax),
+* (inclusive for _Xmin and _Ymin while exclusive for _Xmax and _Ymax)
+*/
/*@{*/
- GLint _Xmin, _Xmax; /** inclusive */
- GLint _Ymin, _Ymax; /** exclusive */
+ GLint _Xmin, _Xmax;
+ GLint _Ymin, _Ymax;
/*@}*/
/** \name Derived Z buffer stuff */
@@ -3164,6 +3181,18 @@ struct gl_framebuffer
/** One of the GL_FRAMEBUFFER_(IN)COMPLETE_* tokens */
GLenum _Status;
+ /** True if one of Attachment has Type != GL_NONE
+* NOTE: the values for Width and Height are set to 0 in
+* case of no attachments, a backend driver supporting
+* GL_ARB_framebuffer_no_attachments must check for the
+* flag _HasAttachments and if GL_FALSE, must then use
+* the values in DefaultGeometry to initialize its
+* viewport, scissor and so on (in particular _Xmin,
+* _Xmax, _Ymin and _Ymax do NOT take into account
+* _HasAttachments being false)
+*/
+ GLboolean _HasAttachments;
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
This line is too long. (It will not fit in 80 columns in git log since git log adds some spaces before each commit message line.) What is the accepted maximum length for a line in a commit message? - extension table - additions to gl_framebuffer v1 - v2 Spacing and trailing spaces fixes. This looks odd to me. I think you only need 'v2:' here. So, either I have seen a number of patches with the notation v1 - v2 to list changes between versions. Those patches that I saw using that notation did not have comments about using the format v1-v2. If people want v2: instead of v1-v2, I am fine with it, but was just following what I saw in some patch series. Given the number of nits around (that I seem to hit regularly), it might be beneficial for Mesa to have a short document listing the formatting requirements, of which I have so far collected: 1. 80 column limit in source files 2. Justify comments to 80 columns as well 3. comparison expressions require spaces around both sides of comparison operator 4. successive parenthesis must have spaces between parenthesis 5. git commit messages have limit of N characters per line (the value of N I am asking above). 6. Use whether condition when describing a bool instead of true if condition is true 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line 10. no white spaces at end of line 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. 12. (Guessing here) consecutive empty lines are not allowed 13. If changing a line that violates the nit rules, fix the line too rather than just adding the change I suspect there are more as I listen to the nits, I think it might be beneficial to collect all the formatting nits and write them down to the coding standard thing in Mesa so that others can refer to it. Especially useful for those that work on multiple projects with different coding standards. -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 07/10] mesa: add helper function for testing if current fragment shader has atomics
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote:
From: Kevin Rogovin kevin.rogo...@intel.com
Add helper function that checks if current fragment shader active
of gl_context has atomic buffer access.
v1 - v3
Added in v3 of patch series.
Signed-off-by: Kevin Rogovin kevin.rogo...@intel.com
---
src/mesa/main/mtypes.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 7e1f0e0..b88b10a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4464,7 +4464,12 @@ enum _debug
DEBUG_INCOMPLETE_FBO = (1 3)
};
-
+static inline bool
+_mesa_active_fragment_shader_has_atomic_ops(const struct gl_context *ctx)
+{
+ return ctx-Shader._CurrentFragmentProgram!=NULL
Spaces around !=...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 00/10] Implement extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote: From: Kevin Rogovin kevin.rogo...@intel.com This patch series implements: - the needed functionality in Mesa-core for ARB_framebuffer_no_attachments - implements and enables the extension i965 I think there was one patch I didn't notice some kind of obvious style problem with. Extra spaces or newlines, weird line wrapping, no spaces around operators... please look over the patches yourself before sending them. We should not have this many style nits by v3. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nv50: make pipeline statistics queries use 64-bits, based on nvc0
Series is: Reviewed-by: Ilia Mirkin imir...@alum.mit.edu
But please adjust the commit descriptions to explain what they fix.
Like this fixes PIPELINE_STATISTICS with HUD. The previous one fixes
64-bit queries with HUD.
On Thu, May 21, 2015 at 5:58 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
Tested on NVA8. No regression for ARB_pipeline_statistics piglit tests.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index a3c8841..da41209 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -114,7 +114,8 @@ nv50_query_create(struct pipe_context *pipe, unsigned
type, unsigned index)
q-is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
type == PIPE_QUERY_PRIMITIVES_EMITTED ||
- type == PIPE_QUERY_SO_STATISTICS);
+ type == PIPE_QUERY_SO_STATISTICS ||
+ type == PIPE_QUERY_PIPELINE_STATISTICS);
q-type = type;
if (q-type == PIPE_QUERY_OCCLUSION_COUNTER) {
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 00/10] Implement extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 4:23 PM, Matt Turner matts...@gmail.com wrote: On Thu, May 21, 2015 at 2:30 PM, kevin.rogo...@intel.com wrote: From: Kevin Rogovin kevin.rogo...@intel.com This patch series implements: - the needed functionality in Mesa-core for ARB_framebuffer_no_attachments - implements and enables the extension i965 I think there was one patch I didn't notice some kind of obvious style problem with. Extra spaces or newlines, weird line wrapping, no spaces around operators... please look over the patches yourself before sending them. We should not have this many style nits by v3. Also, the patches were sent to mesa-...@freedesktop.org, but the appropriate address is mesa-dev@lists.freedesktop.org. I'm surprised they made it through. You can avoid ever typo'ing the address by putting this in the .git/config of your mesa repository: [sendemail] to = mesa-dev@lists.freedesktop.org With that set, you don't even need to specify --to=... with git send-email. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 06/10] i965: Use _mesa_geometry_ functions appropriately
src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +++--- src/mesa/drivers/dri/i965/brw_sf_state.c | 8 As is this? brw_misc_state.c is active for all Gens. The change to brw_sf_state.c is to add a comment warning that using gl_framebuffer::Width and so on is only ok if _HasAttachments is false. BEGIN_BATCH(4); OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE 16 | (4 - 2)); OUT_BATCH(0); /* xmin, ymin */ - OUT_BATCH(((ctx-DrawBuffer-Width - 1) 0x) | - ((ctx-DrawBuffer-Height - 1) 16)); + OUT_BATCH(((fb_width - 1) 0x) | + ((fb_height - 1) 16)); Remove the tab on this line and indent it properly while we're changing it. Actually it fits on one line, so that will be the change :) + /* Accessing the fields Width and Height of +* gl_framebuffer to produce the values to +* program the viewport and scissor is fine +* as long as the gl_framebuffer has atleast +* one attachment. Line wrapping... OK. struct brw_state_flags state = brw-state.pipelines[pipeline]; + int fb_samples = (int)_mesa_geometric_samples(ctx-DrawBuffer); The cast looks strange Is there a spacing missing in the cast? or is it strange because of the types? These casts look weird. (Happens elsewhere in this patch too). Assuming brw_context::num_samples doesn't need to be signed, I'd be inclined to change its type to unsigned and remove the casts. Grepping for 'num_samples = ' shows some other instances of us implicitly converting unsigned - int. I was hesitant to be changing types in the patch series. I wanted to minimize the changes and try to keep it consistent with what is left unchanged. If the types should be unsigned int, I can do that, but I would like to hear the opinions of others too. Extra new line. OK. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 06/10] i965: Use _mesa_geometry_ functions appropriately
On Thu, May 21, 2015 at 4:44 PM, Rogovin, Kevin kevin.rogo...@intel.com wrote: struct brw_state_flags state = brw-state.pipelines[pipeline]; + int fb_samples = (int)_mesa_geometric_samples(ctx-DrawBuffer); The cast looks strange Is there a spacing missing in the cast? or is it strange because of the types? Strange because of the types -- presumably fb_samples is an int because its uses are in a comparison with another int (that probably doesn't need to be an int :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
HI, Or 78 columns, to be safe, but there's exceptions, like if you're defining a big static table/array of info. Uggg I don't mind exceptions, but knowing them is key. 4. successive parenthesis must have spaces between parenthesis Example? if (some_func(some_argument)) is bad, but if (some_func(some_argument) ) is good. I am also guessing that a = foo(bar(x)); is bad and must be changed to a = foo(bar(x) ); 6. Use whether condition when describing a bool instead of true if condition is true not sure we care about that. I hit a nit for it in the series, so someone cared. 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line The 'indent' command in the docs should cover that. 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. Not sure about that, but that's not a coding style standard. Perhaps coding standard is not the right word, but it is a requirement (atleast it seems that way) and is a trivial requirement to satisfy. 12. (Guessing here) consecutive empty lines are not allowed Generally true, except between functions. Ugg... I hit a nit from an extra space between functions. 13. If changing a line that violates the nit rules, fix the line too rather than just adding the change Yeah, probably. Feel free to submit a patch against docs/devinfo.html with this info. :) I do not mind submitting the patch, but I want to know what the rules are; preferably explicitly written rather than inferred (by me). Especially since I seem to hit nits like no tomorrow even when trying not to :) -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 17/22] i965/gen9: Add XY_FAST_COPY_BLT support to intelEmitCopyBlit()
On Thu, May 21, 2015 at 12:26 PM, Ben Widawsky b...@bwidawsk.net wrote:
A lot of opinion stuff is below, feel free to ignore them if you don't think
there are improvements.
On Fri, Apr 17, 2015 at 04:51:38PM -0700, Anuj Phogat wrote:
This patch enables using XY_FAST_COPY_BLT only for Yf/Ys tiled buffers.
Later It can be turned on for other tiling patterns (X,Y).
Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
src/mesa/drivers/dri/i965/intel_blit.c | 292
+++
src/mesa/drivers/dri/i965/intel_blit.h | 3 +
src/mesa/drivers/dri/i965/intel_copy_image.c | 3 +
src/mesa/drivers/dri/i965/intel_reg.h| 33 +++
4 files changed, 291 insertions(+), 40 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 9500bd7..36746c4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -43,6 +43,23 @@
#define FILE_DEBUG_FLAG DEBUG_BLIT
+#define SET_TILING_XY_FAST_COPY_BLT(tiling, tr_mode, type) \
+({ \
+ switch (tiling) { \
+ case I915_TILING_X: \
+ CMD |= type ## _TILED_X; \
+ break; \
+ case I915_TILING_Y: \
assert(tr_mode != INTEL_MIPTREE_TRMODE_YF); ?
INTEL_MIPTREE_TRMODE_{YF, NONE} are allowed and covered in else case.
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YS)\
+ CMD |= type ## _TILED_64K; \
+ else \
+ CMD |= type ## _TILED_Y;\
+ break; \
+ default: \
+ unreachable(not reached);\
+ } \
+})
+
static void
intel_miptree_set_alpha_to_one(struct brw_context *brw,
struct intel_mipmap_tree *mt,
@@ -75,6 +92,12 @@ static uint32_t
br13_for_cpp(int cpp)
{
switch (cpp) {
+ case 16:nn
+ return BR13_32323232;
+ break;
+ case 8:
+ return BR13_16161616;
+ break;
case 4:
return BR13_;
break;
@@ -89,6 +112,132 @@ br13_for_cpp(int cpp)
}
}
+static uint32_t
+get_tr_horizontal_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
+ /*Alignment tables for YF/YS tiled surfaces. */
+ const uint32_t align_2d_yf[] = {64, 64, 32, 32, 16};
+ const uint32_t align_2d_ys[] = {256, 256, 128, 128, 64};
+ const uint32_t bpp = cpp * 8;
+ uint32_t align;
+ int i = 0;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
+ return 0;
+
+ /* Compute array index. */
+ assert (bpp = 8 bpp = 128 (bpp (bpp - 1)) == 0);
assert(bpp = 8 bpp = 128 _mesa_bitcount(bpp) == 1);
(I couldn't find a is_pow2, but one must exist).
There is a is_power_of_two() in main/macros.h. I'll use that here.
+ while (bpp (i + 4))
+ i++;
+
Since you just asserted this was a power of 2 above, isn't this just:
ffs(bpp/8) - 1;
Yes, ffs() will work too. I'll use it.
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YF)
+ align = align_2d_yf[i];
+ else
+ align = align_2d_ys[i];
+
+ switch(align) {
+ case 512:
+ return is_src ? XY_SRC_H_ALIGN_512 : XY_DST_H_ALIGN_512;
+ case 256:
+ return is_src ? XY_SRC_H_ALIGN_256 : XY_DST_H_ALIGN_256;
+ case 128:
+ return is_src ? XY_SRC_H_ALIGN_128 : XY_DST_H_ALIGN_128;
+ case 64:
+ return is_src ? XY_SRC_H_ALIGN_64 : XY_DST_H_ALIGN_64;
+ case 32:
+ /* XY_FAST_COPY_BLT doesn't support horizontal alignment of 16. */
+ case 16:
+ return is_src ? XY_SRC_H_ALIGN_32 : XY_DST_H_ALIGN_32;
+ default:
+ unreachable(not reached);
+ }
+}
+
+static uint32_t
+get_tr_vertical_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
+ /* Vertical alignment tables for YF/YS tiled surfaces. */
+ const unsigned align_2d_yf[] = {64, 32, 32, 16, 16};
+ const unsigned align_2d_ys[] = {256, 128, 128, 64, 64};
+ const uint32_t bpp = cpp * 8;
+ uint32_t align;
+ int i = 0;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
+ return 0;
+
+ /* Compute array index. */
+ assert (bpp = 8 bpp = 128 (bpp (bpp - 1)) == 0);
+ while (bpp (i + 4))
+ i++;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YF)
+ align = align_2d_yf[i];
+ else
+ align = align_2d_ys[i];
+
Comments above apply here too. Also, it looks easy enough to combine these two
functions, but I'm okay with leaving them
Re: [Mesa-dev] [PATCH V2 17/22] i965/gen9: Add XY_FAST_COPY_BLT support to intelEmitCopyBlit()
On Thu, May 21, 2015 at 1:51 PM, Matt Turner matts...@gmail.com wrote:
On Thu, May 21, 2015 at 12:26 PM, Ben Widawsky b...@bwidawsk.net wrote:
A lot of opinion stuff is below, feel free to ignore them if you don't think
there are improvements.
On Fri, Apr 17, 2015 at 04:51:38PM -0700, Anuj Phogat wrote:
This patch enables using XY_FAST_COPY_BLT only for Yf/Ys tiled buffers.
Later It can be turned on for other tiling patterns (X,Y).
Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
src/mesa/drivers/dri/i965/intel_blit.c | 292
+++
src/mesa/drivers/dri/i965/intel_blit.h | 3 +
src/mesa/drivers/dri/i965/intel_copy_image.c | 3 +
src/mesa/drivers/dri/i965/intel_reg.h| 33 +++
4 files changed, 291 insertions(+), 40 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 9500bd7..36746c4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -43,6 +43,23 @@
#define FILE_DEBUG_FLAG DEBUG_BLIT
+#define SET_TILING_XY_FAST_COPY_BLT(tiling, tr_mode, type) \
+({ \
+ switch (tiling) { \
+ case I915_TILING_X: \
+ CMD |= type ## _TILED_X; \
+ break; \
+ case I915_TILING_Y: \
assert(tr_mode != INTEL_MIPTREE_TRMODE_YF); ?
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YS)\
+ CMD |= type ## _TILED_64K; \
+ else \
+ CMD |= type ## _TILED_Y;\
+ break; \
+ default: \
+ unreachable(not reached);\
+ } \
+})
+
static void
intel_miptree_set_alpha_to_one(struct brw_context *brw,
struct intel_mipmap_tree *mt,
@@ -75,6 +92,12 @@ static uint32_t
br13_for_cpp(int cpp)
{
switch (cpp) {
+ case 16:
+ return BR13_32323232;
+ break;
+ case 8:
+ return BR13_16161616;
+ break;
case 4:
return BR13_;
break;
No need for break after return.
Yes, copy-pasted code.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: do not use _glapi_new_nop_table() for DRI builds
Commit 4bdbb588a9d38 introduced new _glapi_new_nop_table() and
_glapi_set_nop_handler() functions in the glapi dispatcher (which
live in libGL.so). The calls to those functions from context.c
would be undefined (i.e. an ABI break) if the libGL used at runtime
was older.
For the time being, use the old single generic_nop() function for
non-Windows builds to avoid this problem. At some point in the future
it should be safe to remove this work-around. See comments for more
details.
v2: Incorporate feedback from Emil. Use _WIN32 instead of
GLX_DIRECT_RENDERING to control behavior, move comments.
Cc: 10.6 mesa-sta...@lists.freedesktop.org
---
src/mesa/main/context.c | 62 +++--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 544cc14..02875ba 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -883,6 +883,19 @@ update_default_objects(struct gl_context *ctx)
}
+/* XXX this is temporary and should be removed at some point in the
+ * future when there's a reasonable expectation that the libGL library
+ * contains the _glapi_new_nop_table() and _glapi_set_nop_handler()
+ * functions which were added in Mesa 10.6.
+ */
+#if !defined(_WIN32)
+/* Avoid libGL / driver ABI break */
+#define USE_GLAPI_NOP_FEATURES 0
+#else
+#define USE_GLAPI_NOP_FEATURES 1
+#endif
+
+
/**
* This function is called by the glapi no-op functions. For each OpenGL
* function/entrypoint there's a simple no-op function. These no-op
@@ -898,6 +911,7 @@ update_default_objects(struct gl_context *ctx)
*
* \param name the name of the OpenGL function
*/
+#if USE_GLAPI_NOP_FEATURES
static void
nop_handler(const char *name)
{
@@ -914,6 +928,7 @@ nop_handler(const char *name)
}
#endif
}
+#endif
/**
@@ -923,7 +938,45 @@ nop_handler(const char *name)
static void GLAPIENTRY
nop_glFlush(void)
{
- /* don't record an error like we do in _mesa_generic_nop() */
+ /* don't record an error like we do in nop_handler() */
+}
+#endif
+
+
+#if !USE_GLAPI_NOP_FEATURES
+static int
+generic_nop(void)
+{
+ GET_CURRENT_CONTEXT(ctx);
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ unsupported function called
+ (unsupported extension or deprecated function?));
+ return 0;
+}
+#endif
+
+
+/**
+ * Create a new API dispatch table in which all entries point to the
+ * generic_nop() function. This will not work on Windows because of
+ * the __stdcall convention which requires the callee to clean up the
+ * call stack. That's impossible with one generic no-op function.
+ */
+#if !USE_GLAPI_NOP_FEATURES
+static struct _glapi_table *
+new_nop_table(unsigned numEntries)
+{
+ struct _glapi_table *table;
+
+ table = malloc(numEntries * sizeof(_glapi_proc));
+ if (table) {
+ _glapi_proc *entry = (_glapi_proc *) table;
+ unsigned i;
+ for (i = 0; i numEntries; i++) {
+ entry[i] = (_glapi_proc) generic_nop;
+ }
+ }
+ return table;
}
#endif
@@ -941,7 +994,11 @@ alloc_dispatch_table(void)
* Mesa we do this to accommodate different versions of libGL and various
* DRI drivers.
*/
- GLint numEntries = MAX2(_glapi_get_dispatch_table_size(), _gloffset_COUNT);
+ int numEntries = MAX2(_glapi_get_dispatch_table_size(), _gloffset_COUNT);
+
+#if !USE_GLAPI_NOP_FEATURES
+ struct _glapi_table *table = new_nop_table(numEntries);
+#else
struct _glapi_table *table = _glapi_new_nop_table(numEntries);
#if defined(_WIN32)
@@ -967,6 +1024,7 @@ alloc_dispatch_table(void)
#endif
_glapi_set_nop_handler(nop_handler);
+#endif
return table;
}
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 06/10] i965: Use _mesa_geometry_ functions appropriately
Strange because of the types -- presumably fb_samples is an int because its uses are in a comparison with another int (that probably doesn't need to be an int :) I am really hesitant to start changing types all over the place; I have a sinking suspicion that changing the types of fb_width, _height and so on to unsigned int might require some careful checking (the minus ones here and there need some checking. If it really must be done then so be it, but the current series preserves more of the old behavior and I want to try to not add even more changes. So just to ask explicitly: do you want that those values fb_width = _mesa_geometry_width() and related buggers to be unsigned int and go through the process of making sure unsigned int is ok? There are some uglies related to a few minus one 's that make me twitch. Lastly, when I posted v2 of this series, no one commented on the use of int's. For the v1, Topi requested to have some values start as floats to avoid later casts (and this was done). -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On 05/21/2015 05:05 PM, Rogovin, Kevin wrote: This line is too long. (It will not fit in 80 columns in git log since git log adds some spaces before each commit message line.) What is the accepted maximum length for a line in a commit message? Probably 75 chars since there's 4 spaces of indenting and we try to make things look nice for 80-column terminals and editors. - extension table - additions to gl_framebuffer v1 - v2 Spacing and trailing spaces fixes. This looks odd to me. I think you only need 'v2:' here. So, either I have seen a number of patches with the notation v1 - v2 to list changes between versions. Those patches that I saw using that notation did not have comments about using the format v1-v2. If people want v2: instead of v1-v2, I am fine with it, but was just following what I saw in some patch series. Given the number of nits around (that I seem to hit regularly), it might be beneficial for Mesa to have a short document listing the formatting requirements, of which I have so far collected: 1. 80 column limit in source files 2. Justify comments to 80 columns as well Or 78 columns, to be safe, but there's exceptions, like if you're defining a big static table/array of info. 3. comparison expressions require spaces around both sides of comparison operator Generally, a space on both sides of an operator like +, *, /, , =, etc. 4. successive parenthesis must have spaces between parenthesis Example? 5. git commit messages have limit of N characters per line (the value of N I am asking above). 75. 6. Use whether condition when describing a bool instead of true if condition is true not sure we care about that. 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line The 'indent' command in the docs should cover that. 10. no white spaces at end of line 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. Not sure about that, but that's not a coding style standard. 12. (Guessing here) consecutive empty lines are not allowed Generally true, except between functions. 13. If changing a line that violates the nit rules, fix the line too rather than just adding the change Yeah, probably. I suspect there are more as I listen to the nits, I think it might be beneficial to collect all the formatting nits and write them down to the coding standard thing in Mesa so that others can refer to it. Especially useful for those that work on multiple projects with different coding standards. Feel free to submit a patch against docs/devinfo.html with this info. :) -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 5:05 PM, Rogovin, Kevin kevin.rogo...@intel.com wrote: This line is too long. (It will not fit in 80 columns in git log since git log adds some spaces before each commit message line.) What is the accepted maximum length for a line in a commit message? Gentoo's default vim configuration line wraps the commit message at 72 (I think because 72 + an 8-space tab in git log fits in an 80 column window). It also changes the highlighting of the commit title itself after 50 characters. I think both of those are reasonable, although staying in 50 characters for the title is sometimes hard. - extension table - additions to gl_framebuffer v1 - v2 Spacing and trailing spaces fixes. This looks odd to me. I think you only need 'v2:' here. So, either I have seen a number of patches with the notation v1 - v2 to list changes between versions. Those patches that I saw using that notation did not have comments about using the format v1-v2. If people want v2: instead of v1-v2, I am fine with it, but was just following what I saw in some patch series. Given the number of nits around (that I seem to hit regularly), it might be beneficial for Mesa to have a short document listing the formatting requirements, of which I have so far collected: 1. 80 column limit in source files 2. Justify comments to 80 columns as well 3. comparison expressions require spaces around both sides of comparison operator 4. successive parenthesis must have spaces between parenthesis 5. git commit messages have limit of N characters per line (the value of N I am asking above). 6. Use whether condition when describing a bool instead of true if condition is true 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line 10. no white spaces at end of line 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. 12. (Guessing here) consecutive empty lines are not allowed 13. If changing a line that violates the nit rules, fix the line too rather than just adding the change I suspect there are more as I listen to the nits, I think it might be beneficial to collect all the formatting nits and write them down to the coding standard thing in Mesa so that others can refer to it. Especially useful for those that work on multiple projects with different coding standards. I suppose it could be useful, but I think we've been mostly successful at just expecting people to recognize when what they're writing doesn't look like the code around it. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On 05/21/2015 05:26 PM, Rogovin, Kevin wrote: HI, Or 78 columns, to be safe, but there's exceptions, like if you're defining a big static table/array of info. Uggg I don't mind exceptions, but knowing them is key. 4. successive parenthesis must have spaces between parenthesis Example? if (some_func(some_argument)) is bad, but if (some_func(some_argument) ) is good. I am also guessing that a = foo(bar(x)); is bad and must be changed to a = foo(bar(x) ); No, I don't think that's our intention. a = foo(bar(x)); would be my choice. Seems perfectly readable. I haven't been following this thread so perhaps I've missed someone else's suggestion. 6. Use whether condition when describing a bool instead of true if condition is true not sure we care about that. I hit a nit for it in the series, so someone cared. Well, we want our comments to be clear, concise and helpful and sometimes the best language is a personal preference. Hard to be specific there. 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line The 'indent' command in the docs should cover that. 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. Not sure about that, but that's not a coding style standard. Perhaps coding standard is not the right word, but it is a requirement (atleast it seems that way) and is a trivial requirement to satisfy. 12. (Guessing here) consecutive empty lines are not allowed Generally true, except between functions. Ugg... I hit a nit from an extra space between functions. 13. If changing a line that violates the nit rules, fix the line too rather than just adding the change Yeah, probably. Feel free to submit a patch against docs/devinfo.html with this info. :) I do not mind submitting the patch, but I want to know what the rules are; preferably explicitly written rather than inferred (by me). Especially since I seem to hit nits like no tomorrow even when trying not to :) I can understand your frustration. Going around and around with tiny changes isn't fun. But I think we're all interested in getting things to look right the first time, rather than having to clean it up later. Thanks for your patience and persistence. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
I suppose it could be useful, but I think we've been mostly successful at just expecting people to recognize when what they're writing doesn't look like the code around it. This is my point. Older code had different style/expectations than newer code. For this patch series, I have hit a number of nits and quasi-nits that are ambiguous: 0. In v1 to v2, avoid GL types atleast in i965 although much of the code uses that (the explanation given was to move away from GL types). Does this apply to Mesa core as well? 1. the casting bit in the i965 patch to use the _mesa_geomety_ functions. These are not quite nits, but in some ways not a big deal. I hit these because there were ints there before. In this regard doing what was there before was ungood (atleast for this review). 2. the line between function thing. In truth I just missed that extra line for the added to framebuffer.h (and I should not have), but there are places in the code that there are multiple empty lines between function definitions. I do not mind saying no extra empty lines, but not knowing the rules and attempting to infer them from the code, I seem to hit too many nits. 3. Even on the subject of git commit, I am seeing different answers: 75, but try 50 usually, but understandable if cannot do it. Utterly ambiguous. 4. on the subject of line length I have this so far: usually 78, but 80 sometimes is ok. Does ok, for example, include making a large-ish comment block more justified? 5. Consecutive empty lines is not ok, except in function definitions, but then only sometimes. I am guessing sometimes is for grouping function definitions, but plenty of files follow different conventions (hence what Brian Paul said). Given that nits just add traffic (and I want to minimize that silliness) I think it would be wise to set down some precise rules so there is no judgement calls required for something as silly as formatting nits. Ideally, we would have a lint script that would do it for us :) I see that reviews usually first hit nits, then content of patches. That is fine, but I'd rather know all the rules rather than hitting the nits at all. Again, I really have no preference since someone is paying me to do this, but knowing the exact rules would be more efficient. Inferring rules is quite error prone IMO (indeed, at one point, the GLSL pre-processor appeared to be written in a different style). -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 8:05 PM, Rogovin, Kevin kevin.rogo...@intel.com wrote: This line is too long. (It will not fit in 80 columns in git log since git log adds some spaces before each commit message line.) What is the accepted maximum length for a line in a commit message? - extension table - additions to gl_framebuffer v1 - v2 Spacing and trailing spaces fixes. This looks odd to me. I think you only need 'v2:' here. So, either I have seen a number of patches with the notation v1 - v2 to list changes between versions. Those patches that I saw using that notation did not have comments about using the format v1-v2. If people want v2: instead of v1-v2, I am fine with it, but was just following what I saw in some patch series. Given the number of nits around (that I seem to hit regularly), it might be beneficial for Mesa to have a short document listing the formatting requirements, of which I have so far collected: 1. 80 column limit in source files 2. Justify comments to 80 columns as well 3. comparison expressions require spaces around both sides of comparison operator 4. successive parenthesis must have spaces between parenthesis 5. git commit messages have limit of N characters per line (the value of N I am asking above). 6. Use whether condition when describing a bool instead of true if condition is true 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line 10. no white spaces at end of line 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. 12. (Guessing here) consecutive empty lines are not allowed 13. If changing a line that violates the nit rules, fix the line too rather than just adding the change I suspect there are more as I listen to the nits, I think it might be beneficial to collect all the formatting nits and write them down to the coding standard thing in Mesa so that others can refer to it. Especially useful for those that work on multiple projects with different coding standards. FWIW, the kernel standards for commit messages are at: https://www.kernel.org/doc/Documentation/SubmittingPatches Most of those rules apply to Mesa too. It says the body should be wrapped to 75 chars (although I've been using 72 like Matt said). -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 5:26 PM, Rogovin, Kevin kevin.rogo...@intel.com wrote: HI, Or 78 columns, to be safe, but there's exceptions, like if you're defining a big static table/array of info. Uggg I don't mind exceptions, but knowing them is key. 4. successive parenthesis must have spaces between parenthesis Example? if (some_func(some_argument)) is bad, but if (some_func(some_argument) ) is good. I am also guessing that a = foo(bar(x)); is bad and must be changed to a = foo(bar(x) ); That's not a style I've ever seen. Some old Mesa code always puts spaces around the condition in an if statement, like if ( cond ), but there's nothing about just an extra space at the end. 6. Use whether condition when describing a bool instead of true if condition is true not sure we care about that. I hit a nit for it in the series, so someone cared. 7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution) 8. indenting is 3 spaces, except after switch where it is 0 (but after case it is 3). 9. open bracket on same line The 'indent' command in the docs should cover that. 11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called. Not sure about that, but that's not a coding style standard. Perhaps coding standard is not the right word, but it is a requirement (atleast it seems that way) and is a trivial requirement to satisfy. 12. (Guessing here) consecutive empty lines are not allowed Generally true, except between functions. Ugg... I hit a nit from an extra space between functions. You hit a nit because you were inconsistent. Look at your patch. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
FWIW, the kernel standards for commit messages are at: https://www.kernel.org/doc/Documentation/SubmittingPatches Most of those rules apply to Mesa too. It says the body should be wrapped to 75 chars (although I've been using 72 like Matt said). This is my point: use most rules, but not all.. and I've been more conservative than X but I did not need to be. What I am seeing is that there is, in some collective form, a set of consistent rules (in the form of ranges) that are strongly enforced and yet not written down. Let's write them all down here and now, put them in some file for others to read and to refer to. Maybe even con someone to write a lint-like script for those rules. -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: do not use _glapi_new_nop_table() for DRI builds
On 05/15/2015 02:28 PM, Emil Velikov wrote:
On 15/05/15 17:58, Brian Paul wrote:
On 05/15/2015 11:14 AM, Emil Velikov wrote:
On 15/05/15 15:13, Brian Paul wrote:
Commit 4bdbb588a9d38 introduced new _glapi_new_nop_table() and
_glapi_set_nop_handler() functions in the glapi dispatcher (which
live in libGL.so). The calls to those functions from context.c
would be undefined (i.e. an ABI break) if the libGL used at runtime
was older.
For the time being, use the old single generic_nop() function for
DRI builds to avoid this problem. At some point in the future it
should be safe to remove this work-around. See comments for more
details.
---
src/mesa/main/context.c | 60
-
1 file changed, 59 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 544cc14..0649708 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -883,6 +883,19 @@ update_default_objects(struct gl_context *ctx)
}
+/* XXX this is temporary and should be removed at some point in the
+ * future when there's a reasonable expectation that the libGL library
+ * contains the _glapi_new_nop_table() and _glapi_set_nop_handler()
+ * functions which were added in Mesa 10.6.
+ */
+#if defined(GLX_DIRECT_RENDERING)
I'd say that this should be WIN32, similar to the original code. The
code is indirectly used by gbm/egl as well, so introducing
GLX_{IN,}DIRECT_RENDERING here might not fair so well.
Sure, that's fine.
+/* Avoid libGL / driver ABI break */
+#define USE_GLAPI_NOP_FEATURES 0
+#else
+#define USE_GLAPI_NOP_FEATURES 1
+#endif
+
+
/**
* This function is called by the glapi no-op functions. For each
OpenGL
* function/entrypoint there's a simple no-op function. These no-op
@@ -898,6 +911,7 @@ update_default_objects(struct gl_context *ctx)
*
* \param name the name of the OpenGL function
*/
+#if USE_GLAPI_NOP_FEATURES
static void
nop_handler(const char *name)
{
@@ -914,6 +928,7 @@ nop_handler(const char *name)
}
#endif
}
+#endif
/**
@@ -928,6 +943,44 @@ nop_glFlush(void)
Seems like the macro guarding nop_glFlush() should be
USE_GLAPI_NOP_FEATURES ? Then we can drop the explicit
!USE_GLAPI_NOP_FEATURES below and use #else.
No, nop_glFlush() is specifically a Windows / WGL fix. See comments
elsewhere.
I was definitely to stingy on my explanation here, let me elaborate: The
original code had two implementations guarded by the _WIN32 macro. As
we bring back a similar distinction, it will be great to consistently
use the new macro USE_GLAPI_NOP_FEATURES.
That made me think that it'll be cleaner if the whole thing is wrapped
as follows
#if USE_GLAPI_NOP_FEATURES
nop_handler()
nop_glFlush()
#else
generic_nop()
new_nop_table()
#endif
Although as you mentioned below you prefer having separate guards. It
was a slightly bike-shed like suggestion :)
The comment within nop_glFlush could use an update as well.
Will do.
#endif
+#if !USE_GLAPI_NOP_FEATURES
Fold this and the follow up function new_nop_table() into #if block ?
My preference is wrap each individual function for readability.
+static int
+generic_nop(void)
+{
+ GET_CURRENT_CONTEXT(ctx);
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ unsupported function called
+ (unsupported extension or deprecated function?));
+ return 0;
+}
+#endif
+
+
+/**
+ * Create a new API dispatch table in which all entries point to the
+ * generic_nop() function. This will not work on Windows because of
+ * the __stdcall convention which requires the callee to clean up the
+ * call stack. That's impossible with one generic no-op function.
+ */
+#if !USE_GLAPI_NOP_FEATURES
+static struct _glapi_table *
+new_nop_table(unsigned numEntries)
+{
+ struct _glapi_table *table;
+
+ table = malloc(numEntries * sizeof(_glapi_proc));
+ if (table) {
+ _glapi_proc *entry = (_glapi_proc *) table;
+ unsigned i;
+ for (i = 0; i numEntries; i++) {
+ entry[i] = (_glapi_proc) generic_nop;
+ }
Bikeshed: One could use memset, analogous to the memcpy() in
_glapi_new_nop_table.
How would memset work? I'm assigning 4 or 8-byte pointers.
Brain fart. Please ignore.
+ }
+ return table;
+}
+#endif
+
+
/**
* Allocate and initialize a new dispatch table. The table will be
* populated with pointers to no-op functions. In turn, the no-op
@@ -936,12 +989,16 @@ nop_glFlush(void)
static struct _glapi_table *
alloc_dispatch_table(void)
{
+ int numEntries = MAX2(_glapi_get_dispatch_table_size(),
_gloffset_COUNT);
+
+#if !USE_GLAPI_NOP_FEATURES
+ struct _glapi_table *table = new_nop_table(numEntries);
+#else
/* Find the larger of Mesa's dispatch table and libGL's dispatch
table.
* In practice, this'll be the same for stand-alone Mesa. But
for DRI
* Mesa we do this to accommodate different versions of libGL
and various
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 8:40 PM, Rogovin, Kevin kevin.rogo...@intel.com wrote: I suppose it could be useful, but I think we've been mostly successful at just expecting people to recognize when what they're writing doesn't look like the code around it. This is my point. Older code had different style/expectations than newer code. For this patch series, I have hit a number of nits and quasi-nits that are ambiguous: 0. In v1 to v2, avoid GL types atleast in i965 although much of the code uses that (the explanation given was to move away from GL types). Does this apply to Mesa core as well? 1. the casting bit in the i965 patch to use the _mesa_geomety_ functions. These are not quite nits, but in some ways not a big deal. I hit these because there were ints there before. In this regard doing what was there before was ungood (atleast for this review). 2. the line between function thing. In truth I just missed that extra line for the added to framebuffer.h (and I should not have), but there are places in the code that there are multiple empty lines between function definitions. I do not mind saying no extra empty lines, but not knowing the rules and attempting to infer them from the code, I seem to hit too many nits. 3. Even on the subject of git commit, I am seeing different answers: 75, but try 50 usually, but understandable if cannot do it. Utterly ambiguous. No, that's because you misread it. The rules for commit messages (and usually, for everything else as well) have a purpose, and that's why they might seem ambiguous to you: we're adults here, so we don't need to have a huge debate on 75 vs. 72 chars for commit message bodies, because we know that both satisfy the *purpose* of being able to do 'git log' on an 80-char terminal without ugly line-wrapping due to 'git log' indenting your message an extra 4 spaces. Sometimes we get too wrapped up in sweating the details, but in the end it's just about consistency and making things easier for people. You're also missing the difference between the subject line, which is the thing people will skim over when looking for something and therefore should be as short as possible (ideally under 50 chars), and the body (the part after the empty line), which is what people already looking at your commit will read to fully understand what it's changing. The body can be as long as it needs to be, as long as it's concise and to the point, and it should be wrapped to at most 75 chars. 4. on the subject of line length I have this so far: usually 78, but 80 sometimes is ok. Does ok, for example, include making a large-ish comment block more justified? 5. Consecutive empty lines is not ok, except in function definitions, but then only sometimes. I am guessing sometimes is for grouping function definitions, but plenty of files follow different conventions (hence what Brian Paul said). Given that nits just add traffic (and I want to minimize that silliness) I think it would be wise to set down some precise rules so there is no judgement calls required for something as silly as formatting nits. Ideally, we would have a lint script that would do it for us :) I see that reviews usually first hit nits, then content of patches. That is fine, but I'd rather know all the rules rather than hitting the nits at all. Again, I really have no preference since someone is paying me to do this, but knowing the exact rules would be more efficient. Inferring rules is quite error prone IMO (indeed, at one point, the GLSL pre-processor appeared to be written in a different style). -Kevin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments
On Thu, May 21, 2015 at 8:51 PM, Rogovin, Kevin kevin.rogo...@intel.com wrote: FWIW, the kernel standards for commit messages are at: https://www.kernel.org/doc/Documentation/SubmittingPatches Most of those rules apply to Mesa too. It says the body should be wrapped to 75 chars (although I've been using 72 like Matt said). This is my point: use most rules, but not all.. and I've been more conservative than X but I did not need to be. What I am seeing is that there is, in some collective form, a set of consistent rules (in the form of ranges) that are strongly enforced and yet not written down. Let's write them all down here and now, put them in some file for others to read and to refer to. Maybe even con someone to write a lint-like script for those rules. -Kevin I said use most rules, but all because I'm expecting you to use your common sense to see what rules apply and which might need to be modified to apply to Mesa. For example, as you've probably noticed, Mesa doesn't have any formal maintainers like Linux does, so the process for getting your patch into the tree goes from get a sign-off from the maintainer and they'll merge it into their tree to get a reviewed-by from someone well-known and familiar with the code you're modifying, and they'll commit it if you don't have commit access. Usually figuring out who that is pretty straightforward (hint: git blame), but if not you can ask. We don't have a style guide or the equivalent of checkpatch.pl for the same reason we don't have formal maintainers: the project isn't large enough, and doesn't have enough infrastructure, for it. There haven't been enough people like you that have to be hand-fed every detail to justify the work; instead, we just note problems when they occur and rely on the patch author to fix them. If there's anyone to be conned to do that work, it's going to be you. You've certainly been given enough information by now to be able to do so. Finally, I'll quote a section of the file I linked to: Be sure to tell the reviewers what changes you are making and to thank them for their time. Code review is a tiring and time-consuming process, and reviewers sometimes get grumpy. Even in that case, though, respond politely and address the problems they have pointed out. Everyone makes mistakes. Heck, I just sent out a series with some minor style nits that Matt pointed out. When that happens, you fix the problem, try to remember it for the future, and then move on. Starting a 13 (now 14) email thread where you do nothing but complain about it is a great way to not get your patches merged, and that would be a good thing to remember in case you actually care about getting them merged; but if you don't, then why did you send them and waste our time in the first place? Connor ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 01/14] nir/vars_to_ssa: don't rewrite removed instructions
Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com On May 21, 2015 9:41 AM, Connor Abbott cwabbo...@gmail.com wrote: We were rewriting the uses of the intrinsic instruction to point to something else after removing it, which only happened to work since we were lax about having dangling uses when removing instructions. Fix that up. Signed-off-by: Connor Abbott cwabbo...@gmail.com --- src/glsl/nir/nir_lower_vars_to_ssa.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/glsl/nir/nir_lower_vars_to_ssa.c b/src/glsl/nir/nir_lower_vars_to_ssa.c index ccb8f99..8d0ae1b 100644 --- a/src/glsl/nir/nir_lower_vars_to_ssa.c +++ b/src/glsl/nir/nir_lower_vars_to_ssa.c @@ -647,11 +647,12 @@ rename_variables_block(nir_block *block, struct lower_variables_state *state) intrin-num_components, NULL); nir_instr_insert_before(intrin-instr, mov-instr); -nir_instr_remove(intrin-instr); nir_ssa_def_rewrite_uses(intrin-dest.ssa, nir_src_for_ssa(mov-dest.dest.ssa), state-shader); + +nir_instr_remove(intrin-instr); break; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir/remove_phis: handle trivial back-edges
This looks totally sane to me.
Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com
I'll try and get some shader-db numbers for you tomorrow.
--Jason
On May 21, 2015 5:07 PM, Connor Abbott cwabbo...@gmail.com wrote:
Some loops may have phi nodes that look like:
foo = ...
loop {
bar = phi(foo, bar)
...
}
in which case we can remove the phi node and replace all uses of 'bar'
with 'foo'. In particular, there are some L4D2 vertex shaders with loops
that, after optimization, look like:
/* succs: block_1 */
loop {
block block_1:
/* preds: block_0 block_4 */
vec1 ssa_2195 = phi block_0: ssa_2136, block_4: ssa_994
vec1 ssa_7321 = phi block_0: ssa_8195, block_4: ssa_7321
vec1 ssa_7324 = phi block_0: ssa_8198, block_4: ssa_7324
vec1 ssa_7327 = phi block_0: ssa_8174, block_4: ssa_7327
vec1 ssa_8139 = intrinsic load_uniform () () (232)
vec1 ssa_588 = ige ssa_2195, ssa_8139
/* succs: block_2 block_3 */
if ssa_588 {
block block_2:
/* preds: block_1 */
break
/* succs: block_5 */
} else {
block block_3:
/* preds: block_1 */
/* succs: block_4 */
}
block block_4:
/* preds: block_3 */
vec1 ssa_994 = iadd ssa_2195, ssa_2150
/* succs: block_1 */
}
where after removing the second, third, and fourth phi nodes, the loop
becomes entirely dead, and this patch combined with my nir-dead-cf-v4
branch will cause the loop to be deleted entirely.
No piglit regressions.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir_opt_remove_phis.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/glsl/nir/nir_opt_remove_phis.c
b/src/glsl/nir/nir_opt_remove_phis.c
index 7896584..3660413 100644
--- a/src/glsl/nir/nir_opt_remove_phis.c
+++ b/src/glsl/nir/nir_opt_remove_phis.c
@@ -60,6 +60,21 @@ remove_phis_block(nir_block *block, void *state)
nir_foreach_phi_src(phi, src) {
assert(src-src.is_ssa);
+
+ /* For phi nodes at the beginning of loops, we may encounter
some
+ * sources from backedges that point back to the destination of
the
+ * same phi, i.e. something like:
+ *
+ * a = phi(a, b, ...)
+ *
+ * We can safely ignore these sources, since if all of the
normal
+ * sources point to the same definition, then that definition
must
+ * still dominate the phi node, and the phi will still always
take
+ * the value of that definition.
+ */
+
+ if (src-src.ssa == phi-dest.ssa)
+continue;
if (def == NULL) {
def = src-src.ssa;
@@ -74,6 +89,11 @@ remove_phis_block(nir_block *block, void *state)
if (!srcs_same)
continue;
+ /* We must have found at least one definition, since there must be
at
+ * least one forward edge.
+ */
+ assert(def != NULL);
+
assert(phi-dest.is_ssa);
nir_ssa_def_rewrite_uses(phi-dest.ssa, nir_src_for_ssa(def),
mem_ctx);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir/remove_phis: handle trivial back-edges
On Thu, May 21, 2015 at 5:07 PM, Connor Abbott cwabbo...@gmail.com wrote:
Some loops may have phi nodes that look like:
foo = ...
loop {
bar = phi(foo, bar)
...
}
in which case we can remove the phi node and replace all uses of 'bar'
with 'foo'. In particular, there are some L4D2 vertex shaders with loops
that, after optimization, look like:
/* succs: block_1 */
loop {
block block_1:
/* preds: block_0 block_4 */
vec1 ssa_2195 = phi block_0: ssa_2136, block_4: ssa_994
vec1 ssa_7321 = phi block_0: ssa_8195, block_4: ssa_7321
vec1 ssa_7324 = phi block_0: ssa_8198, block_4: ssa_7324
vec1 ssa_7327 = phi block_0: ssa_8174, block_4: ssa_7327
vec1 ssa_8139 = intrinsic load_uniform () () (232)
vec1 ssa_588 = ige ssa_2195, ssa_8139
/* succs: block_2 block_3 */
if ssa_588 {
block block_2:
/* preds: block_1 */
break
/* succs: block_5 */
} else {
block block_3:
/* preds: block_1 */
/* succs: block_4 */
}
block block_4:
/* preds: block_3 */
vec1 ssa_994 = iadd ssa_2195, ssa_2150
/* succs: block_1 */
}
where after removing the second, third, and fourth phi nodes, the loop
becomes entirely dead, and this patch combined with my nir-dead-cf-v4
branch will cause the loop to be deleted entirely.
No piglit regressions.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
On top of the first 13-patches (on BDW):
instructions in affected programs: 5824 - 5664 (-2.75%)
helped:32
(to save Jason from rerunning it tomorrow)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 02/14] nir: insert ssa_undef instructions when cleaning up defs/uses
On May 21, 2015 9:41 AM, Connor Abbott cwabbo...@gmail.com wrote:
The point of cleanup_defs_uses() is to make an instruction safe to
remove by removing any references that the rest of the shader may have
to it. Previously, it was handling register use/def sets and removing
the instruction from the use sets of any SSA sources it had, but if the
instruction defined an SSA value that was used by other instructions it
wasn't doing anything. This was ok, since we were always careful to make
sure that no removed instruction ever had any uses, but now we want to
start removing unreachable instruction which might still be used in
unreachable instructions plural
reachable parts of the code. In that case, the value that any uses get
is undefined since the instruction never actually executes, so we can
just replace the instruction with an ssa_undef_instr.
v2: split out other fixes
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 38 --
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index f03e80a..704553f 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1206,26 +1206,26 @@ stitch_blocks(nir_block *before, nir_block *after)
}
static void
-remove_defs_uses(nir_instr *instr);
+remove_defs_uses(nir_instr *instr, nir_function_impl *impl);
static void
-cleanup_cf_node(nir_cf_node *node)
+cleanup_cf_node(nir_cf_node *node, nir_function_impl *impl)
{
switch (node-type) {
case nir_cf_node_block: {
nir_block *block = nir_cf_node_as_block(node);
/* We need to walk the instructions and clean up defs/uses */
nir_foreach_instr(block, instr)
- remove_defs_uses(instr);
+ remove_defs_uses(instr, impl);
break;
}
case nir_cf_node_if: {
nir_if *if_stmt = nir_cf_node_as_if(node);
foreach_list_typed(nir_cf_node, child, node, if_stmt-then_list)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
foreach_list_typed(nir_cf_node, child, node, if_stmt-else_list)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
list_del(if_stmt-condition.use_link);
break;
@@ -1234,13 +1234,12 @@ cleanup_cf_node(nir_cf_node *node)
case nir_cf_node_loop: {
nir_loop *loop = nir_cf_node_as_loop(node);
foreach_list_typed(nir_cf_node, child, node, loop-body)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
break;
}
case nir_cf_node_function: {
- nir_function_impl *impl = nir_cf_node_as_function(node);
foreach_list_typed(nir_cf_node, child, node, impl-body)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
break;
}
default:
@@ -1443,16 +1442,35 @@ remove_def_cb(nir_dest *dest, void *state)
return true;
}
+static bool
+remove_ssa_def_cb(nir_ssa_def *def, void *state)
+{
+ nir_function_impl *impl = state;
+ nir_shader *shader = impl-overload-function-shader;
+
+ if (!list_empty(def-uses) || !list_empty(def-if_uses)) {
+ nir_ssa_undef_instr *undef =
+ nir_ssa_undef_instr_create(shader, def-num_components);
+ nir_instr_insert_before_cf_list(impl-body, undef-instr);
+ nir_ssa_def_rewrite_uses(def, nir_src_for_ssa(undef-def),
shader);
+ }
+
+ return true;
+}
+
+
static void
-remove_defs_uses(nir_instr *instr)
+remove_defs_uses(nir_instr *instr, nir_function_impl *impl)
{
nir_foreach_dest(instr, remove_def_cb, instr);
nir_foreach_src(instr, remove_use_cb, instr);
+ nir_foreach_ssa_def(instr, remove_ssa_def_cb, impl);
}
void nir_instr_remove(nir_instr *instr)
{
- remove_defs_uses(instr);
+ nir_function_impl *impl =
nir_cf_node_get_function(instr-block-cf_node);
I'm not entirely happy that this is done unconditionally here given that it
may be somewhat expensive and most of the time isn't needed. However, I'm
not going to quibble over it too bad.
Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com
+ remove_defs_uses(instr, impl);
exec_node_remove(instr-node);
if (instr-type == nir_instr_type_jump) {
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] nv50: fix 64 bits queries, based on nvc0
On 05/21/2015 09:33 PM, Ilia Mirkin wrote:
So... this doesn't fix a known issue? Just because nvc0 does
something, doesn't make it right. nvc0 has a ton of internal mp/pm
stats too, which are 64-bit, and provide other differences.
Sure, but in this case, it seems to fix the issue...
As an aside, nv50_query_end for PIPE_QUERY_TIMESTAMP_DISJOINT *is*
busted... it sets q-ready = TRUE and then immediately sets it to
false :(
Yep :/
On Thu, May 21, 2015 at 3:28 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
According to nvc0, 64-bits queries use a fence to make sure
the result is available.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index 6690aa2..a3c8841 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -46,6 +46,7 @@ struct nv50_query {
boolean flushed;
boolean is64bit;
struct nouveau_mm_allocation *mm;
+ struct nouveau_fence *fence;
};
#define NV50_QUERY_ALLOC_SPACE 256
@@ -92,6 +93,7 @@ static void
nv50_query_destroy(struct pipe_context *pipe, struct pipe_query *pq)
{
nv50_query_allocate(nv50_context(pipe), nv50_query(pq), 0);
+ nouveau_fence_ref(NULL, nv50_query(pq)-fence);
FREE(nv50_query(pq));
}
@@ -260,12 +262,22 @@ nv50_query_end(struct pipe_context *pipe, struct
pipe_query *pq)
break;
}
q-ready = q-flushed = FALSE;
+
+ if (q-is64bit)
+ nouveau_fence_ref(nv50-screen-base.fence.current, q-fence);
}
static INLINE boolean
nv50_query_ready(struct nv50_query *q)
{
- return q-ready || (!q-is64bit (q-data[0] == q-sequence));
+ if (q-is64bit) {
+ if (nouveau_fence_signalled(q-fence))
+ return TRUE;
+ } else {
+ if (q-data[0] == q-sequence)
+ return TRUE;
+ }
+ return FALSE;
}
static boolean
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 17/22] i965/gen9: Add XY_FAST_COPY_BLT support to intelEmitCopyBlit()
A lot of opinion stuff is below, feel free to ignore them if you don't think
there are improvements.
On Fri, Apr 17, 2015 at 04:51:38PM -0700, Anuj Phogat wrote:
This patch enables using XY_FAST_COPY_BLT only for Yf/Ys tiled buffers.
Later It can be turned on for other tiling patterns (X,Y).
Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
src/mesa/drivers/dri/i965/intel_blit.c | 292
+++
src/mesa/drivers/dri/i965/intel_blit.h | 3 +
src/mesa/drivers/dri/i965/intel_copy_image.c | 3 +
src/mesa/drivers/dri/i965/intel_reg.h| 33 +++
4 files changed, 291 insertions(+), 40 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 9500bd7..36746c4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -43,6 +43,23 @@
#define FILE_DEBUG_FLAG DEBUG_BLIT
+#define SET_TILING_XY_FAST_COPY_BLT(tiling, tr_mode, type) \
+({ \
+ switch (tiling) { \
+ case I915_TILING_X: \
+ CMD |= type ## _TILED_X; \
+ break; \
+ case I915_TILING_Y: \
assert(tr_mode != INTEL_MIPTREE_TRMODE_YF); ?
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YS)\
+ CMD |= type ## _TILED_64K; \
+ else \
+ CMD |= type ## _TILED_Y;\
+ break; \
+ default: \
+ unreachable(not reached);\
+ } \
+})
+
static void
intel_miptree_set_alpha_to_one(struct brw_context *brw,
struct intel_mipmap_tree *mt,
@@ -75,6 +92,12 @@ static uint32_t
br13_for_cpp(int cpp)
{
switch (cpp) {
+ case 16:
+ return BR13_32323232;
+ break;
+ case 8:
+ return BR13_16161616;
+ break;
case 4:
return BR13_;
break;
@@ -89,6 +112,132 @@ br13_for_cpp(int cpp)
}
}
+static uint32_t
+get_tr_horizontal_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
+ /*Alignment tables for YF/YS tiled surfaces. */
+ const uint32_t align_2d_yf[] = {64, 64, 32, 32, 16};
+ const uint32_t align_2d_ys[] = {256, 256, 128, 128, 64};
+ const uint32_t bpp = cpp * 8;
+ uint32_t align;
+ int i = 0;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
+ return 0;
+
+ /* Compute array index. */
+ assert (bpp = 8 bpp = 128 (bpp (bpp - 1)) == 0);
assert(bpp = 8 bpp = 128 _mesa_bitcount(bpp) == 1);
(I couldn't find a is_pow2, but one must exist).
+ while (bpp (i + 4))
+ i++;
+
Since you just asserted this was a power of 2 above, isn't this just:
ffs(bpp/8) - 1;
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YF)
+ align = align_2d_yf[i];
+ else
+ align = align_2d_ys[i];
+
+ switch(align) {
+ case 512:
+ return is_src ? XY_SRC_H_ALIGN_512 : XY_DST_H_ALIGN_512;
+ case 256:
+ return is_src ? XY_SRC_H_ALIGN_256 : XY_DST_H_ALIGN_256;
+ case 128:
+ return is_src ? XY_SRC_H_ALIGN_128 : XY_DST_H_ALIGN_128;
+ case 64:
+ return is_src ? XY_SRC_H_ALIGN_64 : XY_DST_H_ALIGN_64;
+ case 32:
+ /* XY_FAST_COPY_BLT doesn't support horizontal alignment of 16. */
+ case 16:
+ return is_src ? XY_SRC_H_ALIGN_32 : XY_DST_H_ALIGN_32;
+ default:
+ unreachable(not reached);
+ }
+}
+
+static uint32_t
+get_tr_vertical_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
+ /* Vertical alignment tables for YF/YS tiled surfaces. */
+ const unsigned align_2d_yf[] = {64, 32, 32, 16, 16};
+ const unsigned align_2d_ys[] = {256, 128, 128, 64, 64};
+ const uint32_t bpp = cpp * 8;
+ uint32_t align;
+ int i = 0;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
+ return 0;
+
+ /* Compute array index. */
+ assert (bpp = 8 bpp = 128 (bpp (bpp - 1)) == 0);
+ while (bpp (i + 4))
+ i++;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YF)
+ align = align_2d_yf[i];
+ else
+ align = align_2d_ys[i];
+
Comments above apply here too. Also, it looks easy enough to combine these two
functions, but I'm okay with leaving them separate if^Wwhen things change in the
future.
+ switch(align) {
+ case 256:
+ return is_src ? XY_SRC_V_ALIGN_256 : XY_DST_V_ALIGN_256;
+ case 128:
+ return is_src ? XY_SRC_V_ALIGN_128 : XY_DST_V_ALIGN_128;
+ case 64:
+ /*
Re: [Mesa-dev] [PATCH 2/2] nv50: fix pipeline statistics queries, based on nvc0
Yeah, ARB_pipeline_statistics. HUD is a different use-case, hmmm...
perhaps it's forgetting to do something?
On Thu, May 21, 2015 at 3:43 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
On 05/21/2015 09:34 PM, Ilia Mirkin wrote:
Do the piglits currently fail? But this does seem right, although the
distinction between 64-bit and 32-bit queries is a *bit* unclear to me
(wrt the diff approaches the code takes).
I tested with the HUD and I compared results between NVA8 and NVD9.
Do we have piglit tests for those queries? I'm not really sure.
On Thu, May 21, 2015 at 3:28 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
These queries use 64 bits. Tested on NVA8.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index a3c8841..da41209 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -114,7 +114,8 @@ nv50_query_create(struct pipe_context *pipe, unsigned
type, unsigned index)
q-is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
type == PIPE_QUERY_PRIMITIVES_EMITTED ||
- type == PIPE_QUERY_SO_STATISTICS);
+ type == PIPE_QUERY_SO_STATISTICS ||
+ type == PIPE_QUERY_PIPELINE_STATISTICS);
q-type = type;
if (q-type == PIPE_QUERY_OCCLUSION_COUNTER) {
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nv50: fix pipeline statistics queries, based on nvc0
On 05/21/2015 09:34 PM, Ilia Mirkin wrote:
Do the piglits currently fail? But this does seem right, although the
distinction between 64-bit and 32-bit queries is a *bit* unclear to me
(wrt the diff approaches the code takes).
I tested with the HUD and I compared results between NVA8 and NVD9.
Do we have piglit tests for those queries? I'm not really sure.
On Thu, May 21, 2015 at 3:28 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
These queries use 64 bits. Tested on NVA8.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index a3c8841..da41209 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -114,7 +114,8 @@ nv50_query_create(struct pipe_context *pipe, unsigned type,
unsigned index)
q-is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
type == PIPE_QUERY_PRIMITIVES_EMITTED ||
- type == PIPE_QUERY_SO_STATISTICS);
+ type == PIPE_QUERY_SO_STATISTICS ||
+ type == PIPE_QUERY_PIPELINE_STATISTICS);
q-type = type;
if (q-type == PIPE_QUERY_OCCLUSION_COUNTER) {
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] nv50: fix 64 bits queries, based on nvc0
According to nvc0, 64-bits queries use a fence to make sure
the result is available.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index 6690aa2..a3c8841 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -46,6 +46,7 @@ struct nv50_query {
boolean flushed;
boolean is64bit;
struct nouveau_mm_allocation *mm;
+ struct nouveau_fence *fence;
};
#define NV50_QUERY_ALLOC_SPACE 256
@@ -92,6 +93,7 @@ static void
nv50_query_destroy(struct pipe_context *pipe, struct pipe_query *pq)
{
nv50_query_allocate(nv50_context(pipe), nv50_query(pq), 0);
+ nouveau_fence_ref(NULL, nv50_query(pq)-fence);
FREE(nv50_query(pq));
}
@@ -260,12 +262,22 @@ nv50_query_end(struct pipe_context *pipe, struct
pipe_query *pq)
break;
}
q-ready = q-flushed = FALSE;
+
+ if (q-is64bit)
+ nouveau_fence_ref(nv50-screen-base.fence.current, q-fence);
}
static INLINE boolean
nv50_query_ready(struct nv50_query *q)
{
- return q-ready || (!q-is64bit (q-data[0] == q-sequence));
+ if (q-is64bit) {
+ if (nouveau_fence_signalled(q-fence))
+ return TRUE;
+ } else {
+ if (q-data[0] == q-sequence)
+ return TRUE;
+ }
+ return FALSE;
}
static boolean
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] nv50: fix pipeline statistics queries, based on nvc0
These queries use 64 bits. Tested on NVA8.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index a3c8841..da41209 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -114,7 +114,8 @@ nv50_query_create(struct pipe_context *pipe, unsigned type,
unsigned index)
q-is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
type == PIPE_QUERY_PRIMITIVES_EMITTED ||
- type == PIPE_QUERY_SO_STATISTICS);
+ type == PIPE_QUERY_SO_STATISTICS ||
+ type == PIPE_QUERY_PIPELINE_STATISTICS);
q-type = type;
if (q-type == PIPE_QUERY_OCCLUSION_COUNTER) {
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] nv50: fix 64 bits queries, based on nvc0
So... this doesn't fix a known issue? Just because nvc0 does
something, doesn't make it right. nvc0 has a ton of internal mp/pm
stats too, which are 64-bit, and provide other differences.
As an aside, nv50_query_end for PIPE_QUERY_TIMESTAMP_DISJOINT *is*
busted... it sets q-ready = TRUE and then immediately sets it to
false :(
On Thu, May 21, 2015 at 3:28 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
According to nvc0, 64-bits queries use a fence to make sure
the result is available.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index 6690aa2..a3c8841 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -46,6 +46,7 @@ struct nv50_query {
boolean flushed;
boolean is64bit;
struct nouveau_mm_allocation *mm;
+ struct nouveau_fence *fence;
};
#define NV50_QUERY_ALLOC_SPACE 256
@@ -92,6 +93,7 @@ static void
nv50_query_destroy(struct pipe_context *pipe, struct pipe_query *pq)
{
nv50_query_allocate(nv50_context(pipe), nv50_query(pq), 0);
+ nouveau_fence_ref(NULL, nv50_query(pq)-fence);
FREE(nv50_query(pq));
}
@@ -260,12 +262,22 @@ nv50_query_end(struct pipe_context *pipe, struct
pipe_query *pq)
break;
}
q-ready = q-flushed = FALSE;
+
+ if (q-is64bit)
+ nouveau_fence_ref(nv50-screen-base.fence.current, q-fence);
}
static INLINE boolean
nv50_query_ready(struct nv50_query *q)
{
- return q-ready || (!q-is64bit (q-data[0] == q-sequence));
+ if (q-is64bit) {
+ if (nouveau_fence_signalled(q-fence))
+ return TRUE;
+ } else {
+ if (q-data[0] == q-sequence)
+ return TRUE;
+ }
+ return FALSE;
}
static boolean
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nv50: fix pipeline statistics queries, based on nvc0
Do the piglits currently fail? But this does seem right, although the
distinction between 64-bit and 32-bit queries is a *bit* unclear to me
(wrt the diff approaches the code takes).
On Thu, May 21, 2015 at 3:28 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
These queries use 64 bits. Tested on NVA8.
Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
src/gallium/drivers/nouveau/nv50/nv50_query.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_query.c
b/src/gallium/drivers/nouveau/nv50/nv50_query.c
index a3c8841..da41209 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_query.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_query.c
@@ -114,7 +114,8 @@ nv50_query_create(struct pipe_context *pipe, unsigned
type, unsigned index)
q-is64bit = (type == PIPE_QUERY_PRIMITIVES_GENERATED ||
type == PIPE_QUERY_PRIMITIVES_EMITTED ||
- type == PIPE_QUERY_SO_STATISTICS);
+ type == PIPE_QUERY_SO_STATISTICS ||
+ type == PIPE_QUERY_PIPELINE_STATISTICS);
q-type = type;
if (q-type == PIPE_QUERY_OCCLUSION_COUNTER) {
--
2.4.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/skl: Add a message header for the TXF_MCS instruction in vec4vs
When using SIMD4x2 on Skylake, the sampler instructions need a message
header to select the correct mode. This was added for most sample
instructions in 0ac4c2727 but the TXF_MCS instruction is emitted
separately and it was missed.
This fixes a bunch of Piglit tests which test texelFetch in a geometry
shader, for example:
spec/arb_texture_multisample/texelfetch/2-gs-sampler2dms
---
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 5a60fe4..59a73a9 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -2460,11 +2460,27 @@ vec4_visitor::emit_mcs_fetch(ir_texture *ir, src_reg
coordinate, src_reg sampler
new(mem_ctx) vec4_instruction(SHADER_OPCODE_TXF_MCS,
dst_reg(this, glsl_type::uvec4_type));
inst-base_mrf = 2;
- inst-mlen = 1;
inst-src[1] = sampler;
+ int param_base;
+
+ if (devinfo-gen = 9) {
+ /* Gen9+ needs a message header in order to use SIMD4x2 mode */
+ vec4_instruction *header_inst = new(mem_ctx)
+ vec4_instruction(VS_OPCODE_SET_SIMD4X2_HEADER_GEN9,
+ dst_reg(MRF, inst-base_mrf));
+
+ emit(header_inst);
+
+ inst-mlen = 2;
+ inst-header_size = 1;
+ param_base = inst-base_mrf + 1;
+ } else {
+ inst-mlen = 1;
+ param_base = inst-base_mrf;
+ }
+
/* parameters are: u, v, r, lod; lod will always be zero due to api
restrictions */
- int param_base = inst-base_mrf;
int coord_mask = (1 ir-coordinate-type-vector_elements) - 1;
int zero_mask = 0xf ~coord_mask;
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 04/14] nir: properly clean up jumps when removing cf nodes
On Thu, May 21, 2015 at 9:40 AM, Connor Abbott cwabbo...@gmail.com wrote:
Before, we might have left dangling predecessors from jumps that were
going to be removed.
v2: split out from nir: insert ssa_undef instructions when cleaning up
defs/uses
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index a2b5e7c..dc6d63f 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1214,9 +1214,15 @@ cleanup_cf_node(nir_cf_node *node, nir_function_impl
*impl)
switch (node-type) {
case nir_cf_node_block: {
nir_block *block = nir_cf_node_as_block(node);
- /* We need to walk the instructions and clean up defs/uses */
- nir_foreach_instr(block, instr)
+ /* We need to walk the instructions and clean up defs/uses,
+ * as well as clean up any jumps to control flow that may not be
getting
+ * deleted.
I think you can line wrap this block a little better.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 03/14] nir: cleanup cf nodes earlier in nir_cf_node_remove()
On Thu, May 21, 2015 at 9:40 AM, Connor Abbott cwabbo...@gmail.com wrote: Before, when we were deleting a cf node that was a block, we were first removing all the instructions and then calling cleanup_cf_node(), at which point cleanup_cf_node() couldn't do its job. Just move it before everything else, which should be ok for the non-block case too. What's the distinction between a cf node and a block? A cf node may not have actual instructions, like for avoiding critical edges? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 08/14] nir: add an optimization for removing dead control flow
On Thu, May 21, 2015 at 9:41 AM, Connor Abbott cwabbo...@gmail.com wrote:
I'm not so sure about where to put the helper currently in nir.c... on
the one hand, it's pretty specific to this pass, but on the other hand
it needs to do some very fiddly low-level things to the control flow
which is why it needs access to a static function in nir.c
(stitch_blocks()) that I'd rather not expose publically.
v2: use nir_cf_node_remove_after() instead of our own broken thing.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 26
src/glsl/nir/nir.h | 7 +++
src/glsl/nir/nir_opt_dead_cf.c | 138
+
3 files changed, 171 insertions(+)
create mode 100644 src/glsl/nir/nir_opt_dead_cf.c
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 0223fcd..79c4a4a 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1426,6 +1426,32 @@ nir_cf_node_remove_after(nir_cf_node *node)
}
+/* Takes a control flow list 'cf_list,' presumed to be a child of the control
+ * flow node 'node,' pastes cf_list after node, and then deletes node.
+ */
+
Extra newline.
+void
+nir_cf_list_move_after_node(nir_cf_node *node, struct exec_list *cf_list)
+{
+ nir_cf_node *after = nir_cf_node_next(node);
+ assert(after-type == nir_cf_node_block);
+ nir_block *after_block = nir_cf_node_as_block(after);
+
+ foreach_list_typed(nir_cf_node, child, node, cf_list) {
+ child-parent = node-parent;
+ }
+
+ nir_cf_node *last = exec_node_data(nir_cf_node,
exec_list_get_tail(cf_list),
+ node);
+ assert(last-type == nir_cf_node_block);
+ nir_block *last_block = nir_cf_node_as_block(last);
+
+ exec_node_insert_list_before(after-node, cf_list);
+ stitch_blocks(last_block, after_block);
+
+ nir_cf_node_remove(node);
+}
+
static bool
add_use_cb(nir_src *src, void *state)
{
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index d6702b4..38bd9c4 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -1524,6 +1524,11 @@ void nir_cf_node_remove(nir_cf_node *node);
/** removes everything after the given control flow node */
void nir_cf_node_remove_after(nir_cf_node *node);
+/** Takes a control flow list 'cf_list,' presumed to be a child of the
control
+ * flow node 'node,' pastes cf_list after node, and then deletes node.
+ */
I don't think I'd put the comment in both places (I'd leave it with
the function definition). Otherwise it'll just inevitably get out of
sync.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 03/14] nir: cleanup cf nodes earlier in nir_cf_node_remove()
On Thu, May 21, 2015 at 12:54 PM, Matt Turner matts...@gmail.com wrote: On Thu, May 21, 2015 at 9:40 AM, Connor Abbott cwabbo...@gmail.com wrote: Before, when we were deleting a cf node that was a block, we were first removing all the instructions and then calling cleanup_cf_node(), at which point cleanup_cf_node() couldn't do its job. Just move it before everything else, which should be ok for the non-block case too. What's the distinction between a cf node and a block? A cf node may not have actual instructions, like for avoiding critical edges? A cf node (control-flow node) may be a block, a loop, or an if statement -- it's an element of the control-flow tree. It's defined as 'nir_cf_node' in nir.h, and embedded in nir_if, nir_loop, and nir_block. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/27] glapi: gl_procs.py: Fix a few long hanging style things
Err, yes. Fixed. On Wed, May 20, 2015 at 06:27:22PM -0700, Matt Turner wrote: glapi: gl_procs.py: Fix a few long hanging style things s/long/low/ presumably signature.asc Description: Digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 14/14] XXX disable opt_if_simplification
This helps with testing the NIR dead cf pass. Signed-off-by: Connor Abbott cwabbo...@gmail.com --- src/glsl/glsl_parser_extras.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp index be6713c..42e9b49 100644 --- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -1646,7 +1646,7 @@ do_common_optimization(exec_list *ir, bool linked, progress = do_dead_functions(ir) || progress; progress = do_structure_splitting(ir) || progress; } - progress = do_if_simplification(ir) || progress; + //progress = do_if_simplification(ir) || progress; progress = opt_flatten_nested_if_blocks(ir) || progress; progress = opt_conditional_discard(ir) || progress; progress = do_copy_propagation(ir) || progress; -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 09/14] nir/dead_cf: delete code that's unreachable due to jumps
v2: use nir_cf_node_remove_after().
v2: use foreach_list_typed() instead of hardcoding a list walk.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir_opt_dead_cf.c | 123 ++---
1 file changed, 115 insertions(+), 8 deletions(-)
diff --git a/src/glsl/nir/nir_opt_dead_cf.c b/src/glsl/nir/nir_opt_dead_cf.c
index e0d4859..d71538c 100644
--- a/src/glsl/nir/nir_opt_dead_cf.c
+++ b/src/glsl/nir/nir_opt_dead_cf.c
@@ -39,6 +39,26 @@
* We delete the if statement and paste the contents of the always-executed
* branch into the surrounding control flow, possibly removing more code if
* the branch had a jump at the end.
+ *
+ * The other way is that control flow can end in a jump so that code after it
+ * never gets executed. In particular, this can happen after optimizing
+ * something like:
+ *
+ * if (true) {
+ *...
+ *break;
+ * }
+ * ...
+ *
+ * We also consider the case where both branches of an if end in a jump, e.g.:
+ *
+ * if (...) {
+ *break;
+ * } else {
+ *continue;
+ * }
+ * ...
+ *
*/
static void
@@ -94,30 +114,117 @@ opt_constant_if(nir_if *if_stmt, bool condition)
}
static bool
-dead_cf_cb(nir_block *block, void *state)
+dead_cf_block(nir_block *block)
{
- bool *progress = state;
-
nir_if *following_if = nir_block_get_following_if(block);
if (!following_if)
- return true;
+ return false;
nir_const_value *const_value =
nir_src_as_const_value(following_if-condition);
if (!const_value)
- return true;
+ return false;
opt_constant_if(following_if, const_value-u[0] != 0);
- *progress = true;
return true;
}
static bool
-opt_dead_cf_impl(nir_function_impl *impl)
+ends_in_jump(nir_block *block)
+{
+ if (exec_list_is_empty(block-instr_list))
+ return false;
+
+ nir_instr *instr = nir_block_last_instr(block);
+ return instr-type == nir_instr_type_jump;
+}
+
+static bool
+dead_cf_list(struct exec_list *list, bool *list_ends_in_jump)
{
bool progress = false;
- nir_foreach_block(impl, dead_cf_cb, progress);
+ *list_ends_in_jump = false;
+
+ nir_cf_node *prev = NULL;
+
+ foreach_list_typed(nir_cf_node, cur, node, list) {
+ switch (cur-type) {
+ case nir_cf_node_block: {
+ nir_block *block = nir_cf_node_as_block(cur);
+ if (dead_cf_block(block)) {
+/* We just deleted the if after this block, so we may have
+ * deleted the block before or after it -- which one is an
+ * implementation detail. Therefore, to recover the place we were
+ * at, we have to use the previous cf_node.
+ */
+
+if (prev) {
+ cur = nir_cf_node_next(prev);
+} else {
+ cur = exec_node_data(nir_cf_node, exec_list_get_head(list),
+node);
+}
+
+block = nir_cf_node_as_block(cur);
+
+progress = true;
+ }
+
+ if (ends_in_jump(block)) {
+*list_ends_in_jump = true;
+
+if (!exec_node_is_tail_sentinel(cur-node.next)) {
+ nir_cf_node_remove_after(cur);
+ return true;
+}
+ }
+
+ break;
+ }
+
+ case nir_cf_node_if: {
+ nir_if *if_stmt = nir_cf_node_as_if(cur);
+ bool then_ends_in_jump, else_ends_in_jump;
+ progress |= dead_cf_list(if_stmt-then_list, then_ends_in_jump);
+ progress |= dead_cf_list(if_stmt-else_list, else_ends_in_jump);
+
+ if (then_ends_in_jump else_ends_in_jump) {
+*list_ends_in_jump = true;
+nir_block *next = nir_cf_node_as_block(nir_cf_node_next(cur));
+if (!exec_list_is_empty(next-instr_list) ||
+!exec_node_is_tail_sentinel(next-cf_node.node.next)) {
+ nir_cf_node_remove_after(cur);
+ return true;
+}
+ }
+
+ break;
+ }
+
+ case nir_cf_node_loop: {
+ nir_loop *loop = nir_cf_node_as_loop(cur);
+ bool dummy;
+ progress |= dead_cf_list(loop-body, dummy);
+
+ break;
+ }
+
+ default:
+ unreachable(unknown cf node type);
+ }
+
+ prev = cur;
+ }
+
+ return progress;
+}
+
+static bool
+opt_dead_cf_impl(nir_function_impl *impl)
+{
+ bool dummy;
+ bool progress = dead_cf_list(impl-body, dummy);
if (progress)
nir_metadata_preserve(impl, nir_metadata_none);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 11/14] nir: add a helper for iterating over blocks in a cf node
We were already doing this internally for iterating over a function
implementation, so just expose it directly.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 7 +++
src/glsl/nir/nir.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index cbee507..e2b001d 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -2200,6 +2200,13 @@ foreach_cf_node(nir_cf_node *node, nir_foreach_block_cb
cb,
}
bool
+nir_foreach_block_in_cf_node(nir_cf_node *node, nir_foreach_block_cb cb,
+ void *state)
+{
+ return foreach_cf_node(node, cb, false, state);
+}
+
+bool
nir_foreach_block(nir_function_impl *impl, nir_foreach_block_cb cb, void
*state)
{
foreach_list_typed_safe(nir_cf_node, node, node, impl-body) {
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 8f9e31e..220d16f 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -1606,6 +1606,8 @@ bool nir_foreach_block(nir_function_impl *impl,
nir_foreach_block_cb cb,
void *state);
bool nir_foreach_block_reverse(nir_function_impl *impl, nir_foreach_block_cb
cb,
void *state);
+bool nir_foreach_block_in_cf_node(nir_cf_node *node, nir_foreach_block_cb cb,
+ void *state);
/* If the following CF node is an if, this function returns that if.
* Otherwise, it returns NULL.
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 13/14] i965/fs/nir: enable the dead control flow optimization
Doesn't do anything on the public shader-db. Signed-off-by: Connor Abbott cwabbo...@gmail.com --- src/glsl/Makefile.sources | 1 + src/mesa/drivers/dri/i965/brw_nir.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources index d784a81..7bdd895 100644 --- a/src/glsl/Makefile.sources +++ b/src/glsl/Makefile.sources @@ -50,6 +50,7 @@ NIR_FILES = \ nir/nir_opt_copy_propagate.c \ nir/nir_opt_cse.c \ nir/nir_opt_dce.c \ + nir/nir_opt_dead_cf.c \ nir/nir_opt_gcm.c \ nir/nir_opt_global_to_local.c \ nir/nir_opt_peephole_ffma.c \ diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c index de4d7aa..4001190 100644 --- a/src/mesa/drivers/dri/i965/brw_nir.c +++ b/src/mesa/drivers/dri/i965/brw_nir.c @@ -52,6 +52,8 @@ nir_optimize(nir_shader *nir) nir_validate_shader(nir); progress |= nir_opt_constant_folding(nir); nir_validate_shader(nir); + progress |= nir_opt_dead_cf(nir); + nir_validate_shader(nir); progress |= nir_opt_remove_phis(nir); nir_validate_shader(nir); } while (progress); -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/27] glapi: gl_gentable.py: Replace getopt with argparse
Fixed locally On Wed, May 20, 2015 at 06:28:58PM -0700, Matt Turner wrote: On Wed, May 20, 2015 at 6:03 PM, Dylan Baker baker.dyla...@gmail.com wrote: Signed-off-by: Dylan Baker dylanx.c.ba...@intel.com --- src/mapi/glapi/gen/gl_gentable.py | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/mapi/glapi/gen/gl_gentable.py b/src/mapi/glapi/gen/gl_gentable.py index 06a5ebf..f7ffaf0 100644 --- a/src/mapi/glapi/gen/gl_gentable.py +++ b/src/mapi/glapi/gen/gl_gentable.py @@ -2,6 +2,7 @@ # (C) Copyright IBM Corporation 2004, 2005 # (C) Copyright Apple Inc. 2011 +# Copyright (C) 2015 Itnel Corporation s/Itnel/Intel/ signature.asc Description: Digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 05/14] nir: fix up phi nodes when removing cf nodes
When we deleted jumps like 'break' and 'continue,' we weren't removing
the phi sources that corresponded to them. Also, if we had a loop like
loop {
...
break;
}
and we deleted the break at the end because it was unreachable, we
wouldn't add an undefined source to the phi nodes at the beginning of
the loop. Finally, we weren't updating the phi sources when stitching
two basic blocks together after deleting a control-flow node. This patch
fixes these issues by adding codepaths to the helpers for
adding/removing control flow nodes that deal with updating phi sources.
There's one somewhat unrelated fix to stitch_blocks() snuck in, where we
weren't updating the successors correctly if the earlier block had a
jump, but a lot of the logic for it overlaps with the phi node changes
and it seemed hard to split it out so I've left it in for now.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 99 +-
src/glsl/nir/nir.h | 3 ++
2 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index dc6d63f..bd4f6cc 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -945,8 +945,62 @@ handle_jump(nir_block *block)
}
static void
+add_undef_phi_src(nir_block *pred, nir_block *block)
+{
+ nir_function_impl *impl = nir_cf_node_get_function(block-cf_node);
+ void *mem_ctx = ralloc_parent(block);
+
+ nir_foreach_instr(block, instr) {
+ if (instr-type != nir_instr_type_phi)
+ break;
+
+ nir_phi_instr *phi = nir_instr_as_phi(instr);
+
+ nir_ssa_undef_instr *undef =
+ nir_ssa_undef_instr_create(mem_ctx, phi-dest.ssa.num_components);
+
+ nir_instr_insert_before_cf_list(impl-body, undef-instr);
+
+ nir_phi_src *src = ralloc(phi, nir_phi_src);
+ src-pred = pred;
+ src-src.parent_instr = phi-instr;
+ src-src.is_ssa = true;
+ src-src.ssa = undef-def;
+
+ list_addtail(src-src.use_link, src-src.ssa-uses);
+
+ exec_list_push_tail(phi-srcs, src-node);
+ }
+}
+
+static void
handle_remove_jump(nir_block *block, nir_jump_type type)
{
+ /*
+* If the jump we're removing is a break or continue, then we're removing
+* predecessors from a basic block that may have phi nodes at the beginning
+* so we need to update them by removing the source corresponding to us.
+*/
+
+ if (type == nir_jump_break || type == nir_jump_continue) {
+ nir_block *succ = block-successors[0];
+
+ nir_foreach_instr(succ, instr) {
+ if (instr-type != nir_instr_type_phi)
+break;
+
+ nir_phi_instr *phi = nir_instr_as_phi(instr);
+
+ nir_foreach_phi_src_safe(phi, src) {
+if (src-pred == block) {
+ list_del(src-src.use_link);
+ exec_node_remove(src-node);
+ break;
+}
+ }
+ }
+ }
+
unlink_block_successors(block);
if (exec_node_is_tail_sentinel(block-cf_node.node.next)) {
@@ -957,6 +1011,7 @@ handle_remove_jump(nir_block *block, nir_jump_type type)
nir_block *next_block = nir_cf_node_as_block(next);
link_blocks(block, next_block, NULL);
+ add_undef_phi_src(block, next_block);
} else {
assert(parent-type == nir_cf_node_loop);
nir_loop *loop = nir_cf_node_as_loop(parent);
@@ -966,6 +1021,7 @@ handle_remove_jump(nir_block *block, nir_jump_type type)
nir_block *head_block = nir_cf_node_as_block(head);
link_blocks(block, head_block, NULL);
+ add_undef_phi_src(block, head_block);
}
} else {
nir_cf_node *next = nir_cf_node_next(block-cf_node);
@@ -990,6 +1046,7 @@ handle_remove_jump(nir_block *block, nir_jump_type type)
nir_block *first_block = nir_cf_node_as_block(first);
link_blocks(block, first_block, NULL);
+ add_undef_phi_src(block, first_block);
}
}
@@ -1008,6 +1065,7 @@ handle_remove_jump(nir_block *block, nir_jump_type type)
last_block-successors[1] = next_block;
block_add_pred(next_block, last_block);
+ add_undef_phi_src(last_block, next_block);
}
}
@@ -1195,7 +1253,46 @@ stitch_blocks(nir_block *before, nir_block *after)
* TODO: special case when before is empty and after isn't?
*/
- move_successors(after, before);
+ /*
+* First, we have to consider the case where 'after' may have some phi
+* nodes that refer to it, so we have to update those to refer to 'before'
+* instead, unless 'before' ends in a jump in which case those phi sources
+* are unreachable and we should delete them.
+*/
+
+ bool before_ends_in_jump = !exec_list_is_empty(before-instr_list)
+ nir_block_last_instr(before)-type ==
nir_instr_type_jump;
+
+ assert(!before_ends_in_jump || exec_list_is_empty(after-instr_list));
+
+ for (unsigned i = 0; i 2; i++) {
+ nir_block
[Mesa-dev] [PATCH v2 02/14] nir: insert ssa_undef instructions when cleaning up defs/uses
The point of cleanup_defs_uses() is to make an instruction safe to
remove by removing any references that the rest of the shader may have
to it. Previously, it was handling register use/def sets and removing
the instruction from the use sets of any SSA sources it had, but if the
instruction defined an SSA value that was used by other instructions it
wasn't doing anything. This was ok, since we were always careful to make
sure that no removed instruction ever had any uses, but now we want to
start removing unreachable instruction which might still be used in
reachable parts of the code. In that case, the value that any uses get
is undefined since the instruction never actually executes, so we can
just replace the instruction with an ssa_undef_instr.
v2: split out other fixes
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 38 --
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index f03e80a..704553f 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1206,26 +1206,26 @@ stitch_blocks(nir_block *before, nir_block *after)
}
static void
-remove_defs_uses(nir_instr *instr);
+remove_defs_uses(nir_instr *instr, nir_function_impl *impl);
static void
-cleanup_cf_node(nir_cf_node *node)
+cleanup_cf_node(nir_cf_node *node, nir_function_impl *impl)
{
switch (node-type) {
case nir_cf_node_block: {
nir_block *block = nir_cf_node_as_block(node);
/* We need to walk the instructions and clean up defs/uses */
nir_foreach_instr(block, instr)
- remove_defs_uses(instr);
+ remove_defs_uses(instr, impl);
break;
}
case nir_cf_node_if: {
nir_if *if_stmt = nir_cf_node_as_if(node);
foreach_list_typed(nir_cf_node, child, node, if_stmt-then_list)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
foreach_list_typed(nir_cf_node, child, node, if_stmt-else_list)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
list_del(if_stmt-condition.use_link);
break;
@@ -1234,13 +1234,12 @@ cleanup_cf_node(nir_cf_node *node)
case nir_cf_node_loop: {
nir_loop *loop = nir_cf_node_as_loop(node);
foreach_list_typed(nir_cf_node, child, node, loop-body)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
break;
}
case nir_cf_node_function: {
- nir_function_impl *impl = nir_cf_node_as_function(node);
foreach_list_typed(nir_cf_node, child, node, impl-body)
- cleanup_cf_node(child);
+ cleanup_cf_node(child, impl);
break;
}
default:
@@ -1443,16 +1442,35 @@ remove_def_cb(nir_dest *dest, void *state)
return true;
}
+static bool
+remove_ssa_def_cb(nir_ssa_def *def, void *state)
+{
+ nir_function_impl *impl = state;
+ nir_shader *shader = impl-overload-function-shader;
+
+ if (!list_empty(def-uses) || !list_empty(def-if_uses)) {
+ nir_ssa_undef_instr *undef =
+ nir_ssa_undef_instr_create(shader, def-num_components);
+ nir_instr_insert_before_cf_list(impl-body, undef-instr);
+ nir_ssa_def_rewrite_uses(def, nir_src_for_ssa(undef-def), shader);
+ }
+
+ return true;
+}
+
+
static void
-remove_defs_uses(nir_instr *instr)
+remove_defs_uses(nir_instr *instr, nir_function_impl *impl)
{
nir_foreach_dest(instr, remove_def_cb, instr);
nir_foreach_src(instr, remove_use_cb, instr);
+ nir_foreach_ssa_def(instr, remove_ssa_def_cb, impl);
}
void nir_instr_remove(nir_instr *instr)
{
- remove_defs_uses(instr);
+ nir_function_impl *impl = nir_cf_node_get_function(instr-block-cf_node);
+ remove_defs_uses(instr, impl);
exec_node_remove(instr-node);
if (instr-type == nir_instr_type_jump) {
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 08/14] nir: add an optimization for removing dead control flow
I'm not so sure about where to put the helper currently in nir.c... on
the one hand, it's pretty specific to this pass, but on the other hand
it needs to do some very fiddly low-level things to the control flow
which is why it needs access to a static function in nir.c
(stitch_blocks()) that I'd rather not expose publically.
v2: use nir_cf_node_remove_after() instead of our own broken thing.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 26
src/glsl/nir/nir.h | 7 +++
src/glsl/nir/nir_opt_dead_cf.c | 138 +
3 files changed, 171 insertions(+)
create mode 100644 src/glsl/nir/nir_opt_dead_cf.c
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 0223fcd..79c4a4a 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1426,6 +1426,32 @@ nir_cf_node_remove_after(nir_cf_node *node)
}
+/* Takes a control flow list 'cf_list,' presumed to be a child of the control
+ * flow node 'node,' pastes cf_list after node, and then deletes node.
+ */
+
+void
+nir_cf_list_move_after_node(nir_cf_node *node, struct exec_list *cf_list)
+{
+ nir_cf_node *after = nir_cf_node_next(node);
+ assert(after-type == nir_cf_node_block);
+ nir_block *after_block = nir_cf_node_as_block(after);
+
+ foreach_list_typed(nir_cf_node, child, node, cf_list) {
+ child-parent = node-parent;
+ }
+
+ nir_cf_node *last = exec_node_data(nir_cf_node, exec_list_get_tail(cf_list),
+ node);
+ assert(last-type == nir_cf_node_block);
+ nir_block *last_block = nir_cf_node_as_block(last);
+
+ exec_node_insert_list_before(after-node, cf_list);
+ stitch_blocks(last_block, after_block);
+
+ nir_cf_node_remove(node);
+}
+
static bool
add_use_cb(nir_src *src, void *state)
{
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index d6702b4..38bd9c4 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -1524,6 +1524,11 @@ void nir_cf_node_remove(nir_cf_node *node);
/** removes everything after the given control flow node */
void nir_cf_node_remove_after(nir_cf_node *node);
+/** Takes a control flow list 'cf_list,' presumed to be a child of the control
+ * flow node 'node,' pastes cf_list after node, and then deletes node.
+ */
+void nir_cf_list_move_after_node(nir_cf_node *node, struct exec_list *cf_list);
+
/** requests that the given pieces of metadata be generated */
void nir_metadata_require(nir_function_impl *impl, nir_metadata required);
/** dirties all but the preserved metadata */
@@ -1698,6 +1703,8 @@ bool nir_opt_cse(nir_shader *shader);
bool nir_opt_dce_impl(nir_function_impl *impl);
bool nir_opt_dce(nir_shader *shader);
+bool nir_opt_dead_cf(nir_shader *shader);
+
void nir_opt_gcm(nir_shader *shader);
bool nir_opt_peephole_select(nir_shader *shader);
diff --git a/src/glsl/nir/nir_opt_dead_cf.c b/src/glsl/nir/nir_opt_dead_cf.c
new file mode 100644
index 000..e0d4859
--- /dev/null
+++ b/src/glsl/nir/nir_opt_dead_cf.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright © 2014 Connor Abbott
+ *
+ * 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:
+ *Connor Abbott (cwabbo...@gmail.com)
+ *
+ */
+
+#include nir.h
+
+/*
+ * This file implements an optimization that deletes statically unreachable
+ * code. In NIR, one way this can happen if if an if statement has a constant
+ * condition:
+ *
+ * if (true) {
+ *...
+ * }
+ *
+ * We delete the if statement and paste the contents of the always-executed
+ * branch into the surrounding control flow, possibly removing more code if
+ * the branch had a jump at the end.
+ */
+
+static void
+opt_constant_if(nir_if *if_stmt, bool condition)
+{
+ void *mem_ctx = ralloc_parent(if_stmt);
+
+ /* First, we need to remove any phi nodes after the if by rewriting uses to
+* point to the correct source.
+*/
+ nir_block *after =
[Mesa-dev] [PATCH v2 04/14] nir: properly clean up jumps when removing cf nodes
Before, we might have left dangling predecessors from jumps that were
going to be removed.
v2: split out from nir: insert ssa_undef instructions when cleaning up
defs/uses
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index a2b5e7c..dc6d63f 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1214,9 +1214,15 @@ cleanup_cf_node(nir_cf_node *node, nir_function_impl
*impl)
switch (node-type) {
case nir_cf_node_block: {
nir_block *block = nir_cf_node_as_block(node);
- /* We need to walk the instructions and clean up defs/uses */
- nir_foreach_instr(block, instr)
+ /* We need to walk the instructions and clean up defs/uses,
+ * as well as clean up any jumps to control flow that may not be getting
+ * deleted.
+ */
+ nir_foreach_instr(block, instr) {
+ if (instr-type == nir_instr_type_jump)
+handle_remove_jump(block, nir_instr_as_jump(instr)-type);
remove_defs_uses(instr, impl);
+ }
break;
}
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 10/14] nir: add nir_block_get_following_loop() helper
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 16
src/glsl/nir/nir.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 79c4a4a..cbee507 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -2242,6 +2242,22 @@ nir_block_get_following_if(nir_block *block)
return nir_cf_node_as_if(next_node);
}
+nir_loop *
+nir_block_get_following_loop(nir_block *block)
+{
+ if (exec_node_is_tail_sentinel(block-cf_node.node))
+ return NULL;
+
+ if (nir_cf_node_is_last(block-cf_node))
+ return NULL;
+
+ nir_cf_node *next_node = nir_cf_node_next(block-cf_node);
+
+ if (next_node-type != nir_cf_node_loop)
+ return NULL;
+
+ return nir_cf_node_as_loop(next_node);
+}
static bool
index_block(nir_block *block, void *state)
{
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 38bd9c4..8f9e31e 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -1612,6 +1612,8 @@ bool nir_foreach_block_reverse(nir_function_impl *impl,
nir_foreach_block_cb cb,
*/
nir_if *nir_block_get_following_if(nir_block *block);
+nir_loop *nir_block_get_following_loop(nir_block *block);
+
void nir_index_local_regs(nir_function_impl *impl);
void nir_index_global_regs(nir_shader *shader);
void nir_index_ssa_defs(nir_function_impl *impl);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 12/14] nir/dead_cf: add support for removing useless loops
v2: fix detecting if the loop has any phi nodes after it.
v2: use nir_foreach_ssa_def() instead of nir_foreach_dest() when
checking for values live after the loop to catch const_load
instructions.
v2: fix handling return instructions
v2: add some documentation to loop_is_dead()
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir_opt_dead_cf.c | 121 +
1 file changed, 109 insertions(+), 12 deletions(-)
diff --git a/src/glsl/nir/nir_opt_dead_cf.c b/src/glsl/nir/nir_opt_dead_cf.c
index d71538c..a8d438f 100644
--- a/src/glsl/nir/nir_opt_dead_cf.c
+++ b/src/glsl/nir/nir_opt_dead_cf.c
@@ -28,9 +28,9 @@
#include nir.h
/*
- * This file implements an optimization that deletes statically unreachable
- * code. In NIR, one way this can happen if if an if statement has a constant
- * condition:
+ * This file implements an optimization that deletes statically
+ * unreachable/dead code. In NIR, one way this can happen if if an if
+ * statement has a constant condition:
*
* if (true) {
*...
@@ -40,7 +40,7 @@
* branch into the surrounding control flow, possibly removing more code if
* the branch had a jump at the end.
*
- * The other way is that control flow can end in a jump so that code after it
+ * Another way is that control flow can end in a jump so that code after it
* never gets executed. In particular, this can happen after optimizing
* something like:
*
@@ -59,6 +59,12 @@
* }
* ...
*
+ * Finally, we also handle removing useless loops, i.e. loops with no side
+ * effects and without any definitions that are used elsewhere. This case is a
+ * little different from the first two in that the code is actually run (it
+ * just never does anything), but there are similar issues with needing to
+ * be careful with restarting after deleting the cf_node (see dead_cf_list())
+ * so this is a convenient place to remove them.
*/
static void
@@ -114,19 +120,107 @@ opt_constant_if(nir_if *if_stmt, bool condition)
}
static bool
+block_has_no_side_effects(nir_block *block, void *state)
+{
+ (void) state;
+
+ nir_foreach_instr(block, instr) {
+ if (instr-type == nir_instr_type_call)
+ return false;
+
+ /* Return instructions can cause us to skip over other side-effecting
+ * instructions after the loop, so consider them to have side effects
+ * here.
+ */
+
+ if (instr-type == nir_instr_type_jump
+ nir_instr_as_jump(instr)-type == nir_jump_return)
+ return false;
+
+ if (instr-type != nir_instr_type_intrinsic)
+ continue;
+
+ nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
+ if (!nir_intrinsic_infos[intrin-intrinsic].flags
+ NIR_INTRINSIC_CAN_ELIMINATE)
+ return false;
+ }
+
+ return true;
+}
+
+static bool
+def_not_live_out(nir_ssa_def *def, void *state)
+{
+ nir_block *after = state;
+
+ return !BITSET_TEST(after-live_in, def-live_index);
+}
+
+/*
+ * Test if a loop is dead. A loop is dead if:
+ *
+ * 1) It has no side effects (i.e. intrinsics which could possibly affect the
+ * state of the program aside from producing an SSA value, indicated by a lack
+ * of NIR_INTRINSIC_CAN_ELIMINATE).
+ *
+ * 2) It has no phi nodes after it, since those indicate values inside the
+ * loop being used after the loop.
+ *
+ * 3) If there are no phi nodes after the loop, then the only way a value
+ * defined inside the loop can be used outside the loop is if its definition
+ * dominates the block after the loop. If none of the definitions that
+ * dominate the loop exit are used outside the loop, then the loop is dead
+ * and it can be deleted.
+ */
+
+static bool
+loop_is_dead(nir_loop *loop)
+{
+ nir_block *before = nir_cf_node_as_block(nir_cf_node_prev(loop-cf_node));
+ nir_block *after = nir_cf_node_as_block(nir_cf_node_next(loop-cf_node));
+
+ if (!exec_list_is_empty(after-instr_list)
+ nir_block_first_instr(after)-type == nir_instr_type_phi)
+ return false;
+
+ if (!nir_foreach_block_in_cf_node(loop-cf_node, block_has_no_side_effects,
+ NULL))
+ return false;
+
+ for (nir_block *cur = after-imm_dom; cur != before; cur = cur-imm_dom) {
+ nir_foreach_instr(cur, instr) {
+ if (!nir_foreach_ssa_def(instr, def_not_live_out, after))
+return false;
+ }
+ }
+
+ return true;
+}
+
+static bool
dead_cf_block(nir_block *block)
{
nir_if *following_if = nir_block_get_following_if(block);
- if (!following_if)
- return false;
+ if (following_if) {
+ nir_const_value *const_value =
+nir_src_as_const_value(following_if-condition);
+
+ if (!const_value)
+return false;
- nir_const_value *const_value =
- nir_src_as_const_value(following_if-condition);
+ opt_constant_if(following_if, const_value-u[0] != 0);
+ return true;
+ }
- if (!const_value)
- return false;
+
[Mesa-dev] [PATCH v2 03/14] nir: cleanup cf nodes earlier in nir_cf_node_remove()
Before, when we were deleting a cf node that was a block, we were first
removing all the instructions and then calling cleanup_cf_node(), at
which point cleanup_cf_node() couldn't do its job. Just move it before
everything else, which should be ok for the non-block case too.
v2: split out from previous commit
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 704553f..a2b5e7c 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1253,6 +1253,8 @@ nir_cf_node_remove(nir_cf_node *node)
nir_function_impl *impl = nir_cf_node_get_function(node);
nir_metadata_preserve(impl, nir_metadata_none);
+ cleanup_cf_node(node, impl);
+
if (node-type == nir_cf_node_block) {
/*
* Basic blocks can't really be removed by themselves, since they act as
@@ -1274,8 +1276,6 @@ nir_cf_node_remove(nir_cf_node *node)
exec_node_remove(node-node);
stitch_blocks(before_block, after_block);
}
-
- cleanup_cf_node(node);
}
static bool
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 07/14] nir/validate: validate successors at the end of a loop
I found this useful while debugging some control flow bugs while working
on the dead control flow pass.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir_validate.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
index da92ed9..0332c7d 100644
--- a/src/glsl/nir/nir_validate.c
+++ b/src/glsl/nir/nir_validate.c
@@ -78,6 +78,9 @@ typedef struct {
/* the parent of the current cf node being visited */
nir_cf_node *parent_node;
+ /* whether the current loop we're visiting has a break statement */
+ bool has_break;
+
/* the current function implementation being validated */
nir_function_impl *impl;
@@ -513,6 +516,8 @@ validate_instr(nir_instr *instr, validate_state *state)
break;
case nir_instr_type_jump:
+ if (nir_instr_as_jump(instr)-type == nir_jump_break)
+ state-has_break = true;
break;
default:
@@ -641,6 +646,9 @@ validate_if(nir_if *if_stmt, validate_state *state)
static void
validate_loop(nir_loop *loop, validate_state *state)
{
+ bool old_has_break = state-has_break;
+ state-has_break = false;
+
assert(!exec_node_is_head_sentinel(loop-cf_node.node.prev));
nir_cf_node *prev_node = nir_cf_node_prev(loop-cf_node);
assert(prev_node-type == nir_cf_node_block);
@@ -663,7 +671,20 @@ validate_loop(nir_loop *loop, validate_state *state)
validate_cf_node(cf_node, state);
}
+ nir_block *last_block = nir_cf_node_as_block(nir_loop_last_cf_node(loop));
+ if (exec_list_is_empty(last_block-instr_list) ||
+ nir_block_last_instr(last_block)-type != nir_instr_type_jump) {
+ assert(last_block-successors[0]-cf_node ==
nir_loop_first_cf_node(loop));
+ }
+ if (state-has_break) {
+ assert(last_block-successors[1] == NULL);
+ } else {
+ nir_block *next = nir_cf_node_as_block(nir_cf_node_next(loop-cf_node));
+ assert(last_block-successors[1] == next);
+ }
+
state-parent_node = old_parent;
+ state-has_break = old_has_break;
}
static void
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 00/14] NIR dead control-flow removal
This is the second version of my dead control-flow series. In addition to fixing up a few things pointed out by Jason and cleaning up a few other odds and ends, I've fixed a number of bugs caught by disabling opt_if_simplification in GLSL IR as in the last patch. In particular, patch 5 deals with rewriting phi nodes when deleting control flow, something I forgot when doing the original series. I've run the series, including the last patch which is just for testing, through piglit and there are no regressions except for shaders/glsl-const-folding-01, which is due to the last patch disabling the GLSL IR optimization that the test is relying on, and glslparsertest/shaders/correctfull.frag, which now crashes in the i965 fs backend -- it seems to be an issue with i965, and not NIR, that was uncovered after NIR was made more aggressive. I've also verified that the public shader-db doesn't crash with this series (including the last patch), and manually tested some of the tricker cases, although it would be a good idea to run the entire thing through the private shader-db too. This series is also available at git://people.freedesktop.org/~cwabbott0/mesa nir-dead-cf-v4 Connor Abbott (14): nir/vars_to_ssa: don't rewrite removed instructions nir: insert ssa_undef instructions when cleaning up defs/uses nir: cleanup cf nodes earlier in nir_cf_node_remove() nir: properly clean up jumps when removing cf nodes nir: fix up phi nodes when removing cf nodes nir: add nir_cf_node_remove_after() nir/validate: validate successors at the end of a loop nir: add an optimization for removing dead control flow nir/dead_cf: delete code that's unreachable due to jumps nir: add nir_block_get_following_loop() helper nir: add a helper for iterating over blocks in a cf node nir/dead_cf: add support for removing useless loops i965/fs/nir: enable the dead control flow optimization XXX disable opt_if_simplification src/glsl/Makefile.sources| 1 + src/glsl/glsl_parser_extras.cpp | 2 +- src/glsl/nir/nir.c | 243 +++-- src/glsl/nir/nir.h | 17 ++ src/glsl/nir/nir_lower_vars_to_ssa.c | 3 +- src/glsl/nir/nir_opt_dead_cf.c | 342 +++ src/glsl/nir/nir_validate.c | 21 +++ src/mesa/drivers/dri/i965/brw_nir.c | 2 + 8 files changed, 615 insertions(+), 16 deletions(-) create mode 100644 src/glsl/nir/nir_opt_dead_cf.c -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 01/14] nir/vars_to_ssa: don't rewrite removed instructions
We were rewriting the uses of the intrinsic instruction to point to something else after removing it, which only happened to work since we were lax about having dangling uses when removing instructions. Fix that up. Signed-off-by: Connor Abbott cwabbo...@gmail.com --- src/glsl/nir/nir_lower_vars_to_ssa.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/glsl/nir/nir_lower_vars_to_ssa.c b/src/glsl/nir/nir_lower_vars_to_ssa.c index ccb8f99..8d0ae1b 100644 --- a/src/glsl/nir/nir_lower_vars_to_ssa.c +++ b/src/glsl/nir/nir_lower_vars_to_ssa.c @@ -647,11 +647,12 @@ rename_variables_block(nir_block *block, struct lower_variables_state *state) intrin-num_components, NULL); nir_instr_insert_before(intrin-instr, mov-instr); -nir_instr_remove(intrin-instr); nir_ssa_def_rewrite_uses(intrin-dest.ssa, nir_src_for_ssa(mov-dest.dest.ssa), state-shader); + +nir_instr_remove(intrin-instr); break; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 06/14] nir: add nir_cf_node_remove_after()
This will be used in deleting unreachable control flow.
Signed-off-by: Connor Abbott cwabbo...@gmail.com
---
src/glsl/nir/nir.c | 45 +
src/glsl/nir/nir.h | 3 +++
2 files changed, 48 insertions(+)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index bd4f6cc..0223fcd 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1381,6 +1381,51 @@ nir_cf_node_remove(nir_cf_node *node)
}
}
+void
+nir_cf_node_remove_after(nir_cf_node *node)
+{
+ nir_function_impl *impl = nir_cf_node_get_function(node);
+
+ /*
+* For non-block cf nodes, empty the block after it and then delete the
+* rest of the nodes after the block so that the list still ends with a
+* block.
+*/
+
+ if (node-type != nir_cf_node_block) {
+ node = nir_cf_node_next(node);
+ cleanup_cf_node(node, impl);
+ exec_list_make_empty(nir_cf_node_as_block(node)-instr_list);
+ }
+
+ /*
+* Now that we know that node is a block, we can just nuke everything after
+* it.
+*/
+
+ while (!nir_cf_node_is_last(node)) {
+ nir_cf_node *next = nir_cf_node_next(node);
+
+ cleanup_cf_node(next, impl);
+
+ if (nir_cf_node_is_last(next)) {
+ nir_block *next_block = nir_cf_node_as_block(next);
+ exec_list_make_empty(next_block-instr_list);
+
+ /*
+ * There are a number of issues here around fixing up successors and
+ * phi nodes, but those are solved already by the stitch_blocks()
+ * function.
+ */
+
+ stitch_blocks(nir_cf_node_as_block(node), next_block);
+ } else {
+ exec_node_remove(next-node);
+ }
+ }
+}
+
+
static bool
add_use_cb(nir_src *src, void *state)
{
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 5ae261a..d6702b4 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -1521,6 +1521,9 @@ void nir_cf_node_insert_end(struct exec_list *list,
nir_cf_node *node);
/** removes a control flow node, doing any cleanup necessary */
void nir_cf_node_remove(nir_cf_node *node);
+/** removes everything after the given control flow node */
+void nir_cf_node_remove_after(nir_cf_node *node);
+
/** requests that the given pieces of metadata be generated */
void nir_metadata_require(nir_function_impl *impl, nir_metadata required);
/** dirties all but the preserved metadata */
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 00/14] NIR dead control-flow removal
On Thu, May 21, 2015 at 9:40 AM, Connor Abbott cwabbo...@gmail.com wrote: This is the second version of my dead control-flow series. In addition to fixing up a few things pointed out by Jason and cleaning up a few other odds and ends, I've fixed a number of bugs caught by disabling opt_if_simplification in GLSL IR as in the last patch. In particular, patch 5 deals with rewriting phi nodes when deleting control flow, something I forgot when doing the original series. I've run the series, including the last patch which is just for testing, through piglit and there are no regressions except for shaders/glsl-const-folding-01, which is due to the last patch disabling the GLSL IR optimization that the test is relying on, and glslparsertest/shaders/correctfull.frag, which now crashes in the i965 fs backend -- it seems to be an issue with i965, and not NIR, that was uncovered after NIR was made more aggressive. I've also verified that the public shader-db doesn't crash with this series (including the last patch), and manually tested some of the tricker cases, although it would be a good idea to run the entire thing through the private shader-db too. I ran this through our shader-db. Through patch 13 gets us (on BDW): instructions in affected programs: 3054 - 2884 (-5.57%) helped:29 and disabling the GLSL IR optimization: instructions in affected programs: 27515 - 27913 (1.45%) helped:4 HURT: 150 I suspect there's still some room for improvement. Notably, the shader that I pointed out to Jason in reply to his series enabling NIR on Gen8+ VS isn't helped, and still contains a dead looking loop. I'll send you the shader. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/20] t_dd_dmatmp: Allow flat shaded polygons with tri fans
On Fri, May 15, 2015 at 12:07:54PM -0700, Ian Romanick wrote:
On 03/23/2015 05:47 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
We can allow rendering flat shaded polygons using tri fans if we check
the provoking vertex convention.
This sounds reasonable since it matches the DX behavior. Is there a
piglit test that would hit this?
Dunno. And in fact we won't hit it with i915 sine we HAVE_POLYGONS.
Failed to realize that before I cooked up the patch :)
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
src/mesa/tnl_dd/t_dd_dmatmp.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/mesa/tnl_dd/t_dd_dmatmp.h b/src/mesa/tnl_dd/t_dd_dmatmp.h
index 5ea2d31..3ed4a98 100644
--- a/src/mesa/tnl_dd/t_dd_dmatmp.h
+++ b/src/mesa/tnl_dd/t_dd_dmatmp.h
@@ -406,7 +406,9 @@ static void TAG(render_poly_verts)( struct gl_context
*ctx,
FLUSH();
}
- else if (HAVE_TRI_FANS ctx-Light.ShadeModel == GL_SMOOTH) {
+ else if (HAVE_TRI_FANS
+ (ctx-Light.ShadeModel == GL_SMOOTH ||
+ctx-Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION_EXT)) {
TAG(render_tri_fan_verts)( ctx, start, count, flags );
} else {
fprintf(stderr, %s - cannot draw primitive\n, __FUNCTION__);
@@ -885,7 +887,9 @@ static void TAG(render_poly_elts)( struct gl_context
*ctx,
FLUSH();
currentsz = dmasz;
}
- } else if (HAVE_TRI_FANS ctx-Light.ShadeModel == GL_SMOOTH) {
+ } else if (HAVE_TRI_FANS
+ (ctx-Light.ShadeModel == GL_SMOOTH ||
+ ctx-Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION_EXT)) {
TAG(render_tri_fan_verts)( ctx, start, count, flags );
} else {
fprintf(stderr, %s - cannot draw primitive\n, __FUNCTION__);
@@ -1109,7 +1113,9 @@ static GLboolean TAG(validate_render)( struct
gl_context *ctx,
ok = GL_TRUE;
}
else {
- ok = (HAVE_TRI_FANS ctx-Light.ShadeModel == GL_SMOOTH);
+ ok = (HAVE_TRI_FANS
+ (ctx-Light.ShadeModel == GL_SMOOTH ||
+ ctx-Light.ProvokingVertex ==
GL_FIRST_VERTEX_CONVENTION_EXT));
}
break;
case GL_QUAD_STRIP:
--
Ville Syrjälä
Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/20] t_dd_dmatmp: Disallow flat shading when rendering quad strips via tri strips
On Fri, May 15, 2015 at 12:08:33PM -0700, Ian Romanick wrote:
On 03/23/2015 05:47 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
When rendering quad strips via tri strips we can't get the provoking
vertex right, so disallow flat shading.
Same comments as for patch 2.
I'm not sure about piglit, but there's bunch of stuff in mesa-demos
that hit this stuff.
To clarify a bit, this series does fix the piglit provoking vertex
test on 855, but I'm unsure which patches precisely are the key to
that since a lot of them try to fix various provoking vertex issues.
I was too lazy to reverse bisect my own patches.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
src/mesa/tnl_dd/t_dd_dmatmp.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/mesa/tnl_dd/t_dd_dmatmp.h b/src/mesa/tnl_dd/t_dd_dmatmp.h
index 3ed4a98..f56b0aa 100644
--- a/src/mesa/tnl_dd/t_dd_dmatmp.h
+++ b/src/mesa/tnl_dd/t_dd_dmatmp.h
@@ -447,7 +447,7 @@ static void TAG(render_quad_strip_verts)( struct
gl_context *ctx,
FLUSH();
}
- else if (HAVE_TRI_STRIPS) {
+ else if (HAVE_TRI_STRIPS ctx-Light.ShadeModel == GL_SMOOTH) {
LOCAL_VARS;
int dmasz = GET_SUBSEQUENT_VB_MAX_VERTS();
int currentsz;
@@ -1124,7 +1124,7 @@ static GLboolean TAG(validate_render)( struct
gl_context *ctx,
} else if (HAVE_QUAD_STRIPS) {
ok = GL_TRUE;
} else {
- ok = HAVE_TRI_STRIPS;
+ ok = (HAVE_TRI_STRIPS ctx-Light.ShadeModel == GL_SMOOTH);
}
break;
case GL_QUADS:
--
Ville Syrjälä
Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
