Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-23 Thread Jason Ekstrand
On Mon, Apr 23, 2018 at 11:11 AM, Nanley Chery 
wrote:

> On Mon, Apr 23, 2018 at 11:01:12AM -0700, Jason Ekstrand wrote:
> > Why is this useful in light of patch 4?  I don't mean to be overly
> critical
> > but the main purpose of this helper seems to be to deal with the fact
> that
> > we have two different fields.  If it's just one field, why the helper?
> >
> > --Jason
> >
> > On Wed, Apr 11, 2018 at 1:42 PM, Nanley Chery 
> wrote:
> >
> > > Make the next patch easier to read by eliminating most of the would-be
> > > duplicate field accesses now.
>
> No worries, honest questions are welcome :) The rationale is mentioned
> here in the commit message. By the way, I leave the function behind in
> patch 4 because I'm currently working under the (possibly naive)
> assumption that getters are a good thing in general. That said, I
> haven't thought about it much and I'm not opposed to deleting it. Maybe
> I shouldn't introduce two ways of getting at the same field?
>

I'm not sure if getters are good in general or not.  I think they
frequently can be if the thing you're getting is complicated or if you want
to do a bunch of asserting around it.  I don't think they do any good if
it's feasable and common (which it is) to get at it directly.  In that
case, I think they just provide a false sense of security.


> -Nanley
>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16
> +---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24
> > > 
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17
> +
> > >  4 files changed, 24 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > index 5dcd95e9f44..962a316c5cf 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > >.aux_usage = aux_usage,
> > > };
> > >
> > > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > > -   if (mt->mcs_buf)
> > > -  aux_buf = mt->mcs_buf;
> > > -   else if (mt->hiz_buf)
> > > -  aux_buf = mt->hiz_buf;
> > > -
> > > if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> > > devinfo->gen <= 7)
> > >mt->r8stencil_needs_update = true;
> > > @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > > */
> > >surf->clear_color = mt->fast_clear_color;
> > >
> > > +  struct intel_miptree_aux_buffer *aux_buf =
> > > + intel_miptree_get_aux_buffer(mt);
> > >surf->aux_surf = _buf->surf;
> > >surf->aux_addr = (struct blorp_address) {
> > >   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > index 3fb101bf68b..06f739faf61 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> > > struct brw_bo *aux_bo = NULL;
> > > struct isl_surf *aux_surf = NULL;
> > > uint64_t aux_offset = 0;
> > > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > > -   switch (aux_usage) {
> > > -   case ISL_AUX_USAGE_MCS:
> > > -   case ISL_AUX_USAGE_CCS_D:
> > > -   case ISL_AUX_USAGE_CCS_E:
> > > -  aux_buf = mt->mcs_buf;
> > > -  break;
> > > -
> > > -   case ISL_AUX_USAGE_HIZ:
> > > -  aux_buf = mt->hiz_buf;
> > > -  break;
> > > -
> > > -   case ISL_AUX_USAGE_NONE:
> > > -  break;
> > > -   }
> > > +   struct intel_miptree_aux_buffer *aux_buf =
> > > intel_miptree_get_aux_buffer(mt);
> > >
> > > if (aux_usage != ISL_AUX_USAGE_NONE) {
> > >aux_surf = _buf->surf;
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index d95128de119..ba5b02bc0aa 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree
> **mt)
> > >brw_bo_unreference((*mt)->bo);
> > >intel_miptree_release(&(*mt)->stencil_mt);
> > >intel_miptree_release(&(*mt)->r8stencil_mt);
> > > -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> > > -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> > > +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*
> mt));
> > >free_aux_state_map((*mt)->aux_state);
> > >
> > >intel_miptree_release(&(*mt)->plane[0]);
> > > @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct
> brw_context
> > > *brw,
> > >  0, 

Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-23 Thread Nanley Chery
On Mon, Apr 23, 2018 at 11:01:12AM -0700, Jason Ekstrand wrote:
> Why is this useful in light of patch 4?  I don't mean to be overly critical
> but the main purpose of this helper seems to be to deal with the fact that
> we have two different fields.  If it's just one field, why the helper?
> 
> --Jason
> 
> On Wed, Apr 11, 2018 at 1:42 PM, Nanley Chery  wrote:
> 
> > Make the next patch easier to read by eliminating most of the would-be
> > duplicate field accesses now.

No worries, honest questions are welcome :) The rationale is mentioned
here in the commit message. By the way, I leave the function behind in
patch 4 because I'm currently working under the (possibly naive)
assumption that getters are a good thing in general. That said, I
haven't thought about it much and I'm not opposed to deleting it. Maybe
I shouldn't introduce two ways of getting at the same field?

-Nanley

> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24
> > 
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
> >  4 files changed, 24 insertions(+), 41 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index 5dcd95e9f44..962a316c5cf 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
> >.aux_usage = aux_usage,
> > };
> >
> > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > -   if (mt->mcs_buf)
> > -  aux_buf = mt->mcs_buf;
> > -   else if (mt->hiz_buf)
> > -  aux_buf = mt->hiz_buf;
> > -
> > if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> > devinfo->gen <= 7)
> >mt->r8stencil_needs_update = true;
> > @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > */
> >surf->clear_color = mt->fast_clear_color;
> >
> > +  struct intel_miptree_aux_buffer *aux_buf =
> > + intel_miptree_get_aux_buffer(mt);
> >surf->aux_surf = _buf->surf;
> >surf->aux_addr = (struct blorp_address) {
> >   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > index 3fb101bf68b..06f739faf61 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> > struct brw_bo *aux_bo = NULL;
> > struct isl_surf *aux_surf = NULL;
> > uint64_t aux_offset = 0;
> > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > -   switch (aux_usage) {
> > -   case ISL_AUX_USAGE_MCS:
> > -   case ISL_AUX_USAGE_CCS_D:
> > -   case ISL_AUX_USAGE_CCS_E:
> > -  aux_buf = mt->mcs_buf;
> > -  break;
> > -
> > -   case ISL_AUX_USAGE_HIZ:
> > -  aux_buf = mt->hiz_buf;
> > -  break;
> > -
> > -   case ISL_AUX_USAGE_NONE:
> > -  break;
> > -   }
> > +   struct intel_miptree_aux_buffer *aux_buf =
> > intel_miptree_get_aux_buffer(mt);
> >
> > if (aux_usage != ISL_AUX_USAGE_NONE) {
> >aux_surf = _buf->surf;
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index d95128de119..ba5b02bc0aa 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
> >brw_bo_unreference((*mt)->bo);
> >intel_miptree_release(&(*mt)->stencil_mt);
> >intel_miptree_release(&(*mt)->r8stencil_mt);
> > -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> > -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> > +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
> >free_aux_state_map((*mt)->aux_state);
> >
> >intel_miptree_release(&(*mt)->plane[0]);
> > @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context
> > *brw,
> >  0, INTEL_REMAINING_LAYERS,
> >  ISL_AUX_USAGE_NONE, false);
> >
> > -   if (mt->mcs_buf) {
> > -  intel_miptree_aux_buffer_free(mt->mcs_buf);
> > +   struct intel_miptree_aux_buffer *aux_buf =
> > intel_miptree_get_aux_buffer(mt);
> > +   if (aux_buf) {
> > +  intel_miptree_aux_buffer_free(aux_buf);
> >mt->mcs_buf = NULL;
> > -
> > -  /* Any pending MCS/CCS operations are no longer needed. Trying to
> > -   * execute any will likely crash due to the missing aux buffer. So
> > let's
> > -   * delete all pending ops.
> > -   */
> > -  free(mt->aux_state);
> > -  

Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-23 Thread Jason Ekstrand
Why is this useful in light of patch 4?  I don't mean to be overly critical
but the main purpose of this helper seems to be to deal with the fact that
we have two different fields.  If it's just one field, why the helper?

--Jason

On Wed, Apr 11, 2018 at 1:42 PM, Nanley Chery  wrote:

> Make the next patch easier to read by eliminating most of the would-be
> duplicate field accesses now.
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24
> 
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
>  4 files changed, 24 insertions(+), 41 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index 5dcd95e9f44..962a316c5cf 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
>.aux_usage = aux_usage,
> };
>
> -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> -   if (mt->mcs_buf)
> -  aux_buf = mt->mcs_buf;
> -   else if (mt->hiz_buf)
> -  aux_buf = mt->hiz_buf;
> -
> if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> devinfo->gen <= 7)
>mt->r8stencil_needs_update = true;
> @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> */
>surf->clear_color = mt->fast_clear_color;
>
> +  struct intel_miptree_aux_buffer *aux_buf =
> + intel_miptree_get_aux_buffer(mt);
>surf->aux_surf = _buf->surf;
>surf->aux_addr = (struct blorp_address) {
>   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 3fb101bf68b..06f739faf61 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> struct brw_bo *aux_bo = NULL;
> struct isl_surf *aux_surf = NULL;
> uint64_t aux_offset = 0;
> -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> -   switch (aux_usage) {
> -   case ISL_AUX_USAGE_MCS:
> -   case ISL_AUX_USAGE_CCS_D:
> -   case ISL_AUX_USAGE_CCS_E:
> -  aux_buf = mt->mcs_buf;
> -  break;
> -
> -   case ISL_AUX_USAGE_HIZ:
> -  aux_buf = mt->hiz_buf;
> -  break;
> -
> -   case ISL_AUX_USAGE_NONE:
> -  break;
> -   }
> +   struct intel_miptree_aux_buffer *aux_buf =
> intel_miptree_get_aux_buffer(mt);
>
> if (aux_usage != ISL_AUX_USAGE_NONE) {
>aux_surf = _buf->surf;
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index d95128de119..ba5b02bc0aa 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>brw_bo_unreference((*mt)->bo);
>intel_miptree_release(&(*mt)->stencil_mt);
>intel_miptree_release(&(*mt)->r8stencil_mt);
> -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
>free_aux_state_map((*mt)->aux_state);
>
>intel_miptree_release(&(*mt)->plane[0]);
> @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context
> *brw,
>  0, INTEL_REMAINING_LAYERS,
>  ISL_AUX_USAGE_NONE, false);
>
> -   if (mt->mcs_buf) {
> -  intel_miptree_aux_buffer_free(mt->mcs_buf);
> +   struct intel_miptree_aux_buffer *aux_buf =
> intel_miptree_get_aux_buffer(mt);
> +   if (aux_buf) {
> +  intel_miptree_aux_buffer_free(aux_buf);
>mt->mcs_buf = NULL;
> -
> -  /* Any pending MCS/CCS operations are no longer needed. Trying to
> -   * execute any will likely crash due to the missing aux buffer. So
> let's
> -   * delete all pending ops.
> -   */
> -  free(mt->aux_state);
> -  mt->aux_state = NULL;
> -  brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
> -   }
> -
> -   if (mt->hiz_buf) {
> -  intel_miptree_aux_buffer_free(mt->hiz_buf);
>mt->hiz_buf = NULL;
>
>for (uint32_t l = mt->first_level; l <= mt->last_level; ++l) {
>   mt->level[l].has_hiz = false;
>}
>
> -  /* Any pending HiZ operations are no longer needed. Trying to
> execute
> -   * any will likely crash due to the missing aux buffer. So let's
> delete
> -   * all pending ops.
> -   */
>free(mt->aux_state);
>mt->aux_state = NULL;
>brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
> diff --git 

Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-23 Thread Nanley Chery
On Fri, Apr 20, 2018 at 03:51:34PM -0700, Rafael Antognolli wrote:
> On Fri, Apr 20, 2018 at 02:38:37PM -0700, Nanley Chery wrote:
> > On Fri, Apr 20, 2018 at 09:58:38AM -0700, Rafael Antognolli wrote:
> > > Nice, I was planning to do something like this later but didn't want to
> > > include many more changes on my ongoing series. This looks great, just a
> > > little comment below.
> > > 
> > > On Wed, Apr 11, 2018 at 01:42:20PM -0700, Nanley Chery wrote:
> > > > Make the next patch easier to read by eliminating most of the would-be
> > > > duplicate field accesses now.
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
> > > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
> > > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24 
> > > > 
> > > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
> > > >  4 files changed, 24 insertions(+), 41 deletions(-)
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c 
> > > > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > index 5dcd95e9f44..962a316c5cf 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > > >.aux_usage = aux_usage,
> > > > };
> > > >  
> > > > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > > > -   if (mt->mcs_buf)
> > > > -  aux_buf = mt->mcs_buf;
> > > > -   else if (mt->hiz_buf)
> > > > -  aux_buf = mt->hiz_buf;
> > > > -
> > > > if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> > > > devinfo->gen <= 7)
> > > >mt->r8stencil_needs_update = true;
> > > > @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > > > */
> > > >surf->clear_color = mt->fast_clear_color;
> > > >  
> > > > +  struct intel_miptree_aux_buffer *aux_buf =
> > > > + intel_miptree_get_aux_buffer(mt);
> > > >surf->aux_surf = _buf->surf;
> > > >surf->aux_addr = (struct blorp_address) {
> > > >   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> > > > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > > index 3fb101bf68b..06f739faf61 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > > @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> > > > struct brw_bo *aux_bo = NULL;
> > > > struct isl_surf *aux_surf = NULL;
> > > > uint64_t aux_offset = 0;
> > > > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > > > -   switch (aux_usage) {
> > > > -   case ISL_AUX_USAGE_MCS:
> > > > -   case ISL_AUX_USAGE_CCS_D:
> > > > -   case ISL_AUX_USAGE_CCS_E:
> > > > -  aux_buf = mt->mcs_buf;
> > > > -  break;
> > > > -
> > > > -   case ISL_AUX_USAGE_HIZ:
> > > > -  aux_buf = mt->hiz_buf;
> > > > -  break;
> > > > -
> > > > -   case ISL_AUX_USAGE_NONE:
> > > > -  break;
> > > > -   }
> > > > +   struct intel_miptree_aux_buffer *aux_buf = 
> > > > intel_miptree_get_aux_buffer(mt);
> > > >  
> > > > if (aux_usage != ISL_AUX_USAGE_NONE) {
> > > >aux_surf = _buf->surf;
> > > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> > > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > index d95128de119..ba5b02bc0aa 100644
> > > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree 
> > > > **mt)
> > > >brw_bo_unreference((*mt)->bo);
> > > >intel_miptree_release(&(*mt)->stencil_mt);
> > > >intel_miptree_release(&(*mt)->r8stencil_mt);
> > > > -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> > > > -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> > > > +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
> > > >free_aux_state_map((*mt)->aux_state);
> > > >  
> > > >intel_miptree_release(&(*mt)->plane[0]);
> > > > @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context 
> > > > *brw,
> > > >  0, INTEL_REMAINING_LAYERS,
> > > >  ISL_AUX_USAGE_NONE, false);
> > > >  
> > > > -   if (mt->mcs_buf) {
> > > > -  intel_miptree_aux_buffer_free(mt->mcs_buf);
> > > > +   struct intel_miptree_aux_buffer *aux_buf = 
> > > > intel_miptree_get_aux_buffer(mt);
> > > > +   if (aux_buf) {
> > > > +  intel_miptree_aux_buffer_free(aux_buf);
> > > >mt->mcs_buf = NULL;
> > > > -
> > > > -  /* Any pending MCS/CCS operations are no longer needed. Trying to
> > > > -   * execute any will likely crash due to the missing aux buffer. 
> > > > So let's
> > > > -   * delete all 

Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-20 Thread Rafael Antognolli
On Fri, Apr 20, 2018 at 02:38:37PM -0700, Nanley Chery wrote:
> On Fri, Apr 20, 2018 at 09:58:38AM -0700, Rafael Antognolli wrote:
> > Nice, I was planning to do something like this later but didn't want to
> > include many more changes on my ongoing series. This looks great, just a
> > little comment below.
> > 
> > On Wed, Apr 11, 2018 at 01:42:20PM -0700, Nanley Chery wrote:
> > > Make the next patch easier to read by eliminating most of the would-be
> > > duplicate field accesses now.
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24 
> > > 
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
> > >  4 files changed, 24 insertions(+), 41 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c 
> > > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > index 5dcd95e9f44..962a316c5cf 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > >.aux_usage = aux_usage,
> > > };
> > >  
> > > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > > -   if (mt->mcs_buf)
> > > -  aux_buf = mt->mcs_buf;
> > > -   else if (mt->hiz_buf)
> > > -  aux_buf = mt->hiz_buf;
> > > -
> > > if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> > > devinfo->gen <= 7)
> > >mt->r8stencil_needs_update = true;
> > > @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > > */
> > >surf->clear_color = mt->fast_clear_color;
> > >  
> > > +  struct intel_miptree_aux_buffer *aux_buf =
> > > + intel_miptree_get_aux_buffer(mt);
> > >surf->aux_surf = _buf->surf;
> > >surf->aux_addr = (struct blorp_address) {
> > >   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> > > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > index 3fb101bf68b..06f739faf61 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> > > struct brw_bo *aux_bo = NULL;
> > > struct isl_surf *aux_surf = NULL;
> > > uint64_t aux_offset = 0;
> > > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > > -   switch (aux_usage) {
> > > -   case ISL_AUX_USAGE_MCS:
> > > -   case ISL_AUX_USAGE_CCS_D:
> > > -   case ISL_AUX_USAGE_CCS_E:
> > > -  aux_buf = mt->mcs_buf;
> > > -  break;
> > > -
> > > -   case ISL_AUX_USAGE_HIZ:
> > > -  aux_buf = mt->hiz_buf;
> > > -  break;
> > > -
> > > -   case ISL_AUX_USAGE_NONE:
> > > -  break;
> > > -   }
> > > +   struct intel_miptree_aux_buffer *aux_buf = 
> > > intel_miptree_get_aux_buffer(mt);
> > >  
> > > if (aux_usage != ISL_AUX_USAGE_NONE) {
> > >aux_surf = _buf->surf;
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index d95128de119..ba5b02bc0aa 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
> > >brw_bo_unreference((*mt)->bo);
> > >intel_miptree_release(&(*mt)->stencil_mt);
> > >intel_miptree_release(&(*mt)->r8stencil_mt);
> > > -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> > > -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> > > +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
> > >free_aux_state_map((*mt)->aux_state);
> > >  
> > >intel_miptree_release(&(*mt)->plane[0]);
> > > @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context 
> > > *brw,
> > >  0, INTEL_REMAINING_LAYERS,
> > >  ISL_AUX_USAGE_NONE, false);
> > >  
> > > -   if (mt->mcs_buf) {
> > > -  intel_miptree_aux_buffer_free(mt->mcs_buf);
> > > +   struct intel_miptree_aux_buffer *aux_buf = 
> > > intel_miptree_get_aux_buffer(mt);
> > > +   if (aux_buf) {
> > > +  intel_miptree_aux_buffer_free(aux_buf);
> > >mt->mcs_buf = NULL;
> > > -
> > > -  /* Any pending MCS/CCS operations are no longer needed. Trying to
> > > -   * execute any will likely crash due to the missing aux buffer. So 
> > > let's
> > > -   * delete all pending ops.
> > > -   */
> > 
> > It looks like the above comment, and the equivalent one about HiZ, just
> > disappear. Unless there's a reason for that, I think you should still
> > keep them, maybe like "Any pending HiZ/MCS/CCS operations...".
> > 
> > With that, this patch is
> > 

Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-20 Thread Nanley Chery
On Fri, Apr 20, 2018 at 09:58:38AM -0700, Rafael Antognolli wrote:
> Nice, I was planning to do something like this later but didn't want to
> include many more changes on my ongoing series. This looks great, just a
> little comment below.
> 
> On Wed, Apr 11, 2018 at 01:42:20PM -0700, Nanley Chery wrote:
> > Make the next patch easier to read by eliminating most of the would-be
> > duplicate field accesses now.
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24 
> > 
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
> >  4 files changed, 24 insertions(+), 41 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c 
> > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index 5dcd95e9f44..962a316c5cf 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
> >.aux_usage = aux_usage,
> > };
> >  
> > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > -   if (mt->mcs_buf)
> > -  aux_buf = mt->mcs_buf;
> > -   else if (mt->hiz_buf)
> > -  aux_buf = mt->hiz_buf;
> > -
> > if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> > devinfo->gen <= 7)
> >mt->r8stencil_needs_update = true;
> > @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> > */
> >surf->clear_color = mt->fast_clear_color;
> >  
> > +  struct intel_miptree_aux_buffer *aux_buf =
> > + intel_miptree_get_aux_buffer(mt);
> >surf->aux_surf = _buf->surf;
> >surf->aux_addr = (struct blorp_address) {
> >   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > index 3fb101bf68b..06f739faf61 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> > struct brw_bo *aux_bo = NULL;
> > struct isl_surf *aux_surf = NULL;
> > uint64_t aux_offset = 0;
> > -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> > -   switch (aux_usage) {
> > -   case ISL_AUX_USAGE_MCS:
> > -   case ISL_AUX_USAGE_CCS_D:
> > -   case ISL_AUX_USAGE_CCS_E:
> > -  aux_buf = mt->mcs_buf;
> > -  break;
> > -
> > -   case ISL_AUX_USAGE_HIZ:
> > -  aux_buf = mt->hiz_buf;
> > -  break;
> > -
> > -   case ISL_AUX_USAGE_NONE:
> > -  break;
> > -   }
> > +   struct intel_miptree_aux_buffer *aux_buf = 
> > intel_miptree_get_aux_buffer(mt);
> >  
> > if (aux_usage != ISL_AUX_USAGE_NONE) {
> >aux_surf = _buf->surf;
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index d95128de119..ba5b02bc0aa 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
> >brw_bo_unreference((*mt)->bo);
> >intel_miptree_release(&(*mt)->stencil_mt);
> >intel_miptree_release(&(*mt)->r8stencil_mt);
> > -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> > -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> > +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
> >free_aux_state_map((*mt)->aux_state);
> >  
> >intel_miptree_release(&(*mt)->plane[0]);
> > @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context 
> > *brw,
> >  0, INTEL_REMAINING_LAYERS,
> >  ISL_AUX_USAGE_NONE, false);
> >  
> > -   if (mt->mcs_buf) {
> > -  intel_miptree_aux_buffer_free(mt->mcs_buf);
> > +   struct intel_miptree_aux_buffer *aux_buf = 
> > intel_miptree_get_aux_buffer(mt);
> > +   if (aux_buf) {
> > +  intel_miptree_aux_buffer_free(aux_buf);
> >mt->mcs_buf = NULL;
> > -
> > -  /* Any pending MCS/CCS operations are no longer needed. Trying to
> > -   * execute any will likely crash due to the missing aux buffer. So 
> > let's
> > -   * delete all pending ops.
> > -   */
> 
> It looks like the above comment, and the equivalent one about HiZ, just
> disappear. Unless there's a reason for that, I think you should still
> keep them, maybe like "Any pending HiZ/MCS/CCS operations...".
> 
> With that, this patch is
> 
> Reviewed-by: Rafael Antognolli 
> 

Thanks for the reviews thus far! I deleted this comment because I
couldn't really make sense of it. After looking into it's origin (commit
de0b0a3a9cfd25ac5082223322002710a23da8a), I think it was placed above
the 

Re: [Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-20 Thread Rafael Antognolli
Nice, I was planning to do something like this later but didn't want to
include many more changes on my ongoing series. This looks great, just a
little comment below.

On Wed, Apr 11, 2018 at 01:42:20PM -0700, Nanley Chery wrote:
> Make the next patch easier to read by eliminating most of the would-be
> duplicate field accesses now.
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24 
> 
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
>  4 files changed, 24 insertions(+), 41 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c 
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index 5dcd95e9f44..962a316c5cf 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
>.aux_usage = aux_usage,
> };
>  
> -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> -   if (mt->mcs_buf)
> -  aux_buf = mt->mcs_buf;
> -   else if (mt->hiz_buf)
> -  aux_buf = mt->hiz_buf;
> -
> if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
> devinfo->gen <= 7)
>mt->r8stencil_needs_update = true;
> @@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
> */
>surf->clear_color = mt->fast_clear_color;
>  
> +  struct intel_miptree_aux_buffer *aux_buf =
> + intel_miptree_get_aux_buffer(mt);
>surf->aux_surf = _buf->surf;
>surf->aux_addr = (struct blorp_address) {
>   .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 3fb101bf68b..06f739faf61 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
> struct brw_bo *aux_bo = NULL;
> struct isl_surf *aux_surf = NULL;
> uint64_t aux_offset = 0;
> -   struct intel_miptree_aux_buffer *aux_buf = NULL;
> -   switch (aux_usage) {
> -   case ISL_AUX_USAGE_MCS:
> -   case ISL_AUX_USAGE_CCS_D:
> -   case ISL_AUX_USAGE_CCS_E:
> -  aux_buf = mt->mcs_buf;
> -  break;
> -
> -   case ISL_AUX_USAGE_HIZ:
> -  aux_buf = mt->hiz_buf;
> -  break;
> -
> -   case ISL_AUX_USAGE_NONE:
> -  break;
> -   }
> +   struct intel_miptree_aux_buffer *aux_buf = 
> intel_miptree_get_aux_buffer(mt);
>  
> if (aux_usage != ISL_AUX_USAGE_NONE) {
>aux_surf = _buf->surf;
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index d95128de119..ba5b02bc0aa 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>brw_bo_unreference((*mt)->bo);
>intel_miptree_release(&(*mt)->stencil_mt);
>intel_miptree_release(&(*mt)->r8stencil_mt);
> -  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
> -  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
> +  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
>free_aux_state_map((*mt)->aux_state);
>  
>intel_miptree_release(&(*mt)->plane[0]);
> @@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context *brw,
>  0, INTEL_REMAINING_LAYERS,
>  ISL_AUX_USAGE_NONE, false);
>  
> -   if (mt->mcs_buf) {
> -  intel_miptree_aux_buffer_free(mt->mcs_buf);
> +   struct intel_miptree_aux_buffer *aux_buf = 
> intel_miptree_get_aux_buffer(mt);
> +   if (aux_buf) {
> +  intel_miptree_aux_buffer_free(aux_buf);
>mt->mcs_buf = NULL;
> -
> -  /* Any pending MCS/CCS operations are no longer needed. Trying to
> -   * execute any will likely crash due to the missing aux buffer. So 
> let's
> -   * delete all pending ops.
> -   */

It looks like the above comment, and the equivalent one about HiZ, just
disappear. Unless there's a reason for that, I think you should still
keep them, maybe like "Any pending HiZ/MCS/CCS operations...".

With that, this patch is

Reviewed-by: Rafael Antognolli 

> -  free(mt->aux_state);
> -  mt->aux_state = NULL;
> -  brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
> -   }
> -
> -   if (mt->hiz_buf) {
> -  intel_miptree_aux_buffer_free(mt->hiz_buf);
>mt->hiz_buf = NULL;
>  
>for (uint32_t l = mt->first_level; l <= mt->last_level; ++l) {
>   mt->level[l].has_hiz = false;
>}
>  
> -  /* Any pending HiZ operations are no longer needed. Trying to execute
> -   * any will likely crash due to the missing aux buffer. So 

[Mesa-dev] [PATCH v3 3/9] i965: Add and use a getter for the miptree aux buffer

2018-04-11 Thread Nanley Chery
Make the next patch easier to read by eliminating most of the would-be
duplicate field accesses now.
---
 src/mesa/drivers/dri/i965/brw_blorp.c|  8 ++--
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 16 +---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 24 
 src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 17 +
 4 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c 
b/src/mesa/drivers/dri/i965/brw_blorp.c
index 5dcd95e9f44..962a316c5cf 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -154,12 +154,6 @@ blorp_surf_for_miptree(struct brw_context *brw,
   .aux_usage = aux_usage,
};
 
-   struct intel_miptree_aux_buffer *aux_buf = NULL;
-   if (mt->mcs_buf)
-  aux_buf = mt->mcs_buf;
-   else if (mt->hiz_buf)
-  aux_buf = mt->hiz_buf;
-
if (mt->format == MESA_FORMAT_S_UINT8 && is_render_target &&
devinfo->gen <= 7)
   mt->r8stencil_needs_update = true;
@@ -174,6 +168,8 @@ blorp_surf_for_miptree(struct brw_context *brw,
*/
   surf->clear_color = mt->fast_clear_color;
 
+  struct intel_miptree_aux_buffer *aux_buf =
+ intel_miptree_get_aux_buffer(mt);
   surf->aux_surf = _buf->surf;
   surf->aux_addr = (struct blorp_address) {
  .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0,
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 3fb101bf68b..06f739faf61 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -155,21 +155,7 @@ brw_emit_surface_state(struct brw_context *brw,
struct brw_bo *aux_bo = NULL;
struct isl_surf *aux_surf = NULL;
uint64_t aux_offset = 0;
-   struct intel_miptree_aux_buffer *aux_buf = NULL;
-   switch (aux_usage) {
-   case ISL_AUX_USAGE_MCS:
-   case ISL_AUX_USAGE_CCS_D:
-   case ISL_AUX_USAGE_CCS_E:
-  aux_buf = mt->mcs_buf;
-  break;
-
-   case ISL_AUX_USAGE_HIZ:
-  aux_buf = mt->hiz_buf;
-  break;
-
-   case ISL_AUX_USAGE_NONE:
-  break;
-   }
+   struct intel_miptree_aux_buffer *aux_buf = intel_miptree_get_aux_buffer(mt);
 
if (aux_usage != ISL_AUX_USAGE_NONE) {
   aux_surf = _buf->surf;
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index d95128de119..ba5b02bc0aa 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1249,8 +1249,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
   brw_bo_unreference((*mt)->bo);
   intel_miptree_release(&(*mt)->stencil_mt);
   intel_miptree_release(&(*mt)->r8stencil_mt);
-  intel_miptree_aux_buffer_free((*mt)->hiz_buf);
-  intel_miptree_aux_buffer_free((*mt)->mcs_buf);
+  intel_miptree_aux_buffer_free(intel_miptree_get_aux_buffer(*mt));
   free_aux_state_map((*mt)->aux_state);
 
   intel_miptree_release(&(*mt)->plane[0]);
@@ -2876,31 +2875,16 @@ intel_miptree_make_shareable(struct brw_context *brw,
 0, INTEL_REMAINING_LAYERS,
 ISL_AUX_USAGE_NONE, false);
 
-   if (mt->mcs_buf) {
-  intel_miptree_aux_buffer_free(mt->mcs_buf);
+   struct intel_miptree_aux_buffer *aux_buf = intel_miptree_get_aux_buffer(mt);
+   if (aux_buf) {
+  intel_miptree_aux_buffer_free(aux_buf);
   mt->mcs_buf = NULL;
-
-  /* Any pending MCS/CCS operations are no longer needed. Trying to
-   * execute any will likely crash due to the missing aux buffer. So let's
-   * delete all pending ops.
-   */
-  free(mt->aux_state);
-  mt->aux_state = NULL;
-  brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
-   }
-
-   if (mt->hiz_buf) {
-  intel_miptree_aux_buffer_free(mt->hiz_buf);
   mt->hiz_buf = NULL;
 
   for (uint32_t l = mt->first_level; l <= mt->last_level; ++l) {
  mt->level[l].has_hiz = false;
   }
 
-  /* Any pending HiZ operations are no longer needed. Trying to execute
-   * any will likely crash due to the missing aux buffer. So let's delete
-   * all pending ops.
-   */
   free(mt->aux_state);
   mt->aux_state = NULL;
   brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 2f754427fc5..8fe5c4add67 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -485,6 +485,23 @@ enum isl_dim_layout
 get_isl_dim_layout(const struct gen_device_info *devinfo,
enum isl_tiling tiling, GLenum target);
 
+static inline struct intel_miptree_aux_buffer *
+intel_miptree_get_aux_buffer(const struct intel_mipmap_tree *mt)
+{
+   switch (mt->aux_usage) {
+   case ISL_AUX_USAGE_MCS:
+