[PATCH 1/4] drm/i915: don't block resume on fb console resume

[PATCH 1/4] drm/i915: don't block resume on fb console resume

[Jesse Barnes]

The console lock can be contended, so rather than prevent other drivers
after us from being held up, queue the console suspend into the global
work queue that can happen anytime.  I've measured this to take around
200ms on my T420.  Combined with the ring freq/turbo change, we should
save almost 1/2 a second on resume.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_dma.c |    3 +++
 drivers/gpu/drm/i915/i915_drv.c |   17 ++++++++++++++---
 drivers/gpu/drm/i915/i915_drv.h |    7 +++++++
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d04facb..14526dc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1329,6 +1329,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	intel_modeset_gem_init(dev);
 
+	INIT_WORK(&dev_priv->console_resume_work, intel_console_resume);
+
 	ret = drm_irq_install(dev);
 	if (ret)
 		goto cleanup_gem;
@@ -1723,6 +1725,7 @@ int i915_driver_unload(struct drm_device *dev)
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		intel_fbdev_fini(dev);
 		intel_modeset_cleanup(dev);
+		cancel_work_sync(&dev_priv->console_resume_work);
 
 		/*
 		 * free the memory space allocated for the child device
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4238853..4cd7404 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -519,6 +519,18 @@ int i915_suspend(struct drm_device *dev, pm_message_t state)
 	return 0;
 }
 
+void intel_console_resume(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private,
+			     console_resume_work);
+	struct drm_device *dev = dev_priv->dev;
+
+	console_lock();
+	intel_fbdev_set_suspend(dev, 0);
+	console_unlock();
+}
+
 static int i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -555,9 +567,8 @@ static int i915_drm_thaw(struct drm_device *dev)
 
 	dev_priv->modeset_on_lid = 0;
 
-	console_lock();
-	intel_fbdev_set_suspend(dev, 0);
-	console_unlock();
+	schedule_work(&dev_priv->console_resume_work);
+
 	return error;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a2c5e89..045963d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -455,6 +455,12 @@ typedef struct drm_i915_private {
 	u32 hotplug_supported_mask;
 	struct work_struct hotplug_work;
 
+	/*
+	 * The console may be contended at resume, but we don't
+	 * want it to block on it.
+	 */
+	struct work_struct console_resume_work;
+
 	int num_pipe;
 	int num_pch_pll;
 
@@ -1255,6 +1261,7 @@ extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
 extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
 extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
 
+extern void intel_console_resume(struct work_struct *work);
 
 /* i915_irq.c */
 void i915_hangcheck_elapsed(unsigned long data);
-- 
1.7.9.5

[PATCH 2/4] drm/i915: put ring frequency and turbo setup into a work queue v3

[Jesse Barnes]

Communicating via the mailbox registers with the PCU can take quite
awhile.  And updating the ring frequency or enabling turbo is not
something that needs to happen synchronously, so take it out of our init
and resume paths to speed things up (~200ms on my T420).

v2: add comment about why we use a work queue (Daniel)
    make sure work queue is idle on suspend (Daniel)
    use a delayed work queue since there's no hurry (Daniel)
v3: make cleanup symmetric and just call cancel work directly (Daniel)

References: https://bugs.freedesktop.org/show_bug.cgi?id=54089

Signed-of-by: Jesse Barnes <jbarnes@virtuougseek.org>

drm/i915: move gt_cleanup into disable_gt_powersave for consistency
---
 drivers/gpu/drm/i915/i915_drv.c |    2 ++
 drivers/gpu/drm/i915/i915_drv.h |    2 ++
 drivers/gpu/drm/i915/intel_pm.c |   28 ++++++++++++++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4cd7404..9feedb4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -470,6 +470,8 @@ static int i915_drm_freeze(struct drm_device *dev)
 			return error;
 		}
 
+		cancel_delayed_work_sync(&dev_priv->gen6_power_work);
+
 		intel_modeset_disable(dev);
 
 		drm_irq_uninstall(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 045963d..2f01de8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -873,6 +873,8 @@ typedef struct drm_i915_private {
 		int r_t;
 	} ips;
 
+	struct delayed_work gen6_power_work;
+
 	enum no_fbc_reason no_fbc_reason;
 
 	struct drm_mm_node *compressed_fb;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 59c31f6..7360c91 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3304,23 +3304,45 @@ static void intel_init_emon(struct drm_device *dev)
 
 void intel_disable_gt_powersave(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_disable_drps(dev);
 		ironlake_disable_rc6(dev);
 	} else if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) {
+		cancel_delayed_work_sync(&dev_priv->gen6_power_work);
 		gen6_disable_rps(dev);
 	}
 }
 
+static void intel_gen6_powersave_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private,
+			     gen6_power_work.work);
+	struct drm_device *dev = dev_priv->dev;
+
+	mutex_lock(&dev->struct_mutex);
+	gen6_enable_rps(dev);
+	gen6_update_ring_freq(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
+
 void intel_enable_gt_powersave(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_enable_drps(dev);
 		ironlake_enable_rc6(dev);
 		intel_init_emon(dev);
 	} else if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
-		gen6_enable_rps(dev);
-		gen6_update_ring_freq(dev);
+		/*
+		 * PCU communication is slow and this doesn't need to be
+		 * done at any specific time, so do this out of our fast path
+		 * to make resume and init faster.
+		 */
+		schedule_delayed_work(&dev_priv->gen6_power_work, HZ);
 	}
 }
 
@@ -4183,6 +4205,8 @@ void intel_gt_init(struct drm_device *dev)
 		dev_priv->gt.force_wake_get = __gen6_gt_force_wake_get;
 		dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
 	}
+	INIT_DELAYED_WORK(&dev_priv->gen6_power_work,
+			  intel_gen6_powersave_work);
 }
 
 int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
-- 
1.7.9.5

[PATCH 3/4] drm/i915: protect RPS/RC6 related accesses (including PCU) with a new mutex

[Jesse Barnes]

This allows the power related code to run independently of the rest of
the pipeline, extending the resume and init time improvements into
userspace, which would otherwise have been blocked on the struct mutex
if we were doing PCU communication.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   20 ++++++++++----------
 drivers/gpu/drm/i915/i915_dma.c     |    2 ++
 drivers/gpu/drm/i915/i915_drv.h     |    6 ++++++
 drivers/gpu/drm/i915/i915_irq.c     |    4 ++--
 drivers/gpu/drm/i915/intel_pm.c     |   16 +++++++++-------
 5 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0e405e5..2cae435 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1280,7 +1280,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 		return 0;
 	}
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
 		return ret;
 
@@ -1296,7 +1296,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 		seq_printf(m, "%d\t\t%d\n", gpu_freq * GT_FREQUENCY_MULTIPLIER, ia_freq * 100);
 	}
 
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return 0;
 }
