On Fri, 14 Jan 2022 at 08:25, Stanislav Lisovskiy
<stanislav.lisovskiy@xxxxxxxxx> wrote:
>
> Using i915_gem_object_pin_map_unlocked instead of
> i915_gem_object_lmem_io_map, would eliminate the need
> of using I915_BO_ALLOC_CONTIGUOUS, when calling
> i915_vma_pin_iomap, because it supports non-contiguous
> allocation as well.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_vma.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 1f15c3298112..194ad92013f6 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -547,10 +547,16 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
> * of pages, that way we can also drop the
> * I915_BO_ALLOC_CONTIGUOUS when allocating the object.
> */
Remove the TODO above also?
> - if (i915_gem_object_is_lmem(vma->obj))
> - ptr = i915_gem_object_lmem_io_map(vma->obj, 0,
> - vma->obj->base.size);
> - else
> + if (i915_gem_object_is_lmem(vma->obj)) {
> + ptr = (void __iomem *)
> + i915_gem_object_pin_map_unlocked(vma->obj,
> + I915_MAP_WC);
Do you know if we need some kind of sanity check here to ensure that
the vma->pages == obj->mm.pages, when dealing with LMEM? AFAIK the vma
could in theory remap the pages, and here pin_map only cares about the
obj->mm.pages? Maybe check if the vma is VIEW_NORMAL or something?
> + if (IS_ERR(ptr)) {
> + err = PTR_ERR(ptr);
> + goto err;
> + }
> + ptr = page_pack_bits(ptr, 1);
AFAIK, the guidance is to move away from pointer packing. Can we just
make the iounmap check for is_lmem()?
> + } else
> ptr = io_mapping_map_wc(&i915_vm_to_ggtt(vma->vm)->iomap,
> vma->node.start,
> vma->node.size);
> @@ -560,7 +566,10 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
> }
>
> if (unlikely(cmpxchg(&vma->iomap, NULL, ptr))) {
> - io_mapping_unmap(ptr);
> + if (page_unmask_bits(ptr))
> + __i915_gem_object_release_map(vma->obj);
> + else
> + io_mapping_unmap(ptr);
> ptr = vma->iomap;
> }
> }
> @@ -574,7 +583,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
> i915_vma_set_ggtt_write(vma);
>
> /* NB Access through the GTT requires the device to be awake. */
> - return ptr;
> + return page_mask_bits(ptr);
>
> err_unpin:
> __i915_vma_unpin(vma);
> @@ -1687,7 +1696,11 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
> if (vma->iomap == NULL)
> return;
>
> - io_mapping_unmap(vma->iomap);
> + if (page_unmask_bits(vma->iomap))
> + __i915_gem_object_release_map(vma->obj);
> + else
> + io_mapping_unmap(vma->iomap);
> +
> vma->iomap = NULL;
> }
>
> --
> 2.24.1.485.gad05a3d8e5
>
