* [Intel-gfx] [PATCH v6 0/3] drm/doc/rfc: i915 VM_BIND feature design + uapi
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: paulo.r.zanoni, chris.p.wilson, thomas.hellstrom, matthew.auld,
daniel.vetter, christian.koenig
This is the i915 driver VM_BIND feature design RFC patch series along
with the required uapi definition and description of intended use cases.
v2: Reduce the scope to simple Mesa use case.
Remove all compute related uapi, vm_bind/unbind queue support and
only support a timeline out fence instead of an in/out timeline
fence array.
v3: Expand documentation on dma-resv usage, TLB flushing, execbuf3 and
VM_UNBIND. Add FENCE_VALID and TLB_FLUSH flags.
v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
uapi documentation for vm_bind/unbind.
v5: Update TLB flushing documentation.
Add version support to stage implementation.
v6: Define and use drm_i915_gem_timeline_fence structure for
execbuf3 and vm_bind/unbind timeline fences.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Niranjana Vishwanathapura (3):
drm/doc/rfc: VM_BIND feature design document
drm/i915: Update i915 uapi documentation
drm/doc/rfc: VM_BIND uapi definition
Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++
Documentation/gpu/rfc/i915_vm_bind.rst | 246 +++++++++++++++++++++
Documentation/gpu/rfc/index.rst | 4 +
include/uapi/drm/i915_drm.h | 205 ++++++++++++++----
4 files changed, 696 insertions(+), 45 deletions(-)
create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
-1 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: matthew.brost, paulo.r.zanoni, lionel.g.landwerlin,
tvrtko.ursulin, chris.p.wilson, thomas.hellstrom, oak.zeng,
matthew.auld, jason, daniel.vetter, christian.koenig
VM_BIND design document with description of intended use cases.
v2: Reduce the scope to simple Mesa use case.
v3: Expand documentation on dma-resv usage, TLB flushing and
execbuf3.
v4: Remove vm_bind tlb flush request support.
v5: Update TLB flushing documentation.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
---
Documentation/gpu/rfc/i915_vm_bind.rst | 246 +++++++++++++++++++++++++
Documentation/gpu/rfc/index.rst | 4 +
2 files changed, 250 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst
diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst b/Documentation/gpu/rfc/i915_vm_bind.rst
new file mode 100644
index 000000000000..534adf0c6c7a
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.rst
@@ -0,0 +1,246 @@
+==========================================
+I915 VM_BIND feature design and use cases
+==========================================
+
+VM_BIND feature
+================
+DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
+objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
+specified address space (VM). These mappings (also referred to as persistent
+mappings) will be persistent across multiple GPU submissions (execbuf calls)
+issued by the UMD, without user having to provide a list of all required
+mappings during each submission (as required by older execbuf mode).
+
+The VM_BIND/UNBIND calls allow UMDs to request a timeline fence for signaling
+the completion of bind/unbind operation.
+
+VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.
+User has to opt-in for VM_BIND mode of binding for an address space (VM)
+during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
+
+The bind/unbind operation can get completed asynchronously and out of
+submission order. The out fence when specified will be signaled upon
+completion of bind/unbind operation.
+
+VM_BIND features include:
+
+* Multiple Virtual Address (VA) mappings can map to the same physical pages
+ of an object (aliasing).
+* VA mapping can map to a partial section of the BO (partial binding).
+* Support capture of persistent mappings in the dump upon GPU error.
+* Support for userptr gem objects (no special uapi is required for this).
+
+TLB flush consideration
+------------------------
+The i915 driver flushes the TLB for each submission and when an object's
+pages are released. The VM_BIND/UNBIND operation will not do any additional
+TLB flush. Any VM_BIND mapping added will be in the working set for subsequent
+submissions on that VM and will not be in the working set for currently running
+batches (which would require additional TLB flushes, which is not supported).
+
+Execbuf ioctl in VM_BIND mode
+-------------------------------
+A VM in VM_BIND mode will not support older execbuf mode of binding.
+The execbuf ioctl handling in VM_BIND mode differs significantly from the
+older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2).
+Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See
+struct drm_i915_gem_execbuffer3). The execbuf3 ioctl will not accept any
+execlist. Hence, no support for implicit sync. It is expected that the below
+work will be able to support requirements of object dependency setting in all
+use cases:
+
+"dma-buf: Add an API for exporting sync files"
+(https://lwn.net/Articles/859290/)
+
+The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only
+works with execbuf3 ioctl for submission. All BOs mapped on that VM (through
+VM_BIND call) at the time of execbuf3 call are deemed required for that
+submission.
+
+The execbuf3 ioctl directly specifies the batch addresses instead of as
+object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not
+support many of the older features like in/out/submit fences, fence array,
+default gem context and many more (See struct drm_i915_gem_execbuffer3).
+
+In VM_BIND mode, VA allocation is completely managed by the user instead of
+the i915 driver. Hence all VA assignment, eviction are not applicable in
+VM_BIND mode. Also, for determining object activeness, VM_BIND mode will not
+be using the i915_vma active reference tracking. It will instead use dma-resv
+object for that (See `VM_BIND dma_resv usage`_).
+
+So, a lot of existing code supporting execbuf2 ioctl, like relocations, VA
+evictions, vma lookup table, implicit sync, vma active reference tracking etc.,
+are not applicable for execbuf3 ioctl. Hence, all execbuf3 specific handling
+should be in a separate file and only functionalities common to these ioctls
+can be the shared code where possible.
+
+VM_PRIVATE objects
+-------------------
+By default, BOs can be mapped on multiple VMs and can also be dma-buf
+exported. Hence these BOs are referred to as Shared BOs.
+During each execbuf submission, the request fence must be added to the
+dma-resv fence list of all shared BOs mapped on the VM.
+
+VM_BIND feature introduces an optimization where user can create BO which
+is private to a specified VM via I915_GEM_CREATE_EXT_VM_PRIVATE flag during
+BO creation. Unlike Shared BOs, these VM private BOs can only be mapped on
+the VM they are private to and can't be dma-buf exported.
+All private BOs of a VM share the dma-resv object. Hence during each execbuf
+submission, they need only one dma-resv fence list updated. Thus, the fast
+path (where required mappings are already bound) submission latency is O(1)
+w.r.t the number of VM private BOs.
+
+VM_BIND locking hirarchy
+-------------------------
+The locking design here supports the older (execlist based) execbuf mode, the
+newer VM_BIND mode, the VM_BIND mode with GPU page faults and possible future
+system allocator support (See `Shared Virtual Memory (SVM) support`_).
+The older execbuf mode and the newer VM_BIND mode without page faults manages
+residency of backing storage using dma_fence. The VM_BIND mode with page faults
+and the system allocator support do not use any dma_fence at all.
+
+VM_BIND locking order is as below.
+
+1) Lock-A: A vm_bind mutex will protect vm_bind lists. This lock is taken in
+ vm_bind/vm_unbind ioctl calls, in the execbuf path and while releasing the
+ mapping.
+
+ In future, when GPU page faults are supported, we can potentially use a
+ rwsem instead, so that multiple page fault handlers can take the read side
+ lock to lookup the mapping and hence can run in parallel.
+ The older execbuf mode of binding do not need this lock.
+
+2) Lock-B: The object's dma-resv lock will protect i915_vma state and needs to
+ be held while binding/unbinding a vma in the async worker and while updating
+ dma-resv fence list of an object. Note that private BOs of a VM will all
+ share a dma-resv object.
+
+ The future system allocator support will use the HMM prescribed locking
+ instead.
+
+3) Lock-C: Spinlock/s to protect some of the VM's lists like the list of
+ invalidated vmas (due to eviction and userptr invalidation) etc.
+
+When GPU page faults are supported, the execbuf path do not take any of these
+locks. There we will simply smash the new batch buffer address into the ring and
+then tell the scheduler run that. The lock taking only happens from the page
+fault handler, where we take lock-A in read mode, whichever lock-B we need to
+find the backing storage (dma_resv lock for gem objects, and hmm/core mm for
+system allocator) and some additional locks (lock-D) for taking care of page
+table races. Page fault mode should not need to ever manipulate the vm lists,
+so won't ever need lock-C.
+
+VM_BIND LRU handling
+---------------------
+We need to ensure VM_BIND mapped objects are properly LRU tagged to avoid
+performance degradation. We will also need support for bulk LRU movement of
+VM_BIND objects to avoid additional latencies in execbuf path.
+
+The page table pages are similar to VM_BIND mapped objects (See
+`Evictable page table allocations`_) and are maintained per VM and needs to
+be pinned in memory when VM is made active (ie., upon an execbuf call with
+that VM). So, bulk LRU movement of page table pages is also needed.
+
+VM_BIND dma_resv usage
+-----------------------
+Fences needs to be added to all VM_BIND mapped objects. During each execbuf
+submission, they are added with DMA_RESV_USAGE_BOOKKEEP usage to prevent
+over sync (See enum dma_resv_usage). One can override it with either
+DMA_RESV_USAGE_READ or DMA_RESV_USAGE_WRITE usage during explicit object
+dependency setting.
+
+Note that DRM_I915_GEM_WAIT and DRM_I915_GEM_BUSY ioctls do not check for
+DMA_RESV_USAGE_BOOKKEEP usage and hence should not be used for end of batch
+check. Instead, the execbuf3 out fence should be used for end of batch check
+(See struct drm_i915_gem_execbuffer3).
+
+Also, in VM_BIND mode, use dma-resv apis for determining object activeness
+(See dma_resv_test_signaled() and dma_resv_wait_timeout()) and do not use the
+older i915_vma active reference tracking which is deprecated. This should be
+easier to get it working with the current TTM backend.
+
+Mesa use case
+--------------
+VM_BIND can potentially reduce the CPU overhead in Mesa (both Vulkan and Iris),
+hence improving performance of CPU-bound applications. It also allows us to
+implement Vulkan's Sparse Resources. With increasing GPU hardware performance,
+reducing CPU overhead becomes more impactful.
+
+
+Other VM_BIND use cases
+========================
+
+Long running Compute contexts
+------------------------------
+Usage of dma-fence expects that they complete in reasonable amount of time.
+Compute on the other hand can be long running. Hence it is appropriate for
+compute to use user/memory fence (See `User/Memory Fence`_) and dma-fence usage
+must be limited to in-kernel consumption only.
+
+Where GPU page faults are not available, kernel driver upon buffer invalidation
+will initiate a suspend (preemption) of long running context, finish the
+invalidation, revalidate the BO and then resume the compute context. This is
+done by having a per-context preempt fence which is enabled when someone tries
+to wait on it and triggers the context preemption.
+
+User/Memory Fence
+~~~~~~~~~~~~~~~~~~
+User/Memory fence is a <address, value> pair. To signal the user fence, the
+specified value will be written at the specified virtual address and wakeup the
+waiting process. User fence can be signaled either by the GPU or kernel async
+worker (like upon bind completion). User can wait on a user fence with a new
+user fence wait ioctl.
+
+Here is some prior work on this:
+https://patchwork.freedesktop.org/patch/349417/
+
+Low Latency Submission
+~~~~~~~~~~~~~~~~~~~~~~~
+Allows compute UMD to directly submit GPU jobs instead of through execbuf
+ioctl. This is made possible by VM_BIND is not being synchronized against
+execbuf. VM_BIND allows bind/unbind of mappings required for the directly
+submitted jobs.
+
+Debugger
+---------
+With debug event interface user space process (debugger) is able to keep track
+of and act upon resources created by another process (debugged) and attached
+to GPU via vm_bind interface.
+
+GPU page faults
+----------------
+GPU page faults when supported (in future), will only be supported in the
+VM_BIND mode. While both the older execbuf mode and the newer VM_BIND mode of
+binding will require using dma-fence to ensure residency, the GPU page faults
+mode when supported, will not use any dma-fence as residency is purely managed
+by installing and removing/invalidating page table entries.
+
+Page level hints settings
+--------------------------
+VM_BIND allows any hints setting per mapping instead of per BO.
+Possible hints include read-only mapping, placement and atomicity.
+Sub-BO level placement hint will be even more relevant with
+upcoming GPU on-demand page fault support.
+
+Page level Cache/CLOS settings
+-------------------------------
+VM_BIND allows cache/CLOS settings per mapping instead of per BO.
+
+Evictable page table allocations
+---------------------------------
+Make pagetable allocations evictable and manage them similar to VM_BIND
+mapped objects. Page table pages are similar to persistent mappings of a
+VM (difference here are that the page table pages will not have an i915_vma
+structure and after swapping pages back in, parent page link needs to be
+updated).
+
+Shared Virtual Memory (SVM) support
+------------------------------------
+VM_BIND interface can be used to map system memory directly (without gem BO
+abstraction) using the HMM interface. SVM is only supported with GPU page
+faults enabled.
+
+VM_BIND UAPI
+=============
+
+.. kernel-doc:: Documentation/gpu/rfc/i915_vm_bind.h
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index 91e93a705230..7d10c36b268d 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -23,3 +23,7 @@ host such documentation:
.. toctree::
i915_scheduler.rst
+
+.. toctree::
+
+ i915_vm_bind.rst
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Intel-gfx] [PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: paulo.r.zanoni, chris.p.wilson, thomas.hellstrom, matthew.auld,
daniel.vetter, christian.koenig
VM_BIND design document with description of intended use cases.
v2: Reduce the scope to simple Mesa use case.
v3: Expand documentation on dma-resv usage, TLB flushing and
execbuf3.
v4: Remove vm_bind tlb flush request support.
v5: Update TLB flushing documentation.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
---
Documentation/gpu/rfc/i915_vm_bind.rst | 246 +++++++++++++++++++++++++
Documentation/gpu/rfc/index.rst | 4 +
2 files changed, 250 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst
diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst b/Documentation/gpu/rfc/i915_vm_bind.rst
new file mode 100644
index 000000000000..534adf0c6c7a
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.rst
@@ -0,0 +1,246 @@
+==========================================
+I915 VM_BIND feature design and use cases
+==========================================
+
+VM_BIND feature
+================
+DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
+objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
+specified address space (VM). These mappings (also referred to as persistent
+mappings) will be persistent across multiple GPU submissions (execbuf calls)
+issued by the UMD, without user having to provide a list of all required
+mappings during each submission (as required by older execbuf mode).
+
+The VM_BIND/UNBIND calls allow UMDs to request a timeline fence for signaling
+the completion of bind/unbind operation.
+
+VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.
+User has to opt-in for VM_BIND mode of binding for an address space (VM)
+during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
+
+The bind/unbind operation can get completed asynchronously and out of
+submission order. The out fence when specified will be signaled upon
+completion of bind/unbind operation.
+
+VM_BIND features include:
+
+* Multiple Virtual Address (VA) mappings can map to the same physical pages
+ of an object (aliasing).
+* VA mapping can map to a partial section of the BO (partial binding).
+* Support capture of persistent mappings in the dump upon GPU error.
+* Support for userptr gem objects (no special uapi is required for this).
+
+TLB flush consideration
+------------------------
+The i915 driver flushes the TLB for each submission and when an object's
+pages are released. The VM_BIND/UNBIND operation will not do any additional
+TLB flush. Any VM_BIND mapping added will be in the working set for subsequent
+submissions on that VM and will not be in the working set for currently running
+batches (which would require additional TLB flushes, which is not supported).
+
+Execbuf ioctl in VM_BIND mode
+-------------------------------
+A VM in VM_BIND mode will not support older execbuf mode of binding.
+The execbuf ioctl handling in VM_BIND mode differs significantly from the
+older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2).
+Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See
+struct drm_i915_gem_execbuffer3). The execbuf3 ioctl will not accept any
+execlist. Hence, no support for implicit sync. It is expected that the below
+work will be able to support requirements of object dependency setting in all
+use cases:
+
+"dma-buf: Add an API for exporting sync files"
+(https://lwn.net/Articles/859290/)
+
+The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only
+works with execbuf3 ioctl for submission. All BOs mapped on that VM (through
+VM_BIND call) at the time of execbuf3 call are deemed required for that
+submission.
+
+The execbuf3 ioctl directly specifies the batch addresses instead of as
+object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not
+support many of the older features like in/out/submit fences, fence array,
+default gem context and many more (See struct drm_i915_gem_execbuffer3).
+
+In VM_BIND mode, VA allocation is completely managed by the user instead of
+the i915 driver. Hence all VA assignment, eviction are not applicable in
+VM_BIND mode. Also, for determining object activeness, VM_BIND mode will not
+be using the i915_vma active reference tracking. It will instead use dma-resv
+object for that (See `VM_BIND dma_resv usage`_).
+
+So, a lot of existing code supporting execbuf2 ioctl, like relocations, VA
+evictions, vma lookup table, implicit sync, vma active reference tracking etc.,
+are not applicable for execbuf3 ioctl. Hence, all execbuf3 specific handling
+should be in a separate file and only functionalities common to these ioctls
+can be the shared code where possible.
+
+VM_PRIVATE objects
+-------------------
+By default, BOs can be mapped on multiple VMs and can also be dma-buf
+exported. Hence these BOs are referred to as Shared BOs.
+During each execbuf submission, the request fence must be added to the
+dma-resv fence list of all shared BOs mapped on the VM.
+
+VM_BIND feature introduces an optimization where user can create BO which
+is private to a specified VM via I915_GEM_CREATE_EXT_VM_PRIVATE flag during
+BO creation. Unlike Shared BOs, these VM private BOs can only be mapped on
+the VM they are private to and can't be dma-buf exported.
+All private BOs of a VM share the dma-resv object. Hence during each execbuf
+submission, they need only one dma-resv fence list updated. Thus, the fast
+path (where required mappings are already bound) submission latency is O(1)
+w.r.t the number of VM private BOs.
+
+VM_BIND locking hirarchy
+-------------------------
+The locking design here supports the older (execlist based) execbuf mode, the
+newer VM_BIND mode, the VM_BIND mode with GPU page faults and possible future
+system allocator support (See `Shared Virtual Memory (SVM) support`_).
+The older execbuf mode and the newer VM_BIND mode without page faults manages
+residency of backing storage using dma_fence. The VM_BIND mode with page faults
+and the system allocator support do not use any dma_fence at all.
+
+VM_BIND locking order is as below.
+
+1) Lock-A: A vm_bind mutex will protect vm_bind lists. This lock is taken in
+ vm_bind/vm_unbind ioctl calls, in the execbuf path and while releasing the
+ mapping.
+
+ In future, when GPU page faults are supported, we can potentially use a
+ rwsem instead, so that multiple page fault handlers can take the read side
+ lock to lookup the mapping and hence can run in parallel.
+ The older execbuf mode of binding do not need this lock.
+
+2) Lock-B: The object's dma-resv lock will protect i915_vma state and needs to
+ be held while binding/unbinding a vma in the async worker and while updating
+ dma-resv fence list of an object. Note that private BOs of a VM will all
+ share a dma-resv object.
+
+ The future system allocator support will use the HMM prescribed locking
+ instead.
+
+3) Lock-C: Spinlock/s to protect some of the VM's lists like the list of
+ invalidated vmas (due to eviction and userptr invalidation) etc.
+
+When GPU page faults are supported, the execbuf path do not take any of these
+locks. There we will simply smash the new batch buffer address into the ring and
+then tell the scheduler run that. The lock taking only happens from the page
+fault handler, where we take lock-A in read mode, whichever lock-B we need to
+find the backing storage (dma_resv lock for gem objects, and hmm/core mm for
+system allocator) and some additional locks (lock-D) for taking care of page
+table races. Page fault mode should not need to ever manipulate the vm lists,
+so won't ever need lock-C.
+
+VM_BIND LRU handling
+---------------------
+We need to ensure VM_BIND mapped objects are properly LRU tagged to avoid
+performance degradation. We will also need support for bulk LRU movement of
+VM_BIND objects to avoid additional latencies in execbuf path.
+
+The page table pages are similar to VM_BIND mapped objects (See
+`Evictable page table allocations`_) and are maintained per VM and needs to
+be pinned in memory when VM is made active (ie., upon an execbuf call with
+that VM). So, bulk LRU movement of page table pages is also needed.
+
+VM_BIND dma_resv usage
+-----------------------
+Fences needs to be added to all VM_BIND mapped objects. During each execbuf
+submission, they are added with DMA_RESV_USAGE_BOOKKEEP usage to prevent
+over sync (See enum dma_resv_usage). One can override it with either
+DMA_RESV_USAGE_READ or DMA_RESV_USAGE_WRITE usage during explicit object
+dependency setting.
+
+Note that DRM_I915_GEM_WAIT and DRM_I915_GEM_BUSY ioctls do not check for
+DMA_RESV_USAGE_BOOKKEEP usage and hence should not be used for end of batch
+check. Instead, the execbuf3 out fence should be used for end of batch check
+(See struct drm_i915_gem_execbuffer3).
+
+Also, in VM_BIND mode, use dma-resv apis for determining object activeness
+(See dma_resv_test_signaled() and dma_resv_wait_timeout()) and do not use the
+older i915_vma active reference tracking which is deprecated. This should be
+easier to get it working with the current TTM backend.
+
+Mesa use case
+--------------
+VM_BIND can potentially reduce the CPU overhead in Mesa (both Vulkan and Iris),
+hence improving performance of CPU-bound applications. It also allows us to
+implement Vulkan's Sparse Resources. With increasing GPU hardware performance,
+reducing CPU overhead becomes more impactful.
+
+
+Other VM_BIND use cases
+========================
+
+Long running Compute contexts
+------------------------------
+Usage of dma-fence expects that they complete in reasonable amount of time.
+Compute on the other hand can be long running. Hence it is appropriate for
+compute to use user/memory fence (See `User/Memory Fence`_) and dma-fence usage
+must be limited to in-kernel consumption only.
+
+Where GPU page faults are not available, kernel driver upon buffer invalidation
+will initiate a suspend (preemption) of long running context, finish the
+invalidation, revalidate the BO and then resume the compute context. This is
+done by having a per-context preempt fence which is enabled when someone tries
+to wait on it and triggers the context preemption.
+
+User/Memory Fence
+~~~~~~~~~~~~~~~~~~
+User/Memory fence is a <address, value> pair. To signal the user fence, the
+specified value will be written at the specified virtual address and wakeup the
+waiting process. User fence can be signaled either by the GPU or kernel async
+worker (like upon bind completion). User can wait on a user fence with a new
+user fence wait ioctl.
+
+Here is some prior work on this:
+https://patchwork.freedesktop.org/patch/349417/
+
+Low Latency Submission
+~~~~~~~~~~~~~~~~~~~~~~~
+Allows compute UMD to directly submit GPU jobs instead of through execbuf
+ioctl. This is made possible by VM_BIND is not being synchronized against
+execbuf. VM_BIND allows bind/unbind of mappings required for the directly
+submitted jobs.
+
+Debugger
+---------
+With debug event interface user space process (debugger) is able to keep track
+of and act upon resources created by another process (debugged) and attached
+to GPU via vm_bind interface.
+
+GPU page faults
+----------------
+GPU page faults when supported (in future), will only be supported in the
+VM_BIND mode. While both the older execbuf mode and the newer VM_BIND mode of
+binding will require using dma-fence to ensure residency, the GPU page faults
+mode when supported, will not use any dma-fence as residency is purely managed
+by installing and removing/invalidating page table entries.
+
+Page level hints settings
+--------------------------
+VM_BIND allows any hints setting per mapping instead of per BO.
+Possible hints include read-only mapping, placement and atomicity.
+Sub-BO level placement hint will be even more relevant with
+upcoming GPU on-demand page fault support.
+
+Page level Cache/CLOS settings
+-------------------------------
+VM_BIND allows cache/CLOS settings per mapping instead of per BO.
+
+Evictable page table allocations
+---------------------------------
+Make pagetable allocations evictable and manage them similar to VM_BIND
+mapped objects. Page table pages are similar to persistent mappings of a
+VM (difference here are that the page table pages will not have an i915_vma
+structure and after swapping pages back in, parent page link needs to be
+updated).
+
+Shared Virtual Memory (SVM) support
+------------------------------------
+VM_BIND interface can be used to map system memory directly (without gem BO
+abstraction) using the HMM interface. SVM is only supported with GPU page
+faults enabled.
+
+VM_BIND UAPI
+=============
+
+.. kernel-doc:: Documentation/gpu/rfc/i915_vm_bind.h
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index 91e93a705230..7d10c36b268d 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -23,3 +23,7 @@ host such documentation:
.. toctree::
i915_scheduler.rst
+
+.. toctree::
+
+ i915_vm_bind.rst
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
@ 2022-06-24 19:31 ` Daniel Vetter
-1 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-06-24 19:31 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: matthew.brost, chris.p.wilson, paulo.r.zanoni, tvrtko.ursulin,
intel-gfx, lionel.g.landwerlin, thomas.hellstrom, oak.zeng,
dri-devel, jason, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 10:49:34AM -0700, Niranjana Vishwanathapura wrote:
> VM_BIND design document with description of intended use cases.
>
> v2: Reduce the scope to simple Mesa use case.
> v3: Expand documentation on dma-resv usage, TLB flushing and
> execbuf3.
> v4: Remove vm_bind tlb flush request support.
> v5: Update TLB flushing documentation.
>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> ---
> Documentation/gpu/rfc/i915_vm_bind.rst | 246 +++++++++++++++++++++++++
> Documentation/gpu/rfc/index.rst | 4 +
> 2 files changed, 250 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst
>
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst b/Documentation/gpu/rfc/i915_vm_bind.rst
> new file mode 100644
> index 000000000000..534adf0c6c7a
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.rst
> @@ -0,0 +1,246 @@
> +==========================================
> +I915 VM_BIND feature design and use cases
> +==========================================
> +
> +VM_BIND feature
> +================
> +DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
> +objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
> +specified address space (VM). These mappings (also referred to as persistent
> +mappings) will be persistent across multiple GPU submissions (execbuf calls)
> +issued by the UMD, without user having to provide a list of all required
> +mappings during each submission (as required by older execbuf mode).
> +
> +The VM_BIND/UNBIND calls allow UMDs to request a timeline fence for signaling
> +the completion of bind/unbind operation.
> +
> +VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.
> +User has to opt-in for VM_BIND mode of binding for an address space (VM)
> +during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
> +
> +The bind/unbind operation can get completed asynchronously and out of
> +submission order. The out fence when specified will be signaled upon
> +completion of bind/unbind operation.
> +
> +VM_BIND features include:
> +
> +* Multiple Virtual Address (VA) mappings can map to the same physical pages
> + of an object (aliasing).
> +* VA mapping can map to a partial section of the BO (partial binding).
> +* Support capture of persistent mappings in the dump upon GPU error.
> +* Support for userptr gem objects (no special uapi is required for this).
> +
> +TLB flush consideration
> +------------------------
> +The i915 driver flushes the TLB for each submission and when an object's
> +pages are released. The VM_BIND/UNBIND operation will not do any additional
> +TLB flush. Any VM_BIND mapping added will be in the working set for subsequent
> +submissions on that VM and will not be in the working set for currently running
> +batches (which would require additional TLB flushes, which is not supported).
> +
> +Execbuf ioctl in VM_BIND mode
> +-------------------------------
> +A VM in VM_BIND mode will not support older execbuf mode of binding.
> +The execbuf ioctl handling in VM_BIND mode differs significantly from the
> +older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2).
> +Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See
> +struct drm_i915_gem_execbuffer3). The execbuf3 ioctl will not accept any
> +execlist. Hence, no support for implicit sync. It is expected that the below
> +work will be able to support requirements of object dependency setting in all
> +use cases:
> +
> +"dma-buf: Add an API for exporting sync files"
> +(https://lwn.net/Articles/859290/)
> +
> +The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only
> +works with execbuf3 ioctl for submission. All BOs mapped on that VM (through
> +VM_BIND call) at the time of execbuf3 call are deemed required for that
> +submission.
> +
> +The execbuf3 ioctl directly specifies the batch addresses instead of as
> +object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not
> +support many of the older features like in/out/submit fences, fence array,
> +default gem context and many more (See struct drm_i915_gem_execbuffer3).
> +
> +In VM_BIND mode, VA allocation is completely managed by the user instead of
> +the i915 driver. Hence all VA assignment, eviction are not applicable in
> +VM_BIND mode. Also, for determining object activeness, VM_BIND mode will not
> +be using the i915_vma active reference tracking. It will instead use dma-resv
> +object for that (See `VM_BIND dma_resv usage`_).
> +
> +So, a lot of existing code supporting execbuf2 ioctl, like relocations, VA
> +evictions, vma lookup table, implicit sync, vma active reference tracking etc.,
> +are not applicable for execbuf3 ioctl. Hence, all execbuf3 specific handling
> +should be in a separate file and only functionalities common to these ioctls
> +can be the shared code where possible.
> +
> +VM_PRIVATE objects
> +-------------------
> +By default, BOs can be mapped on multiple VMs and can also be dma-buf
> +exported. Hence these BOs are referred to as Shared BOs.
> +During each execbuf submission, the request fence must be added to the
> +dma-resv fence list of all shared BOs mapped on the VM.
> +
> +VM_BIND feature introduces an optimization where user can create BO which
> +is private to a specified VM via I915_GEM_CREATE_EXT_VM_PRIVATE flag during
> +BO creation. Unlike Shared BOs, these VM private BOs can only be mapped on
> +the VM they are private to and can't be dma-buf exported.
> +All private BOs of a VM share the dma-resv object. Hence during each execbuf
> +submission, they need only one dma-resv fence list updated. Thus, the fast
> +path (where required mappings are already bound) submission latency is O(1)
> +w.r.t the number of VM private BOs.
> +
> +VM_BIND locking hirarchy
> +-------------------------
> +The locking design here supports the older (execlist based) execbuf mode, the
> +newer VM_BIND mode, the VM_BIND mode with GPU page faults and possible future
> +system allocator support (See `Shared Virtual Memory (SVM) support`_).
> +The older execbuf mode and the newer VM_BIND mode without page faults manages
> +residency of backing storage using dma_fence. The VM_BIND mode with page faults
> +and the system allocator support do not use any dma_fence at all.
> +
> +VM_BIND locking order is as below.
> +
> +1) Lock-A: A vm_bind mutex will protect vm_bind lists. This lock is taken in
> + vm_bind/vm_unbind ioctl calls, in the execbuf path and while releasing the
> + mapping.
> +
> + In future, when GPU page faults are supported, we can potentially use a
> + rwsem instead, so that multiple page fault handlers can take the read side
> + lock to lookup the mapping and hence can run in parallel.
> + The older execbuf mode of binding do not need this lock.
> +
> +2) Lock-B: The object's dma-resv lock will protect i915_vma state and needs to
> + be held while binding/unbinding a vma in the async worker and while updating
> + dma-resv fence list of an object. Note that private BOs of a VM will all
> + share a dma-resv object.
> +
> + The future system allocator support will use the HMM prescribed locking
> + instead.
> +
> +3) Lock-C: Spinlock/s to protect some of the VM's lists like the list of
> + invalidated vmas (due to eviction and userptr invalidation) etc.
> +
> +When GPU page faults are supported, the execbuf path do not take any of these
> +locks. There we will simply smash the new batch buffer address into the ring and
> +then tell the scheduler run that. The lock taking only happens from the page
> +fault handler, where we take lock-A in read mode, whichever lock-B we need to
> +find the backing storage (dma_resv lock for gem objects, and hmm/core mm for
> +system allocator) and some additional locks (lock-D) for taking care of page
> +table races. Page fault mode should not need to ever manipulate the vm lists,
> +so won't ever need lock-C.
> +
> +VM_BIND LRU handling
> +---------------------
> +We need to ensure VM_BIND mapped objects are properly LRU tagged to avoid
> +performance degradation. We will also need support for bulk LRU movement of
> +VM_BIND objects to avoid additional latencies in execbuf path.
> +
> +The page table pages are similar to VM_BIND mapped objects (See
> +`Evictable page table allocations`_) and are maintained per VM and needs to
> +be pinned in memory when VM is made active (ie., upon an execbuf call with
> +that VM). So, bulk LRU movement of page table pages is also needed.
> +
> +VM_BIND dma_resv usage
> +-----------------------
> +Fences needs to be added to all VM_BIND mapped objects. During each execbuf
> +submission, they are added with DMA_RESV_USAGE_BOOKKEEP usage to prevent
> +over sync (See enum dma_resv_usage). One can override it with either
> +DMA_RESV_USAGE_READ or DMA_RESV_USAGE_WRITE usage during explicit object
> +dependency setting.
I thought we've ditched that in favour of the import/export ioctls, and
execbuf will only ever set BOOKKEEPING?
> +Note that DRM_I915_GEM_WAIT and DRM_I915_GEM_BUSY ioctls do not check for
> +DMA_RESV_USAGE_BOOKKEEP usage and hence should not be used for end of batch
> +check. Instead, the execbuf3 out fence should be used for end of batch check
> +(See struct drm_i915_gem_execbuffer3).
This is a bit confused. Changing gem_wait and gem_busy ioctls to also look
at bookkeeping will not fix the issue, because in a vm_bind vm _all_
objects (and hence also all batchbuffers mapped into the vm) always have
the same fences in their dma_resv from that vm. Which means that if any
batchbuffer is still running, all batchbuffers are considered active. And
so checking fences in the dma_resv is useless for the umd and it needs to
track batchbuffer usage itself somewhere.
> +
> +Also, in VM_BIND mode, use dma-resv apis for determining object activeness
> +(See dma_resv_test_signaled() and dma_resv_wait_timeout()) and do not use the
> +older i915_vma active reference tracking which is deprecated. This should be
> +easier to get it working with the current TTM backend.
Hm I thought he plan is to require ttm, since ttm has the bulk lru stuff
only and our i915-gem shrinker/lru code is very busted? Maybe needs to be
clarified slightly.
> +
> +Mesa use case
> +--------------
> +VM_BIND can potentially reduce the CPU overhead in Mesa (both Vulkan and Iris),
> +hence improving performance of CPU-bound applications. It also allows us to
> +implement Vulkan's Sparse Resources. With increasing GPU hardware performance,
> +reducing CPU overhead becomes more impactful.
> +
> +
> +Other VM_BIND use cases
> +========================
> +
> +Long running Compute contexts
> +------------------------------
> +Usage of dma-fence expects that they complete in reasonable amount of time.
> +Compute on the other hand can be long running. Hence it is appropriate for
> +compute to use user/memory fence (See `User/Memory Fence`_) and dma-fence usage
> +must be limited to in-kernel consumption only.
> +
> +Where GPU page faults are not available, kernel driver upon buffer invalidation
> +will initiate a suspend (preemption) of long running context, finish the
> +invalidation, revalidate the BO and then resume the compute context. This is
> +done by having a per-context preempt fence which is enabled when someone tries
> +to wait on it and triggers the context preemption.
> +
> +User/Memory Fence
> +~~~~~~~~~~~~~~~~~~
> +User/Memory fence is a <address, value> pair. To signal the user fence, the
> +specified value will be written at the specified virtual address and wakeup the
> +waiting process. User fence can be signaled either by the GPU or kernel async
> +worker (like upon bind completion). User can wait on a user fence with a new
> +user fence wait ioctl.
> +
> +Here is some prior work on this:
> +https://patchwork.freedesktop.org/patch/349417/
> +
> +Low Latency Submission
> +~~~~~~~~~~~~~~~~~~~~~~~
> +Allows compute UMD to directly submit GPU jobs instead of through execbuf
> +ioctl. This is made possible by VM_BIND is not being synchronized against
> +execbuf. VM_BIND allows bind/unbind of mappings required for the directly
> +submitted jobs.
> +
> +Debugger
> +---------
> +With debug event interface user space process (debugger) is able to keep track
> +of and act upon resources created by another process (debugged) and attached
> +to GPU via vm_bind interface.
> +
> +GPU page faults
> +----------------
> +GPU page faults when supported (in future), will only be supported in the
> +VM_BIND mode. While both the older execbuf mode and the newer VM_BIND mode of
> +binding will require using dma-fence to ensure residency, the GPU page faults
> +mode when supported, will not use any dma-fence as residency is purely managed
> +by installing and removing/invalidating page table entries.
> +
> +Page level hints settings
> +--------------------------
> +VM_BIND allows any hints setting per mapping instead of per BO.
> +Possible hints include read-only mapping, placement and atomicity.
> +Sub-BO level placement hint will be even more relevant with
> +upcoming GPU on-demand page fault support.
> +
> +Page level Cache/CLOS settings
> +-------------------------------
> +VM_BIND allows cache/CLOS settings per mapping instead of per BO.
> +
> +Evictable page table allocations
> +---------------------------------
> +Make pagetable allocations evictable and manage them similar to VM_BIND
> +mapped objects. Page table pages are similar to persistent mappings of a
> +VM (difference here are that the page table pages will not have an i915_vma
> +structure and after swapping pages back in, parent page link needs to be
> +updated).
> +
> +Shared Virtual Memory (SVM) support
> +------------------------------------
> +VM_BIND interface can be used to map system memory directly (without gem BO
> +abstraction) using the HMM interface. SVM is only supported with GPU page
> +faults enabled.
> +
> +VM_BIND UAPI
> +=============
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_vm_bind.h
> diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
> index 91e93a705230..7d10c36b268d 100644
> --- a/Documentation/gpu/rfc/index.rst
> +++ b/Documentation/gpu/rfc/index.rst
> @@ -23,3 +23,7 @@ host such documentation:
> .. toctree::
>
> i915_scheduler.rst
> +
> +.. toctree::
> +
> + i915_vm_bind.rst
> --
> 2.21.0.rc0.32.g243a4c7e27
So I had a few comments/questions on the dma_resv side, but frankly we can
also adjust them later on - either when we land the code or with a quick
patch for the rfc docs here.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Intel-gfx] [PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document
@ 2022-06-24 19:31 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-06-24 19:31 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: chris.p.wilson, paulo.r.zanoni, intel-gfx, thomas.hellstrom,
dri-devel, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 10:49:34AM -0700, Niranjana Vishwanathapura wrote:
> VM_BIND design document with description of intended use cases.
>
> v2: Reduce the scope to simple Mesa use case.
> v3: Expand documentation on dma-resv usage, TLB flushing and
> execbuf3.
> v4: Remove vm_bind tlb flush request support.
> v5: Update TLB flushing documentation.
>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> ---
> Documentation/gpu/rfc/i915_vm_bind.rst | 246 +++++++++++++++++++++++++
> Documentation/gpu/rfc/index.rst | 4 +
> 2 files changed, 250 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst
>
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst b/Documentation/gpu/rfc/i915_vm_bind.rst
> new file mode 100644
> index 000000000000..534adf0c6c7a
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.rst
> @@ -0,0 +1,246 @@
> +==========================================
> +I915 VM_BIND feature design and use cases
> +==========================================
> +
> +VM_BIND feature
> +================
> +DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
> +objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
> +specified address space (VM). These mappings (also referred to as persistent
> +mappings) will be persistent across multiple GPU submissions (execbuf calls)
> +issued by the UMD, without user having to provide a list of all required
> +mappings during each submission (as required by older execbuf mode).
> +
> +The VM_BIND/UNBIND calls allow UMDs to request a timeline fence for signaling
> +the completion of bind/unbind operation.
> +
> +VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.
> +User has to opt-in for VM_BIND mode of binding for an address space (VM)
> +during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
> +
> +The bind/unbind operation can get completed asynchronously and out of
> +submission order. The out fence when specified will be signaled upon
> +completion of bind/unbind operation.
> +
> +VM_BIND features include:
> +
> +* Multiple Virtual Address (VA) mappings can map to the same physical pages
> + of an object (aliasing).
> +* VA mapping can map to a partial section of the BO (partial binding).
> +* Support capture of persistent mappings in the dump upon GPU error.
> +* Support for userptr gem objects (no special uapi is required for this).
> +
> +TLB flush consideration
> +------------------------
> +The i915 driver flushes the TLB for each submission and when an object's
> +pages are released. The VM_BIND/UNBIND operation will not do any additional
> +TLB flush. Any VM_BIND mapping added will be in the working set for subsequent
> +submissions on that VM and will not be in the working set for currently running
> +batches (which would require additional TLB flushes, which is not supported).
> +
> +Execbuf ioctl in VM_BIND mode
> +-------------------------------
> +A VM in VM_BIND mode will not support older execbuf mode of binding.
> +The execbuf ioctl handling in VM_BIND mode differs significantly from the
> +older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2).
> +Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See
> +struct drm_i915_gem_execbuffer3). The execbuf3 ioctl will not accept any
> +execlist. Hence, no support for implicit sync. It is expected that the below
> +work will be able to support requirements of object dependency setting in all
> +use cases:
> +
> +"dma-buf: Add an API for exporting sync files"
> +(https://lwn.net/Articles/859290/)
> +
> +The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only
> +works with execbuf3 ioctl for submission. All BOs mapped on that VM (through
> +VM_BIND call) at the time of execbuf3 call are deemed required for that
> +submission.
> +
> +The execbuf3 ioctl directly specifies the batch addresses instead of as
> +object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not
> +support many of the older features like in/out/submit fences, fence array,
> +default gem context and many more (See struct drm_i915_gem_execbuffer3).
> +
> +In VM_BIND mode, VA allocation is completely managed by the user instead of
> +the i915 driver. Hence all VA assignment, eviction are not applicable in
> +VM_BIND mode. Also, for determining object activeness, VM_BIND mode will not
> +be using the i915_vma active reference tracking. It will instead use dma-resv
> +object for that (See `VM_BIND dma_resv usage`_).
> +
> +So, a lot of existing code supporting execbuf2 ioctl, like relocations, VA
> +evictions, vma lookup table, implicit sync, vma active reference tracking etc.,
> +are not applicable for execbuf3 ioctl. Hence, all execbuf3 specific handling
> +should be in a separate file and only functionalities common to these ioctls
> +can be the shared code where possible.
> +
> +VM_PRIVATE objects
> +-------------------
> +By default, BOs can be mapped on multiple VMs and can also be dma-buf
> +exported. Hence these BOs are referred to as Shared BOs.
> +During each execbuf submission, the request fence must be added to the
> +dma-resv fence list of all shared BOs mapped on the VM.
> +
> +VM_BIND feature introduces an optimization where user can create BO which
> +is private to a specified VM via I915_GEM_CREATE_EXT_VM_PRIVATE flag during
> +BO creation. Unlike Shared BOs, these VM private BOs can only be mapped on
> +the VM they are private to and can't be dma-buf exported.
> +All private BOs of a VM share the dma-resv object. Hence during each execbuf
> +submission, they need only one dma-resv fence list updated. Thus, the fast
> +path (where required mappings are already bound) submission latency is O(1)
> +w.r.t the number of VM private BOs.
> +
> +VM_BIND locking hirarchy
> +-------------------------
> +The locking design here supports the older (execlist based) execbuf mode, the
> +newer VM_BIND mode, the VM_BIND mode with GPU page faults and possible future
> +system allocator support (See `Shared Virtual Memory (SVM) support`_).
> +The older execbuf mode and the newer VM_BIND mode without page faults manages
> +residency of backing storage using dma_fence. The VM_BIND mode with page faults
> +and the system allocator support do not use any dma_fence at all.
> +
> +VM_BIND locking order is as below.
> +
> +1) Lock-A: A vm_bind mutex will protect vm_bind lists. This lock is taken in
> + vm_bind/vm_unbind ioctl calls, in the execbuf path and while releasing the
> + mapping.
> +
> + In future, when GPU page faults are supported, we can potentially use a
> + rwsem instead, so that multiple page fault handlers can take the read side
> + lock to lookup the mapping and hence can run in parallel.
> + The older execbuf mode of binding do not need this lock.
> +
> +2) Lock-B: The object's dma-resv lock will protect i915_vma state and needs to
> + be held while binding/unbinding a vma in the async worker and while updating
> + dma-resv fence list of an object. Note that private BOs of a VM will all
> + share a dma-resv object.
> +
> + The future system allocator support will use the HMM prescribed locking
> + instead.
> +
> +3) Lock-C: Spinlock/s to protect some of the VM's lists like the list of
> + invalidated vmas (due to eviction and userptr invalidation) etc.
> +
> +When GPU page faults are supported, the execbuf path do not take any of these
> +locks. There we will simply smash the new batch buffer address into the ring and
> +then tell the scheduler run that. The lock taking only happens from the page
> +fault handler, where we take lock-A in read mode, whichever lock-B we need to
> +find the backing storage (dma_resv lock for gem objects, and hmm/core mm for
> +system allocator) and some additional locks (lock-D) for taking care of page
> +table races. Page fault mode should not need to ever manipulate the vm lists,
> +so won't ever need lock-C.
> +
> +VM_BIND LRU handling
> +---------------------
> +We need to ensure VM_BIND mapped objects are properly LRU tagged to avoid
> +performance degradation. We will also need support for bulk LRU movement of
> +VM_BIND objects to avoid additional latencies in execbuf path.
> +
> +The page table pages are similar to VM_BIND mapped objects (See
> +`Evictable page table allocations`_) and are maintained per VM and needs to
> +be pinned in memory when VM is made active (ie., upon an execbuf call with
> +that VM). So, bulk LRU movement of page table pages is also needed.
> +
> +VM_BIND dma_resv usage
> +-----------------------
> +Fences needs to be added to all VM_BIND mapped objects. During each execbuf
> +submission, they are added with DMA_RESV_USAGE_BOOKKEEP usage to prevent
> +over sync (See enum dma_resv_usage). One can override it with either
> +DMA_RESV_USAGE_READ or DMA_RESV_USAGE_WRITE usage during explicit object
> +dependency setting.
I thought we've ditched that in favour of the import/export ioctls, and
execbuf will only ever set BOOKKEEPING?
> +Note that DRM_I915_GEM_WAIT and DRM_I915_GEM_BUSY ioctls do not check for
> +DMA_RESV_USAGE_BOOKKEEP usage and hence should not be used for end of batch
> +check. Instead, the execbuf3 out fence should be used for end of batch check
> +(See struct drm_i915_gem_execbuffer3).
This is a bit confused. Changing gem_wait and gem_busy ioctls to also look
at bookkeeping will not fix the issue, because in a vm_bind vm _all_
objects (and hence also all batchbuffers mapped into the vm) always have
the same fences in their dma_resv from that vm. Which means that if any
batchbuffer is still running, all batchbuffers are considered active. And
so checking fences in the dma_resv is useless for the umd and it needs to
track batchbuffer usage itself somewhere.
> +
> +Also, in VM_BIND mode, use dma-resv apis for determining object activeness
> +(See dma_resv_test_signaled() and dma_resv_wait_timeout()) and do not use the
> +older i915_vma active reference tracking which is deprecated. This should be
> +easier to get it working with the current TTM backend.
Hm I thought he plan is to require ttm, since ttm has the bulk lru stuff
only and our i915-gem shrinker/lru code is very busted? Maybe needs to be
clarified slightly.
> +
> +Mesa use case
> +--------------
> +VM_BIND can potentially reduce the CPU overhead in Mesa (both Vulkan and Iris),
> +hence improving performance of CPU-bound applications. It also allows us to
> +implement Vulkan's Sparse Resources. With increasing GPU hardware performance,
> +reducing CPU overhead becomes more impactful.
> +
> +
> +Other VM_BIND use cases
> +========================
> +
> +Long running Compute contexts
> +------------------------------
> +Usage of dma-fence expects that they complete in reasonable amount of time.
> +Compute on the other hand can be long running. Hence it is appropriate for
> +compute to use user/memory fence (See `User/Memory Fence`_) and dma-fence usage
> +must be limited to in-kernel consumption only.
> +
> +Where GPU page faults are not available, kernel driver upon buffer invalidation
> +will initiate a suspend (preemption) of long running context, finish the
> +invalidation, revalidate the BO and then resume the compute context. This is
> +done by having a per-context preempt fence which is enabled when someone tries
> +to wait on it and triggers the context preemption.
> +
> +User/Memory Fence
> +~~~~~~~~~~~~~~~~~~
> +User/Memory fence is a <address, value> pair. To signal the user fence, the
> +specified value will be written at the specified virtual address and wakeup the
> +waiting process. User fence can be signaled either by the GPU or kernel async
> +worker (like upon bind completion). User can wait on a user fence with a new
> +user fence wait ioctl.
> +
> +Here is some prior work on this:
> +https://patchwork.freedesktop.org/patch/349417/
> +
> +Low Latency Submission
> +~~~~~~~~~~~~~~~~~~~~~~~
> +Allows compute UMD to directly submit GPU jobs instead of through execbuf
> +ioctl. This is made possible by VM_BIND is not being synchronized against
> +execbuf. VM_BIND allows bind/unbind of mappings required for the directly
> +submitted jobs.
> +
> +Debugger
> +---------
> +With debug event interface user space process (debugger) is able to keep track
> +of and act upon resources created by another process (debugged) and attached
> +to GPU via vm_bind interface.
> +
> +GPU page faults
> +----------------
> +GPU page faults when supported (in future), will only be supported in the
> +VM_BIND mode. While both the older execbuf mode and the newer VM_BIND mode of
> +binding will require using dma-fence to ensure residency, the GPU page faults
> +mode when supported, will not use any dma-fence as residency is purely managed
> +by installing and removing/invalidating page table entries.
> +
> +Page level hints settings
> +--------------------------
> +VM_BIND allows any hints setting per mapping instead of per BO.
> +Possible hints include read-only mapping, placement and atomicity.
> +Sub-BO level placement hint will be even more relevant with
> +upcoming GPU on-demand page fault support.
> +
> +Page level Cache/CLOS settings
> +-------------------------------
> +VM_BIND allows cache/CLOS settings per mapping instead of per BO.
> +
> +Evictable page table allocations
> +---------------------------------
> +Make pagetable allocations evictable and manage them similar to VM_BIND
> +mapped objects. Page table pages are similar to persistent mappings of a
> +VM (difference here are that the page table pages will not have an i915_vma
> +structure and after swapping pages back in, parent page link needs to be
> +updated).
> +
> +Shared Virtual Memory (SVM) support
> +------------------------------------
> +VM_BIND interface can be used to map system memory directly (without gem BO
> +abstraction) using the HMM interface. SVM is only supported with GPU page
> +faults enabled.
> +
> +VM_BIND UAPI
> +=============
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_vm_bind.h
> diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
> index 91e93a705230..7d10c36b268d 100644
> --- a/Documentation/gpu/rfc/index.rst
> +++ b/Documentation/gpu/rfc/index.rst
> @@ -23,3 +23,7 @@ host such documentation:
> .. toctree::
>
> i915_scheduler.rst
> +
> +.. toctree::
> +
> + i915_vm_bind.rst
> --
> 2.21.0.rc0.32.g243a4c7e27
So I had a few comments/questions on the dma_resv side, but frankly we can
also adjust them later on - either when we land the code or with a quick
patch for the rfc docs here.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
-1 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: matthew.brost, paulo.r.zanoni, lionel.g.landwerlin,
tvrtko.ursulin, chris.p.wilson, thomas.hellstrom, oak.zeng,
matthew.auld, jason, daniel.vetter, christian.koenig
Add some missing i915 upai documentation which the new
i915 VM_BIND feature documentation will be refer to.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
include/uapi/drm/i915_drm.h | 205 ++++++++++++++++++++++++++++--------
1 file changed, 160 insertions(+), 45 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index de49b68b4fc8..4afe95d8b98b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
/* Must be kept compact -- no holes and well documented */
-typedef struct drm_i915_getparam {
+/**
+ * struct drm_i915_getparam - Driver parameter query structure.
+ */
+struct drm_i915_getparam {
+ /** @param: Driver parameter to query. */
__s32 param;
- /*
+
+ /**
+ * @value: Address of memory where queried value should be put.
+ *
* WARNING: Using pointers instead of fixed-size u64 means we need to write
* compat32 code. Don't repeat this mistake.
*/
int __user *value;
-} drm_i915_getparam_t;
+};
+
+/**
+ * typedef drm_i915_getparam_t - Driver parameter query structure.
+ * See struct drm_i915_getparam.
+ */
+typedef struct drm_i915_getparam drm_i915_getparam_t;
/* Ioctl to set kernel params:
*/
@@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
__u64 rsvd2;
};
+/**
+ * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
+ * ioctl.
+ *
+ * The request will wait for input fence to signal before submission.
+ *
+ * The returned output fence will be signaled after the completion of the
+ * request.
+ */
struct drm_i915_gem_exec_fence {
- /**
- * User's handle for a drm_syncobj to wait on or signal.
- */
+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
__u32 handle;
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_EXEC_FENCE_WAIT:
+ * Wait for the input fence before request submission.
+ *
+ * I915_EXEC_FENCE_SIGNAL:
+ * Return request completion fence as output
+ */
+ __u32 flags;
#define I915_EXEC_FENCE_WAIT (1<<0)
#define I915_EXEC_FENCE_SIGNAL (1<<1)
#define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
- __u32 flags;
};
-/*
- * See drm_i915_gem_execbuffer_ext_timeline_fences.
- */
-#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
-
-/*
+/**
+ * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
+ * for execbuf ioctl.
+ *
* This structure describes an array of drm_syncobj and associated points for
* timeline variants of drm_syncobj. It is invalid to append this structure to
* the execbuf if I915_EXEC_FENCE_ARRAY is set.
*/
struct drm_i915_gem_execbuffer_ext_timeline_fences {
+#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
+ /** @base: Extension link. See struct i915_user_extension. */
struct i915_user_extension base;
/**
- * Number of element in the handles_ptr & value_ptr arrays.
+ * @fence_count: Number of elements in the @handles_ptr & @value_ptr
+ * arrays.
*/
__u64 fence_count;
/**
- * Pointer to an array of struct drm_i915_gem_exec_fence of length
- * fence_count.
+ * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
+ * of length @fence_count.
*/
__u64 handles_ptr;
/**
- * Pointer to an array of u64 values of length fence_count. Values
- * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
- * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
+ * @values_ptr: Pointer to an array of u64 values of length
+ * @fence_count.
+ * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
+ * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
+ * binary one.
*/
__u64 values_ptr;
};
+/**
+ * struct drm_i915_gem_execbuffer2 - Structure for DRM_I915_GEM_EXECBUFFER2
+ * ioctl.
+ */
struct drm_i915_gem_execbuffer2 {
- /**
- * List of gem_exec_object2 structs
- */
+ /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
__u64 buffers_ptr;
+
+ /** @buffer_count: Number of elements in @buffers_ptr array */
__u32 buffer_count;
- /** Offset in the batchbuffer to start execution from. */
+ /**
+ * @batch_start_offset: Offset in the batchbuffer to start execution
+ * from.
+ */
__u32 batch_start_offset;
- /** Bytes used in batchbuffer from batch_start_offset */
+
+ /**
+ * @batch_len: Length in bytes of the batch buffer, starting from the
+ * @batch_start_offset. If 0, length is assumed to be the batch buffer
+ * object size.
+ */
__u32 batch_len;
+
+ /** @DR1: deprecated */
__u32 DR1;
+
+ /** @DR4: deprecated */
__u32 DR4;
+
+ /** @num_cliprects: See @cliprects_ptr */
__u32 num_cliprects;
+
/**
- * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
- * & I915_EXEC_USE_EXTENSIONS are not set.
+ * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
+ *
+ * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
+ * I915_EXEC_USE_EXTENSIONS flags are not set.
*
* If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
- * of struct drm_i915_gem_exec_fence and num_cliprects is the length
- * of the array.
+ * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
+ * array.
*
* If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
- * single struct i915_user_extension and num_cliprects is 0.
+ * single &i915_user_extension and num_cliprects is 0.
*/
__u64 cliprects_ptr;
+
+ /** @flags: Execbuf flags */
+ __u64 flags;
#define I915_EXEC_RING_MASK (0x3f)
#define I915_EXEC_DEFAULT (0<<0)
#define I915_EXEC_RENDER (1<<0)
@@ -1326,10 +1382,6 @@ struct drm_i915_gem_execbuffer2 {
#define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
#define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6)
#define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
- __u64 flags;
- __u64 rsvd1; /* now used for context info */
- __u64 rsvd2;
-};
/** Resets the SO write offset registers for transform feedback on gen7. */
#define I915_EXEC_GEN7_SOL_RESET (1<<8)
@@ -1432,9 +1484,23 @@ struct drm_i915_gem_execbuffer2 {
* drm_i915_gem_execbuffer_ext enum.
*/
#define I915_EXEC_USE_EXTENSIONS (1 << 21)
-
#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
+ /** @rsvd1: Context id */
+ __u64 rsvd1;
+
+ /**
+ * @rsvd2: in and out sync_file file descriptors.
+ *
+ * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
+ * lower 32 bits of this field will have the in sync_file fd (input).
+ *
+ * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
+ * field will have the out sync_file fd (output).
+ */
+ __u64 rsvd2;
+};
+
#define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
#define i915_execbuffer2_set_context_id(eb2, context) \
(eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
@@ -1814,19 +1880,58 @@ struct drm_i915_gem_context_create {
__u32 pad;
};
+/**
+ * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
+ */
struct drm_i915_gem_context_create_ext {
- __u32 ctx_id; /* output: id of new context*/
+ /** @ctx_id: Id of the created context (output) */
+ __u32 ctx_id;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
+ *
+ * Extensions may be appended to this structure and driver must check
+ * for those. See @extensions.
+ *
+ * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
+ *
+ * Created context will have single timeline.
+ */
__u32 flags;
#define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS (1u << 0)
#define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE (1u << 1)
#define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
(-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
+
+ /**
+ * @extensions: Zero-terminated chain of extensions.
+ *
+ * I915_CONTEXT_CREATE_EXT_SETPARAM:
+ * Context parameter to set or query during context creation.
+ * See struct drm_i915_gem_context_create_ext_setparam.
+ *
+ * I915_CONTEXT_CREATE_EXT_CLONE:
+ * This extension has been removed. On the off chance someone somewhere
+ * has attempted to use it, never re-use this extension number.
+ */
__u64 extensions;
+#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
+#define I915_CONTEXT_CREATE_EXT_CLONE 1
};
+/**
+ * struct drm_i915_gem_context_param - Context parameter to set or query.
+ */
struct drm_i915_gem_context_param {
+ /** @ctx_id: Context id */
__u32 ctx_id;
+
+ /** @size: Size of the parameter @value */
__u32 size;
+
+ /** @param: Parameter to set or query */
__u64 param;
#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
/* I915_CONTEXT_PARAM_NO_ZEROMAP has been removed. On the off chance
@@ -1973,6 +2078,7 @@ struct drm_i915_gem_context_param {
#define I915_CONTEXT_PARAM_PROTECTED_CONTENT 0xd
/* Must be kept compact -- no holes and well documented */
+ /** @value: Context parameter value to be set or queried */
__u64 value;
};
@@ -2371,23 +2477,29 @@ struct i915_context_param_engines {
struct i915_engine_class_instance engines[N__]; \
} __attribute__((packed)) name__
+/**
+ * struct drm_i915_gem_context_create_ext_setparam - Context parameter
+ * to set or query during context creation.
+ */
struct drm_i915_gem_context_create_ext_setparam {
-#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
+ /** @base: Extension link. See struct i915_user_extension. */
struct i915_user_extension base;
+
+ /**
+ * @param: Context parameter to set or query.
+ * See struct drm_i915_gem_context_param.
+ */
struct drm_i915_gem_context_param param;
};
-/* This API has been removed. On the off chance someone somewhere has
- * attempted to use it, never re-use this extension number.
- */
-#define I915_CONTEXT_CREATE_EXT_CLONE 1
-
struct drm_i915_gem_context_destroy {
__u32 ctx_id;
__u32 pad;
};
-/*
+/**
+ * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
+ *
* DRM_I915_GEM_VM_CREATE -
*
* Create a new virtual memory address space (ppGTT) for use within a context
@@ -2397,20 +2509,23 @@ struct drm_i915_gem_context_destroy {
* The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
* returned in the outparam @id.
*
- * No flags are defined, with all bits reserved and must be zero.
- *
* An extension chain maybe provided, starting with @extensions, and terminated
* by the @next_extension being 0. Currently, no extensions are defined.
*
* DRM_I915_GEM_VM_DESTROY -
*
- * Destroys a previously created VM id, specified in @id.
+ * Destroys a previously created VM id, specified in @vm_id.
*
* No extensions or flags are allowed currently, and so must be zero.
*/
struct drm_i915_gem_vm_control {
+ /** @extensions: Zero-terminated chain of extensions. */
__u64 extensions;
+
+ /** @flags: reserved for future usage, currently MBZ */
__u32 flags;
+
+ /** @vm_id: Id of the VM created or to be destroyed */
__u32 vm_id;
};
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Intel-gfx] [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: paulo.r.zanoni, chris.p.wilson, thomas.hellstrom, matthew.auld,
daniel.vetter, christian.koenig
Add some missing i915 upai documentation which the new
i915 VM_BIND feature documentation will be refer to.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
include/uapi/drm/i915_drm.h | 205 ++++++++++++++++++++++++++++--------
1 file changed, 160 insertions(+), 45 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index de49b68b4fc8..4afe95d8b98b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
/* Must be kept compact -- no holes and well documented */
-typedef struct drm_i915_getparam {
+/**
+ * struct drm_i915_getparam - Driver parameter query structure.
+ */
+struct drm_i915_getparam {
+ /** @param: Driver parameter to query. */
__s32 param;
- /*
+
+ /**
+ * @value: Address of memory where queried value should be put.
+ *
* WARNING: Using pointers instead of fixed-size u64 means we need to write
* compat32 code. Don't repeat this mistake.
*/
int __user *value;
-} drm_i915_getparam_t;
+};
+
+/**
+ * typedef drm_i915_getparam_t - Driver parameter query structure.
+ * See struct drm_i915_getparam.
+ */
+typedef struct drm_i915_getparam drm_i915_getparam_t;
/* Ioctl to set kernel params:
*/
@@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
__u64 rsvd2;
};
+/**
+ * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
+ * ioctl.
+ *
+ * The request will wait for input fence to signal before submission.
+ *
+ * The returned output fence will be signaled after the completion of the
+ * request.
+ */
struct drm_i915_gem_exec_fence {
- /**
- * User's handle for a drm_syncobj to wait on or signal.
- */
+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
__u32 handle;
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_EXEC_FENCE_WAIT:
+ * Wait for the input fence before request submission.
+ *
+ * I915_EXEC_FENCE_SIGNAL:
+ * Return request completion fence as output
+ */
+ __u32 flags;
#define I915_EXEC_FENCE_WAIT (1<<0)
#define I915_EXEC_FENCE_SIGNAL (1<<1)
#define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
- __u32 flags;
};
-/*
- * See drm_i915_gem_execbuffer_ext_timeline_fences.
- */
-#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
-
-/*
+/**
+ * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
+ * for execbuf ioctl.
+ *
* This structure describes an array of drm_syncobj and associated points for
* timeline variants of drm_syncobj. It is invalid to append this structure to
* the execbuf if I915_EXEC_FENCE_ARRAY is set.
*/
struct drm_i915_gem_execbuffer_ext_timeline_fences {
+#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
+ /** @base: Extension link. See struct i915_user_extension. */
struct i915_user_extension base;
/**
- * Number of element in the handles_ptr & value_ptr arrays.
+ * @fence_count: Number of elements in the @handles_ptr & @value_ptr
+ * arrays.
*/
__u64 fence_count;
/**
- * Pointer to an array of struct drm_i915_gem_exec_fence of length
- * fence_count.
+ * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
+ * of length @fence_count.
*/
__u64 handles_ptr;
/**
- * Pointer to an array of u64 values of length fence_count. Values
- * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
- * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
+ * @values_ptr: Pointer to an array of u64 values of length
+ * @fence_count.
+ * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
+ * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
+ * binary one.
*/
__u64 values_ptr;
};
+/**
+ * struct drm_i915_gem_execbuffer2 - Structure for DRM_I915_GEM_EXECBUFFER2
+ * ioctl.
+ */
struct drm_i915_gem_execbuffer2 {
- /**
- * List of gem_exec_object2 structs
- */
+ /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
__u64 buffers_ptr;
+
+ /** @buffer_count: Number of elements in @buffers_ptr array */
__u32 buffer_count;
- /** Offset in the batchbuffer to start execution from. */
+ /**
+ * @batch_start_offset: Offset in the batchbuffer to start execution
+ * from.
+ */
__u32 batch_start_offset;
- /** Bytes used in batchbuffer from batch_start_offset */
+
+ /**
+ * @batch_len: Length in bytes of the batch buffer, starting from the
+ * @batch_start_offset. If 0, length is assumed to be the batch buffer
+ * object size.
+ */
__u32 batch_len;
+
+ /** @DR1: deprecated */
__u32 DR1;
+
+ /** @DR4: deprecated */
__u32 DR4;
+
+ /** @num_cliprects: See @cliprects_ptr */
__u32 num_cliprects;
+
/**
- * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
- * & I915_EXEC_USE_EXTENSIONS are not set.
+ * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
+ *
+ * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
+ * I915_EXEC_USE_EXTENSIONS flags are not set.
*
* If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
- * of struct drm_i915_gem_exec_fence and num_cliprects is the length
- * of the array.
+ * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
+ * array.
*
* If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
- * single struct i915_user_extension and num_cliprects is 0.
+ * single &i915_user_extension and num_cliprects is 0.
*/
__u64 cliprects_ptr;
+
+ /** @flags: Execbuf flags */
+ __u64 flags;
#define I915_EXEC_RING_MASK (0x3f)
#define I915_EXEC_DEFAULT (0<<0)
#define I915_EXEC_RENDER (1<<0)
@@ -1326,10 +1382,6 @@ struct drm_i915_gem_execbuffer2 {
#define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
#define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6)
#define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
- __u64 flags;
- __u64 rsvd1; /* now used for context info */
- __u64 rsvd2;
-};
/** Resets the SO write offset registers for transform feedback on gen7. */
#define I915_EXEC_GEN7_SOL_RESET (1<<8)
@@ -1432,9 +1484,23 @@ struct drm_i915_gem_execbuffer2 {
* drm_i915_gem_execbuffer_ext enum.
*/
#define I915_EXEC_USE_EXTENSIONS (1 << 21)
-
#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
+ /** @rsvd1: Context id */
+ __u64 rsvd1;
+
+ /**
+ * @rsvd2: in and out sync_file file descriptors.
+ *
+ * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
+ * lower 32 bits of this field will have the in sync_file fd (input).
+ *
+ * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
+ * field will have the out sync_file fd (output).
+ */
+ __u64 rsvd2;
+};
+
#define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
#define i915_execbuffer2_set_context_id(eb2, context) \
(eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
@@ -1814,19 +1880,58 @@ struct drm_i915_gem_context_create {
__u32 pad;
};
+/**
+ * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
+ */
struct drm_i915_gem_context_create_ext {
- __u32 ctx_id; /* output: id of new context*/
+ /** @ctx_id: Id of the created context (output) */
+ __u32 ctx_id;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
+ *
+ * Extensions may be appended to this structure and driver must check
+ * for those. See @extensions.
+ *
+ * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
+ *
+ * Created context will have single timeline.
+ */
__u32 flags;
#define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS (1u << 0)
#define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE (1u << 1)
#define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
(-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
+
+ /**
+ * @extensions: Zero-terminated chain of extensions.
+ *
+ * I915_CONTEXT_CREATE_EXT_SETPARAM:
+ * Context parameter to set or query during context creation.
+ * See struct drm_i915_gem_context_create_ext_setparam.
+ *
+ * I915_CONTEXT_CREATE_EXT_CLONE:
+ * This extension has been removed. On the off chance someone somewhere
+ * has attempted to use it, never re-use this extension number.
+ */
__u64 extensions;
+#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
+#define I915_CONTEXT_CREATE_EXT_CLONE 1
};
+/**
+ * struct drm_i915_gem_context_param - Context parameter to set or query.
+ */
struct drm_i915_gem_context_param {
+ /** @ctx_id: Context id */
__u32 ctx_id;
+
+ /** @size: Size of the parameter @value */
__u32 size;
+
+ /** @param: Parameter to set or query */
__u64 param;
#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
/* I915_CONTEXT_PARAM_NO_ZEROMAP has been removed. On the off chance
@@ -1973,6 +2078,7 @@ struct drm_i915_gem_context_param {
#define I915_CONTEXT_PARAM_PROTECTED_CONTENT 0xd
/* Must be kept compact -- no holes and well documented */
+ /** @value: Context parameter value to be set or queried */
__u64 value;
};
@@ -2371,23 +2477,29 @@ struct i915_context_param_engines {
struct i915_engine_class_instance engines[N__]; \
} __attribute__((packed)) name__
+/**
+ * struct drm_i915_gem_context_create_ext_setparam - Context parameter
+ * to set or query during context creation.
+ */
struct drm_i915_gem_context_create_ext_setparam {
-#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
+ /** @base: Extension link. See struct i915_user_extension. */
struct i915_user_extension base;
+
+ /**
+ * @param: Context parameter to set or query.
+ * See struct drm_i915_gem_context_param.
+ */
struct drm_i915_gem_context_param param;
};
-/* This API has been removed. On the off chance someone somewhere has
- * attempted to use it, never re-use this extension number.
- */
-#define I915_CONTEXT_CREATE_EXT_CLONE 1
-
struct drm_i915_gem_context_destroy {
__u32 ctx_id;
__u32 pad;
};
-/*
+/**
+ * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
+ *
* DRM_I915_GEM_VM_CREATE -
*
* Create a new virtual memory address space (ppGTT) for use within a context
@@ -2397,20 +2509,23 @@ struct drm_i915_gem_context_destroy {
* The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
* returned in the outparam @id.
*
- * No flags are defined, with all bits reserved and must be zero.
- *
* An extension chain maybe provided, starting with @extensions, and terminated
* by the @next_extension being 0. Currently, no extensions are defined.
*
* DRM_I915_GEM_VM_DESTROY -
*
- * Destroys a previously created VM id, specified in @id.
+ * Destroys a previously created VM id, specified in @vm_id.
*
* No extensions or flags are allowed currently, and so must be zero.
*/
struct drm_i915_gem_vm_control {
+ /** @extensions: Zero-terminated chain of extensions. */
__u64 extensions;
+
+ /** @flags: reserved for future usage, currently MBZ */
__u32 flags;
+
+ /** @vm_id: Id of the VM created or to be destroyed */
__u32 vm_id;
};
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
@ 2022-06-24 19:38 ` Daniel Vetter
-1 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-06-24 19:38 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: matthew.brost, chris.p.wilson, paulo.r.zanoni, tvrtko.ursulin,
intel-gfx, lionel.g.landwerlin, thomas.hellstrom, oak.zeng,
dri-devel, jason, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 10:49:35AM -0700, Niranjana Vishwanathapura wrote:
> Add some missing i915 upai documentation which the new
> i915 VM_BIND feature documentation will be refer to.
>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
> include/uapi/drm/i915_drm.h | 205 ++++++++++++++++++++++++++++--------
> 1 file changed, 160 insertions(+), 45 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index de49b68b4fc8..4afe95d8b98b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
>
> /* Must be kept compact -- no holes and well documented */
>
> -typedef struct drm_i915_getparam {
> +/**
> + * struct drm_i915_getparam - Driver parameter query structure.
> + */
> +struct drm_i915_getparam {
> + /** @param: Driver parameter to query. */
> __s32 param;
> - /*
> +
> + /**
> + * @value: Address of memory where queried value should be put.
> + *
> * WARNING: Using pointers instead of fixed-size u64 means we need to write
> * compat32 code. Don't repeat this mistake.
> */
> int __user *value;
> -} drm_i915_getparam_t;
> +};
> +
> +/**
> + * typedef drm_i915_getparam_t - Driver parameter query structure.
> + * See struct drm_i915_getparam.
> + */
> +typedef struct drm_i915_getparam drm_i915_getparam_t;
>
> /* Ioctl to set kernel params:
> */
> @@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
> __u64 rsvd2;
> };
>
> +/**
> + * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
> + * ioctl.
> + *
> + * The request will wait for input fence to signal before submission.
> + *
> + * The returned output fence will be signaled after the completion of the
> + * request.
> + */
> struct drm_i915_gem_exec_fence {
> - /**
> - * User's handle for a drm_syncobj to wait on or signal.
> - */
> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
> __u32 handle;
>
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_EXEC_FENCE_WAIT:
> + * Wait for the input fence before request submission.
> + *
> + * I915_EXEC_FENCE_SIGNAL:
> + * Return request completion fence as output
> + */
> + __u32 flags;
> #define I915_EXEC_FENCE_WAIT (1<<0)
> #define I915_EXEC_FENCE_SIGNAL (1<<1)
> #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
> - __u32 flags;
> };
>
> -/*
> - * See drm_i915_gem_execbuffer_ext_timeline_fences.
> - */
> -#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> -
> -/*
> +/**
> + * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
> + * for execbuf ioctl.
> + *
> * This structure describes an array of drm_syncobj and associated points for
> * timeline variants of drm_syncobj. It is invalid to append this structure to
> * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> */
> struct drm_i915_gem_execbuffer_ext_timeline_fences {
> +#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> + /** @base: Extension link. See struct i915_user_extension. */
> struct i915_user_extension base;
>
> /**
> - * Number of element in the handles_ptr & value_ptr arrays.
> + * @fence_count: Number of elements in the @handles_ptr & @value_ptr
> + * arrays.
> */
> __u64 fence_count;
>
> /**
> - * Pointer to an array of struct drm_i915_gem_exec_fence of length
> - * fence_count.
> + * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
> + * of length @fence_count.
> */
> __u64 handles_ptr;
>
> /**
> - * Pointer to an array of u64 values of length fence_count. Values
> - * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
> - * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
> + * @values_ptr: Pointer to an array of u64 values of length
> + * @fence_count.
> + * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
> + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> + * binary one.
> */
> __u64 values_ptr;
> };
>
> +/**
> + * struct drm_i915_gem_execbuffer2 - Structure for DRM_I915_GEM_EXECBUFFER2
> + * ioctl.
> + */
> struct drm_i915_gem_execbuffer2 {
> - /**
> - * List of gem_exec_object2 structs
> - */
> + /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
> __u64 buffers_ptr;
> +
> + /** @buffer_count: Number of elements in @buffers_ptr array */
> __u32 buffer_count;
>
> - /** Offset in the batchbuffer to start execution from. */
> + /**
> + * @batch_start_offset: Offset in the batchbuffer to start execution
> + * from.
> + */
> __u32 batch_start_offset;
> - /** Bytes used in batchbuffer from batch_start_offset */
> +
> + /**
> + * @batch_len: Length in bytes of the batch buffer, starting from the
> + * @batch_start_offset. If 0, length is assumed to be the batch buffer
> + * object size.
> + */
> __u32 batch_len;
> +
> + /** @DR1: deprecated */
> __u32 DR1;
> +
> + /** @DR4: deprecated */
> __u32 DR4;
> +
> + /** @num_cliprects: See @cliprects_ptr */
> __u32 num_cliprects;
> +
> /**
> - * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
> - * & I915_EXEC_USE_EXTENSIONS are not set.
> + * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
> + *
> + * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
> + * I915_EXEC_USE_EXTENSIONS flags are not set.
> *
> * If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
> - * of struct drm_i915_gem_exec_fence and num_cliprects is the length
> - * of the array.
> + * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
> + * array.
> *
> * If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
> - * single struct i915_user_extension and num_cliprects is 0.
> + * single &i915_user_extension and num_cliprects is 0.
> */
> __u64 cliprects_ptr;
> +
> + /** @flags: Execbuf flags */
> + __u64 flags;
> #define I915_EXEC_RING_MASK (0x3f)
> #define I915_EXEC_DEFAULT (0<<0)
> #define I915_EXEC_RENDER (1<<0)
> @@ -1326,10 +1382,6 @@ struct drm_i915_gem_execbuffer2 {
> #define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
> #define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6)
> #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
> - __u64 flags;
> - __u64 rsvd1; /* now used for context info */
> - __u64 rsvd2;
> -};
>
> /** Resets the SO write offset registers for transform feedback on gen7. */
> #define I915_EXEC_GEN7_SOL_RESET (1<<8)
> @@ -1432,9 +1484,23 @@ struct drm_i915_gem_execbuffer2 {
> * drm_i915_gem_execbuffer_ext enum.
> */
> #define I915_EXEC_USE_EXTENSIONS (1 << 21)
> -
> #define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
>
> + /** @rsvd1: Context id */
> + __u64 rsvd1;
> +
> + /**
> + * @rsvd2: in and out sync_file file descriptors.
> + *
> + * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
> + * lower 32 bits of this field will have the in sync_file fd (input).
> + *
> + * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
> + * field will have the out sync_file fd (output).
> + */
> + __u64 rsvd2;
> +};
> +
> #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
> #define i915_execbuffer2_set_context_id(eb2, context) \
> (eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
> @@ -1814,19 +1880,58 @@ struct drm_i915_gem_context_create {
> __u32 pad;
> };
>
> +/**
> + * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
I think a follow up patch which explains that despite that the uapi seems
to allow context properties to be mutable this isn't the case, and the
first execbuf call will freeze the context. Not super important for
vm_bind except that this means the execbuf will convert the proto context
into a real context and freeze the vm, which also locks down all the vm
related semantics we're adding. So might be good to add a paragraph here
explaining how this works (and then perhaps go through all the context
parameters and explain which can still be changed after the first
execbuf).
> + */
> struct drm_i915_gem_context_create_ext {
> - __u32 ctx_id; /* output: id of new context*/
> + /** @ctx_id: Id of the created context (output) */
> + __u32 ctx_id;
> +
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
> + *
> + * Extensions may be appended to this structure and driver must check
> + * for those. See @extensions.
> + *
> + * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
> + *
> + * Created context will have single timeline.
> + */
> __u32 flags;
> #define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS (1u << 0)
> #define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE (1u << 1)
> #define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
> (-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
> +
> + /**
> + * @extensions: Zero-terminated chain of extensions.
> + *
> + * I915_CONTEXT_CREATE_EXT_SETPARAM:
> + * Context parameter to set or query during context creation.
> + * See struct drm_i915_gem_context_create_ext_setparam.
> + *
> + * I915_CONTEXT_CREATE_EXT_CLONE:
> + * This extension has been removed. On the off chance someone somewhere
> + * has attempted to use it, never re-use this extension number.
> + */
> __u64 extensions;
> +#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
> +#define I915_CONTEXT_CREATE_EXT_CLONE 1
> };
>
> +/**
> + * struct drm_i915_gem_context_param - Context parameter to set or query.
> + */
> struct drm_i915_gem_context_param {
> + /** @ctx_id: Context id */
> __u32 ctx_id;
> +
> + /** @size: Size of the parameter @value */
> __u32 size;
> +
> + /** @param: Parameter to set or query */
> __u64 param;
> #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
> /* I915_CONTEXT_PARAM_NO_ZEROMAP has been removed. On the off chance
> @@ -1973,6 +2078,7 @@ struct drm_i915_gem_context_param {
> #define I915_CONTEXT_PARAM_PROTECTED_CONTENT 0xd
> /* Must be kept compact -- no holes and well documented */
>
> + /** @value: Context parameter value to be set or queried */
> __u64 value;
> };
>
> @@ -2371,23 +2477,29 @@ struct i915_context_param_engines {
> struct i915_engine_class_instance engines[N__]; \
> } __attribute__((packed)) name__
>
> +/**
> + * struct drm_i915_gem_context_create_ext_setparam - Context parameter
> + * to set or query during context creation.
> + */
> struct drm_i915_gem_context_create_ext_setparam {
> -#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
> + /** @base: Extension link. See struct i915_user_extension. */
> struct i915_user_extension base;
> +
> + /**
> + * @param: Context parameter to set or query.
> + * See struct drm_i915_gem_context_param.
> + */
> struct drm_i915_gem_context_param param;
> };
>
> -/* This API has been removed. On the off chance someone somewhere has
> - * attempted to use it, never re-use this extension number.
> - */
> -#define I915_CONTEXT_CREATE_EXT_CLONE 1
> -
> struct drm_i915_gem_context_destroy {
> __u32 ctx_id;
> __u32 pad;
> };
>
> -/*
> +/**
> + * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
> + *
> * DRM_I915_GEM_VM_CREATE -
> *
> * Create a new virtual memory address space (ppGTT) for use within a context
> @@ -2397,20 +2509,23 @@ struct drm_i915_gem_context_destroy {
> * The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
> * returned in the outparam @id.
> *
> - * No flags are defined, with all bits reserved and must be zero.
> - *
> * An extension chain maybe provided, starting with @extensions, and terminated
> * by the @next_extension being 0. Currently, no extensions are defined.
> *
> * DRM_I915_GEM_VM_DESTROY -
> *
> - * Destroys a previously created VM id, specified in @id.
> + * Destroys a previously created VM id, specified in @vm_id.
> *
> * No extensions or flags are allowed currently, and so must be zero.
> */
> struct drm_i915_gem_vm_control {
> + /** @extensions: Zero-terminated chain of extensions. */
> __u64 extensions;
> +
> + /** @flags: reserved for future usage, currently MBZ */
> __u32 flags;
> +
> + /** @vm_id: Id of the VM created or to be destroyed */
> __u32 vm_id;
> };
Thanks for giving some much needed care to our docs.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> --
> 2.21.0.rc0.32.g243a4c7e27
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
@ 2022-06-24 19:38 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-06-24 19:38 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: chris.p.wilson, paulo.r.zanoni, intel-gfx, thomas.hellstrom,
dri-devel, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 10:49:35AM -0700, Niranjana Vishwanathapura wrote:
> Add some missing i915 upai documentation which the new
> i915 VM_BIND feature documentation will be refer to.
>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
> include/uapi/drm/i915_drm.h | 205 ++++++++++++++++++++++++++++--------
> 1 file changed, 160 insertions(+), 45 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index de49b68b4fc8..4afe95d8b98b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
>
> /* Must be kept compact -- no holes and well documented */
>
> -typedef struct drm_i915_getparam {
> +/**
> + * struct drm_i915_getparam - Driver parameter query structure.
> + */
> +struct drm_i915_getparam {
> + /** @param: Driver parameter to query. */
> __s32 param;
> - /*
> +
> + /**
> + * @value: Address of memory where queried value should be put.
> + *
> * WARNING: Using pointers instead of fixed-size u64 means we need to write
> * compat32 code. Don't repeat this mistake.
> */
> int __user *value;
> -} drm_i915_getparam_t;
> +};
> +
> +/**
> + * typedef drm_i915_getparam_t - Driver parameter query structure.
> + * See struct drm_i915_getparam.
> + */
> +typedef struct drm_i915_getparam drm_i915_getparam_t;
>
> /* Ioctl to set kernel params:
> */
> @@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
> __u64 rsvd2;
> };
>
> +/**
> + * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
> + * ioctl.
> + *
> + * The request will wait for input fence to signal before submission.
> + *
> + * The returned output fence will be signaled after the completion of the
> + * request.
> + */
> struct drm_i915_gem_exec_fence {
> - /**
> - * User's handle for a drm_syncobj to wait on or signal.
> - */
> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
> __u32 handle;
>
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_EXEC_FENCE_WAIT:
> + * Wait for the input fence before request submission.
> + *
> + * I915_EXEC_FENCE_SIGNAL:
> + * Return request completion fence as output
> + */
> + __u32 flags;
> #define I915_EXEC_FENCE_WAIT (1<<0)
> #define I915_EXEC_FENCE_SIGNAL (1<<1)
> #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
> - __u32 flags;
> };
>
> -/*
> - * See drm_i915_gem_execbuffer_ext_timeline_fences.
> - */
> -#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> -
> -/*
> +/**
> + * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
> + * for execbuf ioctl.
> + *
> * This structure describes an array of drm_syncobj and associated points for
> * timeline variants of drm_syncobj. It is invalid to append this structure to
> * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> */
> struct drm_i915_gem_execbuffer_ext_timeline_fences {
> +#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> + /** @base: Extension link. See struct i915_user_extension. */
> struct i915_user_extension base;
>
> /**
> - * Number of element in the handles_ptr & value_ptr arrays.
> + * @fence_count: Number of elements in the @handles_ptr & @value_ptr
> + * arrays.
> */
> __u64 fence_count;
>
> /**
> - * Pointer to an array of struct drm_i915_gem_exec_fence of length
> - * fence_count.
> + * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
> + * of length @fence_count.
> */
> __u64 handles_ptr;
>
> /**
> - * Pointer to an array of u64 values of length fence_count. Values
> - * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
> - * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
> + * @values_ptr: Pointer to an array of u64 values of length
> + * @fence_count.
> + * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
> + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> + * binary one.
> */
> __u64 values_ptr;
> };
>
> +/**
> + * struct drm_i915_gem_execbuffer2 - Structure for DRM_I915_GEM_EXECBUFFER2
> + * ioctl.
> + */
> struct drm_i915_gem_execbuffer2 {
> - /**
> - * List of gem_exec_object2 structs
> - */
> + /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
> __u64 buffers_ptr;
> +
> + /** @buffer_count: Number of elements in @buffers_ptr array */
> __u32 buffer_count;
>
> - /** Offset in the batchbuffer to start execution from. */
> + /**
> + * @batch_start_offset: Offset in the batchbuffer to start execution
> + * from.
> + */
> __u32 batch_start_offset;
> - /** Bytes used in batchbuffer from batch_start_offset */
> +
> + /**
> + * @batch_len: Length in bytes of the batch buffer, starting from the
> + * @batch_start_offset. If 0, length is assumed to be the batch buffer
> + * object size.
> + */
> __u32 batch_len;
> +
> + /** @DR1: deprecated */
> __u32 DR1;
> +
> + /** @DR4: deprecated */
> __u32 DR4;
> +
> + /** @num_cliprects: See @cliprects_ptr */
> __u32 num_cliprects;
> +
> /**
> - * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
> - * & I915_EXEC_USE_EXTENSIONS are not set.
> + * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
> + *
> + * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
> + * I915_EXEC_USE_EXTENSIONS flags are not set.
> *
> * If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
> - * of struct drm_i915_gem_exec_fence and num_cliprects is the length
> - * of the array.
> + * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
> + * array.
> *
> * If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
> - * single struct i915_user_extension and num_cliprects is 0.
> + * single &i915_user_extension and num_cliprects is 0.
> */
> __u64 cliprects_ptr;
> +
> + /** @flags: Execbuf flags */
> + __u64 flags;
> #define I915_EXEC_RING_MASK (0x3f)
> #define I915_EXEC_DEFAULT (0<<0)
> #define I915_EXEC_RENDER (1<<0)
> @@ -1326,10 +1382,6 @@ struct drm_i915_gem_execbuffer2 {
> #define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
> #define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6)
> #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
> - __u64 flags;
> - __u64 rsvd1; /* now used for context info */
> - __u64 rsvd2;
> -};
>
> /** Resets the SO write offset registers for transform feedback on gen7. */
> #define I915_EXEC_GEN7_SOL_RESET (1<<8)
> @@ -1432,9 +1484,23 @@ struct drm_i915_gem_execbuffer2 {
> * drm_i915_gem_execbuffer_ext enum.
> */
> #define I915_EXEC_USE_EXTENSIONS (1 << 21)
> -
> #define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
>
> + /** @rsvd1: Context id */
> + __u64 rsvd1;
> +
> + /**
> + * @rsvd2: in and out sync_file file descriptors.
> + *
> + * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
> + * lower 32 bits of this field will have the in sync_file fd (input).
> + *
> + * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
> + * field will have the out sync_file fd (output).
> + */
> + __u64 rsvd2;
> +};
> +
> #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
> #define i915_execbuffer2_set_context_id(eb2, context) \
> (eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
> @@ -1814,19 +1880,58 @@ struct drm_i915_gem_context_create {
> __u32 pad;
> };
>
> +/**
> + * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
I think a follow up patch which explains that despite that the uapi seems
to allow context properties to be mutable this isn't the case, and the
first execbuf call will freeze the context. Not super important for
vm_bind except that this means the execbuf will convert the proto context
into a real context and freeze the vm, which also locks down all the vm
related semantics we're adding. So might be good to add a paragraph here
explaining how this works (and then perhaps go through all the context
parameters and explain which can still be changed after the first
execbuf).
> + */
> struct drm_i915_gem_context_create_ext {
> - __u32 ctx_id; /* output: id of new context*/
> + /** @ctx_id: Id of the created context (output) */
> + __u32 ctx_id;
> +
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
> + *
> + * Extensions may be appended to this structure and driver must check
> + * for those. See @extensions.
> + *
> + * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
> + *
> + * Created context will have single timeline.
> + */
> __u32 flags;
> #define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS (1u << 0)
> #define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE (1u << 1)
> #define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
> (-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
> +
> + /**
> + * @extensions: Zero-terminated chain of extensions.
> + *
> + * I915_CONTEXT_CREATE_EXT_SETPARAM:
> + * Context parameter to set or query during context creation.
> + * See struct drm_i915_gem_context_create_ext_setparam.
> + *
> + * I915_CONTEXT_CREATE_EXT_CLONE:
> + * This extension has been removed. On the off chance someone somewhere
> + * has attempted to use it, never re-use this extension number.
> + */
> __u64 extensions;
> +#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
> +#define I915_CONTEXT_CREATE_EXT_CLONE 1
> };
>
> +/**
> + * struct drm_i915_gem_context_param - Context parameter to set or query.
> + */
> struct drm_i915_gem_context_param {
> + /** @ctx_id: Context id */
> __u32 ctx_id;
> +
> + /** @size: Size of the parameter @value */
> __u32 size;
> +
> + /** @param: Parameter to set or query */
> __u64 param;
> #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
> /* I915_CONTEXT_PARAM_NO_ZEROMAP has been removed. On the off chance
> @@ -1973,6 +2078,7 @@ struct drm_i915_gem_context_param {
> #define I915_CONTEXT_PARAM_PROTECTED_CONTENT 0xd
> /* Must be kept compact -- no holes and well documented */
>
> + /** @value: Context parameter value to be set or queried */
> __u64 value;
> };
>
> @@ -2371,23 +2477,29 @@ struct i915_context_param_engines {
> struct i915_engine_class_instance engines[N__]; \
> } __attribute__((packed)) name__
>
> +/**
> + * struct drm_i915_gem_context_create_ext_setparam - Context parameter
> + * to set or query during context creation.
> + */
> struct drm_i915_gem_context_create_ext_setparam {
> -#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
> + /** @base: Extension link. See struct i915_user_extension. */
> struct i915_user_extension base;
> +
> + /**
> + * @param: Context parameter to set or query.
> + * See struct drm_i915_gem_context_param.
> + */
> struct drm_i915_gem_context_param param;
> };
>
> -/* This API has been removed. On the off chance someone somewhere has
> - * attempted to use it, never re-use this extension number.
> - */
> -#define I915_CONTEXT_CREATE_EXT_CLONE 1
> -
> struct drm_i915_gem_context_destroy {
> __u32 ctx_id;
> __u32 pad;
> };
>
> -/*
> +/**
> + * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
> + *
> * DRM_I915_GEM_VM_CREATE -
> *
> * Create a new virtual memory address space (ppGTT) for use within a context
> @@ -2397,20 +2509,23 @@ struct drm_i915_gem_context_destroy {
> * The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
> * returned in the outparam @id.
> *
> - * No flags are defined, with all bits reserved and must be zero.
> - *
> * An extension chain maybe provided, starting with @extensions, and terminated
> * by the @next_extension being 0. Currently, no extensions are defined.
> *
> * DRM_I915_GEM_VM_DESTROY -
> *
> - * Destroys a previously created VM id, specified in @id.
> + * Destroys a previously created VM id, specified in @vm_id.
> *
> * No extensions or flags are allowed currently, and so must be zero.
> */
> struct drm_i915_gem_vm_control {
> + /** @extensions: Zero-terminated chain of extensions. */
> __u64 extensions;
> +
> + /** @flags: reserved for future usage, currently MBZ */
> __u32 flags;
> +
> + /** @vm_id: Id of the VM created or to be destroyed */
> __u32 vm_id;
> };
Thanks for giving some much needed care to our docs.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> --
> 2.21.0.rc0.32.g243a4c7e27
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
2022-06-24 19:38 ` [Intel-gfx] " Daniel Vetter
@ 2022-06-25 19:08 ` Niranjana Vishwanathapura
-1 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-25 19:08 UTC (permalink / raw)
To: Daniel Vetter
Cc: matthew.brost, paulo.r.zanoni, dri-devel, tvrtko.ursulin,
intel-gfx, lionel.g.landwerlin, thomas.hellstrom, oak.zeng,
chris.p.wilson, jason, daniel.vetter, christian.koenig,
matthew.auld
On Fri, Jun 24, 2022 at 09:38:20PM +0200, Daniel Vetter wrote:
>On Fri, Jun 24, 2022 at 10:49:35AM -0700, Niranjana Vishwanathapura wrote:
>> Add some missing i915 upai documentation which the new
>> i915 VM_BIND feature documentation will be refer to.
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>> ---
>> include/uapi/drm/i915_drm.h | 205 ++++++++++++++++++++++++++++--------
>> 1 file changed, 160 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index de49b68b4fc8..4afe95d8b98b 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
>>
>> /* Must be kept compact -- no holes and well documented */
>>
>> -typedef struct drm_i915_getparam {
>> +/**
>> + * struct drm_i915_getparam - Driver parameter query structure.
>> + */
>> +struct drm_i915_getparam {
>> + /** @param: Driver parameter to query. */
>> __s32 param;
>> - /*
>> +
>> + /**
>> + * @value: Address of memory where queried value should be put.
>> + *
>> * WARNING: Using pointers instead of fixed-size u64 means we need to write
>> * compat32 code. Don't repeat this mistake.
>> */
>> int __user *value;
>> -} drm_i915_getparam_t;
>> +};
>> +
>> +/**
>> + * typedef drm_i915_getparam_t - Driver parameter query structure.
>> + * See struct drm_i915_getparam.
>> + */
>> +typedef struct drm_i915_getparam drm_i915_getparam_t;
>>
>> /* Ioctl to set kernel params:
>> */
>> @@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
>> __u64 rsvd2;
>> };
>>
>> +/**
>> + * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
>> + * ioctl.
>> + *
>> + * The request will wait for input fence to signal before submission.
>> + *
>> + * The returned output fence will be signaled after the completion of the
>> + * request.
>> + */
>> struct drm_i915_gem_exec_fence {
>> - /**
>> - * User's handle for a drm_syncobj to wait on or signal.
>> - */
>> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>> __u32 handle;
>>
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_EXEC_FENCE_WAIT:
>> + * Wait for the input fence before request submission.
>> + *
>> + * I915_EXEC_FENCE_SIGNAL:
>> + * Return request completion fence as output
>> + */
>> + __u32 flags;
>> #define I915_EXEC_FENCE_WAIT (1<<0)
>> #define I915_EXEC_FENCE_SIGNAL (1<<1)
>> #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
>> - __u32 flags;
>> };
>>
>> -/*
>> - * See drm_i915_gem_execbuffer_ext_timeline_fences.
>> - */
>> -#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
>> -
>> -/*
>> +/**
>> + * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
>> + * for execbuf ioctl.
>> + *
>> * This structure describes an array of drm_syncobj and associated points for
>> * timeline variants of drm_syncobj. It is invalid to append this structure to
>> * the execbuf if I915_EXEC_FENCE_ARRAY is set.
>> */
>> struct drm_i915_gem_execbuffer_ext_timeline_fences {
>> +#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
>> + /** @base: Extension link. See struct i915_user_extension. */
>> struct i915_user_extension base;
>>
>> /**
>> - * Number of element in the handles_ptr & value_ptr arrays.
>> + * @fence_count: Number of elements in the @handles_ptr & @value_ptr
>> + * arrays.
>> */
>> __u64 fence_count;
>>
>> /**
>> - * Pointer to an array of struct drm_i915_gem_exec_fence of length
>> - * fence_count.
>> + * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
>> + * of length @fence_count.
>> */
>> __u64 handles_ptr;
>>
>> /**
>> - * Pointer to an array of u64 values of length fence_count. Values
>> - * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
>> - * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
>> + * @values_ptr: Pointer to an array of u64 values of length
>> + * @fence_count.
>> + * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
>> + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
>> + * binary one.
>> */
>> __u64 values_ptr;
>> };
>>
>> +/**
>> + * struct drm_i915_gem_execbuffer2 - Structure for DRM_I915_GEM_EXECBUFFER2
>> + * ioctl.
>> + */
>> struct drm_i915_gem_execbuffer2 {
>> - /**
>> - * List of gem_exec_object2 structs
>> - */
>> + /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
>> __u64 buffers_ptr;
>> +
>> + /** @buffer_count: Number of elements in @buffers_ptr array */
>> __u32 buffer_count;
>>
>> - /** Offset in the batchbuffer to start execution from. */
>> + /**
>> + * @batch_start_offset: Offset in the batchbuffer to start execution
>> + * from.
>> + */
>> __u32 batch_start_offset;
>> - /** Bytes used in batchbuffer from batch_start_offset */
>> +
>> + /**
>> + * @batch_len: Length in bytes of the batch buffer, starting from the
>> + * @batch_start_offset. If 0, length is assumed to be the batch buffer
>> + * object size.
>> + */
>> __u32 batch_len;
>> +
>> + /** @DR1: deprecated */
>> __u32 DR1;
>> +
>> + /** @DR4: deprecated */
>> __u32 DR4;
>> +
>> + /** @num_cliprects: See @cliprects_ptr */
>> __u32 num_cliprects;
>> +
>> /**
>> - * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
>> - * & I915_EXEC_USE_EXTENSIONS are not set.
>> + * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
>> + *
>> + * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
>> + * I915_EXEC_USE_EXTENSIONS flags are not set.
>> *
>> * If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
>> - * of struct drm_i915_gem_exec_fence and num_cliprects is the length
>> - * of the array.
>> + * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
>> + * array.
>> *
>> * If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
>> - * single struct i915_user_extension and num_cliprects is 0.
>> + * single &i915_user_extension and num_cliprects is 0.
>> */
>> __u64 cliprects_ptr;
>> +
>> + /** @flags: Execbuf flags */
>> + __u64 flags;
>> #define I915_EXEC_RING_MASK (0x3f)
>> #define I915_EXEC_DEFAULT (0<<0)
>> #define I915_EXEC_RENDER (1<<0)
>> @@ -1326,10 +1382,6 @@ struct drm_i915_gem_execbuffer2 {
>> #define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
>> #define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6)
>> #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
>> - __u64 flags;
>> - __u64 rsvd1; /* now used for context info */
>> - __u64 rsvd2;
>> -};
>>
>> /** Resets the SO write offset registers for transform feedback on gen7. */
>> #define I915_EXEC_GEN7_SOL_RESET (1<<8)
>> @@ -1432,9 +1484,23 @@ struct drm_i915_gem_execbuffer2 {
>> * drm_i915_gem_execbuffer_ext enum.
>> */
>> #define I915_EXEC_USE_EXTENSIONS (1 << 21)
>> -
>> #define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
>>
>> + /** @rsvd1: Context id */
>> + __u64 rsvd1;
>> +
>> + /**
>> + * @rsvd2: in and out sync_file file descriptors.
>> + *
>> + * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
>> + * lower 32 bits of this field will have the in sync_file fd (input).
>> + *
>> + * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
>> + * field will have the out sync_file fd (output).
>> + */
>> + __u64 rsvd2;
>> +};
>> +
>> #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
>> #define i915_execbuffer2_set_context_id(eb2, context) \
>> (eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
>> @@ -1814,19 +1880,58 @@ struct drm_i915_gem_context_create {
>> __u32 pad;
>> };
>>
>> +/**
>> + * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
>
>I think a follow up patch which explains that despite that the uapi seems
>to allow context properties to be mutable this isn't the case, and the
>first execbuf call will freeze the context. Not super important for
>vm_bind except that this means the execbuf will convert the proto context
>into a real context and freeze the vm, which also locks down all the vm
>related semantics we're adding. So might be good to add a paragraph here
>explaining how this works (and then perhaps go through all the context
>parameters and explain which can still be changed after the first
>execbuf).
I had to update this as compute use case of VM_BIND was making a reference
to this structure. But we no longer have that use case for VM_BIND, but
didn't want to revert this documentation :).
Yah, I think it will help immensely if we document the proto context
design here. Ok, a follow up patch sounds good.
>
>> + */
>> struct drm_i915_gem_context_create_ext {
>> - __u32 ctx_id; /* output: id of new context*/
>> + /** @ctx_id: Id of the created context (output) */
>> + __u32 ctx_id;
>> +
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
>> + *
>> + * Extensions may be appended to this structure and driver must check
>> + * for those. See @extensions.
>> + *
>> + * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
>> + *
>> + * Created context will have single timeline.
>> + */
>> __u32 flags;
>> #define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS (1u << 0)
>> #define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE (1u << 1)
>> #define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
>> (-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
>> +
>> + /**
>> + * @extensions: Zero-terminated chain of extensions.
>> + *
>> + * I915_CONTEXT_CREATE_EXT_SETPARAM:
>> + * Context parameter to set or query during context creation.
>> + * See struct drm_i915_gem_context_create_ext_setparam.
>> + *
>> + * I915_CONTEXT_CREATE_EXT_CLONE:
>> + * This extension has been removed. On the off chance someone somewhere
>> + * has attempted to use it, never re-use this extension number.
>> + */
>> __u64 extensions;
>> +#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
>> +#define I915_CONTEXT_CREATE_EXT_CLONE 1
>> };
>>
>> +/**
>> + * struct drm_i915_gem_context_param - Context parameter to set or query.
>> + */
>> struct drm_i915_gem_context_param {
>> + /** @ctx_id: Context id */
>> __u32 ctx_id;
>> +
>> + /** @size: Size of the parameter @value */
>> __u32 size;
>> +
>> + /** @param: Parameter to set or query */
>> __u64 param;
>> #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
>> /* I915_CONTEXT_PARAM_NO_ZEROMAP has been removed. On the off chance
>> @@ -1973,6 +2078,7 @@ struct drm_i915_gem_context_param {
>> #define I915_CONTEXT_PARAM_PROTECTED_CONTENT 0xd
>> /* Must be kept compact -- no holes and well documented */
>>
>> + /** @value: Context parameter value to be set or queried */
>> __u64 value;
>> };
>>
>> @@ -2371,23 +2477,29 @@ struct i915_context_param_engines {
>> struct i915_engine_class_instance engines[N__]; \
>> } __attribute__((packed)) name__
>>
>> +/**
>> + * struct drm_i915_gem_context_create_ext_setparam - Context parameter
>> + * to set or query during context creation.
>> + */
>> struct drm_i915_gem_context_create_ext_setparam {
>> -#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
>> + /** @base: Extension link. See struct i915_user_extension. */
>> struct i915_user_extension base;
>> +
>> + /**
>> + * @param: Context parameter to set or query.
>> + * See struct drm_i915_gem_context_param.
>> + */
>> struct drm_i915_gem_context_param param;
>> };
>>
>> -/* This API has been removed. On the off chance someone somewhere has
>> - * attempted to use it, never re-use this extension number.
>> - */
>> -#define I915_CONTEXT_CREATE_EXT_CLONE 1
>> -
>> struct drm_i915_gem_context_destroy {
>> __u32 ctx_id;
>> __u32 pad;
>> };
>>
>> -/*
>> +/**
>> + * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
>> + *
>> * DRM_I915_GEM_VM_CREATE -
>> *
>> * Create a new virtual memory address space (ppGTT) for use within a context
>> @@ -2397,20 +2509,23 @@ struct drm_i915_gem_context_destroy {
>> * The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
>> * returned in the outparam @id.
>> *
>> - * No flags are defined, with all bits reserved and must be zero.
>> - *
>> * An extension chain maybe provided, starting with @extensions, and terminated
>> * by the @next_extension being 0. Currently, no extensions are defined.
>> *
>> * DRM_I915_GEM_VM_DESTROY -
>> *
>> - * Destroys a previously created VM id, specified in @id.
>> + * Destroys a previously created VM id, specified in @vm_id.
>> *
>> * No extensions or flags are allowed currently, and so must be zero.
>> */
>> struct drm_i915_gem_vm_control {
>> + /** @extensions: Zero-terminated chain of extensions. */
>> __u64 extensions;
>> +
>> + /** @flags: reserved for future usage, currently MBZ */
>> __u32 flags;
>> +
>> + /** @vm_id: Id of the VM created or to be destroyed */
>> __u32 vm_id;
>> };
>
>Thanks for giving some much needed care to our docs.
>
>Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Thanks.
Niranjana
>>
>> --
>> 2.21.0.rc0.32.g243a4c7e27
>>
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
@ 2022-06-25 19:08 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-25 19:08 UTC (permalink / raw)
To: Daniel Vetter
Cc: paulo.r.zanoni, dri-devel, intel-gfx, thomas.hellstrom,
chris.p.wilson, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 09:38:20PM +0200, Daniel Vetter wrote:
>On Fri, Jun 24, 2022 at 10:49:35AM -0700, Niranjana Vishwanathapura wrote:
>> Add some missing i915 upai documentation which the new
>> i915 VM_BIND feature documentation will be refer to.
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>> ---
>> include/uapi/drm/i915_drm.h | 205 ++++++++++++++++++++++++++++--------
>> 1 file changed, 160 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index de49b68b4fc8..4afe95d8b98b 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
>>
>> /* Must be kept compact -- no holes and well documented */
>>
>> -typedef struct drm_i915_getparam {
>> +/**
>> + * struct drm_i915_getparam - Driver parameter query structure.
>> + */
>> +struct drm_i915_getparam {
>> + /** @param: Driver parameter to query. */
>> __s32 param;
>> - /*
>> +
>> + /**
>> + * @value: Address of memory where queried value should be put.
>> + *
>> * WARNING: Using pointers instead of fixed-size u64 means we need to write
>> * compat32 code. Don't repeat this mistake.
>> */
>> int __user *value;
>> -} drm_i915_getparam_t;
>> +};
>> +
>> +/**
>> + * typedef drm_i915_getparam_t - Driver parameter query structure.
>> + * See struct drm_i915_getparam.
>> + */
>> +typedef struct drm_i915_getparam drm_i915_getparam_t;
>>
>> /* Ioctl to set kernel params:
>> */
>> @@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
>> __u64 rsvd2;
>> };
>>
>> +/**
>> + * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
>> + * ioctl.
>> + *
>> + * The request will wait for input fence to signal before submission.
>> + *
>> + * The returned output fence will be signaled after the completion of the
>> + * request.
>> + */
>> struct drm_i915_gem_exec_fence {
>> - /**
>> - * User's handle for a drm_syncobj to wait on or signal.
>> - */
>> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>> __u32 handle;
>>
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_EXEC_FENCE_WAIT:
>> + * Wait for the input fence before request submission.
>> + *
>> + * I915_EXEC_FENCE_SIGNAL:
>> + * Return request completion fence as output
>> + */
>> + __u32 flags;
>> #define I915_EXEC_FENCE_WAIT (1<<0)
>> #define I915_EXEC_FENCE_SIGNAL (1<<1)
>> #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
>> - __u32 flags;
>> };
>>
>> -/*
>> - * See drm_i915_gem_execbuffer_ext_timeline_fences.
>> - */
>> -#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
>> -
>> -/*
>> +/**
>> + * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
>> + * for execbuf ioctl.
>> + *
>> * This structure describes an array of drm_syncobj and associated points for
>> * timeline variants of drm_syncobj. It is invalid to append this structure to
>> * the execbuf if I915_EXEC_FENCE_ARRAY is set.
>> */
>> struct drm_i915_gem_execbuffer_ext_timeline_fences {
>> +#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
>> + /** @base: Extension link. See struct i915_user_extension. */
>> struct i915_user_extension base;
>>
>> /**
>> - * Number of element in the handles_ptr & value_ptr arrays.
>> + * @fence_count: Number of elements in the @handles_ptr & @value_ptr
>> + * arrays.
>> */
>> __u64 fence_count;
>>
>> /**
>> - * Pointer to an array of struct drm_i915_gem_exec_fence of length
>> - * fence_count.
>> + * @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
>> + * of length @fence_count.
>> */
>> __u64 handles_ptr;
>>
>> /**
>> - * Pointer to an array of u64 values of length fence_count. Values
>> - * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
>> - * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
>> + * @values_ptr: Pointer to an array of u64 values of length
>> + * @fence_count.
>> + * Values must be 0 for a binary drm_syncobj. A Value of 0 for a
>> + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
>> + * binary one.
>> */
>> __u64 values_ptr;
>> };
>>
>> +/**
>> + * struct drm_i915_gem_execbuffer2 - Structure for DRM_I915_GEM_EXECBUFFER2
>> + * ioctl.
>> + */
>> struct drm_i915_gem_execbuffer2 {
>> - /**
>> - * List of gem_exec_object2 structs
>> - */
>> + /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
>> __u64 buffers_ptr;
>> +
>> + /** @buffer_count: Number of elements in @buffers_ptr array */
>> __u32 buffer_count;
>>
>> - /** Offset in the batchbuffer to start execution from. */
>> + /**
>> + * @batch_start_offset: Offset in the batchbuffer to start execution
>> + * from.
>> + */
>> __u32 batch_start_offset;
>> - /** Bytes used in batchbuffer from batch_start_offset */
>> +
>> + /**
>> + * @batch_len: Length in bytes of the batch buffer, starting from the
>> + * @batch_start_offset. If 0, length is assumed to be the batch buffer
>> + * object size.
>> + */
>> __u32 batch_len;
>> +
>> + /** @DR1: deprecated */
>> __u32 DR1;
>> +
>> + /** @DR4: deprecated */
>> __u32 DR4;
>> +
>> + /** @num_cliprects: See @cliprects_ptr */
>> __u32 num_cliprects;
>> +
>> /**
>> - * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
>> - * & I915_EXEC_USE_EXTENSIONS are not set.
>> + * @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
>> + *
>> + * It is invalid to use this field if I915_EXEC_FENCE_ARRAY or
>> + * I915_EXEC_USE_EXTENSIONS flags are not set.
>> *
>> * If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
>> - * of struct drm_i915_gem_exec_fence and num_cliprects is the length
>> - * of the array.
>> + * of &drm_i915_gem_exec_fence and @num_cliprects is the length of the
>> + * array.
>> *
>> * If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
>> - * single struct i915_user_extension and num_cliprects is 0.
>> + * single &i915_user_extension and num_cliprects is 0.
>> */
>> __u64 cliprects_ptr;
>> +
>> + /** @flags: Execbuf flags */
>> + __u64 flags;
>> #define I915_EXEC_RING_MASK (0x3f)
>> #define I915_EXEC_DEFAULT (0<<0)
>> #define I915_EXEC_RENDER (1<<0)
>> @@ -1326,10 +1382,6 @@ struct drm_i915_gem_execbuffer2 {
>> #define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */
>> #define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6)
>> #define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */
>> - __u64 flags;
>> - __u64 rsvd1; /* now used for context info */
>> - __u64 rsvd2;
>> -};
>>
>> /** Resets the SO write offset registers for transform feedback on gen7. */
>> #define I915_EXEC_GEN7_SOL_RESET (1<<8)
>> @@ -1432,9 +1484,23 @@ struct drm_i915_gem_execbuffer2 {
>> * drm_i915_gem_execbuffer_ext enum.
>> */
>> #define I915_EXEC_USE_EXTENSIONS (1 << 21)
>> -
>> #define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS << 1))
>>
>> + /** @rsvd1: Context id */
>> + __u64 rsvd1;
>> +
>> + /**
>> + * @rsvd2: in and out sync_file file descriptors.
>> + *
>> + * When I915_EXEC_FENCE_IN or I915_EXEC_FENCE_SUBMIT flag is set, the
>> + * lower 32 bits of this field will have the in sync_file fd (input).
>> + *
>> + * When I915_EXEC_FENCE_OUT flag is set, the upper 32 bits of this
>> + * field will have the out sync_file fd (output).
>> + */
>> + __u64 rsvd2;
>> +};
>> +
>> #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)
>> #define i915_execbuffer2_set_context_id(eb2, context) \
>> (eb2).rsvd1 = context & I915_EXEC_CONTEXT_ID_MASK
>> @@ -1814,19 +1880,58 @@ struct drm_i915_gem_context_create {
>> __u32 pad;
>> };
>>
>> +/**
>> + * struct drm_i915_gem_context_create_ext - Structure for creating contexts.
>
>I think a follow up patch which explains that despite that the uapi seems
>to allow context properties to be mutable this isn't the case, and the
>first execbuf call will freeze the context. Not super important for
>vm_bind except that this means the execbuf will convert the proto context
>into a real context and freeze the vm, which also locks down all the vm
>related semantics we're adding. So might be good to add a paragraph here
>explaining how this works (and then perhaps go through all the context
>parameters and explain which can still be changed after the first
>execbuf).
I had to update this as compute use case of VM_BIND was making a reference
to this structure. But we no longer have that use case for VM_BIND, but
didn't want to revert this documentation :).
Yah, I think it will help immensely if we document the proto context
design here. Ok, a follow up patch sounds good.
>
>> + */
>> struct drm_i915_gem_context_create_ext {
>> - __u32 ctx_id; /* output: id of new context*/
>> + /** @ctx_id: Id of the created context (output) */
>> + __u32 ctx_id;
>> +
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS:
>> + *
>> + * Extensions may be appended to this structure and driver must check
>> + * for those. See @extensions.
>> + *
>> + * I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE
>> + *
>> + * Created context will have single timeline.
>> + */
>> __u32 flags;
>> #define I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS (1u << 0)
>> #define I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE (1u << 1)
>> #define I915_CONTEXT_CREATE_FLAGS_UNKNOWN \
>> (-(I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE << 1))
>> +
>> + /**
>> + * @extensions: Zero-terminated chain of extensions.
>> + *
>> + * I915_CONTEXT_CREATE_EXT_SETPARAM:
>> + * Context parameter to set or query during context creation.
>> + * See struct drm_i915_gem_context_create_ext_setparam.
>> + *
>> + * I915_CONTEXT_CREATE_EXT_CLONE:
>> + * This extension has been removed. On the off chance someone somewhere
>> + * has attempted to use it, never re-use this extension number.
>> + */
>> __u64 extensions;
>> +#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
>> +#define I915_CONTEXT_CREATE_EXT_CLONE 1
>> };
>>
>> +/**
>> + * struct drm_i915_gem_context_param - Context parameter to set or query.
>> + */
>> struct drm_i915_gem_context_param {
>> + /** @ctx_id: Context id */
>> __u32 ctx_id;
>> +
>> + /** @size: Size of the parameter @value */
>> __u32 size;
>> +
>> + /** @param: Parameter to set or query */
>> __u64 param;
>> #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
>> /* I915_CONTEXT_PARAM_NO_ZEROMAP has been removed. On the off chance
>> @@ -1973,6 +2078,7 @@ struct drm_i915_gem_context_param {
>> #define I915_CONTEXT_PARAM_PROTECTED_CONTENT 0xd
>> /* Must be kept compact -- no holes and well documented */
>>
>> + /** @value: Context parameter value to be set or queried */
>> __u64 value;
>> };
>>
>> @@ -2371,23 +2477,29 @@ struct i915_context_param_engines {
>> struct i915_engine_class_instance engines[N__]; \
>> } __attribute__((packed)) name__
>>
>> +/**
>> + * struct drm_i915_gem_context_create_ext_setparam - Context parameter
>> + * to set or query during context creation.
>> + */
>> struct drm_i915_gem_context_create_ext_setparam {
>> -#define I915_CONTEXT_CREATE_EXT_SETPARAM 0
>> + /** @base: Extension link. See struct i915_user_extension. */
>> struct i915_user_extension base;
>> +
>> + /**
>> + * @param: Context parameter to set or query.
>> + * See struct drm_i915_gem_context_param.
>> + */
>> struct drm_i915_gem_context_param param;
>> };
>>
>> -/* This API has been removed. On the off chance someone somewhere has
>> - * attempted to use it, never re-use this extension number.
>> - */
>> -#define I915_CONTEXT_CREATE_EXT_CLONE 1
>> -
>> struct drm_i915_gem_context_destroy {
>> __u32 ctx_id;
>> __u32 pad;
>> };
>>
>> -/*
>> +/**
>> + * struct drm_i915_gem_vm_control - Structure to create or destroy VM.
>> + *
>> * DRM_I915_GEM_VM_CREATE -
>> *
>> * Create a new virtual memory address space (ppGTT) for use within a context
>> @@ -2397,20 +2509,23 @@ struct drm_i915_gem_context_destroy {
>> * The id of new VM (bound to the fd) for use with I915_CONTEXT_PARAM_VM is
>> * returned in the outparam @id.
>> *
>> - * No flags are defined, with all bits reserved and must be zero.
>> - *
>> * An extension chain maybe provided, starting with @extensions, and terminated
>> * by the @next_extension being 0. Currently, no extensions are defined.
>> *
>> * DRM_I915_GEM_VM_DESTROY -
>> *
>> - * Destroys a previously created VM id, specified in @id.
>> + * Destroys a previously created VM id, specified in @vm_id.
>> *
>> * No extensions or flags are allowed currently, and so must be zero.
>> */
>> struct drm_i915_gem_vm_control {
>> + /** @extensions: Zero-terminated chain of extensions. */
>> __u64 extensions;
>> +
>> + /** @flags: reserved for future usage, currently MBZ */
>> __u32 flags;
>> +
>> + /** @vm_id: Id of the VM created or to be destroyed */
>> __u32 vm_id;
>> };
>
>Thanks for giving some much needed care to our docs.
>
>Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Thanks.
Niranjana
>>
>> --
>> 2.21.0.rc0.32.g243a4c7e27
>>
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
-1 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: matthew.brost, paulo.r.zanoni, lionel.g.landwerlin,
tvrtko.ursulin, chris.p.wilson, thomas.hellstrom, oak.zeng,
matthew.auld, jason, daniel.vetter, christian.koenig
VM_BIND and related uapi definitions
v2: Reduce the scope to simple Mesa use case.
v3: Expand VM_UNBIND documentation and add
I915_GEM_VM_BIND/UNBIND_FENCE_VALID
and I915_GEM_VM_BIND_TLB_FLUSH flags.
v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
documentation for vm_bind/unbind.
v5: Remove TLB flush requirement on VM_UNBIND.
Add version support to stage implementation.
v6: Define and use drm_i915_gem_timeline_fence structure for
all timeline fences.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
---
Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
1 file changed, 286 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
new file mode 100644
index 000000000000..c784dc0c48b3
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.h
@@ -0,0 +1,286 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+/**
+ * DOC: I915_PARAM_HAS_VM_BIND
+ *
+ * VM_BIND feature availability.
+ * See typedef drm_i915_getparam_t param.
+ * bit[0]: If set, VM_BIND is supported, otherwise not.
+ * bits[8-15]: VM_BIND implementation version.
+ * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
+ * created previously with the VM_BIND call. i.e., i915 will not support
+ * splitting/merging of the mappings created with VM_BIND call (See
+ * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
+ */
+#define I915_PARAM_HAS_VM_BIND 57
+
+/**
+ * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
+ *
+ * Flag to opt-in for VM_BIND mode of binding during VM creation.
+ * See struct drm_i915_gem_vm_control flags.
+ *
+ * The older execbuf2 ioctl will not support VM_BIND mode of operation.
+ * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
+ * execlist (See struct drm_i915_gem_execbuffer3 for more details).
+ *
+ */
+#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND 0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_EXECBUFFER3 0x3f
+
+#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
+
+/**
+ * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
+ *
+ * The operation will wait for input fence to signal.
+ *
+ * The returned output fence will be signaled after the completion of the
+ * operation.
+ */
+struct drm_i915_gem_timeline_fence {
+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
+ __u32 handle;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_TIMELINE_FENCE_WAIT:
+ * Wait for the input fence before the operation.
+ *
+ * I915_TIMELINE_FENCE_SIGNAL:
+ * Return operation completion fence as output.
+ */
+ __u32 flags;
+#define I915_TIMELINE_FENCE_WAIT (1<<0)
+#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
+#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
+
+ /**
+ * @value: A point in the timeline.
+ * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
+ * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
+ * binary one.
+ */
+ __u64 value;
+};
+
+/**
+ * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
+ *
+ * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
+ * virtual address (VA) range to the section of an object that should be bound
+ * in the device page table of the specified address space (VM).
+ * The VA range specified must be unique (ie., not currently bound) and can
+ * be mapped to whole object or a section of the object (partial binding).
+ * Multiple VA mappings can be created to the same section of the object
+ * (aliasing).
+ *
+ * The @start, @offset and @length should be 4K page aligned. However the DG2
+ * and XEHPSDV has 64K page size for device local-memory and has compact page
+ * table. On those platforms, for binding device local-memory objects, the
+ * @start should be 2M aligned, @offset and @length should be 64K aligned.
+ * Also, on those platforms, error -ENOSPC will be returned if user tries to
+ * bind a device local-memory object and a system memory object in a single 2M
+ * section of VA range.
+ *
+ * Error code -EINVAL will be returned if @start, @offset and @length are not
+ * properly aligned. Error code of -ENOSPC will be returned if the VA range
+ * specified can't be reserved.
+ *
+ * The bind operation can get completed asynchronously and out of submission
+ * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
+ * signaled upon completion of bind operation.
+ */
+struct drm_i915_gem_vm_bind {
+ /** @vm_id: VM (address space) id to bind */
+ __u32 vm_id;
+
+ /** @handle: Object handle */
+ __u32 handle;
+
+ /** @start: Virtual Address start to bind */
+ __u64 start;
+
+ /** @offset: Offset in object to bind */
+ __u64 offset;
+
+ /** @length: Length of mapping to bind */
+ __u64 length;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_GEM_VM_BIND_FENCE_VALID:
+ * @fence is valid, needs bind completion notification.
+ *
+ * I915_GEM_VM_BIND_READONLY:
+ * Mapping is read-only.
+ *
+ * I915_GEM_VM_BIND_CAPTURE:
+ * Capture this mapping in the dump upon GPU error.
+ */
+ __u64 flags;
+#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
+#define I915_GEM_VM_BIND_READONLY (1 << 1)
+#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
+
+ /**
+ * @fence: Timeline fence for bind completion signaling.
+ *
+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
+ * is invalid, and an error will be returned.
+ */
+ struct drm_i915_gem_timeline_fence fence;
+
+ /** @extensions: 0-terminated chain of extensions */
+ __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
+ *
+ * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
+ * address (VA) range that should be unbound from the device page table of the
+ * specified address space (VM). VM_UNBIND will force unbind the specified
+ * range from device page table without waiting for any GPU job to complete.
+ * It is UMDs responsibility to ensure the mapping is no longer in use before
+ * calling VM_UNBIND.
+ *
+ * If the specified mapping is not found, the ioctl will simply return without
+ * any error.
+ *
+ * The unbind operation can get completed asynchronously and out of submission
+ * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
+ * signaled upon completion of unbind operation.
+ */
+struct drm_i915_gem_vm_unbind {
+ /** @vm_id: VM (address space) id to bind */
+ __u32 vm_id;
+
+ /** @rsvd: Reserved, MBZ */
+ __u32 rsvd;
+
+ /** @start: Virtual Address start to unbind */
+ __u64 start;
+
+ /** @length: Length of mapping to unbind */
+ __u64 length;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_GEM_VM_UNBIND_FENCE_VALID:
+ * @fence is valid, needs unbind completion notification.
+ */
+ __u64 flags;
+#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
+
+ /**
+ * @fence: Timeline fence for unbind completion signaling.
+ *
+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
+ * is invalid, and an error will be returned.
+ */
+ struct drm_i915_gem_timeline_fence fence;
+
+ /** @extensions: 0-terminated chain of extensions */
+ __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
+ * ioctl.
+ *
+ * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
+ * only works with this ioctl for submission.
+ * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
+ */
+struct drm_i915_gem_execbuffer3 {
+ /**
+ * @ctx_id: Context id
+ *
+ * Only contexts with user engine map are allowed.
+ */
+ __u32 ctx_id;
+
+ /**
+ * @engine_idx: Engine index
+ *
+ * An index in the user engine map of the context specified by @ctx_id.
+ */
+ __u32 engine_idx;
+
+ /** @rsvd1: Reserved, MBZ */
+ __u32 rsvd1;
+
+ /**
+ * @batch_count: Number of batches in @batch_address array.
+ *
+ * 0 is invalid. For parallel submission, it should be equal to the
+ * number of (parallel) engines involved in that submission.
+ */
+ __u32 batch_count;
+
+ /**
+ * @batch_address: Array of batch gpu virtual addresses.
+ *
+ * If @batch_count is 1, then it is the gpu virtual address of the
+ * batch buffer. If @batch_count > 1, then it is a pointer to an array
+ * of batch buffer gpu virtual addresses.
+ */
+ __u64 batch_address;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_EXEC3_SECURE:
+ * Request a privileged ("secure") batch buffer/s.
+ * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
+ */
+ __u64 flags;
+#define I915_EXEC3_SECURE (1 << 0)
+
+ /** @rsvd2: Reserved, MBZ */
+ __u32 rsvd2;
+
+ /** @fence_count: Number of fences in @timeline_fences array. */
+ __u32 fence_count;
+
+ /**
+ * @timeline_fences: Pointer to an array of timeline fences.
+ *
+ * Timeline fences are of format struct drm_i915_gem_timeline_fence.
+ */
+ __u64 timeline_fences;
+
+ /** @rsvd3: Reserved, MBZ */
+ __u64 rsvd3;
+
+ /** @extensions: Zero-terminated chain of extensions. */
+ __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
+ * private to the specified VM.
+ *
+ * See struct drm_i915_gem_create_ext.
+ */
+struct drm_i915_gem_create_ext_vm_private {
+#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
+ /** @base: Extension link. See struct i915_user_extension. */
+ struct i915_user_extension base;
+
+ /** @vm_id: Id of the VM to which the object is private */
+ __u32 vm_id;
+};
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
@ 2022-06-24 17:49 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-24 17:49 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: paulo.r.zanoni, chris.p.wilson, thomas.hellstrom, matthew.auld,
daniel.vetter, christian.koenig
VM_BIND and related uapi definitions
v2: Reduce the scope to simple Mesa use case.
v3: Expand VM_UNBIND documentation and add
I915_GEM_VM_BIND/UNBIND_FENCE_VALID
and I915_GEM_VM_BIND_TLB_FLUSH flags.
v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
documentation for vm_bind/unbind.
v5: Remove TLB flush requirement on VM_UNBIND.
Add version support to stage implementation.
v6: Define and use drm_i915_gem_timeline_fence structure for
all timeline fences.
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
---
Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
1 file changed, 286 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
new file mode 100644
index 000000000000..c784dc0c48b3
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.h
@@ -0,0 +1,286 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+/**
+ * DOC: I915_PARAM_HAS_VM_BIND
+ *
+ * VM_BIND feature availability.
+ * See typedef drm_i915_getparam_t param.
+ * bit[0]: If set, VM_BIND is supported, otherwise not.
+ * bits[8-15]: VM_BIND implementation version.
+ * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
+ * created previously with the VM_BIND call. i.e., i915 will not support
+ * splitting/merging of the mappings created with VM_BIND call (See
+ * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
+ */
+#define I915_PARAM_HAS_VM_BIND 57
+
+/**
+ * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
+ *
+ * Flag to opt-in for VM_BIND mode of binding during VM creation.
+ * See struct drm_i915_gem_vm_control flags.
+ *
+ * The older execbuf2 ioctl will not support VM_BIND mode of operation.
+ * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
+ * execlist (See struct drm_i915_gem_execbuffer3 for more details).
+ *
+ */
+#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND 0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_EXECBUFFER3 0x3f
+
+#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
+
+/**
+ * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
+ *
+ * The operation will wait for input fence to signal.
+ *
+ * The returned output fence will be signaled after the completion of the
+ * operation.
+ */
+struct drm_i915_gem_timeline_fence {
+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
+ __u32 handle;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_TIMELINE_FENCE_WAIT:
+ * Wait for the input fence before the operation.
+ *
+ * I915_TIMELINE_FENCE_SIGNAL:
+ * Return operation completion fence as output.
+ */
+ __u32 flags;
+#define I915_TIMELINE_FENCE_WAIT (1<<0)
+#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
+#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
+
+ /**
+ * @value: A point in the timeline.
+ * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
+ * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
+ * binary one.
+ */
+ __u64 value;
+};
+
+/**
+ * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
+ *
+ * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
+ * virtual address (VA) range to the section of an object that should be bound
+ * in the device page table of the specified address space (VM).
+ * The VA range specified must be unique (ie., not currently bound) and can
+ * be mapped to whole object or a section of the object (partial binding).
+ * Multiple VA mappings can be created to the same section of the object
+ * (aliasing).
+ *
+ * The @start, @offset and @length should be 4K page aligned. However the DG2
+ * and XEHPSDV has 64K page size for device local-memory and has compact page
+ * table. On those platforms, for binding device local-memory objects, the
+ * @start should be 2M aligned, @offset and @length should be 64K aligned.
+ * Also, on those platforms, error -ENOSPC will be returned if user tries to
+ * bind a device local-memory object and a system memory object in a single 2M
+ * section of VA range.
+ *
+ * Error code -EINVAL will be returned if @start, @offset and @length are not
+ * properly aligned. Error code of -ENOSPC will be returned if the VA range
+ * specified can't be reserved.
+ *
+ * The bind operation can get completed asynchronously and out of submission
+ * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
+ * signaled upon completion of bind operation.
+ */
+struct drm_i915_gem_vm_bind {
+ /** @vm_id: VM (address space) id to bind */
+ __u32 vm_id;
+
+ /** @handle: Object handle */
+ __u32 handle;
+
+ /** @start: Virtual Address start to bind */
+ __u64 start;
+
+ /** @offset: Offset in object to bind */
+ __u64 offset;
+
+ /** @length: Length of mapping to bind */
+ __u64 length;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_GEM_VM_BIND_FENCE_VALID:
+ * @fence is valid, needs bind completion notification.
+ *
+ * I915_GEM_VM_BIND_READONLY:
+ * Mapping is read-only.
+ *
+ * I915_GEM_VM_BIND_CAPTURE:
+ * Capture this mapping in the dump upon GPU error.
+ */
+ __u64 flags;
+#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
+#define I915_GEM_VM_BIND_READONLY (1 << 1)
+#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
+
+ /**
+ * @fence: Timeline fence for bind completion signaling.
+ *
+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
+ * is invalid, and an error will be returned.
+ */
+ struct drm_i915_gem_timeline_fence fence;
+
+ /** @extensions: 0-terminated chain of extensions */
+ __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
+ *
+ * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
+ * address (VA) range that should be unbound from the device page table of the
+ * specified address space (VM). VM_UNBIND will force unbind the specified
+ * range from device page table without waiting for any GPU job to complete.
+ * It is UMDs responsibility to ensure the mapping is no longer in use before
+ * calling VM_UNBIND.
+ *
+ * If the specified mapping is not found, the ioctl will simply return without
+ * any error.
+ *
+ * The unbind operation can get completed asynchronously and out of submission
+ * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
+ * signaled upon completion of unbind operation.
+ */
+struct drm_i915_gem_vm_unbind {
+ /** @vm_id: VM (address space) id to bind */
+ __u32 vm_id;
+
+ /** @rsvd: Reserved, MBZ */
+ __u32 rsvd;
+
+ /** @start: Virtual Address start to unbind */
+ __u64 start;
+
+ /** @length: Length of mapping to unbind */
+ __u64 length;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_GEM_VM_UNBIND_FENCE_VALID:
+ * @fence is valid, needs unbind completion notification.
+ */
+ __u64 flags;
+#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
+
+ /**
+ * @fence: Timeline fence for unbind completion signaling.
+ *
+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
+ * is invalid, and an error will be returned.
+ */
+ struct drm_i915_gem_timeline_fence fence;
+
+ /** @extensions: 0-terminated chain of extensions */
+ __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
+ * ioctl.
+ *
+ * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
+ * only works with this ioctl for submission.
+ * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
+ */
+struct drm_i915_gem_execbuffer3 {
+ /**
+ * @ctx_id: Context id
+ *
+ * Only contexts with user engine map are allowed.
+ */
+ __u32 ctx_id;
+
+ /**
+ * @engine_idx: Engine index
+ *
+ * An index in the user engine map of the context specified by @ctx_id.
+ */
+ __u32 engine_idx;
+
+ /** @rsvd1: Reserved, MBZ */
+ __u32 rsvd1;
+
+ /**
+ * @batch_count: Number of batches in @batch_address array.
+ *
+ * 0 is invalid. For parallel submission, it should be equal to the
+ * number of (parallel) engines involved in that submission.
+ */
+ __u32 batch_count;
+
+ /**
+ * @batch_address: Array of batch gpu virtual addresses.
+ *
+ * If @batch_count is 1, then it is the gpu virtual address of the
+ * batch buffer. If @batch_count > 1, then it is a pointer to an array
+ * of batch buffer gpu virtual addresses.
+ */
+ __u64 batch_address;
+
+ /**
+ * @flags: Supported flags are:
+ *
+ * I915_EXEC3_SECURE:
+ * Request a privileged ("secure") batch buffer/s.
+ * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
+ */
+ __u64 flags;
+#define I915_EXEC3_SECURE (1 << 0)
+
+ /** @rsvd2: Reserved, MBZ */
+ __u32 rsvd2;
+
+ /** @fence_count: Number of fences in @timeline_fences array. */
+ __u32 fence_count;
+
+ /**
+ * @timeline_fences: Pointer to an array of timeline fences.
+ *
+ * Timeline fences are of format struct drm_i915_gem_timeline_fence.
+ */
+ __u64 timeline_fences;
+
+ /** @rsvd3: Reserved, MBZ */
+ __u64 rsvd3;
+
+ /** @extensions: Zero-terminated chain of extensions. */
+ __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
+ * private to the specified VM.
+ *
+ * See struct drm_i915_gem_create_ext.
+ */
+struct drm_i915_gem_create_ext_vm_private {
+#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
+ /** @base: Extension link. See struct i915_user_extension. */
+ struct i915_user_extension base;
+
+ /** @vm_id: Id of the VM to which the object is private */
+ __u32 vm_id;
+};
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
(?)
@ 2022-06-24 20:07 ` Daniel Vetter
2022-06-25 19:02 ` Niranjana Vishwanathapura
-1 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2022-06-24 20:07 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: paulo.r.zanoni, intel-gfx, chris.p.wilson, thomas.hellstrom,
dri-devel, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 10:49:36AM -0700, Niranjana Vishwanathapura wrote:
> VM_BIND and related uapi definitions
>
> v2: Reduce the scope to simple Mesa use case.
> v3: Expand VM_UNBIND documentation and add
> I915_GEM_VM_BIND/UNBIND_FENCE_VALID
> and I915_GEM_VM_BIND_TLB_FLUSH flags.
> v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
> documentation for vm_bind/unbind.
> v5: Remove TLB flush requirement on VM_UNBIND.
> Add version support to stage implementation.
> v6: Define and use drm_i915_gem_timeline_fence structure for
> all timeline fences.
>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> ---
> Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
> 1 file changed, 286 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
>
> diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
> new file mode 100644
> index 000000000000..c784dc0c48b3
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> @@ -0,0 +1,286 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +/**
> + * DOC: I915_PARAM_HAS_VM_BIND
> + *
> + * VM_BIND feature availability.
> + * See typedef drm_i915_getparam_t param.
> + * bit[0]: If set, VM_BIND is supported, otherwise not.
> + * bits[8-15]: VM_BIND implementation version.
> + * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
> + * created previously with the VM_BIND call. i.e., i915 will not support
> + * splitting/merging of the mappings created with VM_BIND call (See
> + * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
I think this would read a lot clearer when you make it an itemized list
(or whatever it's called) like we do for atomic properties. Also it reads
a bit strange, so
* The following versions of VM_BIND have been defined already:
*
* 0: In VM_UNBIND calls the UMD must specify the exact mappings created
* previously with VM_BIND, the ioctl will not support unbinding
* multiple mappings or splitting them. Similar restriction applies to
* VM_BIND.
*
* 1: The restrictions on unbinding partial or multiple mappings is
* lifted, similarly binding will replace any mappings in the given range.
Pls check I got the formatting right, I didn't check with sphinx :-)
Bikeshed aside: This is a bit a strange encoding, usually we just use a
simple number and start with 1 (so that 0 is reserved for "doesn't exist")
and then have a #define for each version.
> + */
> +#define I915_PARAM_HAS_VM_BIND 57
> +
> +/**
> + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> + *
> + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> + * See struct drm_i915_gem_vm_control flags.
> + *
> + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
> + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
> + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
> + *
Empty line here.
> + */
> +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
> +
> +/* VM_BIND related ioctls */
> +#define DRM_I915_GEM_VM_BIND 0x3d
> +#define DRM_I915_GEM_VM_UNBIND 0x3e
> +#define DRM_I915_GEM_EXECBUFFER3 0x3f
> +
> +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
> +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
> +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
> +
> +/**
> + * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
> + *
> + * The operation will wait for input fence to signal.
> + *
> + * The returned output fence will be signaled after the completion of the
> + * operation.
> + */
> +struct drm_i915_gem_timeline_fence {
> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
> + __u32 handle;
> +
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_TIMELINE_FENCE_WAIT:
> + * Wait for the input fence before the operation.
> + *
> + * I915_TIMELINE_FENCE_SIGNAL:
> + * Return operation completion fence as output.
> + */
> + __u32 flags;
> +#define I915_TIMELINE_FENCE_WAIT (1<<0)
> +#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
> +#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
> +
> + /**
> + * @value: A point in the timeline.
> + * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
> + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> + * binary one.
> + */
> + __u64 value;
> +};
> +
> +/**
> + * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
> + *
> + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
> + * virtual address (VA) range to the section of an object that should be bound
> + * in the device page table of the specified address space (VM).
> + * The VA range specified must be unique (ie., not currently bound) and can
> + * be mapped to whole object or a section of the object (partial binding).
> + * Multiple VA mappings can be created to the same section of the object
> + * (aliasing).
> + *
> + * The @start, @offset and @length should be 4K page aligned. However the DG2
s/should/must/ here
> + * and XEHPSDV has 64K page size for device local-memory and has compact page
> + * table. On those platforms, for binding device local-memory objects, the
> + * @start should be 2M aligned, @offset and @length should be 64K aligned.
^^ this should is really a recommendation, right?
^^s/should/must on
the 2nd one here,
it's mandatory
> + * Also, on those platforms, error -ENOSPC will be returned if user tries to
> + * bind a device local-memory object and a system memory object in a single 2M
> + * section of VA range.
> + *
> + * Error code -EINVAL will be returned if @start, @offset and @length are not
> + * properly aligned. Error code of -ENOSPC will be returned if the VA range
> + * specified can't be reserved.
To check my understanding: ENOSPC can only happen on vm_bind ver 0, not on
vm_bind ver 1 right? We should make sure userspace doesn't rely on this in
some way.
> + *
> + * The bind operation can get completed asynchronously and out of submission
> + * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
"submission order" reads strange for something which is not an engine
(well was in the past). I think you wan here something like
VM_BIND IOCLT calls executed on different cpu threads concurrently are not
ordered. Furthermore parts of the VM_BIND operation can be done
asynchronously, if I915_GEM_VM_BIND_FENCE_VALID is set.
> + * signaled upon completion of bind operation.
> + */
> +struct drm_i915_gem_vm_bind {
> + /** @vm_id: VM (address space) id to bind */
> + __u32 vm_id;
> +
> + /** @handle: Object handle */
> + __u32 handle;
> +
> + /** @start: Virtual Address start to bind */
> + __u64 start;
> +
> + /** @offset: Offset in object to bind */
> + __u64 offset;
> +
> + /** @length: Length of mapping to bind */
> + __u64 length;
> +
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_GEM_VM_BIND_FENCE_VALID:
> + * @fence is valid, needs bind completion notification.
I guess we have a rule that FENCE_VALID can be set if and only if
FENCE_WAIT flag in the embedded fence structure is also set? Since we do
not put this into the extension chain like with execbuf it's always there,
but could be empty. ioctl input validation needs to be really careful here
and make sure these fields are all cleared if we don't have a fence.
> + *
> + * I915_GEM_VM_BIND_READONLY:
> + * Mapping is read-only.
> + *
> + * I915_GEM_VM_BIND_CAPTURE:
> + * Capture this mapping in the dump upon GPU error.
> + */
> + __u64 flags;
> +#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
> +#define I915_GEM_VM_BIND_READONLY (1 << 1)
> +#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
> +
> + /**
> + * @fence: Timeline fence for bind completion signaling.
> + *
> + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
> + * is invalid, and an error will be returned.
> + */
> + struct drm_i915_gem_timeline_fence fence;
> +
> + /** @extensions: 0-terminated chain of extensions */
> + __u64 extensions;
> +};
> +
> +/**
> + * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
> + *
> + * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
> + * address (VA) range that should be unbound from the device page table of the
> + * specified address space (VM). VM_UNBIND will force unbind the specified
> + * range from device page table without waiting for any GPU job to complete.
> + * It is UMDs responsibility to ensure the mapping is no longer in use before
> + * calling VM_UNBIND.
> + *
> + * If the specified mapping is not found, the ioctl will simply return without
> + * any error.
> + *
> + * The unbind operation can get completed asynchronously and out of submission
> + * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
> + * signaled upon completion of unbind operation.
Same here as above.
> + */
> +struct drm_i915_gem_vm_unbind {
> + /** @vm_id: VM (address space) id to bind */
> + __u32 vm_id;
> +
> + /** @rsvd: Reserved, MBZ */
> + __u32 rsvd;
> +
> + /** @start: Virtual Address start to unbind */
> + __u64 start;
> +
> + /** @length: Length of mapping to unbind */
> + __u64 length;
> +
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_GEM_VM_UNBIND_FENCE_VALID:
> + * @fence is valid, needs unbind completion notification.
> + */
> + __u64 flags;
> +#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
> +
> + /**
> + * @fence: Timeline fence for unbind completion signaling.
> + *
> + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
> + * is invalid, and an error will be returned.
> + */
> + struct drm_i915_gem_timeline_fence fence;
> +
> + /** @extensions: 0-terminated chain of extensions */
> + __u64 extensions;
> +};
> +
> +/**
> + * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
> + * ioctl.
> + *
> + * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
> + * only works with this ioctl for submission.
> + * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
> + */
> +struct drm_i915_gem_execbuffer3 {
> + /**
> + * @ctx_id: Context id
> + *
> + * Only contexts with user engine map are allowed.
> + */
> + __u32 ctx_id;
> +
> + /**
> + * @engine_idx: Engine index
> + *
> + * An index in the user engine map of the context specified by @ctx_id.
> + */
> + __u32 engine_idx;
> +
> + /** @rsvd1: Reserved, MBZ */
> + __u32 rsvd1;
> +
> + /**
> + * @batch_count: Number of batches in @batch_address array.
> + *
> + * 0 is invalid. For parallel submission, it should be equal to the
> + * number of (parallel) engines involved in that submission.
> + */
> + __u32 batch_count;
I think this is a leftover from the old parallel engine design, where
userspace stitched together the parallel batches? With the new design we
have in execbuf2 the batch_count is fixed at context creation time (that's
why document how that works is important), and so specifying this again in
execbuf3 could only lead to bugs and hence the field here should be
dropped.
> +
> + /**
> + * @batch_address: Array of batch gpu virtual addresses.
> + *
> + * If @batch_count is 1, then it is the gpu virtual address of the
> + * batch buffer. If @batch_count > 1, then it is a pointer to an array
> + * of batch buffer gpu virtual addresses.
We derive the number of batchbuffers a context needs from
ctx->parallel.num_children + 1. No need for userspace to supply this on
every execbuf, but the rules about the address still are the same.
> + */
> + __u64 batch_address;
> +
> + /**
> + * @flags: Supported flags are:
> + *
> + * I915_EXEC3_SECURE:
> + * Request a privileged ("secure") batch buffer/s.
> + * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
Do we actually still need this for any userspace? Iirc the only user was
SNA, which isn't a thing anymore since about gen9 or so. Or am I mistaken?
A quick git grep doesn't show up anything.
igt probably uses it for lots of fun stuff, might be time to remove it on
the execbuf2 side too on modern platforms since it's kinda a risky thing
to do (and flat out not a good idea on secure boot platforms even when
you're root).
> + */
> + __u64 flags;
> +#define I915_EXEC3_SECURE (1 << 0)
> +
> + /** @rsvd2: Reserved, MBZ */
> + __u32 rsvd2;
> +
> + /** @fence_count: Number of fences in @timeline_fences array. */
> + __u32 fence_count;
> +
> + /**
> + * @timeline_fences: Pointer to an array of timeline fences.
> + *
> + * Timeline fences are of format struct drm_i915_gem_timeline_fence.
> + */
> + __u64 timeline_fences;
> +
> + /** @rsvd3: Reserved, MBZ */
> + __u64 rsvd3;
> +
> + /** @extensions: Zero-terminated chain of extensions. */
Maybe add "See struct i915_users_extension" here and make it clear that
this is what this points to.
> + __u64 extensions;
> +};
> +
> +/**
> + * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
> + * private to the specified VM.
> + *
> + * See struct drm_i915_gem_create_ext.
> + */
> +struct drm_i915_gem_create_ext_vm_private {
> +#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
> + /** @base: Extension link. See struct i915_user_extension. */
> + struct i915_user_extension base;
> +
> + /** @vm_id: Id of the VM to which the object is private */
> + __u32 vm_id;
> +};
With the nits addressed:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> --
> 2.21.0.rc0.32.g243a4c7e27
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-24 20:07 ` Daniel Vetter
@ 2022-06-25 19:02 ` Niranjana Vishwanathapura
2022-06-26 1:08 ` Niranjana Vishwanathapura
2022-06-27 8:55 ` Daniel Vetter
0 siblings, 2 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-25 19:02 UTC (permalink / raw)
To: Daniel Vetter
Cc: paulo.r.zanoni, intel-gfx, chris.p.wilson, thomas.hellstrom,
dri-devel, daniel.vetter, christian.koenig, matthew.auld
On Fri, Jun 24, 2022 at 10:07:26PM +0200, Daniel Vetter wrote:
>On Fri, Jun 24, 2022 at 10:49:36AM -0700, Niranjana Vishwanathapura wrote:
>> VM_BIND and related uapi definitions
>>
>> v2: Reduce the scope to simple Mesa use case.
>> v3: Expand VM_UNBIND documentation and add
>> I915_GEM_VM_BIND/UNBIND_FENCE_VALID
>> and I915_GEM_VM_BIND_TLB_FLUSH flags.
>> v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
>> documentation for vm_bind/unbind.
>> v5: Remove TLB flush requirement on VM_UNBIND.
>> Add version support to stage implementation.
>> v6: Define and use drm_i915_gem_timeline_fence structure for
>> all timeline fences.
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> ---
>> Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
>> 1 file changed, 286 insertions(+)
>> create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
>>
>> diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
>> new file mode 100644
>> index 000000000000..c784dc0c48b3
>> --- /dev/null
>> +++ b/Documentation/gpu/rfc/i915_vm_bind.h
>> @@ -0,0 +1,286 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +/**
>> + * DOC: I915_PARAM_HAS_VM_BIND
>> + *
>> + * VM_BIND feature availability.
>> + * See typedef drm_i915_getparam_t param.
>> + * bit[0]: If set, VM_BIND is supported, otherwise not.
>> + * bits[8-15]: VM_BIND implementation version.
>> + * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
>> + * created previously with the VM_BIND call. i.e., i915 will not support
>> + * splitting/merging of the mappings created with VM_BIND call (See
>> + * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
>
>I think this would read a lot clearer when you make it an itemized list
>(or whatever it's called) like we do for atomic properties. Also it reads
>a bit strange, so
>
> * The following versions of VM_BIND have been defined already:
> *
> * 0: In VM_UNBIND calls the UMD must specify the exact mappings created
> * previously with VM_BIND, the ioctl will not support unbinding
> * multiple mappings or splitting them. Similar restriction applies to
> * VM_BIND.
> *
> * 1: The restrictions on unbinding partial or multiple mappings is
> * lifted, similarly binding will replace any mappings in the given range.
>
>Pls check I got the formatting right, I didn't check with sphinx :-)
>
>Bikeshed aside: This is a bit a strange encoding, usually we just use a
>simple number and start with 1 (so that 0 is reserved for "doesn't exist")
>and then have a #define for each version.
>
Thanks Daniel,
Ok, will eloborate and define the versions.
>> + */
>> +#define I915_PARAM_HAS_VM_BIND 57
>> +
>> +/**
>> + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
>> + *
>> + * Flag to opt-in for VM_BIND mode of binding during VM creation.
>> + * See struct drm_i915_gem_vm_control flags.
>> + *
>> + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
>> + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
>> + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
>> + *
>
>Empty line here.
Ok, will fix.
>
>> + */
>> +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
>> +
>> +/* VM_BIND related ioctls */
>> +#define DRM_I915_GEM_VM_BIND 0x3d
>> +#define DRM_I915_GEM_VM_UNBIND 0x3e
>> +#define DRM_I915_GEM_EXECBUFFER3 0x3f
>> +
>> +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
>> +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
>> +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
>> +
>> +/**
>> + * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
>> + *
>> + * The operation will wait for input fence to signal.
>> + *
>> + * The returned output fence will be signaled after the completion of the
>> + * operation.
>> + */
>> +struct drm_i915_gem_timeline_fence {
>> + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>> + __u32 handle;
>> +
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_TIMELINE_FENCE_WAIT:
>> + * Wait for the input fence before the operation.
>> + *
>> + * I915_TIMELINE_FENCE_SIGNAL:
>> + * Return operation completion fence as output.
>> + */
>> + __u32 flags;
>> +#define I915_TIMELINE_FENCE_WAIT (1<<0)
>> +#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
>> +#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
>> +
>> + /**
>> + * @value: A point in the timeline.
>> + * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
>> + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
>> + * binary one.
>> + */
>> + __u64 value;
>> +};
>> +
>> +/**
>> + * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
>> + *
>> + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
>> + * virtual address (VA) range to the section of an object that should be bound
>> + * in the device page table of the specified address space (VM).
>> + * The VA range specified must be unique (ie., not currently bound) and can
>> + * be mapped to whole object or a section of the object (partial binding).
>> + * Multiple VA mappings can be created to the same section of the object
>> + * (aliasing).
>> + *
>> + * The @start, @offset and @length should be 4K page aligned. However the DG2
> s/should/must/ here
>
ok
>> + * and XEHPSDV has 64K page size for device local-memory and has compact page
>> + * table. On those platforms, for binding device local-memory objects, the
>> + * @start should be 2M aligned, @offset and @length should be 64K aligned.
> ^^ this should is really a recommendation, right?
> ^^s/should/must on
> the 2nd one here,
> it's mandatory
>
No, it is not a recommendation, will change it to 'must'.
This was discussed in an earlier thread with Matt Auld. On 64K lmem platforms,
vm_bind will impose same restrictions as current i915 driver does for soft
pinning (ie., require 2M alignment for @start and reserve the whole 2M block).
>> + * Also, on those platforms, error -ENOSPC will be returned if user tries to
>> + * bind a device local-memory object and a system memory object in a single 2M
>> + * section of VA range.
>> + *
>> + * Error code -EINVAL will be returned if @start, @offset and @length are not
>> + * properly aligned. Error code of -ENOSPC will be returned if the VA range
>> + * specified can't be reserved.
>
>To check my understanding: ENOSPC can only happen on vm_bind ver 0, not on
>vm_bind ver 1 right? We should make sure userspace doesn't rely on this in
>some way.
>
Yah, will update.
>> + *
>> + * The bind operation can get completed asynchronously and out of submission
>> + * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
>
>"submission order" reads strange for something which is not an engine
>(well was in the past). I think you wan here something like
>
>VM_BIND IOCLT calls executed on different cpu threads concurrently are not
>ordered. Furthermore parts of the VM_BIND operation can be done
>asynchronously, if I915_GEM_VM_BIND_FENCE_VALID is set.
>
Hmm...current plan was to do vm_bind asynchronously when an object move is
underway, without regard to whether user has specified out fence or not.
I think this is not correct as there always should be an out fence if binding
is done asynchronously.
So, I will change the design as below.
1. When an out fence is specified by UMD,
1a. i915 will bind asynchronously if the object move is underway.
1b. i915 will bind synchronously otherwise.
Out fence is signaled after the binding is complete.
2. When out fence is not specified by UMD,
2a. i915 will bind synchronously if object is not being moved.
2b. i915 will return -EAGAIN if object is being moved.
Does that look good?
>> + * signaled upon completion of bind operation.
>> + */
>> +struct drm_i915_gem_vm_bind {
>> + /** @vm_id: VM (address space) id to bind */
>> + __u32 vm_id;
>> +
>> + /** @handle: Object handle */
>> + __u32 handle;
>> +
>> + /** @start: Virtual Address start to bind */
>> + __u64 start;
>> +
>> + /** @offset: Offset in object to bind */
>> + __u64 offset;
>> +
>> + /** @length: Length of mapping to bind */
>> + __u64 length;
>> +
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_GEM_VM_BIND_FENCE_VALID:
>> + * @fence is valid, needs bind completion notification.
>
>I guess we have a rule that FENCE_VALID can be set if and only if
>FENCE_WAIT flag in the embedded fence structure is also set? Since we do
>not put this into the extension chain like with execbuf it's always there,
>but could be empty. ioctl input validation needs to be really careful here
>and make sure these fields are all cleared if we don't have a fence.
>
Yah, given that now we have a generic timeline fence, we can check the
drm_i915_gem_timeline_fence.flags to see if valid fence is specified or
not. I will remove this FENCE_VALID flag here and in vm_unbind struct.
>> + *
>> + * I915_GEM_VM_BIND_READONLY:
>> + * Mapping is read-only.
>> + *
>> + * I915_GEM_VM_BIND_CAPTURE:
>> + * Capture this mapping in the dump upon GPU error.
>> + */
>> + __u64 flags;
>> +#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
>> +#define I915_GEM_VM_BIND_READONLY (1 << 1)
>> +#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
>> +
>> + /**
>> + * @fence: Timeline fence for bind completion signaling.
>> + *
>> + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>> + * is invalid, and an error will be returned.
>> + */
>> + struct drm_i915_gem_timeline_fence fence;
>> +
>> + /** @extensions: 0-terminated chain of extensions */
>> + __u64 extensions;
>> +};
>> +
>> +/**
>> + * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
>> + *
>> + * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
>> + * address (VA) range that should be unbound from the device page table of the
>> + * specified address space (VM). VM_UNBIND will force unbind the specified
>> + * range from device page table without waiting for any GPU job to complete.
>> + * It is UMDs responsibility to ensure the mapping is no longer in use before
>> + * calling VM_UNBIND.
>> + *
>> + * If the specified mapping is not found, the ioctl will simply return without
>> + * any error.
>> + *
>> + * The unbind operation can get completed asynchronously and out of submission
>> + * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
>> + * signaled upon completion of unbind operation.
>
>Same here as above.
>
>> + */
>> +struct drm_i915_gem_vm_unbind {
>> + /** @vm_id: VM (address space) id to bind */
>> + __u32 vm_id;
>> +
>> + /** @rsvd: Reserved, MBZ */
>> + __u32 rsvd;
>> +
>> + /** @start: Virtual Address start to unbind */
>> + __u64 start;
>> +
>> + /** @length: Length of mapping to unbind */
>> + __u64 length;
>> +
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_GEM_VM_UNBIND_FENCE_VALID:
>> + * @fence is valid, needs unbind completion notification.
>> + */
>> + __u64 flags;
>> +#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
>> +
>> + /**
>> + * @fence: Timeline fence for unbind completion signaling.
>> + *
>> + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>> + * is invalid, and an error will be returned.
>> + */
>> + struct drm_i915_gem_timeline_fence fence;
>> +
>> + /** @extensions: 0-terminated chain of extensions */
>> + __u64 extensions;
>> +};
>> +
>> +/**
>> + * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
>> + * ioctl.
>> + *
>> + * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
>> + * only works with this ioctl for submission.
>> + * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
>> + */
>> +struct drm_i915_gem_execbuffer3 {
>> + /**
>> + * @ctx_id: Context id
>> + *
>> + * Only contexts with user engine map are allowed.
>> + */
>> + __u32 ctx_id;
>> +
>> + /**
>> + * @engine_idx: Engine index
>> + *
>> + * An index in the user engine map of the context specified by @ctx_id.
>> + */
>> + __u32 engine_idx;
>> +
>> + /** @rsvd1: Reserved, MBZ */
>> + __u32 rsvd1;
>> +
>> + /**
>> + * @batch_count: Number of batches in @batch_address array.
>> + *
>> + * 0 is invalid. For parallel submission, it should be equal to the
>> + * number of (parallel) engines involved in that submission.
>> + */
>> + __u32 batch_count;
>
>I think this is a leftover from the old parallel engine design, where
>userspace stitched together the parallel batches? With the new design we
>have in execbuf2 the batch_count is fixed at context creation time (that's
>why document how that works is important), and so specifying this again in
>execbuf3 could only lead to bugs and hence the field here should be
>dropped.
Ok, will remove.
>
>> +
>> + /**
>> + * @batch_address: Array of batch gpu virtual addresses.
>> + *
>> + * If @batch_count is 1, then it is the gpu virtual address of the
>> + * batch buffer. If @batch_count > 1, then it is a pointer to an array
>> + * of batch buffer gpu virtual addresses.
>
>We derive the number of batchbuffers a context needs from
>ctx->parallel.num_children + 1. No need for userspace to supply this on
>every execbuf, but the rules about the address still are the same.
>
My only worry is if the number of batch addresses provided is not equal
to 'ctx->parallel.num_children + 1', then we can get into harder to debug
GPU hangs (where as with batch_count, we can return an error). Besides,
without explicit batch_count here, treating this field differently when
one batch case vs multiple batch case makes the uapi look bit complicated.
But, I guess, user can error up and get into harder to debug GPU hangs
even with batch_count. I think removing the batch_count and properly
documenting this batch_address field here should be good. Will update.
>> + */
>> + __u64 batch_address;
>> +
>> + /**
>> + * @flags: Supported flags are:
>> + *
>> + * I915_EXEC3_SECURE:
>> + * Request a privileged ("secure") batch buffer/s.
>> + * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
>
>Do we actually still need this for any userspace? Iirc the only user was
>SNA, which isn't a thing anymore since about gen9 or so. Or am I mistaken?
>A quick git grep doesn't show up anything.
>
>igt probably uses it for lots of fun stuff, might be time to remove it on
>the execbuf2 side too on modern platforms since it's kinda a risky thing
>to do (and flat out not a good idea on secure boot platforms even when
>you're root).
>
Yah, looks like it is only used on very old platforms, will remove this
flag.
>> + */
>> + __u64 flags;
>> +#define I915_EXEC3_SECURE (1 << 0)
>> +
>> + /** @rsvd2: Reserved, MBZ */
>> + __u32 rsvd2;
>> +
>> + /** @fence_count: Number of fences in @timeline_fences array. */
>> + __u32 fence_count;
>> +
>> + /**
>> + * @timeline_fences: Pointer to an array of timeline fences.
>> + *
>> + * Timeline fences are of format struct drm_i915_gem_timeline_fence.
>> + */
>> + __u64 timeline_fences;
>> +
>> + /** @rsvd3: Reserved, MBZ */
>> + __u64 rsvd3;
>> +
>> + /** @extensions: Zero-terminated chain of extensions. */
>
>Maybe add "See struct i915_users_extension" here and make it clear that
>this is what this points to.
>
Ok.
>> + __u64 extensions;
>> +};
>> +
>> +/**
>> + * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
>> + * private to the specified VM.
>> + *
>> + * See struct drm_i915_gem_create_ext.
>> + */
>> +struct drm_i915_gem_create_ext_vm_private {
>> +#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
>> + /** @base: Extension link. See struct i915_user_extension. */
>> + struct i915_user_extension base;
>> +
>> + /** @vm_id: Id of the VM to which the object is private */
>> + __u32 vm_id;
>> +};
>
>With the nits addressed:
>
>Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks,
Niranjana
>
>> --
>> 2.21.0.rc0.32.g243a4c7e27
>>
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-25 19:02 ` Niranjana Vishwanathapura
@ 2022-06-26 1:08 ` Niranjana Vishwanathapura
2022-06-27 9:07 ` Daniel Vetter
2022-06-27 8:55 ` Daniel Vetter
1 sibling, 1 reply; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-26 1:08 UTC (permalink / raw)
To: Daniel Vetter
Cc: paulo.r.zanoni, intel-gfx, dri-devel, thomas.hellstrom,
chris.p.wilson, daniel.vetter, christian.koenig, matthew.auld
On Sat, Jun 25, 2022 at 12:02:19PM -0700, Niranjana Vishwanathapura wrote:
>On Fri, Jun 24, 2022 at 10:07:26PM +0200, Daniel Vetter wrote:
>>On Fri, Jun 24, 2022 at 10:49:36AM -0700, Niranjana Vishwanathapura wrote:
>>>VM_BIND and related uapi definitions
>>>
>>>v2: Reduce the scope to simple Mesa use case.
>>>v3: Expand VM_UNBIND documentation and add
>>> I915_GEM_VM_BIND/UNBIND_FENCE_VALID
>>> and I915_GEM_VM_BIND_TLB_FLUSH flags.
>>>v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
>>> documentation for vm_bind/unbind.
>>>v5: Remove TLB flush requirement on VM_UNBIND.
>>> Add version support to stage implementation.
>>>v6: Define and use drm_i915_gem_timeline_fence structure for
>>> all timeline fences.
>>>
>>>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>>>---
>>> Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
>>> 1 file changed, 286 insertions(+)
>>> create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
>>>
>>>diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
>>>new file mode 100644
>>>index 000000000000..c784dc0c48b3
>>>--- /dev/null
>>>+++ b/Documentation/gpu/rfc/i915_vm_bind.h
>>>@@ -0,0 +1,286 @@
>>>+/* SPDX-License-Identifier: MIT */
>>>+/*
>>>+ * Copyright © 2022 Intel Corporation
>>>+ */
>>>+
>>>+/**
>>>+ * DOC: I915_PARAM_HAS_VM_BIND
>>>+ *
>>>+ * VM_BIND feature availability.
>>>+ * See typedef drm_i915_getparam_t param.
>>>+ * bit[0]: If set, VM_BIND is supported, otherwise not.
>>>+ * bits[8-15]: VM_BIND implementation version.
>>>+ * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
>>>+ * created previously with the VM_BIND call. i.e., i915 will not support
>>>+ * splitting/merging of the mappings created with VM_BIND call (See
>>>+ * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
>>
>>I think this would read a lot clearer when you make it an itemized list
>>(or whatever it's called) like we do for atomic properties. Also it reads
>>a bit strange, so
>>
>>* The following versions of VM_BIND have been defined already:
>>*
>>* 0: In VM_UNBIND calls the UMD must specify the exact mappings created
>>* previously with VM_BIND, the ioctl will not support unbinding
>>* multiple mappings or splitting them. Similar restriction applies to
>>* VM_BIND.
>>*
>>* 1: The restrictions on unbinding partial or multiple mappings is
>>* lifted, similarly binding will replace any mappings in the given range.
>>
>>Pls check I got the formatting right, I didn't check with sphinx :-)
>>
>>Bikeshed aside: This is a bit a strange encoding, usually we just use a
>>simple number and start with 1 (so that 0 is reserved for "doesn't exist")
>>and then have a #define for each version.
>>
>
>Thanks Daniel,
>
>Ok, will eloborate and define the versions.
Will rename this field to I915_PARAM_VM_BIND_VERSION with 0 meaning no vm_bind
support.
>
>>>+ */
>>>+#define I915_PARAM_HAS_VM_BIND 57
>>>+
>>>+/**
>>>+ * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
>>>+ *
>>>+ * Flag to opt-in for VM_BIND mode of binding during VM creation.
>>>+ * See struct drm_i915_gem_vm_control flags.
>>>+ *
>>>+ * The older execbuf2 ioctl will not support VM_BIND mode of operation.
>>>+ * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
>>>+ * execlist (See struct drm_i915_gem_execbuffer3 for more details).
>>>+ *
>>
>>Empty line here.
>
>Ok, will fix.
>
>>
>>>+ */
>>>+#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
>>>+
>>>+/* VM_BIND related ioctls */
>>>+#define DRM_I915_GEM_VM_BIND 0x3d
>>>+#define DRM_I915_GEM_VM_UNBIND 0x3e
>>>+#define DRM_I915_GEM_EXECBUFFER3 0x3f
>>>+
>>>+#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
>>>+#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
>>>+#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
>>>+
>>>+/**
>>>+ * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
>>>+ *
>>>+ * The operation will wait for input fence to signal.
>>>+ *
>>>+ * The returned output fence will be signaled after the completion of the
>>>+ * operation.
>>>+ */
>>>+struct drm_i915_gem_timeline_fence {
>>>+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>>>+ __u32 handle;
>>>+
>>>+ /**
>>>+ * @flags: Supported flags are:
>>>+ *
>>>+ * I915_TIMELINE_FENCE_WAIT:
>>>+ * Wait for the input fence before the operation.
>>>+ *
>>>+ * I915_TIMELINE_FENCE_SIGNAL:
>>>+ * Return operation completion fence as output.
>>>+ */
>>>+ __u32 flags;
>>>+#define I915_TIMELINE_FENCE_WAIT (1<<0)
>>>+#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
>>>+#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
>>>+
>>>+ /**
>>>+ * @value: A point in the timeline.
>>>+ * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
>>>+ * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
>>>+ * binary one.
>>>+ */
>>>+ __u64 value;
>>>+};
>>>+
>>>+/**
>>>+ * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
>>>+ *
>>>+ * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
>>>+ * virtual address (VA) range to the section of an object that should be bound
>>>+ * in the device page table of the specified address space (VM).
>>>+ * The VA range specified must be unique (ie., not currently bound) and can
>>>+ * be mapped to whole object or a section of the object (partial binding).
>>>+ * Multiple VA mappings can be created to the same section of the object
>>>+ * (aliasing).
>>>+ *
>>>+ * The @start, @offset and @length should be 4K page aligned. However the DG2
>> s/should/must/ here
>>
>
>ok
>
>>>+ * and XEHPSDV has 64K page size for device local-memory and has compact page
>>>+ * table. On those platforms, for binding device local-memory objects, the
>>>+ * @start should be 2M aligned, @offset and @length should be 64K aligned.
>> ^^ this should is really a recommendation, right?
>> ^^s/should/must on
>> the 2nd one here,
>> it's mandatory
>>
>
>No, it is not a recommendation, will change it to 'must'.
>This was discussed in an earlier thread with Matt Auld. On 64K lmem platforms,
>vm_bind will impose same restrictions as current i915 driver does for soft
>pinning (ie., require 2M alignment for @start and reserve the whole 2M block).
>
>>>+ * Also, on those platforms, error -ENOSPC will be returned if user tries to
>>>+ * bind a device local-memory object and a system memory object in a single 2M
>>>+ * section of VA range.
>>>+ *
>>>+ * Error code -EINVAL will be returned if @start, @offset and @length are not
>>>+ * properly aligned. Error code of -ENOSPC will be returned if the VA range
>>>+ * specified can't be reserved.
>>
>>To check my understanding: ENOSPC can only happen on vm_bind ver 0, not on
>>vm_bind ver 1 right? We should make sure userspace doesn't rely on this in
>>some way.
>>
>
>Yah, will update.
>
>>>+ *
>>>+ * The bind operation can get completed asynchronously and out of submission
>>>+ * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
>>
>>"submission order" reads strange for something which is not an engine
>>(well was in the past). I think you wan here something like
>>
>>VM_BIND IOCLT calls executed on different cpu threads concurrently are not
>>ordered. Furthermore parts of the VM_BIND operation can be done
>>asynchronously, if I915_GEM_VM_BIND_FENCE_VALID is set.
>>
>
>Hmm...current plan was to do vm_bind asynchronously when an object move is
>underway, without regard to whether user has specified out fence or not.
>I think this is not correct as there always should be an out fence if binding
>is done asynchronously.
>
>So, I will change the design as below.
>1. When an out fence is specified by UMD,
> 1a. i915 will bind asynchronously if the object move is underway.
> 1b. i915 will bind synchronously otherwise.
> Out fence is signaled after the binding is complete.
>2. When out fence is not specified by UMD,
> 2a. i915 will bind synchronously if object is not being moved.
> 2b. i915 will return -EAGAIN if object is being moved.
>
Ok, will keep it simple by making the ioctl always synchronous if
out fence is not specified (instead of returning -EAGAIN).
ie., if binding is scheduled behind an object move, then vm_bind ioctl
will wait until bind is complete before returning if the out fence is
not specified.
Niranjana
>Does that look good?
>
>>>+ * signaled upon completion of bind operation.
>>>+ */
>>>+struct drm_i915_gem_vm_bind {
>>>+ /** @vm_id: VM (address space) id to bind */
>>>+ __u32 vm_id;
>>>+
>>>+ /** @handle: Object handle */
>>>+ __u32 handle;
>>>+
>>>+ /** @start: Virtual Address start to bind */
>>>+ __u64 start;
>>>+
>>>+ /** @offset: Offset in object to bind */
>>>+ __u64 offset;
>>>+
>>>+ /** @length: Length of mapping to bind */
>>>+ __u64 length;
>>>+
>>>+ /**
>>>+ * @flags: Supported flags are:
>>>+ *
>>>+ * I915_GEM_VM_BIND_FENCE_VALID:
>>>+ * @fence is valid, needs bind completion notification.
>>
>>I guess we have a rule that FENCE_VALID can be set if and only if
>>FENCE_WAIT flag in the embedded fence structure is also set? Since we do
>>not put this into the extension chain like with execbuf it's always there,
>>but could be empty. ioctl input validation needs to be really careful here
>>and make sure these fields are all cleared if we don't have a fence.
>>
>
>Yah, given that now we have a generic timeline fence, we can check the
>drm_i915_gem_timeline_fence.flags to see if valid fence is specified or
>not. I will remove this FENCE_VALID flag here and in vm_unbind struct.
>
>>>+ *
>>>+ * I915_GEM_VM_BIND_READONLY:
>>>+ * Mapping is read-only.
>>>+ *
>>>+ * I915_GEM_VM_BIND_CAPTURE:
>>>+ * Capture this mapping in the dump upon GPU error.
>>>+ */
>>>+ __u64 flags;
>>>+#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
>>>+#define I915_GEM_VM_BIND_READONLY (1 << 1)
>>>+#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
>>>+
>>>+ /**
>>>+ * @fence: Timeline fence for bind completion signaling.
>>>+ *
>>>+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>>>+ * is invalid, and an error will be returned.
>>>+ */
>>>+ struct drm_i915_gem_timeline_fence fence;
>>>+
>>>+ /** @extensions: 0-terminated chain of extensions */
>>>+ __u64 extensions;
>>>+};
>>>+
>>>+/**
>>>+ * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
>>>+ *
>>>+ * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
>>>+ * address (VA) range that should be unbound from the device page table of the
>>>+ * specified address space (VM). VM_UNBIND will force unbind the specified
>>>+ * range from device page table without waiting for any GPU job to complete.
>>>+ * It is UMDs responsibility to ensure the mapping is no longer in use before
>>>+ * calling VM_UNBIND.
>>>+ *
>>>+ * If the specified mapping is not found, the ioctl will simply return without
>>>+ * any error.
>>>+ *
>>>+ * The unbind operation can get completed asynchronously and out of submission
>>>+ * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
>>>+ * signaled upon completion of unbind operation.
>>
>>Same here as above.
>>
>>>+ */
>>>+struct drm_i915_gem_vm_unbind {
>>>+ /** @vm_id: VM (address space) id to bind */
>>>+ __u32 vm_id;
>>>+
>>>+ /** @rsvd: Reserved, MBZ */
>>>+ __u32 rsvd;
>>>+
>>>+ /** @start: Virtual Address start to unbind */
>>>+ __u64 start;
>>>+
>>>+ /** @length: Length of mapping to unbind */
>>>+ __u64 length;
>>>+
>>>+ /**
>>>+ * @flags: Supported flags are:
>>>+ *
>>>+ * I915_GEM_VM_UNBIND_FENCE_VALID:
>>>+ * @fence is valid, needs unbind completion notification.
>>>+ */
>>>+ __u64 flags;
>>>+#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
>>>+
>>>+ /**
>>>+ * @fence: Timeline fence for unbind completion signaling.
>>>+ *
>>>+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>>>+ * is invalid, and an error will be returned.
>>>+ */
>>>+ struct drm_i915_gem_timeline_fence fence;
>>>+
>>>+ /** @extensions: 0-terminated chain of extensions */
>>>+ __u64 extensions;
>>>+};
>>>+
>>>+/**
>>>+ * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
>>>+ * ioctl.
>>>+ *
>>>+ * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
>>>+ * only works with this ioctl for submission.
>>>+ * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
>>>+ */
>>>+struct drm_i915_gem_execbuffer3 {
>>>+ /**
>>>+ * @ctx_id: Context id
>>>+ *
>>>+ * Only contexts with user engine map are allowed.
>>>+ */
>>>+ __u32 ctx_id;
>>>+
>>>+ /**
>>>+ * @engine_idx: Engine index
>>>+ *
>>>+ * An index in the user engine map of the context specified by @ctx_id.
>>>+ */
>>>+ __u32 engine_idx;
>>>+
>>>+ /** @rsvd1: Reserved, MBZ */
>>>+ __u32 rsvd1;
>>>+
>>>+ /**
>>>+ * @batch_count: Number of batches in @batch_address array.
>>>+ *
>>>+ * 0 is invalid. For parallel submission, it should be equal to the
>>>+ * number of (parallel) engines involved in that submission.
>>>+ */
>>>+ __u32 batch_count;
>>
>>I think this is a leftover from the old parallel engine design, where
>>userspace stitched together the parallel batches? With the new design we
>>have in execbuf2 the batch_count is fixed at context creation time (that's
>>why document how that works is important), and so specifying this again in
>>execbuf3 could only lead to bugs and hence the field here should be
>>dropped.
>
>Ok, will remove.
>
>>
>>>+
>>>+ /**
>>>+ * @batch_address: Array of batch gpu virtual addresses.
>>>+ *
>>>+ * If @batch_count is 1, then it is the gpu virtual address of the
>>>+ * batch buffer. If @batch_count > 1, then it is a pointer to an array
>>>+ * of batch buffer gpu virtual addresses.
>>
>>We derive the number of batchbuffers a context needs from
>>ctx->parallel.num_children + 1. No need for userspace to supply this on
>>every execbuf, but the rules about the address still are the same.
>>
>
>My only worry is if the number of batch addresses provided is not equal
>to 'ctx->parallel.num_children + 1', then we can get into harder to debug
>GPU hangs (where as with batch_count, we can return an error). Besides,
>without explicit batch_count here, treating this field differently when
>one batch case vs multiple batch case makes the uapi look bit complicated.
>
>But, I guess, user can error up and get into harder to debug GPU hangs
>even with batch_count. I think removing the batch_count and properly
>documenting this batch_address field here should be good. Will update.
>
>>>+ */
>>>+ __u64 batch_address;
>>>+
>>>+ /**
>>>+ * @flags: Supported flags are:
>>>+ *
>>>+ * I915_EXEC3_SECURE:
>>>+ * Request a privileged ("secure") batch buffer/s.
>>>+ * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
>>
>>Do we actually still need this for any userspace? Iirc the only user was
>>SNA, which isn't a thing anymore since about gen9 or so. Or am I mistaken?
>>A quick git grep doesn't show up anything.
>>
>>igt probably uses it for lots of fun stuff, might be time to remove it on
>>the execbuf2 side too on modern platforms since it's kinda a risky thing
>>to do (and flat out not a good idea on secure boot platforms even when
>>you're root).
>>
>
>Yah, looks like it is only used on very old platforms, will remove this
>flag.
>
>>>+ */
>>>+ __u64 flags;
>>>+#define I915_EXEC3_SECURE (1 << 0)
>>>+
>>>+ /** @rsvd2: Reserved, MBZ */
>>>+ __u32 rsvd2;
>>>+
>>>+ /** @fence_count: Number of fences in @timeline_fences array. */
>>>+ __u32 fence_count;
>>>+
>>>+ /**
>>>+ * @timeline_fences: Pointer to an array of timeline fences.
>>>+ *
>>>+ * Timeline fences are of format struct drm_i915_gem_timeline_fence.
>>>+ */
>>>+ __u64 timeline_fences;
>>>+
>>>+ /** @rsvd3: Reserved, MBZ */
>>>+ __u64 rsvd3;
>>>+
>>>+ /** @extensions: Zero-terminated chain of extensions. */
>>
>>Maybe add "See struct i915_users_extension" here and make it clear that
>>this is what this points to.
>>
>
>Ok.
>
>>>+ __u64 extensions;
>>>+};
>>>+
>>>+/**
>>>+ * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
>>>+ * private to the specified VM.
>>>+ *
>>>+ * See struct drm_i915_gem_create_ext.
>>>+ */
>>>+struct drm_i915_gem_create_ext_vm_private {
>>>+#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
>>>+ /** @base: Extension link. See struct i915_user_extension. */
>>>+ struct i915_user_extension base;
>>>+
>>>+ /** @vm_id: Id of the VM to which the object is private */
>>>+ __u32 vm_id;
>>>+};
>>
>>With the nits addressed:
>>
>>Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>Thanks,
>Niranjana
>
>>
>>>--
>>>2.21.0.rc0.32.g243a4c7e27
>>>
>>
>>--
>>Daniel Vetter
>>Software Engineer, Intel Corporation
>>http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-26 1:08 ` Niranjana Vishwanathapura
@ 2022-06-27 9:07 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-06-27 9:07 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: paulo.r.zanoni, intel-gfx, dri-devel, thomas.hellstrom,
chris.p.wilson, daniel.vetter, christian.koenig, matthew.auld
On Sat, Jun 25, 2022 at 06:08:21PM -0700, Niranjana Vishwanathapura wrote:
> On Sat, Jun 25, 2022 at 12:02:19PM -0700, Niranjana Vishwanathapura wrote:
> > On Fri, Jun 24, 2022 at 10:07:26PM +0200, Daniel Vetter wrote:
> > > On Fri, Jun 24, 2022 at 10:49:36AM -0700, Niranjana Vishwanathapura wrote:
> > > > VM_BIND and related uapi definitions
> > > >
> > > > v2: Reduce the scope to simple Mesa use case.
> > > > v3: Expand VM_UNBIND documentation and add
> > > > I915_GEM_VM_BIND/UNBIND_FENCE_VALID
> > > > and I915_GEM_VM_BIND_TLB_FLUSH flags.
> > > > v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
> > > > documentation for vm_bind/unbind.
> > > > v5: Remove TLB flush requirement on VM_UNBIND.
> > > > Add version support to stage implementation.
> > > > v6: Define and use drm_i915_gem_timeline_fence structure for
> > > > all timeline fences.
> > > >
> > > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > > > ---
> > > > Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
> > > > 1 file changed, 286 insertions(+)
> > > > create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
> > > >
> > > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
> > > > new file mode 100644
> > > > index 000000000000..c784dc0c48b3
> > > > --- /dev/null
> > > > +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> > > > @@ -0,0 +1,286 @@
> > > > +/* SPDX-License-Identifier: MIT */
> > > > +/*
> > > > + * Copyright © 2022 Intel Corporation
> > > > + */
> > > > +
> > > > +/**
> > > > + * DOC: I915_PARAM_HAS_VM_BIND
> > > > + *
> > > > + * VM_BIND feature availability.
> > > > + * See typedef drm_i915_getparam_t param.
> > > > + * bit[0]: If set, VM_BIND is supported, otherwise not.
> > > > + * bits[8-15]: VM_BIND implementation version.
> > > > + * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
> > > > + * created previously with the VM_BIND call. i.e., i915 will not support
> > > > + * splitting/merging of the mappings created with VM_BIND call (See
> > > > + * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
> > >
> > > I think this would read a lot clearer when you make it an itemized list
> > > (or whatever it's called) like we do for atomic properties. Also it reads
> > > a bit strange, so
> > >
> > > * The following versions of VM_BIND have been defined already:
> > > *
> > > * 0: In VM_UNBIND calls the UMD must specify the exact mappings created
> > > * previously with VM_BIND, the ioctl will not support unbinding
> > > * multiple mappings or splitting them. Similar restriction applies to
> > > * VM_BIND.
> > > *
> > > * 1: The restrictions on unbinding partial or multiple mappings is
> > > * lifted, similarly binding will replace any mappings in the given range.
> > >
> > > Pls check I got the formatting right, I didn't check with sphinx :-)
> > >
> > > Bikeshed aside: This is a bit a strange encoding, usually we just use a
> > > simple number and start with 1 (so that 0 is reserved for "doesn't exist")
> > > and then have a #define for each version.
> > >
> >
> > Thanks Daniel,
> >
> > Ok, will eloborate and define the versions.
>
> Will rename this field to I915_PARAM_VM_BIND_VERSION with 0 meaning no vm_bind
> support.
>
> >
> > > > + */
> > > > +#define I915_PARAM_HAS_VM_BIND 57
> > > > +
> > > > +/**
> > > > + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> > > > + *
> > > > + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> > > > + * See struct drm_i915_gem_vm_control flags.
> > > > + *
> > > > + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
> > > > + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
> > > > + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
> > > > + *
> > >
> > > Empty line here.
> >
> > Ok, will fix.
> >
> > >
> > > > + */
> > > > +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
> > > > +
> > > > +/* VM_BIND related ioctls */
> > > > +#define DRM_I915_GEM_VM_BIND 0x3d
> > > > +#define DRM_I915_GEM_VM_UNBIND 0x3e
> > > > +#define DRM_I915_GEM_EXECBUFFER3 0x3f
> > > > +
> > > > +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
> > > > +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
> > > > +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
> > > > +
> > > > +/**
> > > > + * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
> > > > + *
> > > > + * The operation will wait for input fence to signal.
> > > > + *
> > > > + * The returned output fence will be signaled after the completion of the
> > > > + * operation.
> > > > + */
> > > > +struct drm_i915_gem_timeline_fence {
> > > > + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
> > > > + __u32 handle;
> > > > +
> > > > + /**
> > > > + * @flags: Supported flags are:
> > > > + *
> > > > + * I915_TIMELINE_FENCE_WAIT:
> > > > + * Wait for the input fence before the operation.
> > > > + *
> > > > + * I915_TIMELINE_FENCE_SIGNAL:
> > > > + * Return operation completion fence as output.
> > > > + */
> > > > + __u32 flags;
> > > > +#define I915_TIMELINE_FENCE_WAIT (1<<0)
> > > > +#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
> > > > +#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
> > > > +
> > > > + /**
> > > > + * @value: A point in the timeline.
> > > > + * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
> > > > + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> > > > + * binary one.
> > > > + */
> > > > + __u64 value;
> > > > +};
> > > > +
> > > > +/**
> > > > + * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
> > > > + *
> > > > + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
> > > > + * virtual address (VA) range to the section of an object that should be bound
> > > > + * in the device page table of the specified address space (VM).
> > > > + * The VA range specified must be unique (ie., not currently bound) and can
> > > > + * be mapped to whole object or a section of the object (partial binding).
> > > > + * Multiple VA mappings can be created to the same section of the object
> > > > + * (aliasing).
> > > > + *
> > > > + * The @start, @offset and @length should be 4K page aligned. However the DG2
> > > s/should/must/ here
> > >
> >
> > ok
> >
> > > > + * and XEHPSDV has 64K page size for device local-memory and has compact page
> > > > + * table. On those platforms, for binding device local-memory objects, the
> > > > + * @start should be 2M aligned, @offset and @length should be 64K aligned.
> > > ^^ this should is really a recommendation, right?
> > > ^^s/should/must on
> > > the 2nd one here,
> > > it's mandatory
> > >
> >
> > No, it is not a recommendation, will change it to 'must'.
> > This was discussed in an earlier thread with Matt Auld. On 64K lmem platforms,
> > vm_bind will impose same restrictions as current i915 driver does for soft
> > pinning (ie., require 2M alignment for @start and reserve the whole 2M block).
> >
> > > > + * Also, on those platforms, error -ENOSPC will be returned if user tries to
> > > > + * bind a device local-memory object and a system memory object in a single 2M
> > > > + * section of VA range.
> > > > + *
> > > > + * Error code -EINVAL will be returned if @start, @offset and @length are not
> > > > + * properly aligned. Error code of -ENOSPC will be returned if the VA range
> > > > + * specified can't be reserved.
> > >
> > > To check my understanding: ENOSPC can only happen on vm_bind ver 0, not on
> > > vm_bind ver 1 right? We should make sure userspace doesn't rely on this in
> > > some way.
> > >
> >
> > Yah, will update.
> >
> > > > + *
> > > > + * The bind operation can get completed asynchronously and out of submission
> > > > + * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
> > >
> > > "submission order" reads strange for something which is not an engine
> > > (well was in the past). I think you wan here something like
> > >
> > > VM_BIND IOCLT calls executed on different cpu threads concurrently are not
> > > ordered. Furthermore parts of the VM_BIND operation can be done
> > > asynchronously, if I915_GEM_VM_BIND_FENCE_VALID is set.
> > >
> >
> > Hmm...current plan was to do vm_bind asynchronously when an object move is
> > underway, without regard to whether user has specified out fence or not.
> > I think this is not correct as there always should be an out fence if binding
> > is done asynchronously.
> >
> > So, I will change the design as below.
> > 1. When an out fence is specified by UMD,
> > 1a. i915 will bind asynchronously if the object move is underway.
> > 1b. i915 will bind synchronously otherwise.
> > Out fence is signaled after the binding is complete.
> > 2. When out fence is not specified by UMD,
> > 2a. i915 will bind synchronously if object is not being moved.
> > 2b. i915 will return -EAGAIN if object is being moved.
> >
>
> Ok, will keep it simple by making the ioctl always synchronous if
> out fence is not specified (instead of returning -EAGAIN).
> ie., if binding is scheduled behind an object move, then vm_bind ioctl
> will wait until bind is complete before returning if the out fence is
> not specified.
Ah yes, async mode without a fence isn't useful, that wasn't clear to me.
Also unconditionally returning EGAIN would simply mean that userspace
retries in a busy loop (since userspace has no idea what it needs to wait
on first).
-Daniel
>
> Niranjana
>
> > Does that look good?
> >
> > > > + * signaled upon completion of bind operation.
> > > > + */
> > > > +struct drm_i915_gem_vm_bind {
> > > > + /** @vm_id: VM (address space) id to bind */
> > > > + __u32 vm_id;
> > > > +
> > > > + /** @handle: Object handle */
> > > > + __u32 handle;
> > > > +
> > > > + /** @start: Virtual Address start to bind */
> > > > + __u64 start;
> > > > +
> > > > + /** @offset: Offset in object to bind */
> > > > + __u64 offset;
> > > > +
> > > > + /** @length: Length of mapping to bind */
> > > > + __u64 length;
> > > > +
> > > > + /**
> > > > + * @flags: Supported flags are:
> > > > + *
> > > > + * I915_GEM_VM_BIND_FENCE_VALID:
> > > > + * @fence is valid, needs bind completion notification.
> > >
> > > I guess we have a rule that FENCE_VALID can be set if and only if
> > > FENCE_WAIT flag in the embedded fence structure is also set? Since we do
> > > not put this into the extension chain like with execbuf it's always there,
> > > but could be empty. ioctl input validation needs to be really careful here
> > > and make sure these fields are all cleared if we don't have a fence.
> > >
> >
> > Yah, given that now we have a generic timeline fence, we can check the
> > drm_i915_gem_timeline_fence.flags to see if valid fence is specified or
> > not. I will remove this FENCE_VALID flag here and in vm_unbind struct.
> >
> > > > + *
> > > > + * I915_GEM_VM_BIND_READONLY:
> > > > + * Mapping is read-only.
> > > > + *
> > > > + * I915_GEM_VM_BIND_CAPTURE:
> > > > + * Capture this mapping in the dump upon GPU error.
> > > > + */
> > > > + __u64 flags;
> > > > +#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
> > > > +#define I915_GEM_VM_BIND_READONLY (1 << 1)
> > > > +#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
> > > > +
> > > > + /**
> > > > + * @fence: Timeline fence for bind completion signaling.
> > > > + *
> > > > + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
> > > > + * is invalid, and an error will be returned.
> > > > + */
> > > > + struct drm_i915_gem_timeline_fence fence;
> > > > +
> > > > + /** @extensions: 0-terminated chain of extensions */
> > > > + __u64 extensions;
> > > > +};
> > > > +
> > > > +/**
> > > > + * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
> > > > + *
> > > > + * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
> > > > + * address (VA) range that should be unbound from the device page table of the
> > > > + * specified address space (VM). VM_UNBIND will force unbind the specified
> > > > + * range from device page table without waiting for any GPU job to complete.
> > > > + * It is UMDs responsibility to ensure the mapping is no longer in use before
> > > > + * calling VM_UNBIND.
> > > > + *
> > > > + * If the specified mapping is not found, the ioctl will simply return without
> > > > + * any error.
> > > > + *
> > > > + * The unbind operation can get completed asynchronously and out of submission
> > > > + * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
> > > > + * signaled upon completion of unbind operation.
> > >
> > > Same here as above.
> > >
> > > > + */
> > > > +struct drm_i915_gem_vm_unbind {
> > > > + /** @vm_id: VM (address space) id to bind */
> > > > + __u32 vm_id;
> > > > +
> > > > + /** @rsvd: Reserved, MBZ */
> > > > + __u32 rsvd;
> > > > +
> > > > + /** @start: Virtual Address start to unbind */
> > > > + __u64 start;
> > > > +
> > > > + /** @length: Length of mapping to unbind */
> > > > + __u64 length;
> > > > +
> > > > + /**
> > > > + * @flags: Supported flags are:
> > > > + *
> > > > + * I915_GEM_VM_UNBIND_FENCE_VALID:
> > > > + * @fence is valid, needs unbind completion notification.
> > > > + */
> > > > + __u64 flags;
> > > > +#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
> > > > +
> > > > + /**
> > > > + * @fence: Timeline fence for unbind completion signaling.
> > > > + *
> > > > + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
> > > > + * is invalid, and an error will be returned.
> > > > + */
> > > > + struct drm_i915_gem_timeline_fence fence;
> > > > +
> > > > + /** @extensions: 0-terminated chain of extensions */
> > > > + __u64 extensions;
> > > > +};
> > > > +
> > > > +/**
> > > > + * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
> > > > + * ioctl.
> > > > + *
> > > > + * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
> > > > + * only works with this ioctl for submission.
> > > > + * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
> > > > + */
> > > > +struct drm_i915_gem_execbuffer3 {
> > > > + /**
> > > > + * @ctx_id: Context id
> > > > + *
> > > > + * Only contexts with user engine map are allowed.
> > > > + */
> > > > + __u32 ctx_id;
> > > > +
> > > > + /**
> > > > + * @engine_idx: Engine index
> > > > + *
> > > > + * An index in the user engine map of the context specified by @ctx_id.
> > > > + */
> > > > + __u32 engine_idx;
> > > > +
> > > > + /** @rsvd1: Reserved, MBZ */
> > > > + __u32 rsvd1;
> > > > +
> > > > + /**
> > > > + * @batch_count: Number of batches in @batch_address array.
> > > > + *
> > > > + * 0 is invalid. For parallel submission, it should be equal to the
> > > > + * number of (parallel) engines involved in that submission.
> > > > + */
> > > > + __u32 batch_count;
> > >
> > > I think this is a leftover from the old parallel engine design, where
> > > userspace stitched together the parallel batches? With the new design we
> > > have in execbuf2 the batch_count is fixed at context creation time (that's
> > > why document how that works is important), and so specifying this again in
> > > execbuf3 could only lead to bugs and hence the field here should be
> > > dropped.
> >
> > Ok, will remove.
> >
> > >
> > > > +
> > > > + /**
> > > > + * @batch_address: Array of batch gpu virtual addresses.
> > > > + *
> > > > + * If @batch_count is 1, then it is the gpu virtual address of the
> > > > + * batch buffer. If @batch_count > 1, then it is a pointer to an array
> > > > + * of batch buffer gpu virtual addresses.
> > >
> > > We derive the number of batchbuffers a context needs from
> > > ctx->parallel.num_children + 1. No need for userspace to supply this on
> > > every execbuf, but the rules about the address still are the same.
> > >
> >
> > My only worry is if the number of batch addresses provided is not equal
> > to 'ctx->parallel.num_children + 1', then we can get into harder to debug
> > GPU hangs (where as with batch_count, we can return an error). Besides,
> > without explicit batch_count here, treating this field differently when
> > one batch case vs multiple batch case makes the uapi look bit complicated.
> >
> > But, I guess, user can error up and get into harder to debug GPU hangs
> > even with batch_count. I think removing the batch_count and properly
> > documenting this batch_address field here should be good. Will update.
> >
> > > > + */
> > > > + __u64 batch_address;
> > > > +
> > > > + /**
> > > > + * @flags: Supported flags are:
> > > > + *
> > > > + * I915_EXEC3_SECURE:
> > > > + * Request a privileged ("secure") batch buffer/s.
> > > > + * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
> > >
> > > Do we actually still need this for any userspace? Iirc the only user was
> > > SNA, which isn't a thing anymore since about gen9 or so. Or am I mistaken?
> > > A quick git grep doesn't show up anything.
> > >
> > > igt probably uses it for lots of fun stuff, might be time to remove it on
> > > the execbuf2 side too on modern platforms since it's kinda a risky thing
> > > to do (and flat out not a good idea on secure boot platforms even when
> > > you're root).
> > >
> >
> > Yah, looks like it is only used on very old platforms, will remove this
> > flag.
> >
> > > > + */
> > > > + __u64 flags;
> > > > +#define I915_EXEC3_SECURE (1 << 0)
> > > > +
> > > > + /** @rsvd2: Reserved, MBZ */
> > > > + __u32 rsvd2;
> > > > +
> > > > + /** @fence_count: Number of fences in @timeline_fences array. */
> > > > + __u32 fence_count;
> > > > +
> > > > + /**
> > > > + * @timeline_fences: Pointer to an array of timeline fences.
> > > > + *
> > > > + * Timeline fences are of format struct drm_i915_gem_timeline_fence.
> > > > + */
> > > > + __u64 timeline_fences;
> > > > +
> > > > + /** @rsvd3: Reserved, MBZ */
> > > > + __u64 rsvd3;
> > > > +
> > > > + /** @extensions: Zero-terminated chain of extensions. */
> > >
> > > Maybe add "See struct i915_users_extension" here and make it clear that
> > > this is what this points to.
> > >
> >
> > Ok.
> >
> > > > + __u64 extensions;
> > > > +};
> > > > +
> > > > +/**
> > > > + * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
> > > > + * private to the specified VM.
> > > > + *
> > > > + * See struct drm_i915_gem_create_ext.
> > > > + */
> > > > +struct drm_i915_gem_create_ext_vm_private {
> > > > +#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
> > > > + /** @base: Extension link. See struct i915_user_extension. */
> > > > + struct i915_user_extension base;
> > > > +
> > > > + /** @vm_id: Id of the VM to which the object is private */
> > > > + __u32 vm_id;
> > > > +};
> > >
> > > With the nits addressed:
> > >
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Thanks,
> > Niranjana
> >
> > >
> > > > --
> > > > 2.21.0.rc0.32.g243a4c7e27
> > > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-25 19:02 ` Niranjana Vishwanathapura
2022-06-26 1:08 ` Niranjana Vishwanathapura
@ 2022-06-27 8:55 ` Daniel Vetter
2022-06-30 6:21 ` Niranjana Vishwanathapura
1 sibling, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2022-06-27 8:55 UTC (permalink / raw)
To: Niranjana Vishwanathapura
Cc: paulo.r.zanoni, intel-gfx, chris.p.wilson, thomas.hellstrom,
dri-devel, daniel.vetter, christian.koenig, matthew.auld
On Sat, Jun 25, 2022 at 12:02:19PM -0700, Niranjana Vishwanathapura wrote:
> On Fri, Jun 24, 2022 at 10:07:26PM +0200, Daniel Vetter wrote:
> > On Fri, Jun 24, 2022 at 10:49:36AM -0700, Niranjana Vishwanathapura wrote:
> > > VM_BIND and related uapi definitions
> > >
> > > v2: Reduce the scope to simple Mesa use case.
> > > v3: Expand VM_UNBIND documentation and add
> > > I915_GEM_VM_BIND/UNBIND_FENCE_VALID
> > > and I915_GEM_VM_BIND_TLB_FLUSH flags.
> > > v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
> > > documentation for vm_bind/unbind.
> > > v5: Remove TLB flush requirement on VM_UNBIND.
> > > Add version support to stage implementation.
> > > v6: Define and use drm_i915_gem_timeline_fence structure for
> > > all timeline fences.
> > >
> > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > > ---
> > > Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
> > > 1 file changed, 286 insertions(+)
> > > create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
> > >
> > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
> > > new file mode 100644
> > > index 000000000000..c784dc0c48b3
> > > --- /dev/null
> > > +++ b/Documentation/gpu/rfc/i915_vm_bind.h
> > > @@ -0,0 +1,286 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > +
> > > +/**
> > > + * DOC: I915_PARAM_HAS_VM_BIND
> > > + *
> > > + * VM_BIND feature availability.
> > > + * See typedef drm_i915_getparam_t param.
> > > + * bit[0]: If set, VM_BIND is supported, otherwise not.
> > > + * bits[8-15]: VM_BIND implementation version.
> > > + * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
> > > + * created previously with the VM_BIND call. i.e., i915 will not support
> > > + * splitting/merging of the mappings created with VM_BIND call (See
> > > + * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
> >
> > I think this would read a lot clearer when you make it an itemized list
> > (or whatever it's called) like we do for atomic properties. Also it reads
> > a bit strange, so
> >
> > * The following versions of VM_BIND have been defined already:
> > *
> > * 0: In VM_UNBIND calls the UMD must specify the exact mappings created
> > * previously with VM_BIND, the ioctl will not support unbinding
> > * multiple mappings or splitting them. Similar restriction applies to
> > * VM_BIND.
> > *
> > * 1: The restrictions on unbinding partial or multiple mappings is
> > * lifted, similarly binding will replace any mappings in the given range.
> >
> > Pls check I got the formatting right, I didn't check with sphinx :-)
> >
> > Bikeshed aside: This is a bit a strange encoding, usually we just use a
> > simple number and start with 1 (so that 0 is reserved for "doesn't exist")
> > and then have a #define for each version.
> >
>
> Thanks Daniel,
>
> Ok, will eloborate and define the versions.
>
> > > + */
> > > +#define I915_PARAM_HAS_VM_BIND 57
> > > +
> > > +/**
> > > + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
> > > + *
> > > + * Flag to opt-in for VM_BIND mode of binding during VM creation.
> > > + * See struct drm_i915_gem_vm_control flags.
> > > + *
> > > + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
> > > + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
> > > + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
> > > + *
> >
> > Empty line here.
>
> Ok, will fix.
>
> >
> > > + */
> > > +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
> > > +
> > > +/* VM_BIND related ioctls */
> > > +#define DRM_I915_GEM_VM_BIND 0x3d
> > > +#define DRM_I915_GEM_VM_UNBIND 0x3e
> > > +#define DRM_I915_GEM_EXECBUFFER3 0x3f
> > > +
> > > +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
> > > +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
> > > +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
> > > +
> > > +/**
> > > + * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
> > > + *
> > > + * The operation will wait for input fence to signal.
> > > + *
> > > + * The returned output fence will be signaled after the completion of the
> > > + * operation.
> > > + */
> > > +struct drm_i915_gem_timeline_fence {
> > > + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
> > > + __u32 handle;
> > > +
> > > + /**
> > > + * @flags: Supported flags are:
> > > + *
> > > + * I915_TIMELINE_FENCE_WAIT:
> > > + * Wait for the input fence before the operation.
> > > + *
> > > + * I915_TIMELINE_FENCE_SIGNAL:
> > > + * Return operation completion fence as output.
> > > + */
> > > + __u32 flags;
> > > +#define I915_TIMELINE_FENCE_WAIT (1<<0)
> > > +#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
> > > +#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
> > > +
> > > + /**
> > > + * @value: A point in the timeline.
> > > + * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
> > > + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
> > > + * binary one.
> > > + */
> > > + __u64 value;
> > > +};
> > > +
> > > +/**
> > > + * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
> > > + *
> > > + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
> > > + * virtual address (VA) range to the section of an object that should be bound
> > > + * in the device page table of the specified address space (VM).
> > > + * The VA range specified must be unique (ie., not currently bound) and can
> > > + * be mapped to whole object or a section of the object (partial binding).
> > > + * Multiple VA mappings can be created to the same section of the object
> > > + * (aliasing).
> > > + *
> > > + * The @start, @offset and @length should be 4K page aligned. However the DG2
> > s/should/must/ here
> >
>
> ok
>
> > > + * and XEHPSDV has 64K page size for device local-memory and has compact page
> > > + * table. On those platforms, for binding device local-memory objects, the
> > > + * @start should be 2M aligned, @offset and @length should be 64K aligned.
> > ^^ this should is really a recommendation, right?
> > ^^s/should/must on
> > the 2nd one here,
> > it's mandatory
> >
>
> No, it is not a recommendation, will change it to 'must'.
> This was discussed in an earlier thread with Matt Auld. On 64K lmem platforms,
> vm_bind will impose same restrictions as current i915 driver does for soft
> pinning (ie., require 2M alignment for @start and reserve the whole 2M block).
Ok. I didn't check what exact rules we agreed on for these.
> > > + * Also, on those platforms, error -ENOSPC will be returned if user tries to
> > > + * bind a device local-memory object and a system memory object in a single 2M
> > > + * section of VA range.
> > > + *
> > > + * Error code -EINVAL will be returned if @start, @offset and @length are not
> > > + * properly aligned. Error code of -ENOSPC will be returned if the VA range
> > > + * specified can't be reserved.
> >
> > To check my understanding: ENOSPC can only happen on vm_bind ver 0, not on
> > vm_bind ver 1 right? We should make sure userspace doesn't rely on this in
> > some way.
> >
>
> Yah, will update.
>
> > > + *
> > > + * The bind operation can get completed asynchronously and out of submission
> > > + * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
> >
> > "submission order" reads strange for something which is not an engine
> > (well was in the past). I think you wan here something like
> >
> > VM_BIND IOCLT calls executed on different cpu threads concurrently are not
> > ordered. Furthermore parts of the VM_BIND operation can be done
> > asynchronously, if I915_GEM_VM_BIND_FENCE_VALID is set.
> >
>
> Hmm...current plan was to do vm_bind asynchronously when an object move is
> underway, without regard to whether user has specified out fence or not.
> I think this is not correct as there always should be an out fence if binding
> is done asynchronously.
>
> So, I will change the design as below.
> 1. When an out fence is specified by UMD,
> 1a. i915 will bind asynchronously if the object move is underway.
> 1b. i915 will bind synchronously otherwise.
> Out fence is signaled after the binding is complete.
> 2. When out fence is not specified by UMD,
> 2a. i915 will bind synchronously if object is not being moved.
> 2b. i915 will return -EAGAIN if object is being moved.
>
> Does that look good?
This is not what I meant, it's really only about the word "submission
order". It's really just a bikeshed, so on 2nd thought leave it as is.
We should definitely not specify in the docs when exactly we do something
async, since that's really implementation detail. For vm_bind/unbind async
is a pure performance optimization, there's no semantic guarantees here
now that we moved the in-fence waiting into userspace.
>
> > > + * signaled upon completion of bind operation.
> > > + */
> > > +struct drm_i915_gem_vm_bind {
> > > + /** @vm_id: VM (address space) id to bind */
> > > + __u32 vm_id;
> > > +
> > > + /** @handle: Object handle */
> > > + __u32 handle;
> > > +
> > > + /** @start: Virtual Address start to bind */
> > > + __u64 start;
> > > +
> > > + /** @offset: Offset in object to bind */
> > > + __u64 offset;
> > > +
> > > + /** @length: Length of mapping to bind */
> > > + __u64 length;
> > > +
> > > + /**
> > > + * @flags: Supported flags are:
> > > + *
> > > + * I915_GEM_VM_BIND_FENCE_VALID:
> > > + * @fence is valid, needs bind completion notification.
> >
> > I guess we have a rule that FENCE_VALID can be set if and only if
> > FENCE_WAIT flag in the embedded fence structure is also set? Since we do
> > not put this into the extension chain like with execbuf it's always there,
> > but could be empty. ioctl input validation needs to be really careful here
> > and make sure these fields are all cleared if we don't have a fence.
> >
>
> Yah, given that now we have a generic timeline fence, we can check the
> drm_i915_gem_timeline_fence.flags to see if valid fence is specified or
> not. I will remove this FENCE_VALID flag here and in vm_unbind struct.
Ok works for me. Maybe keep a hint in this flags documentation that
there's more flags in the timeline struct :-)
> > > + *
> > > + * I915_GEM_VM_BIND_READONLY:
> > > + * Mapping is read-only.
> > > + *
> > > + * I915_GEM_VM_BIND_CAPTURE:
> > > + * Capture this mapping in the dump upon GPU error.
> > > + */
> > > + __u64 flags;
> > > +#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
> > > +#define I915_GEM_VM_BIND_READONLY (1 << 1)
> > > +#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
> > > +
> > > + /**
> > > + * @fence: Timeline fence for bind completion signaling.
> > > + *
> > > + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
> > > + * is invalid, and an error will be returned.
> > > + */
> > > + struct drm_i915_gem_timeline_fence fence;
> > > +
> > > + /** @extensions: 0-terminated chain of extensions */
> > > + __u64 extensions;
> > > +};
> > > +
> > > +/**
> > > + * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
> > > + *
> > > + * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
> > > + * address (VA) range that should be unbound from the device page table of the
> > > + * specified address space (VM). VM_UNBIND will force unbind the specified
> > > + * range from device page table without waiting for any GPU job to complete.
> > > + * It is UMDs responsibility to ensure the mapping is no longer in use before
> > > + * calling VM_UNBIND.
> > > + *
> > > + * If the specified mapping is not found, the ioctl will simply return without
> > > + * any error.
> > > + *
> > > + * The unbind operation can get completed asynchronously and out of submission
> > > + * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
> > > + * signaled upon completion of unbind operation.
> >
> > Same here as above.
> >
> > > + */
> > > +struct drm_i915_gem_vm_unbind {
> > > + /** @vm_id: VM (address space) id to bind */
> > > + __u32 vm_id;
> > > +
> > > + /** @rsvd: Reserved, MBZ */
> > > + __u32 rsvd;
> > > +
> > > + /** @start: Virtual Address start to unbind */
> > > + __u64 start;
> > > +
> > > + /** @length: Length of mapping to unbind */
> > > + __u64 length;
> > > +
> > > + /**
> > > + * @flags: Supported flags are:
> > > + *
> > > + * I915_GEM_VM_UNBIND_FENCE_VALID:
> > > + * @fence is valid, needs unbind completion notification.
> > > + */
> > > + __u64 flags;
> > > +#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
> > > +
> > > + /**
> > > + * @fence: Timeline fence for unbind completion signaling.
> > > + *
> > > + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
> > > + * is invalid, and an error will be returned.
> > > + */
> > > + struct drm_i915_gem_timeline_fence fence;
> > > +
> > > + /** @extensions: 0-terminated chain of extensions */
> > > + __u64 extensions;
> > > +};
> > > +
> > > +/**
> > > + * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
> > > + * ioctl.
> > > + *
> > > + * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
> > > + * only works with this ioctl for submission.
> > > + * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
> > > + */
> > > +struct drm_i915_gem_execbuffer3 {
> > > + /**
> > > + * @ctx_id: Context id
> > > + *
> > > + * Only contexts with user engine map are allowed.
> > > + */
> > > + __u32 ctx_id;
> > > +
> > > + /**
> > > + * @engine_idx: Engine index
> > > + *
> > > + * An index in the user engine map of the context specified by @ctx_id.
> > > + */
> > > + __u32 engine_idx;
> > > +
> > > + /** @rsvd1: Reserved, MBZ */
> > > + __u32 rsvd1;
> > > +
> > > + /**
> > > + * @batch_count: Number of batches in @batch_address array.
> > > + *
> > > + * 0 is invalid. For parallel submission, it should be equal to the
> > > + * number of (parallel) engines involved in that submission.
> > > + */
> > > + __u32 batch_count;
> >
> > I think this is a leftover from the old parallel engine design, where
> > userspace stitched together the parallel batches? With the new design we
> > have in execbuf2 the batch_count is fixed at context creation time (that's
> > why document how that works is important), and so specifying this again in
> > execbuf3 could only lead to bugs and hence the field here should be
> > dropped.
>
> Ok, will remove.
>
> >
> > > +
> > > + /**
> > > + * @batch_address: Array of batch gpu virtual addresses.
> > > + *
> > > + * If @batch_count is 1, then it is the gpu virtual address of the
> > > + * batch buffer. If @batch_count > 1, then it is a pointer to an array
> > > + * of batch buffer gpu virtual addresses.
> >
> > We derive the number of batchbuffers a context needs from
> > ctx->parallel.num_children + 1. No need for userspace to supply this on
> > every execbuf, but the rules about the address still are the same.
> >
>
> My only worry is if the number of batch addresses provided is not equal
> to 'ctx->parallel.num_children + 1', then we can get into harder to debug
> GPU hangs (where as with batch_count, we can return an error). Besides,
> without explicit batch_count here, treating this field differently when
> one batch case vs multiple batch case makes the uapi look bit complicated.
>
> But, I guess, user can error up and get into harder to debug GPU hangs
> even with batch_count. I think removing the batch_count and properly
> documenting this batch_address field here should be good. Will update.
Yeah userspace can submit garbage. Like they could also have an internal
book-keep error in other ways.
> > > + */
> > > + __u64 batch_address;
> > > +
> > > + /**
> > > + * @flags: Supported flags are:
> > > + *
> > > + * I915_EXEC3_SECURE:
> > > + * Request a privileged ("secure") batch buffer/s.
> > > + * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
> >
> > Do we actually still need this for any userspace? Iirc the only user was
> > SNA, which isn't a thing anymore since about gen9 or so. Or am I mistaken?
> > A quick git grep doesn't show up anything.
> >
> > igt probably uses it for lots of fun stuff, might be time to remove it on
> > the execbuf2 side too on modern platforms since it's kinda a risky thing
> > to do (and flat out not a good idea on secure boot platforms even when
> > you're root).
> >
>
> Yah, looks like it is only used on very old platforms, will remove this
> flag.
Yay!
> > > + */
> > > + __u64 flags;
> > > +#define I915_EXEC3_SECURE (1 << 0)
> > > +
> > > + /** @rsvd2: Reserved, MBZ */
> > > + __u32 rsvd2;
> > > +
> > > + /** @fence_count: Number of fences in @timeline_fences array. */
> > > + __u32 fence_count;
> > > +
> > > + /**
> > > + * @timeline_fences: Pointer to an array of timeline fences.
> > > + *
> > > + * Timeline fences are of format struct drm_i915_gem_timeline_fence.
> > > + */
> > > + __u64 timeline_fences;
> > > +
> > > + /** @rsvd3: Reserved, MBZ */
> > > + __u64 rsvd3;
> > > +
> > > + /** @extensions: Zero-terminated chain of extensions. */
> >
> > Maybe add "See struct i915_users_extension" here and make it clear that
> > this is what this points to.
> >
>
> Ok.
>
> > > + __u64 extensions;
> > > +};
> > > +
> > > +/**
> > > + * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
> > > + * private to the specified VM.
> > > + *
> > > + * See struct drm_i915_gem_create_ext.
> > > + */
> > > +struct drm_i915_gem_create_ext_vm_private {
> > > +#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
> > > + /** @base: Extension link. See struct i915_user_extension. */
> > > + struct i915_user_extension base;
> > > +
> > > + /** @vm_id: Id of the VM to which the object is private */
> > > + __u32 vm_id;
> > > +};
> >
> > With the nits addressed:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Thanks,
> Niranjana
Ok I think we have consensus everywhere and it's really only minor stuff,
so please include my r-b on the next round.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
2022-06-27 8:55 ` Daniel Vetter
@ 2022-06-30 6:21 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 23+ messages in thread
From: Niranjana Vishwanathapura @ 2022-06-30 6:21 UTC (permalink / raw)
To: Daniel Vetter
Cc: paulo.r.zanoni, intel-gfx, dri-devel, thomas.hellstrom,
chris.p.wilson, daniel.vetter, christian.koenig, matthew.auld
On Mon, Jun 27, 2022 at 10:55:16AM +0200, Daniel Vetter wrote:
>On Sat, Jun 25, 2022 at 12:02:19PM -0700, Niranjana Vishwanathapura wrote:
>> On Fri, Jun 24, 2022 at 10:07:26PM +0200, Daniel Vetter wrote:
>> > On Fri, Jun 24, 2022 at 10:49:36AM -0700, Niranjana Vishwanathapura wrote:
>> > > VM_BIND and related uapi definitions
>> > >
>> > > v2: Reduce the scope to simple Mesa use case.
>> > > v3: Expand VM_UNBIND documentation and add
>> > > I915_GEM_VM_BIND/UNBIND_FENCE_VALID
>> > > and I915_GEM_VM_BIND_TLB_FLUSH flags.
>> > > v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
>> > > documentation for vm_bind/unbind.
>> > > v5: Remove TLB flush requirement on VM_UNBIND.
>> > > Add version support to stage implementation.
>> > > v6: Define and use drm_i915_gem_timeline_fence structure for
>> > > all timeline fences.
>> > >
>> > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> > > ---
>> > > Documentation/gpu/rfc/i915_vm_bind.h | 286 +++++++++++++++++++++++++++
>> > > 1 file changed, 286 insertions(+)
>> > > create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
>> > >
>> > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.h b/Documentation/gpu/rfc/i915_vm_bind.h
>> > > new file mode 100644
>> > > index 000000000000..c784dc0c48b3
>> > > --- /dev/null
>> > > +++ b/Documentation/gpu/rfc/i915_vm_bind.h
>> > > @@ -0,0 +1,286 @@
>> > > +/* SPDX-License-Identifier: MIT */
>> > > +/*
>> > > + * Copyright © 2022 Intel Corporation
>> > > + */
>> > > +
>> > > +/**
>> > > + * DOC: I915_PARAM_HAS_VM_BIND
>> > > + *
>> > > + * VM_BIND feature availability.
>> > > + * See typedef drm_i915_getparam_t param.
>> > > + * bit[0]: If set, VM_BIND is supported, otherwise not.
>> > > + * bits[8-15]: VM_BIND implementation version.
>> > > + * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
>> > > + * created previously with the VM_BIND call. i.e., i915 will not support
>> > > + * splitting/merging of the mappings created with VM_BIND call (See
>> > > + * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
>> >
>> > I think this would read a lot clearer when you make it an itemized list
>> > (or whatever it's called) like we do for atomic properties. Also it reads
>> > a bit strange, so
>> >
>> > * The following versions of VM_BIND have been defined already:
>> > *
>> > * 0: In VM_UNBIND calls the UMD must specify the exact mappings created
>> > * previously with VM_BIND, the ioctl will not support unbinding
>> > * multiple mappings or splitting them. Similar restriction applies to
>> > * VM_BIND.
>> > *
>> > * 1: The restrictions on unbinding partial or multiple mappings is
>> > * lifted, similarly binding will replace any mappings in the given range.
>> >
>> > Pls check I got the formatting right, I didn't check with sphinx :-)
>> >
>> > Bikeshed aside: This is a bit a strange encoding, usually we just use a
>> > simple number and start with 1 (so that 0 is reserved for "doesn't exist")
>> > and then have a #define for each version.
>> >
>>
>> Thanks Daniel,
>>
>> Ok, will eloborate and define the versions.
>>
>> > > + */
>> > > +#define I915_PARAM_HAS_VM_BIND 57
>> > > +
>> > > +/**
>> > > + * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
>> > > + *
>> > > + * Flag to opt-in for VM_BIND mode of binding during VM creation.
>> > > + * See struct drm_i915_gem_vm_control flags.
>> > > + *
>> > > + * The older execbuf2 ioctl will not support VM_BIND mode of operation.
>> > > + * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
>> > > + * execlist (See struct drm_i915_gem_execbuffer3 for more details).
>> > > + *
>> >
>> > Empty line here.
>>
>> Ok, will fix.
>>
>> >
>> > > + */
>> > > +#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
>> > > +
>> > > +/* VM_BIND related ioctls */
>> > > +#define DRM_I915_GEM_VM_BIND 0x3d
>> > > +#define DRM_I915_GEM_VM_UNBIND 0x3e
>> > > +#define DRM_I915_GEM_EXECBUFFER3 0x3f
>> > > +
>> > > +#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
>> > > +#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
>> > > +#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
>> > > +
>> > > +/**
>> > > + * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
>> > > + *
>> > > + * The operation will wait for input fence to signal.
>> > > + *
>> > > + * The returned output fence will be signaled after the completion of the
>> > > + * operation.
>> > > + */
>> > > +struct drm_i915_gem_timeline_fence {
>> > > + /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>> > > + __u32 handle;
>> > > +
>> > > + /**
>> > > + * @flags: Supported flags are:
>> > > + *
>> > > + * I915_TIMELINE_FENCE_WAIT:
>> > > + * Wait for the input fence before the operation.
>> > > + *
>> > > + * I915_TIMELINE_FENCE_SIGNAL:
>> > > + * Return operation completion fence as output.
>> > > + */
>> > > + __u32 flags;
>> > > +#define I915_TIMELINE_FENCE_WAIT (1<<0)
>> > > +#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
>> > > +#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
>> > > +
>> > > + /**
>> > > + * @value: A point in the timeline.
>> > > + * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
>> > > + * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
>> > > + * binary one.
>> > > + */
>> > > + __u64 value;
>> > > +};
>> > > +
>> > > +/**
>> > > + * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
>> > > + *
>> > > + * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
>> > > + * virtual address (VA) range to the section of an object that should be bound
>> > > + * in the device page table of the specified address space (VM).
>> > > + * The VA range specified must be unique (ie., not currently bound) and can
>> > > + * be mapped to whole object or a section of the object (partial binding).
>> > > + * Multiple VA mappings can be created to the same section of the object
>> > > + * (aliasing).
>> > > + *
>> > > + * The @start, @offset and @length should be 4K page aligned. However the DG2
>> > s/should/must/ here
>> >
>>
>> ok
>>
>> > > + * and XEHPSDV has 64K page size for device local-memory and has compact page
>> > > + * table. On those platforms, for binding device local-memory objects, the
>> > > + * @start should be 2M aligned, @offset and @length should be 64K aligned.
>> > ^^ this should is really a recommendation, right?
>> > ^^s/should/must on
>> > the 2nd one here,
>> > it's mandatory
>> >
>>
>> No, it is not a recommendation, will change it to 'must'.
>> This was discussed in an earlier thread with Matt Auld. On 64K lmem platforms,
>> vm_bind will impose same restrictions as current i915 driver does for soft
>> pinning (ie., require 2M alignment for @start and reserve the whole 2M block).
>
>Ok. I didn't check what exact rules we agreed on for these.
There is some dicussion (comment from Jason) on this in latest patch series.
>
>> > > + * Also, on those platforms, error -ENOSPC will be returned if user tries to
>> > > + * bind a device local-memory object and a system memory object in a single 2M
>> > > + * section of VA range.
>> > > + *
>> > > + * Error code -EINVAL will be returned if @start, @offset and @length are not
>> > > + * properly aligned. Error code of -ENOSPC will be returned if the VA range
>> > > + * specified can't be reserved.
>> >
>> > To check my understanding: ENOSPC can only happen on vm_bind ver 0, not on
>> > vm_bind ver 1 right? We should make sure userspace doesn't rely on this in
>> > some way.
>> >
>>
>> Yah, will update.
>>
>> > > + *
>> > > + * The bind operation can get completed asynchronously and out of submission
>> > > + * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
>> >
>> > "submission order" reads strange for something which is not an engine
>> > (well was in the past). I think you wan here something like
>> >
>> > VM_BIND IOCLT calls executed on different cpu threads concurrently are not
>> > ordered. Furthermore parts of the VM_BIND operation can be done
>> > asynchronously, if I915_GEM_VM_BIND_FENCE_VALID is set.
>> >
>>
>> Hmm...current plan was to do vm_bind asynchronously when an object move is
>> underway, without regard to whether user has specified out fence or not.
>> I think this is not correct as there always should be an out fence if binding
>> is done asynchronously.
>>
>> So, I will change the design as below.
>> 1. When an out fence is specified by UMD,
>> 1a. i915 will bind asynchronously if the object move is underway.
>> 1b. i915 will bind synchronously otherwise.
>> Out fence is signaled after the binding is complete.
>> 2. When out fence is not specified by UMD,
>> 2a. i915 will bind synchronously if object is not being moved.
>> 2b. i915 will return -EAGAIN if object is being moved.
>>
>> Does that look good?
>
>This is not what I meant, it's really only about the word "submission
>order". It's really just a bikeshed, so on 2nd thought leave it as is.
>
>We should definitely not specify in the docs when exactly we do something
>async, since that's really implementation detail. For vm_bind/unbind async
>is a pure performance optimization, there's no semantic guarantees here
>now that we moved the in-fence waiting into userspace.
>
Ok, I kept it simple in latest revision (without implementation details).
>
>>
>> > > + * signaled upon completion of bind operation.
>> > > + */
>> > > +struct drm_i915_gem_vm_bind {
>> > > + /** @vm_id: VM (address space) id to bind */
>> > > + __u32 vm_id;
>> > > +
>> > > + /** @handle: Object handle */
>> > > + __u32 handle;
>> > > +
>> > > + /** @start: Virtual Address start to bind */
>> > > + __u64 start;
>> > > +
>> > > + /** @offset: Offset in object to bind */
>> > > + __u64 offset;
>> > > +
>> > > + /** @length: Length of mapping to bind */
>> > > + __u64 length;
>> > > +
>> > > + /**
>> > > + * @flags: Supported flags are:
>> > > + *
>> > > + * I915_GEM_VM_BIND_FENCE_VALID:
>> > > + * @fence is valid, needs bind completion notification.
>> >
>> > I guess we have a rule that FENCE_VALID can be set if and only if
>> > FENCE_WAIT flag in the embedded fence structure is also set? Since we do
>> > not put this into the extension chain like with execbuf it's always there,
>> > but could be empty. ioctl input validation needs to be really careful here
>> > and make sure these fields are all cleared if we don't have a fence.
>> >
>>
>> Yah, given that now we have a generic timeline fence, we can check the
>> drm_i915_gem_timeline_fence.flags to see if valid fence is specified or
>> not. I will remove this FENCE_VALID flag here and in vm_unbind struct.
>
>Ok works for me. Maybe keep a hint in this flags documentation that
>there's more flags in the timeline struct :-)
Ok
>
>> > > + *
>> > > + * I915_GEM_VM_BIND_READONLY:
>> > > + * Mapping is read-only.
>> > > + *
>> > > + * I915_GEM_VM_BIND_CAPTURE:
>> > > + * Capture this mapping in the dump upon GPU error.
>> > > + */
>> > > + __u64 flags;
>> > > +#define I915_GEM_VM_BIND_FENCE_VALID (1 << 0)
>> > > +#define I915_GEM_VM_BIND_READONLY (1 << 1)
>> > > +#define I915_GEM_VM_BIND_CAPTURE (1 << 2)
>> > > +
>> > > + /**
>> > > + * @fence: Timeline fence for bind completion signaling.
>> > > + *
>> > > + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>> > > + * is invalid, and an error will be returned.
>> > > + */
>> > > + struct drm_i915_gem_timeline_fence fence;
>> > > +
>> > > + /** @extensions: 0-terminated chain of extensions */
>> > > + __u64 extensions;
>> > > +};
>> > > +
>> > > +/**
>> > > + * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
>> > > + *
>> > > + * This structure is passed to VM_UNBIND ioctl and specifies the GPU virtual
>> > > + * address (VA) range that should be unbound from the device page table of the
>> > > + * specified address space (VM). VM_UNBIND will force unbind the specified
>> > > + * range from device page table without waiting for any GPU job to complete.
>> > > + * It is UMDs responsibility to ensure the mapping is no longer in use before
>> > > + * calling VM_UNBIND.
>> > > + *
>> > > + * If the specified mapping is not found, the ioctl will simply return without
>> > > + * any error.
>> > > + *
>> > > + * The unbind operation can get completed asynchronously and out of submission
>> > > + * order. When I915_GEM_VM_UNBIND_FENCE_VALID flag is set, the @fence will be
>> > > + * signaled upon completion of unbind operation.
>> >
>> > Same here as above.
>> >
>> > > + */
>> > > +struct drm_i915_gem_vm_unbind {
>> > > + /** @vm_id: VM (address space) id to bind */
>> > > + __u32 vm_id;
>> > > +
>> > > + /** @rsvd: Reserved, MBZ */
>> > > + __u32 rsvd;
>> > > +
>> > > + /** @start: Virtual Address start to unbind */
>> > > + __u64 start;
>> > > +
>> > > + /** @length: Length of mapping to unbind */
>> > > + __u64 length;
>> > > +
>> > > + /**
>> > > + * @flags: Supported flags are:
>> > > + *
>> > > + * I915_GEM_VM_UNBIND_FENCE_VALID:
>> > > + * @fence is valid, needs unbind completion notification.
>> > > + */
>> > > + __u64 flags;
>> > > +#define I915_GEM_VM_UNBIND_FENCE_VALID (1 << 0)
>> > > +
>> > > + /**
>> > > + * @fence: Timeline fence for unbind completion signaling.
>> > > + *
>> > > + * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>> > > + * is invalid, and an error will be returned.
>> > > + */
>> > > + struct drm_i915_gem_timeline_fence fence;
>> > > +
>> > > + /** @extensions: 0-terminated chain of extensions */
>> > > + __u64 extensions;
>> > > +};
>> > > +
>> > > +/**
>> > > + * struct drm_i915_gem_execbuffer3 - Structure for DRM_I915_GEM_EXECBUFFER3
>> > > + * ioctl.
>> > > + *
>> > > + * DRM_I915_GEM_EXECBUFFER3 ioctl only works in VM_BIND mode and VM_BIND mode
>> > > + * only works with this ioctl for submission.
>> > > + * See I915_VM_CREATE_FLAGS_USE_VM_BIND.
>> > > + */
>> > > +struct drm_i915_gem_execbuffer3 {
>> > > + /**
>> > > + * @ctx_id: Context id
>> > > + *
>> > > + * Only contexts with user engine map are allowed.
>> > > + */
>> > > + __u32 ctx_id;
>> > > +
>> > > + /**
>> > > + * @engine_idx: Engine index
>> > > + *
>> > > + * An index in the user engine map of the context specified by @ctx_id.
>> > > + */
>> > > + __u32 engine_idx;
>> > > +
>> > > + /** @rsvd1: Reserved, MBZ */
>> > > + __u32 rsvd1;
>> > > +
>> > > + /**
>> > > + * @batch_count: Number of batches in @batch_address array.
>> > > + *
>> > > + * 0 is invalid. For parallel submission, it should be equal to the
>> > > + * number of (parallel) engines involved in that submission.
>> > > + */
>> > > + __u32 batch_count;
>> >
>> > I think this is a leftover from the old parallel engine design, where
>> > userspace stitched together the parallel batches? With the new design we
>> > have in execbuf2 the batch_count is fixed at context creation time (that's
>> > why document how that works is important), and so specifying this again in
>> > execbuf3 could only lead to bugs and hence the field here should be
>> > dropped.
>>
>> Ok, will remove.
>>
>> >
>> > > +
>> > > + /**
>> > > + * @batch_address: Array of batch gpu virtual addresses.
>> > > + *
>> > > + * If @batch_count is 1, then it is the gpu virtual address of the
>> > > + * batch buffer. If @batch_count > 1, then it is a pointer to an array
>> > > + * of batch buffer gpu virtual addresses.
>> >
>> > We derive the number of batchbuffers a context needs from
>> > ctx->parallel.num_children + 1. No need for userspace to supply this on
>> > every execbuf, but the rules about the address still are the same.
>> >
>>
>> My only worry is if the number of batch addresses provided is not equal
>> to 'ctx->parallel.num_children + 1', then we can get into harder to debug
>> GPU hangs (where as with batch_count, we can return an error). Besides,
>> without explicit batch_count here, treating this field differently when
>> one batch case vs multiple batch case makes the uapi look bit complicated.
>>
>> But, I guess, user can error up and get into harder to debug GPU hangs
>> even with batch_count. I think removing the batch_count and properly
>> documenting this batch_address field here should be good. Will update.
>
>Yeah userspace can submit garbage. Like they could also have an internal
>book-keep error in other ways.
>
>> > > + */
>> > > + __u64 batch_address;
>> > > +
>> > > + /**
>> > > + * @flags: Supported flags are:
>> > > + *
>> > > + * I915_EXEC3_SECURE:
>> > > + * Request a privileged ("secure") batch buffer/s.
>> > > + * It is only available for DRM_ROOT_ONLY | DRM_MASTER processes.
>> >
>> > Do we actually still need this for any userspace? Iirc the only user was
>> > SNA, which isn't a thing anymore since about gen9 or so. Or am I mistaken?
>> > A quick git grep doesn't show up anything.
>> >
>> > igt probably uses it for lots of fun stuff, might be time to remove it on
>> > the execbuf2 side too on modern platforms since it's kinda a risky thing
>> > to do (and flat out not a good idea on secure boot platforms even when
>> > you're root).
>> >
>>
>> Yah, looks like it is only used on very old platforms, will remove this
>> flag.
>
>Yay!
>
>> > > + */
>> > > + __u64 flags;
>> > > +#define I915_EXEC3_SECURE (1 << 0)
>> > > +
>> > > + /** @rsvd2: Reserved, MBZ */
>> > > + __u32 rsvd2;
>> > > +
>> > > + /** @fence_count: Number of fences in @timeline_fences array. */
>> > > + __u32 fence_count;
>> > > +
>> > > + /**
>> > > + * @timeline_fences: Pointer to an array of timeline fences.
>> > > + *
>> > > + * Timeline fences are of format struct drm_i915_gem_timeline_fence.
>> > > + */
>> > > + __u64 timeline_fences;
>> > > +
>> > > + /** @rsvd3: Reserved, MBZ */
>> > > + __u64 rsvd3;
>> > > +
>> > > + /** @extensions: Zero-terminated chain of extensions. */
>> >
>> > Maybe add "See struct i915_users_extension" here and make it clear that
>> > this is what this points to.
>> >
>>
>> Ok.
>>
>> > > + __u64 extensions;
>> > > +};
>> > > +
>> > > +/**
>> > > + * struct drm_i915_gem_create_ext_vm_private - Extension to make the object
>> > > + * private to the specified VM.
>> > > + *
>> > > + * See struct drm_i915_gem_create_ext.
>> > > + */
>> > > +struct drm_i915_gem_create_ext_vm_private {
>> > > +#define I915_GEM_CREATE_EXT_VM_PRIVATE 2
>> > > + /** @base: Extension link. See struct i915_user_extension. */
>> > > + struct i915_user_extension base;
>> > > +
>> > > + /** @vm_id: Id of the VM to which the object is private */
>> > > + __u32 vm_id;
>> > > +};
>> >
>> > With the nits addressed:
>> >
>> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Thanks,
>> Niranjana
>
>Ok I think we have consensus everywhere and it's really only minor stuff,
>so please include my r-b on the next round.
There are some comments form Mesa team. Will fix them and include r-b/s in
next revision.
Thanks,
Niranjana
>-Daniel
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/doc/rfc: i915 VM_BIND feature design + uapi
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
` (3 preceding siblings ...)
(?)
@ 2022-06-24 18:10 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2022-06-24 18:10 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx
== Series Details ==
Series: drm/doc/rfc: i915 VM_BIND feature design + uapi
URL : https://patchwork.freedesktop.org/series/105608/
State : warning
== Summary ==
Error: dim checkpatch failed
f158211cdc88 drm/doc/rfc: VM_BIND feature design document
-:17: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#17:
new file mode 100644
-:22: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#22: FILE: Documentation/gpu/rfc/i915_vm_bind.rst:1:
+==========================================
total: 0 errors, 2 warnings, 0 checks, 253 lines checked
a37662a9ed5f drm/i915: Update i915 uapi documentation
-:43: WARNING:NEW_TYPEDEFS: do not add new typedefs
#43: FILE: include/uapi/drm/i915_drm.h:774:
+typedef struct drm_i915_getparam drm_i915_getparam_t;
total: 0 errors, 1 warnings, 0 checks, 337 lines checked
3d5081e3cab0 drm/doc/rfc: VM_BIND uapi definition
Traceback (most recent call last):
File "scripts/spdxcheck.py", line 11, in <module>
import git
ModuleNotFoundError: No module named 'git'
-:22: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#22:
new file mode 100644
-:64: WARNING:LONG_LINE: line length of 126 exceeds 100 columns
#64: FILE: Documentation/gpu/rfc/i915_vm_bind.h:38:
+#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
-:65: WARNING:LONG_LINE: line length of 128 exceeds 100 columns
#65: FILE: Documentation/gpu/rfc/i915_vm_bind.h:39:
+#define DRM_IOCTL_I915_GEM_VM_UNBIND DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
-:66: WARNING:LONG_LINE: line length of 134 exceeds 100 columns
#66: FILE: Documentation/gpu/rfc/i915_vm_bind.h:40:
+#define DRM_IOCTL_I915_GEM_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
-:90: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#90: FILE: Documentation/gpu/rfc/i915_vm_bind.h:64:
+#define I915_TIMELINE_FENCE_WAIT (1<<0)
^
-:91: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#91: FILE: Documentation/gpu/rfc/i915_vm_bind.h:65:
+#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
^
total: 0 errors, 4 warnings, 2 checks, 286 lines checked
^ permalink raw reply [flat|nested] 23+ messages in thread* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/doc/rfc: i915 VM_BIND feature design + uapi
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
` (4 preceding siblings ...)
(?)
@ 2022-06-24 18:10 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2022-06-24 18:10 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx
== Series Details ==
Series: drm/doc/rfc: i915 VM_BIND feature design + uapi
URL : https://patchwork.freedesktop.org/series/105608/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 23+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/doc/rfc: i915 VM_BIND feature design + uapi
2022-06-24 17:49 ` [Intel-gfx] " Niranjana Vishwanathapura
` (5 preceding siblings ...)
(?)
@ 2022-06-24 18:31 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2022-06-24 18:31 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 8654 bytes --]
== Series Details ==
Series: drm/doc/rfc: i915 VM_BIND feature design + uapi
URL : https://patchwork.freedesktop.org/series/105608/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11804 -> Patchwork_105608v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_105608v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_105608v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/index.html
Participating hosts (40 -> 38)
------------------------------
Additional (1): bat-adlm-1
Missing (3): fi-bdw-samus fi-rkl-11600 bat-adlp-4
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_105608v1:
### IGT changes ###
#### Possible regressions ####
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
- fi-bsw-kefka: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@core_hotunplug@unbind-rebind:
- {bat-adln-1}: [PASS][3] -> [DMESG-WARN][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/bat-adln-1/igt@core_hotunplug@unbind-rebind.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/bat-adln-1/igt@core_hotunplug@unbind-rebind.html
* igt@i915_selftest@live@hangcheck:
- {bat-adlm-1}: NOTRUN -> [INCOMPLETE][5]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/bat-adlm-1/igt@i915_selftest@live@hangcheck.html
* igt@kms_pipe_crc_basic@hang-read-crc:
- {bat-adlm-1}: NOTRUN -> [SKIP][6] +8 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/bat-adlm-1/igt@kms_pipe_crc_basic@hang-read-crc.html
Known issues
------------
Here are the changes found in Patchwork_105608v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@hangcheck:
- fi-bdw-5557u: NOTRUN -> [INCOMPLETE][7] ([i915#3921])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html
- fi-snb-2600: [PASS][8] -> [INCOMPLETE][9] ([i915#3921])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
* igt@i915_selftest@live@requests:
- fi-pnv-d510: [PASS][10] -> [DMESG-FAIL][11] ([i915#4528])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-pnv-d510/igt@i915_selftest@live@requests.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-pnv-d510/igt@i915_selftest@live@requests.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-hsw-4770: NOTRUN -> [SKIP][12] ([fdo#109271] / [fdo#111827])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html
* igt@kms_flip@basic-plain-flip@a-edp1:
- fi-tgl-u2: [PASS][13] -> [DMESG-WARN][14] ([i915#402]) +2 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html
#### Possible fixes ####
* igt@i915_selftest@live@gtt:
- fi-bdw-5557u: [INCOMPLETE][15] ([i915#5685]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-bdw-5557u/igt@i915_selftest@live@gtt.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-bdw-5557u/igt@i915_selftest@live@gtt.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][17] ([i915#3303] / [i915#4785]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
- {fi-ehl-2}: [INCOMPLETE][19] ([i915#5153]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-ehl-2/igt@i915_selftest@live@hangcheck.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-ehl-2/igt@i915_selftest@live@hangcheck.html
- bat-dg1-5: [DMESG-FAIL][21] ([i915#4494] / [i915#4957]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
* igt@kms_flip@basic-flip-vs-modeset@a-edp1:
- fi-tgl-u2: [DMESG-WARN][23] ([i915#402]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
- {bat-adlp-6}: [DMESG-WARN][25] ([i915#3576]) -> [PASS][26] +1 similar issue
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11804/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
[i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
[i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
[i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153
[i915#5685]: https://gitlab.freedesktop.org/drm/intel/issues/5685
[i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903
Build changes
-------------
* Linux: CI_DRM_11804 -> Patchwork_105608v1
CI-20190529: 20190529
CI_DRM_11804: e4f05a1d08f5dd4e171ad633cf73208c41f62b2c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6542: d38a476ee4b9f9a95d8f452de0d66cc52f7f079b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_105608v1: e4f05a1d08f5dd4e171ad633cf73208c41f62b2c @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
a0e43ff2c053 drm/doc/rfc: VM_BIND uapi definition
e73504e1b5e7 drm/i915: Update i915 uapi documentation
b29e179cae8b drm/doc/rfc: VM_BIND feature design document
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105608v1/index.html
[-- Attachment #2: Type: text/html, Size: 8905 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread