| Message ID | 20200527085322.25861-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Special handling for bonded requests | expand |
Quoting Tvrtko Ursulin (2020-05-27 09:53:22) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Our generic mechanism for specifying aligned request start, > I915_EXEC_FENCE_SUBMIT, has a bit too relaxed rules when it comes to the > actual use case of media scalability on Tigerlake. > > While the submit fence can provide the aligned start, the actual media > pipeline expects that execution remains in lockstep from then onwards. > Otherwise the hardware cannot handle well one of the pair getting > preempted leading to GPU hangs and corrupted media playback. > > This puts us in a difficult position where the only visible solution, > which does not involve adding new uapi, is to give more meaning to the > generic submit fence. At least when used between the video engines. > > The special semantics this patch applies in that case are two fold. First > is that both of the aligned pairs will be marked as non-preemptable and > second is ensuring both are not sharing ELSP ports with any other context. > > Non-preemptable property will ensure that once the start has been aligned > the requests remain executing until completion. > > Single port ELSP property will ensure there are no possible inversions > between different independent pairs of aligned requests. Nak. > Going forward we can think of introducing new uapi to request this > behaviour as a separate, more explicit flag, and at that point retire this > semi-generic special handling. As CI will say, such behaviour will already need a new flag. Forcing no-preemption should be a privileged flag, so I would expect some acknowledge that this is a HW problem that we have to workaround. > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Suggested-by: Xiaogang Li <xiaogang.li@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/intel_context.h | 2 +- > drivers/gpu/drm/i915/gt/intel_lrc.c | 46 +++++++++++++++++++++---- > 2 files changed, 41 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h > index 07be021882cc..576d11c0cdd9 100644 > --- a/drivers/gpu/drm/i915/gt/intel_context.h > +++ b/drivers/gpu/drm/i915/gt/intel_context.h > @@ -212,7 +212,7 @@ intel_context_force_single_submission(const struct intel_context *ce) > static inline void > intel_context_set_single_submission(struct intel_context *ce) > { > - __set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); > + set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); > } > > static inline bool > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index 3214a4ecc31a..88cf20fd92c8 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -1734,8 +1734,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) > > static bool ctx_single_port_submission(const struct intel_context *ce) > { > - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && > - intel_context_force_single_submission(ce)); > + return intel_context_force_single_submission(ce); > } > > static bool can_merge_ctx(const struct intel_context *prev, > @@ -4929,9 +4928,41 @@ static void execlists_park(struct intel_engine_cs *engine) > cancel_timer(&engine->execlists.preempt); > } > > +static void > +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) > +{ > + /* > + * Give (temporary) special meaning to a pair requests with requested > + * aligned start along the video engines. > + * > + * They should be non-preemptable and have all ELSP ports to themselves > + * to avoid any deadlocks caused by inversions. This sentence needs expanding, because you are implying that this is an issue we need to address in the code. If there is a deadlock resulting from incorrect submission ordering, that is a bug in the code. -Chris
On 27/05/2020 10:20, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2020-05-27 09:53:22) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Our generic mechanism for specifying aligned request start, >> I915_EXEC_FENCE_SUBMIT, has a bit too relaxed rules when it comes to the >> actual use case of media scalability on Tigerlake. >> >> While the submit fence can provide the aligned start, the actual media >> pipeline expects that execution remains in lockstep from then onwards. >> Otherwise the hardware cannot handle well one of the pair getting >> preempted leading to GPU hangs and corrupted media playback. >> >> This puts us in a difficult position where the only visible solution, >> which does not involve adding new uapi, is to give more meaning to the >> generic submit fence. At least when used between the video engines. >> >> The special semantics this patch applies in that case are two fold. First >> is that both of the aligned pairs will be marked as non-preemptable and >> second is ensuring both are not sharing ELSP ports with any other context. >> >> Non-preemptable property will ensure that once the start has been aligned >> the requests remain executing until completion. >> >> Single port ELSP property will ensure there are no possible inversions >> between different independent pairs of aligned requests. > > Nak. > >> Going forward we can think of introducing new uapi to request this >> behaviour as a separate, more explicit flag, and at that point retire this >> semi-generic special handling. > > As CI will say, such behaviour will already need a new flag. Forcing > no-preemption should be a privileged flag, so I would expect some > acknowledge that this is a HW problem that we have to workaround. I don't know how hw exactly works so from my side it's only empirical. I agree new flag is needed but we also need a fix ASAP for TGL. And I don't think we can add new uapi in a reasonable time frame here. We would need the ctx set_parallel extension with a dont-preempt flag and multi-batch execbuf. And a lot of work in media-driver which will not be ready for TGL. I don't think a flag at the I915_EXEC_FENCE_SUBMIT level is a solution. >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Suggested-by: Xiaogang Li <xiaogang.li@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> --- >> drivers/gpu/drm/i915/gt/intel_context.h | 2 +- >> drivers/gpu/drm/i915/gt/intel_lrc.c | 46 +++++++++++++++++++++---- >> 2 files changed, 41 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h >> index 07be021882cc..576d11c0cdd9 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_context.h >> +++ b/drivers/gpu/drm/i915/gt/intel_context.h >> @@ -212,7 +212,7 @@ intel_context_force_single_submission(const struct intel_context *ce) >> static inline void >> intel_context_set_single_submission(struct intel_context *ce) >> { >> - __set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); >> + set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); >> } >> >> static inline bool >> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c >> index 3214a4ecc31a..88cf20fd92c8 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c >> @@ -1734,8 +1734,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) >> >> static bool ctx_single_port_submission(const struct intel_context *ce) >> { >> - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && >> - intel_context_force_single_submission(ce)); >> + return intel_context_force_single_submission(ce); >> } >> >> static bool can_merge_ctx(const struct intel_context *prev, >> @@ -4929,9 +4928,41 @@ static void execlists_park(struct intel_engine_cs *engine) >> cancel_timer(&engine->execlists.preempt); >> } >> >> +static void >> +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) >> +{ >> + /* >> + * Give (temporary) special meaning to a pair requests with requested >> + * aligned start along the video engines. >> + * >> + * They should be non-preemptable and have all ELSP ports to themselves >> + * to avoid any deadlocks caused by inversions. > > This sentence needs expanding, because you are implying that this is an > issue we need to address in the code. If there is a deadlock resulting > from incorrect submission ordering, that is a bug in the code. If we add no-preempt I think we have to have single elsp because there is no ordering between unrelated pairs and then elsp inversion certainly sounds like a deadlock. ctx-A: vcs0 + vcs1 ctx-B: vcs0 + vcs1 There is no ordering between A and B but we'd have to pick same port for both pairs of A and B respectively. I don't see that we can do that since all four submissions are async. Regards, Tvrtko
Quoting Tvrtko Ursulin (2020-05-27 10:36:31) > > On 27/05/2020 10:20, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2020-05-27 09:53:22) > >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >> Our generic mechanism for specifying aligned request start, > >> I915_EXEC_FENCE_SUBMIT, has a bit too relaxed rules when it comes to the > >> actual use case of media scalability on Tigerlake. > >> > >> While the submit fence can provide the aligned start, the actual media > >> pipeline expects that execution remains in lockstep from then onwards. > >> Otherwise the hardware cannot handle well one of the pair getting > >> preempted leading to GPU hangs and corrupted media playback. > >> > >> This puts us in a difficult position where the only visible solution, > >> which does not involve adding new uapi, is to give more meaning to the > >> generic submit fence. At least when used between the video engines. > >> > >> The special semantics this patch applies in that case are two fold. First > >> is that both of the aligned pairs will be marked as non-preemptable and > >> second is ensuring both are not sharing ELSP ports with any other context. > >> > >> Non-preemptable property will ensure that once the start has been aligned > >> the requests remain executing until completion. > >> > >> Single port ELSP property will ensure there are no possible inversions > >> between different independent pairs of aligned requests. > > > > Nak. > > > >> Going forward we can think of introducing new uapi to request this > >> behaviour as a separate, more explicit flag, and at that point retire this > >> semi-generic special handling. > > > > As CI will say, such behaviour will already need a new flag. Forcing > > no-preemption should be a privileged flag, so I would expect some > > acknowledge that this is a HW problem that we have to workaround. > > I don't know how hw exactly works so from my side it's only empirical. > > I agree new flag is needed but we also need a fix ASAP for TGL. And I > don't think we can add new uapi in a reasonable time frame here. We > would need the ctx set_parallel extension with a dont-preempt flag and > multi-batch execbuf. And a lot of work in media-driver which will not be > ready for TGL. I don't think a flag at the I915_EXEC_FENCE_SUBMIT level > is a solution. I'm going to say the starting point is to remove the I915_ENGINE_HAS_PREEMPTION flag from vcs*. That way the tests that require preemption will skip. Sadly that's the level of our current API. > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> Suggested-by: Xiaogang Li <xiaogang.li@intel.com> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> --- > >> drivers/gpu/drm/i915/gt/intel_context.h | 2 +- > >> drivers/gpu/drm/i915/gt/intel_lrc.c | 46 +++++++++++++++++++++---- > >> 2 files changed, 41 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h > >> index 07be021882cc..576d11c0cdd9 100644 > >> --- a/drivers/gpu/drm/i915/gt/intel_context.h > >> +++ b/drivers/gpu/drm/i915/gt/intel_context.h > >> @@ -212,7 +212,7 @@ intel_context_force_single_submission(const struct intel_context *ce) > >> static inline void > >> intel_context_set_single_submission(struct intel_context *ce) > >> { > >> - __set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); > >> + set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); > >> } > >> > >> static inline bool > >> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > >> index 3214a4ecc31a..88cf20fd92c8 100644 > >> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > >> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > >> @@ -1734,8 +1734,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) > >> > >> static bool ctx_single_port_submission(const struct intel_context *ce) > >> { > >> - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && > >> - intel_context_force_single_submission(ce)); > >> + return intel_context_force_single_submission(ce); > >> } > >> > >> static bool can_merge_ctx(const struct intel_context *prev, > >> @@ -4929,9 +4928,41 @@ static void execlists_park(struct intel_engine_cs *engine) > >> cancel_timer(&engine->execlists.preempt); > >> } > >> > >> +static void > >> +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) > >> +{ > >> + /* > >> + * Give (temporary) special meaning to a pair requests with requested > >> + * aligned start along the video engines. > >> + * > >> + * They should be non-preemptable and have all ELSP ports to themselves > >> + * to avoid any deadlocks caused by inversions. > > > > This sentence needs expanding, because you are implying that this is an > > issue we need to address in the code. If there is a deadlock resulting > > from incorrect submission ordering, that is a bug in the code. > > If we add no-preempt I think we have to have single elsp because there > is no ordering between unrelated pairs and then elsp inversion certainly > sounds like a deadlock. You don't have a deadlock between unrelated pairs :) > > ctx-A: vcs0 + vcs1 > ctx-B: vcs0 + vcs1 > > There is no ordering between A and B but we'd have to pick same port for > both pairs of A and B respectively. I don't see that we can do that > since all four submissions are async. > > Regards, > > Tvrtko > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
Quoting Tvrtko Ursulin (2020-05-27 09:53:22) > +static void > +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) > +{ > + /* > + * Give (temporary) special meaning to a pair requests with requested > + * aligned start along the video engines. > + * > + * They should be non-preemptable and have all ELSP ports to themselves > + * to avoid any deadlocks caused by inversions. > + * > + * Gen11+ > + */ > + if (INTEL_GEN(rq->i915) < 11 || > + rq->engine->class != VIDEO_DECODE_CLASS || > + rq->engine->class != signal->engine->class) > + return; > + > + set_bit(I915_FENCE_FLAG_NOPREEMPT, &rq->fence.flags); > + set_bit(I915_FENCE_FLAG_NOPREEMPT, &signal->fence.flags); > + > + intel_context_set_single_submission(rq->context); > + intel_context_set_single_submission(signal->context); The thought that just popped into my head: This can be after signal is already submitted into ELSP[1]. -Chris
On 28/05/2020 10:57, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2020-05-27 09:53:22) >> +static void >> +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) >> +{ >> + /* >> + * Give (temporary) special meaning to a pair requests with requested >> + * aligned start along the video engines. >> + * >> + * They should be non-preemptable and have all ELSP ports to themselves >> + * to avoid any deadlocks caused by inversions. >> + * >> + * Gen11+ >> + */ >> + if (INTEL_GEN(rq->i915) < 11 || >> + rq->engine->class != VIDEO_DECODE_CLASS || >> + rq->engine->class != signal->engine->class) >> + return; >> + >> + set_bit(I915_FENCE_FLAG_NOPREEMPT, &rq->fence.flags); >> + set_bit(I915_FENCE_FLAG_NOPREEMPT, &signal->fence.flags); >> + >> + intel_context_set_single_submission(rq->context); >> + intel_context_set_single_submission(signal->context); > > The thought that just popped into my head: > > This can be after signal is already submitted into ELSP[1]. Yep I knew that but thought it would still work. Master in vcs0 port1, slave in vcs1 port0 or queued. If queued that means at worst case another bonded pair is running on same engines, so they should be able to complete. If slave submitted to vcs1 port0 then it will stay there until whatever is in vcs0 port0 finishes and lets master in. Do you see a possibility for things to go bad? Regards, Tvrtko
Quoting Tvrtko Ursulin (2020-05-28 11:20:07) > > On 28/05/2020 10:57, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2020-05-27 09:53:22) > >> +static void > >> +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) > >> +{ > >> + /* > >> + * Give (temporary) special meaning to a pair requests with requested > >> + * aligned start along the video engines. > >> + * > >> + * They should be non-preemptable and have all ELSP ports to themselves > >> + * to avoid any deadlocks caused by inversions. > >> + * > >> + * Gen11+ > >> + */ > >> + if (INTEL_GEN(rq->i915) < 11 || > >> + rq->engine->class != VIDEO_DECODE_CLASS || > >> + rq->engine->class != signal->engine->class) > >> + return; > >> + > >> + set_bit(I915_FENCE_FLAG_NOPREEMPT, &rq->fence.flags); > >> + set_bit(I915_FENCE_FLAG_NOPREEMPT, &signal->fence.flags); > >> + > >> + intel_context_set_single_submission(rq->context); > >> + intel_context_set_single_submission(signal->context); > > > > The thought that just popped into my head: > > > > This can be after signal is already submitted into ELSP[1]. > > Yep I knew that but thought it would still work. > > Master in vcs0 port1, slave in vcs1 port0 or queued. > > If queued that means at worst case another bonded pair is running on > same engines, so they should be able to complete. > > If slave submitted to vcs1 port0 then it will stay there until whatever > is in vcs0 port0 finishes and lets master in. > > Do you see a possibility for things to go bad? Because the master is already submitted in port1, the bond can go into port0. Then a second bond turns up for the master in port0, and we're back at square one. diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 37855ae8f8b3..698608e11df3 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2502,6 +2502,7 @@ static void eb_request_add(struct i915_execbuffer *eb) lockdep_unpin_lock(&tl->mutex, rq->cookie); trace_i915_request_add(rq); + set_bit(I915_FENCE_FLAG_SENTINEL, &rq->fence.flags); prev = __i915_request_commit(rq); Will do the trick. (Plus fixing up the rules for assert_pending_valid). -Chris
On 28/05/2020 21:23, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2020-05-28 11:20:07) >> >> On 28/05/2020 10:57, Chris Wilson wrote: >>> Quoting Tvrtko Ursulin (2020-05-27 09:53:22) >>>> +static void >>>> +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) >>>> +{ >>>> + /* >>>> + * Give (temporary) special meaning to a pair requests with requested >>>> + * aligned start along the video engines. >>>> + * >>>> + * They should be non-preemptable and have all ELSP ports to themselves >>>> + * to avoid any deadlocks caused by inversions. >>>> + * >>>> + * Gen11+ >>>> + */ >>>> + if (INTEL_GEN(rq->i915) < 11 || >>>> + rq->engine->class != VIDEO_DECODE_CLASS || >>>> + rq->engine->class != signal->engine->class) >>>> + return; >>>> + >>>> + set_bit(I915_FENCE_FLAG_NOPREEMPT, &rq->fence.flags); >>>> + set_bit(I915_FENCE_FLAG_NOPREEMPT, &signal->fence.flags); >>>> + >>>> + intel_context_set_single_submission(rq->context); >>>> + intel_context_set_single_submission(signal->context); >>> >>> The thought that just popped into my head: >>> >>> This can be after signal is already submitted into ELSP[1]. >> >> Yep I knew that but thought it would still work. >> >> Master in vcs0 port1, slave in vcs1 port0 or queued. >> >> If queued that means at worst case another bonded pair is running on >> same engines, so they should be able to complete. >> >> If slave submitted to vcs1 port0 then it will stay there until whatever >> is in vcs0 port0 finishes and lets master in. >> >> Do you see a possibility for things to go bad? > > Because the master is already submitted in port1, the bond can go into > port0. Then a second bond turns up for the master in port0, and we're > back at square one. > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index 37855ae8f8b3..698608e11df3 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -2502,6 +2502,7 @@ static void eb_request_add(struct i915_execbuffer *eb) > lockdep_unpin_lock(&tl->mutex, rq->cookie); > > trace_i915_request_add(rq); > + set_bit(I915_FENCE_FLAG_SENTINEL, &rq->fence.flags); > > prev = __i915_request_commit(rq); > > Will do the trick. > > (Plus fixing up the rules for assert_pending_valid). Hmm yes, my logic was flawed by missing to see the async disconnect between master and slave submission on both ends. That's why Xiaogang was saying slaves must not have no-preempt set... But sentinel on everything? Or just everything vcs and gen11+? So if we indeed had slave preemptible the deadlock would have been avoided I think, but can the media pipeline handle that is the question. Another question is that it sounds it could be possible to work around this in userspace, combined with this patch (original thread), if a fence was used to block master until slave is submitted. split_fence = sw_fence_create() execbuf(master, in_fence = split_fence) = master_fence execbuf(slave, submit_fence = master_fence) sw_fence_advance(split_fence) That would make sure the single port and no-preempt properties are applied before either master or slave can enter elsp. Sounds tempting to try, thoughts? Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index 07be021882cc..576d11c0cdd9 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -212,7 +212,7 @@ intel_context_force_single_submission(const struct intel_context *ce) static inline void intel_context_set_single_submission(struct intel_context *ce) { - __set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); + set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ce->flags); } static inline bool diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 3214a4ecc31a..88cf20fd92c8 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1734,8 +1734,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) static bool ctx_single_port_submission(const struct intel_context *ce) { - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && - intel_context_force_single_submission(ce)); + return intel_context_force_single_submission(ce); } static bool can_merge_ctx(const struct intel_context *prev, @@ -4929,9 +4928,41 @@ static void execlists_park(struct intel_engine_cs *engine) cancel_timer(&engine->execlists.preempt); } +static void +mark_bonded_pair(struct i915_request *rq, struct i915_request *signal) +{ + /* + * Give (temporary) special meaning to a pair requests with requested + * aligned start along the video engines. + * + * They should be non-preemptable and have all ELSP ports to themselves + * to avoid any deadlocks caused by inversions. + * + * Gen11+ + */ + if (INTEL_GEN(rq->i915) < 11 || + rq->engine->class != VIDEO_DECODE_CLASS || + rq->engine->class != signal->engine->class) + return; + + set_bit(I915_FENCE_FLAG_NOPREEMPT, &rq->fence.flags); + set_bit(I915_FENCE_FLAG_NOPREEMPT, &signal->fence.flags); + + intel_context_set_single_submission(rq->context); + intel_context_set_single_submission(signal->context); +} + +static void +execlists_bond_execute(struct i915_request *rq, struct dma_fence *signal) +{ + mark_bonded_pair(rq, to_request(signal)); +} + void intel_execlists_set_default_submission(struct intel_engine_cs *engine) { engine->submit_request = execlists_submit_request; + if (engine->class == VIDEO_DECODE_CLASS) + engine->bond_execute = execlists_bond_execute; engine->schedule = i915_schedule; engine->execlists.tasklet.func = execlists_submission_tasklet; @@ -5602,15 +5633,18 @@ virtual_find_bond(struct virtual_engine *ve, } static void -virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal) +virtual_bond_execute(struct i915_request *rq, struct dma_fence *sigfence) { struct virtual_engine *ve = to_virtual_engine(rq->engine); + struct i915_request *signal = to_request(sigfence); intel_engine_mask_t allowed, exec; struct ve_bond *bond; - allowed = ~to_request(signal)->engine->mask; + mark_bonded_pair(rq, signal); + + allowed = ~signal->engine->mask; - bond = virtual_find_bond(ve, to_request(signal)->engine); + bond = virtual_find_bond(ve, signal->engine); if (bond) allowed &= bond->sibling_mask; @@ -5620,7 +5654,7 @@ virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal) ; /* Prevent the master from being re-run on the bonded engines */ - to_request(signal)->execution_mask &= ~allowed; + signal->execution_mask &= ~allowed; } struct intel_context *