@@ -1713,13 +1713,13 @@ i915_max_freq_read(struct file *filp,
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
 		return ret;
 
 	len = snprintf(buf, sizeof(buf),
 		       "max freq: %d\n", dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	if (len > sizeof(buf))
 		len = sizeof(buf);
@@ -1754,7 +1754,7 @@ i915_max_freq_write(struct file *filp,
 
 	DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val);
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
 		return ret;
 
@@ -1764,7 +1764,7 @@ i915_max_freq_write(struct file *filp,
 	dev_priv->rps.max_delay = val / GT_FREQUENCY_MULTIPLIER;
 
 	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return cnt;
 }
@@ -1789,13 +1789,13 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
 		return ret;
 
 	len = snprintf(buf, sizeof(buf),
 		       "min freq: %d\n", dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	if (len > sizeof(buf))
 		len = sizeof(buf);
@@ -1828,7 +1828,7 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
 
 	DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val);
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
 		return ret;
 
@@ -1838,7 +1838,7 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	dev_priv->rps.min_delay = val / GT_FREQUENCY_MULTIPLIER;
 
 	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return cnt;
 }
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 14526dc..1ce7e49 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1625,6 +1625,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	spin_lock_init(&dev_priv->rps.lock);
 	spin_lock_init(&dev_priv->dpio_lock);
 
+	mutex_init(&dev_priv->rps.hw_lock);
+
 	if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
 		dev_priv->num_pipe = 3;
 	else if (IS_MOBILE(dev) || !IS_GEN2(dev))
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2f01de8..b41a90b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -850,6 +850,12 @@ typedef struct drm_i915_private {
 		u8 cur_delay;
 		u8 min_delay;
 		u8 max_delay;
+
+		/*
+		 * Protects RPS/RC6 register access and PCU communication.
+		 * Must be taken after struct_mutex if nested.
+		 */
+		struct mutex hw_lock;
 	} rps;
 
 	/* ilk-only ips/rps state. Everything in here is protected by the global
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b92e6bfb..ad99b10 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -378,7 +378,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0)
 		return;
 
-	mutex_lock(&dev_priv->dev->struct_mutex);
+	mutex_lock(&dev_priv->rps.hw_lock);
 
 	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD)
 		new_delay = dev_priv->rps.cur_delay + 1;
@@ -393,7 +393,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
 		gen6_set_rps(dev_priv->dev, new_delay);
 	}
 
-	mutex_unlock(&dev_priv->dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7360c91..633fa72 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2323,7 +2323,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 limits = gen6_rps_limits(dev_priv, &val);
 
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 	WARN_ON(val > dev_priv->rps.max_delay);
 	WARN_ON(val < dev_priv->rps.min_delay);
 
@@ -2409,7 +2409,7 @@ static void gen6_enable_rps(struct drm_device *dev)
 	int rc6_mode;
 	int i, ret;
 
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 
 	/* Here begins a magic sequence of register writes to enable
 	 * auto-downclocking.
@@ -2550,7 +2550,7 @@ static void gen6_update_ring_freq(struct drm_device *dev)
 	int gpu_freq, ia_freq, max_ia_freq;
 	int scaling_factor = 180;
 
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 
 	max_ia_freq = cpufreq_quick_get_max(0);
 	/*
@@ -3311,7 +3311,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 		ironlake_disable_rc6(dev);
 	} else if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) {
 		cancel_delayed_work_sync(&dev_priv->gen6_power_work);
+		mutex_lock(&dev_priv->rps.hw_lock);
 		gen6_disable_rps(dev);
+		mutex_unlock(&dev_priv->rps.hw_lock);
 	}
 }
 
@@ -3322,10 +3324,10 @@ static void intel_gen6_powersave_work(struct work_struct *work)
 			     gen6_power_work.work);
 	struct drm_device *dev = dev_priv->dev;
 
-	mutex_lock(&dev->struct_mutex);
+	mutex_lock(&dev_priv->rps.hw_lock);
 	gen6_enable_rps(dev);
 	gen6_update_ring_freq(dev);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
 void intel_enable_gt_powersave(struct drm_device *dev)
@@ -4211,7 +4213,7 @@ void intel_gt_init(struct drm_device *dev)
 
 int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
 {
-	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 
 	if (I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
 		DRM_DEBUG_DRIVER("warning: pcode (read) mailbox access failed\n");
@@ -4235,7 +4237,7 @@ 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)
 {
-	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
 
 	if (I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
 		DRM_DEBUG_DRIVER("warning: pcode (write) mailbox access failed\n");
-- 
1.7.9.5

[PATCH 4/4] drm/i915: don't rewrite the GTT on resume v3

[Jesse Barnes]

The BIOS shouldn't be touching this memory across suspend/resume, so
just leave it alone.  This saves us ~6ms on resume on my T420 (retested
with write combined PTEs).

v2: change gtt restore default on pre-gen4 (Chris)
    move needs_gtt_restore flag into dev_priv
v3: make sure we restore GTT on resume from hibernate (Daniel)
    use opregion support as the cutoff for restore from resume (Chris)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_dma.c |    8 +++++++-
 drivers/gpu/drm/i915/i915_drv.c |   39 +++++++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/i915_drv.h |    2 ++
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1ce7e49..69ac4a5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1467,6 +1467,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	struct intel_device_info *info;
 	int ret = 0, mmio_bar, mmio_size;
 	uint32_t aperture_size;
+	bool opregion_supported = false;
 
 	info = (struct intel_device_info *) flags;
 
@@ -1592,7 +1593,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev);
 	intel_setup_gmbus(dev);
-	intel_opregion_setup(dev);
+	if (!intel_opregion_setup(dev))
+		opregion_supported = true;
+
+	/* Gen3+ should have saner BIOSes (we hope) */
+	if (!opregion_supported)
+		dev_priv->needs_gtt_restore = true;
 
 	/* Make sure the bios did its job and set up vital registers */
 	intel_setup_bios(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9feedb4..1ea70ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -533,19 +533,11 @@ void intel_console_resume(struct work_struct *work)
 	console_unlock();
 }
 
-static int i915_drm_thaw(struct drm_device *dev)
+static int __i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int error = 0;
 
-	intel_gt_reset(dev);
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		mutex_lock(&dev->struct_mutex);
-		i915_gem_restore_gtt_mappings(dev);
-		mutex_unlock(&dev->struct_mutex);
-	}
-
 	i915_restore_state(dev);
 	intel_opregion_setup(dev);
 
