[v4,10/18] i965/miptree: Add new BO for clear color.

Submitted by Rafael Antognolli on March 8, 2018, 4:49 p.m.

Details

Message ID 20180308164911.15375-11-rafael.antognolli@intel.com
State New
Headers show
Series "Use clear color address in surface state." ( rev: 4 3 ) in Mesa

Not browsing as part of any series.

Commit Message

Rafael Antognolli March 8, 2018, 4:49 p.m.
Add an extra BO to store clear color when we receive the aux buffer from
the window system. Since we have no control over the aux buffer size in
this case, we need the new BO to store only the clear color.

Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 18 ++++++++++++++++++
 1 file changed, 18 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 22d0ae89367..a8b89d9170a 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -969,6 +969,23 @@  create_ccs_buf_for_image(struct brw_context *brw,
       return false;
    }
 
+   /* On gen10+ we start using an extra space in the aux buffer to store the
+    * indirect clear color. However, if we imported an image from the window
+    * system with CCS, we don't have the extra space at the end of the aux
+    * buffer. So create a new bo here that will store that clear color.
+    */
+   const struct gen_device_info *devinfo = &brw->screen->devinfo;
+   if (devinfo->gen >= 10) {
+      mt->mcs_buf->clear_color_bo =
+         brw_bo_alloc(brw->bufmgr, "clear_color_bo",
+                      brw->isl_dev.ss.clear_color_state_size, 64);
+      if (!mt->mcs_buf->clear_color_bo) {
+         free(mt->mcs_buf);
+         mt->mcs_buf = NULL;
+         return false;
+      }
+   }
+
    mt->mcs_buf->bo = image->bo;
    brw_bo_reference(image->bo);
 
@@ -1211,6 +1228,7 @@  intel_miptree_aux_buffer_free(struct intel_miptree_aux_buffer *aux_buf)
       return;
 
    brw_bo_unreference(aux_buf->bo);
+   brw_bo_unreference(aux_buf->clear_color_bo);
 
    free(aux_buf);
 }

Comments

What about a subject like this?

i965/miptree: Add new clear color BO for winsys aux buffers

On 2018-03-08 08:49:03, Rafael Antognolli wrote:
> Add an extra BO to store clear color when we receive the aux buffer from
> the window system. Since we have no control over the aux buffer size in
> this case, we need the new BO to store only the clear color.
> 
> Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 22d0ae89367..a8b89d9170a 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -969,6 +969,23 @@ create_ccs_buf_for_image(struct brw_context *brw,
>        return false;
>     }
>  
> +   /* On gen10+ we start using an extra space in the aux buffer to store the
> +    * indirect clear color. However, if we imported an image from the window
> +    * system with CCS, we don't have the extra space at the end of the aux
> +    * buffer. So create a new bo here that will store that clear color.
> +    */
> +   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> +   if (devinfo->gen >= 10) {
> +      mt->mcs_buf->clear_color_bo =
> +         brw_bo_alloc(brw->bufmgr, "clear_color_bo",
> +                      brw->isl_dev.ss.clear_color_state_size, 64);
> +      if (!mt->mcs_buf->clear_color_bo) {
> +         free(mt->mcs_buf);
> +         mt->mcs_buf = NULL;
> +         return false;
> +      }
> +   }
> +
>     mt->mcs_buf->bo = image->bo;
>     brw_bo_reference(image->bo);
>  
> @@ -1211,6 +1228,7 @@ intel_miptree_aux_buffer_free(struct intel_miptree_aux_buffer *aux_buf)
>        return;
>  
>     brw_bo_unreference(aux_buf->bo);
> +   brw_bo_unreference(aux_buf->clear_color_bo);

Should this be added in the previous patch?

Should it only happen when gen >= 10? I guess it will be null for gen
< 10, so this will be a no-op.

-Jordan

>  
>     free(aux_buf);
>  }
> -- 
> 2.14.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Wed, Mar 14, 2018 at 05:26:08PM -0700, Jordan Justen wrote:
> What about a subject like this?
> 
> i965/miptree: Add new clear color BO for winsys aux buffers

It's definitely better. I'll update it.

> On 2018-03-08 08:49:03, Rafael Antognolli wrote:
> > Add an extra BO to store clear color when we receive the aux buffer from
> > the window system. Since we have no control over the aux buffer size in
> > this case, we need the new BO to store only the clear color.
> > 
> > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 22d0ae89367..a8b89d9170a 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -969,6 +969,23 @@ create_ccs_buf_for_image(struct brw_context *brw,
> >        return false;
> >     }
> >  
> > +   /* On gen10+ we start using an extra space in the aux buffer to store the
> > +    * indirect clear color. However, if we imported an image from the window
> > +    * system with CCS, we don't have the extra space at the end of the aux
> > +    * buffer. So create a new bo here that will store that clear color.
> > +    */
> > +   const struct gen_device_info *devinfo = &brw->screen->devinfo;
> > +   if (devinfo->gen >= 10) {
> > +      mt->mcs_buf->clear_color_bo =
> > +         brw_bo_alloc(brw->bufmgr, "clear_color_bo",
> > +                      brw->isl_dev.ss.clear_color_state_size, 64);
> > +      if (!mt->mcs_buf->clear_color_bo) {
> > +         free(mt->mcs_buf);
> > +         mt->mcs_buf = NULL;
> > +         return false;
> > +      }
> > +   }
> > +
> >     mt->mcs_buf->bo = image->bo;
> >     brw_bo_reference(image->bo);
> >  
> > @@ -1211,6 +1228,7 @@ intel_miptree_aux_buffer_free(struct intel_miptree_aux_buffer *aux_buf)
> >        return;
> >  
> >     brw_bo_unreference(aux_buf->bo);
> > +   brw_bo_unreference(aux_buf->clear_color_bo);
> 
> Should this be added in the previous patch?

Yeah, looks like I missed it, thanks :-/

> Should it only happen when gen >= 10? I guess it will be null for gen
> < 10, so this will be a no-op.
> 

Exactly, no-op, so I assumed it was safe to leave it like that.

> >  
> >     free(aux_buf);
> >  }
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev