[Mesa-dev] [PATCH 1/6] i965/state: Ignore intel_obj->_Format for depth/stencil and ETC2

[Jason Ekstrand]

On Mon, Feb 19, 2018 at 10:01 AM, Chad Versace <chadversary at chromium.org>
wrote:

> On Wed 24 Jan 2018, Jason Ekstrand wrote:
> > We're about to start letting the intel_obj->_Format be the "real"
> > texture format.  For depth/stencil textures, this may be a combined
> > depth stencil format.  For ETC2 on gen7 and earlier, this will be the
> > actual ETC2 format.  This makes a bit more GL sense but means we have to
> > be careful in state upload.
>
> What is the "real" format? It's not a rhetorical question. Throughout
> Mesa, I never know what's real and what's not. By "real", do you mean
> the untranslated user-specified glTextureView(internalformat) and
> glTexImage2D(internalformat)?  Or do you mean simply "more real than
> before" ;)
>

By "real" format, I mean the one that the core mesa state tracking code
thinks it is.  For texture views, that corresponds directly to an actual GL
internal format.  For textures created through glTexImage2D (not
TexStorage) with an internal format such as GL_RGB, it's something computed
from the internal format and the format used for upload.


> > ---
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > index 38af6bc..844c23b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -507,7 +507,21 @@ brw_update_texture_surface(struct gl_context *ctx,
> >        const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
> >                                  brw_get_texture_swizzle(&brw->ctx,
> obj));
> >
> > -      mesa_format mesa_fmt = plane == 0 ? intel_obj->_Format :
> mt->format;
> > +      mesa_format mesa_fmt;
> > +      if (firstImage->_BaseFormat == GL_DEPTH_STENCIL ||
> > +          firstImage->_BaseFormat == GL_DEPTH_COMPONENT) {
> > +         /* The format from intel_obj may be a combined depth stencil
> format
> > +          * when we just want depth.  Pull it from the miptree
> instead.  This
> > +          * is safe because texture views aren't allowed on
> depth/stencil.
> > +          */
> > +         mesa_fmt = mt->format;
> > +      } else if (mt->etc_format != MESA_FORMAT_NONE) {
> > +         mesa_fmt = mt->format;
>
> This looks like it would break ETC2 texture views on hw where we decode
> the ETC2 on upload (Ivybridge?), if such views worked. I suspect such
> texture views never worked.
>

I'm pretty sure they've never worked.


> > +      } else if (plane > 0) {
> > +         mesa_fmt = mt->format;
> > +      } else {
> > +         mesa_fmt = intel_obj->_Format;
> > +      }
> >        enum isl_format format = translate_tex_format(brw, mesa_fmt,
> >                                                      for_txf ?
> GL_DECODE_EXT :
> >
> sampler->sRGBDecode);
>
> I want to give a r-b, but want to first hear your reply regarding the
> "real" format.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180220/6802c2d5/attachment.html>