[Mesa-dev] [PATCH v5 06/19] intel/isl: Add support to emit clear value address.

[Rafael Antognolli]

On Tue, Apr 03, 2018 at 06:05:06PM +0300, Pohjolainen, Topi wrote:
> On Thu, Mar 29, 2018 at 10:58:40AM -0700, Rafael Antognolli wrote:
> > gen10 can emit the clear color by setting it on a buffer somewhere, and
> > then adding only the address to the surface state.
> > 
> > This commit add support for that on isl_surf_fill_state, and if that is
> > requested, skip setting the clear value itself.
> > 
> > v2: Add assert to make sure we are at least on gen10.
> > 
> > Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> > Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> > ---
> >  src/intel/isl/isl.h               |  9 +++++++++
> >  src/intel/isl/isl_surface_state.c | 18 ++++++++++++++----
> >  2 files changed, 23 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > index 2edf0522e32..c50b78d4701 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -1307,6 +1307,15 @@ struct isl_surf_fill_state_info {
> >      */
> >     union isl_color_value clear_color;
> >  
> > +   /**
> > +    * Send only the clear value address
> > +    *
> > +    * If set, we only pass the clear address to the GPU and it will fetch it
> > +    * from wherever it is.
> > +    */
> > +   bool use_clear_address;
> > +   uint64_t clear_address;
> > +
> >     /**
> >      * Surface write disables for gen4-5
> >      */
> > diff --git a/src/intel/isl/isl_surface_state.c b/src/intel/isl/isl_surface_state.c
> > index 32a5429f2bf..bff9693f02d 100644
> > --- a/src/intel/isl/isl_surface_state.c
> > +++ b/src/intel/isl/isl_surface_state.c
> > @@ -637,11 +637,21 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, void *state,
> >  #endif
> >  
> >     if (info->aux_usage != ISL_AUX_USAGE_NONE) {
> > +      if (info->use_clear_address) {
> > +#if GEN_GEN >= 10
> > +         s.ClearValueAddressEnable = true;
> 
> This will set it for multisampled also and upset piglit tests. We need
> something of this sort:
> 
>             s.ClearValueAddressEnable = info->aux_usage != ISL_AUX_USAGE_MCS;

Hmmm... what piglit tests are having issues with this? And which
platform?

Also, I think it should be fine to always set it, as long as if the
clear color is used (thus read from the clear color buffer), it should
be correct. Maybe there's an issue with the initialization or update of
the value or something like that for MCS.

Why wouldn't we want to enable it for multisampled surfaces?

> > +         s.ClearValueAddress = info->clear_address;
> > +#else
> > +         unreachable("Gen9 and earlier do not support indirect clear colors");
> > +#endif
> > +      }
> >  #if GEN_GEN >= 9
> > -      s.RedClearColor = info->clear_color.u32[0];
> > -      s.GreenClearColor = info->clear_color.u32[1];
> > -      s.BlueClearColor = info->clear_color.u32[2];
> > -      s.AlphaClearColor = info->clear_color.u32[3];
> > +      if (!info->use_clear_address) {
> > +         s.RedClearColor = info->clear_color.u32[0];
> > +         s.GreenClearColor = info->clear_color.u32[1];
> > +         s.BlueClearColor = info->clear_color.u32[2];
> > +         s.AlphaClearColor = info->clear_color.u32[3];
> > +      }
> >  #elif GEN_GEN >= 7
> >        /* Prior to Sky Lake, we only have one bit for the clear color which
> >         * gives us 0 or 1 in whatever the surface's format happens to be.
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev