| Message ID | 20210503155748.1961781-11-jason@jlekstrand.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gem: ioctl clean-ups (v5) | expand |
On Mon, May 03, 2021 at 10:57:31AM -0500, Jason Ekstrand wrote: > Even though FENCE_SUBMIT is only documented to wait until the request in > the in-fence starts instead of waiting until it completes, it has a bit > more magic than that. If FENCE_SUBMIT is used to submit something to a > balanced engine, we would wait to assign engines until the primary > request was ready to start and then attempt to assign it to a different > engine than the primary. There is an IGT test which exercises this by > submitting a primary batch to a specific VCS and then using FENCE_SUBMIT > to submit a secondary which can run on any VCS and have i915 figure out > which VCS to run it on such that they can run in parallel. > > However, this functionality has never been used in the real world. The > media driver (the only user of FENCE_SUBMIT) always picks exactly two > physical engines to bond and never asks us to pick which to use. Maybe reference the specific igt you're break (and removing in the igt series to match this) here. Just for the record and all that. -Daniel > > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> > --- > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 ------- > .../drm/i915/gt/intel_execlists_submission.c | 17 ----------------- > 3 files changed, 1 insertion(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index d640bba6ad9ab..efb2fa3522a42 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -3474,7 +3474,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > if (args->flags & I915_EXEC_FENCE_SUBMIT) > err = i915_request_await_execution(eb.request, > in_fence, > - eb.engine->bond_execute); > + NULL); > else > err = i915_request_await_dma_fence(eb.request, > in_fence); > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h > index 883bafc449024..68cfe5080325c 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > @@ -446,13 +446,6 @@ struct intel_engine_cs { > */ > void (*submit_request)(struct i915_request *rq); > > - /* > - * Called on signaling of a SUBMIT_FENCE, passing along the signaling > - * request down to the bonded pairs. > - */ > - void (*bond_execute)(struct i915_request *rq, > - struct dma_fence *signal); > - > /* > * Call when the priority on a request has changed and it and its > * dependencies may need rescheduling. Note the request itself may > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > index 14378b28169b7..635d6d2494d26 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > @@ -3547,22 +3547,6 @@ static void virtual_submit_request(struct i915_request *rq) > spin_unlock_irqrestore(&ve->base.active.lock, flags); > } > > -static void > -virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal) > -{ > - intel_engine_mask_t allowed, exec; > - > - allowed = ~to_request(signal)->engine->mask; > - > - /* Restrict the bonded request to run on only the available engines */ > - exec = READ_ONCE(rq->execution_mask); > - while (!try_cmpxchg(&rq->execution_mask, &exec, exec & allowed)) > - ; > - > - /* Prevent the master from being re-run on the bonded engines */ > - to_request(signal)->execution_mask &= ~allowed; > -} > - > struct intel_context * > intel_execlists_create_virtual(struct intel_engine_cs **siblings, > unsigned int count) > @@ -3616,7 +3600,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, > > ve->base.schedule = i915_schedule; > ve->base.submit_request = virtual_submit_request; > - ve->base.bond_execute = virtual_bond_execute; > > INIT_LIST_HEAD(virtual_queue(ve)); > ve->base.execlists.queue_priority_hint = INT_MIN; > -- > 2.31.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, May 4, 2021 at 3:56 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, May 03, 2021 at 10:57:31AM -0500, Jason Ekstrand wrote: > > Even though FENCE_SUBMIT is only documented to wait until the request in > > the in-fence starts instead of waiting until it completes, it has a bit > > more magic than that. If FENCE_SUBMIT is used to submit something to a > > balanced engine, we would wait to assign engines until the primary > > request was ready to start and then attempt to assign it to a different > > engine than the primary. There is an IGT test which exercises this by > > submitting a primary batch to a specific VCS and then using FENCE_SUBMIT > > to submit a secondary which can run on any VCS and have i915 figure out > > which VCS to run it on such that they can run in parallel. > > > > However, this functionality has never been used in the real world. The > > media driver (the only user of FENCE_SUBMIT) always picks exactly two > > physical engines to bond and never asks us to pick which to use. > > Maybe reference the specific igt you're break (and removing in the igt > series to match this) here. Just for the record and all that. Done. > -Daniel > > > > > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> > > --- > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 ------- > > .../drm/i915/gt/intel_execlists_submission.c | 17 ----------------- > > 3 files changed, 1 insertion(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > > index d640bba6ad9ab..efb2fa3522a42 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > > @@ -3474,7 +3474,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > if (args->flags & I915_EXEC_FENCE_SUBMIT) > > err = i915_request_await_execution(eb.request, > > in_fence, > > - eb.engine->bond_execute); > > + NULL); > > else > > err = i915_request_await_dma_fence(eb.request, > > in_fence); > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h > > index 883bafc449024..68cfe5080325c 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > > @@ -446,13 +446,6 @@ struct intel_engine_cs { > > */ > > void (*submit_request)(struct i915_request *rq); > > > > - /* > > - * Called on signaling of a SUBMIT_FENCE, passing along the signaling > > - * request down to the bonded pairs. > > - */ > > - void (*bond_execute)(struct i915_request *rq, > > - struct dma_fence *signal); > > - > > /* > > * Call when the priority on a request has changed and it and its > > * dependencies may need rescheduling. Note the request itself may > > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > > index 14378b28169b7..635d6d2494d26 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > > @@ -3547,22 +3547,6 @@ static void virtual_submit_request(struct i915_request *rq) > > spin_unlock_irqrestore(&ve->base.active.lock, flags); > > } > > > > -static void > > -virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal) > > -{ > > - intel_engine_mask_t allowed, exec; > > - > > - allowed = ~to_request(signal)->engine->mask; > > - > > - /* Restrict the bonded request to run on only the available engines */ > > - exec = READ_ONCE(rq->execution_mask); > > - while (!try_cmpxchg(&rq->execution_mask, &exec, exec & allowed)) > > - ; > > - > > - /* Prevent the master from being re-run on the bonded engines */ > > - to_request(signal)->execution_mask &= ~allowed; > > -} > > - > > struct intel_context * > > intel_execlists_create_virtual(struct intel_engine_cs **siblings, > > unsigned int count) > > @@ -3616,7 +3600,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, > > > > ve->base.schedule = i915_schedule; > > ve->base.submit_request = virtual_submit_request; > > - ve->base.bond_execute = virtual_bond_execute; > > > > INIT_LIST_HEAD(virtual_queue(ve)); > > ve->base.execlists.queue_priority_hint = INT_MIN; > > -- > > 2.31.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index d640bba6ad9ab..efb2fa3522a42 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -3474,7 +3474,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, if (args->flags & I915_EXEC_FENCE_SUBMIT) err = i915_request_await_execution(eb.request, in_fence, - eb.engine->bond_execute); + NULL); else err = i915_request_await_dma_fence(eb.request, in_fence); diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 883bafc449024..68cfe5080325c 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -446,13 +446,6 @@ struct intel_engine_cs { */ void (*submit_request)(struct i915_request *rq); - /* - * Called on signaling of a SUBMIT_FENCE, passing along the signaling - * request down to the bonded pairs. - */ - void (*bond_execute)(struct i915_request *rq, - struct dma_fence *signal); - /* * Call when the priority on a request has changed and it and its * dependencies may need rescheduling. Note the request itself may diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 14378b28169b7..635d6d2494d26 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -3547,22 +3547,6 @@ static void virtual_submit_request(struct i915_request *rq) spin_unlock_irqrestore(&ve->base.active.lock, flags); } -static void -virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal) -{ - intel_engine_mask_t allowed, exec; - - allowed = ~to_request(signal)->engine->mask; - - /* Restrict the bonded request to run on only the available engines */ - exec = READ_ONCE(rq->execution_mask); - while (!try_cmpxchg(&rq->execution_mask, &exec, exec & allowed)) - ; - - /* Prevent the master from being re-run on the bonded engines */ - to_request(signal)->execution_mask &= ~allowed; -} - struct intel_context * intel_execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count) @@ -3616,7 +3600,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, ve->base.schedule = i915_schedule; ve->base.submit_request = virtual_submit_request; - ve->base.bond_execute = virtual_bond_execute; INIT_LIST_HEAD(virtual_queue(ve)); ve->base.execlists.queue_priority_hint = INT_MIN;
Even though FENCE_SUBMIT is only documented to wait until the request in the in-fence starts instead of waiting until it completes, it has a bit more magic than that. If FENCE_SUBMIT is used to submit something to a balanced engine, we would wait to assign engines until the primary request was ready to start and then attempt to assign it to a different engine than the primary. There is an IGT test which exercises this by submitting a primary batch to a specific VCS and then using FENCE_SUBMIT to submit a secondary which can run on any VCS and have i915 figure out which VCS to run it on such that they can run in parallel. However, this functionality has never been used in the real world. The media driver (the only user of FENCE_SUBMIT) always picks exactly two physical engines to bond and never asks us to pick which to use. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/gt/intel_engine_types.h | 7 ------- .../drm/i915/gt/intel_execlists_submission.c | 17 ----------------- 3 files changed, 1 insertion(+), 25 deletions(-)