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!
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?
--Jason
On Thu, Nov 29, 2018 at 2:24 AM Kenneth Graunke
wrote:
> 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,
>
