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