On Oct 25, 2016 2:18 AM, "Pohjolainen, Topi" <<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote: > > On Sat, Oct 22, 2016 at 10:50:32AM -0700, Jason Ekstrand wrote: > > Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > --- > > src/intel/blorp/blorp_genX_exec.h | 33 ++++---------- > > src/intel/isl/isl.c | 19 ++++++++ > > src/intel/isl/isl.h | 11 +++++ > > src/intel/vulkan/anv_batch_chain.c | 4 +- > > src/intel/vulkan/genX_cmd_buffer.c | 8 +--- > > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 56 +++++++++++------------- > > 6 files changed, 69 insertions(+), 62 deletions(-) > > > > diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h > > index ec0d022..d455714 100644 > > --- a/src/intel/blorp/blorp_genX_exec.h > > +++ b/src/intel/blorp/blorp_genX_exec.h > > @@ -995,28 +995,13 @@ blorp_emit_depth_stencil_state(struct blorp_batch *batch, > > return offset; > > } > > > > -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; > > -}; > > - > > -static const struct surface_state_info surface_state_infos[] = { > > - [6] = {6, 32, 1, 0}, > > - [7] = {8, 32, 1, 6}, > > - [8] = {13, 64, 8, 10}, > > - [9] = {16, 64, 8, 10}, > > -}; > > - > > static void > > blorp_emit_surface_state(struct blorp_batch *batch, > > const struct brw_blorp_surface_info *surface, > > - uint32_t *state, uint32_t state_offset, > > + void *state, uint32_t state_offset, > > bool is_render_target) > > { > > - const struct surface_state_info ss_info = surface_state_infos[GEN_GEN]; > > - > > + const struct isl_device *isl_dev = batch->blorp->isl_dev; > > struct isl_surf surf = surface->surf; > > > > if (surf.dim == ISL_SURF_DIM_1D && > > @@ -1038,7 +1023,7 @@ blorp_emit_surface_state(struct blorp_batch *batch, > > .aux_surf = &surface->aux_surf, .aux_usage = aux_usage, > > .mocs = mocs, .clear_color = surface->clear_color); > > > > - blorp_surface_reloc(batch, state_offset + ss_info.reloc_dw * 4, > > + blorp_surface_reloc(batch, state_offset + isl_dev->ss.addr_offset, > > surface->addr, 0); > > > > if (aux_usage != ISL_AUX_USAGE_NONE) { > > @@ -1047,8 +1032,9 @@ blorp_emit_surface_state(struct blorp_batch *batch, > > * surface buffer addresses are always 4K page alinged. > > */ > > assert((surface->aux_addr.offset & 0xfff) == 0); > > - blorp_surface_reloc(batch, state_offset + ss_info.aux_reloc_dw * 4, > > - surface->aux_addr, state[ss_info.aux_reloc_dw]); > > + uint32_t *aux_addr = state + isl_dev->ss.aux_addr_offset; > > Previously 'state' got indexed in dwords (ss_info.aux_reloc_dw). Now this > uses 'isl_dev->ss.aux_addr_offset' instead (which is in number of bytes). > Aren't we offsetting four times too far? No. State is now a void* (See above) > > + blorp_surface_reloc(batch, state_offset + isl_dev->ss.aux_addr_offset, > > + surface->aux_addr, *aux_addr); > > } > > } > > > > @@ -1086,14 +1072,13 @@ static void > > blorp_emit_surface_states(struct blorp_batch *batch, > > const struct blorp_params *params) > > { > > + const struct isl_device *isl_dev = batch->blorp->isl_dev; > > uint32_t bind_offset, surface_offsets[2]; > > void *surface_maps[2]; > > > > - const unsigned ss_size = GENX(RENDER_SURFACE_STATE_length) * 4; > > - const unsigned ss_align = GENX(RENDER_SURFACE_STATE_length) > 8 ? 64 : 32; > > - > > unsigned num_surfaces = 1 + params->src.enabled; > > - blorp_alloc_binding_table(batch, num_surfaces, ss_size, ss_align, > > + blorp_alloc_binding_table(batch, num_surfaces, > > + isl_dev->ss.size, isl_dev->ss.align, > > &bind_offset, surface_offsets, surface_maps); > > > > if (params->dst.enabled) { > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > > index 7831c5e..ec53072 100644 > > --- a/src/intel/isl/isl.c > > +++ b/src/intel/isl/isl.c > > @@ -46,6 +46,20 @@ __isl_finishme(const char *file, int line, const char *fmt, ...) > > fprintf(stderr, "%s:%d: FINISHME: %s\n", file, line, buf); > > } > > > > +static const struct { > > + uint8_t size; > > + uint8_t align; > > + uint8_t addr_offset; > > + uint8_t aux_addr_offset; > > +} ss_infos[] = { > > + [4] = {24, 32, 4}, > > + [5] = {24, 32, 4}, > > + [6] = {24, 32, 4}, > > + [7] = {32, 32, 4, 24}, > > + [8] = {52, 64, 32, 40}, > > + [9] = {64, 64, 32, 40}, > > +}; > > + > > void > > isl_device_init(struct isl_device *dev, > > const struct gen_device_info *info, > > @@ -67,6 +81,11 @@ isl_device_init(struct isl_device *dev, > > assert(info->has_hiz_and_separate_stencil); > > if (info->must_use_separate_stencil) > > assert(ISL_DEV_USE_SEPARATE_STENCIL(dev)); > > + > > + dev->ss.size = ss_infos[ISL_DEV_GEN(dev)].size; > > + dev->ss.align = ss_infos[ISL_DEV_GEN(dev)].align; > > + dev->ss.addr_offset = ss_infos[ISL_DEV_GEN(dev)].addr_offset; > > + dev->ss.aux_addr_offset = ss_infos[ISL_DEV_GEN(dev)].aux_addr_offset; > > } > > > > /** > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > > index 11ad891..07368f9 100644 > > --- a/src/intel/isl/isl.h > > +++ b/src/intel/isl/isl.h > > @@ -671,6 +671,17 @@ struct isl_device { > > const struct gen_device_info *info; > > bool use_separate_stencil; > > bool has_bit6_swizzling; > > + > > + /** > > + * Describes the layout of a RENDER_SURFACE_STATE structure for the > > + * current gen. > > + */ > > + struct { > > + uint8_t size; > > + uint8_t align; > > + uint8_t addr_offset; > > + uint8_t aux_addr_offset; > > + } ss; > > }; > > > > struct isl_extent2d { > > diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c > > index dfa9abf..93425f0 100644 > > --- a/src/intel/vulkan/anv_batch_chain.c > > +++ b/src/intel/vulkan/anv_batch_chain.c > > @@ -550,7 +550,9 @@ anv_cmd_buffer_alloc_binding_table(struct anv_cmd_buffer *cmd_buffer, > > struct anv_state > > anv_cmd_buffer_alloc_surface_state(struct anv_cmd_buffer *cmd_buffer) > > { > > - return anv_state_stream_alloc(&cmd_buffer->surface_state_stream, 64, 64); > > + struct isl_device *isl_dev = &cmd_buffer->device->isl_dev; > > + return anv_state_stream_alloc(&cmd_buffer->surface_state_stream, > > + isl_dev->ss.size, isl_dev->ss.align); > > } > > > > struct anv_state > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c > > index 24e0012..ce53526 100644 > > --- a/src/intel/vulkan/genX_cmd_buffer.c > > +++ b/src/intel/vulkan/genX_cmd_buffer.c > > @@ -633,14 +633,10 @@ add_surface_state_reloc(struct anv_cmd_buffer *cmd_buffer, > > struct anv_state state, struct anv_bo *bo, > > uint32_t offset) > > { > > - /* The address goes in SURFACE_STATE dword 1 for gens < 8 and dwords 8 and > > - * 9 for gen8+. We only write the first dword for gen8+ here and rely on > > - * the initial state to set the high bits to 0. */ > > - > > - const uint32_t dword = GEN_GEN < 8 ? 1 : 8; > > + const struct isl_device *isl_dev = &cmd_buffer->device->isl_dev; > > > > anv_reloc_list_add(&cmd_buffer->surface_relocs, &cmd_buffer->pool->alloc, > > - state.offset + dword * 4, bo, offset); > > + state.offset + isl_dev->ss.addr_offset, bo, offset); > > } > > > > static struct anv_state > > 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 b774294..df59541 100644 > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > @@ -60,22 +60,16 @@ enum { > > INTEL_AUX_BUFFER_DISABLED = 1 << 1, > > }; > > > > -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; > > +uint32_t tex_mocs[] = { > > + [7] = GEN7_MOCS_L3, > > + [8] = BDW_MOCS_WB, > > + [9] = SKL_MOCS_WB, > > }; > > > > -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}, > > +uint32_t rb_mocs[] = { > > + [7] = GEN7_MOCS_L3, > > + [8] = BDW_MOCS_PTE, > > + [9] = SKL_MOCS_PTE, > > }; > > > > static void > > @@ -85,7 +79,6 @@ brw_emit_surface_state(struct brw_context *brw, > > uint32_t mocs, 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]; > > uint32_t tile_x = mt->level[0].slice[0].x_offset; > > uint32_t tile_y = mt->level[0].slice[0].y_offset; > > uint32_t offset = mt->offset; > > @@ -155,11 +148,12 @@ brw_emit_surface_state(struct brw_context *brw, > > clear_color = intel_miptree_get_isl_clear_color(brw, mt); > > } > > > > - uint32_t *dw = __brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, > > - ss_info.num_dwords * 4, ss_info.ss_align, > > - surf_index, surf_offset); > > + void *state = __brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, > > + brw->isl_dev.ss.size, > > + brw->isl_dev.ss.align, > > + surf_index, surf_offset); > > > > - isl_surf_fill_state(&brw->isl_dev, dw, .surf = &surf, .view = &view, > > + isl_surf_fill_state(&brw->isl_dev, state, .surf = &surf, .view = &view, > > .address = mt->bo->offset64 + offset, > > .aux_surf = aux_surf, .aux_usage = aux_usage, > > .aux_address = aux_offset, > > @@ -167,7 +161,7 @@ brw_emit_surface_state(struct brw_context *brw, > > .x_offset_sa = tile_x, .y_offset_sa = tile_y); > > > > drm_intel_bo_emit_reloc(brw-><a href="http://batch.bo">batch.bo</a>, > > - *surf_offset + 4 * ss_info.reloc_dw, > > + *surf_offset + brw->isl_dev.ss.addr_offset, > > mt->bo, offset, > > read_domains, write_domains); > > > > @@ -179,9 +173,10 @@ brw_emit_surface_state(struct brw_context *brw, > > * an ordinary reloc to do the necessary address translation. > > */ > > assert((aux_offset & 0xfff) == 0); > > + uint32_t *aux_addr = state + brw->isl_dev.ss.aux_addr_offset; > > Same question here. > > > drm_intel_bo_emit_reloc(brw-><a href="http://batch.bo">batch.bo</a>, > > - *surf_offset + 4 * ss_info.aux_reloc_dw, > > - mt->mcs_mt->bo, dw[ss_info.aux_reloc_dw] & 0xfff, > > + *surf_offset + brw->isl_dev.ss.aux_addr_offset, > > + mt->mcs_mt->bo, *aux_addr & 0xfff, > > read_domains, write_domains); > > } > > } > > @@ -226,7 +221,7 @@ brw_update_renderbuffer_surface(struct brw_context *brw, > > > > uint32_t offset; > > brw_emit_surface_state(brw, mt, flags, mt->target, view, > > - surface_state_infos[brw->gen].rb_mocs, > > + rb_mocs[brw->gen], > > &offset, surf_index, > > I915_GEM_DOMAIN_RENDER, > > I915_GEM_DOMAIN_RENDER); > > @@ -627,7 +622,7 @@ brw_update_texture_surface(struct gl_context *ctx, > > const int flags = > > brw_disable_aux_surface(brw, mt) ? INTEL_AUX_BUFFER_DISABLED : 0; > > brw_emit_surface_state(brw, mt, flags, mt->target, view, > > - surface_state_infos[brw->gen].tex_mocs, > > + tex_mocs[brw->gen], > > surf_offset, surf_index, > > I915_GEM_DOMAIN_SAMPLER, 0); > > } > > @@ -643,10 +638,9 @@ brw_emit_buffer_surface_state(struct brw_context *brw, > > unsigned pitch, > > bool rw) > > { > > - const struct surface_state_info ss_info = surface_state_infos[brw->gen]; > > - > > uint32_t *dw = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, > > - ss_info.num_dwords * 4, ss_info.ss_align, > > + brw->isl_dev.ss.size, > > + brw->isl_dev.ss.align, > > out_offset); > > > > isl_buffer_fill_state(&brw->isl_dev, dw, > > @@ -654,11 +648,11 @@ brw_emit_buffer_surface_state(struct brw_context *brw, > > .size = buffer_size, > > .format = surface_format, > > .stride = pitch, > > - .mocs = ss_info.tex_mocs); > > + .mocs = tex_mocs[brw->gen]); > > > > if (bo) { > > drm_intel_bo_emit_reloc(brw-><a href="http://batch.bo">batch.bo</a>, > > - *out_offset + 4 * ss_info.reloc_dw, > > + *out_offset + brw->isl_dev.ss.addr_offset, > > bo, buffer_offset, > > I915_GEM_DOMAIN_SAMPLER, > > (rw ? I915_GEM_DOMAIN_SAMPLER : 0)); > > @@ -1202,7 +1196,7 @@ update_renderbuffer_read_surfaces(struct brw_context *brw) > > const int flags = brw->draw_aux_buffer_disabled[i] ? > > INTEL_AUX_BUFFER_DISABLED : 0; > > brw_emit_surface_state(brw, irb->mt, flags, target, view, > > - surface_state_infos[brw->gen].tex_mocs, > > + tex_mocs[brw->gen], > > surf_offset, surf_index, > > I915_GEM_DOMAIN_SAMPLER, 0); > > > > @@ -1759,7 +1753,7 @@ update_image_surface(struct brw_context *brw, > > mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED ? > > INTEL_AUX_BUFFER_DISABLED : 0; > > brw_emit_surface_state(brw, mt, flags, mt->target, view, > > - surface_state_infos[brw->gen].tex_mocs, > > + tex_mocs[brw->gen], > > surf_offset, surf_index, > > I915_GEM_DOMAIN_SAMPLER, > > access == GL_READ_ONLY ? 0 : > > -- > > 2.5.0.400.gff86faf > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>