On Thu 28 May 2015, Ben Widawsky wrote:
> This restriction was attempted in this commit:
> commit 47053464630888f819ef8cc44278f1a1220159b9
> Author: Anuj Phogat <anuj.pho...@gmail.com>
> Date: Fri Feb 13 11:21:21 2015 -0800
>
> i965/gen8: Use HALIGN_16 if MCS is enabled for non-MSRT
>
> However, the commit itself doesn't achieve the desired goal as determined by
> the
> asserts which the next patch adds. mcs_mt is never a value because we're in
> the
> process of allocating the mcs_mt miptree when we get to this function. I
> didn't
> check, but perhaps this would work with blorp, however, meta clears allocate
> the
> miptree structure (which AFAICT needs the alignment also) way before it
> allocates using meta clears where the renderbuffer is allocated way before the
> aux buffer.
>
> The restriction is referenced in a few places, but the most concise one [IMO]
> from the spec is for Gen9. Gen8 8 loosens the restriction in that it only
> requires this for non-msrt surface.
>
> When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN 16
> must
> be used.
>
> With the code before the miptree layout flag rework (patches preceding this),
> accomplishing this workaround is very difficult.
>
> Cc: Anuj Phogat <anuj.pho...@gmail.com>
> Cc: Neil Roberts <n...@linux.intel.com>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/brw_tex_layout.c | 16 ++++++++++------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 ++++++++++++---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 4 +++-
> 3 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 72b02a2..b51c7c7 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -41,8 +41,13 @@
>
> static unsigned int
> intel_horizontal_texture_alignment_unit(struct brw_context *brw,
> - struct intel_mipmap_tree *mt)
> + struct intel_mipmap_tree *mt,
> + uint32_t layout_flags)
> {
> +
> + if (layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16)
> + return 16;
> +
This snippet makes sense. If the force flag is set, then don't bother
inspecting any other state.
> /**
> * From the "Alignment Unit Size" section of various specs, namely:
> * - Gen3 Spec: "Memory Data Formats" Volume, Section 1.20.1.4
> @@ -91,9 +96,6 @@ intel_horizontal_texture_alignment_unit(struct brw_context
> *brw,
> if (brw->gen >= 7 && mt->format == MESA_FORMAT_Z_UNORM16)
> return 8;
>
> - if (brw->gen == 8 && mt->mcs_mt && mt->num_samples <= 1)
> - return 16;
> -
> return 4;
> }
>
> @@ -459,7 +461,8 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
> }
>
> void
> -brw_miptree_layout(struct brw_context *brw, struct intel_mipmap_tree *mt)
> +brw_miptree_layout(struct brw_context *brw, struct intel_mipmap_tree *mt,
> + uint32_t layout_flags)
> {
> bool multisampled = mt->num_samples > 1;
> bool gen6_hiz_or_stencil = false;
> @@ -492,8 +495,9 @@ brw_miptree_layout(struct brw_context *brw, struct
> intel_mipmap_tree *mt)
> mt->align_w = 128 / mt->cpp;
> mt->align_h = 32;
> }
> + assert((layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0);
This assertion makes sense.
> } else {
> - mt->align_w = intel_horizontal_texture_alignment_unit(brw, mt);
> + mt->align_w = intel_horizontal_texture_alignment_unit(brw, mt,
> layout_flags);
> mt->align_h =
> intel_vertical_texture_alignment_unit(brw, mt->format,
> multisampled);
> }
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 75ee19a..a1ac0cf 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -475,7 +475,10 @@ intel_miptree_create_layout(struct brw_context *brw,
> if (layout_flags & MIPTREE_LAYOUT_FORCE_ALL_SLICE_AT_LOD)
> mt->array_layout = ALL_SLICES_AT_EACH_LOD;
>
> - brw_miptree_layout(brw, mt);
> + if (intel_miptree_is_fast_clear_capable(brw, mt))
> + layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
> +
> + brw_miptree_layout(brw, mt, layout_flags);
This does not make sense to me. I though HALIGN16 didn't exist before
Skylake, but here you're setting it on Ivybridge through Broadwell too.
Am I misunderstanding something?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
