intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] PM: make VT switching to the suspend console optional v3
@ 2013-02-04 13:37 Jesse Barnes
  2013-02-04 13:37 ` [PATCH 2/3] fb: add support for drivers not needing VT switch at suspend/resume time Jesse Barnes
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jesse Barnes @ 2013-02-04 13:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: intel-gfx, linux-pm, rjw

KMS drivers can potentially restore the display configuration without
userspace help.  Such drivers can can call a new funciton,
pm_vt_switch_required(false) if they support this feature.  In that
case, the PM layer won't VT switch to the suspend console at suspend
time and then back to the original VT on resume, but rather leave things
alone for a nicer looking suspend and resume sequence.

v2: make a function so we can handle multiple drivers (Alan)
v3: use a list to track device requests (Rafael)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 include/linux/pm.h     |    4 ++
 kernel/power/console.c |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 03d7bb1..98310eb 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -35,6 +35,10 @@ extern void (*pm_idle)(void);
 extern void (*pm_power_off)(void);
 extern void (*pm_power_off_prepare)(void);
 
+struct device; /* we have a circular dep with device.h */
+extern void pm_vt_switch_required(struct device *dev, bool required);
+extern void pm_vt_switch_unregister(struct device *dev);
+
 /*
  * Device power management
  */
diff --git a/kernel/power/console.c b/kernel/power/console.c
index b1dc456..4871ca9 100644
--- a/kernel/power/console.c
+++ b/kernel/power/console.c
@@ -4,6 +4,7 @@
  * Originally from swsusp.
  */
 
+#include <linux/console.h>
 #include <linux/vt_kern.h>
 #include <linux/kbd_kern.h>
 #include <linux/vt.h>
@@ -14,8 +15,119 @@
 
 static int orig_fgconsole, orig_kmsg;
 
+DEFINE_MUTEX(vt_switch_mutex);
+
+struct pm_vt_switch {
+	struct list_head head;
+	struct device *dev;
+	bool required;
+};
+
+LIST_HEAD(pm_vt_switch_list);
+
+
+/**
+ * pm_vt_switch_required - indicate VT switch at suspend requirements
+ * @dev: device
+ * @required: if true, caller needs VT switch at suspend/resume time
+ *
+ * The different console drivers may or may not require VT switches across
+ * suspend/resume, depending on how they handle restoring video state and
+ * what may be running.
+ *
+ * Drivers can indicate support for switchless suspend/resume, which can
+ * save time and flicker, by using this routine and passing 'false' as
+ * the argument.  If any loaded driver needs VT switching, or the
+ * no_console_suspend argument has been passed on the command line, VT
+ * switches will occur.
+ */
+void pm_vt_switch_required(struct device *dev, bool required)
+{
+	struct pm_vt_switch *entry, *tmp;
+
+	mutex_lock(&vt_switch_mutex);
+	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
+		if (tmp->dev == dev) {
+			/* already registered, update requirement */
+			tmp->required = required;
+			goto out;
+		}
+	}
+
+	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		goto out;
+
+	entry->required = required;
+	entry->dev = dev;
+
+	list_add(&entry->head, &pm_vt_switch_list);
+out:
+	mutex_unlock(&vt_switch_mutex);
+}
+EXPORT_SYMBOL(pm_vt_switch_required);
+
+/**
+ * pm_vt_switch_unregister - stop tracking a device's VT switching needs
+ * @dev: device
+ *
+ * Remove @dev from the vt switch list.
+ */
+void pm_vt_switch_unregister(struct device *dev)
+{
+	struct pm_vt_switch *tmp;
+
+	mutex_lock(&vt_switch_mutex);
+	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
+		if (tmp->dev == dev) {
+			list_del(&tmp->head);
+			break;
+		}
+	}
+	mutex_unlock(&vt_switch_mutex);
+}
+
+/*
+ * There are three cases when a VT switch on suspend/resume are required:
+ *   1) no driver has indicated a requirement one way or another, so preserve
+ *      the old behavior
+ *   2) console suspend is disabled, we want to see debug messages across
+ *      suspend/resume
+ *   3) any registered driver indicates it needs a VT switch
+ *
+ * If none of these conditions is present, meaning we have at least one driver
+ * that doesn't need the switch, and none that do, we can avoid it to make
+ * resume look a little prettier (and suspend too, but that's usually hidden,
+ * e.g. when closing the lid on a laptop).
+ */
+static bool pm_vt_switch(void)
+{
+	struct pm_vt_switch *entry;
+	bool ret = true;
+
+	mutex_lock(&vt_switch_mutex);
+	if (list_empty(&pm_vt_switch_list))
+		goto out;
+
+	if (!console_suspend_enabled)
+		goto out;
+
+	list_for_each_entry(entry, &pm_vt_switch_list, head) {
+		if (entry->required)
+			goto out;
+	}
+
+	ret = false;
+out:
+	mutex_unlock(&vt_switch_mutex);
+	return ret;
+}
+
 int pm_prepare_console(void)
 {
+	if (!pm_vt_switch())
+		return 0;
+
 	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
 	if (orig_fgconsole < 0)
 		return 1;
@@ -26,6 +138,9 @@ int pm_prepare_console(void)
 
 void pm_restore_console(void)
 {
+	if (!pm_vt_switch())
+		return;
+
 	if (orig_fgconsole >= 0) {
 		vt_move_to_console(orig_fgconsole, 0);
 		vt_kmsg_redirect(orig_kmsg);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] fb: add support for drivers not needing VT switch at suspend/resume time
  2013-02-04 13:37 [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Jesse Barnes
@ 2013-02-04 13:37 ` Jesse Barnes
  2013-02-04 13:37 ` [PATCH 3/3] drm/i915: support resume without VT switch v2 Jesse Barnes
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Jesse Barnes @ 2013-02-04 13:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: rjw, intel-gfx, linux-pm

Use the new PM routines to indicate whether we need to VT switch at suspend
and resume time.  When a new driver is bound, set its flag accordingly,
and when unbound, remove it from the PM's console tracking list.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/video/fbmem.c |    7 +++++++
 include/linux/fb.h    |    2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 3ff0105..240d783 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1643,6 +1643,11 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	if (!fb_info->modelist.prev || !fb_info->modelist.next)
 		INIT_LIST_HEAD(&fb_info->modelist);
 
+	if (fb_info->skip_vt_switch)
+		pm_vt_switch_required(fb_info->dev, false);
+	else
+		pm_vt_switch_required(fb_info->dev, true);
+
 	fb_var_to_videomode(&mode, &fb_info->var);
 	fb_add_videomode(&mode, &fb_info->modelist);
 	registered_fb[i] = fb_info;
@@ -1673,6 +1678,8 @@ static int do_unregister_framebuffer(struct fb_info *fb_info)
 	if (ret)
 		return -EINVAL;
 
+	pm_vt_switch_unregister(fb_info->dev);
+
 	unlink_framebuffer(fb_info);
 	if (fb_info->pixmap.addr &&
 	    (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..9a7538a 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -499,6 +499,8 @@ struct fb_info {
 			resource_size_t size;
 		} ranges[0];
 	} *apertures;
+
+	bool skip_vt_switch; /* no VT switch on suspend/resume required */
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] drm/i915: support resume without VT switch v2
  2013-02-04 13:37 [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Jesse Barnes
  2013-02-04 13:37 ` [PATCH 2/3] fb: add support for drivers not needing VT switch at suspend/resume time Jesse Barnes
