On Fri, Aug 29, 2014 at 1:21 PM, Jason Ekstrand <[email protected]> wrote: > Also, if you could make sure that we don't end up with unused stuff in the > blit structure that would be good to. Yes. I'll do that.
> --Jason > > > On Fri, Aug 29, 2014 at 1:16 PM, Anuj Phogat <[email protected]> wrote: >> >> On Fri, Aug 29, 2014 at 11:02 AM, Jason Ekstrand <[email protected]> >> wrote: >> > Other than the comment I made yesterday, this series looks good to me. >> Thanks. I'll add your r-b. >> >> > --Jason Ekstrand >> > >> > >> > On Thu, Aug 28, 2014 at 7:31 PM, Jason Ekstrand <[email protected]> >> > 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. >> Right. I'll squash the patches into one. >> >> >> >> >> --Jason Ekstrand >> >> >> >> On Aug 28, 2014 4:48 PM, "Anuj Phogat" <[email protected]> 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 <[email protected]> >> >>> >> >>> --- >> >>> 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 <[email protected]> >> >>> --- >> >>> 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 >> >>> [email protected] >> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > >> > > > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
