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.vers...@linux.intel.com>
Signed-off-by: Ben Widawsky <b...@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.


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to