[Mesa-dev] [PATCH v2 01/14] i965/hiz: Start to separate miptree out from hiz buffers
Jordan Justen
jordan.l.justen at intel.com
Tue Nov 11 14:52:08 PST 2014
On 2014-11-10 10:42:16, Ben Widawsky wrote:
> On Mon, Jul 21, 2014 at 11:00:50PM -0700, Jordan Justen wrote:
> > Today we allocate a miptree's for the hiz buffer. We needed this in
> > the past because we would point the hardware at offsets of the hiz
> > buffer. Since the hiz format is not documented, this is not a good
> > idea.
> >
> > Since moving to support layered rendering on Gen7+, we no longer point
> > at an offset into the buffer on Gen7+.
> >
> > Therefore, to support hiz on Gen7+, we don't need a full miptree
> > structure allocated.
> >
> > This patch starts to create a new auxiliary buffer structure
> > (intel_miptree_aux_buffer) that can be a more simplistic miptree
> > side-band buffer associated with a miptree. (For example, to serve the
> > needs of the hiz buffer.)
> >
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_misc_state.c | 4 +-
> > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 2 +-
> > src/mesa/drivers/dri/i965/gen7_blorp.cpp | 2 +-
> > src/mesa/drivers/dri/i965/gen7_misc_state.c | 2 +-
> > src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +--
> > src/mesa/drivers/dri/i965/intel_fbo.c | 4 +-
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 55 +++++++++++++++++++--------
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 29 ++++++++++++--
> > 8 files changed, 76 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > index 76e22bd..af900cd 100644
> > --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > @@ -182,7 +182,7 @@ brw_get_depthstencil_tile_masks(struct intel_mipmap_tree *depth_mt,
> >
> > if (intel_miptree_level_has_hiz(depth_mt, depth_level)) {
> > uint32_t hiz_tile_mask_x, hiz_tile_mask_y;
> > - intel_miptree_get_tile_masks(depth_mt->hiz_mt,
> > + intel_miptree_get_tile_masks(depth_mt->hiz_buf->mt,
> > &hiz_tile_mask_x, &hiz_tile_mask_y,
> > false);
> >
> > @@ -643,7 +643,7 @@ brw_emit_depth_stencil_hiz(struct brw_context *brw,
> >
> > /* Emit hiz buffer. */
> > if (hiz) {
> > - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
> > + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> > BEGIN_BATCH(3);
> > OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> > OUT_BATCH(hiz_mt->pitch - 1);
> > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > index eb865b9..282c7b2 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > @@ -855,7 +855,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context *brw,
> >
> > /* 3DSTATE_HIER_DEPTH_BUFFER */
> > {
> > - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
> > + struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> > uint32_t hiz_offset =
> > intel_miptree_get_aligned_offset(hiz_mt,
> > draw_x & ~tile_mask_x,
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index 0ad570b..d0d623d 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -752,7 +752,7 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context *brw,
> >
> > /* 3DSTATE_HIER_DEPTH_BUFFER */
> > {
> > - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
> > + struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> >
> > BEGIN_BATCH(3);
> > OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> > diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > index 22911bf..6c6a79b 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > @@ -145,7 +145,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
> > OUT_BATCH(0);
> > ADVANCE_BATCH();
> > } else {
> > - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
> > + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> > BEGIN_BATCH(3);
> > OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
> > OUT_BATCH((mocs << 25) |
> > diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > index 7c3bfe0..b6f373d 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > @@ -89,10 +89,10 @@ emit_depth_packets(struct brw_context *brw,
> > } else {
> > BEGIN_BATCH(5);
> > OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2));
> > - OUT_BATCH((depth_mt->hiz_mt->pitch - 1) | BDW_MOCS_WB << 25);
> > - OUT_RELOC64(depth_mt->hiz_mt->bo,
> > + OUT_BATCH((depth_mt->hiz_buf->mt->pitch - 1) | BDW_MOCS_WB << 25);
> > + OUT_RELOC64(depth_mt->hiz_buf->mt->bo,
> > I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
> > - OUT_BATCH(depth_mt->hiz_mt->qpitch >> 2);
> > + OUT_BATCH(depth_mt->hiz_buf->mt->qpitch >> 2);
> > ADVANCE_BATCH();
> > }
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
> > index e43e18b..7f9964c 100644
> > --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> > +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> > @@ -560,9 +560,9 @@ intel_renderbuffer_update_wrapper(struct brw_context *brw,
> >
> > intel_renderbuffer_set_draw_offset(irb);
> >
> > - if (mt->hiz_mt == NULL && brw_is_hiz_depth_format(brw, rb->Format)) {
> > + if (mt->hiz_buf == NULL && brw_is_hiz_depth_format(brw, rb->Format)) {
> > intel_miptree_alloc_hiz(brw, mt);
> > - if (!mt->hiz_mt)
> > + if (!mt->hiz_buf)
> > return false;
> > }
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b36ffc7..d5a2227 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -822,7 +822,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
> >
> > drm_intel_bo_unreference((*mt)->bo);
> > intel_miptree_release(&(*mt)->stencil_mt);
> > - intel_miptree_release(&(*mt)->hiz_mt);
> > + if ((*mt)->hiz_buf) {
>
> In addition, I would have put:
> assert(&(*mt)->hiz_buf->mt)
In patch 4, i965/gen7: Don't allocate hiz miptree structure
This becomes
if (&(*mt)->hiz_buf->mt)
intel_miptree_release(&(*mt)->hiz_buf->mt);
since we might not allocate a full miptree for the hiz buffer as of
that patch.
The current plan will be that gen <= 6 will allocate the miptree, and
gen >= 7 will not. (Due to the gen6 hiz not supporting miplevels, I
think this is where things will have to stay.)
> > + intel_miptree_release(&(*mt)->hiz_buf->mt);
> > + free((*mt)->hiz_buf);
> > + }
> > intel_miptree_release(&(*mt)->mcs_mt);
> > intel_resolve_map_clear(&(*mt)->hiz_map);
> >
> > @@ -1347,7 +1350,7 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
> > struct intel_mipmap_tree *mt,
> > uint32_t level)
> > {
> > - assert(mt->hiz_mt);
> > + assert(mt->hiz_buf);
> >
> > if (brw->gen >= 8 || brw->is_haswell) {
> > uint32_t width = minify(mt->physical_width0, level);
> > @@ -1371,25 +1374,47 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
> > }
> >
> >
> > +static struct intel_miptree_aux_buffer *
> > +intel_hiz_miptree_buf_create(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt)
> > +{
> > + struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
> > +
> > + if (!buf)
> > + return NULL;
> > +
> > + buf->mt = intel_miptree_create(brw,
> > + mt->target,
> > + mt->format,
> > + mt->first_level,
> > + mt->last_level,
> > + mt->logical_width0,
> > + mt->logical_height0,
> > + mt->logical_depth0,
> > + true,
> > + mt->num_samples,
> > + INTEL_MIPTREE_TILING_ANY);
> > + if (!buf->mt) {
> > + free(buf);
> > + return NULL;
> > + }
> > +
> > + buf->bo = buf->mt->bo;
> > + buf->pitch = buf->mt->pitch;
> > + buf->qpitch = buf->mt->qpitch;
> > +
> > + return buf;
> > +}
> > +
> >
> > bool
> > intel_miptree_alloc_hiz(struct brw_context *brw,
> > struct intel_mipmap_tree *mt)
> > {
> > - assert(mt->hiz_mt == NULL);
> > - mt->hiz_mt = intel_miptree_create(brw,
> > - mt->target,
> > - mt->format,
> > - mt->first_level,
> > - mt->last_level,
> > - mt->logical_width0,
> > - mt->logical_height0,
> > - mt->logical_depth0,
> > - true,
> > - mt->num_samples,
> > - INTEL_MIPTREE_TILING_ANY);
> > + assert(mt->hiz_buf == NULL);
> > + mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt);
> >
> > - if (!mt->hiz_mt)
> > + if (!mt->hiz_buf)
> > return false;
> >
> > /* Mark that all slices need a HiZ resolve. */
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > index bb04084..2a421a1 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > @@ -255,6 +255,29 @@ enum intel_fast_clear_state
> > INTEL_FAST_CLEAR_STATE_CLEAR,
> > };
> >
> > +/**
> > + * Miptree aux buffer. These buffers are associated with a miptree, but the
> > + * format is managed by the hardware.
> > + *
> > + * For Gen7+, we always give the hardware the start of the buffer, and let it
> > + * handle all accesses to the buffer. Therefore we don't need the full miptree
> > + * layout structure for this buffer.
> > + *
> > + * For Gen6, we need a hiz miptree structure for this buffer so we can program
> > + * offsets to slices & miplevels.
> > + */
> > +struct intel_miptree_aux_buffer
> > +{
> > + /** Buffer object containing the pixel data. */
> > + drm_intel_bo *bo;
> > +
> > + uint32_t pitch; /**< pitch in bytes. */
> > +
> > + uint32_t qpitch; /**< The distance in rows between array slices. */
> > +
> > + struct intel_mipmap_tree *mt; /**< hiz miptree used with Gen6 */
> > +};
> > +
>
> Doesn't the needs resolve flag belong here too?
We're still using the separate 'hiz_map' list to track resolves.
> > struct intel_mipmap_tree
> > {
> > /** Buffer object containing the pixel data. */
> > @@ -362,15 +385,15 @@ struct intel_mipmap_tree
> > uint32_t offset;
> >
> > /**
> > - * \brief HiZ miptree
> > + * \brief HiZ aux buffer
> > *
> > * The hiz miptree contains the miptree's hiz buffer. To allocate the hiz
> > - * miptree, use intel_miptree_alloc_hiz().
> > + * buffer, use intel_miptree_alloc_hiz().
> > *
> > * To determine if hiz is enabled, do not check this pointer. Instead, use
> > * intel_miptree_slice_has_hiz().
> > */
> > - struct intel_mipmap_tree *hiz_mt;
> > + struct intel_miptree_aux_buffer *hiz_buf;
>
> >
> > /**
> > * \brief Map of miptree slices to needed resolves.
> > --
>
> Just some additional notes:
> Given that the series is driving towards AUX buffer support, and you
> mention "aux" in the comment, maybe it's time to rename this something
> other than "hiz"?
Later in the full bdw-aux-hiz branch, I rename the mcs_mt field to
mcs_buf and change its type to be intel_miptree_aux_buffer as well.
(2211d18 i965: Wrap MCS miptree in intel_miptree_aux_buffer)
So, the intel_miptree_aux_buffer structure is generically named, but
we would have a 'hiz_buf' one for hiz, and a 'mcs_buf' one for mcs.
-Jordan
> Also, as a cheap transitional hack you could have probably done
> #define hiz_mt hiz_buf->mt
> That would have removed a bunch of LOC.
>
> I see nothing wrong.
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
>
> --
> Ben Widawsky, Intel Open Source Technology Center
More information about the mesa-dev
mailing list