[Mesa-dev] [PATCH 31/34] i965: Use partial resolves for CCS buffers being scanned out

[Pohjolainen, Topi]

On Tue, Jan 31, 2017 at 01:37:25PM -0800, Jason Ekstrand wrote:
>    On Wed, Jan 25, 2017 at 10:39 AM, Pohjolainen, Topi
>    <[1]topi.pohjolainen at gmail.com> wrote:
> 
>      On Mon, Jan 23, 2017 at 10:21:54PM -0800, Ben Widawsky wrote:
>      > On Gen9 hardware, the display engine is able to scanout a
>      compressed
>      > framebuffer by providing an offset to auxiliary compression
>      information.
>      > Unfortunately, the hardware is incapable of doing the same thing
>      for the
>      > fast clear color.
>      >
>      > To mitigate this, the hardware introduced a new resolve type
>      called a
>      > partial resolve. The partial resolve will only do a resolve of the
>      fast
>      > clear color and leave the rest of the compressed data alone.
>      >
>      > This patch enables using this resolve type for cases where the
>      > framebuffer will be passed along to the kernel for display.
>      >
>      > v2: Add early exit from intel_miptree_make_shareable() when it's
>      > scanout.
>      >
>      > Signed-off-by: Ben Widawsky <[2]ben at bwidawsk.net>
>      > Acked-by: Daniel Stone <[3]daniels at collabora.com>
>      > Reviewed-by: Topi Pohjolainen <[4]topi.pohjolainen at intel.com> (v1)
>      v2 is also
>      Reviewed-by: Topi Pohjolainen <[5]topi.pohjolainen at intel.com>
> 
>    > ---
>    >  src/mesa/drivers/dri/i965/brw_context.c       | 3 ++-
>    >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 9 ++++++++-
>    >  2 files changed, 10 insertions(+), 2 deletions(-)
>    >
>    > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
>    b/src/mesa/drivers/dri/i965/brw_context.c
>    > index 64b55a8cb7..adfd4c449e 100644
>    > --- a/src/mesa/drivers/dri/i965/brw_context.c
>    > +++ b/src/mesa/drivers/dri/i965/brw_context.c
>    > @@ -1360,7 +1360,8 @@ intel_resolve_for_dri2_flush(struct brw_context
>    *brw,
>    >        if (rb->mt->num_samples <= 1) {
>    >           assert(rb->mt_layer == 0 && rb->mt_level == 0 &&
>    >                  rb->layer_count == 1);
>    > -         intel_miptree_resolve_color(brw, rb->mt, 0, 0, 1, 0);
>    > +         intel_miptree_resolve_color(brw, rb->mt, 0, 0, 1,
>    > +                                     INTEL_RESOLVE_HINT_CLEAR_
>    COLOR);
>    >        } else {
>    >           intel_renderbuffer_downsample(brw, rb);
>    >        }
>    > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    > index 217e80ae31..7edce7d92e 100644
>    > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    > @@ -2415,7 +2415,14 @@ intel_miptree_make_shareable(struct
>    brw_context *brw,
>    >     assert(mt->msaa_layout == INTEL_MSAA_LAYOUT_NONE ||
>    mt->num_samples <= 1);
>    >
>    >     if (mt->mcs_buf) {
>    > -      intel_miptree_all_slices_resolve_color(brw, mt, 0);
>    > +      intel_miptree_all_slices_resolve_color(brw, mt, mt->is_scanout
>    ?
>    > +
>    INTEL_RESOLVE_HINT_CLEAR_COLOR :
>    > +
>    INTEL_RESOLVE_HINT_FULL);
> 
>    Do we need to be checking modifiers here?  Just because it's marked as
>    scanout doesn't mean that the client using it actually knows about CCS
>    and will pass it on to the kernel.

I think I had similar concerns earlier. My current understanding (after
dicussing with Ben) is that the choice of support is made when the surfaces
are created. In other words, if mcs buffer exists for scanout miptree it
means that the client has told that it understands auxiliary buffers.

Here the flag tells to the resolver that client doesn't understand fast clear
but lossless compression is supported. Ben, does this explanation agree with
what you told me?

> 
>    > +      if (mt->is_scanout) {
>    > +         assert(!mt->hiz_buf);
>    > +         return;
>    > +      }
>    > +
>    >        mt->aux_disable |= (INTEL_AUX_DISABLE_CCS |
>    INTEL_AUX_DISABLE_MCS);
>    >        drm_intel_bo_unreference(mt->mcs_buf->bo);
>    >        free(mt->mcs_buf);
>    > --
>    > 2.11.0
>    >
>    > _______________________________________________
>    > mesa-dev mailing list
>    > [6]mesa-dev at lists.freedesktop.org
>    > [7]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>    _______________________________________________
>    mesa-dev mailing list
>    [8]mesa-dev at lists.freedesktop.org
>    [9]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:topi.pohjolainen at gmail.com
>    2. mailto:ben at bwidawsk.net
>    3. mailto:daniels at collabora.com
>    4. mailto:topi.pohjolainen at intel.com
>    5. mailto:topi.pohjolainen at intel.com
>    6. mailto:mesa-dev at lists.freedesktop.org
>    7. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>    8. mailto:mesa-dev at lists.freedesktop.org
>    9. https://lists.freedesktop.org/mailman/listinfo/mesa-dev