@@ -574,8 +566,26 @@ static int i915_drm_thaw(struct drm_device *dev)
 	return error;
 }
 
+static int i915_drm_thaw(struct drm_device *dev)
+{
+	int error = 0;
+
+	intel_gt_reset(dev);
+
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		mutex_lock(&dev->struct_mutex);
+		i915_gem_restore_gtt_mappings(dev);
+		mutex_unlock(&dev->struct_mutex);
+	}
+
+	__i915_drm_thaw(dev);
+
+	return error;
+}
+
 int i915_resume(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
@@ -586,7 +596,16 @@ int i915_resume(struct drm_device *dev)
 
 	pci_set_master(dev->pdev);
 
-	ret = i915_drm_thaw(dev);
+	intel_gt_reset(dev);
+
+	if (drm_core_check_feature(dev, DRIVER_MODESET) &&
+	    dev_priv->needs_gtt_restore) {
+		mutex_lock(&dev->struct_mutex);
+		i915_gem_restore_gtt_mappings(dev);
+		mutex_unlock(&dev->struct_mutex);
+	}
+
+	ret = __i915_drm_thaw(dev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b41a90b..df65e48 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -881,6 +881,8 @@ typedef struct drm_i915_private {
 
 	struct delayed_work gen6_power_work;
 
+	bool needs_gtt_restore;
+
 	enum no_fbc_reason no_fbc_reason;
 
 	struct drm_mm_node *compressed_fb;
-- 
1.7.9.5

Re: [PATCH 1/4] drm/i915: don't block resume on fb console resume

[Chris Wilson]

On Wed, 31 Oct 2012 15:41:02 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> The console lock can be contended, so rather than prevent other drivers
> after us from being held up, queue the console suspend into the global
> work queue that can happen anytime.  I've measured this to take around
> 200ms on my T420.  Combined with the ring freq/turbo change, we should
> save almost 1/2 a second on resume.

Hmm, I'd rather not postpone the work unless actually contended,
otherwise we may end up just increasing the resume time whilst avoiding
the blame. Perhaps:
  if (console_trylock() {
      intel_fbdev_set_suspend(dev, 0);
      console_unlock();
  } else
     schedule_work(&dev_priv->console_resume_work);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/4] drm/i915: put ring frequency and turbo setup into a work queue v3

[Chris Wilson]

On Wed, 31 Oct 2012 15:41:03 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Communicating via the mailbox registers with the PCU can take quite
> awhile.  And updating the ring frequency or enabling turbo is not
> something that needs to happen synchronously, so take it out of our init
> and resume paths to speed things up (~200ms on my T420).
> 
> v2: add comment about why we use a work queue (Daniel)
>     make sure work queue is idle on suspend (Daniel)
>     use a delayed work queue since there's no hurry (Daniel)
> v3: make cleanup symmetric and just call cancel work directly (Daniel)
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=54089

Use round_jiffies_up_relative(HZ) to compute the delay to align the
wakeup with others.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/4] drm/i915: protect RPS/RC6 related accesses (including PCU) with a new mutex

[Chris Wilson]

On Wed, 31 Oct 2012 15:41:04 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> This allows the power related code to run independently of the rest of
> the pipeline, extending the resume and init time improvements into
> userspace, which would otherwise have been blocked on the struct mutex
> if we were doing PCU communication.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 4/4] drm/i915: don't rewrite the GTT on resume v3

[Chris Wilson]

On Wed, 31 Oct 2012 15:41:05 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> The BIOS shouldn't be touching this memory across suspend/resume, so
> just leave it alone.  This saves us ~6ms on resume on my T420 (retested
> with write combined PTEs).
> 
> v2: change gtt restore default on pre-gen4 (Chris)
>     move needs_gtt_restore flag into dev_priv
> v3: make sure we restore GTT on resume from hibernate (Daniel)
>     use opregion support as the cutoff for restore from resume (Chris)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

My troublesome PNV of yore remains happy with this patch, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Bikeshed: intel_bios_has_opregion(), as we test for this in different
locations in different ways.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 4/4] drm/i915: don't rewrite the GTT on resume v3

