[Mesa-dev] [PATCH 08/13] i965/miptree: Share the miptree format in miptree_create

[Pohjolainen, Topi]

On Wed, Jun 13, 2018 at 09:20:55AM -0700, Nanley Chery wrote:
> On Wed, Jun 13, 2018 at 09:33:41AM +0300, Pohjolainen, Topi wrote:
> > On Tue, Jun 12, 2018 at 12:22:00PM -0700, Nanley Chery wrote:
> > > ---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 30 +++++++++----------
> > >  1 file changed, 15 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index 03628e3fd9f..97de30076e0 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -696,8 +696,19 @@ miptree_create(struct brw_context *brw,
> > >     if (devinfo->gen < 6 && _mesa_is_format_color_format(format))
> > >        tiling_flags &= ~ISL_TILING_Y0_BIT;
> > >  
> > > +   mesa_format mt_fmt;
> > > +   if (_mesa_is_format_color_format(format)) {
> > > +      mt_fmt = intel_lower_compressed_format(brw, format);
> > > +   } else {
> > > +      /* Fix up the Z miptree format for how we're splitting out separate
> > > +       * stencil. Gen7 expects there to be no stencil bits in its depth buffer.
> > > +       */
> > > +      mt_fmt = (devinfo->gen < 6) ? format :
> > > +               intel_depth_format_for_depthstencil_format(format);
> > > +   }
> > 
> > I wonder if we need to add something of this sort for coverity not complaining
> > later on (I don't know if it is clever to know what
> > _mesa_is_format_color_format() does):
> > 
> >       } else {
> >          unreachable("Format with invalid base");
> >       }
> > 
> > 
> 
> Where would we be adding this unreachable? There is already an else case here.

Yeah, same thing as with the other patch, I was thinking the STENCIL_INDEX
case in my head and somehow stopped reading what you actually had here. This
is all fine, sorry.

> 
> -Nanley
> 
> > > +
> > >     if (format == MESA_FORMAT_S_UINT8)
> > > -      return make_surface(brw, target, format, first_level, last_level,
> > > +      return make_surface(brw, target, mt_fmt, first_level, last_level,
> > >                            width0, height0, depth0, num_samples,
> > >                            tiling_flags,
> > >                            ISL_SURF_USAGE_STENCIL_BIT |
> > > @@ -709,13 +720,8 @@ miptree_create(struct brw_context *brw,
> > >     const GLenum base_format = _mesa_get_format_base_format(format);
> > >     if ((base_format == GL_DEPTH_COMPONENT ||
> > >          base_format == GL_DEPTH_STENCIL)) {
> > > -      /* Fix up the Z miptree format for how we're splitting out separate
> > > -       * stencil.  Gen7 expects there to be no stencil bits in its depth buffer.
> > > -       */
> > > -      const mesa_format depth_only_format =
> > > -         intel_depth_format_for_depthstencil_format(format);
> > >        struct intel_mipmap_tree *mt = make_surface(
> > > -         brw, target, devinfo->gen >= 6 ? depth_only_format : format,
> > > +         brw, target, mt_fmt,
> > >           first_level, last_level,
> > >           width0, height0, depth0, num_samples, tiling_flags,
> > >           ISL_SURF_USAGE_DEPTH_BIT | ISL_SURF_USAGE_TEXTURE_BIT,
> > > @@ -733,19 +739,13 @@ miptree_create(struct brw_context *brw,
> > >        return mt;
> > >     }
> > >  
> > > -   mesa_format tex_format = format;
> > > -   mesa_format etc_format = MESA_FORMAT_NONE;
> > >     uint32_t alloc_flags = 0;
> > >  
> > > -   format = intel_lower_compressed_format(brw, format);
> > > -
> > > -   etc_format = (format != tex_format) ? tex_format : MESA_FORMAT_NONE;
> > > -
> > >     if (flags & MIPTREE_CREATE_BUSY)
> > >        alloc_flags |= BO_ALLOC_BUSY;
> > >  
> > >     struct intel_mipmap_tree *mt = make_surface(
> > > -                                     brw, target, format,
> > > +                                     brw, target, mt_fmt,
> > >                                       first_level, last_level,
> > >                                       width0, height0, depth0,
> > >                                       num_samples, tiling_flags,
> > > @@ -755,7 +755,7 @@ miptree_create(struct brw_context *brw,
> > >     if (!mt)
> > >        return NULL;
> > >  
> > > -   mt->etc_format = etc_format;
> > > +   mt->etc_format = (mt_fmt != format) ? format : MESA_FORMAT_NONE;
> > >  
> > >     if (!(flags & MIPTREE_CREATE_NO_AUX))
> > >        intel_miptree_choose_aux_usage(brw, mt);
> > > -- 
> > > 2.17.0
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev