[Intel-gfx] [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly

[Tvrtko Ursulin]

On 10/03/2017 10:09, Chris Wilson wrote:
> On Fri, Mar 10, 2017 at 09:57:47AM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> If we avoid initializing forcewake domains when running as
>> a guest, and also use gen2 mmio accessors in that case, we
>> can avoid the timer traffic and any looping through the
>> forcewake code which is currently just so it can end up in
>> the no-op forcewake implementation.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Weinan Li <weinan.z.li at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/i915/intel_uncore.c | 76 +++++++++++++------------------------
>>  1 file changed, 27 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 71b9b387ad04..09f5f02d7901 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -138,13 +138,6 @@ fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_doma
>>  }
>>
>>  static void
>> -vgpu_fw_domains_nop(struct drm_i915_private *dev_priv,
>> -		    enum forcewake_domains fw_domains)
>> -{
>> -	/* Guest driver doesn't need to takes care forcewake. */
>> -}
>> -
>> -static void
>>  fw_domains_posting_read(struct drm_i915_private *dev_priv)
>>  {
>>  	struct intel_uncore_forcewake_domain *d;
>> @@ -1187,7 +1180,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
>>
>>  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>>  {
>> -	if (INTEL_INFO(dev_priv)->gen <= 5)
>> +	if (INTEL_GEN(dev_priv) <= 5 || intel_vgpu_active(dev_priv))
>
> Make these separate ifs, they aren't semantically related so be verbose.
>
>>  		return;
>>
>>  	if (IS_GEN9(dev_priv)) {
>> @@ -1273,11 +1266,6 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>>  			       FORCEWAKE, FORCEWAKE_ACK);
>>  	}
>>
>> -	if (intel_vgpu_active(dev_priv)) {
>> -		dev_priv->uncore.funcs.force_wake_get = vgpu_fw_domains_nop;
>> -		dev_priv->uncore.funcs.force_wake_put = vgpu_fw_domains_nop;
>> -	}
>> -
>>  	/* All future platforms are expected to require complex power gating */
>>  	WARN_ON(dev_priv->uncore.fw_domains == 0);
>>  }
>> @@ -1327,22 +1315,22 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
>>  	dev_priv->uncore.pmic_bus_access_nb.notifier_call =
>>  		i915_pmic_bus_access_notifier;
>>
>> -	switch (INTEL_INFO(dev_priv)->gen) {
>> -	default:
>> -	case 9:
>> -		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
>> -		ASSIGN_WRITE_MMIO_VFUNCS(fwtable);
>> -		ASSIGN_READ_MMIO_VFUNCS(fwtable);
>> -		if (HAS_DECOUPLED_MMIO(dev_priv)) {
>> -			dev_priv->uncore.funcs.mmio_readl =
>> -						gen9_decoupled_read32;
>> -			dev_priv->uncore.funcs.mmio_readq =
>> -						gen9_decoupled_read64;
>> -			dev_priv->uncore.funcs.mmio_writel =
>> -						gen9_decoupled_write32;
>> +	if (IS_GEN(dev_priv, 2, 4) || intel_vgpu_active(dev_priv)) {
>
> Ok, this doesn't look too bad.
>
> Do the gvt-g hosts in CI now provide coverage for us of vgpu paths?

No idea.

Adding Zhenyu. So this patch avoids burning CPU cycles in guests and 
scheduling timers when all of that ends up in the dummy/no-op forcewake 
implementation.

If interesting to you, would it be easy for you to test it or how should 
we proceed?

Regards,

Tvrtko