Re: [Mesa-dev] [PATCH] meta: Use AMD_vertex_shader_layer instead of a GS for layered clears.

2014-06-24 Thread Kenneth Graunke 
On Monday, June 23, 2014 09:11:09 PM Chris Forbes wrote:
 Reviewed-by: Chris Forbes chr...@ijw.co.nz
 
 Have you got a case where this makes a noticeable difference to performance?

No, not particularly - I was just looking at INTEL_DEBUG=state when running 
Shadowrun and noticed a bunch of GS and URB churn in an application that 
purely uses ARB programs.  Seemed strange.  With the patch, GS remained 
untouched.

A couple quick tests doesn't show much gain from the change.  But it does 
simplify the code a bit, at least.

--Ken

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] meta: Use AMD_vertex_shader_layer instead of a GS for layered clears.

2014-06-23 Thread Chris Forbes 
Reviewed-by: Chris Forbes chr...@ijw.co.nz

Have you got a case where this makes a noticeable difference to performance?

On Mon, Jun 23, 2014 at 5:27 PM, Kenneth Graunke kenn...@whitecape.org wrote:
 On i965, enabling and disabling the GS is not free: you have to do a
 full pipeline stall, reconfigure the URB and push constant space, and
 emit a bunch of state.  Most clears aren't layered, so the GS isn't
 needed in the common case.  But we turned it on anyway.

 As far as I know, most GPUs that support layered rendering can support
 the GL_AMD_vertex_shader_layer extension.  i965 does, and it's also the
 only driver using this path that does layered rendering.  Doing so
 allows us to skip setting up the GS.

 Signed-off-by: Kenneth Graunke kenn...@whitecape.org
 ---
  src/mesa/drivers/common/meta.c | 53 
 +-
  1 file changed, 16 insertions(+), 37 deletions(-)

 diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
 index cab0dd8..a4634a3 100644
 --- a/src/mesa/drivers/common/meta.c
 +++ b/src/mesa/drivers/common/meta.c
 @@ -1515,23 +1515,15 @@ static void
  meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)
  {
 const char *vs_source =
 +  #extension GL_AMD_vertex_shader_layer : enable\n
attribute vec4 position;\n
 -  void main()\n
 -  {\n
 - gl_Position = position;\n
 -  }\n;
 -   const char *gs_source =
 -  #version 150\n
 -  layout(triangles) in;\n
 -  layout(triangle_strip, max_vertices = 4) out;\n
uniform int layer;\n
void main()\n
{\n
 -for (int i = 0; i  3; i++) {\n
 -  gl_Layer = layer;\n
 -  gl_Position = gl_in[i].gl_Position;\n
 -  EmitVertex();\n
 -}\n
 +  #ifdef GL_AMD_vertex_shader_layer\n
 + gl_Layer = layer;\n
 +  #endif\n
 + gl_Position = position;\n
}\n;
 const char *fs_source =
uniform vec4 color;\n
 @@ -1539,7 +1531,7 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
{\n
   gl_FragColor = color;\n
}\n;
 -   GLuint vs, gs = 0, fs;
 +   GLuint vs, fs;
 bool has_integer_textures;

 _mesa_meta_setup_vertex_objects(clear-VAO, clear-VBO, true, 3, 0, 0);
 @@ -1551,12 +1543,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
 _mesa_ShaderSource(vs, 1, vs_source, NULL);
 _mesa_CompileShader(vs);

 -   if (_mesa_has_geometry_shaders(ctx)) {
 -  gs = _mesa_CreateShader(GL_GEOMETRY_SHADER);
 -  _mesa_ShaderSource(gs, 1, gs_source, NULL);
 -  _mesa_CompileShader(gs);
 -   }
 -
 fs = _mesa_CreateShader(GL_FRAGMENT_SHADER);
 _mesa_ShaderSource(fs, 1, fs_source, NULL);
 _mesa_CompileShader(fs);
 @@ -1564,20 +1550,14 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
 clear-ShaderProg = _mesa_CreateProgram();
 _mesa_AttachShader(clear-ShaderProg, fs);
 _mesa_DeleteShader(fs);
 -   if (gs != 0)
 -  _mesa_AttachShader(clear-ShaderProg, gs);
 _mesa_AttachShader(clear-ShaderProg, vs);
 _mesa_DeleteShader(vs);
 _mesa_BindAttribLocation(clear-ShaderProg, 0, position);
 _mesa_ObjectLabel(GL_PROGRAM, clear-ShaderProg, -1, meta clear);
 _mesa_LinkProgram(clear-ShaderProg);

 -   clear-ColorLocation = _mesa_GetUniformLocation(clear-ShaderProg,
 - color);
 -   if (gs != 0) {
 -  clear-LayerLocation = _mesa_GetUniformLocation(clear-ShaderProg,
 - layer);
 -   }
 +   clear-ColorLocation = _mesa_GetUniformLocation(clear-ShaderProg, 
 color);
 +   clear-LayerLocation = _mesa_GetUniformLocation(clear-ShaderProg, 
 layer);

 has_integer_textures = _mesa_is_gles3(ctx) ||
(_mesa_is_desktop_gl(ctx)  ctx-Const.GLSLVersion = 130);
 @@ -1587,9 +1567,14 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
const char *vs_int_source =
   ralloc_asprintf(shader_source_mem_ctx,
   #version 130\n
 + #extension GL_AMD_vertex_shader_layer : enable\n
   in vec4 position;\n
 + uniform int layer;\n
   void main()\n
   {\n
 + #ifdef GL_AMD_vertex_shader_layer\n
 +gl_Layer = layer;\n
 + #endif\n
  gl_Position = position;\n
   }\n);
const char *fs_int_source =
 @@ -1612,8 +1597,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
clear-IntegerShaderProg = _mesa_CreateProgram();
_mesa_AttachShader(clear-IntegerShaderProg, fs);
_mesa_DeleteShader(fs);
 -  if (gs != 0)
 - _mesa_AttachShader(clear-IntegerShaderProg, gs);

Re: [Mesa-dev] [PATCH] meta: Use AMD_vertex_shader_layer instead of a GS for layered clears.

2014-06-23 Thread Ilia Mirkin 
On Mon, Jun 23, 2014 at 1:27 AM, Kenneth Graunke kenn...@whitecape.org wrote:
 On i965, enabling and disabling the GS is not free: you have to do a
 full pipeline stall, reconfigure the URB and push constant space, and
 emit a bunch of state.  Most clears aren't layered, so the GS isn't
 needed in the common case.  But we turned it on anyway.

 As far as I know, most GPUs that support layered rendering can support
 the GL_AMD_vertex_shader_layer extension.  i965 does, and it's also the

Not that this matters for meta, but FTR, NVIDIA GPU's don't support
AMD_vertex_shader_layer.

 only driver using this path that does layered rendering.  Doing so
 allows us to skip setting up the GS.

 Signed-off-by: Kenneth Graunke kenn...@whitecape.org
 ---
  src/mesa/drivers/common/meta.c | 53 
 +-
  1 file changed, 16 insertions(+), 37 deletions(-)

 diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
 index cab0dd8..a4634a3 100644
 --- a/src/mesa/drivers/common/meta.c
 +++ b/src/mesa/drivers/common/meta.c
 @@ -1515,23 +1515,15 @@ static void
  meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)
  {
 const char *vs_source =
 +  #extension GL_AMD_vertex_shader_layer : enable\n
attribute vec4 position;\n
 -  void main()\n
 -  {\n
 - gl_Position = position;\n
 -  }\n;
 -   const char *gs_source =
 -  #version 150\n
 -  layout(triangles) in;\n
 -  layout(triangle_strip, max_vertices = 4) out;\n
uniform int layer;\n
void main()\n
{\n
 -for (int i = 0; i  3; i++) {\n
 -  gl_Layer = layer;\n
 -  gl_Position = gl_in[i].gl_Position;\n
 -  EmitVertex();\n
 -}\n
 +  #ifdef GL_AMD_vertex_shader_layer\n
 + gl_Layer = layer;\n
 +  #endif\n
 + gl_Position = position;\n
}\n;
 const char *fs_source =
uniform vec4 color;\n
 @@ -1539,7 +1531,7 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
{\n
   gl_FragColor = color;\n
}\n;
 -   GLuint vs, gs = 0, fs;
 +   GLuint vs, fs;
 bool has_integer_textures;

 _mesa_meta_setup_vertex_objects(clear-VAO, clear-VBO, true, 3, 0, 0);
 @@ -1551,12 +1543,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
 _mesa_ShaderSource(vs, 1, vs_source, NULL);
 _mesa_CompileShader(vs);

 -   if (_mesa_has_geometry_shaders(ctx)) {
 -  gs = _mesa_CreateShader(GL_GEOMETRY_SHADER);
 -  _mesa_ShaderSource(gs, 1, gs_source, NULL);
 -  _mesa_CompileShader(gs);
 -   }
 -
 fs = _mesa_CreateShader(GL_FRAGMENT_SHADER);
 _mesa_ShaderSource(fs, 1, fs_source, NULL);
 _mesa_CompileShader(fs);
 @@ -1564,20 +1550,14 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
 clear-ShaderProg = _mesa_CreateProgram();
 _mesa_AttachShader(clear-ShaderProg, fs);
 _mesa_DeleteShader(fs);
 -   if (gs != 0)
 -  _mesa_AttachShader(clear-ShaderProg, gs);
 _mesa_AttachShader(clear-ShaderProg, vs);
 _mesa_DeleteShader(vs);
 _mesa_BindAttribLocation(clear-ShaderProg, 0, position);
 _mesa_ObjectLabel(GL_PROGRAM, clear-ShaderProg, -1, meta clear);
 _mesa_LinkProgram(clear-ShaderProg);

 -   clear-ColorLocation = _mesa_GetUniformLocation(clear-ShaderProg,
 - color);
 -   if (gs != 0) {
 -  clear-LayerLocation = _mesa_GetUniformLocation(clear-ShaderProg,
 - layer);
 -   }
 +   clear-ColorLocation = _mesa_GetUniformLocation(clear-ShaderProg, 
 color);
 +   clear-LayerLocation = _mesa_GetUniformLocation(clear-ShaderProg, 
 layer);

 has_integer_textures = _mesa_is_gles3(ctx) ||
(_mesa_is_desktop_gl(ctx)  ctx-Const.GLSLVersion = 130);
 @@ -1587,9 +1567,14 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
const char *vs_int_source =
   ralloc_asprintf(shader_source_mem_ctx,
   #version 130\n
 + #extension GL_AMD_vertex_shader_layer : enable\n
   in vec4 position;\n
 + uniform int layer;\n
   void main()\n
   {\n
 + #ifdef GL_AMD_vertex_shader_layer\n
 +gl_Layer = layer;\n
 + #endif\n
  gl_Position = position;\n
   }\n);
const char *fs_int_source =
 @@ -1612,8 +1597,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
 clear_state *clear)
