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

Jason Ekstrand jason at jlekstrand.net
Mon Apr 23 18:01:12 UTC 2018


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 <nanleychery at gmail.com> 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 = &aux_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 = &aux_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:
> +   case ISL_AUX_USAGE_CCS_D:
> +   case ISL_AUX_USAGE_CCS_E:
> +      return mt->mcs_buf;
> +   case ISL_AUX_USAGE_HIZ:
> +      return mt->hiz_buf;
> +   case ISL_AUX_USAGE_NONE:
> +      return NULL;
> +   default:
> +      unreachable("Invalid aux_usage!\n");
> +   }
> +}
> +
>  void
>  intel_get_image_dims(struct gl_texture_image *image,
>                       int *width, int *height, int *depth);
> --
> 2.16.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180423/08d70111/attachment-0001.html>


More information about the mesa-dev mailing list