[Mesa-dev] [PATCH 01/15] i965/miptree: Refactor mapping table alloc

[Nanley Chery]

On Wed, Jun 14, 2017 at 09:45:46PM +0300, Pohjolainen, Topi wrote:
> On Tue, Jun 13, 2017 at 04:31:26PM -0700, Nanley Chery wrote:
> > On Tue, Jun 13, 2017 at 05:49:59PM +0300, Topi Pohjolainen wrote:
> > > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 29 +++++++++++++++++++++++++--
> > >  1 file changed, 27 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 253d833b13..78a223a7f3 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -285,6 +285,26 @@ intel_depth_format_for_depthstencil_format(mesa_format format) {
> > >     }
> > >  }
> > >  
> > > +static bool
> > > +create_mapping_table(GLenum target, unsigned first_level, unsigned last_level,
> > > +                     unsigned depth0, struct intel_mipmap_level *table)
> > > +{
> > > +   for (unsigned level = first_level; level <= last_level; level++) {
> > > +      const unsigned d = target == GL_TEXTURE_3D ? depth0 >> level : depth0;
> > 
> > There's a bug here. If the target is GL_TEXTURE_3D we should
> > minify(depth0, level) to avoid setting a depth of 0.
> 
> Oops, definitely.
> 
> > 
> > This seems to be more than a refactor. Prior to this patch,
> > brw_miptree_layout_gen6_hiz_stencil wouldn't shrink the number of slices
> > per mipmap level as the level increases, but does so now.
> 
> Right. I actually missed that. How do want to handle that? I could write a
> patch against brw_miptree_layout_gen6_hiz_stencil() doing the same thing there
> (modifying the argument given to intel_miptree_set_level_info() but keeping
> actual allocation size as it was in order to have space for level 0 qpitch).
> 

That should be sufficient.

> > 
> > -Nanley
> > 
> > > +
> > > +      table[level].slice = calloc(d, sizeof(*table[0].slice));
> > > +      if (!table[level].slice)
> > > +         goto unwind;
> > > +   }
> > > +
> > > +   return true;
> > > +
> > > +unwind:
> > > +   for (unsigned level = first_level; level <= last_level; level++)
> > > +      free(table[level].slice);
> > > +
> > > +   return false;
> > > +}
> > >  
> > >  /**
> > >   * @param for_bo Indicates that the caller is
> > > @@ -424,6 +444,12 @@ intel_miptree_create_layout(struct brw_context *brw,
> > >        }
> > >     }
> > >  
> > > +   if (!create_mapping_table(target, first_level, last_level, depth0,
> > > +                             mt->level)) {
> > > +      free(mt);
> > > +      return NULL;
> > > +   }
> > > +
> > >     /* Set array_layout to ALL_SLICES_AT_EACH_LOD when array_spacing_lod0 can
> > >      * be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces on
> > >      * Gen 7 and 8. On Gen 8 and 9 this layout is not available but it is still
> > > @@ -1103,9 +1129,8 @@ intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
> > >     DBG("%s level %d, depth %d, offset %d,%d\n", __func__,
> > >         level, d, x, y);
> > >  
> > > -   assert(mt->level[level].slice == NULL);
> > > +   assert(mt->level[level].slice);
> > >  
> > > -   mt->level[level].slice = calloc(d, sizeof(*mt->level[0].slice));
> > >     mt->level[level].slice[0].x_offset = mt->level[level].level_x;
> > >     mt->level[level].slice[0].y_offset = mt->level[level].level_y;
> > >  }
> > > -- 
> > > 2.11.0
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev