<div dir="ltr">This is probably a good idea thought it doesn't solve any actual bugs ATM since the kernel always provides us canonical offsets. I considered doing it earlier when I was bringing up 48bit but didn't for that reason. I dug up the patch I had for doing it directly in genxml and sent it out. I'm not sure if it's better to do it there or here though if we're going to do it here, then blorp and i965 need to be updated as well. </div><div class="gmail_extra"> <div class="gmail_quote">On Thu, May 11, 2017 at 6:13 AM, Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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). v2: Rebuild reloc_size lost in collateral damage. Signed-off-by: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> Cc: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> Cc: Grazvydas Ignotas <<a href="mailto:notasas@gmail.com">notasas@gmail.com</a>> --- src/intel/vulkan/anv_batch_chain.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 9def174b42..45952da012 100644 <div><div class="h5">--- 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 </div></div>@@ -1076,18 +1096,8 @@ 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); - *(uint64_t *)p = (((int64_t)v) << shift) >> shift; + *(uint64_t *)p = gen8_canonical_address(v); } else { reloc_size = sizeof(uint32_t); *(uint32_t *)p = v; -- 2.11.0 <div class="HOEnZb"><div class="h5">_______________________________________________ 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" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div>