[Mesa-dev] [PATCH 5/8] i965/miptree: Create a hiz mcs type

Lionel Landwerlin lionel.g.landwerlin at intel.com
Mon Nov 7 10:24:35 UTC 2016


On 07/11/16 10:18, Pohjolainen, Topi wrote:
> On Thu, Nov 03, 2016 at 10:39:40AM +0000, Lionel Landwerlin wrote:
>> From: Ben Widawsky <benjamin.widawsky at intel.com>
>>
>> This seems counter to the goal of consolidating hiz, mcs, and later ccs buffers.
>> Unfortunately, hiz on gen6 is a thing the code supports, and this wart will be
>> helpful to achieve that. Overall, I believe it does help unify AUX buffers on
>> gen7+.
>>
>> I updated the size field which I introduced in the previous patch, even though
>> we have no use for it.
>>
>> XXX: As I mentioned in the last patch, the height given to the MCS buffer
>> allocation in intel_miptree_alloc_mcs() looks wrong, but I don't claim to fully
>> understand how the MCS buffer is laid out.
>>
>> v2: rebase on master (Lionel)
>>
>> Signed-off-by: Ben Widawsky <benjamin.widawsky at intel.com> (v1)
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com> (v2)
>> ---
>>   src/mesa/drivers/dri/i965/brw_blorp.c         |  4 +-
>>   src/mesa/drivers/dri/i965/gen7_misc_state.c   |  6 +--
>>   src/mesa/drivers/dri/i965/gen8_depth_state.c  |  6 +--
>>   src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 61 ++++++++++++++-------------
>>   src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 14 ++++--
>>   5 files changed, 49 insertions(+), 42 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
>> index 5adb4c6..f0ad074 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> @@ -214,8 +214,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
>>               }
>>               assert(hiz_mt->pitch == aux_surf->row_pitch);
>>            } else {
>> -            surf->aux_addr.buffer = mt->hiz_buf->bo;
>> -            surf->aux_addr.offset = 0;
>> +            surf->aux_addr.buffer = mt->hiz_buf->aux_base.bo;
>> +            surf->aux_addr.offset = mt->hiz_buf->aux_base.offset;
>>            }
>>         }
>>      } else {
>> diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
>> index 271d962..7bd5cd5 100644
>> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
>> @@ -146,13 +146,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
>>         ADVANCE_BATCH();
>>      } else {
>>         assert(depth_mt);
>> -      struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf;
>> +      struct intel_miptree_hiz_buffer *hiz_buf = depth_mt->hiz_buf;
>>   
>>         BEGIN_BATCH(3);
>>         OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
>>         OUT_BATCH((mocs << 25) |
>> -                (hiz_buf->pitch - 1));
>> -      OUT_RELOC(hiz_buf->bo,
>> +                (hiz_buf->aux_base.pitch - 1));
>> +      OUT_RELOC(hiz_buf->aux_base.bo,
>>                   I915_GEM_DOMAIN_RENDER,
>>                   I915_GEM_DOMAIN_RENDER,
>>                   0);
>> diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c
>> index 73b2186..8920910 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
>> @@ -93,10 +93,10 @@ emit_depth_packets(struct brw_context *brw,
>>         assert(depth_mt);
>>         BEGIN_BATCH(5);
>>         OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2));
>> -      OUT_BATCH((depth_mt->hiz_buf->pitch - 1) | mocs_wb << 25);
>> -      OUT_RELOC64(depth_mt->hiz_buf->bo,
>> +      OUT_BATCH((depth_mt->hiz_buf->aux_base.pitch - 1) | mocs_wb << 25);
>> +      OUT_RELOC64(depth_mt->hiz_buf->aux_base.bo,
>>                     I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
>> -      OUT_BATCH(depth_mt->hiz_buf->qpitch >> 2);
>> +      OUT_BATCH(depth_mt->hiz_buf->aux_base.qpitch >> 2);
>>         ADVANCE_BATCH();
>>      }
>>   
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 3d1bdb1..af0e1a4 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -1016,11 +1016,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>>            if ((*mt)->hiz_buf->mt)
>>               intel_miptree_release(&(*mt)->hiz_buf->mt);
>>            else
>> -            drm_intel_bo_unreference((*mt)->hiz_buf->bo);
>> +            drm_intel_bo_unreference((*mt)->hiz_buf->aux_base.bo);
>>            free((*mt)->hiz_buf);
>>         }
>>         if ((*mt)->mcs_buf) {
>> -         intel_miptree_release(&(*mt)->mcs_buf->mt);
> Doesn't this belong to the one of the earlier patches?
>
>>            free((*mt)->mcs_buf);
>>         }
>>         intel_resolve_map_clear(&(*mt)->hiz_map);
>> @@ -1726,7 +1725,7 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
>>    * Helper for intel_miptree_alloc_hiz() that determines the required hiz
>>    * buffer dimensions and allocates a bo for the hiz buffer.
>>    */
>> -static struct intel_miptree_aux_buffer *
>> +static struct intel_miptree_hiz_buffer *
>>   intel_gen7_hiz_buf_create(struct brw_context *brw,
>>                             struct intel_mipmap_tree *mt)
>>   {
>> @@ -1734,7 +1733,7 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
>>      unsigned z_height = mt->logical_height0;
>>      const unsigned z_depth = MAX2(mt->logical_depth0, 1);
>>      unsigned hz_width, hz_height;
>> -   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
>> +   struct intel_miptree_hiz_buffer *buf = calloc(sizeof(*buf), 1);
>>   
>>      if (!buf)
>>         return NULL;
>> @@ -1791,20 +1790,21 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
>>   
>>      unsigned long pitch;
>>      uint32_t tiling = I915_TILING_Y;
>> -   buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
>> -                                      hz_width, hz_height, 1,
>> -                                      &tiling, &pitch,
>> -                                      BO_ALLOC_FOR_RENDER);
>> -   if (!buf->bo) {
>> +   buf->aux_base.bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
>> +                                               hz_width, hz_height, 1,
>> +                                               &tiling, &pitch,
>> +                                               BO_ALLOC_FOR_RENDER);
>> +   if (!buf->aux_base.bo) {
>>         free(buf);
>>         return NULL;
>>      } else if (tiling != I915_TILING_Y) {
>> -      drm_intel_bo_unreference(buf->bo);
>> +      drm_intel_bo_unreference(buf->aux_base.bo);
>>         free(buf);
>>         return NULL;
>>      }
>>   
>> -   buf->pitch = pitch;
>> +   buf->aux_base.size = hz_width * hz_height;
>> +   buf->aux_base.pitch = pitch;
>>   
>>      return buf;
>>   }
>> @@ -1814,7 +1814,7 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
>>    * Helper for intel_miptree_alloc_hiz() that determines the required hiz
>>    * buffer dimensions and allocates a bo for the hiz buffer.
>>    */
>> -static struct intel_miptree_aux_buffer *
>> +static struct intel_miptree_hiz_buffer *
>>   intel_gen8_hiz_buf_create(struct brw_context *brw,
>>                             struct intel_mipmap_tree *mt)
>>   {
>> @@ -1822,7 +1822,7 @@ intel_gen8_hiz_buf_create(struct brw_context *brw,
>>      unsigned z_height = mt->logical_height0;
>>      const unsigned z_depth = MAX2(mt->logical_depth0, 1);
>>      unsigned hz_width, hz_height;
>> -   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
>> +   struct intel_miptree_hiz_buffer *buf = calloc(sizeof(*buf), 1);
>>   
>>      if (!buf)
>>         return NULL;
>> @@ -1875,42 +1875,43 @@ intel_gen8_hiz_buf_create(struct brw_context *brw,
>>         Z_i = minify(Z_i, 1);
>>      }
>>      /* HZ_QPitch = h0 + max(h1, sum(i=2 to m; h_i)) */
>> -   buf->qpitch = h0 + MAX2(h1, sum_h_i);
>> +   buf->aux_base.qpitch = h0 + MAX2(h1, sum_h_i);
>>   
>>      if (mt->target == GL_TEXTURE_3D) {
>>         /* (1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
>>         hz_height = DIV_ROUND_UP(hz_height_3d_sum, 2);
>>      } else {
>>         /* HZ_Height (rows) = ceiling( (HZ_QPitch/2)/8) *8 * Z_Depth */
>> -      hz_height = DIV_ROUND_UP(buf->qpitch, 2 * 8) * 8 * Z0;
>> +      hz_height = DIV_ROUND_UP(buf->aux_base.qpitch, 2 * 8) * 8 * Z0;
>>      }
>>   
>>      unsigned long pitch;
>>      uint32_t tiling = I915_TILING_Y;
>> -   buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
>> -                                      hz_width, hz_height, 1,
>> -                                      &tiling, &pitch,
>> -                                      BO_ALLOC_FOR_RENDER);
>> -   if (!buf->bo) {
>> +   buf->aux_base.bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
>> +                                               hz_width, hz_height, 1,
>> +                                               &tiling, &pitch,
>> +                                               BO_ALLOC_FOR_RENDER);
>> +   if (!buf->aux_base.bo) {
>>         free(buf);
>>         return NULL;
>>      } else if (tiling != I915_TILING_Y) {
>> -      drm_intel_bo_unreference(buf->bo);
>> +      drm_intel_bo_unreference(buf->aux_base.bo);
>>         free(buf);
>>         return NULL;
>>      }
>>   
>> -   buf->pitch = pitch;
>> +   buf->aux_base.size = hz_width * hz_height;
>> +   buf->aux_base.pitch = pitch;
>>   
>>      return buf;
>>   }
>>   
>>   
>> -static struct intel_miptree_aux_buffer *
>> +static struct intel_miptree_hiz_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);
>> +   struct intel_miptree_hiz_buffer *buf = calloc(sizeof(*buf), 1);
>>      uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD;
>>   
>>      if (brw->gen == 6)
>> @@ -1935,9 +1936,10 @@ intel_hiz_miptree_buf_create(struct brw_context *brw,
>>         return NULL;
>>      }
>>   
>> -   buf->bo = buf->mt->bo;
>> -   buf->pitch = buf->mt->pitch;
>> -   buf->qpitch = buf->mt->qpitch;
>> +   buf->aux_base.bo = buf->mt->bo;
>> +   buf->aux_base.size = buf->mt->total_height * buf->mt->pitch;
>> +   buf->aux_base.pitch = buf->mt->pitch;
>> +   buf->aux_base.qpitch = buf->mt->qpitch;
>>   
>>      return buf;
>>   }
>> @@ -2183,7 +2185,6 @@ intel_miptree_make_shareable(struct brw_context *brw,
>>   
>>      if (mt->mcs_buf) {
>>         intel_miptree_resolve_color(brw, mt, 0);
>> -      intel_miptree_release(&mt->mcs_buf->mt);
> And this, belongs to earlier patch in the series?

Thanks again, indeed, should be in patch 3.

>
>>         mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_NO_MCS;
>>      }
>>   }
>> @@ -3274,8 +3275,8 @@ intel_miptree_get_aux_isl_surf(struct brw_context *brw,
>>            aux_pitch = mt->hiz_buf->mt->pitch;
>>            aux_qpitch = mt->hiz_buf->mt->qpitch;
>>         } else {
>> -         aux_pitch = mt->hiz_buf->pitch;
>> -         aux_qpitch = mt->hiz_buf->qpitch;
>> +         aux_pitch = mt->hiz_buf->aux_base.pitch;
>> +         aux_qpitch = mt->hiz_buf->aux_base.qpitch;
>>         }
>>   
>>         *usage = ISL_AUX_USAGE_HIZ;
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> index 0b4b353..e9024a1 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> @@ -324,9 +324,6 @@ enum miptree_array_layout {
>>    * 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
>>   {
>> @@ -374,6 +371,15 @@ struct intel_miptree_aux_buffer
>>       * @see 3DSTATE_HIER_DEPTH_BUFFER.SurfaceQPitch
>>       */
>>      uint32_t qpitch;
>> +};
>> +/**
>> + * The HiZ buffer requires extra attributes on earlier GENs. This is easily
>> + * contained within an intel_mipmap_tree. To make sure we do not abuse this, we
>> + * keep the hiz datastructure separate.
>> + */
>> +struct intel_miptree_hiz_buffer
>> +{
>> +   struct intel_miptree_aux_buffer aux_base;
>>   
>>      /**
>>       * Hiz miptree. Used only by Gen6.
>> @@ -609,7 +615,7 @@ struct intel_mipmap_tree
>>       * To determine if hiz is enabled, do not check this pointer. Instead, use
>>       * intel_miptree_slice_has_hiz().
>>       */
>> -   struct intel_miptree_aux_buffer *hiz_buf;
>> +   struct intel_miptree_hiz_buffer *hiz_buf;
>>   
>>      /**
>>       * \brief Map of miptree slices to needed resolves.
>> -- 
>> 2.10.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev




More information about the mesa-dev mailing list