| Message ID | 20180716125646.6330-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/07/2018 13:56, Chris Wilson wrote: > On reset/wedging, we cancel all pending replies from the HW and we also > want to cancel an outstanding preemption event. Since we use the same > function to cancel the pending replies for reset and for a preemption > event, we can simply clear the active tracking for all. > > v2: Keep execlists_user_end() markup for wedging > v3: Move assignment to inline to hide the bare assignment. > v4: Alternate version, use user_end for clearing all leftovers > > Fixes: 60a943245413 ("drm/i915/execlists: Drop clear_gtiir() on GPU reset") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 6fef9d130d55..f9fb32efb97f 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -388,7 +388,7 @@ execlists_user_begin(struct intel_engine_execlists *execlists, > inline void > execlists_user_end(struct intel_engine_execlists *execlists) > { > - execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER); > + execlists_clear_all_active(execlists); Doesn't seem right to break separation of flag domains. Regards, Tvrtko > } > > static inline void > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index d1eee08e5f6b..665b59ba1f45 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -683,6 +683,12 @@ execlists_clear_active(struct intel_engine_execlists *execlists, > __clear_bit(bit, (unsigned long *)&execlists->active); > } > > +static inline void > +execlists_clear_all_active(struct intel_engine_execlists *execlists) > +{ > + execlists->active = 0; > +} > + > static inline bool > execlists_is_active(const struct intel_engine_execlists *execlists, > unsigned int bit) >
Quoting Chris Wilson (2018-07-16 13:56:46) > On reset/wedging, we cancel all pending replies from the HW and we also > want to cancel an outstanding preemption event. Since we use the same > function to cancel the pending replies for reset and for a preemption > event, we can simply clear the active tracking for all. > > v2: Keep execlists_user_end() markup for wedging > v3: Move assignment to inline to hide the bare assignment. > v4: Alternate version, use user_end for clearing all leftovers > > Fixes: 60a943245413 ("drm/i915/execlists: Drop clear_gtiir() on GPU reset") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 6fef9d130d55..f9fb32efb97f 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -388,7 +388,7 @@ execlists_user_begin(struct intel_engine_execlists *execlists, > inline void > execlists_user_end(struct intel_engine_execlists *execlists) > { > - execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER); > + execlists_clear_all_active(execlists); While the doing a simple assignment is nice, we lose that direct link with user == ACTIVE_USER. On the whole, I think it's acceptable to use assignment here rather than clearing the single bit. -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 6fef9d130d55..f9fb32efb97f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -388,7 +388,7 @@ execlists_user_begin(struct intel_engine_execlists *execlists, inline void execlists_user_end(struct intel_engine_execlists *execlists) { - execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER); + execlists_clear_all_active(execlists); } static inline void diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index d1eee08e5f6b..665b59ba1f45 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -683,6 +683,12 @@ execlists_clear_active(struct intel_engine_execlists *execlists, __clear_bit(bit, (unsigned long *)&execlists->active); } +static inline void +execlists_clear_all_active(struct intel_engine_execlists *execlists) +{ + execlists->active = 0; +} + static inline bool execlists_is_active(const struct intel_engine_execlists *execlists, unsigned int bit)
On reset/wedging, we cancel all pending replies from the HW and we also want to cancel an outstanding preemption event. Since we use the same function to cancel the pending replies for reset and for a preemption event, we can simply clear the active tracking for all. v2: Keep execlists_user_end() markup for wedging v3: Move assignment to inline to hide the bare assignment. v4: Alternate version, use user_end for clearing all leftovers Fixes: 60a943245413 ("drm/i915/execlists: Drop clear_gtiir() on GPU reset") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)