@ 2013-02-04 13:37 ` Jesse Barnes
  2013-02-06  9:20   ` [Intel-gfx] " Daniel Vetter
  2013-02-04 20:26 ` [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Rafael J. Wysocki
  2013-02-05 22:01 ` Rafael J. Wysocki
  3 siblings, 1 reply; 9+ messages in thread
From: Jesse Barnes @ 2013-02-04 13:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: rjw, intel-gfx, linux-pm

Add support for resuming the suspend configuration at resume time to
avoid slow and ugly VT switches during suspend and resume.  Also emit a
hotplug event at resume time to make sure any potential configuration
changes (monitors coming and going, dock events) are handled properly.

v2: use new fb flag to indicate we don't need to VT switch

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c      |   28 +++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   25 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_fb.c      |    3 +++
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1172658..7dbaa01 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -483,8 +483,6 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
 
-		intel_modeset_disable(dev);
-
 		drm_irq_uninstall(dev);
 	}
 
@@ -544,6 +542,24 @@ void intel_console_resume(struct work_struct *work)
 	console_unlock();
 }
 
+static void intel_resume_hotplug(struct drm_device *dev)
+{
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct intel_encoder *encoder;
+
+	mutex_lock(&mode_config->mutex);
+	DRM_DEBUG_KMS("running encoder hotplug functions\n");
+
+	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+		if (encoder->hot_plug)
+			encoder->hot_plug(encoder);
+
+	mutex_unlock(&mode_config->mutex);
+
+	/* Just fire off a uevent and let userspace tell us what to do */
+	drm_helper_hpd_irq_event(dev);
+}
+
 static int __i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -563,8 +579,14 @@ static int __i915_drm_thaw(struct drm_device *dev)
 		mutex_unlock(&dev->struct_mutex);
 
 		intel_modeset_init_hw(dev);
