Re: [Mesa-dev] [PATCH v4 10/18] i965/miptree: Add new BO for clear color.

[Rafael Antognolli]

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

Re: [Mesa-dev] [PATCH v4 10/18] i965/miptree: Add new BO for clear color.

[Jordan Justen]

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