Re: [Nouveau] [PATCH drm-misc-next v8 10/12] drm/gpuvm: add an abstraction for a VM / BO combination
On Thu, 2023-11-02 at 00:31 +0100, Danilo Krummrich wrote: > Add an abstraction layer between the drm_gpuva mappings of a > particular > drm_gem_object and this GEM object itself. The abstraction represents > a > combination of a drm_gem_object and drm_gpuvm. The drm_gem_object > holds > a list of drm_gpuvm_bo structures (the structure representing this > abstraction), while each drm_gpuvm_bo contains list of mappings of > this > GEM object. > > This has multiple advantages: > > 1) We can use the drm_gpuvm_bo structure to attach it to various > lists > of the drm_gpuvm. This is useful for tracking external and evicted > objects per VM, which is introduced in subsequent patches. > > 2) Finding mappings of a certain drm_gem_object mapped in a certain > drm_gpuvm becomes much cheaper. > > 3) Drivers can derive and extend the structure to easily represent > driver specific states of a BO for a certain GPUVM. > > The idea of this abstraction was taken from amdgpu, hence the credit > for > this idea goes to the developers of amdgpu. > > Cc: Christian König > Reviewed-by: Boris Brezillon > Signed-off-by: Danilo Krummrich > --- > drivers/gpu/drm/drm_gpuvm.c | 336 +-- > -- > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 63 +++-- > include/drm/drm_gem.h | 32 +-- > include/drm/drm_gpuvm.h | 185 +- > 4 files changed, 530 insertions(+), 86 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpuvm.c > b/drivers/gpu/drm/drm_gpuvm.c > index 6a88eafc5229..2c8fdefb19f0 100644 > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -70,6 +70,18 @@ > * _gem_object, such as the _gem_object containing the root > page table, > * but it can also be a 'dummy' object, which can be allocated with > * drm_gpuvm_resv_object_alloc(). > + * > + * In order to connect a struct drm_gpuva its backing > _gem_object each > + * _gem_object maintains a list of _gpuvm_bo structures, and > each > + * _gpuvm_bo contains a list of _gpuva structures. > + * > + * A _gpuvm_bo is an abstraction that represents a combination > of a > + * _gpuvm and a _gem_object. Every such combination should > be unique. > + * This is ensured by the API through drm_gpuvm_bo_obtain() and > + * drm_gpuvm_bo_obtain_prealloc() which first look into the > corresponding > + * _gem_object list of _gpuvm_bos for an existing instance > of this > + * particular combination. If not existent a new instance is created > and linked > + * to the _gem_object. > */ > > /** > @@ -395,21 +407,28 @@ > /** > * DOC: Locking > * > - * Generally, the GPU VA manager does not take care of locking > itself, it is > - * the drivers responsibility to take care about locking. Drivers > might want to > - * protect the following operations: inserting, removing and > iterating > - * _gpuva objects as well as generating all kinds of operations, > such as > - * split / merge or prefetch. > - * > - * The GPU VA manager also does not take care of the locking of the > backing > - * _gem_object buffers GPU VA lists by itself; drivers are > responsible to > - * enforce mutual exclusion using either the GEMs dma_resv lock or > alternatively > - * a driver specific external lock. For the latter see also > - * drm_gem_gpuva_set_lock(). > - * > - * However, the GPU VA manager contains lockdep checks to ensure > callers of its > - * API hold the corresponding lock whenever the _gem_objects GPU > VA list is > - * accessed by functions such as drm_gpuva_link() or > drm_gpuva_unlink(). > + * In terms of managing _gpuva entries DRM GPUVM does not take > care of > + * locking itself, it is the drivers responsibility to take care > about locking. > + * Drivers might want to protect the following operations: > inserting, removing > + * and iterating _gpuva objects as well as generating all kinds > of > + * operations, such as split / merge or prefetch. > + * > + * DRM GPUVM also does not take care of the locking of the backing > + * _gem_object buffers GPU VA lists and _gpuvm_bo > abstractions by > + * itself; drivers are responsible to enforce mutual exclusion using > either the > + * GEMs dma_resv lock or alternatively a driver specific external > lock. For the > + * latter see also drm_gem_gpuva_set_lock(). > + * > + * However, DRM GPUVM contains lockdep checks to ensure callers of > its API hold > + * the corresponding lock whenever the _gem_objects GPU VA list > is accessed > + * by functions such as drm_gpuva_link() or drm_gpuva_unlink(), but > also > + * drm_gpuvm_bo_obtain() and drm_gpuvm_bo_put(). > + * > + * The latter is required since on creation and destruction of a > _gpuvm_bo > + * the _gpuvm_bo is attached / removed from the _gem_objects > gpuva list. > + * Subsequent calls to drm_gpuvm_bo_obtain() for the same _gpuvm > and > + * _gem_object must be able to observe previous creations and > destructions > + * of _gpuvm_bos in order to keep instances unique. > */ > >
Re: [Nouveau] [PATCH drm-misc-next v8 10/12] drm/gpuvm: add an abstraction for a VM / BO combination
On Thu, 2023-11-02 at 00:31 +0100, Danilo Krummrich wrote: > Add an abstraction layer between the drm_gpuva mappings of a > particular > drm_gem_object and this GEM object itself. The abstraction represents > a > combination of a drm_gem_object and drm_gpuvm. The drm_gem_object > holds > a list of drm_gpuvm_bo structures (the structure representing this > abstraction), while each drm_gpuvm_bo contains list of mappings of > this > GEM object. > > This has multiple advantages: > > 1) We can use the drm_gpuvm_bo structure to attach it to various > lists > of the drm_gpuvm. This is useful for tracking external and evicted > objects per VM, which is introduced in subsequent patches. > > 2) Finding mappings of a certain drm_gem_object mapped in a certain > drm_gpuvm becomes much cheaper. > > 3) Drivers can derive and extend the structure to easily represent > driver specific states of a BO for a certain GPUVM. > > The idea of this abstraction was taken from amdgpu, hence the credit > for > this idea goes to the developers of amdgpu. > > Cc: Christian König > Reviewed-by: Boris Brezillon > Signed-off-by: Danilo Krummrich Reviewed-by: Thomas Hellström > --- > drivers/gpu/drm/drm_gpuvm.c | 336 +-- > -- > drivers/gpu/drm/nouveau/nouveau_uvmm.c | 63 +++-- > include/drm/drm_gem.h | 32 +-- > include/drm/drm_gpuvm.h | 185 +- > 4 files changed, 530 insertions(+), 86 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpuvm.c > b/drivers/gpu/drm/drm_gpuvm.c > index 6a88eafc5229..2c8fdefb19f0 100644 > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -70,6 +70,18 @@ > * _gem_object, such as the _gem_object containing the root > page table, > * but it can also be a 'dummy' object, which can be allocated with > * drm_gpuvm_resv_object_alloc(). > + * > + * In order to connect a struct drm_gpuva its backing > _gem_object each > + * _gem_object maintains a list of _gpuvm_bo structures, and > each > + * _gpuvm_bo contains a list of _gpuva structures. > + * > + * A _gpuvm_bo is an abstraction that represents a combination > of a > + * _gpuvm and a _gem_object. Every such combination should > be unique. > + * This is ensured by the API through drm_gpuvm_bo_obtain() and > + * drm_gpuvm_bo_obtain_prealloc() which first look into the > corresponding > + * _gem_object list of _gpuvm_bos for an existing instance > of this > + * particular combination. If not existent a new instance is created > and linked > + * to the _gem_object. > */ > > /** > @@ -395,21 +407,28 @@ > /** > * DOC: Locking > * > - * Generally, the GPU VA manager does not take care of locking > itself, it is > - * the drivers responsibility to take care about locking. Drivers > might want to > - * protect the following operations: inserting, removing and > iterating > - * _gpuva objects as well as generating all kinds of operations, > such as > - * split / merge or prefetch. > - * > - * The GPU VA manager also does not take care of the locking of the > backing > - * _gem_object buffers GPU VA lists by itself; drivers are > responsible to > - * enforce mutual exclusion using either the GEMs dma_resv lock or > alternatively > - * a driver specific external lock. For the latter see also > - * drm_gem_gpuva_set_lock(). > - * > - * However, the GPU VA manager contains lockdep checks to ensure > callers of its > - * API hold the corresponding lock whenever the _gem_objects GPU > VA list is > - * accessed by functions such as drm_gpuva_link() or > drm_gpuva_unlink(). > + * In terms of managing _gpuva entries DRM GPUVM does not take > care of > + * locking itself, it is the drivers responsibility to take care > about locking. > + * Drivers might want to protect the following operations: > inserting, removing > + * and iterating _gpuva objects as well as generating all kinds > of > + * operations, such as split / merge or prefetch. > + * > + * DRM GPUVM also does not take care of the locking of the backing > + * _gem_object buffers GPU VA lists and _gpuvm_bo > abstractions by > + * itself; drivers are responsible to enforce mutual exclusion using > either the > + * GEMs dma_resv lock or alternatively a driver specific external > lock. For the > + * latter see also drm_gem_gpuva_set_lock(). > + * > + * However, DRM GPUVM contains lockdep checks to ensure callers of > its API hold > + * the corresponding lock whenever the _gem_objects GPU VA list > is accessed > + * by functions such as drm_gpuva_link() or drm_gpuva_unlink(), but > also > + * drm_gpuvm_bo_obtain() and drm_gpuvm_bo_put(). > + * > + * The latter is required since on creation and destruction of a > _gpuvm_bo > + * the _gpuvm_bo is attached / removed from the _gem_objects > gpuva list. > + * Subsequent calls to drm_gpuvm_bo_obtain() for the same _gpuvm > and > + * _gem_object must be able to observe previous creations and > destructions > + * of _gpuvm_bos in order to keep
