[Mesa-dev] [PATCH] anv: Canonicalize known offsets

[Grazvydas Ignotas]

On Thu, May 11, 2017 at 11:09 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> When we add the reloc, we write the value of the relocation target using
> non-canonical form. If we later patch up the address prior to command
> submission, we will rewrite it using canonical form. We should always
> write the address in canonical form in case we skip the relocation (as
> the target did not move since creation of the relocation).
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jason Ekstrand <jason.ekstrand at intel.com>
> ---
>  src/intel/vulkan/anv_batch_chain.c | 35 ++++++++++++++++++++++-------------
>  1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c
> index 73ed82e7a5..93df210b61 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -214,6 +214,26 @@ anv_batch_emit_dwords(struct anv_batch *batch, int num_dwords)
>     return p;
>  }
>
> +static inline uint64_t sign_extend(uint64_t value, int index)
> +{
> +   int shift = 63 - index;
> +   return (int64_t)(value << shift) >> shift;
> +}
> +
> +static uint64_t gen8_canonical_address(uint64_t address)
> +{
> +   /* From the Broadwell PRM Vol. 2a, MI_LOAD_REGISTER_MEM::MemoryAddress:
> +    *
> +    *    "This field specifies the address of the memory location where the
> +    *    register value specified in the DWord above will read from. The
> +    *    address specifies the DWord location of the data. Range =
> +    *    GraphicsVirtualAddress[63:2] for a DWord register GraphicsAddress
> +    *    [63:48] are ignored by the HW and assumed to be in correct
> +    *    canonical form [63:48] == [47]."
> +    */
> +   return sign_extend(address, 47);
> +}
> +
>  uint64_t
>  anv_batch_emit_reloc(struct anv_batch *batch,
>                       void *location, struct anv_bo *bo, uint32_t delta)
> @@ -225,7 +245,7 @@ anv_batch_emit_reloc(struct anv_batch *batch,
>        return 0;
>     }
>
> -   return bo->offset + delta;
> +   return gen8_canonical_address(bo->offset + delta);
>  }
>
>  void
> @@ -1076,18 +1096,7 @@ write_reloc(const struct anv_device *device, void *p, uint64_t v, bool flush)
>  {
>     unsigned reloc_size = 0;
>     if (device->info.gen >= 8) {
> -      /* From the Broadwell PRM Vol. 2a, MI_LOAD_REGISTER_MEM::MemoryAddress:
> -       *
> -       *    "This field specifies the address of the memory location where the
> -       *    register value specified in the DWord above will read from. The
> -       *    address specifies the DWord location of the data. Range =
> -       *    GraphicsVirtualAddress[63:2] for a DWord register GraphicsAddress
> -       *    [63:48] are ignored by the HW and assumed to be in correct
> -       *    canonical form [63:48] == [47]."
> -       */
> -      const int shift = 63 - 47;
> -      reloc_size = sizeof(uint64_t);

I guess this line should be left alone?

Gražvydas