[Mesa-dev] [PATCH 1/2] i965: Extract region use from hiz depth buffer
Chad Versace
chad.versace at linux.intel.com
Thu Sep 26 10:37:55 PDT 2013
On 09/22/2013 10:37 AM, Ben Widawsky wrote:
> Starting with Ivybridge, the hierarchical had relaxed requirements for
^^^^^^^^^^^^^^^
"the hierarchical" doesn't make sense. Just call it "the HiZ buffer" or
"the hiz buffer".
> its allocation. Following a "simple" formula in the bspec was all you
> needed to satisfy the requirement.
>
> To prepare the code for this, extract all places where the miptree was
> used, when we really only needed the region. This allows an upcoming
> patch to simply allocate the region, and not the whole miptree.
>
> CC: Chad Versace <chad.versace at linux.intel.com>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/brw_misc_state.c | 8 +++---
> 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 | 6 ++---
> src/mesa/drivers/dri/i965/intel_fbo.c | 4 +--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 35 +++++++++++++++------------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 +++-
> 7 files changed, 34 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 7f4cd6f..80bdc1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -211,7 +211,7 @@ brw_get_depthstencil_tile_masks(struct intel_mipmap_tree *depth_mt,
>
> if (intel_miptree_slice_has_hiz(depth_mt, depth_level, depth_layer)) {
> uint32_t hiz_tile_mask_x, hiz_tile_mask_y;
> - intel_region_get_tile_masks(depth_mt->hiz_mt->region,
> + intel_region_get_tile_masks(depth_mt->hiz_depth_buffer.region,
> &hiz_tile_mask_x, &hiz_tile_mask_y, false);
>
> /* Each HiZ row represents 2 rows of pixels */
> @@ -667,11 +667,11 @@ 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_region *region = depth_mt->hiz_depth_buffer.region;
> BEGIN_BATCH(3);
> OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> - OUT_BATCH(hiz_mt->region->pitch - 1);
> - OUT_RELOC(hiz_mt->region->bo,
> + OUT_BATCH(region->pitch - 1);
> + OUT_RELOC(region->bo,
> I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> brw->depthstencil.hiz_offset);
> ADVANCE_BATCH();
> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> index da523e5..7b41dad 100644
> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> @@ -887,7 +887,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context *brw,
>
> /* 3DSTATE_HIER_DEPTH_BUFFER */
> {
> - struct intel_region *hiz_region = params->depth.mt->hiz_mt->region;
> + struct intel_region *hiz_region = params->depth.mt->hiz_depth_buffer.region;
> uint32_t hiz_offset =
> intel_region_get_aligned_offset(hiz_region,
> 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 9df3d92..dea5d48 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -737,7 +737,7 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context *brw,
>
> /* 3DSTATE_HIER_DEPTH_BUFFER */
> {
> - struct intel_region *hiz_region = params->depth.mt->hiz_mt->region;
> + struct intel_region *hiz_region = params->depth.mt->hiz_depth_buffer.region;
>
> 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 eb942cf..1685b38 100644
> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> @@ -143,12 +143,12 @@ 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_region *region = depth_mt->hiz_depth_buffer.region;
> BEGIN_BATCH(3);
> OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
> OUT_BATCH((mocs << 25) |
> - (hiz_mt->region->pitch - 1));
> - OUT_RELOC(hiz_mt->region->bo,
> + (region->pitch - 1));
> + OUT_RELOC(region->bo,
> I915_GEM_DOMAIN_RENDER,
> I915_GEM_DOMAIN_RENDER,
> 0);
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 1692325..ef26643 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -452,9 +452,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_depth_buffer.region == NULL && brw_is_hiz_depth_format(brw, rb->Format)) {
> intel_miptree_alloc_hiz(brw, mt);
> - if (!mt->hiz_mt)
> + if (!mt->hiz_depth_buffer.region)
> 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 2f5e04f..cb6ead3 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -793,7 +793,8 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>
> intel_region_release(&((*mt)->region));
> intel_miptree_release(&(*mt)->stencil_mt);
> - intel_miptree_release(&(*mt)->hiz_mt);
> + intel_miptree_release(&(*mt)->hiz_depth_buffer.mt);
> + (*mt)->hiz_depth_buffer.region = NULL;
In patch 2/2, a memory leak occurs here. Use refcounting to fix the leak.
That is, replace
(*mt)->hiz_depth_buffer.region = NULL;
with
intel_region_release(&(*mt)->hiz_depth_buffer.region);
> intel_miptree_release(&(*mt)->mcs_mt);
> intel_miptree_release(&(*mt)->singlesample_mt);
> intel_resolve_map_clear(&(*mt)->hiz_map);
> @@ -1250,7 +1251,7 @@ intel_miptree_slice_enable_hiz(struct brw_context *brw,
> uint32_t level,
> uint32_t layer)
> {
> - assert(mt->hiz_mt);
> + assert(mt->hiz_depth_buffer.region);
>
> if (brw->is_haswell) {
> const struct intel_mipmap_level *l = &mt->level[level];
> @@ -1276,22 +1277,24 @@ 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);
> -
> - if (!mt->hiz_mt)
> + assert(mt->hiz_depth_buffer.mt == NULL);
> + mt->hiz_depth_buffer.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 (!mt->hiz_depth_buffer.mt)
> return false;
>
> + mt->hiz_depth_buffer.region = mt->hiz_depth_buffer.mt->region;
> +
To make the above refcount fix work, replace the above assignment with
intel_region_reference(&mt->hiz_depth_buffer.region, mt->hiz_depth_buffer.mt->region);
> /* Mark that all slices need a HiZ resolve. */
> struct intel_resolve_map *head = &mt->hiz_map;
> for (int level = mt->first_level; level <= mt->last_level; ++level) {
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index d718125..7473570 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -414,7 +414,10 @@ struct intel_mipmap_tree
> * 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 {
> + struct intel_mipmap_tree *mt;
> + struct intel_region *region;
> + } hiz_depth_buffer;
The name 'hiz_depth_buffer' has a redundancy, because the hiz buffer
can accompany only a depth buffer. Let's shorten the name to
'hiz_buffer'.
Also, you need to update the comment above this hunk to reflect the
new data structure.
> /**
> * \brief Map of miptree slices to needed resolves.
>
More information about the mesa-dev
mailing list