| Message ID | 20171222141959.3006-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22/12/2017 14:19, Chris Wilson wrote: > We should only attempt to remove requests from the execution queue that > are on the execution queue. These are the requests that have been > assigned a global_seqno, so we can assert that we only attempt to remove > requests with a nonzero global_seqno. Afterwards we assert that we > remove them in order, i.e. the global_seqno matches the engine's seqno, > but that leaves a small loophole for an unattached request on an unused > engine. > > We can then make the same assertion on queuing the request to the > execution, it must have a zero global_seqno or else we are queuing the > same request twice. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_request.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c > index 59f023bb7015..d575109f7a7f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_request.c > +++ b/drivers/gpu/drm/i915/i915_gem_request.c > @@ -479,6 +479,7 @@ void __i915_gem_request_submit(struct drm_i915_gem_request *request) > /* Transfer from per-context onto the global per-engine timeline */ > timeline = engine->timeline; > GEM_BUG_ON(timeline == request->timeline); > + GEM_BUG_ON(request->global_seqno); > > seqno = timeline_get_seqno(timeline); > GEM_BUG_ON(!seqno); > @@ -525,6 +526,7 @@ void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request) > /* Only unwind in reverse order, required so that the per-context list > * is kept in seqno/ring order. > */ > + GEM_BUG_ON(!request->global_seqno); > GEM_BUG_ON(request->global_seqno != engine->timeline->seqno); > engine->timeline->seqno--; > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 59f023bb7015..d575109f7a7f 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -479,6 +479,7 @@ void __i915_gem_request_submit(struct drm_i915_gem_request *request) /* Transfer from per-context onto the global per-engine timeline */ timeline = engine->timeline; GEM_BUG_ON(timeline == request->timeline); + GEM_BUG_ON(request->global_seqno); seqno = timeline_get_seqno(timeline); GEM_BUG_ON(!seqno); @@ -525,6 +526,7 @@ void __i915_gem_request_unsubmit(struct drm_i915_gem_request *request) /* Only unwind in reverse order, required so that the per-context list * is kept in seqno/ring order. */ + GEM_BUG_ON(!request->global_seqno); GEM_BUG_ON(request->global_seqno != engine->timeline->seqno); engine->timeline->seqno--;
We should only attempt to remove requests from the execution queue that are on the execution queue. These are the requests that have been assigned a global_seqno, so we can assert that we only attempt to remove requests with a nonzero global_seqno. Afterwards we assert that we remove them in order, i.e. the global_seqno matches the engine's seqno, but that leaves a small loophole for an unattached request on an unused engine. We can then make the same assertion on queuing the request to the execution, it must have a zero global_seqno or else we are queuing the same request twice. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> --- drivers/gpu/drm/i915/i915_gem_request.c | 2 ++ 1 file changed, 2 insertions(+)