[PATCH 00/19] Make PC8 become part of runtime PM

[PATCH 00/19] Make PC8 become part of runtime PM

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi

This patch series makes the PC8 feature become part of the runtime PM feature.
They won't be 2 separate features anymore, but just one. You can find some good
arguments for this in the commit message of patch 5.

  - Patch 1 is just a bikeshed that can be discarded if needed.
  - Patches 2-4 do some necessary work to merge PC8 and runtime PM.
  - Patch 5 actually merges the two features. It just does the minimal work
    required to merge the feature and pass all the tests on the test suite.
  - Patches 6-19 do many cleanups that become possible since the features got
    merged. Each patch contains just a very small chunk of code, to make it
    easier for reviewers (also for tracking regressions while I was developing
    the code!).

I hope that with this, it will also be much easier to add runtime PM support on
other platforms. I also think I addressed many of Daniel's comments regarding
how we do the put/get calls, but there may still be something left: please tell
me if you find something!

I also didn't take care of implementing the interrupt changes suggested by
Daniel. This won't be trivial and will require its own patch series.

Thanks,
Paulo

Paulo Zanoni (19):
  drm/i915: rename modeset_update_power_wells
  drm/i915: get/put runtime PM without holding rps.hw_lock
  drm/i915: add forcewake functions that don't touch runtime PM
  drm/i915: extract __hsw_do_{en,dis}able_package_c8
  drm/i915: make PC8 be part of runtime PM suspend/resume
  drm/i915: get/put runtime PM when we get/put a power domain
  drm/i915: remove dev_priv->pc8.requirements_met
  drm/i915: make gpu_idle be part of runtime PM, not PC8
  drm/i915: kill pc8.disable_count
  drm/i915: remove an indirection level on PC8 functions
  drm/i915: don't get/put PC8 reference on freeze/thaw
  drm/i915: get/put runtime PM references for GMBUS and DP AUX
  drm/i915: don't get/put PC8 when getting/putting power wells
  drm/i915: remove dev_priv->pc8.enabled
  drm/i915: move pc8.irqs_disabled to pm.irqs_disabled
  drm/i915: kill struct i915_package_c8
  drm/i915: rename __hsw_do_{en,dis}able_pc8
  drm/i915: update the PC8 and runtime PM documentation.
  drm/i915: init pm.suspended earlier

 drivers/gpu/drm/i915/i915_debugfs.c  |  23 ++---
 drivers/gpu/drm/i915/i915_dma.c      |   2 -
 drivers/gpu/drm/i915/i915_drv.c      |  19 ++--
 drivers/gpu/drm/i915/i915_drv.h      |  74 +++++---------
 drivers/gpu/drm/i915/i915_gem.c      |   2 +-
 drivers/gpu/drm/i915/i915_irq.c      |  58 +++++------
 drivers/gpu/drm/i915/intel_display.c | 186 +++++++++--------------------------
 drivers/gpu/drm/i915/intel_dp.c      |   4 +-
 drivers/gpu/drm/i915/intel_drv.h     |  11 +--
 drivers/gpu/drm/i915/intel_i2c.c     |   4 +-
 drivers/gpu/drm/i915/intel_pm.c      |  45 +++------
 drivers/gpu/drm/i915/intel_uncore.c  |  30 ++++--
 12 files changed, 158 insertions(+), 300 deletions(-)

-- 
1.8.3.1

[PATCH 01/19] drm/i915: rename modeset_update_power_wells

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

To modeset_update_crtc_power_domains, since this function is
responsible for updating all the power domains of all CRTCs after a
modeset. In the future we should also run this function on all
platforms, not just Haswell.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d1357a..c661423 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6860,7 +6860,7 @@ void intel_display_set_init_power(struct drm_device *dev, bool enable)
 	dev_priv->power_domains.init_power_on = enable;
 }
 
-static void modeset_update_power_wells(struct drm_device *dev)
+static void modeset_update_crtc_power_domains(struct drm_device *dev)
 {
 	unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
 	struct intel_crtc *crtc;
@@ -6897,7 +6897,7 @@ static void modeset_update_power_wells(struct drm_device *dev)
 
 static void haswell_modeset_global_resources(struct drm_device *dev)
 {
-	modeset_update_power_wells(dev);
+	modeset_update_crtc_power_domains(dev);
 	hsw_update_package_c8(dev);
 }
 
-- 
1.8.3.1

[PATCH 02/19] drm/i915: get/put runtime PM without holding rps.hw_lock

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We'll need this when we merge PC8 and Runtime PM: the PC8
enable/disable functions need that lock.

Also, it's good practice to not hold a lock for longer than strictly
needed.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 430eb3e..1cdc5dd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1414,7 +1414,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	int ret;
+	int ret = 0;
 	int gpu_freq, ia_freq;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
@@ -1422,12 +1422,13 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 		return 0;
 	}
 
+	intel_runtime_pm_get(dev_priv);
+
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
 	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
-		return ret;
-	intel_runtime_pm_get(dev_priv);
+		goto out;
 
 	seq_puts(m, "GPU freq (MHz)\tEffective CPU freq (MHz)\tEffective Ring freq (MHz)\n");
 
@@ -1444,10 +1445,11 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 			   ((ia_freq >> 8) & 0xff) * 100);
 	}
 
-	intel_runtime_pm_put(dev_priv);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	return 0;
+out:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
 }
 
 static int i915_gfxec(struct seq_file *m, void *unused)
-- 
1.8.3.1

[PATCH 03/19] drm/i915: add forcewake functions that don't touch runtime PM

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

To solve a chicken-and-egg problem. Currently when we get/put
forcewake we also get/put runtime PM and this works fine because the
runtime PM code doesn't need forcewake. But we're going to merge PC8
and runtime PM into a single feature, and the PC8 code (the LCPLL
code) does need the forcewake, so that specific piece of code needs to
call the _no_rpm version so it doesn't try to get runtime PM in the
code that gets runtime PM.

For now the new functions are unused, we'll use them on the patch that
merges PC8 with runtime PM.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  4 ++++
 drivers/gpu/drm/i915/intel_uncore.c | 30 ++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cc8afff..7f8ec08 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2533,6 +2533,10 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
  */
 void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine);
 void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine);
+void gen6_gt_force_wake_get_no_rpm(struct drm_i915_private *dev_priv,
+				   int fw_engine);
+void gen6_gt_force_wake_put_no_rpm(struct drm_i915_private *dev_priv,
+				   int fw_engine);
 
 int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
 int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 646fecf..6118d2c 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -368,15 +368,11 @@ void intel_uncore_sanitize(struct drm_device *dev)
  * be called at the beginning of the sequence followed by a call to
  * gen6_gt_force_wake_put() at the end of the sequence.
  */
-void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
+void gen6_gt_force_wake_get_no_rpm(struct drm_i915_private *dev_priv,
+				   int fw_engine)
 {
 	unsigned long irqflags;
 
-	if (!dev_priv->uncore.funcs.force_wake_get)
-		return;
-
-	intel_runtime_pm_get(dev_priv);
-
 	/* Redirect to VLV specific routine */
 	if (IS_VALLEYVIEW(dev_priv->dev))
 		return vlv_force_wake_get(dev_priv, fw_engine);
@@ -387,16 +383,23 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
+{
+	if (!dev_priv->uncore.funcs.force_wake_get)
+		return;
+
+	intel_runtime_pm_get(dev_priv);
+	gen6_gt_force_wake_get_no_rpm(dev_priv, fw_engine);
+}
+
 /*
  * see gen6_gt_force_wake_get()
  */
-void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
+void gen6_gt_force_wake_put_no_rpm(struct drm_i915_private *dev_priv,
+				   int fw_engine)
 {
 	unsigned long irqflags;
 
-	if (!dev_priv->uncore.funcs.force_wake_put)
-		return;
-
 	/* Redirect to VLV specific routine */
 	if (IS_VALLEYVIEW(dev_priv->dev))
 		return vlv_force_wake_put(dev_priv, fw_engine);
@@ -410,7 +413,14 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 				 1);
 	}
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
+{
+	if (!dev_priv->uncore.funcs.force_wake_put)
+		return;
 
+	gen6_gt_force_wake_put_no_rpm(dev_priv, fw_engine);
 	intel_runtime_pm_put(dev_priv);
 }
 
-- 
1.8.3.1

[PATCH 04/19] drm/i915: extract __hsw_do_{en, dis}able_package_c8

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

When we merge PC8 and runtime PM, these new functions are going to be
called by the runtime suspend/resume functions, and their callers are
going to be removed.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 64 +++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c661423..db6523e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6638,19 +6638,11 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
-void hsw_enable_pc8_work(struct work_struct *__work)
+static void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv =
-		container_of(to_delayed_work(__work), struct drm_i915_private,
-			     pc8.enable_work);
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
 
-	WARN_ON(!HAS_PC8(dev));
-
-	if (dev_priv->pc8.enabled)
-		return;
-
 	DRM_DEBUG_KMS("Enabling package C8+\n");
 
 	dev_priv->pc8.enabled = true;
@@ -6664,7 +6656,21 @@ void hsw_enable_pc8_work(struct work_struct *__work)
 	lpt_disable_clkout_dp(dev);
 	hsw_pc8_disable_interrupts(dev);
 	hsw_disable_lcpll(dev_priv, true, true);
+}
+
+void hsw_enable_pc8_work(struct work_struct *__work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(to_delayed_work(__work), struct drm_i915_private,
+			     pc8.enable_work);
+	struct drm_device *dev = dev_priv->dev;
+
+	WARN_ON(!HAS_PC8(dev));
 
+	if (dev_priv->pc8.enabled)
+		return;
+
+	__hsw_do_enable_pc8(dev_priv);
 	intel_runtime_pm_put(dev_priv);
 }
 
@@ -6682,29 +6688,13 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 			      msecs_to_jiffies(i915_pc8_timeout));
 }
 
