[Mesa-dev] [PATCH 3/3] meta: Avoid shader recompilation for msaa color resolve blit

Jason Ekstrand jason at jlekstrand.net
Thu Aug 28 19:31:17 PDT 2014


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.

--Jason Ekstrand
On Aug 28, 2014 4:48 PM, "Anuj Phogat" <anuj.phogat at gmail.com> 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 <anuj.phogat at gmail.com>
>
> ---
> 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 <anuj.phogat at gmail.com>
> ---
>  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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140828/d29e0101/attachment-0001.html>


More information about the mesa-dev mailing list