clear-IntegerShaderProg = _mesa_CreateProgram();
_mesa_AttachShader(clear-IntegerShaderProg, fs);
_mesa_DeleteShader(fs);
 -  if (gs != 0)
 - _mesa_AttachShader(clear-IntegerShaderProg, gs);

[Mesa-dev] [PATCH] meta: Use AMD_vertex_shader_layer instead of a GS for layered clears.

2014-06-22 Thread Kenneth Graunke 
On i965, enabling and disabling the GS is not free: you have to do a
full pipeline stall, reconfigure the URB and push constant space, and
emit a bunch of state.  Most clears aren't layered, so the GS isn't
needed in the common case.  But we turned it on anyway.

As far as I know, most GPUs that support layered rendering can support
the GL_AMD_vertex_shader_layer extension.  i965 does, and it's also the
only driver using this path that does layered rendering.  Doing so
allows us to skip setting up the GS.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/common/meta.c | 53 +-
 1 file changed, 16 insertions(+), 37 deletions(-)

diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index cab0dd8..a4634a3 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -1515,23 +1515,15 @@ static void
 meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)
 {
const char *vs_source =
+  #extension GL_AMD_vertex_shader_layer : enable\n
   attribute vec4 position;\n
-  void main()\n
-  {\n
- gl_Position = position;\n
-  }\n;
-   const char *gs_source =
-  #version 150\n
-  layout(triangles) in;\n
-  layout(triangle_strip, max_vertices = 4) out;\n
   uniform int layer;\n
   void main()\n
   {\n
-for (int i = 0; i  3; i++) {\n
-  gl_Layer = layer;\n
-  gl_Position = gl_in[i].gl_Position;\n
-  EmitVertex();\n
-}\n
+  #ifdef GL_AMD_vertex_shader_layer\n
+ gl_Layer = layer;\n
+  #endif\n
+ gl_Position = position;\n
   }\n;
const char *fs_source =
   uniform vec4 color;\n
@@ -1539,7 +1531,7 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
clear_state *clear)
   {\n
  gl_FragColor = color;\n
   }\n;