-static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
+static void __hsw_do_disable_package_c8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
-	WARN(dev_priv->pc8.disable_count < 0,
-	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
-
-	dev_priv->pc8.disable_count++;
-	if (dev_priv->pc8.disable_count != 1)
-		return;
-
-	WARN_ON(!HAS_PC8(dev));
-
-	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
-	if (!dev_priv->pc8.enabled)
-		return;
-
 	DRM_DEBUG_KMS("Disabling package C8+\n");
 
-	intel_runtime_pm_get(dev_priv);
-
 	hsw_restore_lcpll(dev_priv);
 	hsw_pc8_restore_interrupts(dev);
 	lpt_init_pch_refclk(dev);
@@ -6723,6 +6713,28 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 	dev_priv->pc8.enabled = false;
 }
 
+static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
+	WARN(dev_priv->pc8.disable_count < 0,
+	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
+
+	dev_priv->pc8.disable_count++;
+	if (dev_priv->pc8.disable_count != 1)
+		return;
+
+	WARN_ON(!HAS_PC8(dev));
+
+	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
+	if (!dev_priv->pc8.enabled)
+		return;
+
+	intel_runtime_pm_get(dev_priv);
+	__hsw_do_disable_package_c8(dev_priv);
+}
+
 void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_PC8(dev_priv->dev))
-- 
1.8.3.1

[PATCH 05/19] drm/i915: make PC8 be part of runtime PM suspend/resume

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Currently, when our driver becomes idle for i915_pc8_timeout (default:
5s) we enable PC8, so we save some power, but not everything we can.
Then, while PC8 is enabled, if we stay idle for more
autosuspend_delay_ms (default: 10s) we'll enter runtime PM and put the
graphics device in D3 state, saving even more power. The two features
are separate things with increasing levels of power savings, but if we
disable PC8 we'll never get into D3.

While from the modularity point of view it would be nice to keep these
features as separate, we have reasons to merge them:
 - We are not aware of anybody wanting a "PC8 without D3" environment.
 - If we keep both features as separate, we'll have to to test both
   PC8 and PC8+D3 code paths. We're already having a major pain to
   make QA do automated testing of just one thing, testing both paths
   will cost even more.
 - Only Haswell+ supports PC8, so if we want to add runtime PM support
   to, for example, IVB, we'll have to copy some code from the PC8
   feature to runtime PM, so merging both features as a single tihng
   will make it easier for enabling runtime PM on other platforms.

This patch only does the very basic steps required to have PC8 and
runtime PM merged on a single feature: the next patches will take care
of cleaning up everything.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |  2 --
 drivers/gpu/drm/i915/i915_drv.c      | 14 +++++++++---
 drivers/gpu/drm/i915/i915_drv.h      |  7 +++---
 drivers/gpu/drm/i915/intel_display.c | 43 ++++++++----------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 drivers/gpu/drm/i915/intel_pm.c      |  1 -
 6 files changed, 26 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ee9502b..ae6a622 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1760,8 +1760,6 @@ int i915_driver_unload(struct drm_device *dev)
 	cancel_work_sync(&dev_priv->gpu_error.work);
 	i915_destroy_error_state(dev);
 
-	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
-
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 61fb9fc..3b86541 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -144,11 +144,11 @@ MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time "
 
 int i915_enable_pc8 __read_mostly = 1;
 module_param_named(enable_pc8, i915_enable_pc8, int, 0600);
-MODULE_PARM_DESC(enable_pc8, "Enable support for low power package C states (PC8+) (default: true)");
+MODULE_PARM_DESC(enable_pc8, "Ignored. Deprecated by runtime PM support.");
 
-int i915_pc8_timeout __read_mostly = 5000;
+int i915_pc8_timeout __read_mostly = 0;
 module_param_named(pc8_timeout, i915_pc8_timeout, int, 0600);
-MODULE_PARM_DESC(pc8_timeout, "Number of msecs of idleness required to enter PC8+ (default: 5000)");
+MODULE_PARM_DESC(pc8_timeout, "Ignored. Deprecated by runtime PM autosuspend delay.");
 
 bool i915_prefault_disable __read_mostly;
 module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
@@ -921,12 +921,16 @@ static int i915_runtime_suspend(struct device *device)
 
 	DRM_DEBUG_KMS("Suspending device\n");
 
+	if (HAS_PC8(dev))
+		__hsw_do_enable_pc8(dev_priv);
+
 	i915_gem_release_all_mmaps(dev_priv);
 
 	del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
 	dev_priv->pm.suspended = true;
 	intel_opregion_notify_adapter(dev, PCI_D3cold);
 
+	DRM_DEBUG_KMS("Device suspended\n");
 	return 0;
 }
 
@@ -943,6 +947,10 @@ static int i915_runtime_resume(struct device *device)
 	intel_opregion_notify_adapter(dev, PCI_D0);
 	dev_priv->pm.suspended = false;
 
+	if (HAS_PC8(dev))
+		__hsw_do_disable_pc8(dev_priv);
+
+	DRM_DEBUG_KMS("Device resumed\n");
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7f8ec08..b05e07b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1256,6 +1256,10 @@ struct ilk_wm_values {
 /*
  * This struct tracks the state needed for the Package C8+ feature.
  *
+ * TODO: we're merging the Package C8+ feature with the runtime PM support. To
+ * avoid having to update the documentation at each patch of the series, we'll
+ * do a final update at the end.
+ *
  * Package states C8 and deeper are really deep PC states that can only be
  * reached when all the devices on the system allow it, so even if the graphics
  * device allows PC8+, it doesn't mean the system will actually get to these
@@ -1311,7 +1315,6 @@ struct i915_package_c8 {
 	bool enabled;
 	int disable_count;
 	struct mutex lock;
-	struct delayed_work enable_work;
 
 	struct {
 		uint32_t deimr;
@@ -1915,8 +1918,6 @@ extern unsigned int i915_preliminary_hw_support __read_mostly;
 extern int i915_disable_power_well __read_mostly;
 extern int i915_enable_ips __read_mostly;
 extern bool i915_fastboot __read_mostly;
-extern int i915_enable_pc8 __read_mostly;
-extern int i915_pc8_timeout __read_mostly;
 extern bool i915_prefault_disable __read_mostly;
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index db6523e..5055a71 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6601,7 +6601,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 
 	/* Make sure we're not on PC8 state before disabling PC8, otherwise
 	 * we'll hang the machine! */
-	gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+	gen6_gt_force_wake_get_no_rpm(dev_priv, FORCEWAKE_ALL);
 
 	if (val & LCPLL_POWER_DOWN_ALLOW) {
 		val &= ~LCPLL_POWER_DOWN_ALLOW;
@@ -6635,14 +6635,16 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 			DRM_ERROR("Switching back to LCPLL failed\n");
 	}
 
-	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+	gen6_gt_force_wake_put_no_rpm(dev_priv, FORCEWAKE_ALL);
 }
 
-static void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
+void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
 
+	WARN_ON(!HAS_PC8(dev));
+
 	DRM_DEBUG_KMS("Enabling package C8+\n");
 
 	dev_priv->pc8.enabled = true;
@@ -6658,22 +6660,6 @@ static void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 	hsw_disable_lcpll(dev_priv, true, true);
 }
 
-void hsw_enable_pc8_work(struct work_struct *__work)
-{
-	struct drm_i915_private *dev_priv =
-		container_of(to_delayed_work(__work), struct drm_i915_private,
-			     pc8.enable_work);
-	struct drm_device *dev = dev_priv->dev;
-
-	WARN_ON(!HAS_PC8(dev));
-
-	if (dev_priv->pc8.enabled)
-		return;
-
-	__hsw_do_enable_pc8(dev_priv);
-	intel_runtime_pm_put(dev_priv);
-}
-
 static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 {
 	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
@@ -6684,15 +6670,16 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 	if (dev_priv->pc8.disable_count != 0)
 		return;
 
-	schedule_delayed_work(&dev_priv->pc8.enable_work,
-			      msecs_to_jiffies(i915_pc8_timeout));
+	intel_runtime_pm_put(dev_priv);
 }
 
-static void __hsw_do_disable_package_c8(struct drm_i915_private *dev_priv)
+void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
 
+	WARN_ON(!HAS_PC8(dev));
+
 	DRM_DEBUG_KMS("Disabling package C8+\n");
 
 	hsw_restore_lcpll(dev_priv);
@@ -6715,8 +6702,6 @@ static void __hsw_do_disable_package_c8(struct drm_i915_private *dev_priv)
 
 static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 {
-	struct drm_device *dev = dev_priv->dev;
-
 	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
 	WARN(dev_priv->pc8.disable_count < 0,
 	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
@@ -6725,14 +6710,7 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 	if (dev_priv->pc8.disable_count != 1)
 		return;
 
-	WARN_ON(!HAS_PC8(dev));
-
-	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
-	if (!dev_priv->pc8.enabled)
-		return;
-
 	intel_runtime_pm_get(dev_priv);
-	__hsw_do_disable_package_c8(dev_priv);
 }
 
 void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
@@ -6790,9 +6768,6 @@ static void hsw_update_package_c8(struct drm_device *dev)
 	if (!HAS_PC8(dev_priv->dev))
 		return;
 
-	if (!i915_enable_pc8)
-		return;
-
 	mutex_lock(&dev_priv->pc8.lock);
 
 	allow = hsw_can_enable_package_c8(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 46aea6c..6985126 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -694,7 +694,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 					     unsigned int bpp,
 					     unsigned int pitch);
 void intel_display_handle_reset(struct drm_device *dev);
-void hsw_enable_pc8_work(struct work_struct *__work);
+void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv);
+void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv);
 void hsw_enable_package_c8(struct drm_i915_private *dev_priv);
 void hsw_disable_package_c8(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 469170c..c644a6f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5785,7 +5785,6 @@ void intel_pm_setup(struct drm_device *dev)
 	dev_priv->pc8.irqs_disabled = false;
 	dev_priv->pc8.enabled = false;
 	dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */
-	INIT_DELAYED_WORK(&dev_priv->pc8.enable_work, hsw_enable_pc8_work);
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 }
-- 
1.8.3.1

