<p dir="ltr">Anuj,<br>
A cursory reading on my phone says these patches should be OK. I'll give it a more thorough look tomorrow when I have the full source in front of me.</p>
<p dir="ltr">One comment though: Is there a good reason this is 3 patches? The first redactors stuff just so you can do a check in the second which you then remove in the third. Why not just make one patch to do what the third one does? In particular, the stuff you added to the blit state in the first patch is left in the blit state but is effectively unused once we get to the third.</p>
<p dir="ltr">--Jason Ekstrand</p>
<div class="gmail_quote">On Aug 28, 2014 4:48 PM, "Anuj Phogat" <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Currently, setup_glsl_msaa_blit_shader() doesn't store compiled<br>
msaa shaders generated for specific sample counts. This causes<br>
the recompilation BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE* and<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE* shaders every<br>
time there is a change in source buffer sample count. This can<br>
hit the performance of an application which continuously changes<br>
sample count of multisample buffer. Unnecessary compilations can<br>
be avoided by storing the compiled shaders for all supported<br>
sample counts.<br>
<br>
Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
<br>
---<br>
This patch continues with the current approach of storing the<br>
compiled shaders in msaa_shaders array. But, as suggested by<br>
Ken, a better approach in the future would be to implement a<br>
shader cache for meta and maintain a program key for each<br>
shader. Any changes to the program key will trigger the<br>
recompilation of the shader. It'll be similar to<br>
brw_blorp_blit_prog_key used for blit shaders in i965 BLORP<br>
backend. I'll keep this task in my todo list but not planning<br>
to pick it up anytime soon. Feel free to take it off my list.<br>
<br>
Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
---<br>
src/mesa/drivers/common/meta.h | 40 +++++++++++++++++++++++++++++--------<br>
src/mesa/drivers/common/meta_blit.c | 37 +++++++++++++++++++++-------------<br>
2 files changed, 55 insertions(+), 22 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h<br>
index 75a869c..d2965b5 100644<br>
--- a/src/mesa/drivers/common/meta.h<br>
+++ b/src/mesa/drivers/common/meta.h<br>
@@ -235,21 +235,45 @@ struct blit_shader_table {<br>
/**<br>
* Indices in the blit_state->msaa_shaders[] array<br>
*<br>
- * Note that setup_glsl_msaa_blit_shader() assumes that the _INT enums are one<br>
- * more than the non-_INT version and _UINT is one beyond that.<br>
+ * Note that setup_glsl_msaa_blit_shader() assumes that the _INT enums are five<br>
+ * more than the corresponding non-_INT versions and _UINT are five beyond that.<br>
*/<br>
enum blit_msaa_shader {<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE,<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT,<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT,<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE,<br>
+ BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE,<br>
+ BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE,<br>
+ BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE,<br>
+ BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE,<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT,<br>
+ BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT,<br>
+ BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT,<br>
+ BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT,<br>
+ BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT,<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT,<br>
+ BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT,<br>
+ BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT,<br>
+ BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT,<br>
+ BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_COPY,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_COPY_INT,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_COPY_UINT,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_DEPTH_RESOLVE,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_DEPTH_COPY,<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE,<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_INT,<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_UINT,<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE,<br>
+ BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE,<br>
+ BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE,<br>
+ BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE,<br>
+ BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE,<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_INT,<br>
+ BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_INT,<br>
+ BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_INT,<br>
+ BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_INT,<br>
+ BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_INT,<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_UINT,<br>
+ BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_UINT,<br>
+ BLIT_4X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_UINT,<br>
+ BLIT_8X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_UINT,<br>
+ BLIT_16X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE_UINT,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_COPY,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_COPY_INT,<br>
BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_COPY_UINT,<br>
diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c<br>
index 1c2d8c1..63cb01a 100644<br>
--- a/src/mesa/drivers/common/meta_blit.c<br>
+++ b/src/mesa/drivers/common/meta_blit.c<br>
@@ -72,6 +72,15 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,<br>
char *name;<br>
const char *texcoord_type = "vec2";<br>
const int samples = MAX2(src_rb->NumSamples, 1);<br>
+ int shader_offset = 0;<br>
+<br>
+ /* We expect only power of 2 samples in source multisample buffer. */<br>
+ assert((samples & (samples - 1)) == 0);<br>
+ while (samples >> (shader_offset + 1)) {<br>
+ shader_offset++;<br>
+ }<br>
+ /* Update the assert if we plan to support more than 16X MSAA. */<br>
+ assert(shader_offset >= 0 && shader_offset <= 4);<br>
<br>
if (src_rb) {<br>
src_datatype = _mesa_get_format_datatype(src_rb->Format);<br>
@@ -109,13 +118,15 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,<br>
} else {<br>
if (dst_is_msaa)<br>
shader_index = BLIT_MSAA_SHADER_2D_MULTISAMPLE_COPY;<br>
- else<br>
- shader_index = BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE;<br>
+ else {<br>
+ shader_index = BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE +<br>
+ shader_offset;<br>
+ }<br>
}<br>
<br>
if (target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) {<br>
- shader_index += (BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE -<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE);<br>
+ shader_index += (BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_RESOLVE -<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE);<br>
sampler_array_suffix = "Array";<br>
texcoord_type = "vec3";<br>
}<br>
@@ -123,19 +134,19 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,<br>
default:<br>
_mesa_problem(ctx, "Unkown texture target %s\n",<br>
_mesa_lookup_enum_by_nr(target));<br>
- shader_index = BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE;<br>
+ shader_index = BLIT_2X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE;<br>
}<br>
<br>
/* We rely on the enum being sorted this way. */<br>
- STATIC_ASSERT(BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT ==<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE + 1);<br>
- STATIC_ASSERT(BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT ==<br>
- BLIT_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE + 2);<br>
+ STATIC_ASSERT(BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_INT ==<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE + 5);<br>
+ STATIC_ASSERT(BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE_UINT ==<br>
+ BLIT_1X_MSAA_SHADER_2D_MULTISAMPLE_RESOLVE + 10);<br>
if (src_datatype == GL_INT) {<br>
- shader_index++;<br>
+ shader_index += 5;<br>
vec4_prefix = "i";<br>
} else if (src_datatype == GL_UNSIGNED_INT) {<br>
- shader_index += 2;<br>
+ shader_index += 10;<br>
vec4_prefix = "u";<br>
} else {<br>
vec4_prefix = "";<br>
@@ -147,9 +158,7 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,<br>
shader_index == BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_DEPTH_COPY ||<br>
shader_index == BLIT_MSAA_SHADER_2D_MULTISAMPLE_DEPTH_COPY;<br>
<br>
- if (blit->msaa_shaders[shader_index] &&<br>
- (is_shader_msaa_depth_resolve_or_copy ||<br>
- blit->samples == samples)) {<br>
+ if (blit->msaa_shaders[shader_index]) {<br>
_mesa_UseProgram(blit->msaa_shaders[shader_index]);<br>
return;<br>
}<br>
--<br>
1.9.3<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div>