-		intel_modeset_setup_hw_state(dev, false);
 		drm_irq_install(dev);
+
+		/* Resume the modeset for every activated CRTC */
+		mutex_lock(&dev->mode_config.mutex);
+		intel_resume_force_mode(dev);
+		mutex_unlock(&dev->mode_config.mutex);
+
+		intel_resume_hotplug(dev);
 	}
 
 	intel_opregion_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 12ab3bd..c8b1896 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1680,6 +1680,7 @@ extern void gen6_set_rps(struct drm_device *dev, u8 val);
 extern void intel_detect_pch(struct drm_device *dev);
 extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
 extern int intel_enable_rc6(const struct drm_device *dev);
+extern int intel_resume_force_mode(struct drm_device *dev);
 
 extern bool i915_semaphore_is_enabled(struct drm_device *dev);
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index da1ad9c..a127877 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9369,6 +9369,31 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	drm_mode_config_cleanup(dev);
 }
 
+int intel_resume_force_mode(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
+	struct drm_encoder_helper_funcs *encoder_funcs;
+	struct drm_crtc_helper_funcs *crtc_funcs;
+	int ret;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+
+		if (!crtc->enabled) {
+			DRM_ERROR("skipping disabled crtc\n");
+			continue;
+		}
+
+		ret = intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
+				     crtc->fb);
+
+		if (ret == false)
+			DRM_ERROR("failed to set mode on crtc %p\n", crtc);
+	}
+
+	return 0;
+}
+
 /*
  * Return which encoder is currently attached for connector.
  */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 7b30b5c..d4955b5 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -148,6 +148,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	}
 	info->screen_size = size;
 