[PATCH 06/19] drm/i915: get/put runtime PM when we get/put a power domain

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Any power domain will require the HW to be in PCI D0 state, so just do
the simple thing.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c644a6f..273e806 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5301,6 +5301,8 @@ void intel_display_power_get(struct drm_device *dev,
 	struct i915_power_well *power_well;
 	int i;
 
+	intel_runtime_pm_get(dev_priv);
+
 	power_domains = &dev_priv->power_domains;
 
 	mutex_lock(&power_domains->lock);
@@ -5332,6 +5334,8 @@ void intel_display_power_put(struct drm_device *dev,
 		__intel_power_well_put(dev, power_well);
 
 	mutex_unlock(&power_domains->lock);
+
+	intel_runtime_pm_put(dev_priv);
 }
 
 static struct i915_power_domains *hsw_pwr;
-- 
1.8.3.1

[PATCH 07/19] drm/i915: remove dev_priv->pc8.requirements_met

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The requirements_met variable was used to track two things: enabled
CRTCs and the power well. After the latest chagnes, we get a runtime
PM reference whenever we get any of the power domains, and we get
power domains when we enable CRTCs or the power well, so we should
already be covered, not needing this specific tracking.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  2 --
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_display.c | 54 ------------------------------------
 drivers/gpu/drm/i915/intel_pm.c      |  3 +-
 4 files changed, 1 insertion(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1cdc5dd..0f2c356 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1902,8 +1902,6 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
 	}
 
 	mutex_lock(&dev_priv->pc8.lock);
-	seq_printf(m, "Requirements met: %s\n",
-		   yesno(dev_priv->pc8.requirements_met));
 	seq_printf(m, "GPU idle: %s\n", yesno(dev_priv->pc8.gpu_idle));
 	seq_printf(m, "Disable count: %d\n", dev_priv->pc8.disable_count);
 	seq_printf(m, "IRQs disabled: %s\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b05e07b..94f0926 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1308,7 +1308,6 @@ struct ilk_wm_values {
  * For more, read "Display Sequences for Package C8" on our documentation.
  */
 struct i915_package_c8 {
-	bool requirements_met;
 	bool gpu_idle;
 	bool irqs_disabled;
 	/* Only true after the delayed work task actually enables it. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5055a71..c949a16 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6733,59 +6733,6 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->pc8.lock);
 }
 
-static bool hsw_can_enable_package_c8(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	struct intel_crtc *crtc;
-	uint32_t val;
-
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
-		if (crtc->base.enabled)
-			return false;
-
-	/* This case is still possible since we have the i915.disable_power_well
-	 * parameter and also the KVMr or something else might be requesting the
-	 * power well. */
-	val = I915_READ(HSW_PWR_WELL_DRIVER);
-	if (val != 0) {
-		DRM_DEBUG_KMS("Not enabling PC8: power well on\n");
-		return false;
-	}
-
-	return true;
-}
-
-/* Since we're called from modeset_global_resources there's no way to
- * symmetrically increase and decrease the refcount, so we use
- * dev_priv->pc8.requirements_met to track whether we already have the refcount
- * or not.
- */
-static void hsw_update_package_c8(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	bool allow;
-
-	if (!HAS_PC8(dev_priv->dev))
-		return;
-
-	mutex_lock(&dev_priv->pc8.lock);
-
-	allow = hsw_can_enable_package_c8(dev_priv);
-
-	if (allow == dev_priv->pc8.requirements_met)
-		goto done;
-
-	dev_priv->pc8.requirements_met = allow;
-
-	if (allow)
-		__hsw_enable_package_c8(dev_priv);
-	else
-		__hsw_disable_package_c8(dev_priv);
-
-done:
-	mutex_unlock(&dev_priv->pc8.lock);
-}
-
 static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_PC8(dev_priv->dev))
@@ -6885,7 +6832,6 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
 static void haswell_modeset_global_resources(struct drm_device *dev)
 {
 	modeset_update_crtc_power_domains(dev);
-	hsw_update_package_c8(dev);
 }
 
 static int haswell_crtc_mode_set(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 273e806..bf2b963 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5784,11 +5784,10 @@ void intel_pm_setup(struct drm_device *dev)
 	mutex_init(&dev_priv->rps.hw_lock);
 
 	mutex_init(&dev_priv->pc8.lock);
-	dev_priv->pc8.requirements_met = false;
 	dev_priv->pc8.gpu_idle = false;
 	dev_priv->pc8.irqs_disabled = false;
 	dev_priv->pc8.enabled = false;
-	dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */
+	dev_priv->pc8.disable_count = 1; /* gpu_idle */
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 }
-- 
1.8.3.1

[PATCH 08/19] drm/i915: make gpu_idle be part of runtime PM, not PC8

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because PC8 is part of runtime PM, and platforms without PC8 will need
gpu_idle for runtime PM support.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  1 -
 drivers/gpu/drm/i915/i915_drv.h      |  3 ++-
 drivers/gpu/drm/i915/intel_display.c | 32 ++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_pm.c      |  8 ++++++--
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0f2c356..5943f49 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1902,7 +1902,6 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
 	}
 
 	mutex_lock(&dev_priv->pc8.lock);
