[5/8] i965/miptree: Add space to store the clear value in the aux surface.

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

Details

Message ID 20171215225335.28009-5-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.
Similarly to vulkan where we store the clear value in the aux surface,
we can do the same in GL.

Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index ead0c359c0f..6400a2a616a 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1663,6 +1663,21 @@  intel_miptree_init_mcs(struct brw_context *brw,
    brw_bo_unmap(mt->mcs_buf->bo);
 }
 
+static unsigned
+fast_clear_state_entry_size(const struct brw_context *brw)
+{
+   assert(brw);
+
+   /* Entry contents:
+    *   +--------------------------------------------+
+    *   | clear value dword(s)                       |
+    *   +--------------------------------------------+
+    */
+   assert(brw->isl_dev.ss.clear_value_size % 4 == 0);
+
+   return brw->isl_dev.ss.clear_value_size;
+}
+
 static struct intel_miptree_aux_buffer *
 intel_alloc_aux_buffer(struct brw_context *brw,
                        const char *name,
@@ -1675,6 +1690,16 @@  intel_alloc_aux_buffer(struct brw_context *brw,
       return false;
 
    buf->size = aux_surf->size;
+
+   const struct gen_device_info *devinfo = &brw->screen->devinfo;
+   if (devinfo->gen >= 10) {
+      /* On CNL, instead of setting the clear color in the SURFACE_STATE, we
+       * will set a pointer to a dword somewhere that contains the color. So,
+       * allocate the space for the clear color value here on the aux buffer.
+       */
+      buf->size += fast_clear_state_entry_size(brw);
+   }
+
    buf->pitch = aux_surf->row_pitch;
    buf->qpitch = isl_surf_get_array_pitch_sa_rows(aux_surf);
 

Comments

On Fri, Dec 15, 2017 at 02:53:32PM -0800, Rafael Antognolli wrote:
> Similarly to vulkan where we store the clear value in the aux surface,
> we can do the same in GL.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index ead0c359c0f..6400a2a616a 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1663,6 +1663,21 @@ intel_miptree_init_mcs(struct brw_context *brw,
>     brw_bo_unmap(mt->mcs_buf->bo);
>  }
>  
> +static unsigned
> +fast_clear_state_entry_size(const struct brw_context *brw)
> +{
> +   assert(brw);
> +
> +   /* Entry contents:
> +    *   +--------------------------------------------+
> +    *   | clear value dword(s)                       |
> +    *   +--------------------------------------------+
> +    */
> +   assert(brw->isl_dev.ss.clear_value_size % 4 == 0);
> +
> +   return brw->isl_dev.ss.clear_value_size;
> +}
> +

Do you think more may fields may be added to this in the future? I'm
trying to understand why we have an additional function here.

>  static struct intel_miptree_aux_buffer *
>  intel_alloc_aux_buffer(struct brw_context *brw,
>                         const char *name,
> @@ -1675,6 +1690,16 @@ intel_alloc_aux_buffer(struct brw_context *brw,
>        return false;
>  
>     buf->size = aux_surf->size;
> +
> +   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> +   if (devinfo->gen >= 10) {
> +      /* On CNL, instead of setting the clear color in the SURFACE_STATE, we
> +       * will set a pointer to a dword somewhere that contains the color. So,
> +       * allocate the space for the clear color value here on the aux buffer.
> +       */
> +      buf->size += fast_clear_state_entry_size(brw);

I like the plan of allocating more space here.

> +   }
> +
>     buf->pitch = aux_surf->row_pitch;
>     buf->qpitch = isl_surf_get_array_pitch_sa_rows(aux_surf);
>  
> -- 
> 2.14.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Mon, Jan 08, 2018 at 03:14:54PM -0800, Nanley Chery wrote:
> On Fri, Dec 15, 2017 at 02:53:32PM -0800, Rafael Antognolli wrote:
> > Similarly to vulkan where we store the clear value in the aux surface,
> > we can do the same in GL.
> > 
> > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index ead0c359c0f..6400a2a616a 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1663,6 +1663,21 @@ intel_miptree_init_mcs(struct brw_context *brw,
> >     brw_bo_unmap(mt->mcs_buf->bo);
> >  }
> >  
> > +static unsigned
> > +fast_clear_state_entry_size(const struct brw_context *brw)
> > +{
> > +   assert(brw);
> > +
> > +   /* Entry contents:
> > +    *   +--------------------------------------------+
> > +    *   | clear value dword(s)                       |
> > +    *   +--------------------------------------------+
> > +    */
> > +   assert(brw->isl_dev.ss.clear_value_size % 4 == 0);
> > +
> > +   return brw->isl_dev.ss.clear_value_size;
> > +}
> > +
> 
> Do you think more may fields may be added to this in the future? I'm
> trying to understand why we have an additional function here.

No, I just tried to make it similar to anv, but anv at least has a
reason to have such function. I'll remove it in the next iteration.

> >  static struct intel_miptree_aux_buffer *
> >  intel_alloc_aux_buffer(struct brw_context *brw,
> >                         const char *name,
> > @@ -1675,6 +1690,16 @@ intel_alloc_aux_buffer(struct brw_context *brw,
> >        return false;
> >  
> >     buf->size = aux_surf->size;
> > +
> > +   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> > +   if (devinfo->gen >= 10) {
> > +      /* On CNL, instead of setting the clear color in the SURFACE_STATE, we
> > +       * will set a pointer to a dword somewhere that contains the color. So,
> > +       * allocate the space for the clear color value here on the aux buffer.
> > +       */
> > +      buf->size += fast_clear_state_entry_size(brw);
> 
> I like the plan of allocating more space here.
> 
> > +   }
> > +
> >     buf->pitch = aux_surf->row_pitch;
> >     buf->qpitch = isl_surf_get_array_pitch_sa_rows(aux_surf);
> >  
> > -- 
> > 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