On Thu, May 3, 2018 at 12:03 PM, Nanley Chery <nanleych...@gmail.com> wrote:
> There isn't much that changes between the aux allocation functions.
> Remove the duplicated code.
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 227
> +++++++++++---------------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 9 -
> 2 files changed, 95 insertions(+), 141 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 84be7c07c6f..08976d2680f 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1736,95 +1736,9 @@ intel_alloc_aux_buffer(struct brw_context *brw,
> return buf;
> }
>
> -static bool
> -intel_miptree_alloc_mcs(struct brw_context *brw,
> - struct intel_mipmap_tree *mt,
> - GLuint num_samples)
> -{
> - assert(brw->screen->devinfo.gen >= 7); /* MCS only used on Gen7+ */
> - assert(mt->aux_buf == NULL);
> - assert(mt->aux_usage == ISL_AUX_USAGE_MCS);
> -
> - /* Multisampled miptrees are only supported for single level. */
> - assert(mt->first_level == 0);
> - enum isl_aux_state **aux_state =
> - create_aux_state_map(mt, ISL_AUX_STATE_CLEAR);
> - if (!aux_state)
> - return false;
> -
> - struct isl_surf temp_mcs_surf;
> -
> - MAYBE_UNUSED bool ok =
> - isl_surf_get_mcs_surf(&brw->isl_dev, &mt->surf, &temp_mcs_surf);
> - assert(ok);
> -
> - /* From the Ivy Bridge PRM, Vol 2 Part 1 p326:
> - *
> - * When MCS buffer is enabled and bound to MSRT, it is required
> that it
> - * is cleared prior to any rendering.
> - *
> - * Since we don't use the MCS buffer for any purpose other than
> rendering,
> - * it makes sense to just clear it immediately upon allocation.
> - *
> - * Note: the clear value for MCS buffers is all 1's, so we memset to
> 0xff.
> - */
> - mt->aux_buf = intel_alloc_aux_buffer(brw, &temp_mcs_surf, true, 0xFF);
> - if (!mt->aux_buf) {
> - free(aux_state);
> - return false;
> - }
> -
> - mt->aux_state = aux_state;
> -
> - return true;
> -}
> -
> -static bool
> -intel_miptree_alloc_ccs(struct brw_context *brw,
> - struct intel_mipmap_tree *mt)
> -{
> - assert(mt->aux_buf == NULL);
> - assert(mt->aux_usage == ISL_AUX_USAGE_CCS_E ||
> - mt->aux_usage == ISL_AUX_USAGE_CCS_D);
> -
> - struct isl_surf temp_ccs_surf;
> -
> - if (!isl_surf_get_ccs_surf(&brw->isl_dev, &mt->surf, &temp_ccs_surf,
> 0))
> - return false;
> -
> - assert(temp_ccs_surf.size &&
> - (temp_ccs_surf.size % temp_ccs_surf.row_pitch == 0));
> -
> - enum isl_aux_state **aux_state =
> - create_aux_state_map(mt, ISL_AUX_STATE_PASS_THROUGH);
> - if (!aux_state)
> - return false;
> -
> - /* When CCS_E is used, we need to ensure that the CCS starts off in a
> valid
> - * state. From the Sky Lake PRM, "MCS Buffer for Render Target(s)":
> - *
> - * "If Software wants to enable Color Compression without Fast
> clear,
> - * Software needs to initialize MCS with zeros."
> - *
> - * A CCS value of 0 indicates that the corresponding block is in the
> - * pass-through state which is what we want.
> - *
> - * For CCS_D, do the same thing. On gen9+, this avoids having any
> undefined
> - * bits in the aux buffer.
> - */
> - mt->aux_buf = intel_alloc_aux_buffer(brw, &temp_ccs_surf, true, 0);
> - if (!mt->aux_buf) {
> - free(aux_state);
> - return false;
> - }
> -
> - mt->aux_state = aux_state;
> -
> - return true;
> -}
>
> /**
> - * Helper for intel_miptree_alloc_hiz() that sets
> + * Helper for intel_miptree_alloc_aux() that sets
> * \c mt->level[level].has_hiz. Return true if and only if
> * \c has_hiz was set.
> */
> @@ -1859,37 +1773,78 @@ intel_miptree_level_enable_hiz(struct brw_context
> *brw,
> return true;
> }
>
> -bool
> -intel_miptree_alloc_hiz(struct brw_context *brw,
> - struct intel_mipmap_tree *mt)
> -{
> - assert(mt->aux_buf == NULL);
> - assert(mt->aux_usage == ISL_AUX_USAGE_HIZ);
>
> - enum isl_aux_state **aux_state =
> - create_aux_state_map(mt, ISL_AUX_STATE_AUX_INVALID);
> - if (!aux_state)
> - return false;
> +/* Returns true iff all params are successfully filled. */
> +static bool
> +get_aux_buf_params(const struct brw_context *brw,
> + const struct intel_mipmap_tree *mt,
> + enum isl_aux_state *initial_state,
> + uint8_t *memset_value,
> + struct isl_surf *aux_surf)
>
I think it'd be better if we inlined this into the function below. Having
it split out isn't really buying us much and separates the logic into two
pieces.
> +{
> + assert(initial_state && memset_value && aux_surf);
>
> - struct isl_surf temp_hiz_surf;
> + switch (mt->aux_usage) {
> + case ISL_AUX_USAGE_NONE:
> + aux_surf->size = 0;
> + return true;
> + case ISL_AUX_USAGE_HIZ:
> + assert(!_mesa_is_format_color_format(mt->format));
>
> - MAYBE_UNUSED bool ok =
> - isl_surf_get_hiz_surf(&brw->isl_dev, &mt->surf, &temp_hiz_surf);
> - assert(ok);
> + *initial_state = ISL_AUX_STATE_AUX_INVALID;
> + {
> + MAYBE_UNUSED bool ok =
>
It might be cleaner if we declared this little helper bool outside the
switch so we could drop the braces.
> + isl_surf_get_hiz_surf(&brw->isl_dev, &mt->surf, aux_surf);
> + assert(ok);
> + }
> + return true;
> + case ISL_AUX_USAGE_MCS:
> + assert(_mesa_is_format_color_format(mt->format));
> + assert(brw->screen->devinfo.gen >= 7); /* MCS only used on Gen7+ */
>
> - mt->aux_buf = intel_alloc_aux_buffer(brw, &temp_hiz_surf, false, 0);
> + /* From the Ivy Bridge PRM, Vol 2 Part 1 p326:
> + *
> + * When MCS buffer is enabled and bound to MSRT, it is required
> that
> + * it is cleared prior to any rendering.
> + *
> + * Since we don't use the MCS buffer for any purpose other than
> + * rendering, it makes sense to just clear it immediately upon
> + * allocation.
> + *
> + * Note: the clear value for MCS buffers is all 1's, so we memset to
> + * 0xff.
> + */
> + *initial_state = ISL_AUX_STATE_CLEAR;
> + *memset_value = 0xFF;
> + {
> + MAYBE_UNUSED bool ok =
> + isl_surf_get_mcs_surf(&brw->isl_dev, &mt->surf, aux_surf);
> + assert(ok);
> + }
> + return true;
> + case ISL_AUX_USAGE_CCS_D:
> + case ISL_AUX_USAGE_CCS_E:
> + assert(_mesa_is_format_color_format(mt->format));
>
> - if (!mt->aux_buf) {
> - free(aux_state);
> - return false;
> + /* When CCS_E is used, we need to ensure that the CCS starts off in
> a
> + * valid state. From the Sky Lake PRM, "MCS Buffer for Render
> + * Target(s)":
> + *
> + * "If Software wants to enable Color Compression without Fast
> + * clear, Software needs to initialize MCS with zeros."
> + *
> + * A CCS value of 0 indicates that the corresponding block is in the
> + * pass-through state which is what we want.
> + *
> + * For CCS_D, do the same thing. On gen9+, this avoids having any
> undefined
> + * bits in the aux buffer.
> + */
> + *initial_state = ISL_AUX_STATE_PASS_THROUGH;
> + *memset_value = 0;
> + return isl_surf_get_ccs_surf(&brw->isl_dev, &mt->surf, aux_surf,
> 0);
> }
>
> - for (unsigned level = mt->first_level; level <= mt->last_level;
> ++level)
> - intel_miptree_level_enable_hiz(brw, mt, level);
> -
> - mt->aux_state = aux_state;
> -
> - return true;
> + unreachable("Invalid aux usage");
> }
>
>
> @@ -1904,33 +1859,41 @@ bool
> intel_miptree_alloc_aux(struct brw_context *brw,
> struct intel_mipmap_tree *mt)
> {
> - switch (mt->aux_usage) {
> - case ISL_AUX_USAGE_NONE:
> - return true;
> + assert(mt->aux_buf == NULL);
>
> - case ISL_AUX_USAGE_HIZ:
> - assert(!_mesa_is_format_color_format(mt->format));
> - if (!intel_miptree_alloc_hiz(brw, mt))
> - return false;
> - return true;
> + /* Get the aux buf allocation parameters for this miptree. */
> + enum isl_aux_state initial_state;
> + uint8_t memset_value;
> + struct isl_surf aux_surf;
> + if (!get_aux_buf_params(brw, mt, &initial_state, &memset_value,
> &aux_surf))
> + return false;
>
> - case ISL_AUX_USAGE_MCS:
> - assert(_mesa_is_format_color_format(mt->format));
> - assert(mt->surf.samples > 1);
> - if (!intel_miptree_alloc_mcs(brw, mt, mt->surf.samples))
> - return false;
> + /* No work is needed for a zero-sized auxiliary buffer. */
> + if (aux_surf.size == 0)
> return true;
>
> - case ISL_AUX_USAGE_CCS_D:
> - case ISL_AUX_USAGE_CCS_E:
> - assert(_mesa_is_format_color_format(mt->format));
> - assert(mt->surf.samples == 1);
> - if (!intel_miptree_alloc_ccs(brw, mt))
> - return false;
> - return true;
> + /* Create the aux_state for the auxiliary buffer. */
> + mt->aux_state = create_aux_state_map(mt, initial_state);
> + if (mt->aux_state == NULL)
> + return false;
> +
> + /* Allocate the auxiliary buffer. */
> + const bool needs_memset = initial_state != ISL_AUX_STATE_AUX_INVALID;
> + mt->aux_buf = intel_alloc_aux_buffer(brw, &aux_surf, needs_memset,
> + memset_value);
> + if (mt->aux_buf == NULL) {
> + free_aux_state_map(mt->aux_state);
> + mt->aux_state = NULL;
> + return false;
> }
>
> - unreachable("Invalid aux usage");
> + /* Perform aux_usage-specific initialization. */
> + if (mt->aux_usage == ISL_AUX_USAGE_HIZ) {
> + for (unsigned level = mt->first_level; level <= mt->last_level;
> ++level)
> + intel_miptree_level_enable_hiz(brw, mt, level);
> + }
> +
> + return true;
> }
>
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 9a5d97bf348..d8d69698f54 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -536,15 +536,6 @@ intel_miptree_copy_teximage(struct brw_context *brw,
> * functions on a miptree without HiZ. In that case, each function is a
> no-op.
> */
>
> -/**
> - * \brief Allocate the miptree's embedded HiZ miptree.
> - * \see intel_mipmap_tree:hiz_mt
> - * \return false if allocation failed
> - */
> -bool
> -intel_miptree_alloc_hiz(struct brw_context *brw,
> - struct intel_mipmap_tree *mt);
> -
> bool
> intel_miptree_level_has_hiz(const struct intel_mipmap_tree *mt, uint32_t
> level);
>
> --
> 2.16.2
>
> _______________________________________________
> 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
