[Intel-gfx] [PATCH] drm/i915: Restore GT performance in headless mode with DMC loaded
Daniel Vetter
daniel at ffwll.ch
Wed Nov 29 12:47:13 UTC 2017
On Wed, Nov 29, 2017 at 11:34:27AM +0000, Tvrtko Ursulin wrote:
>
> On 29/11/2017 11:12, Daniel Vetter wrote:
> > On Wed, Nov 29, 2017 at 10:59:27AM +0000, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > >
> > > 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 at intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100572
> > > Testcase: igt/gem_exec_nop/headless
> > > Cc: Imre Deak <imre.deak at intel.com>
> > > Acked-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
> > > ---
> > > 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.
>
> Oh yeah, well spotted.
>
> > I think given that DMC is strongly recommended there shouldn't be a real
> > cost with making this unconditional.
>
> I don't know, not liking it on the first go. But then again I have no idea
> how much power would that waste for use cases where DMC fw is deliberately
> not present.
>
> Perhaps it would be acceptable to mark GT busy during the async CSR load.
> Chris, any thoughts?
I only meant that we pm_put without pm_get (because when we would have
called pm_get dmc_payload == NULL and then != NULL when we reach the check
for pm_put).
The actual get/put vs. dmc loading should be protected already by the
async dmc load code holding pm references to prevent any havoc.
-Daniel
>
> Regards,
>
> Tvrtko
>
> > With that changed:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >
> > > + 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 at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list