+	/* This driver doesn't need a VT switch to restore the mode on resume */
+	info->skip_vt_switch = true;
+
 //	memset(info->screen_base, 0, size);
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] PM: make VT switching to the suspend console optional v3
  2013-02-04 13:37 [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Jesse Barnes
  2013-02-04 13:37 ` [PATCH 2/3] fb: add support for drivers not needing VT switch at suspend/resume time Jesse Barnes
  2013-02-04 13:37 ` [PATCH 3/3] drm/i915: support resume without VT switch v2 Jesse Barnes
@ 2013-02-04 20:26 ` Rafael J. Wysocki
  2013-02-05 13:55   ` Jesse Barnes
  2013-02-05 22:01 ` Rafael J. Wysocki
  3 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-02-04 20:26 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: linux-kernel, intel-gfx, linux-pm

On Monday, February 04, 2013 01:37:20 PM Jesse Barnes wrote:
> KMS drivers can potentially restore the display configuration without
> userspace help.  Such drivers can can call a new funciton,
> pm_vt_switch_required(false) if they support this feature.  In that
> case, the PM layer won't VT switch to the suspend console at suspend
> time and then back to the original VT on resume, but rather leave things
> alone for a nicer looking suspend and resume sequence.
> 
> v2: make a function so we can handle multiple drivers (Alan)
> v3: use a list to track device requests (Rafael)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

for all [1-3/3].

Thanks,
Rafael


> ---
>  include/linux/pm.h     |    4 ++
>  kernel/power/console.c |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 119 insertions(+)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 03d7bb1..98310eb 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -35,6 +35,10 @@ extern void (*pm_idle)(void);
>  extern void (*pm_power_off)(void);
>  extern void (*pm_power_off_prepare)(void);
>  
> +struct device; /* we have a circular dep with device.h */
> +extern void pm_vt_switch_required(struct device *dev, bool required);
> +extern void pm_vt_switch_unregister(struct device *dev);
> +
>  /*
>   * Device power management
>   */
> diff --git a/kernel/power/console.c b/kernel/power/console.c
> index b1dc456..4871ca9 100644
> --- a/kernel/power/console.c
> +++ b/kernel/power/console.c
> @@ -4,6 +4,7 @@
>   * Originally from swsusp.
>   */
>  
> +#include <linux/console.h>
>  #include <linux/vt_kern.h>
>  #include <linux/kbd_kern.h>
>  #include <linux/vt.h>
> @@ -14,8 +15,119 @@
>  
>  static int orig_fgconsole, orig_kmsg;
>  
> +DEFINE_MUTEX(vt_switch_mutex);
> +
> +struct pm_vt_switch {
> +	struct list_head head;
> +	struct device *dev;
> +	bool required;
> +};
> +
> +LIST_HEAD(pm_vt_switch_list);
> +
> +
> +/**
> + * pm_vt_switch_required - indicate VT switch at suspend requirements
> + * @dev: device
> + * @required: if true, caller needs VT switch at suspend/resume time
> + *
> + * The different console drivers may or may not require VT switches across
> + * suspend/resume, depending on how they handle restoring video state and
> + * what may be running.
> + *
> + * Drivers can indicate support for switchless suspend/resume, which can
> + * save time and flicker, by using this routine and passing 'false' as
> + * the argument.  If any loaded driver needs VT switching, or the
> + * no_console_suspend argument has been passed on the command line, VT
> + * switches will occur.
> + */
> +void pm_vt_switch_required(struct device *dev, bool required)
> +{
> +	struct pm_vt_switch *entry, *tmp;
> +
> +	mutex_lock(&vt_switch_mutex);
> +	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
> +		if (tmp->dev == dev) {
> +			/* already registered, update requirement */
> +			tmp->required = required;
> +			goto out;
> +		}
> +	}
> +
> +	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		goto out;
> +
> +	entry->required = required;
> +	entry->dev = dev;
> +
> +	list_add(&entry->head, &pm_vt_switch_list);
> +out:
> +	mutex_unlock(&vt_switch_mutex);
> +}
> +EXPORT_SYMBOL(pm_vt_switch_required);
> +
> +/**
> + * pm_vt_switch_unregister - stop tracking a device's VT switching needs
> + * @dev: device
> + *
> + * Remove @dev from the vt switch list.
> + */
> +void pm_vt_switch_unregister(struct device *dev)
> +{
> +	struct pm_vt_switch *tmp;
> +
> +	mutex_lock(&vt_switch_mutex);
> +	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
> +		if (tmp->dev == dev) {
> +			list_del(&tmp->head);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&vt_switch_mutex);
> +}
> +
> +/*
> + * There are three cases when a VT switch on suspend/resume are required:
> + *   1) no driver has indicated a requirement one way or another, so preserve
> + *      the old behavior
> + *   2) console suspend is disabled, we want to see debug messages across
> + *      suspend/resume
> + *   3) any registered driver indicates it needs a VT switch
> + *
> + * If none of these conditions is present, meaning we have at least one driver
> + * that doesn't need the switch, and none that do, we can avoid it to make
> + * resume look a little prettier (and suspend too, but that's usually hidden,
> + * e.g. when closing the lid on a laptop).
> + */
> +static bool pm_vt_switch(void)
> +{
> +	struct pm_vt_switch *entry;
> +	bool ret = true;
> +
> +	mutex_lock(&vt_switch_mutex);
> +	if (list_empty(&pm_vt_switch_list))
> +		goto out;
> +
> +	if (!console_suspend_enabled)
> +		goto out;
> +
> +	list_for_each_entry(entry, &pm_vt_switch_list, head) {
> +		if (entry->required)
> +			goto out;
> +	}
> +
> +	ret = false;
> +out:
> +	mutex_unlock(&vt_switch_mutex);
> +	return ret;
> +}
> +
>  int pm_prepare_console(void)
>  {
> +	if (!pm_vt_switch())
> +		return 0;
> +
>  	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
>  	if (orig_fgconsole < 0)
>  		return 1;
> @@ -26,6 +138,9 @@ int pm_prepare_console(void)
>  
>  void pm_restore_console(void)
>  {
> +	if (!pm_vt_switch())
> +		return;
> +
>  	if (orig_fgconsole >= 0) {
>  		vt_move_to_console(orig_fgconsole, 0);
>  		vt_kmsg_redirect(orig_kmsg);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] PM: make VT switching to the suspend console optional v3
  2013-02-04 20:26 ` [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Rafael J. Wysocki
@ 2013-02-05 13:55   ` Jesse Barnes
  2013-02-05 22:02     ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Jesse Barnes @ 2013-02-05 13:55 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-kernel, intel-gfx, linux-pm

On Mon, 04 Feb 2013 21:26:26 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Monday, February 04, 2013 01:37:20 PM Jesse Barnes wrote:
> > KMS drivers can potentially restore the display configuration without
> > userspace help.  Such drivers can can call a new funciton,
> > pm_vt_switch_required(false) if they support this feature.  In that
> > case, the PM layer won't VT switch to the suspend console at suspend
> > time and then back to the original VT on resume, but rather leave things
> > alone for a nicer looking suspend and resume sequence.
> > 
> > v2: make a function so we can handle multiple drivers (Alan)
> > v3: use a list to track device requests (Rafael)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> for all [1-3/3].

Any chance for an r-b on the PM one at least?  Then Daniel could
probably push this through drm-intel-next.

Thanks,
Jesse

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] PM: make VT switching to the suspend console optional v3
  2013-02-04 13:37 [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Jesse Barnes
                   ` (2 preceding siblings ...)
  2013-02-04 20:26 ` [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Rafael J. Wysocki
@ 2013-02-05 22:01 ` Rafael J. Wysocki
  3 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05 22:01 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: linux-kernel, intel-gfx, linux-pm

On Monday, February 04, 2013 01:37:20 PM Jesse Barnes wrote:
> KMS drivers can potentially restore the display configuration without
> userspace help.  Such drivers can can call a new funciton,
> pm_vt_switch_required(false) if they support this feature.  In that
> case, the PM layer won't VT switch to the suspend console at suspend
> time and then back to the original VT on resume, but rather leave things
> alone for a nicer looking suspend and resume sequence.
> 
> v2: make a function so we can handle multiple drivers (Alan)
> v3: use a list to track device requests (Rafael)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  include/linux/pm.h     |    4 ++
>  kernel/power/console.c |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 119 insertions(+)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 03d7bb1..98310eb 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -35,6 +35,10 @@ extern void (*pm_idle)(void);
>  extern void (*pm_power_off)(void);
>  extern void (*pm_power_off_prepare)(void);
>  
> +struct device; /* we have a circular dep with device.h */
> +extern void pm_vt_switch_required(struct device *dev, bool required);
> +extern void pm_vt_switch_unregister(struct device *dev);
> +
>  /*
>   * Device power management
>   */
> diff --git a/kernel/power/console.c b/kernel/power/console.c
> index b1dc456..4871ca9 100644
> --- a/kernel/power/console.c
> +++ b/kernel/power/console.c
> @@ -4,6 +4,7 @@
>   * Originally from swsusp.
>   */
>  
> +#include <linux/console.h>
>  #include <linux/vt_kern.h>
>  #include <linux/kbd_kern.h>
>  #include <linux/vt.h>
> @@ -14,8 +15,119 @@
>  
>  static int orig_fgconsole, orig_kmsg;
>  
> +DEFINE_MUTEX(vt_switch_mutex);
> +
> +struct pm_vt_switch {
> +	struct list_head head;
> +	struct device *dev;
> +	bool required;
> +};
> +
> +LIST_HEAD(pm_vt_switch_list);
> +
> +
> +/**
> + * pm_vt_switch_required - indicate VT switch at suspend requirements
> + * @dev: device
> + * @required: if true, caller needs VT switch at suspend/resume time
> + *
> + * The different console drivers may or may not require VT switches across
> + * suspend/resume, depending on how they handle restoring video state and
> + * what may be running.
> + *
> + * Drivers can indicate support for switchless suspend/resume, which can
> + * save time and flicker, by using this routine and passing 'false' as
> + * the argument.  If any loaded driver needs VT switching, or the
> + * no_console_suspend argument has been passed on the command line, VT
> + * switches will occur.
> + */
> +void pm_vt_switch_required(struct device *dev, bool required)
> +{
> +	struct pm_vt_switch *entry, *tmp;
> +
> +	mutex_lock(&vt_switch_mutex);
> +	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
> +		if (tmp->dev == dev) {
> +			/* already registered, update requirement */
> +			tmp->required = required;
> +			goto out;
> +		}
> +	}
> +
> +	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		goto out;
> +
> +	entry->required = required;
> +	entry->dev = dev;
> +
> +	list_add(&entry->head, &pm_vt_switch_list);
> +out:
> +	mutex_unlock(&vt_switch_mutex);
> +}
> +EXPORT_SYMBOL(pm_vt_switch_required);
> +
> +/**
> + * pm_vt_switch_unregister - stop tracking a device's VT switching needs
> + * @dev: device
> + *
> + * Remove @dev from the vt switch list.
> + */
> +void pm_vt_switch_unregister(struct device *dev)
> +{
> +	struct pm_vt_switch *tmp;
> +
> +	mutex_lock(&vt_switch_mutex);
> +	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
> +		if (tmp->dev == dev) {
> +			list_del(&tmp->head);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&vt_switch_mutex);
> +}
> +
> +/*
> + * There are three cases when a VT switch on suspend/resume are required:
> + *   1) no driver has indicated a requirement one way or another, so preserve
> + *      the old behavior
> + *   2) console suspend is disabled, we want to see debug messages across
> + *      suspend/resume
> + *   3) any registered driver indicates it needs a VT switch
> + *
> + * If none of these conditions is present, meaning we have at least one driver
> + * that doesn't need the switch, and none that do, we can avoid it to make
> + * resume look a little prettier (and suspend too, but that's usually hidden,
> + * e.g. when closing the lid on a laptop).
> + */
> +static bool pm_vt_switch(void)
> +{
> +	struct pm_vt_switch *entry;
> +	bool ret = true;
> +
> +	mutex_lock(&vt_switch_mutex);
> +	if (list_empty(&pm_vt_switch_list))
> +		goto out;
> +
> +	if (!console_suspend_enabled)
> +		goto out;
> +
> +	list_for_each_entry(entry, &pm_vt_switch_list, head) {
> +		if (entry->required)
> +			goto out;
> +	}
> +
> +	ret = false;
> +out:
> +	mutex_unlock(&vt_switch_mutex);
> +	return ret;
> +}
> +
>  int pm_prepare_console(void)
>  {
> +	if (!pm_vt_switch())
> +		return 0;
> +
>  	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
>  	if (orig_fgconsole < 0)
>  		return 1;
> @@ -26,6 +138,9 @@ int pm_prepare_console(void)
>  
>  void pm_restore_console(void)
>  {
> +	if (!pm_vt_switch())
> +		return;
> +
>  	if (orig_fgconsole >= 0) {
>  		vt_move_to_console(orig_fgconsole, 0);
>  		vt_kmsg_redirect(orig_kmsg);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] PM: make VT switching to the suspend console optional v3
  2013-02-05 13:55   ` Jesse Barnes
@ 2013-02-05 22:02     ` Rafael J. Wysocki
  2013-02-06  9:21       ` [Intel-gfx] " Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2013-02-05 22:02 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: linux-kernel, intel-gfx, linux-pm

On Tuesday, February 05, 2013 01:55:44 PM Jesse Barnes wrote:
> On Mon, 04 Feb 2013 21:26:26 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > On Monday, February 04, 2013 01:37:20 PM Jesse Barnes wrote:
> > > KMS drivers can potentially restore the display configuration without
> > > userspace help.  Such drivers can can call a new funciton,
> > > pm_vt_switch_required(false) if they support this feature.  In that
> > > case, the PM layer won't VT switch to the suspend console at suspend
> > > time and then back to the original VT on resume, but rather leave things
> > > alone for a nicer looking suspend and resume sequence.
> > > 
> > > v2: make a function so we can handle multiple drivers (Alan)
> > > v3: use a list to track device requests (Rafael)
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > for all [1-3/3].
> 
> Any chance for an r-b on the PM one at least?  Then Daniel could
> probably push this through drm-intel-next.

Done.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 3/3] drm/i915: support resume without VT switch v2
  2013-02-04 13:37 ` [PATCH 3/3] drm/i915: support resume without VT switch v2 Jesse Barnes
@ 2013-02-06  9:20   ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-02-06  9:20 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: linux-kernel, rjw, intel-gfx, linux-pm

On Mon, Feb 04, 2013 at 01:37:22PM +0000, Jesse Barnes wrote:
> Add support for resuming the suspend configuration at resume time to
> avoid slow and ugly VT switches during suspend and resume.  Also emit a
> hotplug event at resume time to make sure any potential configuration
> changes (monitors coming and going, dock events) are handled properly.
> 
> v2: use new fb flag to indicate we don't need to VT switch
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |   28 +++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   25 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_fb.c      |    3 +++
>  4 files changed, 54 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1172658..7dbaa01 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -483,8 +483,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>  		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
>  
> -		intel_modeset_disable(dev);
> -

You can't just drop this without replacement I think, check git blame for
why it's been added.

>  		drm_irq_uninstall(dev);
>  	}
>  
> @@ -544,6 +542,24 @@ void intel_console_resume(struct work_struct *work)
>  	console_unlock();
>  }
>  
> +static void intel_resume_hotplug(struct drm_device *dev)
> +{
> +	struct drm_mode_config *mode_config = &dev->mode_config;
> +	struct intel_encoder *encoder;
> +
> +	mutex_lock(&mode_config->mutex);
> +	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> +
> +	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
> +		if (encoder->hot_plug)
> +			encoder->hot_plug(encoder);
> +
> +	mutex_unlock(&mode_config->mutex);
> +
> +	/* Just fire off a uevent and let userspace tell us what to do */
> +	drm_helper_hpd_irq_event(dev);
> +}

This should imo be in a separate patch, since afaict we don't bother with
this right now. And I guess we should figure out how this should be done
across drm drivers ...

> +
>  static int __i915_drm_thaw(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -563,8 +579,14 @@ static int __i915_drm_thaw(struct drm_device *dev)
>  		mutex_unlock(&dev->struct_mutex);
>  
>  		intel_modeset_init_hw(dev);
> -		intel_modeset_setup_hw_state(dev, false);

Since this doesn't add any delays (besides a few register reads), but adds
tons of paranoid checks for our state-handling: Why drop it? Dropping this
also explains why you get away with killing intel_modeset_disable without
swimming in a see of WARN_ONs ;-)

>  		drm_irq_install(dev);
> +
> +		/* Resume the modeset for every activated CRTC */
> +		mutex_lock(&dev->mode_config.mutex);
> +		intel_resume_force_mode(dev);
> +		mutex_unlock(&dev->mode_config.mutex);
> +
> +		intel_resume_hotplug(dev);
>  	}
>  
>  	intel_opregion_init(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 12ab3bd..c8b1896 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1680,6 +1680,7 @@ extern void gen6_set_rps(struct drm_device *dev, u8 val);
>  extern void intel_detect_pch(struct drm_device *dev);
>  extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
>  extern int intel_enable_rc6(const struct drm_device *dev);
> +extern int intel_resume_force_mode(struct drm_device *dev);
>  
>  extern bool i915_semaphore_is_enabled(struct drm_device *dev);
>  int i915_reg_read_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index da1ad9c..a127877 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9369,6 +9369,31 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  	drm_mode_config_cleanup(dev);
>  }
>  
> +int intel_resume_force_mode(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_encoder *encoder;
> +	struct drm_encoder_helper_funcs *encoder_funcs;
> +	struct drm_crtc_helper_funcs *crtc_funcs;
> +	int ret;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +
> +		if (!crtc->enabled) {
> +			DRM_ERROR("skipping disabled crtc\n");
> +			continue;
> +		}
> +
> +		ret = intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> +				     crtc->fb);
> +
> +		if (ret == false)
> +			DRM_ERROR("failed to set mode on crtc %p\n", crtc);
> +	}
> +
> +	return 0;
> +}

