[Mesa-dev] [RFC PATCH 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan
Add entry for i915 new parallel submission uAPI plan.
v2:
(Daniel Vetter):
- Expand logical order explaination
- Add dummy header
- Only allow N BBs in execbuf IOCTL
- Configure parallel submission per slot not per gem context
v3:
(Marcin Ślusarz):
- Lot's of typos / bad english fixed
(Tvrtko Ursulin):
- Consistent pseudo code, clean up wording in descriptions
Cc: Tvrtko Ursulin
Cc: Tony Ye
CC: Carl Zhang
Cc: Daniel Vetter
Cc: Jason Ekstrand
Signed-off-by: Matthew Brost
---
Documentation/gpu/rfc/i915_parallel_execbuf.h | 145 ++
Documentation/gpu/rfc/i915_scheduler.rst | 55 ++-
2 files changed, 198 insertions(+), 2 deletions(-)
create mode 100644 Documentation/gpu/rfc/i915_parallel_execbuf.h
diff --git a/Documentation/gpu/rfc/i915_parallel_execbuf.h
b/Documentation/gpu/rfc/i915_parallel_execbuf.h
new file mode 100644
index ..20de206e3ab4
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
@@ -0,0 +1,145 @@
+#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see
i915_context_engines_parallel_submit */
+
+/*
+ * i915_context_engines_parallel_submit:
+ *
+ * Setup a slot in the context engine map to allow multiple BBs to be submitted
+ * in a single execbuf IOCTL. Those BBs will then be scheduled to run on the
GPU
+ * in parallel. Multiple hardware contexts are created internally in the i915
+ * run these BBs. Once a slot is configured for N BBs only N BBs can be
+ * submitted in each execbuf IOCTL and this is implicit behavior e.g. The user
+ * doesn't tell the execbuf IOCTL there are N BBs, the execbuf IOCTL know how
+ * many BBs there are based on the slots configuration. The N BBs are the last
N
+ * buffer objects for first N if I915_EXEC_BATCH_FIRST is set.
+ *
+ * There are two currently defined ways to control the placement of the
+ * hardware contexts on physical engines: default behavior (no flags) and
+ * I915_PARALLEL_IMPLICIT_BONDS (a flag). More flags may be added the in the
+ * future as new hardware / use cases arise. Details of how to use this
+ * interface above the flags field in this structure.
+ *
+ * Returns -EINVAL if hardware context placement configuration is invalid or if
+ * the placement configuration isn't supported on the platform / submission
+ * interface.
+ * Returns -ENODEV if extension isn't supported on the platform / submission
+ * inteface.
+ */
+struct i915_context_engines_parallel_submit {
+ struct i915_user_extension base;
+
+ __u16 engine_index; /* slot for parallel engine */
+ __u16 width;/* number of contexts per parallel engine */
+ __u16 num_siblings; /* number of siblings per context */
+ __u16 mbz16;
+/*
+ * Default placement behavior (currently unsupported):
+ *
+ * Allow BBs to be placed on any available engine instance. In this case each
+ * context's engine mask indicates where that context can be placed. It is
+ * implied in this mode that all contexts have mutual exclusive placement.
+ * e.g. If one context is running CSX[0] no other contexts can run on CSX[0]).
+ *
+ * Example 1 pseudo code:
+ * CSX,Y[N] = generic engine class X or Y, logical instance N
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=2,
+ * engines=CSX[0],CSX[1],CSY[0],CSY[1])
+ *
+ * Results in the following valid placements:
+ * CSX[0], CSY[0]
+ * CSX[0], CSY[1]
+ * CSX[1], CSY[0]
+ * CSX[1], CSY[1]
+ *
+ * This can also be thought of as 2 virtual engines described by 2-D array in
+ * the engines the field:
+ * VE[0] = CSX[0], CSX[1]
+ * VE[1] = CSY[0], CSY[1]
+ *
+ * Example 2 pseudo code:
+ * CSX[Y] = generic engine of same class X, logical instance N
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=3,
+ * engines=CSX[0],CSX[1],CSX[2],CSX[0],CSX[1],CSX[2])
+ *
+ * Results in the following valid placements:
+ * CSX[0], CSX[1]
+ * CSX[0], CSX[2]
+ * CSX[1], CSX[0]
+ * CSX[1], CSX[2]
+ * CSX[2], CSX[0]
+ * CSX[2], CSX[1]
+ *
+ * This can also be thought of as 2 virtual engines described by 2-D array in
+ * the engines the field:
+ * VE[0] = CSX[0], CSX[1], CSX[2]
+ * VE[1] = CSX[0], CSX[1], CSX[2]
+
+ * This enables a use case where all engines are created equally, we don't care
+ * where they are scheduled, we just want a certain number of resources, for
+ * those resources to be scheduled in parallel, and possibly across multiple
+ * engine classes.
+ */
+
+/*
+ * I915_PARALLEL_IMPLICIT_BONDS - Create implicit bonds between each context.
+ * Each context must have the same number of sibling and bonds are implicitly
+ * created between each set of siblings.
+ *
+ * Example 1 pseudo code:
+ * CSX[N] = generic engine of same class X, logical instance N
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ *
[Mesa-dev] [RFC PATCH 0/2] GuC submission / DRM scheduler integration plan + new uAPI
Subject and patches say it all. v2: Address comments, patches have details of changes v3: Address comments, patches have details of changes Signed-off-by: Matthew Brost Matthew Brost (2): drm/doc/rfc: i915 GuC submission / DRM scheduler drm/doc/rfc: i915 new parallel submission uAPI plan Documentation/gpu/rfc/i915_parallel_execbuf.h | 145 ++ Documentation/gpu/rfc/i915_scheduler.rst | 136 Documentation/gpu/rfc/index.rst | 4 + 3 files changed, 285 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_parallel_execbuf.h create mode 100644 Documentation/gpu/rfc/i915_scheduler.rst -- 2.28.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 1/2] drm/doc/rfc: i915 GuC submission / DRM scheduler
Add entry for i915 GuC submission / DRM scheduler integration plan. Follow up patch with details of new parallel submission uAPI to come. v2: (Daniel Vetter) - Expand explaination of why bonding isn't supported for GuC submission - CC some of the DRM scheduler maintainers - Add priority inheritance / boosting use case - Add reasoning for removing in order assumptions (Daniel Stone) - Add links to priority spec Cc: Christian König Cc: Luben Tuikov Cc: Alex Deucher Cc: Steven Price Cc: Jon Bloomfield Cc: Jason Ekstrand Cc: Dave Airlie Cc: Daniel Vetter Cc: Jason Ekstrand Cc: dri-de...@lists.freedesktop.org Signed-off-by: Matthew Brost --- Documentation/gpu/rfc/i915_scheduler.rst | 85 Documentation/gpu/rfc/index.rst | 4 ++ 2 files changed, 89 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_scheduler.rst diff --git a/Documentation/gpu/rfc/i915_scheduler.rst b/Documentation/gpu/rfc/i915_scheduler.rst new file mode 100644 index ..7faa46cde088 --- /dev/null +++ b/Documentation/gpu/rfc/i915_scheduler.rst @@ -0,0 +1,85 @@ += +I915 GuC Submission/DRM Scheduler Section += + +Upstream plan += +For upstream the overall plan for landing GuC submission and integrating the +i915 with the DRM scheduler is: + +* Merge basic GuC submission + * Basic submission support for all gen11+ platforms + * Not enabled by default on any current platforms but can be enabled via + modparam enable_guc + * Lots of rework will need to be done to integrate with DRM scheduler so + no need to nit pick everything in the code, it just should be + functional, no major coding style / layering errors, and not regress + execlists + * Update IGTs / selftests as needed to work with GuC submission + * Enable CI on supported platforms for a baseline + * Rework / get CI heathly for GuC submission in place as needed +* Merge new parallel submission uAPI + * Bonding uAPI completely incompatible with GuC submission, plus it has + severe design issues in general, which is why we want to retire it no + matter what + * New uAPI adds I915_CONTEXT_ENGINES_EXT_PARALLEL context setup step + which configures a slot with N contexts + * After I915_CONTEXT_ENGINES_EXT_PARALLEL a user can submit N batches to + a slot in a single execbuf IOCTL and the batches run on the GPU in + paralllel + * Initially only for GuC submission but execlists can be supported if + needed +* Convert the i915 to use the DRM scheduler + * GuC submission backend fully integrated with DRM scheduler + * All request queues removed from backend (e.g. all backpressure + handled in DRM scheduler) + * Resets / cancels hook in DRM scheduler + * Watchdog hooks into DRM scheduler + * Lots of complexity of the GuC backend can be pulled out once + integrated with DRM scheduler (e.g. state machine gets + simplier, locking gets simplier, etc...) + * Execlist backend will do the minimum required to hook in the DRM + scheduler so it can live next to the fully integrated GuC backend + * Legacy interface + * Features like timeslicing / preemption / virtual engines would + be difficult to integrate with the DRM scheduler and these + features are not required for GuC submission as the GuC does + these things for us + * ROI low on fully integrating into DRM scheduler + * Fully integrating would add lots of complexity to DRM + scheduler + * Port i915 priority inheritance / boosting feature in DRM scheduler + * Used for i915 page flip, may be useful to other DRM drivers as + well + * Will be an optional feature in the DRM scheduler + * Remove in-order completion assumptions from DRM scheduler + * Even when using the DRM scheduler the backends will handle + preemption, timeslicing, etc... so it is possible for jobs to + finish out of order + * Pull out i915 priority levels and use DRM priority levels + * Optimize DRM scheduler as needed + +New uAPI for basic GuC submission += +No major changes are required to the uAPI for basic GuC submission. The only +change is a new scheduler attribute: I915_SCHEDULER_CAP_STATIC_PRIORITY_MAP. +This attribute indicates the 2k i915 user priority levels are statically mapped +into 3 levels as follows: + +* -1k to -1 Low priority +* 0 Medium priority +* 1 to 1k High priority + +This is needed because the GuC only has 4 priority bands. The highest priority +band is reserved with the
Re: [Mesa-dev] [PATCH 01/11] drm/amdgpu: Comply with implicit fencing rules
On Wed, May 26, 2021 at 3:32 PM Christian König
wrote:
>
> Am 25.05.21 um 17:23 schrieb Daniel Vetter:
> > On Tue, May 25, 2021 at 5:05 PM Christian König
> > wrote:
> >> Hi Daniel,
> >>
> >> Am 25.05.21 um 15:05 schrieb Daniel Vetter:
> >>> Hi Christian,
> >>>
> >>> On Sat, May 22, 2021 at 10:30:19AM +0200, Christian König wrote:
> Am 21.05.21 um 20:31 schrieb Daniel Vetter:
> This works by adding the fence of the last eviction DMA operation to BOs
> when their backing store is newly allocated. That's what the
> ttm_bo_add_move_fence() function you stumbled over is good for:
> https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/gpu/drm/ttm/ttm_bo.c#L692
>
> Now the problem is it is possible that the application is terminated
> before
> it can complete it's command submission. But since resource management
> only
> waits for the shared fences when there are some there is a chance that we
> free up memory while it is still in use.
> >>> Hm where is this code? Would help in my audit that I wanted to do this
> >>> week? If you look at most other places like
> >>> drm_gem_fence_array_add_implicit() I mentioned earlier, then we don't
> >>> treat the shared fences special and always also include the exclusive one.
> >> See amdgpu_gem_object_close():
> >>
> >> ...
> >> fence = dma_resv_get_excl(bo->tbo.base.resv);
> >> if (fence) {
> >> amdgpu_bo_fence(bo, fence, true);
> >> fence = NULL;
> >> }
> >> ...
> >>
> >> We explicitly added that because resource management of some other
> >> driver was going totally bananas without that.
> >>
> >> But I'm not sure which one that was. Maybe dig a bit in the git and
> >> mailing history of that.
> > Hm I looked and it's
> >
> > commit 82c416b13cb7d22b96ec0888b296a48dff8a09eb
> > Author: Christian König
> > Date: Thu Mar 12 12:03:34 2020 +0100
> >
> > drm/amdgpu: fix and cleanup amdgpu_gem_object_close v4
> >
> > That sounded more like amdgpu itself needing this, not another driver?
>
> No, that patch was just a follow up moving the functionality around.
That patch added the "add exclusive fence to shared slots before
amdgpu_vm_clear_freed() call", which I thought was at least part of
your fix.
> > But looking at amdgpu_vm_bo_update_mapping() it seems to pick the
> > right fencing mode for gpu pte clearing, so I'm really not sure what
> > the bug was that you worked around here?The implementation boils down
> > to amdgpu_sync_resv() which syncs for the exclusive fence, always. And
> > there's nothing else that I could find in public history at least, no
> > references to bug reports or anything. I think you need to dig
> > internally, because as-is I'm not seeing the problem here.
> >
> > Or am I missing something here?
>
> See the code here for example:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nouveau_fence.c#L361
>
> Nouveau assumes that when a shared fence is present it doesn't need to
> wait for the exclusive one because the shared are always supposed to
> finish after the exclusive one.
>
> But for page table unmap fences that isn't true and we ran into a really
> nasty and hard to reproduce bug because of this.
>
> I think it would be much more defensive if we could say that we always
> wait for the exclusive fence and fix the use case in nouveau and double
> check if somebody else does stuff like that as well.
Yeah most other drivers do the defensive thing here. I think it would
be good to standardize on that. I'll add that to my list and do more
auditing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/11] drm/amdgpu: Comply with implicit fencing rules
Am 25.05.21 um 17:23 schrieb Daniel Vetter:
On Tue, May 25, 2021 at 5:05 PM Christian König
wrote:
Hi Daniel,
Am 25.05.21 um 15:05 schrieb Daniel Vetter:
Hi Christian,
On Sat, May 22, 2021 at 10:30:19AM +0200, Christian König wrote:
Am 21.05.21 um 20:31 schrieb Daniel Vetter:
This works by adding the fence of the last eviction DMA operation to BOs
when their backing store is newly allocated. That's what the
ttm_bo_add_move_fence() function you stumbled over is good for:
https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/gpu/drm/ttm/ttm_bo.c#L692
Now the problem is it is possible that the application is terminated before
it can complete it's command submission. But since resource management only
waits for the shared fences when there are some there is a chance that we
free up memory while it is still in use.
Hm where is this code? Would help in my audit that I wanted to do this
week? If you look at most other places like
drm_gem_fence_array_add_implicit() I mentioned earlier, then we don't
treat the shared fences special and always also include the exclusive one.
See amdgpu_gem_object_close():
...
fence = dma_resv_get_excl(bo->tbo.base.resv);
if (fence) {
amdgpu_bo_fence(bo, fence, true);
fence = NULL;
}
...
We explicitly added that because resource management of some other
driver was going totally bananas without that.
But I'm not sure which one that was. Maybe dig a bit in the git and
mailing history of that.
Hm I looked and it's
commit 82c416b13cb7d22b96ec0888b296a48dff8a09eb
Author: Christian König
Date: Thu Mar 12 12:03:34 2020 +0100
drm/amdgpu: fix and cleanup amdgpu_gem_object_close v4
That sounded more like amdgpu itself needing this, not another driver?
No, that patch was just a follow up moving the functionality around.
But looking at amdgpu_vm_bo_update_mapping() it seems to pick the
right fencing mode for gpu pte clearing, so I'm really not sure what
the bug was that you worked around here?The implementation boils down
to amdgpu_sync_resv() which syncs for the exclusive fence, always. And
there's nothing else that I could find in public history at least, no
references to bug reports or anything. I think you need to dig
internally, because as-is I'm not seeing the problem here.
Or am I missing something here?
See the code here for example:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nouveau_fence.c#L361
Nouveau assumes that when a shared fence is present it doesn't need to
wait for the exclusive one because the shared are always supposed to
finish after the exclusive one.
But for page table unmap fences that isn't true and we ran into a really
nasty and hard to reproduce bug because of this.
I think it would be much more defensive if we could say that we always
wait for the exclusive fence and fix the use case in nouveau and double
check if somebody else does stuff like that as well.
Christian.
-Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
