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

Wed Nov 29 11:34:27 UTC 2017

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?

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
> 