intel_modeset_setup_hw_state(dev, true); should do exactly this trick, see
the lid notifier in intel_lvds.c. That presumes though that you don't kill
the modeset state, which might require some changes in
intel_modeset_disable.

> +
>  /*
>   * Return which encoder is currently attached for connector.
>   */
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 7b30b5c..d4955b5 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -148,6 +148,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
>  	}
>  	info->screen_size = size;
>  
> +	/* This driver doesn't need a VT switch to restore the mode on resume */
> +	info->skip_vt_switch = true;
> +
>  //	memset(info->screen_base, 0, size);

Follow-up patch to kill this // commented-line?

>  
>  	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] PM: make VT switching to the suspend console optional v3
  2013-02-05 22:02     ` Rafael J. Wysocki
@ 2013-02-06  9:21       ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-02-06  9:21 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Jesse Barnes, intel-gfx, linux-kernel, linux-pm

On Tue, Feb 05, 2013 at 11:02:17PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, February 05, 2013 01:55:44 PM Jesse Barnes wrote:
> > On Mon, 04 Feb 2013 21:26:26 +0100
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > On Monday, February 04, 2013 01:37:20 PM Jesse Barnes wrote:
> > > > KMS drivers can potentially restore the display configuration without
> > > > userspace help.  Such drivers can can call a new funciton,
> > > > pm_vt_switch_required(false) if they support this feature.  In that
> > > > case, the PM layer won't VT switch to the suspend console at suspend
> > > > time and then back to the original VT on resume, but rather leave things
> > > > alone for a nicer looking suspend and resume sequence.
> > > > 
> > > > v2: make a function so we can handle multiple drivers (Alan)
> > > > v3: use a list to track device requests (Rafael)
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > 
> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > 
> > > for all [1-3/3].
> > 
> > Any chance for an r-b on the PM one at least?  Then Daniel could
> > probably push this through drm-intel-next.
> 
> Done.

Thanks, I've merged the first two patches to drm-intel-next, the i915 one
still needs a bit of care imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-02-06  9:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 13:37 [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Jesse Barnes
2013-02-04 13:37 ` [PATCH 2/3] fb: add support for drivers not needing VT switch at suspend/resume time Jesse Barnes
2013-02-04 13:37 ` [PATCH 3/3] drm/i915: support resume without VT switch v2 Jesse Barnes
2013-02-06  9:20   ` [Intel-gfx] " Daniel Vetter
2013-02-04 20:26 ` [PATCH 1/3] PM: make VT switching to the suspend console optional v3 Rafael J. Wysocki
2013-02-05 13:55   ` Jesse Barnes
2013-02-05 22:02     ` Rafael J. Wysocki
2013-02-06  9:21       ` [Intel-gfx] " Daniel Vetter
2013-02-05 22:01 ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).