Re: [PATCH drm-misc-next v6 4/6] drm/gpuvm: track/lock/validate external/evicted objects

[Danilo Krummrich]

On 10/10/23 08:40, Thomas Hellström wrote:
> On 10/9/23 01:32, Danilo Krummrich wrote:
> > Currently the DRM 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 the DRM GPUVM represent
> > a basis for GPU-VM implementations. In this context, this patch 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.
> >
> > Big thanks to Boris Brezillon for his help to figure out locking for
> > drivers updating the GPU VA space within the fence signalling path.
> >
> > Suggested-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> >
> > +/**
> > + * drm_gpuvm_resv_add_fence - add fence to private and all extobj
> > + * dma-resv
> > + * @gpuvm: the &drm_gpuvm to add a fence to
> > + * @exec: the &drm_exec locking context
> > + * @fence: fence to add
> > + * @private_usage: private dma-resv usage
> > + * @extobj_usage: extobj dma-resv usage
> > + */
> > +void
> > +drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm,
> > +             struct drm_exec *exec,
> > +             struct dma_fence *fence,
> > +             enum dma_resv_usage private_usage,
> > +             enum dma_resv_usage extobj_usage)
> > +{
> > +    struct drm_gem_object *obj;
> > +    unsigned long index;
> > +
> > +    drm_exec_for_each_locked_object(exec, index, obj) {
> > +        dma_resv_assert_held(obj->resv);
> > +        dma_resv_add_fence(obj->resv, fence,
> > +                   drm_gpuvm_is_extobj(gpuvm, obj) ?
> > +                   private_usage : extobj_usage);
>
> It looks like private_usage and extobj_usage are mixed up above?

Good catch, will fix.

> > +    }
> > +}
> > +EXPORT_SYMBOL_GPL(drm_gpuvm_resv_add_fence);
> > +
>
> Thanks,
>
> Thomas