On Thu, Feb 11, 2016 at 08:34:01PM +0200, Topi Pohjolainen wrote:
> Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> ---
> src/mesa/drivers/dri/i965/gen8_surface_state.c | 62
> ++++++++++++--------------
> 1 file changed, 29 insertions(+), 33 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index fc8f701..0a52815 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -197,6 +197,28 @@ gen8_emit_fast_clear_color(struct brw_context *brw,
> surf[7] |= mt->fast_clear_color_value;
> }
>
> +static uint32_t
> +gen8_get_aux_mode(const struct brw_context *brw,
> + const struct intel_mipmap_tree *mt,
> + uint32_t surf_type)
What are we going to need surf_type for? It doesn't seem like something that
should be part of this determination, but perhaps there's something later?
> +{
> + if (mt->mcs_mt == NULL)
> + return GEN8_SURFACE_AUX_MODE_NONE;
> +
> + /*
> + * From the BDW PRM, Volume 2d, page 260 (RENDER_SURFACE_STATE):
> + * "When MCS is enabled for non-MSRT, HALIGN_16 must be used"
> + *
> + * From the hardware spec for GEN9:
> + * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN
> + * 16 must be used."
> + */
> + if (brw->gen >= 9 || mt->num_samples == 1)
> + assert(mt->halign == 16);
> +
> + return GEN8_SURFACE_AUX_MODE_MCS;
> +}
> +
> static void
> gen8_emit_texture_surface_state(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> @@ -209,13 +231,13 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> bool rw, bool for_gather)
> {
> const unsigned depth = max_layer - min_layer;
> - struct intel_mipmap_tree *aux_mt = NULL;
> - uint32_t aux_mode = GEN8_SURFACE_AUX_MODE_NONE;
> + struct intel_mipmap_tree *aux_mt = mt->mcs_mt;
> uint32_t mocs_wb = brw->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB;
> int surf_index = surf_offset - &brw->wm.base.surf_offset[0];
> unsigned tiling_mode, pitch;
> const unsigned tr_mode = surface_tiling_resource_mode(mt->tr_mode);
> const uint32_t surf_type = translate_tex_target(target);
> + uint32_t aux_mode = gen8_get_aux_mode(brw, mt, surf_type);
>
> if (mt->format == MESA_FORMAT_S_UINT8) {
> tiling_mode = GEN8_SURFACE_TILING_W;
> @@ -229,20 +251,9 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
> * buffer should always have been resolved before it is used as a texture
> * so there is no need for it.
> */
> - if (mt->mcs_mt && mt->num_samples > 1) {
> - aux_mt = mt->mcs_mt;
> - aux_mode = GEN8_SURFACE_AUX_MODE_MCS;
> -
> - /*
> - * From the BDW PRM, Volume 2d, page 260 (RENDER_SURFACE_STATE):
> - * "When MCS is enabled for non-MSRT, HALIGN_16 must be used"
> - *
> - * From the hardware spec for GEN9:
> - * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E,
> HALIGN
> - * 16 must be used."
> - */
> - if (brw->gen >= 9 || mt->num_samples == 1)
> - assert(mt->halign == 16);
> + if (mt->num_samples <= 1) {
> + aux_mt = NULL;
> + aux_mode = GEN8_SURFACE_AUX_MODE_NONE;
What I meant earlier which probably wasn't clearly articulated is I think this
could be wrapped into gen8_get_aux_mode() more sensibly. Perhaps though you
wanted to keep the refactor as minimally invasive as possible.
> }
>
> uint32_t *surf = allocate_surface_state(brw, surf_offset, surf_index);
> @@ -418,8 +429,6 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
> struct gl_context *ctx = &brw->ctx;
> struct intel_renderbuffer *irb = intel_renderbuffer(rb);
> struct intel_mipmap_tree *mt = irb->mt;
> - struct intel_mipmap_tree *aux_mt = NULL;
> - uint32_t aux_mode = GEN8_SURFACE_AUX_MODE_NONE;
> unsigned width = mt->logical_width0;
> unsigned height = mt->logical_height0;
> unsigned pitch = mt->pitch;
> @@ -472,21 +481,8 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
> __func__, _mesa_get_format_name(rb_format));
> }
>
> - if (mt->mcs_mt) {
> - aux_mt = mt->mcs_mt;
> - aux_mode = GEN8_SURFACE_AUX_MODE_MCS;
> -
> - /*
> - * From the BDW PRM, Volume 2d, page 260 (RENDER_SURFACE_STATE):
> - * "When MCS is enabled for non-MSRT, HALIGN_16 must be used"
> - *
> - * From the hardware spec for GEN9:
> - * "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E,
> HALIGN
> - * 16 must be used."
> - */
> - if (brw->gen >= 9 || mt->num_samples == 1)
> - assert(mt->halign == 16);
> - }
> + struct intel_mipmap_tree *aux_mt = mt->mcs_mt;
pretty sure you could const this too if you wanted. It shouldn't be changing
after this point.
> + const uint32_t aux_mode = gen8_get_aux_mode(brw, mt, surf_type);
>
> uint32_t *surf = allocate_surface_state(brw, &offset, surf_index);
>
I'd like to see surf_type removed unless you have plans for it. Otherwise:
Reviewed-by: Ben Widawsky <benjamin.widaw...@intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
