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

[Chris Wilson]

On Thu, May 11, 2017 at 01:51:40PM +0300, Grazvydas Ignotas wrote:
> 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?

That would be wise for bsw and friends.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre