Message ID | 20230928191624.13703-1-dakr@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | DRM GPUVM features | expand |
Hi, Danilo On 9/28/23 21:16, Danilo Krummrich wrote: > Currently GPUVM offers common infrastructure to track GPU VA allocations > and mappings, generically connect GPU VA mappings to their backing > buffers and perform more complex mapping operations on the GPU VA space. > > However, there are more design patterns commonly used by drivers, which > can potentially be generalized in order to make GPUVM represent the > basis of a VM implementation. In this context, this patch series aims at > generalizing the following elements. > > 1) Provide a common dma-resv for GEM objects not being used outside of > this GPU-VM. > > 2) Provide tracking of external GEM objects (GEM objects which are > shared with other GPU-VMs). > > 3) Provide functions to efficiently lock all GEM objects dma-resv the > GPU-VM contains mappings of. > > 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings > of, such that validation of evicted GEM objects is accelerated. > > 5) Provide some convinience functions for common patterns. > > The implementation introduces struct drm_gpuvm_bo, which serves as abstraction > combining a struct drm_gpuvm and struct drm_gem_object, similar to what > amdgpu does with struct amdgpu_bo_vm. While this adds a bit of complexity it > improves the efficiency of tracking external and evicted GEM objects. > > This patch series is also available at [3]. > > [1] https://gitlab.freedesktop.org/nouvelles/kernel/-/commits/gpuvm-next > > Changes in V2: > ============== > - rename 'drm_gpuva_manager' -> 'drm_gpuvm' which generally leads to more > consistent naming > - properly separate commits (introduce common dma-resv, drm_gpuvm_bo > abstraction, etc.) > - remove maple tree for tracking external objects, use a list drm_gpuvm_bos > per drm_gpuvm instead > - rework dma-resv locking helpers (Thomas) > - add a locking helper for a given range of the VA space (Christian) > - make the GPUVA manager buildable as module, rather than drm_exec > builtin (Christian) > > Changes in V3: > ============== > - rename missing function and files (Boris) > - warn if vm_obj->obj != obj in drm_gpuva_link() (Boris) > - don't expose drm_gpuvm_bo_destroy() (Boris) > - unlink VM_BO from GEM in drm_gpuvm_bo_destroy() rather than > drm_gpuva_unlink() and link within drm_gpuvm_bo_obtain() to keep > drm_gpuvm_bo instances unique > - add internal locking to external and evicted object lists to support drivers > updating the VA space from within the fence signalling critical path (Boris) > - unlink external objects and evicted objects from the GPUVM's list in > drm_gpuvm_bo_destroy() > - add more documentation and fix some kernel doc issues > > Changes in V4: > ============== > - add a drm_gpuvm_resv() helper (Boris) > - add a drm_gpuvm::<list_name>::local_list field (Boris) > - remove drm_gpuvm_bo_get_unless_zero() helper (Boris) > - fix missing NULL assignment in get_next_vm_bo_from_list() (Boris) > - keep a drm_gem_object reference on potential vm_bo destroy (alternatively we > could free the vm_bo and drop the vm_bo's drm_gem_object reference through > async work) > - introduce DRM_GPUVM_RESV_PROTECTED flag to indicate external locking through > the corresponding dma-resv locks to optimize for drivers already holding > them when needed; add the corresponding lock_assert_held() calls (Thomas) > - make drm_gpuvm_bo_evict() per vm_bo and add a drm_gpuvm_bo_gem_evict() > helper (Thomas) > - pass a drm_gpuvm_bo in drm_gpuvm_ops::vm_bo_validate() (Thomas) > - documentation fixes > > Changes in V5: > ============== > - use a root drm_gem_object provided by the driver as a base for the VM's > common dma-resv (Christian) > - provide a helper to allocate a "dummy" root GEM object in case a driver > specific root GEM object isn't available > - add a dedicated patch for nouveau to make use of the GPUVM's shared dma-resv > - improve documentation (Boris) > - the following patches are removed from the series, since they already landed > in drm-misc-next > - f72c2db47080 ("drm/gpuvm: rename struct drm_gpuva_manager to struct drm_gpuvm") > - fe7acaa727e1 ("drm/gpuvm: allow building as module") > - 78f54469b871 ("drm/nouveau: uvmm: rename 'umgr' to 'base'") > > Danilo Krummrich (6): > drm/gpuvm: add common dma-resv per struct drm_gpuvm > drm/gpuvm: add drm_gpuvm_flags to drm_gpuvm > drm/gpuvm: add an abstraction for a VM / BO combination > drm/gpuvm: track/lock/validate external/evicted objects > drm/nouveau: make use of the GPUVM's shared dma-resv > drm/nouveau: use GPUVM common infrastructure > > drivers/gpu/drm/drm_gpuvm.c | 1036 +++++++++++++++++++++-- > drivers/gpu/drm/nouveau/nouveau_bo.c | 15 +- > drivers/gpu/drm/nouveau/nouveau_bo.h | 5 + > drivers/gpu/drm/nouveau/nouveau_exec.c | 52 +- > drivers/gpu/drm/nouveau/nouveau_exec.h | 4 - > drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +- > drivers/gpu/drm/nouveau/nouveau_sched.h | 4 +- > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 183 ++-- > drivers/gpu/drm/nouveau/nouveau_uvmm.h | 1 - > include/drm/drm_gem.h | 32 +- > include/drm/drm_gpuvm.h | 465 +++++++++- > 11 files changed, 1625 insertions(+), 182 deletions(-) > > > base-commit: a4ead6e37e3290cff399e2598d75e98777b69b37 One comment I had before on the GPUVM code in general was the licensing, but I'm not sure there was a reply. Is it possible to have this code dual MIT / GPLV2? Thanks, Thomas
Hi Thomas, On 10/5/23 11:35, Thomas Hellström wrote: > Hi, Danilo > > On 9/28/23 21:16, Danilo Krummrich wrote: >> Currently GPUVM offers common infrastructure to track GPU VA allocations >> and mappings, generically connect GPU VA mappings to their backing >> buffers and perform more complex mapping operations on the GPU VA space. >> >> However, there are more design patterns commonly used by drivers, which >> can potentially be generalized in order to make GPUVM represent the >> basis of a VM implementation. In this context, this patch series aims at >> generalizing the following elements. >> >> 1) Provide a common dma-resv for GEM objects not being used outside of >> this GPU-VM. >> >> 2) Provide tracking of external GEM objects (GEM objects which are >> shared with other GPU-VMs). >> >> 3) Provide functions to efficiently lock all GEM objects dma-resv the >> GPU-VM contains mappings of. >> >> 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings >> of, such that validation of evicted GEM objects is accelerated. >> >> 5) Provide some convinience functions for common patterns. >> >> The implementation introduces struct drm_gpuvm_bo, which serves as abstraction >> combining a struct drm_gpuvm and struct drm_gem_object, similar to what >> amdgpu does with struct amdgpu_bo_vm. While this adds a bit of complexity it >> improves the efficiency of tracking external and evicted GEM objects. >> >> This patch series is also available at [3]. >> >> [1] https://gitlab.freedesktop.org/nouvelles/kernel/-/commits/gpuvm-next >> >> Changes in V2: >> ============== >> - rename 'drm_gpuva_manager' -> 'drm_gpuvm' which generally leads to more >> consistent naming >> - properly separate commits (introduce common dma-resv, drm_gpuvm_bo >> abstraction, etc.) >> - remove maple tree for tracking external objects, use a list drm_gpuvm_bos >> per drm_gpuvm instead >> - rework dma-resv locking helpers (Thomas) >> - add a locking helper for a given range of the VA space (Christian) >> - make the GPUVA manager buildable as module, rather than drm_exec >> builtin (Christian) >> >> Changes in V3: >> ============== >> - rename missing function and files (Boris) >> - warn if vm_obj->obj != obj in drm_gpuva_link() (Boris) >> - don't expose drm_gpuvm_bo_destroy() (Boris) >> - unlink VM_BO from GEM in drm_gpuvm_bo_destroy() rather than >> drm_gpuva_unlink() and link within drm_gpuvm_bo_obtain() to keep >> drm_gpuvm_bo instances unique >> - add internal locking to external and evicted object lists to support drivers >> updating the VA space from within the fence signalling critical path (Boris) >> - unlink external objects and evicted objects from the GPUVM's list in >> drm_gpuvm_bo_destroy() >> - add more documentation and fix some kernel doc issues >> >> Changes in V4: >> ============== >> - add a drm_gpuvm_resv() helper (Boris) >> - add a drm_gpuvm::<list_name>::local_list field (Boris) >> - remove drm_gpuvm_bo_get_unless_zero() helper (Boris) >> - fix missing NULL assignment in get_next_vm_bo_from_list() (Boris) >> - keep a drm_gem_object reference on potential vm_bo destroy (alternatively we >> could free the vm_bo and drop the vm_bo's drm_gem_object reference through >> async work) >> - introduce DRM_GPUVM_RESV_PROTECTED flag to indicate external locking through >> the corresponding dma-resv locks to optimize for drivers already holding >> them when needed; add the corresponding lock_assert_held() calls (Thomas) >> - make drm_gpuvm_bo_evict() per vm_bo and add a drm_gpuvm_bo_gem_evict() >> helper (Thomas) >> - pass a drm_gpuvm_bo in drm_gpuvm_ops::vm_bo_validate() (Thomas) >> - documentation fixes >> >> Changes in V5: >> ============== >> - use a root drm_gem_object provided by the driver as a base for the VM's >> common dma-resv (Christian) >> - provide a helper to allocate a "dummy" root GEM object in case a driver >> specific root GEM object isn't available >> - add a dedicated patch for nouveau to make use of the GPUVM's shared dma-resv >> - improve documentation (Boris) >> - the following patches are removed from the series, since they already landed >> in drm-misc-next >> - f72c2db47080 ("drm/gpuvm: rename struct drm_gpuva_manager to struct drm_gpuvm") >> - fe7acaa727e1 ("drm/gpuvm: allow building as module") >> - 78f54469b871 ("drm/nouveau: uvmm: rename 'umgr' to 'base'") >> >> Danilo Krummrich (6): >> drm/gpuvm: add common dma-resv per struct drm_gpuvm >> drm/gpuvm: add drm_gpuvm_flags to drm_gpuvm >> drm/gpuvm: add an abstraction for a VM / BO combination >> drm/gpuvm: track/lock/validate external/evicted objects >> drm/nouveau: make use of the GPUVM's shared dma-resv >> drm/nouveau: use GPUVM common infrastructure >> >> drivers/gpu/drm/drm_gpuvm.c | 1036 +++++++++++++++++++++-- >> drivers/gpu/drm/nouveau/nouveau_bo.c | 15 +- >> drivers/gpu/drm/nouveau/nouveau_bo.h | 5 + >> drivers/gpu/drm/nouveau/nouveau_exec.c | 52 +- >> drivers/gpu/drm/nouveau/nouveau_exec.h | 4 - >> drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +- >> drivers/gpu/drm/nouveau/nouveau_sched.h | 4 +- >> drivers/gpu/drm/nouveau/nouveau_uvmm.c | 183 ++-- >> drivers/gpu/drm/nouveau/nouveau_uvmm.h | 1 - >> include/drm/drm_gem.h | 32 +- >> include/drm/drm_gpuvm.h | 465 +++++++++- >> 11 files changed, 1625 insertions(+), 182 deletions(-) >> >> >> base-commit: a4ead6e37e3290cff399e2598d75e98777b69b37 > > One comment I had before on the GPUVM code in general was the licensing, but I'm not sure there was a reply. Is it possible to have this code dual MIT / GPLV2? Personally, I don't have any objections. Please feel free to send a patch to change it, I'm happy to ACK it. - Danilo > > Thanks, > > Thomas > > >