-	seq_printf(m, "GPU idle: %s\n", yesno(dev_priv->pc8.gpu_idle));
 	seq_printf(m, "Disable count: %d\n", dev_priv->pc8.disable_count);
 	seq_printf(m, "IRQs disabled: %s\n",
 		   yesno(dev_priv->pc8.irqs_disabled));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 94f0926..5decc82 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1308,7 +1308,6 @@ struct ilk_wm_values {
  * For more, read "Display Sequences for Package C8" on our documentation.
  */
 struct i915_package_c8 {
-	bool gpu_idle;
 	bool irqs_disabled;
 	/* Only true after the delayed work task actually enables it. */
 	bool enabled;
@@ -1326,6 +1325,8 @@ struct i915_package_c8 {
 
 struct i915_runtime_pm {
 	bool suspended;
+	bool gpu_idle;
+	struct mutex gpu_idle_lock;
 };
 
 enum intel_pipe_crc_source {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c949a16..9ead35e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6733,30 +6733,30 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->pc8.lock);
 }
 
-static void hsw_package_c8_gpu_idle(struct drm_i915_private *dev_priv)
+static void intel_runtime_pm_gpu_idle(struct drm_i915_private *dev_priv)
 {
-	if (!HAS_PC8(dev_priv->dev))
+	if (!HAS_RUNTIME_PM(dev_priv->dev))
 		return;
 
-	mutex_lock(&dev_priv->pc8.lock);
-	if (!dev_priv->pc8.gpu_idle) {
-		dev_priv->pc8.gpu_idle = true;
-		__hsw_enable_package_c8(dev_priv);
+	mutex_lock(&dev_priv->pm.gpu_idle_lock);
+	if (!dev_priv->pm.gpu_idle) {
+		dev_priv->pm.gpu_idle = true;
+		intel_runtime_pm_put(dev_priv);
 	}
-	mutex_unlock(&dev_priv->pc8.lock);
+	mutex_unlock(&dev_priv->pm.gpu_idle_lock);
 }
 
-static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
+static void intel_runtime_pm_gpu_busy(struct drm_i915_private *dev_priv)
 {
-	if (!HAS_PC8(dev_priv->dev))
+	if (!HAS_RUNTIME_PM(dev_priv->dev))
 		return;
 
-	mutex_lock(&dev_priv->pc8.lock);
-	if (dev_priv->pc8.gpu_idle) {
-		dev_priv->pc8.gpu_idle = false;
-		__hsw_disable_package_c8(dev_priv);
+	mutex_lock(&dev_priv->pm.gpu_idle_lock);
+	if (dev_priv->pm.gpu_idle) {
+		dev_priv->pm.gpu_idle = false;
+		intel_runtime_pm_get(dev_priv);
 	}
-	mutex_unlock(&dev_priv->pc8.lock);
+	mutex_unlock(&dev_priv->pm.gpu_idle_lock);
 }
 
 #define for_each_power_domain(domain, mask)				\
@@ -8085,7 +8085,7 @@ void intel_mark_busy(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	hsw_package_c8_gpu_busy(dev_priv);
+	intel_runtime_pm_gpu_busy(dev_priv);
 	i915_update_gfx_val(dev_priv);
 }
 
@@ -8094,7 +8094,7 @@ void intel_mark_idle(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 
-	hsw_package_c8_gpu_idle(dev_priv);
+	intel_runtime_pm_gpu_idle(dev_priv);
 
 	if (!i915_powersave)
 		return;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bf2b963..7027a35 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5526,6 +5526,8 @@ void intel_init_runtime_pm(struct drm_i915_private *dev_priv)
 	pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
 	pm_runtime_mark_last_busy(device);
 	pm_runtime_use_autosuspend(device);
+
+	pm_runtime_put_autosuspend(device);
 }
 
 void intel_fini_runtime_pm(struct drm_i915_private *dev_priv)
@@ -5784,10 +5786,12 @@ void intel_pm_setup(struct drm_device *dev)
 	mutex_init(&dev_priv->rps.hw_lock);
 
 	mutex_init(&dev_priv->pc8.lock);
-	dev_priv->pc8.gpu_idle = false;
 	dev_priv->pc8.irqs_disabled = false;
 	dev_priv->pc8.enabled = false;
-	dev_priv->pc8.disable_count = 1; /* gpu_idle */
+	dev_priv->pc8.disable_count = 0;
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
+
+	dev_priv->pm.gpu_idle = true;
+	mutex_init(&dev_priv->pm.gpu_idle_lock);
 }
-- 
1.8.3.1

[PATCH 09/19] drm/i915: kill pc8.disable_count

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Since after the latest patches it's only being used to prevent
getting/putting the runtime PM refcount.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  1 -
 drivers/gpu/drm/i915/i915_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_display.c | 14 --------------
 drivers/gpu/drm/i915/intel_pm.c      |  1 -
 4 files changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5943f49..204a0cd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1902,7 +1902,6 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
 	}
 
 	mutex_lock(&dev_priv->pc8.lock);
-	seq_printf(m, "Disable count: %d\n", dev_priv->pc8.disable_count);
 	seq_printf(m, "IRQs disabled: %s\n",
 		   yesno(dev_priv->pc8.irqs_disabled));
 	seq_printf(m, "Enabled: %s\n", yesno(dev_priv->pc8.enabled));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5decc82..69845c4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1311,7 +1311,6 @@ struct i915_package_c8 {
 	bool irqs_disabled;
 	/* Only true after the delayed work task actually enables it. */
 	bool enabled;
-	int disable_count;
 	struct mutex lock;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9ead35e..0b846ac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6663,13 +6663,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 {
 	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
-	WARN(dev_priv->pc8.disable_count < 1,
-	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
-
-	dev_priv->pc8.disable_count--;
-	if (dev_priv->pc8.disable_count != 0)
-		return;
-
 	intel_runtime_pm_put(dev_priv);
 }
 
@@ -6703,13 +6696,6 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 {
 	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
-	WARN(dev_priv->pc8.disable_count < 0,
-	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
-
-	dev_priv->pc8.disable_count++;
-	if (dev_priv->pc8.disable_count != 1)
-		return;
-
 	intel_runtime_pm_get(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7027a35..0786eda 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5788,7 +5788,6 @@ void intel_pm_setup(struct drm_device *dev)
 	mutex_init(&dev_priv->pc8.lock);
 	dev_priv->pc8.irqs_disabled = false;
 	dev_priv->pc8.enabled = false;
-	dev_priv->pc8.disable_count = 0;
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 
-- 
1.8.3.1

[PATCH 10/19] drm/i915: remove an indirection level on PC8 functions

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

After the latest changes, the indirection is useless.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0b846ac..316ac4b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6660,12 +6660,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 	hsw_disable_lcpll(dev_priv, true, true);
 }
 
-static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
-{
-	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
-	intel_runtime_pm_put(dev_priv);
-}
-
 void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
@@ -6693,19 +6687,13 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 	dev_priv->pc8.enabled = false;
 }
 
-static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
-{
-	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
-	intel_runtime_pm_get(dev_priv);
-}
-
 void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_PC8(dev_priv->dev))
 		return;
 
 	mutex_lock(&dev_priv->pc8.lock);
-	__hsw_enable_package_c8(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 	mutex_unlock(&dev_priv->pc8.lock);
 }
 
@@ -6715,7 +6703,7 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 		return;
 
 	mutex_lock(&dev_priv->pc8.lock);
-	__hsw_disable_package_c8(dev_priv);
+	intel_runtime_pm_get(dev_priv);
 	mutex_unlock(&dev_priv->pc8.lock);
 }
 
-- 
1.8.3.1

[PATCH 11/19] drm/i915: don't get/put PC8 reference on freeze/thaw

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We already get runtime PM references, and PC8 is now part of runtime
PM, so this is enough.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3b86541..1b6d886 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -515,7 +515,6 @@ 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_display_set_init_power(dev, true);
 
 	drm_kms_helper_poll_disable(dev);
@@ -688,10 +687,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		schedule_work(&dev_priv->console_resume_work);
 	}
 
-	/* Undo what we did at i915_drm_freeze so the refcount goes back to the
-	 * expected level. */
-	hsw_enable_package_c8(dev_priv);
-
 	mutex_lock(&dev_priv->modeset_restore_lock);
 	dev_priv->modeset_restore = MODESET_DONE;
 	mutex_unlock(&dev_priv->modeset_restore_lock);
-- 
1.8.3.1

[PATCH 12/19] drm/i915: get/put runtime PM references for GMBUS and DP AUX

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We had these intel_aux_display_runtime_{get,put} abstractions that
would just get/put PC8 references, but now that runtime PM and PC8
are the same stuff, we just need the runtime PM references, so just
get/put runtime PM directly, because that's what the rest of our code
does.

Another way to solve this problem would be to add DP_AUX and GMBUS
power domains, and then use intel_display_power_{get,put}, but every
power domain already gets/puts runtime PM references, so this would
just make things more complex.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h |  2 --
 drivers/gpu/drm/i915/intel_i2c.c |  4 ++--
 drivers/gpu/drm/i915/intel_pm.c  | 11 -----------
 4 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7df5085..0895622 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -425,7 +425,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	else
 		timeout = DP_AUX_CH_CTL_TIME_OUT_400us;
 
-	intel_aux_display_runtime_get(dev_priv);
+	intel_runtime_pm_get(dev_priv);
 
 	/* Try to wait for any previous AUX channel activity */
 	for (try = 0; try < 3; try++) {
@@ -523,7 +523,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	ret = recv_bytes;
 out:
 	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
-	intel_aux_display_runtime_put(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6985126..fc68c4a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -862,8 +862,6 @@ void ironlake_teardown_rc6(struct drm_device *dev);
 void gen6_update_ring_freq(struct drm_device *dev);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
 void gen6_rps_boost(struct drm_i915_private *dev_priv);
-void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
-void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
 void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index b1dc33f..ea3fb81 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -453,7 +453,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
 	int i, reg_offset;
 	int ret = 0;
 
-	intel_aux_display_runtime_get(dev_priv);
+	intel_runtime_pm_get(dev_priv);
 	mutex_lock(&dev_priv->gmbus_mutex);
 
 	if (bus->force_bit) {
@@ -553,7 +553,7 @@ timeout:
 
 out:
 	mutex_unlock(&dev_priv->gmbus_mutex);
-	intel_aux_display_runtime_put(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0786eda..57eda27 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5476,17 +5476,6 @@ void intel_power_domains_init_hw(struct drm_device *dev)
 		I915_WRITE(HSW_PWR_WELL_BIOS, 0);
 }
 
-/* Disables PC8 so we can use the GMBUS and DP AUX interrupts. */
-void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv)
-{
-	hsw_disable_package_c8(dev_priv);
-}
-
-void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv)
-{
-	hsw_enable_package_c8(dev_priv);
-}
-
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
-- 
1.8.3.1

[PATCH 13/19] drm/i915: don't get/put PC8 when getting/putting power wells

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because we already get/put runtime PM every time we get/put any power
domain, and now PC8 and runtime PM are the same thing.

With this, we can also now kill the hsw_{en,dis}able_package_c8
functions.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 --------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 drivers/gpu/drm/i915/intel_pm.c      | 12 ++----------
 3 files changed, 2 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 316ac4b..5df063c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6687,26 +6687,6 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 	dev_priv->pc8.enabled = false;
 }
 
-void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
-{
-	if (!HAS_PC8(dev_priv->dev))
-		return;
-
-	mutex_lock(&dev_priv->pc8.lock);
-	intel_runtime_pm_put(dev_priv);
-	mutex_unlock(&dev_priv->pc8.lock);
-}
-
-void hsw_disable_package_c8(struct drm_i915_private *dev_priv)
-{
-	if (!HAS_PC8(dev_priv->dev))
-		return;
-
-	mutex_lock(&dev_priv->pc8.lock);
-	intel_runtime_pm_get(dev_priv);
-	mutex_unlock(&dev_priv->pc8.lock);
-}
-
 static void intel_runtime_pm_gpu_idle(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_RUNTIME_PM(dev_priv->dev))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fc68c4a..77b0250 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -696,8 +696,6 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 void intel_display_handle_reset(struct drm_device *dev);
 void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv);
 void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv);
-void hsw_enable_package_c8(struct drm_i915_private *dev_priv);
-void hsw_disable_package_c8(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_config *pipe_config);
 int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 57eda27..dff5deb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5271,26 +5271,18 @@ static void hsw_set_power_well(struct drm_device *dev,
 static void __intel_power_well_get(struct drm_device *dev,
 				   struct i915_power_well *power_well)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (!power_well->count++ && power_well->set) {
-		hsw_disable_package_c8(dev_priv);
+	if (!power_well->count++ && power_well->set)
 		power_well->set(dev, power_well, true);
-	}
 }
 
 static void __intel_power_well_put(struct drm_device *dev,
 				   struct i915_power_well *power_well)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
 	WARN_ON(!power_well->count);
 
 	if (!--power_well->count && power_well->set &&
-	    i915_disable_power_well) {
+	    i915_disable_power_well)
 		power_well->set(dev, power_well, false);
