Messages in this thread | | | Date | Mon, 9 Oct 2023 00:48:59 +0200 | Subject | Re: [PATCH drm-misc-next v5 0/6] [RFC] DRM GPUVM features | From | Danilo Krummrich <> |
| |
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 > > >
| |