[Mesa-dev] [PATCH v2 07/11] intel: tools: aub_mem: reuse already mapped ppgtt buffers

[Rafael Antognolli]

On Tue, Aug 07, 2018 at 06:35:18PM +0100, Lionel Landwerlin wrote:
> When we map a PPGTT buffer into a continous address space of aubinator
> to be able to inspect it, we currently add it to the list of BOs to
> unmap once we're finished. An optimization we can apply it to look up
> that list before trying to remap PPGTT buffers again (we already do
> this for GGTT buffers).
> 
> We need to take some care before doing this because the list also
> contains GGTT BOs. As GGTT & PPGTT are 2 different address spaces, we
> can have matching addresses in both that point to different physical
> locations.

So, before this change, we could have the same address for PPGTT and
GGTT on the map list, but they never clashed because we only added the
PPGTT ones at the end, and then unmapped them? Or was there something
else preventing them from conflicting?

> This changes adds a flag on the elements of the list of mapped BOs to
> differenciate between GGTT & PPGTT, which allows use to reuse that
> list when looking up both address spaces.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/intel/tools/aub_mem.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/intel/tools/aub_mem.c b/src/intel/tools/aub_mem.c
> index 2d29386e57c..3d4dc8061bd 100644
> --- a/src/intel/tools/aub_mem.c
> +++ b/src/intel/tools/aub_mem.c
> @@ -42,6 +42,7 @@ struct bo_map {
>     struct list_head link;
>     struct gen_batch_decode_bo bo;
>     bool unmap_after_use;
> +   bool ppgtt;
>  };
>  
>  struct ggtt_entry {
> @@ -59,10 +60,11 @@ struct phys_mem {
>  };
>  
>  static void
> -add_gtt_bo_map(struct aub_mem *mem, struct gen_batch_decode_bo bo, bool unmap_after_use)
> +add_gtt_bo_map(struct aub_mem *mem, struct gen_batch_decode_bo bo, bool ppgtt, bool unmap_after_use)
>  {
>     struct bo_map *m = calloc(1, sizeof(*m));
>  
> +   m->ppgtt = ppgtt;
>     m->bo = bo;
>     m->unmap_after_use = unmap_after_use;
>     list_add(&m->link, &mem->maps);
> @@ -190,7 +192,7 @@ aub_mem_local_write(void *_mem, uint64_t address,
>        .addr = address,
>        .size = size,
>     };
> -   add_gtt_bo_map(mem, bo, false);
> +   add_gtt_bo_map(mem, bo, false, false);
>  }
>  
>  void
> @@ -253,7 +255,7 @@ aub_mem_get_ggtt_bo(void *_mem, uint64_t address)
>     struct gen_batch_decode_bo bo = {0};
>  
>     list_for_each_entry(struct bo_map, i, &mem->maps, link)
> -      if (i->bo.addr <= address && i->bo.addr + i->bo.size > address)
> +      if (!i->ppgtt && i->bo.addr <= address && i->bo.addr + i->bo.size > address)
>           return i->bo;
>  
>     address &= ~0xfff;
> @@ -292,7 +294,7 @@ aub_mem_get_ggtt_bo(void *_mem, uint64_t address)
>        assert(res != MAP_FAILED);
>     }
>  
> -   add_gtt_bo_map(mem, bo, true);
> +   add_gtt_bo_map(mem, bo, false, true);
>  
>     return bo;
>  }
> @@ -328,6 +330,10 @@ aub_mem_get_ppgtt_bo(void *_mem, uint64_t address)
>     struct aub_mem *mem = _mem;
>     struct gen_batch_decode_bo bo = {0};
>  
> +   list_for_each_entry(struct bo_map, i, &mem->maps, link)
> +      if (i->ppgtt && i->bo.addr <= address && i->bo.addr + i->bo.size > address)
> +         return i->bo;
> +
>     address &= ~0xfff;
>  
>     if (!ppgtt_mapped(mem, mem->pml4, address))
> @@ -353,7 +359,7 @@ aub_mem_get_ppgtt_bo(void *_mem, uint64_t address)
>        assert(res != MAP_FAILED);
>     }
>  
> -   add_gtt_bo_map(mem, bo, true);
> +   add_gtt_bo_map(mem, bo, true, true);
>  
>     return bo;
>  }
> -- 
> 2.18.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev