[Mesa-dev] [PATCH 02/12] i965/rbc: Set aux surface unconditionally for sampling engine

Pohjolainen, Topi topi.pohjolainen at gmail.com
Thu Sep 1 04:51:12 UTC 2016 

On Wed, Aug 31, 2016 at 05:39:07PM -0700, Jason Ekstrand wrote:
>    On Wed, Aug 31, 2016 at 8:17 AM, Topi Pohjolainen
>    <[1]topi.pohjolainen at intel.com> wrote:
> 
>      Once mcs buffer gets allocated without delay for lossless
>      compression (same as we do for msaa), one gets regression in:
>      GL45-CTS.texture_barrier_ARB.same-texel-rw
>      Setting the auxiliary surface for both sampling engine and data
>      port seems to fix this. I haven't found any hardware documentation
>      backing this though.
>      Signed-off-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
>      ---
>      Â src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 46
>      ++++++++++++++++++++++--
>      Â 1 file changed, 43 insertions(+), 3 deletions(-)
>      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 bfd973b..5cd27ef 100644
>      --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>      +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>      @@ -72,6 +72,48 @@ static const struct surface_state_info
>      surface_state_infos[] = {
>          [9] = {16, 64, 8,  10, SKL_MOCS_WB,  SKL_MOCS_PTE},
>      Â };
>      +static bool
>      +brw_needs_aux_surface(const struct brw_context *brw,
>      +Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  const struct intel_mipmap_tree
>      *mt,
>      +Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  const struct isl_view *view)
>      +{
>      +Â  Â if (!mt->mcs_mt)
>      +Â  Â  Â  return false;
>      +
>      +Â  Â if (view->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT)
>      +Â  Â  Â  return true;
>      +
>      +Â  Â const bool is_lossless_compressed =
>      +Â  Â  Â  intel_miptree_is_lossless_compressed(brw, mt);
>      +Â  Â const bool view_format_lossless_compressed =
>      +Â  Â  Â  Â isl_format_supports_lossless_
>      compression(brw->intelScreen->devinfo,
>      +Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â
>      Â  Â  view->format);
>      +
>      +Â  Â /* If the underlying surface is compressed but it is sampled
>      using a
>      +Â  Â  * format that the sampling engine doesn't support as
>      compressed, there
>      +Â  Â  * is no alternative but to treat the surface as
>      non-compressed.
>      +Â  Â  */
> 
>    Does this even work?  I mean, can you render to a texture with one
>    format and texture from it with another and still use color
>    compression?

There is logic specifically for this in
brw_context::intel_texture_view_requires_resolve(). A compressed surface is
explicitly resolved if it is going to be sampled using a non-compatible
format.
Note also that this is an existing problem - we just don't hit it with
current test cases. They clear with slow path meaning the delayed mcs creation
don't trigger and hence compression stays turned off. Later patch in the
series will change to immediate allocation of mcs and the problem being
addressed here becomes visible.

>    Â
> 
>      +Â  Â if (is_lossless_compressed && !view_format_lossless_
>      compressed) {
> 
>    This is more of a change in behavior than the commit message says.  We
>    should probably make the commit message a bit better.

Sure.

>    Â
> 
>      +Â  Â  Â  /* Logic elsewhere needs to take care to resolve the color
>      buffer prior
>      +Â  Â  Â  Â * to sampling it as non-compressed.
>      +Â  Â  Â  Â */
>      +Â  Â  Â  assert(mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_
>      RESOLVED);
>      +Â  Â  Â  return false;
>      +Â  Â }
>      +
>      +Â  Â /* In practise it looks that setting the same lossless
>      compressed surface
>      +Â  Â  * to be sampled without auxiliary surface and to be written
>      with auxiliary
>      +Â  Â  * surface confuses the hardware.
>      +Â  Â  * Currently there is no direct way of knowing if render pass
>      will use the
>      +Â  Â  * sane surface as render target and therefore simply use the
>      auxiliary
>      +Â  Â  * surface unconditionally here.
>      +Â  Â  */
>      +Â  Â if (is_lossless_compressed)
>      +Â  Â  Â  return true;
> 
>    This makes a lot of sense.  If we're sampling from and rendering to
>    the same surface, we'd better have the compression buffer for both.Â
>    That said, if we ever bind a compressed buffer as a render target with
>    a texture-viewed version as a source (as per above), we're going to
>    have a problem...

And like said above we already do, we just don't have a texture view test case
that would do either fast clear or normal render prior to sampling.
I can't really think of anything else than adding logic that turns off
compressed rendering if the same surface is going to be sampled using a view
having incompatible format.

>    Â
> 
>      +
>      +Â  Â return mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_
>      RESOLVED;
>      +}
>      +
>      Â static void
>      Â brw_emit_surface_state(struct brw_context *brw,
>      Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  struct intel_mipmap_tree *mt,
>      @@ -135,9 +177,7 @@ brw_emit_surface_state(struct brw_context *brw,
>      Â  Â  struct isl_surf *aux_surf = NULL, aux_surf_s;
>      Â  Â  uint64_t aux_offset = 0;
>      Â  Â  enum isl_aux_usage aux_usage = ISL_AUX_USAGE_NONE;
>      -Â  Â if (mt->mcs_mt &&
>      -Â  Â  Â  Â ((view.usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
>      -Â  Â  Â  Â  mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_
>      RESOLVED)) {
>      +Â  Â if (brw_needs_aux_surface(brw, mt, &view)) {
>      Â  Â  Â  Â intel_miptree_get_aux_isl_surf(brw, mt, &aux_surf_s,
>      &aux_usage);
>      Â  Â  Â  Â aux_surf = &aux_surf_s;
>      Â  Â  Â  Â assert(mt->mcs_mt->offset == 0);
>      --
>      2.5.5
>      _______________________________________________
>      mesa-dev mailing list
>      [3]mesa-dev at lists.freedesktop.org
>      [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:topi.pohjolainen at intel.com
>    2. mailto:topi.pohjolainen at intel.com
>    3. mailto:mesa-dev at lists.freedesktop.org
>    4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

More information about the mesa-dev mailing list