[2/8] intel/isl: Add support to emit clear value address.

Submitted by Rafael Antognolli on Dec. 15, 2017, 10:53 p.m.

Details

Message ID 20171215225335.28009-2-rafael.antognolli@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Rafael Antognolli Dec. 15, 2017, 10:53 p.m.
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.

Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 src/intel/isl/isl.h               |  9 +++++++++
 src/intel/isl/isl_surface_state.c | 15 +++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
index e3acb0ec280..c6e1fee27c1 100644
--- a/src/intel/isl/isl.h
+++ b/src/intel/isl/isl.h
@@ -1277,6 +1277,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 bfb27fa4a44..14741459687 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -635,11 +635,18 @@  isl_genX(surf_fill_state_s)(const struct isl_device *dev, void *state,
 #endif
 
    if (info->aux_usage != ISL_AUX_USAGE_NONE) {
+#if GEN_GEN >= 10
+      s.ClearValueAddressEnable = info->use_clear_address;
+      s.ClearValueAddressHigh = info->clear_address >> 32;
+      s.ClearValueAddressLow = info->clear_address;
+#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.

Comments

On Fri, Dec 15, 2017 at 02:53:29PM -0800, 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.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> ---
>  src/intel/isl/isl.h               |  9 +++++++++
>  src/intel/isl/isl_surface_state.c | 15 +++++++++++----
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index e3acb0ec280..c6e1fee27c1 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -1277,6 +1277,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 bfb27fa4a44..14741459687 100644
> --- a/src/intel/isl/isl_surface_state.c
> +++ b/src/intel/isl/isl_surface_state.c
> @@ -635,11 +635,18 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, void *state,
>  #endif
>  
>     if (info->aux_usage != ISL_AUX_USAGE_NONE) {
> +#if GEN_GEN >= 10
> +      s.ClearValueAddressEnable = info->use_clear_address;
> +      s.ClearValueAddressHigh = info->clear_address >> 32;
> +      s.ClearValueAddressLow = info->clear_address;
> +#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];
> +      }

It'd be nice to assert that use_clear_address is false for gen9.

-Nanley

>  #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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Mon, Jan 8, 2018 at 2:29 PM, Nanley Chery <nanleychery@gmail.com> wrote:

> On Fri, Dec 15, 2017 at 02:53:29PM -0800, 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.
> >
> > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> > ---
> >  src/intel/isl/isl.h               |  9 +++++++++
> >  src/intel/isl/isl_surface_state.c | 15 +++++++++++----
> >  2 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > index e3acb0ec280..c6e1fee27c1 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -1277,6 +1277,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 bfb27fa4a44..14741459687 100644
> > --- a/src/intel/isl/isl_surface_state.c
> > +++ b/src/intel/isl/isl_surface_state.c
> > @@ -635,11 +635,18 @@ isl_genX(surf_fill_state_s)(const struct
> isl_device *dev, void *state,
> >  #endif
> >
> >     if (info->aux_usage != ISL_AUX_USAGE_NONE) {
> > +#if GEN_GEN >= 10
> > +      s.ClearValueAddressEnable = info->use_clear_address;
> > +      s.ClearValueAddressHigh = info->clear_address >> 32;
> > +      s.ClearValueAddressLow = info->clear_address;
> > +#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];
> > +      }
>
> It'd be nice to assert that use_clear_address is false for gen9.
>

Yes it would.  How about something like this:

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
} else {
   // Set clear colors
}


> -Nanley
>
> >  #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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
On Mon, Jan 08, 2018 at 04:00:37PM -0800, Jason Ekstrand wrote:
> On Mon, Jan 8, 2018 at 2:29 PM, Nanley Chery <nanleychery@gmail.com> wrote:
> 
> > On Fri, Dec 15, 2017 at 02:53:29PM -0800, 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.
> > >
> > > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> > > ---
> > >  src/intel/isl/isl.h               |  9 +++++++++
> > >  src/intel/isl/isl_surface_state.c | 15 +++++++++++----
> > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > > index e3acb0ec280..c6e1fee27c1 100644
> > > --- a/src/intel/isl/isl.h
> > > +++ b/src/intel/isl/isl.h
> > > @@ -1277,6 +1277,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 bfb27fa4a44..14741459687 100644
> > > --- a/src/intel/isl/isl_surface_state.c
> > > +++ b/src/intel/isl/isl_surface_state.c
> > > @@ -635,11 +635,18 @@ isl_genX(surf_fill_state_s)(const struct
> > isl_device *dev, void *state,
> > >  #endif
> > >
> > >     if (info->aux_usage != ISL_AUX_USAGE_NONE) {
> > > +#if GEN_GEN >= 10
> > > +      s.ClearValueAddressEnable = info->use_clear_address;
> > > +      s.ClearValueAddressHigh = info->clear_address >> 32;
> > > +      s.ClearValueAddressLow = info->clear_address;
> > > +#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];
> > > +      }
> >
> > It'd be nice to assert that use_clear_address is false for gen9.
> >
> 
> Yes it would.  How about something like this:
> 
> 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
> } else {
>    // Set clear colors
> }
> 
> 

Looks good to me.

> > -Nanley
> >
> > >  #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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >