[Intel-gfx] [PATCH] drm/i915: Restore GT performance in headless mode with DMC loaded

Wed Nov 29 11:12:38 UTC 2017

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.

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 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