On Wed 13 Jul 2016, Jason Ekstrand wrote:
> Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_state.h | 8 +++
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 79
> ++++++++++++++++++++++++
> 2 files changed, 87 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index a16e876..91ebce9 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -274,6 +274,14 @@ GLuint translate_tex_format(struct brw_context *brw,
> int brw_get_texture_swizzle(const struct gl_context *ctx,
> const struct gl_texture_object *t);
>
Move the forward declaration below to the top of the header. As written,
it looks like the function's return type.
> +struct isl_view;
> +void brw_emit_surface_state(struct brw_context *brw,
> + struct intel_mipmap_tree *mt,
> + const struct isl_view *view,
> + uint32_t mocs, bool for_gather,
> + uint32_t *surf_offset, int surf_index,
> + unsigned read_domains, unsigned write_domains);
> +
> void brw_update_renderbuffer_surfaces(struct brw_context *brw,
> const struct gl_framebuffer *fb,
> uint32_t render_target_start,
> 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 c101e05..65a1f3c 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -35,6 +35,7 @@
> #include "main/mtypes.h"
> #include "main/samplerobj.h"
> #include "main/shaderimage.h"
> +#include "main/teximage.h"
> #include "program/prog_parameter.h"
> #include "program/prog_instruction.h"
> #include "main/framebuffer.h"
> @@ -52,6 +53,84 @@
> #include "brw_defines.h"
> #include "brw_wm.h"
>
> +struct surface_state_info {
> + unsigned num_dwords;
> + unsigned ss_align; /* Required alignment of RENDER_SURFACE_STATE in bytes
> */
> + unsigned reloc_dw;
> + unsigned aux_reloc_dw;
> + unsigned tex_mocs;
> + unsigned rb_mocs;
> +};
> +
> +static const struct surface_state_info surface_state_infos[] = {
> + [4] = {6, 32, 1, 0},
> + [5] = {6, 32, 1, 0},
> + [6] = {6, 32, 1, 0},
> + [7] = {8, 32, 1, 6, GEN7_MOCS_L3, GEN7_MOCS_L3},
> + [8] = {13, 64, 8, 10, BDW_MOCS_WB, BDW_MOCS_PTE},
> + [9] = {16, 64, 8, 10, SKL_MOCS_WB, SKL_MOCS_PTE},
> +};
> +
> +void
> +brw_emit_surface_state(struct brw_context *brw,
> + struct intel_mipmap_tree *mt,
> + const struct isl_view *view,
> + uint32_t mocs, bool for_gather,
> + uint32_t *surf_offset, int surf_index,
> + unsigned read_domains, unsigned write_domains)
> +{
> + const struct surface_state_info ss_info = surface_state_infos[brw->gen];
> +
> + struct isl_surf surf;
> + intel_miptree_get_isl_surf(brw, mt, &surf);
> +
> + union isl_color_value clear_color = { .u32 = { 0, 0, 0, 0 } };
> +
> + struct isl_surf *aux_surf = NULL, aux_surf_s;
> + uint64_t aux_offset = 0;
> + enum isl_aux_usage aux_usage = ISL_AUX_USAGE_NONE;
> + if (mt->mcs_mt &&
> + ((view->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
> + mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED)) {
> + intel_miptree_get_aux_isl_surf(brw, mt, &aux_surf_s, &aux_usage);
> + aux_surf = &aux_surf_s;
> + assert(mt->mcs_mt->offset == 0);
> + aux_offset = mt->mcs_mt->bo->offset64;
> +
> + /* We only really need a clear color if we also have an auxiliary
> + * surfacae. Without one, it does nothing.
> + */
> + clear_color = intel_miptree_get_isl_clear_color(brw, mt);
> + }
What about the auxiliary hiz surface for depth textures? The function only
handles mcs/ccs.
> +
> + uint32_t *dw = __brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> + ss_info.num_dwords * 4, ss_info.ss_align,
> + surf_index, surf_offset);
> +
> + isl_surf_fill_state(&brw->isl_dev, dw, .surf = &surf, .view = view,
> + .address = mt->bo->offset64 + mt->offset,
> + .aux_surf = aux_surf, .aux_usage = aux_usage,
> + .aux_address = aux_offset,
> + .mocs = mocs, .clear_color = clear_color);
> +
> + drm_intel_bo_emit_reloc(brw->batch.bo,
> + *surf_offset + 4 * ss_info.reloc_dw,
> + mt->bo, mt->offset,
> + read_domains, write_domains);
> +
> + if (aux_surf) {
> + /* On gen7 and prior, the bottom 12 bits of the MCS base address are
> + * used to store other information. This should be ok, however,
> because
> + * surface buffer addresses are always 4K page alinged.
> + */
Could you please insert the original comment from gen7_wm_surface_state?
Because...
> + assert((aux_offset & 0xfff) == 0);
> + drm_intel_bo_emit_reloc(brw->batch.bo,
> + *surf_offset + 4 * ss_info.aux_reloc_dw,
> + mt->mcs_mt->bo, dw[ss_info.aux_reloc_dw] &
> 0xfff,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...that ^^^ bit of code REALLY confused me until I found the original
comment. What's happening is obvious in retrospect, yet terribly obscure
without a fuller comment.
> + read_domains, write_domains);
> + }
> +}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