-		hsw_enable_package_c8(dev_priv);
-	}
 }
 
 void intel_display_power_get(struct drm_device *dev,
-- 
1.8.3.1

[PATCH 14/19] drm/i915: remove dev_priv->pc8.enabled

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

It was just being used on debugfs and on a WARN inside
hsw_set_power_well. But now that we PC8 is part of runtime PM and we
get/put runtime PM when we get/put any power domain, we shouldn't need
the WARN anymore.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 1 -
 drivers/gpu/drm/i915/i915_drv.h      | 2 --
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 drivers/gpu/drm/i915/intel_pm.c      | 3 ---
 4 files changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 204a0cd..627fc5c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1904,7 +1904,6 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
 	mutex_lock(&dev_priv->pc8.lock);
 	seq_printf(m, "IRQs disabled: %s\n",
 		   yesno(dev_priv->pc8.irqs_disabled));
-	seq_printf(m, "Enabled: %s\n", yesno(dev_priv->pc8.enabled));
 	mutex_unlock(&dev_priv->pc8.lock);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 69845c4..3fa30e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1309,8 +1309,6 @@ struct ilk_wm_values {
  */
 struct i915_package_c8 {
 	bool irqs_disabled;
-	/* Only true after the delayed work task actually enables it. */
-	bool enabled;
 	struct mutex lock;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5df063c..08be8a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6647,8 +6647,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 
 	DRM_DEBUG_KMS("Enabling package C8+\n");
 
-	dev_priv->pc8.enabled = true;
-
 	if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
 		val = I915_READ(SOUTH_DSPCLK_GATE_D);
 		val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
@@ -6684,7 +6682,6 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 	mutex_lock(&dev_priv->rps.hw_lock);
 	gen6_update_ring_freq(dev);
 	mutex_unlock(&dev_priv->rps.hw_lock);
-	dev_priv->pc8.enabled = false;
 }
 
 static void intel_runtime_pm_gpu_idle(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dff5deb..9322c06 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5238,8 +5238,6 @@ static void hsw_set_power_well(struct drm_device *dev,
 	bool is_enabled, enable_requested;
 	uint32_t tmp;
 
-	WARN_ON(dev_priv->pc8.enabled);
-
 	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
 	is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED;
 	enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST;
@@ -5768,7 +5766,6 @@ void intel_pm_setup(struct drm_device *dev)
 
 	mutex_init(&dev_priv->pc8.lock);
 	dev_priv->pc8.irqs_disabled = false;
-	dev_priv->pc8.enabled = false;
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 
-- 
1.8.3.1

[PATCH 15/19] drm/i915: move pc8.irqs_disabled to pm.irqs_disabled

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

When other platforms add runtime PM support they will also need to
disable interrupts, so move the variable to the runtime PM struct.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  6 ----
 drivers/gpu/drm/i915/i915_drv.h      | 14 ++++-----
 drivers/gpu/drm/i915/i915_gem.c      |  2 +-
 drivers/gpu/drm/i915/i915_irq.c      | 58 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_display.c |  4 +--
 drivers/gpu/drm/i915/intel_drv.h     |  4 +--
 drivers/gpu/drm/i915/intel_pm.c      |  2 +-
 7 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 627fc5c..be1ea64 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1894,18 +1894,12 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (!IS_HASWELL(dev)) {
 		seq_puts(m, "not supported\n");
 		return 0;
 	}
 
-	mutex_lock(&dev_priv->pc8.lock);
-	seq_printf(m, "IRQs disabled: %s\n",
-		   yesno(dev_priv->pc8.irqs_disabled));
-	mutex_unlock(&dev_priv->pc8.lock);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3fa30e8..cdfba82 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1308,8 +1308,14 @@ struct ilk_wm_values {
  * For more, read "Display Sequences for Package C8" on our documentation.
  */
 struct i915_package_c8 {
-	bool irqs_disabled;
 	struct mutex lock;
+};
+
+struct i915_runtime_pm {
+	bool suspended;
+	bool gpu_idle;
+	bool irqs_disabled;
+	struct mutex gpu_idle_lock;
 
 	struct {
 		uint32_t deimr;
@@ -1320,12 +1326,6 @@ struct i915_package_c8 {
 	} regsave;
 };
 
-struct i915_runtime_pm {
-	bool suspended;
-	bool gpu_idle;
-	struct mutex gpu_idle_lock;
-};
-
 enum intel_pipe_crc_source {
 	INTEL_PIPE_CRC_SOURCE_NONE,
 	INTEL_PIPE_CRC_SOURCE_PLANE1,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 656406d..f2b45d9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1022,7 +1022,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
 	unsigned long timeout_expire;
 	int ret;
 
-	WARN(dev_priv->pc8.irqs_disabled, "IRQs disabled\n");
+	WARN(dev_priv->pm.irqs_disabled, "IRQs disabled\n");
 
 	if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
 		return 0;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1d44c79..e1f20c7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -86,9 +86,9 @@ ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
 {
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (dev_priv->pc8.irqs_disabled) {
+	if (dev_priv->pm.irqs_disabled) {
 		WARN(1, "IRQs disabled\n");
-		dev_priv->pc8.regsave.deimr &= ~mask;
+		dev_priv->pm.regsave.deimr &= ~mask;
 		return;
 	}
 
@@ -104,9 +104,9 @@ ironlake_disable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
 {
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (dev_priv->pc8.irqs_disabled) {
+	if (dev_priv->pm.irqs_disabled) {
 		WARN(1, "IRQs disabled\n");
-		dev_priv->pc8.regsave.deimr |= mask;
+		dev_priv->pm.regsave.deimr |= mask;
 		return;
 	}
 
@@ -129,10 +129,10 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
 {
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (dev_priv->pc8.irqs_disabled) {
+	if (dev_priv->pm.irqs_disabled) {
 		WARN(1, "IRQs disabled\n");
-		dev_priv->pc8.regsave.gtimr &= ~interrupt_mask;
-		dev_priv->pc8.regsave.gtimr |= (~enabled_irq_mask &
+		dev_priv->pm.regsave.gtimr &= ~interrupt_mask;
+		dev_priv->pm.regsave.gtimr |= (~enabled_irq_mask &
 						interrupt_mask);
 		return;
 	}
@@ -167,10 +167,10 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
 
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (dev_priv->pc8.irqs_disabled) {
+	if (dev_priv->pm.irqs_disabled) {
 		WARN(1, "IRQs disabled\n");
-		dev_priv->pc8.regsave.gen6_pmimr &= ~interrupt_mask;
-		dev_priv->pc8.regsave.gen6_pmimr |= (~enabled_irq_mask &
+		dev_priv->pm.regsave.gen6_pmimr &= ~interrupt_mask;
+		dev_priv->pm.regsave.gen6_pmimr |= (~enabled_irq_mask &
 						     interrupt_mask);
 		return;
 	}
@@ -301,11 +301,11 @@ static void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
 
 	assert_spin_locked(&dev_priv->irq_lock);
 
-	if (dev_priv->pc8.irqs_disabled &&
+	if (dev_priv->pm.irqs_disabled &&
 	    (interrupt_mask & SDE_HOTPLUG_MASK_CPT)) {
 		WARN(1, "IRQs disabled\n");
-		dev_priv->pc8.regsave.sdeimr &= ~interrupt_mask;
-		dev_priv->pc8.regsave.sdeimr |= (~enabled_irq_mask &
+		dev_priv->pm.regsave.sdeimr &= ~interrupt_mask;
+		dev_priv->pm.regsave.sdeimr |= (~enabled_irq_mask &
 						 interrupt_mask);
 		return;
 	}
@@ -3886,32 +3886,32 @@ void intel_hpd_init(struct drm_device *dev)
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
-/* Disable interrupts so we can allow Package C8+. */
-void hsw_pc8_disable_interrupts(struct drm_device *dev)
+/* Disable interrupts so we can allow runtime PM. */
+void hsw_runtime_pm_disable_interrupts(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 
-	dev_priv->pc8.regsave.deimr = I915_READ(DEIMR);
-	dev_priv->pc8.regsave.sdeimr = I915_READ(SDEIMR);
-	dev_priv->pc8.regsave.gtimr = I915_READ(GTIMR);
-	dev_priv->pc8.regsave.gtier = I915_READ(GTIER);
-	dev_priv->pc8.regsave.gen6_pmimr = I915_READ(GEN6_PMIMR);
+	dev_priv->pm.regsave.deimr = I915_READ(DEIMR);
+	dev_priv->pm.regsave.sdeimr = I915_READ(SDEIMR);
+	dev_priv->pm.regsave.gtimr = I915_READ(GTIMR);
+	dev_priv->pm.regsave.gtier = I915_READ(GTIER);
+	dev_priv->pm.regsave.gen6_pmimr = I915_READ(GEN6_PMIMR);
 
 	ironlake_disable_display_irq(dev_priv, 0xffffffff);
 	ibx_disable_display_interrupt(dev_priv, 0xffffffff);
 	ilk_disable_gt_irq(dev_priv, 0xffffffff);
 	snb_disable_pm_irq(dev_priv, 0xffffffff);
 
-	dev_priv->pc8.irqs_disabled = true;
+	dev_priv->pm.irqs_disabled = true;
 
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
-/* Restore interrupts so we can recover from Package C8+. */
-void hsw_pc8_restore_interrupts(struct drm_device *dev)
+/* Restore interrupts so we can recover from runtime PM. */
+void hsw_runtime_pm_restore_interrupts(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned long irqflags;
@@ -3931,13 +3931,13 @@ void hsw_pc8_restore_interrupts(struct drm_device *dev)
 	val = I915_READ(GEN6_PMIMR);
 	WARN(val != 0xffffffff, "GEN6_PMIMR is 0x%08x\n", val);
 
-	dev_priv->pc8.irqs_disabled = false;
+	dev_priv->pm.irqs_disabled = false;
 
-	ironlake_enable_display_irq(dev_priv, ~dev_priv->pc8.regsave.deimr);
-	ibx_enable_display_interrupt(dev_priv, ~dev_priv->pc8.regsave.sdeimr);
-	ilk_enable_gt_irq(dev_priv, ~dev_priv->pc8.regsave.gtimr);
-	snb_enable_pm_irq(dev_priv, ~dev_priv->pc8.regsave.gen6_pmimr);
-	I915_WRITE(GTIER, dev_priv->pc8.regsave.gtier);
+	ironlake_enable_display_irq(dev_priv, ~dev_priv->pm.regsave.deimr);
+	ibx_enable_display_interrupt(dev_priv, ~dev_priv->pm.regsave.sdeimr);
+	ilk_enable_gt_irq(dev_priv, ~dev_priv->pm.regsave.gtimr);
+	snb_enable_pm_irq(dev_priv, ~dev_priv->pm.regsave.gen6_pmimr);
+	I915_WRITE(GTIER, dev_priv->pm.regsave.gtier);
 
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 08be8a8..4b6c43c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6654,7 +6654,7 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 	}
 
 	lpt_disable_clkout_dp(dev);
-	hsw_pc8_disable_interrupts(dev);
+	hsw_runtime_pm_disable_interrupts(dev);
 	hsw_disable_lcpll(dev_priv, true, true);
 }
 
@@ -6668,7 +6668,7 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_KMS("Disabling package C8+\n");
 
 	hsw_restore_lcpll(dev_priv);
-	hsw_pc8_restore_interrupts(dev);
+	hsw_runtime_pm_restore_interrupts(dev);
 	lpt_init_pch_refclk(dev);
 
 	if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 77b0250..de05c2d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -591,8 +591,8 @@ void ilk_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void ilk_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void snb_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void snb_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
-void hsw_pc8_disable_interrupts(struct drm_device *dev);
-void hsw_pc8_restore_interrupts(struct drm_device *dev);
+void hsw_runtime_pm_disable_interrupts(struct drm_device *dev);
+void hsw_runtime_pm_restore_interrupts(struct drm_device *dev);
 
 
 /* intel_crt.c */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9322c06..2eafcea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5765,10 +5765,10 @@ void intel_pm_setup(struct drm_device *dev)
 	mutex_init(&dev_priv->rps.hw_lock);
 
 	mutex_init(&dev_priv->pc8.lock);
-	dev_priv->pc8.irqs_disabled = false;
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 
+	dev_priv->pm.irqs_disabled = false;
 	dev_priv->pm.gpu_idle = true;
 	mutex_init(&dev_priv->pm.gpu_idle_lock);
 }
-- 
1.8.3.1

[PATCH 16/19] drm/i915: kill struct i915_package_c8

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The only remaining field of the struct was the lock, which was unused.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 6 ------
 drivers/gpu/drm/i915/intel_pm.c | 1 -
 2 files changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cdfba82..56434c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1307,10 +1307,6 @@ struct ilk_wm_values {
  *
  * For more, read "Display Sequences for Package C8" on our documentation.
  */
-struct i915_package_c8 {
-	struct mutex lock;
-};
-
 struct i915_runtime_pm {
 	bool suspended;
 	bool gpu_idle;
@@ -1553,8 +1549,6 @@ typedef struct drm_i915_private {
 		struct ilk_wm_values hw;
 	} wm;
 
-	struct i915_package_c8 pc8;
-
 	struct i915_runtime_pm pm;
 
 	/* Old dri1 support infrastructure, beware the dragons ya fools entering
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2eafcea..fe7785c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5764,7 +5764,6 @@ void intel_pm_setup(struct drm_device *dev)
 
 	mutex_init(&dev_priv->rps.hw_lock);
 
-	mutex_init(&dev_priv->pc8.lock);
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 
-- 
1.8.3.1

[PATCH 17/19] drm/i915: rename __hsw_do_{en, dis}able_pc8

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

After we removed all the intermediate abstractions, we can rename
these functions to just hsw_{en,dis}able_pc8.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 4 ++--
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h     | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1b6d886..323c51f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -917,7 +917,7 @@ static int i915_runtime_suspend(struct device *device)
 	DRM_DEBUG_KMS("Suspending device\n");
 
 	if (HAS_PC8(dev))
-		__hsw_do_enable_pc8(dev_priv);
+		hsw_enable_pc8(dev_priv);
 
 	i915_gem_release_all_mmaps(dev_priv);
 
@@ -943,7 +943,7 @@ static int i915_runtime_resume(struct device *device)
 	dev_priv->pm.suspended = false;
 
 	if (HAS_PC8(dev))
-		__hsw_do_disable_pc8(dev_priv);
+		hsw_disable_pc8(dev_priv);
 
 	DRM_DEBUG_KMS("Device resumed\n");
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4b6c43c..9f451a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6638,7 +6638,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	gen6_gt_force_wake_put_no_rpm(dev_priv, FORCEWAKE_ALL);
 }
 
-void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
+void hsw_enable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
@@ -6658,7 +6658,7 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
 	hsw_disable_lcpll(dev_priv, true, true);
 }
 
-void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
+void hsw_disable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	uint32_t val;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index de05c2d..a0f1a9f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -694,8 +694,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 					     unsigned int bpp,
 					     unsigned int pitch);
 void intel_display_handle_reset(struct drm_device *dev);
-void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv);
-void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv);
+void hsw_enable_pc8(struct drm_i915_private *dev_priv);
+void hsw_disable_pc8(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_config *pipe_config);
 int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
-- 
1.8.3.1

[PATCH 18/19] drm/i915: update the PC8 and runtime PM documentation.

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Now that PC8 got much simpler, there are less things to document.
Also, runtime PM already has a nice documentation, so we don't need to
re-explain it on our driver.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 56 +++++++++++-------------------------
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++
 2 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56434c6..b24b2cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1254,48 +1254,24 @@ struct ilk_wm_values {
 };
 
 /*
- * This struct tracks the state needed for the Package C8+ feature.
+ * This struct helps tracking the state needed for runtime PM, which puts the
+ * device in PCI D3 state. Notice that when this happens, nothing on the
+ * graphics device works, even register access, so we don't get interrupts nor
+ * anything else.
  *
- * TODO: we're merging the Package C8+ feature with the runtime PM support. To
- * avoid having to update the documentation at each patch of the series, we'll
- * do a final update at the end.
+ * Every piece of our code that needs to actually touch the hardware needs to
+ * either call intel_runtime_pm_get or call intel_display_power_get with the
+ * appropriate power domain.
  *
- * Package states C8 and deeper are really deep PC states that can only be
- * reached when all the devices on the system allow it, so even if the graphics
- * device allows PC8+, it doesn't mean the system will actually get to these
- * states.
+ * The gpu_idle variable is just used as a way to make sure that we actually do
+ * put/get calls symmetrically. When we change it to true, we put the runtime PM
+ * reference, and when we change it to false, we get the reference. With this,
+ * we can call intel_runtime_pm_gpu_{idle,busy} asymmetrically without problems.
  *
- * Our driver only allows PC8+ when all the outputs are disabled, the power well
- * is disabled and the GPU is idle. When these conditions are met, we manually
- * do the other conditions: disable the interrupts, clocks and switch LCPLL
- * refclk to Fclk.
- *
- * When we really reach PC8 or deeper states (not just when we allow it) we lose
- * the state of some registers, so when we come back from PC8+ we need to
- * restore this state. We don't get into PC8+ if we're not in RC6, so we don't
- * need to take care of the registers kept by RC6.
- *
- * The interrupt disabling is part of the requirements. We can only leave the
- * PCH HPD interrupts enabled. If we're in PC8+ and we get another interrupt we
- * can lock the machine.
- *
- * Ideally every piece of our code that needs PC8+ disabled would call
- * hsw_disable_package_c8, which would increment disable_count and prevent the
- * system from reaching PC8+. But we don't have a symmetric way to do this for
- * everything, so we have the requirements_met and gpu_idle variables. When we
- * switch requirements_met or gpu_idle to true we decrease disable_count, and
- * increase it in the opposite case. The requirements_met variable is true when
- * all the CRTCs, encoders and the power well are disabled. The gpu_idle
- * variable is true when the GPU is idle.
- *
- * In addition to everything, we only actually enable PC8+ if disable_count
- * stays at zero for at least some seconds. This is implemented with the
- * enable_work variable. We do this so we don't enable/disable PC8 dozens of
- * consecutive times when all screens are disabled and some background app
- * queries the state of our connectors, or we have some application constantly
- * waking up to use the GPU. Only after the enable_work function actually
- * enables PC8+ the "enable" variable will become true, which means that it can
- * be false even if disable_count is 0.
+ * Our driver uses the autosuspend delay feature, which means we'll only really
+ * suspend if we stay with zero refcount for a certain amount of time. The
+ * default value is currently very conservative (see intel_init_runtime_pm), but
+ * it can be changed with the standard runtime PM files frmo sysfs.
  *
  * The irqs_disabled variable becomes true exactly after we disable the IRQs and
  * goes back to false exactly before we reenable the IRQs. We use this variable
@@ -1305,7 +1281,7 @@ struct ilk_wm_values {
  * inside struct regsave so when we restore the IRQs they will contain the
  * latest expected values.
  *
- * For more, read "Display Sequences for Package C8" on our documentation.
+ * For more, read the Documentation/power/runtime_pm.txt.
  */
 struct i915_runtime_pm {
 	bool suspended;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f451a5..6b6be48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6638,6 +6638,28 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	gen6_gt_force_wake_put_no_rpm(dev_priv, FORCEWAKE_ALL);
 }
 
+/*
+ * Package states C8 and deeper are really deep PC states that can only be
+ * reached when all the devices on the system allow it, so even if the graphics
+ * device allows PC8+, it doesn't mean the system will actually get to these
+ * states. Our driver only allows PC8+ when going into runtime PM.
+ *
+ * The requirements for PC8+ are that all the outputs are disabled, the power
+ * well is disabled and the GPU is idle, and these are also the requirements for
+ * runtime PM. When these conditions are met, we manually do the other
+ * conditions: disable the interrupts, clocks and switch LCPLL refclk to Fclk.
+ * If we're in PC8+ and we get an non-hotplug interrupt, we can hard hang the
+ * machine.
+ *
+ * When we really reach PC8 or deeper states (not just when we allow it) we lose
+ * the state of some registers, so when we come back from PC8+ we need to
+ * restore this state. We don't get into PC8+ if we're not in RC6, so we don't
+ * need to take care of the registers kept by RC6. Notice that this happens even
+ * if we don't put the device in PCI D3 state (which is what currently happens
+ * because of the runtime PM support).
+ *
+ * For more, read "Display Sequences for Package C8" on our documentation.
+ */
 void hsw_enable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
-- 
1.8.3.1

[PATCH 19/19] drm/i915: init pm.suspended earlier

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Function intel_init_runtime_pm is supposed to start allowing runtime
PM from that point, but it's called very late on the driver
initialization code, to prevent the driver from trying to suspend
while still initializing. The problem is that variables are accessed
earlier than that, so initalize them at intel_pm_setup, which is
supposed to be the correct place.

Notice that this shouldn't fix any specific bugs because dev_priv is
zeroed when allocated, so the value is already correct right from the
start.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fe7785c..5a88d02 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5495,8 +5495,6 @@ void intel_init_runtime_pm(struct drm_i915_private *dev_priv)
 	struct drm_device *dev = dev_priv->dev;
 	struct device *device = &dev->pdev->dev;
 
-	dev_priv->pm.suspended = false;
-
 	if (!HAS_RUNTIME_PM(dev))
 		return;
 
@@ -5767,6 +5765,7 @@ void intel_pm_setup(struct drm_device *dev)
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
 
+	dev_priv->pm.suspended = false;
 	dev_priv->pm.irqs_disabled = false;
 	dev_priv->pm.gpu_idle = true;
 	mutex_init(&dev_priv->pm.gpu_idle_lock);
-- 
1.8.3.1

Re: [PATCH 02/19] drm/i915: get/put runtime PM without holding rps.hw_lock

[Jesse Barnes]

On Thu, 19 Dec 2013 11:54:52 -0200
Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We'll need this when we merge PC8 and Runtime PM: the PC8
> enable/disable functions need that lock.
> 
> Also, it's good practice to not hold a lock for longer than strictly
> needed.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 430eb3e..1cdc5dd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1414,7 +1414,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	int ret;
> +	int ret = 0;
>  	int gpu_freq, ia_freq;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
> @@ -1422,12 +1422,13 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  		return 0;
>  	}
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
>  	if (ret)
> -		return ret;
> -	intel_runtime_pm_get(dev_priv);
> +		goto out;
>  
>  	seq_puts(m, "GPU freq (MHz)\tEffective CPU freq (MHz)\tEffective Ring freq (MHz)\n");
>  
> @@ -1444,10 +1445,11 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  			   ((ia_freq >> 8) & 0xff) * 100);
>  	}
>  
> -	intel_runtime_pm_put(dev_priv);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	return 0;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  static int i915_gfxec(struct seq_file *m, void *unused)

So we have these runtime_pm_get/put calls all over now.  Is the plan to
convert those to specific wells as we add support for new platforms so
we can have fine grained well control rather than just global control?
I guess I need to dig out Imre's latest stuff and check...

Re: [PATCH 02/19] drm/i915: get/put runtime PM without holding rps.hw_lock

[Daniel Vetter]

On Thu, Dec 19, 2013 at 7:30 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> So we have these runtime_pm_get/put calls all over now.  Is the plan to
> convert those to specific wells as we add support for new platforms so
> we can have fine grained well control rather than just global control?
> I guess I need to dig out Imre's latest stuff and check...

My idea was that the power well stuff was just for the display side,
since there the hw engineers love to slice&dice things up in always
new ways. Other parts of the driver call get/put on specific power
domain functions (atm pc8+D3, but soon only D3).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 02/19] drm/i915: get/put runtime PM without holding rps.hw_lock

[Paulo Zanoni]

2013/12/19 Daniel Vetter <daniel@ffwll.ch>:
> On Thu, Dec 19, 2013 at 7:30 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> So we have these runtime_pm_get/put calls all over now.  Is the plan to
>> convert those to specific wells as we add support for new platforms so
>> we can have fine grained well control rather than just global control?
>> I guess I need to dig out Imre's latest stuff and check...
>
> My idea was that the power well stuff was just for the display side,
> since there the hw engineers love to slice&dice things up in always
> new ways. Other parts of the driver call get/put on specific power
> domain functions (atm pc8+D3, but soon only D3).

I'm open to suggestions. This series removes all the calls to PC8
get/put, leaving only the "runtime PM" get/put and "power domains"
get/put, so IMHO we already do some cleanup.

Since Imre's recent rework I actually started to like the power
domains, and IMHO a nice approach for the future would be to add power
domains as needed and replace the "runtime PM get/put" calls with
proper power domain get/put calls. On this series there's a patch that
makes us get runtime PM every time we get a power domain, so it should
be easy to just add new domains. And I think it makes sense to also
add power domains for non-display things (as long as we rename
intel_display_power_{get,put} to intel_power_domain_{get,put}.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni

Re: [PATCH 06/19] drm/i915: get/put runtime PM when we get/put a power domain

[Paulo Zanoni]

2013/12/19 Paulo Zanoni <przanoni@gmail.com>:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Any power domain will require the HW to be in PCI D0 state, so just do
> the simple thing.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Just a notice, in case anyone decides to apply this patch:

Since both intel_display_power_get and intel_display_power_put are
very similar, "git am" may get confused when you apply the patch, and
it may add the "intel_runtime_pm_get()" call to
intel_display_power_put instead of intel_display_power_get. So please
pay attention, especially if you start getting lots of WARNs when you
enter runtime PM :)

Thanks,
Paulo

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c644a6f..273e806 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5301,6 +5301,8 @@ void intel_display_power_get(struct drm_device *dev,
>         struct i915_power_well *power_well;
>         int i;
>
> +       intel_runtime_pm_get(dev_priv);
> +
>         power_domains = &dev_priv->power_domains;
>
>         mutex_lock(&power_domains->lock);
> @@ -5332,6 +5334,8 @@ void intel_display_power_put(struct drm_device *dev,
>                 __intel_power_well_put(dev, power_well);
>
>         mutex_unlock(&power_domains->lock);
> +
> +       intel_runtime_pm_put(dev_priv);
>  }
>
>  static struct i915_power_domains *hsw_pwr;
> --
> 1.8.3.1
>



-- 
Paulo Zanoni

Re: [PATCH 01/19] drm/i915: rename modeset_update_power_wells

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 1480 bytes --]

On Thu, 2013-12-19 at 11:54 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> To modeset_update_crtc_power_domains, since this function is
> responsible for updating all the power domains of all CRTCs after a
> modeset. In the future we should also run this function on all
> platforms, not just Haswell.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d1357a..c661423 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6860,7 +6860,7 @@ void intel_display_set_init_power(struct drm_device *dev, bool enable)
>  	dev_priv->power_domains.init_power_on = enable;
>  }
>  
> -static void modeset_update_power_wells(struct drm_device *dev)
> +static void modeset_update_crtc_power_domains(struct drm_device *dev)
>  {
>  	unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
>  	struct intel_crtc *crtc;
> @@ -6897,7 +6897,7 @@ static void modeset_update_power_wells(struct drm_device *dev)
>  
>  static void haswell_modeset_global_resources(struct drm_device *dev)
>  {
> -	modeset_update_power_wells(dev);
> +	modeset_update_crtc_power_domains(dev);
>  	hsw_update_package_c8(dev);
>  }
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/19] drm/i915: get/put runtime PM without holding rps.hw_lock

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 1979 bytes --]

On Thu, 2013-12-19 at 11:54 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We'll need this when we merge PC8 and Runtime PM: the PC8
> enable/disable functions need that lock.
> 
> Also, it's good practice to not hold a lock for longer than strictly
> needed.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 430eb3e..1cdc5dd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1414,7 +1414,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	int ret;
> +	int ret = 0;
>  	int gpu_freq, ia_freq;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
> @@ -1422,12 +1422,13 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  		return 0;
>  	}
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
>  	if (ret)
> -		return ret;
> -	intel_runtime_pm_get(dev_priv);
> +		goto out;
>  
>  	seq_puts(m, "GPU freq (MHz)\tEffective CPU freq (MHz)\tEffective Ring freq (MHz)\n");
>  
> @@ -1444,10 +1445,11 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  			   ((ia_freq >> 8) & 0xff) * 100);
>  	}
>  
> -	intel_runtime_pm_put(dev_priv);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	return 0;
> +out:
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
>  }
>  
>  static int i915_gfxec(struct seq_file *m, void *unused)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/19] drm/i915: add forcewake functions that don't touch runtime PM

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 4104 bytes --]

On Thu, 2013-12-19 at 11:54 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> To solve a chicken-and-egg problem. Currently when we get/put
> forcewake we also get/put runtime PM and this works fine because the
> runtime PM code doesn't need forcewake. But we're going to merge PC8
> and runtime PM into a single feature, and the PC8 code (the LCPLL
> code) does need the forcewake, so that specific piece of code needs to
> call the _no_rpm version so it doesn't try to get runtime PM in the
> code that gets runtime PM.
> 
> For now the new functions are unused, we'll use them on the patch that
> merges PC8 with runtime PM.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  4 ++++
>  drivers/gpu/drm/i915/intel_uncore.c | 30 ++++++++++++++++++++----------
>  2 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cc8afff..7f8ec08 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2533,6 +2533,10 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>   */
>  void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine);
>  void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine);
> +void gen6_gt_force_wake_get_no_rpm(struct drm_i915_private *dev_priv,
> +				   int fw_engine);
> +void gen6_gt_force_wake_put_no_rpm(struct drm_i915_private *dev_priv,
> +				   int fw_engine);
>  
>  int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 646fecf..6118d2c 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -368,15 +368,11 @@ void intel_uncore_sanitize(struct drm_device *dev)
>   * be called at the beginning of the sequence followed by a call to
>   * gen6_gt_force_wake_put() at the end of the sequence.
>   */
> -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
> +void gen6_gt_force_wake_get_no_rpm(struct drm_i915_private *dev_priv,
> +				   int fw_engine)
>  {
>  	unsigned long irqflags;
>  
> -	if (!dev_priv->uncore.funcs.force_wake_get)
> -		return;
> -
> -	intel_runtime_pm_get(dev_priv);
> -
>  	/* Redirect to VLV specific routine */
>  	if (IS_VALLEYVIEW(dev_priv->dev))
>  		return vlv_force_wake_get(dev_priv, fw_engine);
> @@ -387,16 +383,23 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
> +{
> +	if (!dev_priv->uncore.funcs.force_wake_get)
> +		return;
> +
> +	intel_runtime_pm_get(dev_priv);
> +	gen6_gt_force_wake_get_no_rpm(dev_priv, fw_engine);
> +}
> +
>  /*
>   * see gen6_gt_force_wake_get()
>   */
> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> +void gen6_gt_force_wake_put_no_rpm(struct drm_i915_private *dev_priv,
> +				   int fw_engine)
>  {
>  	unsigned long irqflags;
>  
> -	if (!dev_priv->uncore.funcs.force_wake_put)
> -		return;
> -
>  	/* Redirect to VLV specific routine */
>  	if (IS_VALLEYVIEW(dev_priv->dev))
>  		return vlv_force_wake_put(dev_priv, fw_engine);
> @@ -410,7 +413,14 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>  				 1);
>  	}
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +}
> +
> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> +{
> +	if (!dev_priv->uncore.funcs.force_wake_put)
> +		return;
>  
> +	gen6_gt_force_wake_put_no_rpm(dev_priv, fw_engine);
>  	intel_runtime_pm_put(dev_priv);
>  }
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/19] drm/i915: add forcewake functions that don't touch runtime PM

[Paulo Zanoni]

Hi

I'm sorry, I forgot to say. This series is quite old, and I changed it
a little bit since then (since I found one or two problems), including
this patch. I think that, to avoid wasting your time reviewing old
patches, I should resend the new series.

The problem is that this series should be on top of the 11 patches I
recently sent (with PC8/runtime PM fixes), so if we could review those
first, it would be better. We also need to decide the relative order
of merging your recent series and these patches, because they have
some conflicts.

Thanks,
Paulo

2014-02-27 11:43 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> On Thu, 2013-12-19 at 11:54 -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> To solve a chicken-and-egg problem. Currently when we get/put
>> forcewake we also get/put runtime PM and this works fine because the
>> runtime PM code doesn't need forcewake. But we're going to merge PC8
>> and runtime PM into a single feature, and the PC8 code (the LCPLL
>> code) does need the forcewake, so that specific piece of code needs to
>> call the _no_rpm version so it doesn't try to get runtime PM in the
>> code that gets runtime PM.
>>
>> For now the new functions are unused, we'll use them on the patch that
>> merges PC8 with runtime PM.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h     |  4 ++++
>>  drivers/gpu/drm/i915/intel_uncore.c | 30 ++++++++++++++++++++----------
>>  2 files changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index cc8afff..7f8ec08 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2533,6 +2533,10 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>>   */
>>  void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine);
>>  void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine);
>> +void gen6_gt_force_wake_get_no_rpm(struct drm_i915_private *dev_priv,
>> +                                int fw_engine);
>> +void gen6_gt_force_wake_put_no_rpm(struct drm_i915_private *dev_priv,
>> +                                int fw_engine);
>>
>>  int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
>>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 646fecf..6118d2c 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -368,15 +368,11 @@ void intel_uncore_sanitize(struct drm_device *dev)
>>   * be called at the beginning of the sequence followed by a call to
>>   * gen6_gt_force_wake_put() at the end of the sequence.
>>   */
>> -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
>> +void gen6_gt_force_wake_get_no_rpm(struct drm_i915_private *dev_priv,
>> +                                int fw_engine)
>>  {
>>       unsigned long irqflags;
>>
>> -     if (!dev_priv->uncore.funcs.force_wake_get)
>> -             return;
>> -
>> -     intel_runtime_pm_get(dev_priv);
>> -
>>       /* Redirect to VLV specific routine */
>>       if (IS_VALLEYVIEW(dev_priv->dev))
>>               return vlv_force_wake_get(dev_priv, fw_engine);
>> @@ -387,16 +383,23 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
>>       spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>>  }
>>
>> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
>> +{
>> +     if (!dev_priv->uncore.funcs.force_wake_get)
>> +             return;
>> +
>> +     intel_runtime_pm_get(dev_priv);
>> +     gen6_gt_force_wake_get_no_rpm(dev_priv, fw_engine);
>> +}
>> +
>>  /*
>>   * see gen6_gt_force_wake_get()
>>   */
>> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>> +void gen6_gt_force_wake_put_no_rpm(struct drm_i915_private *dev_priv,
>> +                                int fw_engine)
>>  {
>>       unsigned long irqflags;
>>
>> -     if (!dev_priv->uncore.funcs.force_wake_put)
>> -             return;
>> -
>>       /* Redirect to VLV specific routine */
>>       if (IS_VALLEYVIEW(dev_priv->dev))
>>               return vlv_force_wake_put(dev_priv, fw_engine);
>> @@ -410,7 +413,14 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>>                                1);
>>       }
>>       spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>> +}
>> +
>> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>> +{
>> +     if (!dev_priv->uncore.funcs.force_wake_put)
>> +             return;
>>
>> +     gen6_gt_force_wake_put_no_rpm(dev_priv, fw_engine);
>>       intel_runtime_pm_put(dev_priv);
>>  }
>>
>



