<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Feb 19, 2018 at 10:01 AM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed 24 Jan 2018, Jason Ekstrand wrote:<br>
> We're about to start letting the intel_obj->_Format be the "real"<br>
> texture format.  For depth/stencil textures, this may be a combined<br>
> depth stencil format.  For ETC2 on gen7 and earlier, this will be the<br>
> actual ETC2 format.  This makes a bit more GL sense but means we have to<br>
> be careful in state upload.<br>
<br>
</span>What is the "real" format? It's not a rhetorical question. Throughout<br>
Mesa, I never know what's real and what's not. By "real", do you mean<br>
the untranslated user-specified glTextureView(internalformat) and<br>
glTexImage2D(internalformat)?  Or do you mean simply "more real than<br>
before" ;)<span class=""><br></span></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> ---<br>
>  src/mesa/drivers/dri/i965/brw_<wbr>wm_surface_state.c | 16 +++++++++++++++-<br>
>  1 file changed, 15 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/<wbr>brw_wm_surface_state.c<br>
> index 38af6bc..844c23b 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_wm_surface_state.c<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_wm_surface_state.c<br>
> @@ -507,7 +507,21 @@ brw_update_texture_surface(<wbr>struct gl_context *ctx,<br>
>        const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :<br>
>                                  brw_get_texture_swizzle(&brw-><wbr>ctx, obj));<br>
><br>
> -      mesa_format mesa_fmt = plane == 0 ? intel_obj->_Format : mt->format;<br>
> +      mesa_format mesa_fmt;<br>
> +      if (firstImage->_BaseFormat == GL_DEPTH_STENCIL ||<br>
> +          firstImage->_BaseFormat == GL_DEPTH_COMPONENT) {<br>
> +         /* The format from intel_obj may be a combined depth stencil format<br>
> +          * when we just want depth.  Pull it from the miptree instead.  This<br>
> +          * is safe because texture views aren't allowed on depth/stencil.<br>
> +          */<br>
> +         mesa_fmt = mt->format;<br>
> +      } else if (mt->etc_format != MESA_FORMAT_NONE) {<br>
> +         mesa_fmt = mt->format;<br>
<br>
</span>This looks like it would break ETC2 texture views on hw where we decode<br>
the ETC2 on upload (Ivybridge?), if such views worked. I suspect such<br>
texture views never worked.<span class=""><br></span></blockquote><div><br></div><div>I'm pretty sure they've never worked.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +      } else if (plane > 0) {<br>
> +         mesa_fmt = mt->format;<br>
> +      } else {<br>
> +         mesa_fmt = intel_obj->_Format;<br>
> +      }<br>
>        enum isl_format format = translate_tex_format(brw, mesa_fmt,<br>
>                                                      for_txf ? GL_DECODE_EXT :<br>
>                                                      sampler->sRGBDecode);<br>
<br>
</span>I want to give a r-b, but want to first hear your reply regarding the<br>
"real" format.<br>
</blockquote></div><br></div></div>