[Intel-gfx] [PATCH v2 5/5] drm/i915: Add power well SW/HW state verification

[Ander Conselvan De Oliveira]

On Fri, 2017-02-17 at 17:39 +0200, Imre Deak wrote:
> Verify that the refcount of all power wells match their HW enabled
> state at the end of modeset HW state readout.
> 
> Also add documentation on how the reference count for each power well is
> supposed to be acquired during initialization and HW state readout.
> 
> Suggested by Ander.
> 
> Cc: Ander Conselvan de Oliveira <conselvan2 at gmail.com>
> Cc: David Weinehall <david.weinehall at linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c    |  2 +
>  drivers/gpu/drm/i915/intel_drv.h        |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 85 ++++++++++++++++++++++++++++++++-
>  3 files changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 88b7d96..00c3fd8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15572,6 +15572,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>  	}
>  	intel_display_set_init_power(dev_priv, false);
>  
> +	intel_power_domains_verify_state(dev_priv);
> +
>  	intel_fbc_init_pipe_state(dev_priv);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6e37fba..50c9329 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1695,6 +1695,7 @@ int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
>  void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
> +void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);
>  void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
>  void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 44d4da3..6b52258 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2683,7 +2683,10 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
>   * @resume: Called from resume code paths or not
>   *
>   * This function initializes the hardware power domain state and enables all
> - * power domains using intel_display_set_init_power().
> + * power wells belonging to the INIT power domain. Power wells in other
> + * domains (and not in the INIT domain) are referenced or disabled during the
> + * modeset state HW readout. After that the reference count of each power well
> + * must match its HW enabled state, see intel_power_domains_verify_state().
>   */
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  {
> @@ -2736,6 +2739,86 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
>  		bxt_display_core_uninit(dev_priv);
>  }
>  
> +static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *power_well;
> +
> +	for_each_power_well(dev_priv, power_well) {
> +		enum intel_display_power_domain domain;
> +
> +		DRM_DEBUG_DRIVER("%-25s %d\n",
> +				 power_well->name, power_well->count);
> +
> +		for_each_power_domain(domain, power_well->domains)
> +			DRM_DEBUG_DRIVER("  %-23s %d\n",
> +					 intel_display_power_domain_str(domain),
> +					 power_domains->domain_use_count[domain]);
> +	}
> +}
> +
> +/**
> + * intel_power_domains_verify_state - verify the HW/SW state for all power wells
> + * @dev_priv: i915 device instance
> + *
> + * Verify if the reference count of each power well matches its HW enabled
> + * state and the total refcount of the domains it belongs to. This must be
> + * called after modeset HW state sanitization, which is responsible for
> + * acquiring reference counts for any power wells in use and disabling the
> + * ones left on by BIOS but not required by any active output.
> + */
> +void intel_power_domains_verify_state(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *power_well;
> +	bool dump_domain_info;
> +
> +	mutex_lock(&power_domains->lock);
> +
> +	dump_domain_info = false;
> +	for_each_power_well(dev_priv, power_well) {
> +		enum intel_display_power_domain domain;
> +		int domains_count;
> +		bool enabled;
> +
> +		/*
> +		 * Power wells not belonging to any domain (like the MISC_IO
> +		 * and PW1 power wells) are under FW control, so ignore them,
> +		 * since their state can change asynchronously.
> +		 */
> +		if (!power_well->domains)
> +			continue;
> +
> +		enabled = power_well->ops->is_enabled(dev_priv, power_well);
> +		if ((power_well->count || power_well->always_on) != enabled)
> +			DRM_ERROR("power well %s state mismatch (refcount %d/enabled %d)",
> +				  power_well->name, power_well->count, enabled);
> +
> +		domains_count = 0;
> +		for_each_power_domain(domain, power_well->domains)
> +			domains_count += power_domains->domain_use_count[domain];
> +
> +		if (power_well->count != domains_count) {
> +			DRM_ERROR("power well %s refcount/domain refcount mismatch "
> +				  "(refcount %d/domains refcount %d)\n",
> +				  power_well->name, power_well->count,
> +				  domains_count);
> +			dump_domain_info = true;
> +		}
> +	}
> +
> +	if (dump_domain_info) {
> +		static bool dumped;
> +
> +		if (!dumped) {
> +			intel_power_domains_dump_info(dev_priv);
> +			dumped = true;
> +		}
> +	}
> +
> +	mutex_unlock(&power_domains->lock);
> +}
> +
>  /**
>   * intel_runtime_pm_get - grab a runtime pm reference
>   * @dev_priv: i915 device instance