Re: [Mesa-dev] [PATCH 1/6] i965/state: Ignore intel_obj->_Format for depth/stencil and ETC2
On Tue 20 Feb 2018, Jason Ekstrand wrote: > On Mon, Feb 19, 2018 at 10:01 AM, Chad Versace <[1]chadvers...@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. I understand now. Sounds good to me. Reviewed-by: Chad Versace___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] i965/state: Ignore intel_obj->_Format for depth/stencil and ETC2
On Mon, Feb 19, 2018 at 10:01 AM, Chad Versacewrote: > 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(>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. > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] i965/state: Ignore intel_obj->_Format for depth/stencil and ETC2
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" ;) > --- > 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(>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. > + } 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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/6] i965/state: Ignore intel_obj->_Format for depth/stencil and ETC2
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. --- 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(>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; + } 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); -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev