Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-24 Thread Tvrtko Ursulin



On 21/05/2021 17:48, Matthew Brost wrote:

On Fri, May 21, 2021 at 01:00:54PM +0100, Tvrtko Ursulin wrote:


[snip]


+ * enables parallel submission across multiple engine classes. In this case 
each
+ * context's logical engine mask indicates where that context can placed. It is
+ * implied in this mode that all contexts have mutual exclusive placement (e.g.
+ * if one context is running CS0 no other contexts can run on CS0).


I think talk about logical context and its mask is too implementation detail
at the uapi level. Instead I would suggest more userspace programmer centric
description.


Ok, can you give me suggestion? Writing DOC isn't really my strength.


Yeah, not mine either. Maybe we need to hire a technical writer. :)

I think in general I would just talk a bit how until now submission was 
along a single engine only and this is adding a wide submission model, 
expanding how it works with more details and only then talking about 
logical contexts if needed.


It depends a bit whether our userspace clients still predominantly think 
in terms of engines, or is it contexts? I don't have an answer there.


It probably isn't the most important thing and probably with a few 
tweaks of what you have it can be good enough. Key probably is simply 
coming up with as intuitive as possible diagrams, with consistent 
naming, showing how the wide engine is built and how it works.



+ *
+ * Example 1 pseudo code:
+ * CSX[Y] = engine class X, logical instance Y
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=2,
+ * engines=CS0[0],CS0[1],CS1[0],CS1[1])
+ *
+ * Results in the following valid placements:
+ * CS0[0], CS1[0]
+ * CS0[0], CS1[1]
+ * CS0[1], CS1[0]
+ * CS0[1], CS1[1]
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS0[0], CS0[1]
+ * VE[1] = CS1[0], CS1[1]


Ah okay so essentially similar to what I was proposing a year ago. But then
it is no longer "set_parallel" really. It is one slot in the engine map,
right, with the idea to super class intel_context in the implementation?



Yes, it is bascially a super class intel_context. In the implementation is
parent-child with the parent having a linked list of child intel_contexts.
  

So really a wide virtual engine, as opposed to single one. In which case I
think it makes sense to stay close to the existing naming of the
load_balance extension for consistency. Load_balance_wide?
Load_balance_parallel? Multi?


I like the way is named but I also don't want to argue about this as I don't
really care. If someone else says this should be renamed, let's do it.


I don't care too much apart from a general desire for more consistency 
and fewer terms in use.



I also have to say the notation "CS0[0]" - I who know this problem space am
finding it hard to penetrate what that actually means. (Also uppercase IMO
makes it hard to read, but maybe it is just me.)



Yea, now I think about it CS0[0] is bad because of using numbers twice. How
about CSX[0] & CSY[1]? I used upper case because in the i915 all engine classes
defines are upper case but agree it might be easier to read it lower case.


What would X and Y represent? Or if I paste this part:

 * VE[0] = CS0[0], CS0[1]
 * VE[1] = CS1[0], CS1[1]

Which index is engine instance and what is the other index?


Looking a bit lower below, extension seems to be taking a 2d array of
class:instance pairs, right? If so then reading these docs in order, or even
just looking further down, I don't think that is explicitly called out
clearly enough.

So I think a paragraph or two explaining clearly how the 2d array of engines
corresponds to the allowed engines for full virtual engine width. Or maybe
just a 2d diagram?

   2-wide virtual engine:
 .engines = [
   /* channel 0 allowed engines: */  [cs0, cs1],
   /* channel 1 allowed engines: */  [cs0, cs1]
  ]

Not sure if that's better.



Yes, it is a 2-d array. Agree the explaination could be better.


Also to be noted, this only allows uniform number of allowed engines per
channel. I am not saying we need the non-uniform setup today but with bonds
there isn't this limitation.



Not exactly. You could do something like this.

witdth = 2
siblings = 2
engines = CSX[0], CSX[1], CSY[0], INVALID

This would allow a placement of:

CSX[0], CSY[0]
CSX[1], CSY[0]

In this case the siblings is just a max value of each entry.


Okay fair, did not think about that or saw it mentioned.

Regards,

