Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Fri, Mar 10, 2017 at 09:57:47AM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
>> Cc: Weinan Li <weinan.z.li@intel.com>
>> Cc: Chris Wilson <chris@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.

First I thought that he hates switches :) But yup, not bad.
-Mika

>
> Do the gvt-g hosts in CI now provide coverage for us of vgpu paths?
> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx