[Mesa-dev] [PATCH] i965/miptree: Refactor CCS_E and CCS_D cases in render_aux_usage

[Nanley Chery]

On Sun, Dec 17, 2017 at 08:03:45PM -0800, Jason Ekstrand wrote:
> This commit unifies the CCS_E and CCS_D cases.  This should fix a couple
> of subtle issues.  One is that when you use INTEL_DEBUG=norbc to disable
> CCS_E, we don't get the sRGB blending workaround.  By unifying the code,
> we give CCS_D that workaround as well.
> 
> The second issue fixed by this refactor is that the blending workaround
> was appears to be enabled on all gens but really only applies on gen9.
> Due to a happy accident in the way code was laid out, it was only
> getting enabled on gen9: gen8 and earlier don't support non-zero-one
> clear colors, and gen10 supports sRGB for CCS_E so it got caught in the
> format_ccs_e_compat_with_miptree case.  This refactor moves it above the
> format_ccs_e_compat_with_miptree case so it's an explicit early exit and
> makes it explicitly only on gen9.
> 
> Cc: "17.3" <mesa-stable at lists.freedesktop.org>
> Cc: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 

Thanks for making the workaround gen9 specific. This patch is
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>

> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 47bfdf5..bbcc2a8 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2684,28 +2684,28 @@ intel_miptree_render_aux_usage(struct brw_context *brw,
>        return ISL_AUX_USAGE_MCS;
>  
>     case ISL_AUX_USAGE_CCS_D:
> -      return mt->mcs_buf ? ISL_AUX_USAGE_CCS_D : ISL_AUX_USAGE_NONE;
> -
> -   case ISL_AUX_USAGE_CCS_E: {
> -      /* If the format supports CCS_E and is compatible with the miptree,
> -       * then we can use it.
> -       */
> -      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 */
> +   case ISL_AUX_USAGE_CCS_E:
> +      if (!mt->mcs_buf) {
> +         assert(mt->aux_usage == ISL_AUX_USAGE_CCS_D);
> +         return ISL_AUX_USAGE_NONE;
> +      }
>  
>        /* 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) &&
> +      if (brw->devinfo->gen == 9 && 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;
>  
> +      if (mt->aux_usage == ISL_AUX_USAGE_CCS_E &&
> +          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 */
>        return ISL_AUX_USAGE_CCS_D;
> -   }
>  
>     default:
>        return ISL_AUX_USAGE_NONE;
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev