diff mbox

[19/41] drm/i915: use power get/put instead of set for power on after init

Message ID 1407146233-24026-20-git-send-email-dheerajx.s.jamwal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dheeraj Jamwal Aug. 4, 2014, 9:56 a.m. UTC 
From: Imre Deak <imre.deak@intel.com>

Currently we make sure that all power domains are enabled during driver
init and turn off unneded ones only after the first modeset. Similarly
during suspend we enable all power domains, which will remain on through
the following resume until the first modeset.

This logic is supported by intel_set_power_well() in the power domain
framework. It would be nice to simplify the API, so that we only have
get/put functions and make it more explicit on the higher level how this
"power well on during init" logic works. This will make it also easier
if in the future we want to shorten the time the power wells are on.

For this add a new device private flag tracking whether we have the
power wells on because of init/suspend and use only
intel_display_power_get()/put(). As nothing else uses
intel_set_power_well() we can remove it.

This also fixes

commit 6efdf354ddb186c6604d1692075421e8d2c740e9
Author: Imre Deak <imre.deak@intel.com>
Date:   Wed Oct 16 17:25:52 2013 +0300

    drm/i915: enable only the needed power domains during modeset

where removing intel_set_power_well() resulted in not releasing the
reference on the power well that was taken during init and thus leaving
the power well on all the time. Regression reported by Paulo.

v2:
- move the init_power_on flag to the power_domains struct (Daniel)

v3:
- add note about this being a regression fix too (Paulo)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit baa707073b6f5374a917f86f4a681149cd39dc64)

Signed-off-by: Dheeraj Jamwal <dheerajx.s.jamwal@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |    2 +-
 drivers/gpu/drm/i915/i915_drv.c      |    2 +-
 drivers/gpu/drm/i915/i915_drv.h      |    8 +++++++-
 drivers/gpu/drm/i915/intel_display.c |   17 ++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 drivers/gpu/drm/i915/intel_pm.c      |   37 +---------------------------------
 6 files changed, 28 insertions(+), 39 deletions(-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f59c2b4..4abe141 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1732,7 +1732,7 @@  int i915_driver_unload(struct drm_device *dev)
 		/* The i915.ko module is still not prepared to be loaded when
 		 * the power well is not enabled, so just enable it in case
 		 * we're going to unload/reload. */
-		intel_set_power_well(dev, true);
+		intel_display_set_init_power(dev, true);
 		i915_remove_power_well(dev);
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7e70c8c..1e3b08e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -477,7 +477,7 @@  static int i915_drm_freeze(struct drm_device *dev)
 	/* We do a lot of poking in a lot of registers, make sure they work
 	 * properly. */
 	hsw_disable_package_c8(dev_priv);
-	intel_set_power_well(dev, true);
+	intel_display_set_init_power(dev, true);
 
 	drm_kms_helper_poll_disable(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5fadafc..fa95111 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -100,6 +100,7 @@  enum intel_display_power_domain {
 	POWER_DOMAIN_TRANSCODER_C,
 	POWER_DOMAIN_TRANSCODER_EDP,
 	POWER_DOMAIN_VGA,
+	POWER_DOMAIN_INIT,
 
 	POWER_DOMAIN_NUM,
 };
@@ -913,12 +914,17 @@  struct i915_power_well {
 	struct drm_device *device;
 	/* power well enable/disable usage count */
 	int count;
-	int i915_request;
 };
 
 #define I915_MAX_POWER_WELLS 1
 
 struct i915_power_domains {
+	/*
+	 * Power wells needed for initialization at driver init and suspend
+	 * time are on. They are kept on until after the first modeset.
+	 */
+	bool init_power_on;
+
 	struct mutex lock;
 	struct i915_power_well power_wells[I915_MAX_POWER_WELLS];
 };
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bd1d0ee..ba39d12 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6573,6 +6573,21 @@  static unsigned long get_pipe_power_domains(struct drm_device *dev,
 	return mask;
 }
 
+void intel_display_set_init_power(struct drm_device *dev, bool enable)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (dev_priv->power_domains.init_power_on == enable)
+		return;
+
+	if (enable)
+		intel_display_power_get(dev, POWER_DOMAIN_INIT);
+	else
+		intel_display_power_put(dev, POWER_DOMAIN_INIT);
+
+	dev_priv->power_domains.init_power_on = enable;
+}
+
 static void modeset_update_power_wells(struct drm_device *dev)
 {
 	unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
@@ -6604,6 +6619,8 @@  static void modeset_update_power_wells(struct drm_device *dev)
 
 		crtc->enabled_power_domains = pipe_domains[crtc->pipe];
 	}
+
+	intel_display_set_init_power(dev, false);
 }
 
 static void haswell_modeset_global_resources(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index af1553c..bf4394a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -692,6 +692,7 @@  bool intel_crtc_active(struct drm_crtc *crtc);
 void i915_disable_vga_mem(struct drm_device *dev);
 void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
+void intel_display_set_init_power(struct drm_device *dev, bool enable);
 
 
 /* intel_dp.c */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5869482..5ae6d53 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5679,41 +5679,6 @@  void i915_remove_power_well(struct drm_device *dev)
 	hsw_pwr = NULL;
 }
 
-void intel_set_power_well(struct drm_device *dev, bool enable)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct i915_power_domains *power_domains = &dev_priv->power_domains;
-	struct i915_power_well *power_well;
-
-	if (!HAS_POWER_WELL(dev))
-		return;
-
-	if (!i915_disable_power_well && !enable)
-		return;
-
-	mutex_lock(&power_domains->lock);
-
-	power_well = &power_domains->power_wells[0];
-	/*
-	 * This function will only ever contribute one
-	 * to the power well reference count. i915_request
-	 * is what tracks whether we have or have not
-	 * added the one to the reference count.
-	 */
-	if (power_well->i915_request == enable)
-		goto out;
-
-	power_well->i915_request = enable;
-
-	if (enable)
-		__intel_power_well_get(power_well);
-	else
-		__intel_power_well_put(power_well);
-
- out:
-	mutex_unlock(&power_domains->lock);
-}
-
 static void intel_resume_power_well(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5745,7 +5710,7 @@  void intel_init_power_well(struct drm_device *dev)
 		return;
 
 	/* For now, we need the power well to be always enabled. */
-	intel_set_power_well(dev, true);
+	intel_display_set_init_power(dev, true);
 	intel_resume_power_well(dev);
 
 	/* We're taking over the BIOS, so clear any requests made by it since