[Mesa-dev] [PATCH 06/16] i965/miptree: Do not rely on msaa type to decide if aux is needed

[Jason Ekstrand]

On Mon, Jul 17, 2017 at 6:34 AM, Topi Pohjolainen <
topi.pohjolainen at gmail.com> wrote:

> Once the driver moves to ISL both compressed and uncompressed have
> the same type. One needs to tell them apart by other means. This
> can be done by checking the existence of mcs_buf.
>
> There is a short period of time within intel_miptree_create()
> where mcs_buf doesn't exist yet (between calls to
> intel_miptree_create_layout() and intel_miptree_alloc_mcs()).
> First compute_msaa_layout() makes the decision if compression is
> to be used and sets the msaa_layout type. Then based on the type
> one sets aux_usage and finally decides if mcs_buf is needed.
>
> This patch duplicates the logic in compute_msaa_layout() and uses
> that to make the decision on aux_usage and mcs_buf allocation.
> Most of the original logic in compute_msaa_layout() will be gone
> in the following patch leaving only one version.
>

Not true since you pulled this patch out for early merging. :-)

That said, I think it's ok to duplicate here.  I'm not sure what we're
going to want all this to look like in the ISLified future anyway.
Probably something like Vulkan where we just call isl_surf_init_mcs and set
aux_layout to NONE if it fails.  In any case, this doesn't seem to make
anything worse.


> Elsewhere only brw_populate_sampler_prog_key_data() needs to know
> if compression is used based on the msaa_type. This is now
> replaced with consideration for number of samples and existence
> of mcs_buf. All other occurrences consider CMS || UMS which can
> be represented using single the type of ISL_MSAA_LAYOUT_ARRAY
> without any tweaks.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_wm.c            |  8 +++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 43
> ++++++++++++++++++++++++++-
>  2 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> b/src/mesa/drivers/dri/i965/brw_wm.c
> index 3a5fcf5485..18056d51d0 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -396,9 +396,11 @@ brw_populate_sampler_prog_key_data(struct gl_context
> *ctx,
>           /* From gen9 onwards some single sampled buffers can also be
>            * compressed. These don't need ld2dms sampling along with mcs
> fetch.
>            */
> -         if (brw->gen >= 7 &&
> -             intel_tex->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS &&
> -             intel_tex->mt->num_samples > 1) {
> +         if (intel_tex->mt->aux_usage == ISL_AUX_USAGE_MCS) {
> +            assert(brw->gen >= 7);
> +            assert(intel_tex->mt->num_samples > 1);
> +            assert(intel_tex->mt->mcs_buf);
> +            assert(intel_tex->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS);
>              key->compressed_multisample_layout_mask |= 1 << s;
>
>              if (intel_tex->mt->num_samples >= 16) {
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 69a9328d6c..bef544c0ae 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -62,6 +62,45 @@ static bool
>  intel_miptree_alloc_aux(struct brw_context *brw,
>                          struct intel_mipmap_tree *mt);
>
> +static bool
> +is_mcs_supported(const struct brw_context *brw, mesa_format format,
> +                 uint32_t layout_flags)
> +{
> +   /* Prior to Gen7, all MSAA surfaces used IMS layout. */
> +   if (brw->gen < 7)
> +      return false;
> +
> +   /* In Gen7, IMS layout is only used for depth and stencil buffers. */
> +   switch (_mesa_get_format_base_format(format)) {
> +   case GL_DEPTH_COMPONENT:
> +   case GL_STENCIL_INDEX:
> +   case GL_DEPTH_STENCIL:
> +      return false;
> +   default:
> +      /* From the Ivy Bridge PRM, Vol4 Part1 p77 ("MCS Enable"):
> +       *
> +       *   This field must be set to 0 for all SINT MSRTs when all RT
> channels
> +       *   are not written
> +       *
> +       * In practice this means that we have to disable MCS for all signed
> +       * integer MSAA buffers.  The alternative, to disable MCS only when
> one
> +       * of the render target channels is disabled, is impractical
> because it
> +       * would require converting between CMS and UMS MSAA layouts on the
> fly,
> +       * which is expensive.
> +       */
> +      if (brw->gen == 7 && _mesa_get_format_datatype(format) == GL_INT) {
> +         return false;
> +      } else if (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) {
> +         /* We can't use the CMS layout because it uses an aux buffer,
> the MCS
> +          * buffer. So fallback to UMS, which is identical to CMS without
> the
> +          * MCS. */
> +         return false;
> +      } else {
> +         return true;
> +      }
> +   }
> +}
> +
>  /**
>   * Determine which MSAA layout should be used by the MSAA surface being
>   * created, based on the chip generation and the surface type.
> @@ -566,7 +605,9 @@ intel_miptree_choose_aux_usage(struct brw_context
> *brw,
>  {
>     assert(mt->aux_usage == ISL_AUX_USAGE_NONE);
>
> -   if (mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) {
> +   const unsigned no_flags = 0;
> +   if (mt->num_samples > 1 && is_mcs_supported(brw, mt->format,
> no_flags)) {
> +      assert(mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS);
>        mt->aux_usage = ISL_AUX_USAGE_MCS;
>     } else if (intel_tiling_supports_ccs(brw, mt->tiling) &&
>                intel_miptree_supports_ccs(brw, mt)) {
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170717/db486988/attachment-0001.html>