[Intel-xe] [PATCH v2 24/31] drm/xe: Userptr refactor
Rodrigo Vivi
rodrigo.vivi at kernel.org
Fri May 5 19:41:27 UTC 2023
On Mon, May 01, 2023 at 05:17:20PM -0700, Matthew Brost wrote:
> Add GPUVA userptr flag, add GPUVA userptr sub-struct, and drop sg
> pointer. A larger follow on cleanup may push more of userptr
> implementation to GPUVA.
here as well, please have xe and drm changes in separated patches.
But the final result looks good....
>
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> ---
> drivers/gpu/drm/xe/xe_pt.c | 6 +--
> drivers/gpu/drm/xe/xe_vm.c | 41 +++++++++++----------
> drivers/gpu/drm/xe/xe_vm.h | 23 +++++++-----
> drivers/gpu/drm/xe/xe_vm_types.h | 20 +++++-----
> include/drm/drm_gpuva_mgr.h | 63 +++++++++++++++++++++-----------
> 5 files changed, 89 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 0f40f1950686..964baa24eba3 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -92,8 +92,8 @@ static dma_addr_t vma_addr(struct xe_vma *vma, u64 offset,
> page = offset >> PAGE_SHIFT;
> offset &= (PAGE_SIZE - 1);
>
> - xe_res_first_sg(vma->userptr.sg, page << PAGE_SHIFT, page_size,
> - &cur);
> + xe_res_first_sg(&vma->userptr.sgt, page << PAGE_SHIFT,
> + page_size, &cur);
> return xe_res_dma(&cur) + offset;
> } else {
> return xe_bo_addr(xe_vma_bo(vma), offset, page_size, is_vram);
> @@ -813,7 +813,7 @@ xe_pt_stage_bind(struct xe_gt *gt, struct xe_vma *vma,
> xe_bo_assert_held(bo);
> if (!xe_vma_is_null(vma)) {
> if (xe_vma_is_userptr(vma))
> - xe_res_first_sg(vma->userptr.sg, 0, xe_vma_size(vma),
> + xe_res_first_sg(&vma->userptr.sgt, 0, xe_vma_size(vma),
> &curs);
> else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
> xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 336e21c710a5..4d734ec4d6ab 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -73,13 +73,13 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
> if (!pages)
> return -ENOMEM;
>
> - if (vma->userptr.sg) {
> + if (xe_vma_userptr_sg_mapped(vma)) {
> dma_unmap_sgtable(xe->drm.dev,
> - vma->userptr.sg,
> + &vma->userptr.sgt,
> read_only ? DMA_TO_DEVICE :
> DMA_BIDIRECTIONAL, 0);
> - sg_free_table(vma->userptr.sg);
> - vma->userptr.sg = NULL;
> + sg_free_table(&vma->userptr.sgt);
> + vma->gpuva.flags &= ~XE_VMA_USERPTR_SG_MAPPED;
> }
>
> pinned = ret = 0;
> @@ -119,19 +119,19 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
> 0, (u64)pinned << PAGE_SHIFT,
> GFP_KERNEL);
> if (ret) {
> - vma->userptr.sg = NULL;
> + vma->gpuva.flags &= ~XE_VMA_USERPTR_SG_MAPPED;
> goto out;
> }
> - vma->userptr.sg = &vma->userptr.sgt;
> + vma->gpuva.flags |= XE_VMA_USERPTR_SG_MAPPED;
>
> - ret = dma_map_sgtable(xe->drm.dev, vma->userptr.sg,
> + ret = dma_map_sgtable(xe->drm.dev, &vma->userptr.sgt,
> read_only ? DMA_TO_DEVICE :
> DMA_BIDIRECTIONAL,
> DMA_ATTR_SKIP_CPU_SYNC |
> DMA_ATTR_NO_KERNEL_MAPPING);
> if (ret) {
> - sg_free_table(vma->userptr.sg);
> - vma->userptr.sg = NULL;
> + sg_free_table(&vma->userptr.sgt);
> + vma->gpuva.flags &= ~XE_VMA_USERPTR_SG_MAPPED;
> goto out;
> }
>
> @@ -820,15 +820,13 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
> */
> INIT_LIST_HEAD(&vma->rebind_link);
>
> - INIT_LIST_HEAD(&vma->gpuva.gem.entry);
> - INIT_LIST_HEAD(&vma->gpuva.gem.extobj_link);
> vma->gpuva.mgr = &vm->mgr;
> vma->gpuva.va.addr = start;
> vma->gpuva.va.range = end - start + 1;
> if (read_only)
> vma->gpuva.flags |= XE_VMA_READ_ONLY;
> if (null)
> - vma->gpuva.flags |= XE_VMA_NULL;
> + vma->gpuva.flags |= DRM_GPUVA_SPARSE;
>
> if (gt_mask) {
> vma->gt_mask = gt_mask;
> @@ -845,6 +843,8 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
> xe_bo_assert_held(bo);
>
> drm_gem_object_get(&bo->ttm.base);
> + INIT_LIST_HEAD(&vma->gpuva.gem.entry);
> + INIT_LIST_HEAD(&vma->gpuva.gem.extobj_link);
> vma->gpuva.gem.obj = &bo->ttm.base;
> vma->gpuva.gem.offset = bo_offset_or_userptr;
> if (!bo->vm)
> @@ -855,7 +855,8 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
> u64 size = end - start + 1;
> int err;
>
> - vma->gpuva.gem.offset = bo_offset_or_userptr;
> + vma->gpuva.flags |= DRM_GPUVA_USERPTR;
> + vma->gpuva.userptr.address= bo_offset_or_userptr;
> err = mmu_interval_notifier_insert(&vma->userptr.notifier,
> current->mm,
> xe_vma_userptr(vma),
> @@ -883,13 +884,13 @@ static void xe_vma_destroy_late(struct xe_vma *vma)
> bool read_only = xe_vma_read_only(vma);
>
> if (xe_vma_is_userptr(vma)) {
> - if (vma->userptr.sg) {
> + if (xe_vma_userptr_sg_mapped(vma)) {
> dma_unmap_sgtable(xe->drm.dev,
> - vma->userptr.sg,
> + &vma->userptr.sgt,
> read_only ? DMA_TO_DEVICE :
> DMA_BIDIRECTIONAL, 0);
> - sg_free_table(vma->userptr.sg);
> - vma->userptr.sg = NULL;
> + sg_free_table(&vma->userptr.sgt);
> + vma->gpuva.flags &= ~XE_VMA_USERPTR_SG_MAPPED;
> }
>
> /*
> @@ -2309,7 +2310,7 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
> XE_VMA_READ_ONLY;
> bool null =
> op->base.remap.unmap->va->flags &
> - XE_VMA_NULL;
> + DRM_GPUVA_SPARSE;
>
> vma = new_vma(vm, op->base.remap.prev,
> op->gt_mask, read_only,
> @@ -2344,7 +2345,7 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm *vm, struct xe_engine *e,
>
> bool null =
> op->base.remap.unmap->va->flags &
> - XE_VMA_NULL;
> + DRM_GPUVA_SPARSE;
>
> vma = new_vma(vm, op->base.remap.next,
> op->gt_mask, read_only,
> @@ -3320,7 +3321,7 @@ int xe_analyze_vm(struct drm_printer *p, struct xe_vm *vm, int gt_id)
> } else if (is_userptr) {
> struct xe_res_cursor cur;
>
> - xe_res_first_sg(vma->userptr.sg, 0, XE_PAGE_SIZE, &cur);
> + xe_res_first_sg(&vma->userptr.sgt, 0, XE_PAGE_SIZE, &cur);
> addr = xe_res_dma(&cur);
> } else {
> addr = xe_bo_addr(xe_vma_bo(vma), 0, XE_PAGE_SIZE, &is_vram);
> diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
> index 12de652d8d1c..f279fa622260 100644
> --- a/drivers/gpu/drm/xe/xe_vm.h
> +++ b/drivers/gpu/drm/xe/xe_vm.h
> @@ -101,12 +101,6 @@ static inline u64 xe_vma_bo_offset(struct xe_vma *vma)
> return vma->gpuva.gem.offset;
> }
>
> -static inline struct xe_bo *xe_vma_bo(struct xe_vma *vma)
> -{
> - return !vma->gpuva.gem.obj ? NULL :
> - container_of(vma->gpuva.gem.obj, struct xe_bo, ttm.base);
> -}
> -
> static inline struct xe_vm *xe_vma_vm(struct xe_vma *vma)
> {
> return container_of(vma->gpuva.mgr, struct xe_vm, mgr);
> @@ -129,7 +123,7 @@ static inline bool xe_vma_read_only(struct xe_vma *vma)
>
> static inline u64 xe_vma_userptr(struct xe_vma *vma)
> {
> - return vma->gpuva.gem.offset;
> + return vma->gpuva.userptr.address;
> }
>
> #define xe_vm_assert_held(vm) dma_resv_assert_held(&(vm)->mgr.resv)
> @@ -197,12 +191,18 @@ static inline void xe_vm_reactivate_rebind(struct xe_vm *vm)
>
> static inline bool xe_vma_is_null(struct xe_vma *vma)
> {
> - return vma->gpuva.flags & XE_VMA_NULL;
> + return vma->gpuva.flags & DRM_GPUVA_SPARSE;
> }
>
> static inline bool xe_vma_is_userptr(struct xe_vma *vma)
> {
> - return !xe_vma_bo(vma) && !xe_vma_is_null(vma);
> + return vma->gpuva.flags & DRM_GPUVA_USERPTR;
> +}
> +
> +static inline struct xe_bo *xe_vma_bo(struct xe_vma *vma)
> +{
> + return xe_vma_is_null(vma) || xe_vma_is_userptr(vma) ? NULL :
> + container_of(vma->gpuva.gem.obj, struct xe_bo, ttm.base);
> }
>
> static inline bool xe_vma_has_no_bo(struct xe_vma *vma)
> @@ -210,6 +210,11 @@ static inline bool xe_vma_has_no_bo(struct xe_vma *vma)
> return !xe_vma_bo(vma);
> }
>
> +static inline bool xe_vma_userptr_sg_mapped(struct xe_vma *vma)
> +{
> + return vma->gpuva.flags & XE_VMA_USERPTR_SG_MAPPED;
> +}
> +
> int xe_vma_userptr_pin_pages(struct xe_vma *vma);
>
> int xe_vma_userptr_check_repin(struct xe_vma *vma);
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index 0b59bde3bc4e..ce1260b8d3ef 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -23,15 +23,15 @@ struct xe_vm;
> #define TEST_VM_ASYNC_OPS_ERROR
> #define FORCE_ASYNC_OP_ERROR BIT(31)
>
> -#define XE_VMA_READ_ONLY DRM_GPUVA_USERBITS
> -#define XE_VMA_DESTROYED (DRM_GPUVA_USERBITS << 1)
> -#define XE_VMA_ATOMIC_PTE_BIT (DRM_GPUVA_USERBITS << 2)
> -#define XE_VMA_FIRST_REBIND (DRM_GPUVA_USERBITS << 3)
> -#define XE_VMA_LAST_REBIND (DRM_GPUVA_USERBITS << 4)
> -#define XE_VMA_NULL (DRM_GPUVA_USERBITS << 5)
> -#define XE_VMA_PTE_4K (DRM_GPUVA_USERBITS << 6)
> -#define XE_VMA_PTE_2M (DRM_GPUVA_USERBITS << 7)
> -#define XE_VMA_PTE_1G (DRM_GPUVA_USERBITS << 8)
> +#define XE_VMA_READ_ONLY DRM_GPUVA_USERBITS
> +#define XE_VMA_DESTROYED (DRM_GPUVA_USERBITS << 1)
> +#define XE_VMA_ATOMIC_PTE_BIT (DRM_GPUVA_USERBITS << 2)
> +#define XE_VMA_FIRST_REBIND (DRM_GPUVA_USERBITS << 3)
> +#define XE_VMA_LAST_REBIND (DRM_GPUVA_USERBITS << 4)
> +#define XE_VMA_USERPTR_SG_MAPPED (DRM_GPUVA_USERBITS << 5)
> +#define XE_VMA_PTE_4K (DRM_GPUVA_USERBITS << 6)
> +#define XE_VMA_PTE_2M (DRM_GPUVA_USERBITS << 7)
> +#define XE_VMA_PTE_1G (DRM_GPUVA_USERBITS << 8)
>
> /** struct xe_userptr - User pointer */
> struct xe_userptr {
> @@ -41,8 +41,6 @@ struct xe_userptr {
> struct mmu_interval_notifier notifier;
> /** @sgt: storage for a scatter gather table */
> struct sg_table sgt;
> - /** @sg: allocated scatter gather table */
> - struct sg_table *sg;
> /** @notifier_seq: notifier sequence number */
> unsigned long notifier_seq;
> /**
> diff --git a/include/drm/drm_gpuva_mgr.h b/include/drm/drm_gpuva_mgr.h
> index 57861a7ed504..943c8fcda533 100644
> --- a/include/drm/drm_gpuva_mgr.h
> +++ b/include/drm/drm_gpuva_mgr.h
> @@ -62,10 +62,17 @@ enum drm_gpuva_flags {
> */
> DRM_GPUVA_EXTOBJ = (1 << 2),
>
> + /**
> + * @DRM_GPUVA_USERPTR:
> + *
> + * Flag indicating that the &drm_gpuva is a user pointer mapping.
> + */
> + DRM_GPUVA_USERPTR = (1 << 3),
> +
> /**
> * @DRM_GPUVA_USERBITS: user defined bits
> */
> - DRM_GPUVA_USERBITS = (1 << 3),
> + DRM_GPUVA_USERBITS = (1 << 4),
> };
>
> /**
> @@ -102,31 +109,45 @@ struct drm_gpuva {
> u64 range;
> } va;
>
> - /**
> - * @gem: structure containing the &drm_gem_object and it's offset
> - */
> - struct {
> - /**
> - * @offset: the offset within the &drm_gem_object
> - */
> - u64 offset;
> -
> - /**
> - * @obj: the mapped &drm_gem_object
> - */
> - struct drm_gem_object *obj;
> -
> + union {
> /**
> - * @entry: the &list_head to attach this object to a &drm_gem_object
> + * @gem: structure containing the &drm_gem_object and it's
> + * offset
> */
> - struct list_head entry;
> + struct {
> + /**
> + * @offset: the offset within the &drm_gem_object
> + */
> + u64 offset;
> +
> + /**
> + * @obj: the mapped &drm_gem_object
> + */
> + struct drm_gem_object *obj;
> +
> + /**
> + * @entry: the &list_head to attach this object to a
> + * &drm_gem_object
> + */
> + struct list_head entry;
> +
> + /**
> + * @extobj_link: the &list_head to attach this object to
> + * a @drm_gpuva_manager.extobj.list
> + */
> + struct list_head extobj_link;
> + } gem;
>
> /**
> - * @extobj_link: the &list_head to attach this object to a
> - * @drm_gpuva_manager.extobj.list
> + * @userptr: structure containing user pointer state
> */
> - struct list_head extobj_link;
> - } gem;
> + struct {
> + /**
> + * @address: user pointer address
> + */
> + u64 address;
> + } userptr;
> + };
> };
>
> void drm_gpuva_link(struct drm_gpuva *va);
> --
> 2.34.1
>
More information about the Intel-xe
mailing list