-   GLuint vs, gs = 0, fs;
+   GLuint vs, fs;
bool has_integer_textures;
 
_mesa_meta_setup_vertex_objects(clear-VAO, clear-VBO, true, 3, 0, 0);
@@ -1551,12 +1543,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
clear_state *clear)
_mesa_ShaderSource(vs, 1, vs_source, NULL);
_mesa_CompileShader(vs);
 
-   if (_mesa_has_geometry_shaders(ctx)) {
-  gs = _mesa_CreateShader(GL_GEOMETRY_SHADER);
-  _mesa_ShaderSource(gs, 1, gs_source, NULL);
-  _mesa_CompileShader(gs);
-   }
-
fs = _mesa_CreateShader(GL_FRAGMENT_SHADER);
_mesa_ShaderSource(fs, 1, fs_source, NULL);
_mesa_CompileShader(fs);
@@ -1564,20 +1550,14 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
clear_state *clear)
clear-ShaderProg = _mesa_CreateProgram();
_mesa_AttachShader(clear-ShaderProg, fs);
_mesa_DeleteShader(fs);
-   if (gs != 0)
-  _mesa_AttachShader(clear-ShaderProg, gs);
_mesa_AttachShader(clear-ShaderProg, vs);
_mesa_DeleteShader(vs);
_mesa_BindAttribLocation(clear-ShaderProg, 0, position);
_mesa_ObjectLabel(GL_PROGRAM, clear-ShaderProg, -1, meta clear);
_mesa_LinkProgram(clear-ShaderProg);
 
