On Wed, Jul 2, 2014 at 5:39 AM, Pohjolainen, Topi
<topi.pohjolai...@intel.com> wrote:
> On Tue, Jul 01, 2014 at 04:53:06PM -0700, Jordan Justen wrote:
>> We now skip allocating a hiz miptree for gen7. Instead, we calculate
>> the required hiz buffer parameters and allocate a bo directly.
>>
>> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
>> ---
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 67
>> ++++++++++++++++++++++++++-
>> 1 file changed, 65 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 8719c29..b308b0c 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -823,7 +823,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>> drm_intel_bo_unreference((*mt)->bo);
>> intel_miptree_release(&(*mt)->stencil_mt);
>> if ((*mt)->hiz_buf) {
>> - intel_miptree_release(&(*mt)->hiz_buf->mt);
>> + if ((*mt)->hiz_buf->mt)
>> + intel_miptree_release(&(*mt)->hiz_buf->mt);
>> + else
>> + drm_intel_bo_unreference((*mt)->hiz_buf->bo);
>> free((*mt)->hiz_buf);
>> }
>> intel_miptree_release(&(*mt)->mcs_mt);
>> @@ -1375,6 +1378,61 @@ intel_miptree_level_enable_hiz(struct brw_context
>> *brw,
>>
>>
>> static struct intel_miptree_aux_buffer *
>> +intel_hiz_buf_create(struct brw_context *brw,
>> + struct intel_mipmap_tree *mt)
>> +{
>> + unsigned hz_width, hz_height;
>> + unsigned H0, h0, h1, Z0;
>> + const unsigned j = 8;
>
> This could read 'vertical_align' instead of 'j'.
I wanted to stick with the 'j' terminology from the docs. Although,
I'm not consistent about sticking with the docs, so how about a v2
where I'm more consistent about that?
>> + struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
>> +
>> + if (!buf)
>> + return NULL;
>> +
>> + H0 = mt->logical_height0;
>> + h0 = ALIGN(H0, j);
>> + h1 = ALIGN(minify(H0, 1), j);
>> + Z0 = mt->logical_depth0;
>> +
>> + buf->qpitch = h0 + h1 + (12 * j);
>> +
>> + hz_width = ALIGN(mt->logical_width0, 16);
>
> Would be clearer with 'horizonatal_align' instead of 16.
>
>> +
>> + if (mt->target == GL_TEXTURE_3D) {
>> + unsigned H_i = H0;
>> + unsigned Z_i = Z0;
>> + unsigned sum_h_i = 0;
>> + hz_height = 0;
>> + for (int level = mt->first_level; level <= mt->last_level; ++level) {
>> + unsigned h_i = ALIGN(H_i, j);
>> + hz_height += h_i * Z_i;
>> + sum_h_i += h_i;
>> + H_i = minify(H_i, 1);
>> + Z_i = minify(Z_i, 1);
>> + }
>> + buf->qpitch = h0 + MAX2(h1, sum_h_i);
>
> I'm a little confused - bspec says that this is the formula for 1D and 2D,
> and specifically that it is not applicable for 3D as the 'hz_height' is
> evaluated without it (as you do).
Hmm, it looks like the loop may be needed for 1D/2D as of gen8?
Looking at the bspec, it seems like a summation is only used for 3D.
I'll re-write a v2, and separate gen7 from gen8.
-Jordan
>> + hz_height = ALIGN(hz_height, 2) >> 1;
>> + } else {
>> + hz_height = (ALIGN(buf->qpitch, 8) / 2) * Z0;
>
> And here bspec says to use the qpitch formula you had for the 3D case:
>
> HZ_Height = ceiling((HZ_QPitch / 2) / 8) * 8 * Z_Depth
>
> And here 'vertical_align' (or 'j') would be clearer than 8 as well.
>
>> + if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
>> + mt->target == GL_TEXTURE_CUBE_MAP) {
>> + hz_height *= 6;
>> + }
>> + }
>> +
>> + unsigned long pitch;
>> + uint32_t tiling = I915_TILING_Y;
>> + buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
>> + hz_width, hz_height, mt->cpp,
>> + &tiling, &pitch,
>> + BO_ALLOC_FOR_RENDER);
>> + buf->pitch = pitch;
>> +
>> + return buf;
>> +}
>> +
>> +
>> +static struct intel_miptree_aux_buffer *
>> intel_hiz_miptree_buf_create(struct brw_context *brw,
>> struct intel_mipmap_tree *mt)
>> {
>> @@ -1412,7 +1470,12 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
>> struct intel_mipmap_tree *mt)
>> {
>> assert(mt->hiz_buf == NULL);
>> - mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt);
>> +
>> + if (brw->gen == 7) {
>> + mt->hiz_buf = intel_hiz_buf_create(brw, mt);
>> + } else {
>> + mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt);
>> + }
>>
>> if (!mt->hiz_buf)
>> return false;
>> --
>> 2.0.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
