On Tue 13 Oct 2015, Ben Widawsky wrote:
> Initially I had this planned as a patch to be squashed in to the enabling
> patch
> because there is no point enabling fast clears without this. However, Chad
> merged a patch which disables fast clears on gen9 explicitly, and so I can
> hide
> this behind the revert of that patch. This is a nice I really wanted this
> patch
> as a distinct patch for review. This is a new, weird, and poorly documented
> restriction for SKL. (In fact, I am still not 100% certain the restriction is
> entirely necessary, but there are around 30 piglit regressions without this).
>
> SKL adds compressible render targets and as a result mutates some of the
> programming for fast clears and resolves. There is a new internal surface type
> called the CCS. The old AUX_MCS bit becomes AUX_CCS_D. "The Auxiliary surface
> is
> a CCS (Color Control Surface) with compression disabled or an MCS with
> compression enabled, depending on number of multisamples. MCS (Multisample
> Control Surface) is a special type of CCS."
>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/brw_context.h | 1 +
> src/mesa/drivers/dri/i965/brw_surface_formats.c | 27
> +++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/gen8_surface_state.c | 8 ++++++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 3 +++
> 4 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index e59478a..32b8250 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1546,6 +1546,7 @@ struct brw_context
>
> uint32_t render_target_format[MESA_FORMAT_COUNT];
> bool format_supported_as_render_target[MESA_FORMAT_COUNT];
> + bool losslessly_compressable[MESA_FORMAT_COUNT];
I agree with Neil. It's a shame to increase the context size for static
information. And there is already a static array in
brw_surface_formats.c for exactly this type of information.
>
> /* Interpolation modes, one byte per vue slot.
> * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 97fff60..d706ecc 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -693,6 +693,33 @@ brw_init_surface_formats(struct brw_context *brw)
> }
> }
>
> + if (brw->gen >= 9) {
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_UNORM16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_SNORM16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_FLOAT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RG_SINT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RG_UINT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBX_FLOAT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_B8G8R8A8_UNORM] = true;
> + brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_UNORM] = true;
> + brw->losslessly_compressable[MESA_FORMAT_R8G8B8A8_SNORM] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_SINT8] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RGBA_UINT8] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RG_SINT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RG_UINT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_RG_FLOAT16] = true;
> + brw->losslessly_compressable[MESA_FORMAT_R_UINT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_R_SINT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_R_FLOAT32] = true;
> + brw->losslessly_compressable[MESA_FORMAT_B8G8R8X8_UNORM] = true;
> + }
Properties of surface formats should go into the monster table that
occurs earlier in the file. Then you can replace
brw_context::losslessly_compressable with a query and keep brw_context
at its current size.
> +
> /* We will check this table for FBO completeness, but the surface format
> * table above only covered color rendering.
> */
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 995b4dd..b19b492 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -243,8 +243,10 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E,
> HALIGN
> * 16 must be used."
> */
> - if (brw->gen >= 9 || mt->num_samples == 1)
> + if (brw->gen >= 9 || mt->num_samples == 1) {
> assert(mt->halign == 16);
> + assert(mt->num_samples || brw->losslessly_compressable[mt->format]
> == true);
> + }
> }
>
> const uint32_t surf_type = translate_tex_target(target);
> @@ -488,8 +490,10 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
> * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E,
> HALIGN
> * 16 must be used."
> */
> - if (brw->gen >= 9 || mt->num_samples == 1)
> + if (brw->gen >= 9 || mt->num_samples == 1) {
> assert(mt->halign == 16);
> + assert(mt->num_samples || brw->losslessly_compressable[mt->format]
> == true);
This assert is wrong. It's perfectly safe to render into a format that
is not losslessly compressable, even when it has a MCS.
> + }
> }
>
> uint32_t *surf = allocate_surface_state(brw, &offset, surf_index);
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 32f0ff7..f108b75 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -265,6 +265,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct
> brw_context *brw,
> if (!brw->format_supported_as_render_target[mt->format])
> return false;
>
> + if (brw->gen >= 9 && !brw->losslessly_compressable[mt->format])
> + return false;
> +
This hunk needs a comment that explains why you're disabling 1x fast
clears for these formats.
> return true;
> }
>
> --
> 2.6.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