[Jesse Barnes]

On Fri, 02 Nov 2012 15:16:36 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Wed, 31 Oct 2012 15:41:05 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > The BIOS shouldn't be touching this memory across suspend/resume, so
> > just leave it alone.  This saves us ~6ms on resume on my T420 (retested
> > with write combined PTEs).
> > 
> > v2: change gtt restore default on pre-gen4 (Chris)
> >     move needs_gtt_restore flag into dev_priv
> > v3: make sure we restore GTT on resume from hibernate (Daniel)
> >     use opregion support as the cutoff for restore from resume (Chris)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> My troublesome PNV of yore remains happy with this patch, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Bikeshed: intel_bios_has_opregion(), as we test for this in different
> locations in different ways.
> -Chris
> 

Ended up using dev_priv->opregion.header as the check, seems simplier.

Integrated your other comments and reposted a new series.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 1/4] drm/i915: don't block resume on fb console resume

[Paul Menzel]

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

Dear Jesse,


unfortunately due to mail problems with SourceForge I did not get the
latest series.

Do you have a public repository with these patches in it?


Am Mittwoch, den 31.10.2012, 15:41 -0700 schrieb Jesse Barnes:
> The console lock can be contended, so rather than prevent other drivers
> after us from being held up, queue the console suspend into the global
> work queue that can happen anytime.  I've measured this to take around
> 200ms on my T420.  Combined with the ring freq/turbo change, we should
> save almost 1/2 a second on resume.

In #intel-gfx on irc.freenode.net, Daniel told me that this is mostly
true for Sandybridge. Could you clarify that in the commit message
please.

> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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 1/4] drm/i915: don't block resume on fb console resume

[Jesse Barnes]

On Sun, 04 Nov 2012 22:16:39 +0100
Paul Menzel <paulepanter@users.sourceforge.net> wrote:

> Dear Jesse,
> 
> 
> unfortunately due to mail problems with SourceForge I did not get the
> latest series.

No I haven't pushed an updated version anywhere, but I think Daniel has
them queued now?

> 
> Do you have a public repository with these patches in it?
> 
> 
> Am Mittwoch, den 31.10.2012, 15:41 -0700 schrieb Jesse Barnes:
> > The console lock can be contended, so rather than prevent other drivers
> > after us from being held up, queue the console suspend into the global
> > work queue that can happen anytime.  I've measured this to take around
> > 200ms on my T420.  Combined with the ring freq/turbo change, we should
> > save almost 1/2 a second on resume.
> 
> In #intel-gfx on irc.freenode.net, Daniel told me that this is mostly
> true for Sandybridge. Could you clarify that in the commit message
> please.

The fb resume and GTT rewrite patches will benefit every Intel gfx
platform.  The work queue for ring freq updates will benefit SNB+.

Jesse