-   clear-ColorLocation = _mesa_GetUniformLocation(clear-ShaderProg,
- color);
-   if (gs != 0) {
-  clear-LayerLocation = _mesa_GetUniformLocation(clear-ShaderProg,
- layer);
-   }
+   clear-ColorLocation = _mesa_GetUniformLocation(clear-ShaderProg, color);
+   clear-LayerLocation = _mesa_GetUniformLocation(clear-ShaderProg, layer);
 
has_integer_textures = _mesa_is_gles3(ctx) ||
   (_mesa_is_desktop_gl(ctx)  ctx-Const.GLSLVersion = 130);
@@ -1587,9 +1567,14 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
clear_state *clear)
   const char *vs_int_source =
  ralloc_asprintf(shader_source_mem_ctx,
  #version 130\n
+ #extension GL_AMD_vertex_shader_layer : enable\n
  in vec4 position;\n
+ uniform int layer;\n
  void main()\n
  {\n
+ #ifdef GL_AMD_vertex_shader_layer\n
+gl_Layer = layer;\n
+ #endif\n
 gl_Position = position;\n
  }\n);
   const char *fs_int_source =
@@ -1612,8 +1597,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
clear_state *clear)
   clear-IntegerShaderProg = _mesa_CreateProgram();
   _mesa_AttachShader(clear-IntegerShaderProg, fs);
   _mesa_DeleteShader(fs);
-  if (gs != 0)
- _mesa_AttachShader(clear-IntegerShaderProg, gs);
   _mesa_AttachShader(clear-IntegerShaderProg, vs);
   _mesa_DeleteShader(vs);
   _mesa_BindAttribLocation(clear-IntegerShaderProg, 0, position);
@@ -1629,13 +1612,9 @@ meta_glsl_clear_init(struct gl_context *ctx, struct 
clear_state *clear)
 
   clear-IntegerColorLocation =