On Wed, Nov 29, 2017 at 10:59:27AM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > It seems that the DMC likes to transition between the DC states a lot when > there are no connected displays (no active power domains) during command > submission. > > This activity on DC states has a negative impact on the performance of the > chip with huge latencies observed in the interrupt handlers and elsewhere. > Simple tests like igt/gem_latency -n 0 are slowed down by a factor of > eight. > > Work around it by introducing a new power domain named, > POWER_DOMAIN_GT_IRQ, associtated with the "DC off" power well, which is > held for the duration of command submission activity. > > v2: > * Add commit text as comment in i915_gem_mark_busy. (Chris Wilson) > * Protect macro body with braces. (Jani Nikula) > > v3: > * Add dedicated power domain for clarity. (Chris, Imre) > * Commit message and comment text updates. > * Apply to all big-core GEN9 parts apart for Skylake which is pending DMC > firmware release. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100572 > Testcase: igt/gem_exec_nop/headless > Cc: Imre Deak <imre.deak@xxxxxxxxx> > Acked-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > drivers/gpu/drm/i915/i915_gem.c | 4 ++++ > drivers/gpu/drm/i915/i915_gem_request.c | 14 ++++++++++++++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++ > 4 files changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index bddd65839f60..17340aadc566 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -398,6 +398,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_AUX_D, > POWER_DOMAIN_GMBUS, > POWER_DOMAIN_MODESET, > + POWER_DOMAIN_GT_IRQ, > POWER_DOMAIN_INIT, > > POWER_DOMAIN_NUM, > @@ -3285,6 +3286,10 @@ intel_info(const struct drm_i915_private *dev_priv) > #define GT_FREQUENCY_MULTIPLIER 50 > #define GEN9_FREQ_SCALER 3 > > +#define NEEDS_CSR_GT_PERF_WA(dev_priv) \ > + (HAS_CSR(dev_priv) && (dev_priv)->csr.dmc_payload && \ Doesn't this check race with the async dmc loading? I.e. when you submit something right at boot-up, before DMC is fully loaded, we might end up with an unbalanced pm refcount. I think given that DMC is strongly recommended there shouldn't be a real cost with making this unconditional. With that changed: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > + IS_GEN9_BC(dev_priv) && !IS_SKYLAKE(dev_priv)) > + > #include "i915_trace.h" > > static inline bool intel_vtd_active(void) > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 354b0546a191..feec2a621120 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3381,7 +3381,11 @@ i915_gem_idle_work_handler(struct work_struct *work) > > if (INTEL_GEN(dev_priv) >= 6) > gen6_rps_idle(dev_priv); > + > intel_runtime_pm_put(dev_priv); > + > + if (NEEDS_CSR_GT_PERF_WA(dev_priv)) > + intel_display_power_put(dev_priv, POWER_DOMAIN_GT_IRQ); > out_unlock: > mutex_unlock(&dev_priv->drm.struct_mutex); > > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c > index a90bdd26571f..619377a05810 100644 > --- a/drivers/gpu/drm/i915/i915_gem_request.c > +++ b/drivers/gpu/drm/i915/i915_gem_request.c > @@ -251,6 +251,20 @@ static void mark_busy(struct drm_i915_private *i915) > > GEM_BUG_ON(!i915->gt.active_requests); > > + /* > + * It seems that the DMC likes to transition between the DC states a lot > + * when there are no connected displays (no active power domains) during > + * command submission. > + * > + * This activity has negative impact on the performance of the chip with > + * huge latencies observed in the interrupt handler and elsewhere. > + * > + * Work around it by grabbing a GT IRQ power domain whilst there is any > + * GT activity, preventing any DC state transitions. > + */ > + if (NEEDS_CSR_GT_PERF_WA(i915)) > + intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ); > + > intel_runtime_pm_get_noresume(i915); > i915->gt.awake = true; > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 8315499452dc..f491c7aaa096 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -130,6 +130,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "INIT"; > case POWER_DOMAIN_MODESET: > return "MODESET"; > + case POWER_DOMAIN_GT_IRQ: > + return "GT_IRQ"; > default: > MISSING_CASE(domain); > return "?"; > @@ -1705,6 +1707,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > BIT_ULL(POWER_DOMAIN_INIT)) > #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > + BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > BIT_ULL(POWER_DOMAIN_MODESET) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > @@ -1785,6 +1788,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > BIT_ULL(POWER_DOMAIN_INIT)) > #define GLK_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > GLK_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > + BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > BIT_ULL(POWER_DOMAIN_MODESET) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
