[Mesa-dev] [PATCH 3/3] Revert "i965: Disable regular fast-clears (CCS_D) on gen9+"

Eero Tamminen eero.t.tamminen at intel.com
Thu Dec 14 13:53:12 UTC 2017


Hi,

As expected, this series fixes the perf regression in GfxBench when fast 
clears were disabled.  On SKL GT2:
* 2-5% Manhattan 3.1
* 1% AztecRuins & CarChase (on top of Francisco's large improvement 
between the perf regression and this fix)

On 14.12.2017 03:54, Jason Ekstrand wrote:
> Better commit message:
> 
>      Re-enable regular fast-clears (CCS_D) on gen9+
> 
>      This reverts commit ee57b15ec764736e2d5360beaef9fb2045ed0f68, "i965:
>      Disable regular fast-clears (CCS_D) on gen9+".  How taht we've 
> fixed the
>      issue with too many different aux usages in the render cache, it should
>      be safe to re-enable CCS_D for sRGB.

* s/How taht/Now that/

* Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=104163

* Tested-by: Eero Tamminen <eero.t.tamminen at intel.com>


	- Eero


> On Wed, Dec 13, 2017 at 5:52 PM, Jason Ekstrand <jason at jlekstrand.net 
> <mailto:jason at jlekstrand.net>> wrote:
> 
>     This reverts commit ee57b15ec764736e2d5360beaef9fb2045ed0f68.
> 
>     Cc: "17.3" <mesa-stable at lists.freedesktop.org
>     <mailto:mesa-stable at lists.freedesktop.org>>
>     ---
>       src/mesa/drivers/dri/i965/brw_meta_util.c     | 10 -----
>       src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 57
>     ++++++++++++---------------
>       2 files changed, 25 insertions(+), 42 deletions(-)
> 
>     diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
>     b/src/mesa/drivers/dri/i965/brw_meta_util.c
>     index 54dc6a5..b311815 100644
>     --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
>     +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
>     @@ -293,17 +293,7 @@ brw_is_color_fast_clear_compatible(struct
>     brw_context *brw,
>              brw->mesa_to_isl_render_format[mt->format])
>             return false;
> 
>     -   /* Gen9 doesn't support fast clear on single-sampled SRGB
>     buffers. When
>     -    * GL_FRAMEBUFFER_SRGB is enabled any color renderbuffers will be
>     -    * resolved in intel_update_state. In that case it's pointless
>     to do a
>     -    * fast clear because it's very likely to be immediately resolved.
>     -    */
>          const bool srgb_rb = _mesa_get_srgb_format_linear(mt->format)
>     != mt->format;
>     -   if (devinfo->gen >= 9 &&
>     -       mt->surf.samples == 1 &&
>     -       ctx->Color.sRGBEnabled && srgb_rb)
>     -      return false;
>     -
>         /* Gen10 doesn't automatically decode the clear color of sRGB
>     buffers. Since
>          * we currently don't perform this decode in software, avoid a
>     fast-clear
>          * altogether. TODO: Do this in software.
>     diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>     b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>     index c1a4ce1..b87d356 100644
>     --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>     +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>     @@ -207,13 +207,7 @@ intel_miptree_supports_ccs(struct brw_context *brw,
>          if (!brw->mesa_format_supports_render[mt->format])
>             return false;
> 
>     -   if (devinfo->gen >= 9) {
>     -      mesa_format linear_format =
>     _mesa_get_srgb_format_linear(mt->format);
>     -      const enum isl_format isl_format =
>     -         brw_isl_format_for_mesa_format(linear_format);
>     -      return isl_format_supports_ccs_e(&brw->screen->devinfo,
>     isl_format);
>     -   } else
>     -      return true;
>     +   return true;
>       }
> 
>       static bool
>     @@ -256,7 +250,7 @@ intel_miptree_supports_hiz(const struct
>     brw_context *brw,
>        * our HW tends to support more linear formats than sRGB ones, we
>     use this
>        * format variant for check for CCS_E compatibility.
>        */
>     -MAYBE_UNUSED static bool
>     +static bool
>       format_ccs_e_compat_with_miptree(const struct gen_device_info
>     *devinfo,
>                                        const struct intel_mipmap_tree *mt,
>                                        enum isl_format access_format)
>     @@ -290,12 +284,13 @@ intel_miptree_supports_ccs_e(struct
>     brw_context *brw,
>          if (!intel_miptree_supports_ccs(brw, mt))
>             return false;
> 
>     -   /* Fast clear can be also used to clear srgb surfaces by using
>     equivalent
>     -    * linear format. This trick, however, can't be extended to be
>     used with
>     -    * lossless compression and therefore a check is needed to see
>     if the format
>     -    * really is linear.
>     +   /* Many window system buffers are sRGB even if they are never
>     rendered as
>     +    * sRGB.  For those, we want CCS_E for when sRGBEncode is
>     false.  When the
>     +    * surface is used as sRGB, we fall back to CCS_D.
>           */
>     -   return _mesa_get_srgb_format_linear(mt->format) == mt->format;
>     +   mesa_format linear_format =
>     _mesa_get_srgb_format_linear(mt->format);
>     +   enum isl_format isl_format =
>     brw_isl_format_for_mesa_format(linear_format);
>     +   return isl_format_supports_ccs_e(&brw->screen->devinfo, isl_format);
>       }
> 
>       /**
>     @@ -2686,29 +2681,27 @@ intel_miptree_render_aux_usage(struct
>     brw_context *brw,
>             return ISL_AUX_USAGE_MCS;
> 
>          case ISL_AUX_USAGE_CCS_D:
>     -      /* If FRAMEBUFFER_SRGB is used on Gen9+ then we need to
>     resolve any of
>     -       * the single-sampled color renderbuffers because the CCS
>     buffer isn't
>     -       * supported for SRGB formats. This only matters if
>     FRAMEBUFFER_SRGB is
>     -       * enabled because otherwise the surface state will be
>     programmed with
>     -       * the linear equivalent format anyway.
>     -       */
>     -      if (isl_format_is_srgb(render_format) &&
>     -          _mesa_get_srgb_format_linear(mt->format) != mt->format) {
>     -         return ISL_AUX_USAGE_NONE;
>     -      } else if (!mt->mcs_buf) {
>     -         return ISL_AUX_USAGE_NONE;
>     -      } else {
>     -         return ISL_AUX_USAGE_CCS_D;
>     -      }
>     +      return mt->mcs_buf ? ISL_AUX_USAGE_CCS_D : ISL_AUX_USAGE_NONE;
> 
>          case ISL_AUX_USAGE_CCS_E: {
>     -      /* Lossless compression is not supported for SRGB formats, it
>     -       * should be impossible to get here with such surfaces.
>     +      /* If the format supports CCS_E and is compatible with the
>     miptree,
>     +       * then we can use it.
>              */
>     -      assert(!isl_format_is_srgb(render_format) ||
>     -             _mesa_get_srgb_format_linear(mt->format) == mt->format);
>     +      if (format_ccs_e_compat_with_miptree(&brw->screen->devinfo,
>     +                                           mt, render_format))
>     +         return ISL_AUX_USAGE_CCS_E;
>     +
>     +      /* Otherwise, we have to fall back to CCS_D */
>     +
>     +      /* gen9 hardware technically supports non-0/1 clear colors
>     with sRGB
>     +       * formats.  However, there are issues with blending where it
>     doesn't
>     +       * properly apply the sRGB curve to the clear color when
>     blending.
>     +       */
>     +      if (blend_enabled && isl_format_is_srgb(render_format) &&
>     +          !isl_color_value_is_zero_one(mt->fast_clear_color,
>     render_format))
>     +         return ISL_AUX_USAGE_NONE;
> 
>     -      return ISL_AUX_USAGE_CCS_E;
>     +      return ISL_AUX_USAGE_CCS_D;
>          }
> 
>          default:
>     --
>     2.5.0.400.gff86faf
> 
> 
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 



More information about the mesa-dev mailing list