| Message ID | 20180514093710.7730-9-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 14/05/2018 10:37, Chris Wilson wrote: > Store whether or not we need to kick the guc's execlists emulation on > the engine itself to avoid chasing the device info. > > gen8_cs_irq_handler 512 428 -84 Impressive, or not, depends whether you look at the saving or code generation. Hm, actually, is it just GEM_BUG_ON in intel_uc_is_using_guc_submission? But blimey, didn't we pencil in long time ago to stop using modparams at runtime in favour of caching the values? This one looks like one to confuse the driver easily if fiddled with at runtime. Regards, Tvrtko > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_irq.c | 2 +- > drivers/gpu/drm/i915/intel_guc_submission.c | 1 + > drivers/gpu/drm/i915/intel_lrc.c | 1 + > drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++++++ > 4 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 034c603867e6..f8c39e5b5cc6 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1467,7 +1467,7 @@ static void gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) > set_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); > > if (iir & GT_RENDER_USER_INTERRUPT) { > - if (USES_GUC_SUBMISSION(engine->i915)) > + if (intel_engine_uses_guc(engine)) > set_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); > > notify_ring(engine); > diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c > index 4e09abf7e206..eb2a25d84a82 100644 > --- a/drivers/gpu/drm/i915/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/intel_guc_submission.c > @@ -1277,6 +1277,7 @@ int intel_guc_submission_enable(struct intel_guc *guc) > engine->unpark = guc_submission_unpark; > > engine->flags &= ~I915_ENGINE_SUPPORTS_STATS; > + engine->flags |= I915_ENGINE_USES_GUC; > } > > return 0; > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 70f1b5dd492a..90b534098d00 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2264,6 +2264,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) > engine->park = NULL; > engine->unpark = NULL; > > + engine->flags &= ~I915_ENGINE_USES_GUC; > engine->flags |= I915_ENGINE_SUPPORTS_STATS; > if (engine->i915->preempt_context) > engine->flags |= I915_ENGINE_HAS_PREEMPTION; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index f6ba354faf89..90d1ca2ca232 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -570,6 +570,7 @@ struct intel_engine_cs { > #define I915_ENGINE_NEEDS_CMD_PARSER BIT(0) > #define I915_ENGINE_SUPPORTS_STATS BIT(1) > #define I915_ENGINE_HAS_PREEMPTION BIT(2) > +#define I915_ENGINE_USES_GUC BIT(3) > unsigned int flags; > > /* > @@ -647,6 +648,12 @@ intel_engine_has_preemption(const struct intel_engine_cs *engine) > return engine->flags & I915_ENGINE_HAS_PREEMPTION; > } > > +static inline bool > +intel_engine_uses_guc(const struct intel_engine_cs *engine) > +{ > + return engine->flags & I915_ENGINE_USES_GUC; > +} > + > static inline bool __execlists_need_preempt(int prio, int last) > { > return prio > max(0, last); >
Quoting Tvrtko Ursulin (2018-05-14 11:27:56) > > On 14/05/2018 10:37, Chris Wilson wrote: > > Store whether or not we need to kick the guc's execlists emulation on > > the engine itself to avoid chasing the device info. > > > > gen8_cs_irq_handler 512 428 -84 > > Impressive, or not, depends whether you look at the saving or code > generation. The code generation also looked better, at least in my eyes. Fewer chained movs and simpler test? > Hm, actually, is it just GEM_BUG_ON in intel_uc_is_using_guc_submission? > > But blimey, didn't we pencil in long time ago to stop using modparams at > runtime in favour of caching the values? This one looks like one to > confuse the driver easily if fiddled with at runtime. Yes, this is one where once upon a time we had an engine->guc_client to inspect that we traded in for a common i915->guc_client, but that will still require a pointer dance. -Chris
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 034c603867e6..f8c39e5b5cc6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1467,7 +1467,7 @@ static void gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) set_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); if (iir & GT_RENDER_USER_INTERRUPT) { - if (USES_GUC_SUBMISSION(engine->i915)) + if (intel_engine_uses_guc(engine)) set_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); notify_ring(engine); diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 4e09abf7e206..eb2a25d84a82 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -1277,6 +1277,7 @@ int intel_guc_submission_enable(struct intel_guc *guc) engine->unpark = guc_submission_unpark; engine->flags &= ~I915_ENGINE_SUPPORTS_STATS; + engine->flags |= I915_ENGINE_USES_GUC; } return 0; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 70f1b5dd492a..90b534098d00 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2264,6 +2264,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) engine->park = NULL; engine->unpark = NULL; + engine->flags &= ~I915_ENGINE_USES_GUC; engine->flags |= I915_ENGINE_SUPPORTS_STATS; if (engine->i915->preempt_context) engine->flags |= I915_ENGINE_HAS_PREEMPTION; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index f6ba354faf89..90d1ca2ca232 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -570,6 +570,7 @@ struct intel_engine_cs { #define I915_ENGINE_NEEDS_CMD_PARSER BIT(0) #define I915_ENGINE_SUPPORTS_STATS BIT(1) #define I915_ENGINE_HAS_PREEMPTION BIT(2) +#define I915_ENGINE_USES_GUC BIT(3) unsigned int flags; /* @@ -647,6 +648,12 @@ intel_engine_has_preemption(const struct intel_engine_cs *engine) return engine->flags & I915_ENGINE_HAS_PREEMPTION; } +static inline bool +intel_engine_uses_guc(const struct intel_engine_cs *engine) +{ + return engine->flags & I915_ENGINE_USES_GUC; +} + static inline bool __execlists_need_preempt(int prio, int last) { return prio > max(0, last);
Store whether or not we need to kick the guc's execlists emulation on the engine itself to avoid chasing the device info. gen8_cs_irq_handler 512 428 -84 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_guc_submission.c | 1 + drivers/gpu/drm/i915/intel_lrc.c | 1 + drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +++++++ 4 files changed, 10 insertions(+), 1 deletion(-)