Tvrtko
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Matthew Brost
On Fri, May 21, 2021 at 01:00:54PM +0100, Tvrtko Ursulin wrote:
> 
> On 19/05/2021 00:58, Matthew Brost wrote:
> > Add entry fpr 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
> > 
> > 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 | 144 ++
> >   Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> >   2 files changed, 196 insertions(+), 1 deletion(-)
> >   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 ..8c64b983ccad
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > @@ -0,0 +1,144 @@
> > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > i915_context_engines_parallel_submit */
> > +
> > +/*
> > + * i915_context_engines_parallel_submit:
> > + *
> > + * Setup a slot 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 implict 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).
> 
> 1)
> Expand the term slot here with "slot in the context engine map" least once
> for clarity.
> 

Sure.

> 2)
> About where execbuf will implicitly be finding batches - suggest to also
> cover first/last flag here. I know you have it in the readme but I think it
> is good if uapi header is as self-contained as possible.
>

Yep, good idea.

> > + *
> > + * Their are two currently defined ways to control the placement of the
> > + * hardware contexts on physical engines: default behavior (no flags) and
> > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > + *
> > + * Returns -EINVAL if hardware context placement configuration 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 behvavior (currently unsupported):
> > + *
> > + * Rather than restricting parallel submission to a single class with a
> > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a 
> > mode that
> 
> What do you mean with logically contiguous here? It sounds ambiguous versus
> logical vs "normal" engine instance numbers.
>

This is a little backwards. I think when I wrote this originally the implicit
placement comments were first. I can reword this.
 
> > + * enables parallel submission across multiple engine classes. In this 
> > case each
> > + * context's logical engine mask indicates where that context can placed. 
> > It is
> > + * implied in this mode that all contexts have mutual exclusive placement 
> > (e.g.
> > + * if one context is running CS0 no other contexts can run on CS0).
> 
> I think talk about logical context and its mask is too implementation detail
> at the uapi level. Instead I would suggest more userspace programmer centric
> description.

Ok, can you give me suggestion? Writing DOC isn't really my strength.

> 
> > + *
> > + * Example 1 pseudo code:
> > + * CSX[Y] = engine class X, logical instance Y
> > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > + * set_engines(INVALID)
> > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])
> > + *
> > + * Results in the following valid placements:
> > + * CS0[0], CS1[0]
> > + * CS0[0], CS1[1]
> > + * CS0[1], CS1[0]
> > + * CS0[1], CS1[1]
> > + *
> > + * This can also be though of as 2 virtual engines:
> > + * VE[0] = CS0[0], CS0[1]
> > + * VE[1] = CS1[0], CS1[1]
> 
> Ah okay so essentially similar to what I was proposing a year ago. But then
> it is no longer "set_parallel" really. It is one slot in th

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Matthew Brost
On Thu, May 20, 2021 at 09:41:20PM +0200, Daniel Vetter wrote:
> On Thu, May 20, 2021 at 11:57:44AM +0100, Tvrtko Ursulin wrote:
> > 
> > On 20/05/2021 10:54, Daniel Vetter wrote:
> > > On Wed, May 19, 2021 at 7:19 PM Matthew Brost  
> > > wrote:
> > > > 
> > > > On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> > > > > On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > > > > > Add entry fpr 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
> > > > > > 
> > > > > > 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 | 144 
> > > > > > ++
> > > > > >   Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > > > > >   2 files changed, 196 insertions(+), 1 deletion(-)
> > > > > >   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 ..8c64b983ccad
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > > > > @@ -0,0 +1,144 @@
> > > > > > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > > > > > i915_context_engines_parallel_submit */
> > > > > > +
> > > > > > +/*
> > > > > > + * i915_context_engines_parallel_submit:
> > > > > > + *
> > > > > > + * Setup a slot 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 implict 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).
> > > > > > + *
> > > > > > + * Their are two currently defined ways to control the placement 
> > > > > > of the
> > > > > > + * hardware contexts on physical engines: default behavior (no 
> > > > > > flags) and
> > > > > > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > > > > > + *
> > > > > > + * Returns -EINVAL if hardware context placement configuration 
> > > > > > 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;
> > > > > 
> > > > > Ok the big picture looks reasonable now, the flags still confuse me.
> > > > > 
> > > > 
> > > > Yea, it is a bit confusing.
> > > > 
> > > > > > +/*
> > > > > > + * Default placement behvavior (currently unsupported):
> > > > > > + *
> > > > > > + * Rather than restricting parallel submission to a single class 
> > > > > > with a
> > > > > > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), 
> > > > > > add a mode that
> > > > > > + * enables parallel submission across multiple engine classes. In 
> > > > > > this case each
> > > > > > + * context's logical engine mask indicates where that context can 
> > > > > > placed. It is
> > > > > > + * implied in this mode that all contexts have mutual exclusive 
> > > > > > placement (e.g.
> > > > > > + * if one context is running CS0 no other contexts can run on CS0).
> > > > > > + *
> > > > > > + * Example 1 pseudo code:
> > > > > > + * CSX[Y] = engine class X, logical instance Y
> > > > > > + * INVALID = I915_ENGINE_CLASS_INVALID, 
> > > > > > I915_ENGINE_CLASS_INVALID_NONE
> > > > > > + * set_engines(INVALID)
> > > > > > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > > > > > + * engines=CS0[0],CS0[1],CS1[0],CS1[1

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Matthew Brost
On Thu, May 20, 2021 at 09:44:59PM +0200, Daniel Vetter wrote:
> On Thu, May 20, 2021 at 08:10:59AM -0700, Matthew Brost wrote:
> > On Thu, May 20, 2021 at 11:54:25AM +0200, Daniel Vetter wrote:
> > > On Wed, May 19, 2021 at 7:19 PM Matthew Brost  
> > > wrote:
> > > >
> > > > On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> > > > > On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > > > > > Add entry fpr 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
> > > > > >
> > > > > > 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 | 144 
> > > > > > ++
> > > > > >  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > > > > >  2 files changed, 196 insertions(+), 1 deletion(-)
> > > > > >  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 ..8c64b983ccad
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > > > > @@ -0,0 +1,144 @@
> > > > > > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > > > > > i915_context_engines_parallel_submit */
> > > > > > +
> > > > > > +/*
> > > > > > + * i915_context_engines_parallel_submit:
> > > > > > + *
> > > > > > + * Setup a slot 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 implict 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).
> > > > > > + *
> > > > > > + * Their are two currently defined ways to control the placement 
> > > > > > of the
> > > > > > + * hardware contexts on physical engines: default behavior (no 
> > > > > > flags) and
> > > > > > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > > > > > + *
> > > > > > + * Returns -EINVAL if hardware context placement configuration 
> > > > > > 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;
> > > > >
> > > > > Ok the big picture looks reasonable now, the flags still confuse me.
> > > > >
> > > >
> > > > Yea, it is a bit confusing.
> > > >
> > > > > > +/*
> > > > > > + * Default placement behvavior (currently unsupported):
> > > > > > + *
> > > > > > + * Rather than restricting parallel submission to a single class 
> > > > > > with a
> > > > > > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), 
> > > > > > add a mode that
> > > > > > + * enables parallel submission across multiple engine classes. In 
> > > > > > this case each
> > > > > > + * context's logical engine mask indicates where that context can 
> > > > > > placed. It is
> > > > > > + * implied in this mode that all contexts have mutual exclusive 
> > > > > > placement (e.g.
> > > > > > + * if one context is running CS0 no other contexts can run on CS0).
> > > > > > + *
> > > > > > + * Example 1 pseudo code:
> > > > > > + * CSX[Y] = engine class X, logical instance Y
> > > > > > + * INVALID = I915_ENGINE_CLASS_INVALID, 
> > > > > > I915_ENGINE_CLASS_INVALID_NONE
> > > > > > + * set_engines(INVALID)
> > > > > > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > > > > > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Matthew Brost
On Fri, May 21, 2021 at 10:35:37AM +0200, Christian König wrote:
> Am 20.05.21 um 23:38 schrieb Jason Ekstrand:
> > On Thu, May 20, 2021 at 10:46 AM Matthew Brost  
> > wrote:
> > > On Thu, May 20, 2021 at 01:11:59PM +0200, Christian König wrote:
> > > > Am 19.05.21 um 18:51 schrieb Matthew Brost:
> > > > > On Wed, May 19, 2021 at 01:45:39PM +0200, Christian König wrote:
> > > > > > Oh, yeah we call that gang submit on the AMD side.
> > > > > > 
> > > > > > Had already some internal discussions how to implement this, but so 
> > > > > > far
> > > > > > couldn't figure out how to cleanly introduce that into the DRM 
> > > > > > scheduler.
> > > > > > 
> > > > > > Can you briefly describe in a few words how that is supposed to 
> > > > > > work on the
> > > > > > Intel side?
> > On Intel, we actually have two cases which don't fit the current
> > drm/scheduler model well: balanced and bonded.
> > 
> > In the balanced model, we want to submit a batch which can go to any
> > one of some set of engines and we don't care which.  It's up to the
> > kernel to pick an engine.  Imagine you had 64 identical HW compute
> > queues, for instance.  This could be done by making all the identical
> > engines share a single drm_gpu_scheduler and round-robin around the HW
> > queues or something.  I don't know that we strictly need drm/scheduler
> > to be aware of it but it might be nice if it grew support for this
> > mode so we could maintain a 1:1 relationship between HW queues and
> > drm_gpu_schedulers.  That said, I'm not sure how this would play with
> > GuC queues so maybe it doesn't help?
> 
> Oh, we do have support for load balancing like that.
> 
> When you call drm_sched_entity_init() you can give a list of
> drm_gpu_scheduler object to use round robing for scheduling.
> 
> New jobs are then scheduler to the drm_gpu_scheduler instance which is idle
> or rather the least busy one.
> 
> > The bonded model is like your ganged, I think.  We want to submit N
> > batches to run in parallel.  And they actually have to be executing on
> > the GPU simultaneously and not just sort-of at similar times.  We need
> > this for video.  There are also potential use-cases in Vulkan or even
> > GL that might be able to use this.  One difference with the balanced
> > mode is that bonds don't, strictly speaking, need to be on the same
> > type of engine.  Imagine, for instance, a 3D batch with a parallel
> > compute batch doing vertex pre-processing.
> > 
> > I'm pretty sure the bonded case is something that the mobile drivers
> > (panfrost, etc.) would like as well for doing Vulkan on tilers where
> > you often have to have two command buffers running in parallel.
> > They're currently doing it by submitting a giant pile of batches where
> > they split the batch and add sync primitives every time some GL call
> > requires them to sync between fragment and vertex pipes.
> 
> Yeah, we have exactly the same problem as well.
> 
> But so far every model we discussed has some drawbacks and it is rather hard
> for the scheduler to guarantee that stuff runs at the same time.
> 
> So if you got any ideas how to cleanly implement that then they would be
> rather welcomed.
> 

Everything Jason said about our submission modes is correct for execlists, we
have balanced + bonded models which is tightly coupled with that backend.

Fortunately with GuC submission most of this complexity goes away as the GuC
handles this for us. e.g. For balanced when we register a context we just give
it a mask of which physical engines a context is allowed to run on. For parallel
we register N contexts in a single command with the placement information +
submit to all the contexts with a command which moves the tails in LRCs for us.
We really don't need to bake any of this into the DRM scheduler for GuC
submission. 

Execlists is different story but I think our plan is to do the minimum possible
to plum that into the DRM scheduler (e.g. leave the balanced / bonded code in
the backend). Could we update the DRM scheduler to understand balanced (seems
like already does to some extent) and bonded? Yes we could but IMO the ROI on
that is low for Intel. The DRM scheduler is quite clean at the moment compared
to our near incomprehensible execlist backend. Execlists are basically a legacy
interface so pushing features which are only required for that backend to the
DRM scheduler doesn't make sense to me. That being said, if this is something
AMD needs in the DRM scheduler of course we can support you.

Matt

> Regards,
> Christian.
> 
> > 
> > So, to sum up, I think there's likely some good collaboration to be
> > had here for everyone. :-)
> > 
> > --Jason
> > 
> > > > > Sure, I've done a quick PoC internally and have been able to hook this
> > > > > into the DRM scheduler.
> > > > > 
> > > > > Basically each BB still maps to a single job as each job is somewhat
> > > > > unique (e.g. each job has its own ring, lrc, seqno, etc...). However 
> > > > > all
> > > > > the j

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Tvrtko Ursulin



On 20/05/2021 20:41, Daniel Vetter wrote:

On Thu, May 20, 2021 at 11:57:44AM +0100, Tvrtko Ursulin wrote:


On 20/05/2021 10:54, Daniel Vetter wrote:

On Wed, May 19, 2021 at 7:19 PM Matthew Brost  wrote:


On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:

On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:

Add entry fpr 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

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 | 144 ++
   Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
   2 files changed, 196 insertions(+), 1 deletion(-)
   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 ..8c64b983ccad
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
@@ -0,0 +1,144 @@
+#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
i915_context_engines_parallel_submit */
+
+/*
+ * i915_context_engines_parallel_submit:
+ *
+ * Setup a slot 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 implict 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).
+ *
+ * Their are two currently defined ways to control the placement of the
+ * hardware contexts on physical engines: default behavior (no flags) and
+ * I915_PARALLEL_IMPLICT_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 below above the flags.
+ *
+ * Returns -EINVAL if hardware context placement configuration 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;


Ok the big picture looks reasonable now, the flags still confuse me.



Yea, it is a bit confusing.


+/*
+ * Default placement behvavior (currently unsupported):
+ *
+ * Rather than restricting parallel submission to a single class with a
+ * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a mode 
that
+ * enables parallel submission across multiple engine classes. In this case 
each
+ * context's logical engine mask indicates where that context can placed. It is
+ * implied in this mode that all contexts have mutual exclusive placement (e.g.
+ * if one context is running CS0 no other contexts can run on CS0).
+ *
+ * Example 1 pseudo code:
+ * CSX[Y] = engine class X, logical instance Y
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=2,
+ * engines=CS0[0],CS0[1],CS1[0],CS1[1])
+ *
+ * Results in the following valid placements:
+ * CS0[0], CS1[0]
+ * CS0[0], CS1[1]
+ * CS0[1], CS1[0]
+ * CS0[1], CS1[1]
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS0[0], CS0[1]
+ * VE[1] = CS1[0], CS1[1]
+ *
+ * Example 2 pseudo code:
+ * CS[X] = generic engine of same class, logical instance X
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=3,
+ * engines=CS[0],CS[1],CS[2],CS[0],CS[1],CS[2])
+ *
+ * Results in the following valid placements:
+ * CS[0], CS[1]
+ * CS[0], CS[2]
+ * CS[1], CS[0]
+ * CS[1], CS[2]
+ * CS[2], CS[0]
+ * CS[2], CS[1]
+ *
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS[0], CS[1], CS[2]
+ * VE[1] = CS[0], CS[1], CS[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.
+ */


So I don't really get what this does compared to setting the flag below.
Is this just about running the batchbuffers the wrong way round, i.e. if
you have (simplest case)

width=2, num_sibglings=1, engines=

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Tvrtko Ursulin



On 19/05/2021 00:58, Matthew Brost wrote:

Add entry fpr 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

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 | 144 ++
  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
  2 files changed, 196 insertions(+), 1 deletion(-)
  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 ..8c64b983ccad
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
@@ -0,0 +1,144 @@
+#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
i915_context_engines_parallel_submit */
+
+/*
+ * i915_context_engines_parallel_submit:
+ *
+ * Setup a slot 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 implict 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).


1)
Expand the term slot here with "slot in the context engine map" least 
once for clarity.


2)
About where execbuf will implicitly be finding batches - suggest to also 
cover first/last flag here. I know you have it in the readme but I think 
it is good if uapi header is as self-contained as possible.



+ *
+ * Their are two currently defined ways to control the placement of the
+ * hardware contexts on physical engines: default behavior (no flags) and
+ * I915_PARALLEL_IMPLICT_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 below above the flags.
+ *
+ * Returns -EINVAL if hardware context placement configuration 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 behvavior (currently unsupported):
+ *
+ * Rather than restricting parallel submission to a single class with a
+ * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a mode 
that


What do you mean with logically contiguous here? It sounds ambiguous 
versus logical vs "normal" engine instance numbers.



+ * enables parallel submission across multiple engine classes. In this case 
each
+ * context's logical engine mask indicates where that context can placed. It is
+ * implied in this mode that all contexts have mutual exclusive placement (e.g.
+ * if one context is running CS0 no other contexts can run on CS0).


I think talk about logical context and its mask is too implementation 
detail at the uapi level. Instead I would suggest more userspace 
programmer centric description.



+ *
+ * Example 1 pseudo code:
+ * CSX[Y] = engine class X, logical instance Y
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=2,
+ * engines=CS0[0],CS0[1],CS1[0],CS1[1])
+ *
+ * Results in the following valid placements:
+ * CS0[0], CS1[0]
+ * CS0[0], CS1[1]
+ * CS0[1], CS1[0]
+ * CS0[1], CS1[1]
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS0[0], CS0[1]
+ * VE[1] = CS1[0], CS1[1]


Ah okay so essentially similar to what I was proposing a year ago. But 
then it is no longer "set_parallel" really. It is one slot in the engine 
map, right, with the idea to super class intel_context in the 
implementation?


So really a wide virtual engine, as opposed to single one. In which case 
I think it makes sense to stay close to the existing naming of the 
load_balance extension for consistency. Load_balance_wide? 
Load_balance_parallel? Multi?


I also have to say the notation "CS0[0]" - I who know this problem space 
am finding it hard to penetrate what that actually means. (Also 
uppercase IMO makes it hard to read, but maybe it is just me.)


Looking a bit lower below, extension seems to be taking a 2d array of 
class:instance pairs, right? If so then reading these docs in order, or 
even just look

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-21 Thread Christian König

Am 20.05.21 um 23:38 schrieb Jason Ekstrand:

On Thu, May 20, 2021 at 10:46 AM Matthew Brost  wrote:

On Thu, May 20, 2021 at 01:11:59PM +0200, Christian König wrote:

Am 19.05.21 um 18:51 schrieb Matthew Brost:

On Wed, May 19, 2021 at 01:45:39PM +0200, Christian König wrote:

Oh, yeah we call that gang submit on the AMD side.

Had already some internal discussions how to implement this, but so far
couldn't figure out how to cleanly introduce that into the DRM scheduler.

Can you briefly describe in a few words how that is supposed to work on the
Intel side?

On Intel, we actually have two cases which don't fit the current
drm/scheduler model well: balanced and bonded.

In the balanced model, we want to submit a batch which can go to any
one of some set of engines and we don't care which.  It's up to the
kernel to pick an engine.  Imagine you had 64 identical HW compute
queues, for instance.  This could be done by making all the identical
engines share a single drm_gpu_scheduler and round-robin around the HW
queues or something.  I don't know that we strictly need drm/scheduler
to be aware of it but it might be nice if it grew support for this
mode so we could maintain a 1:1 relationship between HW queues and
drm_gpu_schedulers.  That said, I'm not sure how this would play with
GuC queues so maybe it doesn't help?


Oh, we do have support for load balancing like that.

When you call drm_sched_entity_init() you can give a list of 
drm_gpu_scheduler object to use round robing for scheduling.


New jobs are then scheduler to the drm_gpu_scheduler instance which is 
idle or rather the least busy one.



The bonded model is like your ganged, I think.  We want to submit N
batches to run in parallel.  And they actually have to be executing on
the GPU simultaneously and not just sort-of at similar times.  We need
this for video.  There are also potential use-cases in Vulkan or even
GL that might be able to use this.  One difference with the balanced
mode is that bonds don't, strictly speaking, need to be on the same
type of engine.  Imagine, for instance, a 3D batch with a parallel
compute batch doing vertex pre-processing.

I'm pretty sure the bonded case is something that the mobile drivers
(panfrost, etc.) would like as well for doing Vulkan on tilers where
you often have to have two command buffers running in parallel.
They're currently doing it by submitting a giant pile of batches where
they split the batch and add sync primitives every time some GL call
requires them to sync between fragment and vertex pipes.


Yeah, we have exactly the same problem as well.

But so far every model we discussed has some drawbacks and it is rather 
hard for the scheduler to guarantee that stuff runs at the same time.


So if you got any ideas how to cleanly implement that then they would be 
rather welcomed.


Regards,
Christian.



So, to sum up, I think there's likely some good collaboration to be
had here for everyone. :-)

--Jason


Sure, I've done a quick PoC internally and have been able to hook this
into the DRM scheduler.

Basically each BB still maps to a single job as each job is somewhat
unique (e.g. each job has its own ring, lrc, seqno, etc...). However all
the jobs configured to run in parallel map to a single sched_entity
which maintains the order each job was generated from the execbuf IOCTL
(1 - N). When the backend receives jobs 1 to N - 1 it basically just
updates some internal state. When the backend sees job N (last job) it
actually does the submit for jobs 1 - N which with GuC submission is a
simple command moving the LRC tail of the N jobs.

Daniel has suggested that we create a single job for the NN BBs but that
would be huge rework to the internals of the i915 and likely won't
happen by the time this code first lands.

Also worth noting one way a job isn't really a treated individually is
the excl slot with dma-resv. In that case we create a composite fence of
all jobs (dma_fence_array).

Yeah, that's something we have discussed as well.

How do you prevent the scheduler from over committing to a single ring
buffer in this scenario?


Each job has its own ring, the execbuf IOCTL throttles itself for each
job if there isn't space in the ring. This is exactly the same as
non-parallel submits.

I think this is what you were asking? If not, maybe try explaining the
question a bit more.

Matt


Christian.


Matt


Thanks,
Christian.

Am 19.05.21 um 01:58 schrieb Matthew Brost:

Add entry fpr 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

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 | 144 ++
Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
2 files changed, 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-20 Thread Jason Ekstrand
On Thu, May 20, 2021 at 10:46 AM Matthew Brost  wrote:
>
> On Thu, May 20, 2021 at 01:11:59PM +0200, Christian König wrote:
> > Am 19.05.21 um 18:51 schrieb Matthew Brost:
> > > On Wed, May 19, 2021 at 01:45:39PM +0200, Christian König wrote:
> > > > Oh, yeah we call that gang submit on the AMD side.
> > > >
> > > > Had already some internal discussions how to implement this, but so far
> > > > couldn't figure out how to cleanly introduce that into the DRM 
> > > > scheduler.
> > > >
> > > > Can you briefly describe in a few words how that is supposed to work on 
> > > > the
> > > > Intel side?

On Intel, we actually have two cases which don't fit the current
drm/scheduler model well: balanced and bonded.

In the balanced model, we want to submit a batch which can go to any
one of some set of engines and we don't care which.  It's up to the
kernel to pick an engine.  Imagine you had 64 identical HW compute
queues, for instance.  This could be done by making all the identical
engines share a single drm_gpu_scheduler and round-robin around the HW
queues or something.  I don't know that we strictly need drm/scheduler
to be aware of it but it might be nice if it grew support for this
mode so we could maintain a 1:1 relationship between HW queues and
drm_gpu_schedulers.  That said, I'm not sure how this would play with
GuC queues so maybe it doesn't help?

The bonded model is like your ganged, I think.  We want to submit N
batches to run in parallel.  And they actually have to be executing on
the GPU simultaneously and not just sort-of at similar times.  We need
this for video.  There are also potential use-cases in Vulkan or even
GL that might be able to use this.  One difference with the balanced
mode is that bonds don't, strictly speaking, need to be on the same
type of engine.  Imagine, for instance, a 3D batch with a parallel
compute batch doing vertex pre-processing.

I'm pretty sure the bonded case is something that the mobile drivers
(panfrost, etc.) would like as well for doing Vulkan on tilers where
you often have to have two command buffers running in parallel.
They're currently doing it by submitting a giant pile of batches where
they split the batch and add sync primitives every time some GL call
requires them to sync between fragment and vertex pipes.

So, to sum up, I think there's likely some good collaboration to be
had here for everyone. :-)

--Jason

> > > Sure, I've done a quick PoC internally and have been able to hook this
> > > into the DRM scheduler.
> > >
> > > Basically each BB still maps to a single job as each job is somewhat
> > > unique (e.g. each job has its own ring, lrc, seqno, etc...). However all
> > > the jobs configured to run in parallel map to a single sched_entity
> > > which maintains the order each job was generated from the execbuf IOCTL
> > > (1 - N). When the backend receives jobs 1 to N - 1 it basically just
> > > updates some internal state. When the backend sees job N (last job) it
> > > actually does the submit for jobs 1 - N which with GuC submission is a
> > > simple command moving the LRC tail of the N jobs.
> > >
> > > Daniel has suggested that we create a single job for the NN BBs but that
> > > would be huge rework to the internals of the i915 and likely won't
> > > happen by the time this code first lands.
> > >
> > > Also worth noting one way a job isn't really a treated individually is
> > > the excl slot with dma-resv. In that case we create a composite fence of
> > > all jobs (dma_fence_array).
> >
> > Yeah, that's something we have discussed as well.
> >
> > How do you prevent the scheduler from over committing to a single ring
> > buffer in this scenario?
> >
>
> Each job has its own ring, the execbuf IOCTL throttles itself for each
> job if there isn't space in the ring. This is exactly the same as
> non-parallel submits.
>
> I think this is what you were asking? If not, maybe try explaining the
> question a bit more.
>
> Matt
>
> > Christian.
> >
> > >
> > > Matt
> > >
> > > > Thanks,
> > > > Christian.
> > > >
> > > > Am 19.05.21 um 01:58 schrieb Matthew Brost:
> > > > > Add entry fpr 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
> > > > >
> > > > > 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 | 144 
> > > > > ++
> > > > >Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > > > >2 files changed, 196 insertions(+), 1 deletion(-)
> > > > >create mode 100644 Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > > >
> > > > > diff --git a/Documentation/gpu/rfc/i915_parall

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-20 Thread Daniel Vetter
On Thu, May 20, 2021 at 08:10:59AM -0700, Matthew Brost wrote:
> On Thu, May 20, 2021 at 11:54:25AM +0200, Daniel Vetter wrote:
> > On Wed, May 19, 2021 at 7:19 PM Matthew Brost  
> > wrote:
> > >
> > > On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> > > > On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > > > > Add entry fpr 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
> > > > >
> > > > > 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 | 144 
> > > > > ++
> > > > >  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > > > >  2 files changed, 196 insertions(+), 1 deletion(-)
> > > > >  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 ..8c64b983ccad
> > > > > --- /dev/null
> > > > > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > > > @@ -0,0 +1,144 @@
> > > > > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > > > > i915_context_engines_parallel_submit */
> > > > > +
> > > > > +/*
> > > > > + * i915_context_engines_parallel_submit:
> > > > > + *
> > > > > + * Setup a slot 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 implict 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).
> > > > > + *
> > > > > + * Their are two currently defined ways to control the placement of 
> > > > > the
> > > > > + * hardware contexts on physical engines: default behavior (no 
> > > > > flags) and
> > > > > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > > > > + *
> > > > > + * Returns -EINVAL if hardware context placement configuration 
> > > > > 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;
> > > >
> > > > Ok the big picture looks reasonable now, the flags still confuse me.
> > > >
> > >
> > > Yea, it is a bit confusing.
> > >
> > > > > +/*
> > > > > + * Default placement behvavior (currently unsupported):
> > > > > + *
> > > > > + * Rather than restricting parallel submission to a single class 
> > > > > with a
> > > > > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add 
> > > > > a mode that
> > > > > + * enables parallel submission across multiple engine classes. In 
> > > > > this case each
> > > > > + * context's logical engine mask indicates where that context can 
> > > > > placed. It is
> > > > > + * implied in this mode that all contexts have mutual exclusive 
> > > > > placement (e.g.
> > > > > + * if one context is running CS0 no other contexts can run on CS0).
> > > > > + *
> > > > > + * Example 1 pseudo code:
> > > > > + * CSX[Y] = engine class X, logical instance Y
> > > > > + * INVALID = I915_ENGINE_CLASS_INVALID, 
> > > > > I915_ENGINE_CLASS_INVALID_NONE
> > > > > + * set_engines(INVALID)
> > > > > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > > > > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])
> > > > > + *
> > > > > + * Results in the following valid placements:
> > > > > + * CS0[0], CS1[0]
> > > > > + * CS0[0], CS1[1]
> > > > > + * CS0[1], CS1[0]
> > > > > + * CS0[1], CS1[1]
> > > > > + *
> > > > > + * This can also be though of as 2 virtual engines:
> > > > > + * VE[0] 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-20 Thread Daniel Vetter
On Thu, May 20, 2021 at 11:57:44AM +0100, Tvrtko Ursulin wrote:
> 
> On 20/05/2021 10:54, Daniel Vetter wrote:
> > On Wed, May 19, 2021 at 7:19 PM Matthew Brost  
> > wrote:
> > > 
> > > On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> > > > On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > > > > Add entry fpr 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
> > > > > 
> > > > > 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 | 144 
> > > > > ++
> > > > >   Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > > > >   2 files changed, 196 insertions(+), 1 deletion(-)
> > > > >   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 ..8c64b983ccad
> > > > > --- /dev/null
> > > > > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > > > @@ -0,0 +1,144 @@
> > > > > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > > > > i915_context_engines_parallel_submit */
> > > > > +
> > > > > +/*
> > > > > + * i915_context_engines_parallel_submit:
> > > > > + *
> > > > > + * Setup a slot 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 implict 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).
> > > > > + *
> > > > > + * Their are two currently defined ways to control the placement of 
> > > > > the
> > > > > + * hardware contexts on physical engines: default behavior (no 
> > > > > flags) and
> > > > > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > > > > + *
> > > > > + * Returns -EINVAL if hardware context placement configuration 
> > > > > 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;
> > > > 
> > > > Ok the big picture looks reasonable now, the flags still confuse me.
> > > > 
> > > 
> > > Yea, it is a bit confusing.
> > > 
> > > > > +/*
> > > > > + * Default placement behvavior (currently unsupported):
> > > > > + *
> > > > > + * Rather than restricting parallel submission to a single class 
> > > > > with a
> > > > > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add 
> > > > > a mode that
> > > > > + * enables parallel submission across multiple engine classes. In 
> > > > > this case each
> > > > > + * context's logical engine mask indicates where that context can 
> > > > > placed. It is
> > > > > + * implied in this mode that all contexts have mutual exclusive 
> > > > > placement (e.g.
> > > > > + * if one context is running CS0 no other contexts can run on CS0).
> > > > > + *
> > > > > + * Example 1 pseudo code:
> > > > > + * CSX[Y] = engine class X, logical instance Y
> > > > > + * INVALID = I915_ENGINE_CLASS_INVALID, 
> > > > > I915_ENGINE_CLASS_INVALID_NONE
> > > > > + * set_engines(INVALID)
> > > > > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > > > > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])
> > > > > + *
> > > > > + * Results in the following valid placements:
> > > > > + * CS0[0], CS1[0]
> > > > > + * CS0[0], CS1[1]
> > > > > + * CS0[1], CS1[0]
> > > > > + * CS0[1], CS1[1]
> > > > > + *
> > > > > + * This can also be though of as 2 virtual engines:
> > > > > + * VE[0] 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-20 Thread Matthew Brost
On Thu, May 20, 2021 at 11:54:25AM +0200, Daniel Vetter wrote:
> On Wed, May 19, 2021 at 7:19 PM Matthew Brost  wrote:
> >
> > On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> > > On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > > > Add entry fpr 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
> > > >
> > > > 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 | 144 ++
> > > >  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > > >  2 files changed, 196 insertions(+), 1 deletion(-)
> > > >  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 ..8c64b983ccad
> > > > --- /dev/null
> > > > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > > @@ -0,0 +1,144 @@
> > > > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > > > i915_context_engines_parallel_submit */
> > > > +
> > > > +/*
> > > > + * i915_context_engines_parallel_submit:
> > > > + *
> > > > + * Setup a slot 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 implict 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).
> > > > + *
> > > > + * Their are two currently defined ways to control the placement of the
> > > > + * hardware contexts on physical engines: default behavior (no flags) 
> > > > and
> > > > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > > > + *
> > > > + * Returns -EINVAL if hardware context placement configuration 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;
> > >
> > > Ok the big picture looks reasonable now, the flags still confuse me.
> > >
> >
> > Yea, it is a bit confusing.
> >
> > > > +/*
> > > > + * Default placement behvavior (currently unsupported):
> > > > + *
> > > > + * Rather than restricting parallel submission to a single class with a
> > > > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a 
> > > > mode that
> > > > + * enables parallel submission across multiple engine classes. In this 
> > > > case each
> > > > + * context's logical engine mask indicates where that context can 
> > > > placed. It is
> > > > + * implied in this mode that all contexts have mutual exclusive 
> > > > placement (e.g.
> > > > + * if one context is running CS0 no other contexts can run on CS0).
> > > > + *
> > > > + * Example 1 pseudo code:
> > > > + * CSX[Y] = engine class X, logical instance Y
> > > > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > > > + * set_engines(INVALID)
> > > > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > > > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])
> > > > + *
> > > > + * Results in the following valid placements:
> > > > + * CS0[0], CS1[0]
> > > > + * CS0[0], CS1[1]
> > > > + * CS0[1], CS1[0]
> > > > + * CS0[1], CS1[1]
> > > > + *
> > > > + * This can also be though of as 2 virtual engines:
> > > > + * VE[0] = CS0[0], CS0[1]
> > > > + * VE[1] = CS1[0], CS1[1]
> > > > + *
> > > > + * Example 2 pseudo code:
> > > > + * CS[X] = generic engine of same class, logical instance X
> > > > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > > > + * set_engines(INVALID)
> > > > + * set_parallel(engine_index=0, width=2, num_siblings=3,
> > > > + * 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-20 Thread Tvrtko Ursulin



On 20/05/2021 10:54, Daniel Vetter wrote:

On Wed, May 19, 2021 at 7:19 PM Matthew Brost  wrote:


On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:

On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:

Add entry fpr 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

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 | 144 ++
  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
  2 files changed, 196 insertions(+), 1 deletion(-)
  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 ..8c64b983ccad
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
@@ -0,0 +1,144 @@
+#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
i915_context_engines_parallel_submit */
+
+/*
+ * i915_context_engines_parallel_submit:
+ *
+ * Setup a slot 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 implict 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).
+ *
+ * Their are two currently defined ways to control the placement of the
+ * hardware contexts on physical engines: default behavior (no flags) and
+ * I915_PARALLEL_IMPLICT_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 below above the flags.
+ *
+ * Returns -EINVAL if hardware context placement configuration 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;


Ok the big picture looks reasonable now, the flags still confuse me.



Yea, it is a bit confusing.


+/*
+ * Default placement behvavior (currently unsupported):
+ *
+ * Rather than restricting parallel submission to a single class with a
+ * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a mode 
that
+ * enables parallel submission across multiple engine classes. In this case 
each
+ * context's logical engine mask indicates where that context can placed. It is
+ * implied in this mode that all contexts have mutual exclusive placement (e.g.
+ * if one context is running CS0 no other contexts can run on CS0).
+ *
+ * Example 1 pseudo code:
+ * CSX[Y] = engine class X, logical instance Y
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=2,
+ * engines=CS0[0],CS0[1],CS1[0],CS1[1])
+ *
+ * Results in the following valid placements:
+ * CS0[0], CS1[0]
+ * CS0[0], CS1[1]
+ * CS0[1], CS1[0]
+ * CS0[1], CS1[1]
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS0[0], CS0[1]
+ * VE[1] = CS1[0], CS1[1]
+ *
+ * Example 2 pseudo code:
+ * CS[X] = generic engine of same class, logical instance X
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=3,
+ * engines=CS[0],CS[1],CS[2],CS[0],CS[1],CS[2])
+ *
+ * Results in the following valid placements:
+ * CS[0], CS[1]
+ * CS[0], CS[2]
+ * CS[1], CS[0]
+ * CS[1], CS[2]
+ * CS[2], CS[0]
+ * CS[2], CS[1]
+ *
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS[0], CS[1], CS[2]
+ * VE[1] = CS[0], CS[1], CS[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.
+ */


So I don't really get what this does compared to setting the flag below.
Is this just about running the batchbuffers the wrong way round, i.e. if
you have (simplest case)

width=2, num_sibglings=1, engines=CS[0], CS[1]

Then both
CS[0], CS[1]
and
CS[1], CS[0]
are possible options for running 2 batches? Iow, the backend is 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-20 Thread Daniel Vetter
On Wed, May 19, 2021 at 7:19 PM Matthew Brost  wrote:
>
> On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> > On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > > Add entry fpr 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
> > >
> > > 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 | 144 ++
> > >  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> > >  2 files changed, 196 insertions(+), 1 deletion(-)
> > >  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 ..8c64b983ccad
> > > --- /dev/null
> > > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > > @@ -0,0 +1,144 @@
> > > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > > i915_context_engines_parallel_submit */
> > > +
> > > +/*
> > > + * i915_context_engines_parallel_submit:
> > > + *
> > > + * Setup a slot 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 implict 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).
> > > + *
> > > + * Their are two currently defined ways to control the placement of the
> > > + * hardware contexts on physical engines: default behavior (no flags) and
> > > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > > + *
> > > + * Returns -EINVAL if hardware context placement configuration 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;
> >
> > Ok the big picture looks reasonable now, the flags still confuse me.
> >
>
> Yea, it is a bit confusing.
>
> > > +/*
> > > + * Default placement behvavior (currently unsupported):
> > > + *
> > > + * Rather than restricting parallel submission to a single class with a
> > > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a 
> > > mode that
> > > + * enables parallel submission across multiple engine classes. In this 
> > > case each
> > > + * context's logical engine mask indicates where that context can 
> > > placed. It is
> > > + * implied in this mode that all contexts have mutual exclusive 
> > > placement (e.g.
> > > + * if one context is running CS0 no other contexts can run on CS0).
> > > + *
> > > + * Example 1 pseudo code:
> > > + * CSX[Y] = engine class X, logical instance Y
> > > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > > + * set_engines(INVALID)
> > > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])
> > > + *
> > > + * Results in the following valid placements:
> > > + * CS0[0], CS1[0]
> > > + * CS0[0], CS1[1]
> > > + * CS0[1], CS1[0]
> > > + * CS0[1], CS1[1]
> > > + *
> > > + * This can also be though of as 2 virtual engines:
> > > + * VE[0] = CS0[0], CS0[1]
> > > + * VE[1] = CS1[0], CS1[1]
> > > + *
> > > + * Example 2 pseudo code:
> > > + * CS[X] = generic engine of same class, logical instance X
> > > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > > + * set_engines(INVALID)
> > > + * set_parallel(engine_index=0, width=2, num_siblings=3,
> > > + * engines=CS[0],CS[1],CS[2],CS[0],CS[1],CS[2])
> > > + *
> > > + * Results in the following valid placements:
> > > + * CS[0], CS[1]
> > > + * CS[0], CS[2]
> > > + * CS[1], CS[0]
> > > + * CS[1], CS[2]
> > > + * CS[2], CS[0]
> > > + * CS[2], CS[1]
> > > + *
> > > + *
> > > + * This can also be though of as 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-19 Thread Matthew Brost
On Wed, May 19, 2021 at 01:10:04PM +0200, Daniel Vetter wrote:
> On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> > Add entry fpr 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
> > 
> > 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 | 144 ++
> >  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
> >  2 files changed, 196 insertions(+), 1 deletion(-)
> >  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 ..8c64b983ccad
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> > @@ -0,0 +1,144 @@
> > +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> > i915_context_engines_parallel_submit */
> > +
> > +/*
> > + * i915_context_engines_parallel_submit:
> > + *
> > + * Setup a slot 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 implict 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).
> > + *
> > + * Their are two currently defined ways to control the placement of the
> > + * hardware contexts on physical engines: default behavior (no flags) and
> > + * I915_PARALLEL_IMPLICT_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 below above the flags.
> > + *
> > + * Returns -EINVAL if hardware context placement configuration 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;
> 
> Ok the big picture looks reasonable now, the flags still confuse me.
> 

Yea, it is a bit confusing.

> > +/*
> > + * Default placement behvavior (currently unsupported):
> > + *
> > + * Rather than restricting parallel submission to a single class with a
> > + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a 
> > mode that
> > + * enables parallel submission across multiple engine classes. In this 
> > case each
> > + * context's logical engine mask indicates where that context can placed. 
> > It is
> > + * implied in this mode that all contexts have mutual exclusive placement 
> > (e.g.
> > + * if one context is running CS0 no other contexts can run on CS0).
> > + *
> > + * Example 1 pseudo code:
> > + * CSX[Y] = engine class X, logical instance Y
> > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > + * set_engines(INVALID)
> > + * set_parallel(engine_index=0, width=2, num_siblings=2,
> > + * engines=CS0[0],CS0[1],CS1[0],CS1[1])
> > + *
> > + * Results in the following valid placements:
> > + * CS0[0], CS1[0]
> > + * CS0[0], CS1[1]
> > + * CS0[1], CS1[0]
> > + * CS0[1], CS1[1]
> > + *
> > + * This can also be though of as 2 virtual engines:
> > + * VE[0] = CS0[0], CS0[1]
> > + * VE[1] = CS1[0], CS1[1]
> > + *
> > + * Example 2 pseudo code:
> > + * CS[X] = generic engine of same class, logical instance X
> > + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> > + * set_engines(INVALID)
> > + * set_parallel(engine_index=0, width=2, num_siblings=3,
> > + * engines=CS[0],CS[1],CS[2],CS[0],CS[1],CS[2])
> > + *
> > + * Results in the following valid placements:
> > + * CS[0], CS[1]
> > + * CS[0], CS[2]
> > + * CS[1], CS[0]
> > + * CS[1], CS[2]
> > + * CS[2], CS[0]
> > + * CS[2], CS[1]
> > + *
> > + *
> > + * This can also be though of as 2 virtual engines:
> > + * VE[0] = CS[0], CS[1], CS[2]
> > + * VE[1] = CS[0], CS[1], CS[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

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-19 Thread Tvrtko Ursulin



On 19/05/2021 12:10, Daniel Vetter wrote:

On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:

Add entry fpr 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

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 | 144 ++
  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
  2 files changed, 196 insertions(+), 1 deletion(-)
  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 ..8c64b983ccad
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
@@ -0,0 +1,144 @@
+#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
i915_context_engines_parallel_submit */
+
+/*
+ * i915_context_engines_parallel_submit:
+ *
+ * Setup a slot 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 implict 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).
+ *
+ * Their are two currently defined ways to control the placement of the
+ * hardware contexts on physical engines: default behavior (no flags) and
+ * I915_PARALLEL_IMPLICT_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 below above the flags.
+ *
+ * Returns -EINVAL if hardware context placement configuration 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;


Ok the big picture looks reasonable now, the flags still confuse me.


+/*
+ * Default placement behvavior (currently unsupported):
+ *
+ * Rather than restricting parallel submission to a single class with a
+ * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a mode 
that
+ * enables parallel submission across multiple engine classes. In this case 
each
+ * context's logical engine mask indicates where that context can placed. It is
+ * implied in this mode that all contexts have mutual exclusive placement (e.g.
+ * if one context is running CS0 no other contexts can run on CS0).
+ *
+ * Example 1 pseudo code:
+ * CSX[Y] = engine class X, logical instance Y
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=2,
+ * engines=CS0[0],CS0[1],CS1[0],CS1[1])
+ *
+ * Results in the following valid placements:
+ * CS0[0], CS1[0]
+ * CS0[0], CS1[1]
+ * CS0[1], CS1[0]
+ * CS0[1], CS1[1]
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS0[0], CS0[1]
+ * VE[1] = CS1[0], CS1[1]
+ *
+ * Example 2 pseudo code:
+ * CS[X] = generic engine of same class, logical instance X
+ * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
+ * set_engines(INVALID)
+ * set_parallel(engine_index=0, width=2, num_siblings=3,
+ * engines=CS[0],CS[1],CS[2],CS[0],CS[1],CS[2])
+ *
+ * Results in the following valid placements:
+ * CS[0], CS[1]
+ * CS[0], CS[2]
+ * CS[1], CS[0]
+ * CS[1], CS[2]
+ * CS[2], CS[0]
+ * CS[2], CS[1]
+ *
+ *
+ * This can also be though of as 2 virtual engines:
+ * VE[0] = CS[0], CS[1], CS[2]
+ * VE[1] = CS[0], CS[1], CS[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.
+ */


