| Message ID | 20191025090952.10135-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Move intel_engine_context_in/out into intel_lrc.c | expand |
Quoting Tvrtko Ursulin (2019-10-25 10:09:52) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Intel_lrc.c is the only caller and so to avoid some header file ordering > issues in future patches move these two over there. How much pain would you feel if we did intel_lrc.c + intel_execlists_submission.c earlier rather than later? > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine.h | 55 -------------------------- > drivers/gpu/drm/i915/gt/intel_lrc.c | 55 ++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+), 55 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h > index 97bbdd9773c9..c6895938b626 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h > @@ -290,61 +290,6 @@ void intel_engine_dump(struct intel_engine_cs *engine, > struct drm_printer *m, > const char *header, ...); > > -static inline void intel_engine_context_in(struct intel_engine_cs *engine) > -{ > - unsigned long flags; > - > - if (READ_ONCE(engine->stats.enabled) == 0) > - return; > - > - write_seqlock_irqsave(&engine->stats.lock, flags); > - > - if (engine->stats.enabled > 0) { > - if (engine->stats.active++ == 0) > - engine->stats.start = ktime_get(); > - GEM_BUG_ON(engine->stats.active == 0); > - } > - > - write_sequnlock_irqrestore(&engine->stats.lock, flags); > -} > - > -static inline void intel_engine_context_out(struct intel_engine_cs *engine) > -{ > - unsigned long flags; > - > - if (READ_ONCE(engine->stats.enabled) == 0) > - return; > - > - write_seqlock_irqsave(&engine->stats.lock, flags); > - > - if (engine->stats.enabled > 0) { > - ktime_t last; > - > - if (engine->stats.active && --engine->stats.active == 0) { > - /* > - * Decrement the active context count and in case GPU > - * is now idle add up to the running total. > - */ > - last = ktime_sub(ktime_get(), engine->stats.start); > - > - engine->stats.total = ktime_add(engine->stats.total, > - last); > - } else if (engine->stats.active == 0) { > - /* > - * After turning on engine stats, context out might be > - * the first event in which case we account from the > - * time stats gathering was turned on. > - */ > - last = ktime_sub(ktime_get(), engine->stats.enabled_at); > - > - engine->stats.total = ktime_add(engine->stats.total, > - last); > - } > - } > - > - write_sequnlock_irqrestore(&engine->stats.lock, flags); > -} > - > int intel_enable_engine_stats(struct intel_engine_cs *engine); > void intel_disable_engine_stats(struct intel_engine_cs *engine); > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index 73eae85a2cc9..523de1fd4452 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -944,6 +944,61 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status) > status, rq); > } > > +static void intel_engine_context_in(struct intel_engine_cs *engine) stats_in() / stats_out() ? Now that's it entirely local and we may end up doing other per-context in/out ops? Purely mechanical, so Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On 25/10/2019 10:13, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-10-25 10:09:52) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Intel_lrc.c is the only caller and so to avoid some header file ordering >> issues in future patches move these two over there. > > How much pain would you feel if we did intel_lrc.c + > intel_execlists_submission.c earlier rather than later? Par for course as you like to say. :) >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_engine.h | 55 -------------------------- >> drivers/gpu/drm/i915/gt/intel_lrc.c | 55 ++++++++++++++++++++++++++ >> 2 files changed, 55 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h >> index 97bbdd9773c9..c6895938b626 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_engine.h >> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h >> @@ -290,61 +290,6 @@ void intel_engine_dump(struct intel_engine_cs *engine, >> struct drm_printer *m, >> const char *header, ...); >> >> -static inline void intel_engine_context_in(struct intel_engine_cs *engine) >> -{ >> - unsigned long flags; >> - >> - if (READ_ONCE(engine->stats.enabled) == 0) >> - return; >> - >> - write_seqlock_irqsave(&engine->stats.lock, flags); >> - >> - if (engine->stats.enabled > 0) { >> - if (engine->stats.active++ == 0) >> - engine->stats.start = ktime_get(); >> - GEM_BUG_ON(engine->stats.active == 0); >> - } >> - >> - write_sequnlock_irqrestore(&engine->stats.lock, flags); >> -} >> - >> -static inline void intel_engine_context_out(struct intel_engine_cs *engine) >> -{ >> - unsigned long flags; >> - >> - if (READ_ONCE(engine->stats.enabled) == 0) >> - return; >> - >> - write_seqlock_irqsave(&engine->stats.lock, flags); >> - >> - if (engine->stats.enabled > 0) { >> - ktime_t last; >> - >> - if (engine->stats.active && --engine->stats.active == 0) { >> - /* >> - * Decrement the active context count and in case GPU >> - * is now idle add up to the running total. >> - */ >> - last = ktime_sub(ktime_get(), engine->stats.start); >> - >> - engine->stats.total = ktime_add(engine->stats.total, >> - last); >> - } else if (engine->stats.active == 0) { >> - /* >> - * After turning on engine stats, context out might be >> - * the first event in which case we account from the >> - * time stats gathering was turned on. >> - */ >> - last = ktime_sub(ktime_get(), engine->stats.enabled_at); >> - >> - engine->stats.total = ktime_add(engine->stats.total, >> - last); >> - } >> - } >> - >> - write_sequnlock_irqrestore(&engine->stats.lock, flags); >> -} >> - >> int intel_enable_engine_stats(struct intel_engine_cs *engine); >> void intel_disable_engine_stats(struct intel_engine_cs *engine); >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c >> index 73eae85a2cc9..523de1fd4452 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c >> @@ -944,6 +944,61 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status) >> status, rq); >> } >> >> +static void intel_engine_context_in(struct intel_engine_cs *engine) > > stats_in() / stats_out() ? > > Now that's it entirely local and we may end up doing other per-context > in/out ops? Yeah, could make sense. I did rename it to intel_context_in/out in a local patch which adds per ce stats. Lets see when I un-bit-rot that work how it will look. > Purely mechanical, so > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> In the meantime I have pushed this so at least header file is smaller. Thanks. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 97bbdd9773c9..c6895938b626 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -290,61 +290,6 @@ void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m, const char *header, ...); -static inline void intel_engine_context_in(struct intel_engine_cs *engine) -{ - unsigned long flags; - - if (READ_ONCE(engine->stats.enabled) == 0) - return; - - write_seqlock_irqsave(&engine->stats.lock, flags); - - if (engine->stats.enabled > 0) { - if (engine->stats.active++ == 0) - engine->stats.start = ktime_get(); - GEM_BUG_ON(engine->stats.active == 0); - } - - write_sequnlock_irqrestore(&engine->stats.lock, flags); -} - -static inline void intel_engine_context_out(struct intel_engine_cs *engine) -{ - unsigned long flags; - - if (READ_ONCE(engine->stats.enabled) == 0) - return; - - write_seqlock_irqsave(&engine->stats.lock, flags); - - if (engine->stats.enabled > 0) { - ktime_t last; - - if (engine->stats.active && --engine->stats.active == 0) { - /* - * Decrement the active context count and in case GPU - * is now idle add up to the running total. - */ - last = ktime_sub(ktime_get(), engine->stats.start); - - engine->stats.total = ktime_add(engine->stats.total, - last); - } else if (engine->stats.active == 0) { - /* - * After turning on engine stats, context out might be - * the first event in which case we account from the - * time stats gathering was turned on. - */ - last = ktime_sub(ktime_get(), engine->stats.enabled_at); - - engine->stats.total = ktime_add(engine->stats.total, - last); - } - } - - write_sequnlock_irqrestore(&engine->stats.lock, flags); -} - int intel_enable_engine_stats(struct intel_engine_cs *engine); void intel_disable_engine_stats(struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 73eae85a2cc9..523de1fd4452 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -944,6 +944,61 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status) status, rq); } +static void intel_engine_context_in(struct intel_engine_cs *engine) +{ + unsigned long flags; + + if (READ_ONCE(engine->stats.enabled) == 0) + return; + + write_seqlock_irqsave(&engine->stats.lock, flags); + + if (engine->stats.enabled > 0) { + if (engine->stats.active++ == 0) + engine->stats.start = ktime_get(); + GEM_BUG_ON(engine->stats.active == 0); + } + + write_sequnlock_irqrestore(&engine->stats.lock, flags); +} + +static void intel_engine_context_out(struct intel_engine_cs *engine) +{ + unsigned long flags; + + if (READ_ONCE(engine->stats.enabled) == 0) + return; + + write_seqlock_irqsave(&engine->stats.lock, flags); + + if (engine->stats.enabled > 0) { + ktime_t last; + + if (engine->stats.active && --engine->stats.active == 0) { + /* + * Decrement the active context count and in case GPU + * is now idle add up to the running total. + */ + last = ktime_sub(ktime_get(), engine->stats.start); + + engine->stats.total = ktime_add(engine->stats.total, + last); + } else if (engine->stats.active == 0) { + /* + * After turning on engine stats, context out might be + * the first event in which case we account from the + * time stats gathering was turned on. + */ + last = ktime_sub(ktime_get(), engine->stats.enabled_at); + + engine->stats.total = ktime_add(engine->stats.total, + last); + } + } + + write_sequnlock_irqrestore(&engine->stats.lock, flags); +} + static inline struct intel_engine_cs * __execlists_schedule_in(struct i915_request *rq) {