[Mesa-dev] [PATCH 12/27] i965/gen6: Calculate stencil offset on demand
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Sat Jan 21 09:19:35 UTC 2017
On Fri, Jan 20, 2017 at 04:35:23PM -0800, Jason Ekstrand wrote:
> On Mon, Jan 16, 2017 at 1:13 AM, Topi Pohjolainen
> <[1]topi.pohjolainen at gmail.com> wrote:
>
> This is kept on purpose in i965. It can be moved to ISL if it
> is needed in vulkan.
> Pointers to miptrees are given solely for verification purposes.
> These will be dropped in following patches.
> Signed-off-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_tex_layout.c | 65
> +++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/gen6_depth_state.c | 14 +++---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 +++
> 3 files changed, 78 insertions(+), 6 deletions(-)
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 768f8a8..80b341a 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -288,6 +288,71 @@ gen9_miptree_layout_1d(struct intel_mipmap_tree
> *mt)
> }
> }
> +static unsigned
> +all_slices_at_each_lod_x_offset(unsigned w0, unsigned align,
> unsigned level)
> +{
> + const unsigned w = level >= 2 ? minify(w0, 1) : 0;
> + return ALIGN(w, align);
> +}
> +
> +static unsigned
> +all_slices_at_each_lod_y_offset(const struct isl_extent4d
> *phys_level0_sa,
> + enum isl_surf_dim dim, unsigned
> align,
> + unsigned level)
> +{
> + unsigned y = 0;
> +
> + /* Add vertical space taken by lower levels one by one. Levels
> one and two
> + * are side-by-side just below level zero. Levels three and
> greater are
> + * stacked one after another below level two.
> + */
> + for (unsigned i = 1; i <= level; ++i) {
> + const unsigned d = dim == ISL_SURF_DIM_3D ?
> + minify(phys_level0_sa->depth, i - 1) :
> + phys_level0_sa->array_len;
> +
> + /* Levels two and greater are stacked just below level zero.
> */
> + if (i != 2) {
> + const unsigned h = minify(phys_level0_sa->height, i - 1);
> + y += d * ALIGN(h, align);
> + }
> + }
> +
> + return y;
> +}
> +
> +uint32_t
> +brw_stencil_all_slices_at_each_lod_offset(const struct isl_surf
> *surf,
> + const struct
> intel_mipmap_tree *mt,
> + unsigned level)
> +{
> + assert(mt->array_layout == ALL_SLICES_AT_EACH_LOD);
> +
> + const unsigned halign = 64;
> + const unsigned valign = 64;
> + const unsigned level_x = all_slices_at_each_lod_x_offset(
> + surf->phys_level0_sa.width, halign, level);
> + const unsigned level_y = all_slices_at_each_lod_y_offset(
> + &surf->phys_level0_sa, surf->dim, valign, level);
> +
> + assert(level_x == mt->level[level].level_x);
> + assert(level_y == mt->level[level].level_y);
> +
> + /* From Vol 2a, 11.5.6.2.1 3DSTATE_STENCIL_BUFFER, field
> "Surface Pitch":
> + * The pitch must be set to 2x the value computed based on
> width, as
> + * the stencil buffer is stored with two rows interleaved.
> + *
> + * While ISL surface stores the pitch expected by hardware, the
> offset
> + * into individual slices needs to be calculated as if rows are
> + * interleaved.
> + */
> + const unsigned two_rows_interleaved_pitch = surf->row_pitch / 2;
> +
> + assert(two_rows_interleaved_pitch == mt->pitch);
> +
> + return level_y * two_rows_interleaved_pitch + level_x * 64;
> +}
>
> There's really no good place in this series to make this comment but I
> think this is as good as I'm going to get...
> I can't quite tell what your long-term plan is for gen6 stencil and
> HiZ. Eventually, I'd like to kill the ALL_SLICES_AT_EACH_LOD stuff.
> The only reason why Jordan and the Igalia people did that in the first
> place was so that they could re-use existing surface calculation code.
> Really, the requirement is that gen6 doesn't support multiple miplevels
> with HiZ and Stencil even though it does support arrays. The point of
> the ALL_SLICES_AT_EACH_LOD layout is to, instead of making it an array
> of miptrees, it lays it out into a bunch of single-LOD arrays. The
> fact that they happen to be layed out like a miptree is immaterial.
> When Nanley and I were talking about this in the office, he had a very
> good suggestion. What he suggested was to just have an array of
> isl_surf's, one for each miplevel and lay the single-LOD arrays out
> linearly in memory. To compute the size of the gen6 stencil or HiZ
> buffer you would do something like this;
> uint32_t size = 0;
> for (level = 0; level < num_levels; level++) {
> isl_surf_init(&mt->gen6_per_level[level].surf,
> .width = minify(width, level),
> .height = minify(height, level),
> .depth = 1,
> .levels = 1,
> .array_len = array_len,
> .format = ISL_FORMAT_HIZ,
> ...);
> mt->gen6_per_level[level].offset = size;
> size += mt->gen6_per_level[level].surf.size;
> }
> Then in the gen6 hiz and stencil setup code, you would just offset it
> by mt->gen6_per_level[level].offset and be done with it. There's no
> real reason why we need to keep all this ALL_SLICES_AT_EACH_LOD
> calculation stuff indefinitely. We just need some way of putting all
> of the per-LOD images in the same BO. (Technically, they don't even
> need to go in the same BO but it's kind of nice to do it that way.)
> Thoughts?
This might be becoming a matter of taste debate. I'm probably thinking too
much from "what does the hw setup need"-point of view. I'm thinking that
as the hw really only wants offset and row pitch why do we need to maintain
all the extra information. This is afterall something that can be calculated
relatively straight forward if needs be. And the need is not clear - as hw
setup don't need it, what is it for? Accessing individual slices using cpu
is also just about the offset and shifting the base level dimensions.
Also continuing with matter of taste. I'm not a big fan of storing information
in tables if it is used infrequently and rather cheap to produce on-demand.
(I'm seeing the array of isl_surf instances as a pre-computed lookup table).
It looks that we agree on the gen6 stencil and hiz storage. We need to
have slices for each level back-to-back so that layered rendering works.
And as you said having all levels in the same bo feels natural. Other than
that the layout can be what we choose it to be.
Having said all this, I think I'm in the end flexible in which way to go. Just
wanted to share my current thinking.
More information about the mesa-dev
mailing list