So I don't really get what this does compared to setting the flag below.
Is this just about running the batchbuffers the wrong way round, i.e. if
you have (simplest case)

width=2, num_sibglings=1, engines=CS[0], CS[1]

Then both
CS[0], CS[1]
and
CS[1], CS[0]
are possible options for running 2 batches? Iow, the backend is allowed to
run the batchbuffers the wrong way round, which gains us nothing, since we
assume the batches take equally long 

Re: [Mesa-dev] [Intel-gfx] [RFC 2/2] drm/doc/rfc: i915 new parallel submission uAPI plan

2021-05-19 Thread Daniel Vetter
On Tue, May 18, 2021 at 04:58:30PM -0700, Matthew Brost wrote:
> Add entry fpr 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
> 
> 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 | 144 ++
>  Documentation/gpu/rfc/i915_scheduler.rst  |  53 ++-
>  2 files changed, 196 insertions(+), 1 deletion(-)
>  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 ..8c64b983ccad
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_parallel_execbuf.h
> @@ -0,0 +1,144 @@
> +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see 
> i915_context_engines_parallel_submit */
> +
> +/*
> + * i915_context_engines_parallel_submit:
> + *
> + * Setup a slot 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 implict 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).
> + *
> + * Their are two currently defined ways to control the placement of the
> + * hardware contexts on physical engines: default behavior (no flags) and
> + * I915_PARALLEL_IMPLICT_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 below above the flags.
> + *
> + * Returns -EINVAL if hardware context placement configuration 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;

Ok the big picture looks reasonable now, the flags still confuse me.

> +/*
> + * Default placement behvavior (currently unsupported):
> + *
> + * Rather than restricting parallel submission to a single class with a
> + * logically contiguous placement (I915_PARALLEL_IMPLICT_BONDS), add a mode 
> that
> + * enables parallel submission across multiple engine classes. In this case 
> each
> + * context's logical engine mask indicates where that context can placed. It 
> is
> + * implied in this mode that all contexts have mutual exclusive placement 
> (e.g.
> + * if one context is running CS0 no other contexts can run on CS0).
> + *
> + * Example 1 pseudo code:
> + * CSX[Y] = engine class X, logical instance Y
> + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> + * set_engines(INVALID)
> + * set_parallel(engine_index=0, width=2, num_siblings=2,
> + *   engines=CS0[0],CS0[1],CS1[0],CS1[1])
> + *
> + * Results in the following valid placements:
> + * CS0[0], CS1[0]
> + * CS0[0], CS1[1]
> + * CS0[1], CS1[0]
> + * CS0[1], CS1[1]
> + *
> + * This can also be though of as 2 virtual engines:
> + * VE[0] = CS0[0], CS0[1]
> + * VE[1] = CS1[0], CS1[1]
> + *
> + * Example 2 pseudo code:
> + * CS[X] = generic engine of same class, logical instance X
> + * INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
> + * set_engines(INVALID)
> + * set_parallel(engine_index=0, width=2, num_siblings=3,
> + *   engines=CS[0],CS[1],CS[2],CS[0],CS[1],CS[2])
> + *
> + * Results in the following valid placements:
> + * CS[0], CS[1]
> + * CS[0], CS[2]
> + * CS[1], CS[0]
> + * CS[1], CS[2]
> + * CS[2], CS[0]
> + * CS[2], CS[1]
> + *
> + *
> + * This can also be though of as 2 virtual engines:
> + * VE[0] = CS[0], CS[1], CS[2]
> + * VE[1] = CS[0], CS[1], CS[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.
> + */

So I don't really get what this does compared to setting the flag below.
Is this just about running the batchbuffers the wrong way round, i.e. if
you have (simplest case)

width=2, num_sibglings=1, engines=CS[0], CS[1]

Then both
CS[0], CS[1]
and
CS[1], CS[0]
are possible op