[Mesa-dev] [04.5/11] i965: Add flag telling if miptree is for client consumption

[Pohjolainen, Topi]

On Thu, May 05, 2016 at 07:08:15PM -0700, Ben Widawsky wrote:
> On Mon, Apr 25, 2016 at 08:10:01PM +0300, Topi Pohjolainen wrote:
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++++---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 9 +++++++++
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b11fafd..cdf2fbd 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -394,6 +394,7 @@ intel_miptree_create_layout(struct brw_context *brw,
> >     mt->logical_depth0 = depth0;
> >     mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_NO_MCS;
> >     mt->disable_aux_buffers = (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) != 0;
> > +   mt->is_scanout = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) != 0;
> >     exec_list_make_empty(&mt->hiz_map);
> >     mt->cpp = _mesa_get_format_bytes(format);
> >     mt->num_samples = num_samples;
> > @@ -900,7 +901,7 @@ intel_update_winsys_renderbuffer_miptree(struct brw_context *intel,
> >                                                   height,
> >                                                   1,
> >                                                   pitch,
> > -                                                 0);
> > +                                                 MIPTREE_LAYOUT_FOR_SCANOUT);
> >     if (!singlesample_mt)
> >        goto fail;
> >  
> > @@ -959,8 +960,8 @@ intel_miptree_create_for_renderbuffer(struct brw_context *brw,
> >     bool ok;
> >     GLenum target = num_samples > 1 ? GL_TEXTURE_2D_MULTISAMPLE : GL_TEXTURE_2D;
> >     const uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> > -                                 MIPTREE_LAYOUT_TILING_ANY;
> > -
> > +                                 MIPTREE_LAYOUT_TILING_ANY |
> > +                                 MIPTREE_LAYOUT_FOR_SCANOUT;
> >  
> >     mt = intel_miptree_create(brw, target, format, 0, 0,
> >                               width, height, depth, num_samples,
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > index d4f8563..bb06522 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > @@ -657,6 +657,13 @@ struct intel_mipmap_tree
> >      */
> >     bool disable_aux_buffers;
> >  
> > +   /**
> > +    * Tells if the underlying buffer is to be also consumed by entities other
> > +    * than the driver. This allows logic to turn off features such as lossless
> > +    * compression which is not currently understood by client applications.
> > +    */
> > +   bool is_scanout;
> > +
> >     /* These are also refcounted:
> >      */
> >     GLuint refcount;
> > @@ -697,6 +704,8 @@ enum {
> >     MIPTREE_LAYOUT_TILING_NONE              = 1 << 6,
> >     MIPTREE_LAYOUT_TILING_ANY               = MIPTREE_LAYOUT_TILING_Y |
> >                                               MIPTREE_LAYOUT_TILING_NONE,
> > +
> > +   MIPTREE_LAYOUT_FOR_SCANOUT              = 1 << 7,
> >  };
> >  
> >  struct intel_mipmap_tree *intel_miptree_create(struct brw_context *brw,
> 
> Since we hope that we one day can use these buffers for scanout, I'd go for
> something more generic, like (requires other changes as well):
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 21e4718..deea30d 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -687,11 +687,14 @@ enum {
>     MIPTREE_LAYOUT_ACCELERATED_UPLOAD       = 1 << 0,
>     MIPTREE_LAYOUT_FORCE_ALL_SLICE_AT_LOD   = 1 << 1,
>     MIPTREE_LAYOUT_FOR_BO                   = 1 << 2,
> -   MIPTREE_LAYOUT_DISABLE_AUX              = 1 << 3,
> -   MIPTREE_LAYOUT_FORCE_HALIGN16           = 1 << 4,
> -
> -   MIPTREE_LAYOUT_TILING_Y                 = 1 << 5,
> -   MIPTREE_LAYOUT_TILING_NONE              = 1 << 6,
> +   MIPTREE_LAYOUT_DISABLE_AUX_MCS /* CCS_D */ = 1 << 3,
> +   MIPTREE_LAYOUT_DISABLE_AUX_CCS_E        = 1 << 4,
> +   MIPTREE_LAYOUT_DISABLE_AUX              = MIPTREE_LAYOUT_DISABLE_AUX_MCS |
> +                                             MIPTREE_LAYOUT_DISABLE_AUX_CCS_E,
> +   MIPTREE_LAYOUT_FORCE_HALIGN16           = 1 << 5,
> +
> +   MIPTREE_LAYOUT_TILING_Y                 = 1 << 6,
> +   MIPTREE_LAYOUT_TILING_NONE              = 1 << 7,
>     MIPTREE_LAYOUT_TILING_ANY               = MIPTREE_LAYOUT_TILING_Y |
>                                               MIPTREE_LAYOUT_TILING_NONE,
>  };
> 
> 
> I do like mt->is_scanout though. That seems useful. Either way:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

I'm wondering what you had in mind in addition to the _LAYOUT_DISABLE_ flags?
These are not available anymore when intel_miptree_prepare_mcs() (next patch,
the deferred allocation of aux buffer) gets called. We would need something
in "struct intel_mipmap_tree", perhaps "bool disable_something"?