[Intel-gfx] [RFC v2 02/12] drm/i915/svm: Runtime (RT) allocator support
Niranjana Vishwanathapura
niranjana.vishwanathapura at intel.com
Wed Dec 18 22:51:57 UTC 2019
On Sun, Dec 15, 2019 at 08:15:24PM -0800, Niranjan Vishwanathapura wrote:
>On Sat, Dec 14, 2019 at 10:56:54AM +0000, Chris Wilson wrote:
>>Quoting Niranjana Vishwanathapura (2019-12-13 21:56:04)
>>>Shared Virtual Memory (SVM) runtime allocator support allows
>>>binding a shared virtual address to a buffer object (BO) in the
>>>device page table through an ioctl call.
>>>
>>>Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>>Cc: Jon Bloomfield <jon.bloomfield at intel.com>
>>>Cc: Daniel Vetter <daniel.vetter at intel.com>
>>>Cc: Sudeep Dutt <sudeep.dutt at intel.com>
>>>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
>>>---
>>> drivers/gpu/drm/i915/Kconfig | 11 ++++
>>> drivers/gpu/drm/i915/Makefile | 3 +
>>> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 58 ++++++++++++++----
>>> drivers/gpu/drm/i915/gem/i915_gem_svm.c | 60 +++++++++++++++++++
>>> drivers/gpu/drm/i915/gem/i915_gem_svm.h | 22 +++++++
>>> drivers/gpu/drm/i915/i915_drv.c | 21 +++++++
>>> drivers/gpu/drm/i915/i915_drv.h | 22 +++++++
>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 1 +
>>> drivers/gpu/drm/i915/i915_gem_gtt.h | 13 ++++
>>> include/uapi/drm/i915_drm.h | 27 +++++++++
>>> 10 files changed, 227 insertions(+), 11 deletions(-)
>>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_svm.c
>>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_svm.h
>>>
>>>diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
>>>index ba9595960bbe..c2e48710eec8 100644
>>>--- a/drivers/gpu/drm/i915/Kconfig
>>>+++ b/drivers/gpu/drm/i915/Kconfig
>>>@@ -137,6 +137,16 @@ config DRM_I915_GVT_KVMGT
>>> Choose this option if you want to enable KVMGT support for
>>> Intel GVT-g.
>>>
>>>+config DRM_I915_SVM
>>>+ bool "Enable Shared Virtual Memory support in i915"
>>>+ depends on STAGING
>>>+ depends on DRM_I915
>>>+ default n
>>>+ help
>>>+ Choose this option if you want Shared Virtual Memory (SVM)
>>>+ support in i915. With SVM support, one can share the virtual
>>>+ address space between a process and the GPU.
>>>+
>>> menu "drm/i915 Debugging"
>>> depends on DRM_I915
>>> depends on EXPERT
>>>diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>>>index e0fd10c0cfb8..75fe45633779 100644
>>>--- a/drivers/gpu/drm/i915/Makefile
>>>+++ b/drivers/gpu/drm/i915/Makefile
>>>@@ -153,6 +153,9 @@ i915-y += \
>>> intel_region_lmem.o \
>>> intel_wopcm.o
>>>
>>>+# SVM code
>>>+i915-$(CONFIG_DRM_I915_SVM) += gem/i915_gem_svm.o
>>>+
>>> # general-purpose microcontroller (GuC) support
>>> obj-y += gt/uc/
>>> i915-y += gt/uc/intel_uc.o \
>>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>index 5003e616a1ad..af360238a392 100644
>>>--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>@@ -2836,10 +2836,14 @@ int
>>> i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
>>> struct drm_file *file)
>>> {
>>>+ struct drm_i915_gem_exec_object2 *exec2_list, *exec2_list_user;
>>> struct drm_i915_gem_execbuffer2 *args = data;
>>>- struct drm_i915_gem_exec_object2 *exec2_list;
>>>- struct drm_syncobj **fences = NULL;
>>> const size_t count = args->buffer_count;
>>>+ struct drm_syncobj **fences = NULL;
>>>+ unsigned int i = 0, svm_count = 0;
>>>+ struct i915_address_space *vm;
>>>+ struct i915_gem_context *ctx;
>>>+ struct i915_svm_obj *svm_obj;
>>> int err;
>>>
>>> if (!check_buffer_count(count)) {
>>>@@ -2851,15 +2855,46 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
>>> if (err)
>>> return err;
>>>
>>>+ ctx = i915_gem_context_lookup(file->driver_priv, args->rsvd1);
>>>+ if (!ctx || !rcu_access_pointer(ctx->vm))
>>>+ return -ENOENT;
>>
>>This is just hopelessly wrong.
>>
>>For persistence, the _ce_->vm will have a list of must-be-present
>>vma, with a flag for whether they need prefaulting (!svm everything must
>>be prefaulted obviously). Then during reservation we ensure that all those
>>persistent vma are in place (so we probably use an eviction list to keep
>>track of those we need to instantiate on this execbuf). We don't even
>>want to individually track activity on those vma, preferring to assume
>>they are used by every request and so on change they need serialising
>>[for explicit uAPI unbind, where possible we strive to do it async for
>>endless, or at least sync against iova semaphore] against the last request
>>in the vm (so we need a vm->active). However, we do need an EXT_EXTENSION
>>to mark writes for implicit fencing (e.g. exported dmabuf) to replace
>>the information lost from execobject[]
>>
>
>I did not understand some points above.
>I am no expert here, and appreciate the feedback.
>My understanding is that [excluding endless batch buffer scenario which
>is not supported in this patch series,] VM_BIND is no different than the
>soft-pinning of objects we have today in the execbuf path. Hence the idea
>here is to add those VM_BIND objects to the execobject[] and let the
>execbuffer path to take care of the rest. Persistence of bindings across
>multiple requests is something not considered. Do we need this flag in
>execobject[] as well in execbuff path (with & without soft-pinning)?
>Other than that, we do have a list of VM_BIND objects in a per 'vm' list
>as you are suggesting above.
>Let me sync with you to better understand this.
>
Ok, we discussed it offline.
I will look into some of the requirement/usecases above to capture change
required to uapi (if any) including around synchronization between
VM_BIND and execbuff paths.
Thanks,
Niranjana
>>>+struct drm_i915_gem_vm_bind {
>>>+ /** VA start to bind **/
>>>+ __u64 start;
>>
>>iova;
>>offset; /* into handle */
>>length; /* from offset */
>>
>
>Here iova is same as 'start' above?
>
>>>+
>>>+ /** Type of memory to [un]bind **/
>>>+ __u32 type;
>>>+#define I915_GEM_VM_BIND_SVM_OBJ 0
>>>+
>>>+ /** Object handle to [un]bind for I915_GEM_VM_BIND_SVM_OBJ type **/
>>>+ __u32 handle;
>>>+
>>>+ /** vm to [un]bind **/
>>>+ __u32 vm_id;
>>>+
>>>+ /** Flags **/
>>>+ __u32 flags;
>>>+#define I915_GEM_VM_BIND_UNBIND (1 << 0)
>>>+#define I915_GEM_VM_BIND_READONLY (1 << 1)
>>
>>And don't forget extensions so that we can define the synchronisation
>>controls.
>
>OK.
>
>Niranjana
>>-Chris
More information about the Intel-gfx
mailing list