-- 
Paulo Zanoni

Re: [PATCH 04/19] drm/i915: extract __hsw_do_{en, dis}able_package_c8

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 3941 bytes --]

On Thu, 2013-12-19 at 11:54 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> When we merge PC8 and runtime PM, these new functions are going to be
> called by the runtime suspend/resume functions, and their callers are
> going to be removed.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 64 +++++++++++++++++++++---------------
>  1 file changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c661423..db6523e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6638,19 +6638,11 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
>  	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
>  }
>  
> -void hsw_enable_pc8_work(struct work_struct *__work)
> +static void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv =
> -		container_of(to_delayed_work(__work), struct drm_i915_private,
> -			     pc8.enable_work);
>  	struct drm_device *dev = dev_priv->dev;
>  	uint32_t val;
>  
> -	WARN_ON(!HAS_PC8(dev));
> -
> -	if (dev_priv->pc8.enabled)
> -		return;
> -
>  	DRM_DEBUG_KMS("Enabling package C8+\n");
>  
>  	dev_priv->pc8.enabled = true;
> @@ -6664,7 +6656,21 @@ void hsw_enable_pc8_work(struct work_struct *__work)
>  	lpt_disable_clkout_dp(dev);
>  	hsw_pc8_disable_interrupts(dev);
>  	hsw_disable_lcpll(dev_priv, true, true);
> +}
> +
> +void hsw_enable_pc8_work(struct work_struct *__work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(to_delayed_work(__work), struct drm_i915_private,
> +			     pc8.enable_work);
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	WARN_ON(!HAS_PC8(dev));
>  
> +	if (dev_priv->pc8.enabled)
> +		return;
> +
> +	__hsw_do_enable_pc8(dev_priv);
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> @@ -6682,29 +6688,13 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
>  			      msecs_to_jiffies(i915_pc8_timeout));
>  }
>  
> -static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
> +static void __hsw_do_disable_package_c8(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	uint32_t val;
>  
> -	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
> -	WARN(dev_priv->pc8.disable_count < 0,
> -	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
> -
> -	dev_priv->pc8.disable_count++;
> -	if (dev_priv->pc8.disable_count != 1)
> -		return;
> -
> -	WARN_ON(!HAS_PC8(dev));
> -
> -	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
> -	if (!dev_priv->pc8.enabled)
> -		return;
> -
>  	DRM_DEBUG_KMS("Disabling package C8+\n");
>  
> -	intel_runtime_pm_get(dev_priv);
> -
>  	hsw_restore_lcpll(dev_priv);
>  	hsw_pc8_restore_interrupts(dev);
>  	lpt_init_pch_refclk(dev);
> @@ -6723,6 +6713,28 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
>  	dev_priv->pc8.enabled = false;
>  }
>  
> +static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
> +	WARN(dev_priv->pc8.disable_count < 0,
> +	     "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
> +
> +	dev_priv->pc8.disable_count++;
> +	if (dev_priv->pc8.disable_count != 1)
> +		return;
> +
> +	WARN_ON(!HAS_PC8(dev));
> +
> +	cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
> +	if (!dev_priv->pc8.enabled)
> +		return;
> +
> +	intel_runtime_pm_get(dev_priv);
> +	__hsw_do_disable_package_c8(dev_priv);
> +}
> +
>  void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
>  {
>  	if (!HAS_PC8(dev_priv->dev))


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/19] drm/i915: add forcewake functions that don't touch runtime PM

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 1107 bytes --]

On Thu, 2014-02-27 at 11:48 -0300, Paulo Zanoni wrote:
> Hi
> 
> I'm sorry, I forgot to say. This series is quite old, and I changed it
> a little bit since then (since I found one or two problems), including
> this patch. I think that, to avoid wasting your time reviewing old
> patches, I should resend the new series.

Ah, ok. Np, I can re-check the updated version once you submitted it.

> The problem is that this series should be on top of the 11 patches I
> recently sent (with PC8/runtime PM fixes), so if we could review those
> first, it would be better. We also need to decide the relative order
> of merging your recent series and these patches, because they have
> some conflicts.

Wrt. "Make PC8 become part of runtime PM" and "vlv power domains
support", I'd merge first the latter to reduce code churn, since you're
adding the RPM get/put calls on the power domain enable/disable path
which has changed somewhat after my patchset.

As we agreed I'm ok if we merge "PC8/runtime PM fixes" before the power
domains patchset, I can rebase my patchset on top.

--Imre


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/19] drm/i915: add forcewake functions that don't touch runtime PM

[Daniel Vetter]

On Thu, Feb 27, 2014 at 11:48:30AM -0300, Paulo Zanoni wrote:
> Hi
> 
> I'm sorry, I forgot to say. This series is quite old, and I changed it
> a little bit since then (since I found one or two problems), including
> this patch. I think that, to avoid wasting your time reviewing old
> patches, I should resend the new series.
> 
> The problem is that this series should be on top of the 11 patches I
> recently sent (with PC8/runtime PM fixes), so if we could review those
> first, it would be better. We also need to decide the relative order
> of merging your recent series and these patches, because they have
> some conflicts.

Hm, I've merged the first two patches of this series already, hope that
doesn't cause a fuzz ;-)

Now looking closer I'm puzzled by the pm_get/put calls in the forcewake
get/put functions. Imo any place which needs to have a power well up and
runtime (I include runtime pm as the overall power well here) should grab
a runtime pm reference for the entire access, not each register cycle.

The patch which added this was quite old, so probably before we've fixed
the bunch of runtime pm issues around gem batch buffers. And it was part
of a big patch which added get/puts mostly over debugfs files and similar
places. So I wonder whether we really still need this?

I'd much prefer if we could remove it, and if that's not possible fix up
the (hopefully few) places where we currently don't grab a runtime pm ref,
but should.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch