diff mbox series

[v2] drm/i915: Don't check power domains state in intel_power_domains_init_hw()

Message ID 20180828122231.14336-1-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: Don't check power domains state in intel_power_domains_init_hw() | expand

Commit Message

Imre Deak Aug. 28, 2018, 12:22 p.m. UTC
During power domains initialization we acquire power well references for
power wells in the INIT power domain. The rest of power wells - which
BIOS could have left enabled - we can only acquire references as needed
during display HW readout and so must defer sanitization until then
(also implying that we must always do HW readout to cleanup unused power
wells).

Thus during initialization these latter power wells can have a refcount
of 0 while still being enabled. To avoid the false-positive state
mismatch error this causes remove the check from
intel_power_domains_init_hw() and rely on the state check in
intel_power_domains_enable() which follows the HW readout.

v2:
- Add comment to log and code clarifying how unused power wells get
  disabled. (Chris)

Fixes: 6dfc4a8f134f ("drm/i915: Verify power domains after enabling them")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
References: https://bugs.freedesktop.org/show_bug.cgi?id=107411
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Chris Wilson Aug. 28, 2018, 12:27 p.m. UTC | #1
Quoting Imre Deak (2018-08-28 13:22:31)
> During power domains initialization we acquire power well references for
> power wells in the INIT power domain. The rest of power wells - which
> BIOS could have left enabled - we can only acquire references as needed
> during display HW readout and so must defer sanitization until then
> (also implying that we must always do HW readout to cleanup unused power
> wells).
> 
> Thus during initialization these latter power wells can have a refcount
> of 0 while still being enabled. To avoid the false-positive state
> mismatch error this causes remove the check from
> intel_power_domains_init_hw() and rely on the state check in
> intel_power_domains_enable() which follows the HW readout.
> 
> v2:
> - Add comment to log and code clarifying how unused power wells get
>   disabled. (Chris)
> 
> Fixes: 6dfc4a8f134f ("drm/i915: Verify power domains after enabling them")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107411
> Signed-off-by: Imre Deak <imre.deak@intel.com>

That's enough to allow me to keep following along,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 2852395125cd..480dadb1047b 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3724,9 +3724,10 @@  static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);
  *
  * This function initializes the hardware power domain state and enables all
  * 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().
+ * domains (and not in the INIT domain) are referenced or disabled by
+ * intel_modeset_readout_hw_state(). After that the reference count of each
+ * power well must match its HW enabled state, see
+ * intel_power_domains_verify_state().
  *
  * It will return with power domains disabled (to be enabled later by
  * intel_power_domains_enable()) and must be paired with
@@ -3767,9 +3768,8 @@  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
 	if (!i915_modparams.disable_power_well)
 		intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 	intel_power_domains_sync_hw(dev_priv);
+
 	power_domains->initializing = false;
-
-	intel_power_domains_verify_state(dev_priv);
 }
 
 /**