<div dir="ltr"><div>I must say, I don't really like this patch. It seems like it's just trying to let you avoid writing two reloc functions which should, by and large, be no-ops for iris. That said, the way blorp does surface relocs is really annoying because it forces the reloc function to write in the value. Unfortunately, untangling this would require passing a reloc function pointer into isl_surf_fill_state. Maybe it's time we did that? On the other hand, future drivers are going to be softpin-only so I'm a bit hesitant to add more interface for relocations. Bah!</div><div> </div><div>In the end, I do think I agree with Jordan in not liking the #ifdef. Maybe we can just force all drivers to define all three functions and anv/i965 can make blorp_use_address() function a no-op. Would that be reasonable?</div><div> </div><div>--Jason </div></div> <div class="gmail_quote"><div dir="ltr">On Thu, Nov 29, 2018 at 2:24 AM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Drivers using softpin do not need (or have) relocations. The old relocation interface is somewhat awkward and limiting. In particular, brw_surface_reloc assumes that all SURFACE_STATEs will exist in a single buffer, and only provides ss_offset. The driver is supposed to implicitly know about this buffer, in order to offset from a CPU map of that buffer to write the value. With softpin, we can put SURFACE_STATEs in any buffer we like, as long as it's within 4GB of Surface State Base Address. So, this model breaks down. Drivers can now #define BLORP_USE_SOFTPIN and define a simpler blorp_use_pinned_bo() helper, which adds the buffer to the validation list for the current batch, and returns the (statically assigned, unchanging) address for the buffer. The upcoming Iris driver will use this interface. --- src/intel/blorp/blorp_genX_exec.h | 36 +++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h index 20f30c7116d..f18de3dc6ce 100644 --- a/src/intel/blorp/blorp_genX_exec.h +++ b/src/intel/blorp/blorp_genX_exec.h @@ -47,10 +47,27 @@ static void * blorp_emit_dwords(struct blorp_batch *batch, unsigned n); +#ifdef BLORP_USE_SOFTPIN +static uint64_t +blorp_use_pinned_bo(struct blorp_batch *batch, struct blorp_address addr); + +/* Wrappers to avoid #ifdefs everywhere */ +#define blorp_emit_reloc _blorp_combine_address +static inline void +blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset, + struct blorp_address address, uint32_t delta) +{ +} +#else static uint64_t blorp_emit_reloc(struct blorp_batch *batch, void *location, struct blorp_address address, uint32_t delta); +static void +blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset, + struct blorp_address address, uint32_t delta); +#endif + static void * blorp_alloc_dynamic_state(struct blorp_batch *batch, uint32_t size, @@ -78,10 +95,6 @@ blorp_alloc_binding_table(struct blorp_batch *batch, unsigned num_entries, static void blorp_flush_range(struct blorp_batch *batch, void *start, size_t size); -static void -blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset, - struct blorp_address address, uint32_t delta); - #if GEN_GEN >= 7 && GEN_GEN < 10 static struct blorp_address blorp_get_surface_base_address(struct blorp_batch *batch); @@ -104,15 +117,23 @@ _blorp_combine_address(struct blorp_batch *batch, void *location, if (address.buffer == NULL) { return address.offset + delta; } else { +#ifdef BLORP_USE_SOFTPIN + return blorp_use_pinned_bo(batch, address) + delta; +#else return blorp_emit_reloc(batch, location, address, delta); +#endif } } static uint64_t KSP(struct blorp_batch *batch, struct blorp_address address) { +#ifdef BLORP_USE_SOFTPIN + return blorp_use_pinned_bo(batch, address); +#else assert(address.buffer == NULL); return address.offset; +#endif } #define __gen_address_type struct blorp_address @@ -1370,6 +1391,13 @@ blorp_emit_surface_state(struct blorp_batch *batch, isl_surf_fill_state(batch->blorp->isl_dev, state, .surf = &surf, .view = &surface->view, .aux_surf = &surface->aux_surf, .aux_usage = aux_usage, +#ifdef BLORP_USE_SOFTPIN + .address = blorp_use_pinned_bo(batch, surface->addr), + .aux_address = aux_usage != ISL_AUX_USAGE_NONE ? + blorp_use_pinned_bo(batch, surface->aux_addr) : 0, + .clear_address = !use_clear_address ? 0 : + blorp_use_pinned_bo(batch, surface->clear_color_addr), +#endif .mocs = surface->addr.mocs, .clear_color = surface->clear_color, .use_clear_address = use_clear_address, -- 2.19.1 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>