[Mesa-dev] [PATCH 2/2] anv: Enable MSAA compression

[Jason Ekstrand]

On Feb 20, 2017 4:47 AM, "Lionel Landwerlin" <lionel.g.landwerlin at intel.com>
wrote:

On 18/02/17 00:03, Jason Ekstrand wrote:

> This just enables basic MSAA compression (no fast clears) for all
> multisampled surfaces.  This improves the framerate of the Sascha
> "multisampling" demo by 76% on my Sky Lake laptop.  Running Talos on
> medium settings with 8x MSAA, this improves the framerate in the
> benchmark by 80%.
> ---
>   src/intel/vulkan/TODO              |  2 +-
>   src/intel/vulkan/anv_blorp.c       |  3 ++-
>   src/intel/vulkan/anv_image.c       |  8 ++++++++
>   src/intel/vulkan/anv_pipeline.c    | 14 ++++++++++++++
>   src/intel/vulkan/genX_cmd_buffer.c |  5 +++++
>   5 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/TODO b/src/intel/vulkan/TODO
> index f8b73a1..daab39f 100644
> --- a/src/intel/vulkan/TODO
> +++ b/src/intel/vulkan/TODO
> @@ -9,7 +9,7 @@ Missing Features:
>     Performance:
>    - Multi-{sampled/gen8,LOD} HiZ
> - - Compressed multisample support
> + - MSAA fast clears
>    - Pushing pieces of UBOs?
>    - Enable guardband clipping
>    - Use soft-pin to avoid relocations
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 4e7078b..902d9af 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1397,7 +1397,8 @@ ccs_resolve_attachment(struct anv_cmd_buffer
> *cmd_buffer,
>      struct anv_attachment_state *att_state =
>         &cmd_buffer->state.attachments[att];
>   -   if (att_state->aux_usage == ISL_AUX_USAGE_NONE)
> +   if (att_state->aux_usage == ISL_AUX_USAGE_NONE ||
> +       att_state->aux_usage == ISL_AUX_USAGE_MCS)
>         return; /* Nothing to resolve */
>        assert(att_state->aux_usage == ISL_AUX_USAGE_CCS_E ||
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 7eb0f8f..cc47a50 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -238,6 +238,14 @@ make_surface(const struct anv_device *dev,
>               }
>            }
>         }
> +   } else if (aspect == VK_IMAGE_ASPECT_COLOR_BIT && vk_info->samples >
> 1) {
> +      assert(image->aux_surface.isl.size == 0);
> +      assert(!(vk_info->usage & VK_IMAGE_USAGE_STORAGE_BIT));
> +      ok = isl_surf_get_mcs_surf(&dev->isl_dev, &anv_surf->isl,
> +                                 &image->aux_surface.isl);
> +      assert(ok);
>

Overall looks good to me, but I think you should drop the assert here.
Looking at the internal documentation I found a 8k width constraint at 16x
msaa (look at SURFACE_STATE's width field, only applies SKL+).
So I guess we fallback to not use the MCS in that case.


Good catch.  Unfortunately, this means it's going to take just a little
more work for is to support compression.  Part of what makes this patch so
simple is the assumption that we can just turn it on all the time.  If that
isn't a valid assumption them we may need to make some compiler changes.
I'll have to run some experiments on sky lake.


+      add_surface(image, &image->aux_surface);
> +      image->aux_usage = ISL_AUX_USAGE_MCS;
>      }
>        return VK_SUCCESS;
> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> index 4410103..3301679 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -228,6 +228,20 @@ static void
>   populate_sampler_prog_key(const struct gen_device_info *devinfo,
>                             struct brw_sampler_prog_key_data *key)
>   {
> +   /* All multisampled textures are compressed. */
> +   key->compressed_multisample_layout_mask = ~0;
> +
> +   /* SkyLake added support for 16x MSAA.  With this came a new message
> for
> +    * reading from a 16x MSAA surface with compression.  The new message
> was
> +    * needed because now the MCS data is 64 bits instead of 32 or lower
> as is
> +    * the case for 8x, 4x, and 2x.  The key->msaa_16 bit-field controls
> which
> +    * message we use.  Fortunately, the 16x message works for 8x, 4x, and
> 2x
> +    * so we can just use it unconditionally.  This may not be quite as
> +    * efficient but it saves us from recompiling.
> +    */
> +   if (devinfo->gen >= 9)
> +      key->msaa_16 = ~0;
> +
>      /* XXX: Handle texture swizzle on HSW- */
>      for (int i = 0; i < MAX_SAMPLERS; i++) {
>         /* Assume color sampler, no swizzling. (Works for BDW+) */
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 40a72f4..5d8c3ea 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -222,6 +222,11 @@ color_attachment_compute_aux_usage(struct anv_device
> *device,
>         att_state->input_aux_usage = ISL_AUX_USAGE_NONE;
>         att_state->fast_clear = false;
>         return;
> +   } else if (iview->image->aux_usage == ISL_AUX_USAGE_MCS) {
> +      att_state->aux_usage = ISL_AUX_USAGE_MCS;
> +      att_state->input_aux_usage = ISL_AUX_USAGE_MCS;
> +      att_state->fast_clear = false;
> +      return;
>      }
>        assert(iview->image->aux_surface.isl.usage &
> ISL_SURF_USAGE_CCS_BIT);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170220/db0c8867/attachment-0001.html>