[Mesa-dev] [PATCH v2 01/14] i965/hiz: Start to separate miptree out from hiz buffers
Ben Widawsky
ben at bwidawsk.net
Mon Nov 10 10:42:16 PST 2014
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)
> + 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?
> 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"?
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