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@xxxxxxxxx>
> Cc: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
