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

[Ben Widawsky]

On 17-01-31 13:37:25, Jason Ekstrand wrote:
>On Wed, Jan 25, 2017 at 10:39 AM, Pohjolainen, Topi <
>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 <ben at bwidawsk.net>
>> > Acked-by: Daniel Stone <daniels at collabora.com>
>> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com> (v1)
>>
>> v2 is also
>>
>> Reviewed-by: Topi Pohjolainen <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.
>
>

The way the code works today, IFF the buffer is scanout, and it has an MCS, then
it must mean the client knows about it.

It seems this needs a comment.

>> > +      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
>> > mesa-dev at lists.freedesktop.org
>> > 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
>>