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

[Jason Ekstrand]

On Mon, Feb 26, 2018 at 1:14 PM, Jordan Justen <jordan.l.justen at intel.com>
wrote:

> On 2018-02-21 13:45:15, 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>
> > ---
> >  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 f1b38efed44..bab0ad3d544 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -1306,6 +1306,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.
>

I would recommend a slightly different comment:

/**
If set, the Clear Value Address field is set to point to a CLEAR_COLOR
state and the inline clear color fields are left empty.
*/



> > +    */
> > +   bool use_clear_address;
>
> I'm still wondering about this field. I think at the end we can just a
> assume that if gen >= 10 and aux_usage != ISL_AUX_USAGE_NONE, then
> we'll use the address.
>

That's not going to work if we want to turn this on for blorp, anv, and
i965 separately.


> I think you mentioned that it could be tough implement the support in
> steps if we had an all or nothing enaling of the address usage. But,
> does that mean that at the end of your series you could add a patch to
> remove this `use_clear_address` field?
>
> Maybe as a test in jenkins, you could add a patch that asserts that if
> gen >= 10 and there is an aux_buffer, then use_clear_address==true in
> your current series.
>
> -Jordan
>
> > +   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 bfb27fa4a44..e7c489df912 100644
> > --- a/src/intel/isl/isl_surface_state.c
> > +++ b/src/intel/isl/isl_surface_state.c
> > @@ -635,11 +635,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;
> > +         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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180226/24805a01/attachment.html>