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

[Ben Widawsky]

On 17-02-01 09:30:21, Topi Pohjolainen Topi Pohjolainen wrote:
>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?
>

Oops. I just saw this - and I said the same thing :-)

>>
>>    > +      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