Commit 5af9a454 authored by Imre Deak's avatar Imre Deak Committed by Jani Nikula

drm/i915: Add helper to get a display power ref if it was already enabled

We have many places in the code where we check if a given display power
domain is enabled and if so access registers backed by this power
domain. We assumed that some modeset lock will prevent the power
reference from vanishing in the middle of the HW access, but this
assumption doesn't always hold. In such cases we get either the wakeref
not held, or an unclaimed register access error message. To fix this in
a future-proof way that's independent of other locks wrap any such
access with a get_ref_if_enabled()/put_ref() pair.

Kudos to Ville and Joonas for the ideas of this new interface.

v2:
- init the power_domains ptr when declaring it everywhere (Joonas)
v3:
- don't report the device to be powered if runtime PM is disabled

CC: Mika Kuoppala <mika.kuoppala@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1455711462-7442-1-git-send-email-imre.deak@intel.com
(cherry picked from commit 09731280)
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 81f70ba2
...@@ -1428,6 +1428,8 @@ bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv, ...@@ -1428,6 +1428,8 @@ bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain); enum intel_display_power_domain domain);
void intel_display_power_get(struct drm_i915_private *dev_priv, void intel_display_power_get(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain); enum intel_display_power_domain domain);
bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain);
void intel_display_power_put(struct drm_i915_private *dev_priv, void intel_display_power_put(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain); enum intel_display_power_domain domain);
...@@ -1514,6 +1516,7 @@ enable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv) ...@@ -1514,6 +1516,7 @@ enable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv)
enable_rpm_wakeref_asserts(dev_priv) enable_rpm_wakeref_asserts(dev_priv)
void intel_runtime_pm_get(struct drm_i915_private *dev_priv); void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv);
void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv); void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
void intel_runtime_pm_put(struct drm_i915_private *dev_priv); void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
......
...@@ -1442,6 +1442,22 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv, ...@@ -1442,6 +1442,22 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
chv_set_pipe_power_well(dev_priv, power_well, false); chv_set_pipe_power_well(dev_priv, power_well, false);
} }
static void
__intel_display_power_get_domain(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *power_well;
int i;
for_each_power_well(i, power_well, BIT(domain), power_domains) {
if (!power_well->count++)
intel_power_well_enable(dev_priv, power_well);
}
power_domains->domain_use_count[domain]++;
}
/** /**
* intel_display_power_get - grab a power domain reference * intel_display_power_get - grab a power domain reference
* @dev_priv: i915 device instance * @dev_priv: i915 device instance
...@@ -1457,24 +1473,53 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv, ...@@ -1457,24 +1473,53 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
void intel_display_power_get(struct drm_i915_private *dev_priv, void intel_display_power_get(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain) enum intel_display_power_domain domain)
{ {
struct i915_power_domains *power_domains; struct i915_power_domains *power_domains = &dev_priv->power_domains;
struct i915_power_well *power_well;
int i;
intel_runtime_pm_get(dev_priv); intel_runtime_pm_get(dev_priv);
power_domains = &dev_priv->power_domains; mutex_lock(&power_domains->lock);
__intel_display_power_get_domain(dev_priv, domain);
mutex_unlock(&power_domains->lock);
}
/**
* intel_display_power_get_if_enabled - grab a reference for an enabled display power domain
* @dev_priv: i915 device instance
* @domain: power domain to reference
*
* This function grabs a power domain reference for @domain and ensures that the
* power domain and all its parents are powered up. Therefore users should only
* grab a reference to the innermost power domain they need.
*
* Any power domain reference obtained by this function must have a symmetric
* call to intel_display_power_put() to release the reference again.
*/
bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain)
{
struct i915_power_domains *power_domains = &dev_priv->power_domains;
bool is_enabled;
if (!intel_runtime_pm_get_if_in_use(dev_priv))
return false;
mutex_lock(&power_domains->lock); mutex_lock(&power_domains->lock);
for_each_power_well(i, power_well, BIT(domain), power_domains) { if (__intel_display_power_is_enabled(dev_priv, domain)) {
if (!power_well->count++) __intel_display_power_get_domain(dev_priv, domain);
intel_power_well_enable(dev_priv, power_well); is_enabled = true;
} else {
is_enabled = false;
} }
power_domains->domain_use_count[domain]++;
mutex_unlock(&power_domains->lock); mutex_unlock(&power_domains->lock);
if (!is_enabled)
intel_runtime_pm_put(dev_priv);
return is_enabled;
} }
/** /**
...@@ -2245,6 +2290,43 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv) ...@@ -2245,6 +2290,43 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
assert_rpm_wakelock_held(dev_priv); assert_rpm_wakelock_held(dev_priv);
} }
/**
* intel_runtime_pm_get_if_in_use - grab a runtime pm reference if device in use
* @dev_priv: i915 device instance
*
* This function grabs a device-level runtime pm reference if the device is
* already in use and ensures that it is powered up.
*
* Any runtime pm reference obtained by this function must have a symmetric
* call to intel_runtime_pm_put() to release the reference again.
*/
bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
struct device *device = &dev->pdev->dev;
int ret;
if (!IS_ENABLED(CONFIG_PM))
return true;
ret = pm_runtime_get_if_in_use(device);
/*
* In cases runtime PM is disabled by the RPM core and we get an
* -EINVAL return value we are not supposed to call this function,
* since the power state is undefined. This applies atm to the
* late/early system suspend/resume handlers.
*/
WARN_ON_ONCE(ret < 0);
if (ret <= 0)
return false;
atomic_inc(&dev_priv->pm.wakeref_count);
assert_rpm_wakelock_held(dev_priv);
return true;
}
/** /**
* intel_runtime_pm_get_noresume - grab a runtime pm reference * intel_runtime_pm_get_noresume - grab a runtime pm reference
* @dev_priv: i915 device instance * @dev_priv: i915 device instance
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment