public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Knut Petersen <Knut_Petersen@t-online.de>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH 3/4] drm/i915: enable VT switchless resume v3
Date: Mon, 21 Apr 2014 18:37:31 +0200	[thread overview]
Message-ID: <5355494B.5040503@t-online.de> (raw)
In-Reply-To: <1364315146-20542-3-git-send-email-jbarnes@virtuousgeek.org>

[-- Attachment #1: Type: text/plain, Size: 4540 bytes --]

On 26.03.2013 17:25, Jesse Barnes wrote:
> With the other bits in place, we can do this safely.
>
> v2: disable backlight on suspend to prevent premature enablement on resume
> v3: disable CRTCs on suspend to allow RTD3 (Kristen)
>
> Signed-off-by: Jesse Barnes<jbarnes@virtuousgeek.org>

I might be a bit late as your patch has long be accepted, but it introduced
a regression. Steps to reproduce the problem:

boot linux,
startx,
suspend (s3),
resume,
terminate xserver,
startx,
mplayer -fs foo.video

Prior to your patch, everything worked fine. With your patch, the xserver starts much slowler,
the monitor is switched off and on again, the kde splash screen is displayed in the upper
left corner (1024x768) on my 1280x1024 monitor. mplayer -fs plays a video correctly, but
the (normally) black bars above and below the video contain garbage for x > ~1024 / y > ~768

Reproducibility: 100% on my system.

Hardware: AOpen i915GMm-hfs, Pentium-M Dothan, Eizo Flexscan L557 monitor

00:02.0 VGA compatible controller: Intel Corporation Mobile 915GM/GMS/910GML Express Graphics Controller (rev 04) (prog-if 00 [VGA controller])
         Subsystem: AOPEN Inc. Device 0554
         Flags: bus master, fast devsel, latency 0, IRQ 16
         Memory at d5300000 (32-bit, non-prefetchable) [size=512K]
         I/O ports at e400 [size=8]
         Memory at c0000000 (32-bit, prefetchable) [size=256M]
         Memory at d5400000 (32-bit, non-prefetchable) [size=256K]
         Expansion ROM at <unassigned> [disabled]
         Capabilities: [d0] Power Management version 2
         Kernel driver in use: i915

00:02.1 Display controller: Intel Corporation Mobile 915GM/GMS/910GML Express Graphics Controller (rev 04)
         Subsystem: AOPEN Inc. Device 0554
         Flags: bus master, fast devsel, latency 0
         Memory at d5380000 (32-bit, non-prefetchable) [size=512K]
         Capabilities: [d0] Power Management version 2

Software: openSuSE 13.1, current git xorg.

last good mainline linux kernel: 3.9.y
kernels affected: 3.10.y, 3.11.y, 3.12.y, 3.13.y, 3.14.y, 3.15-rc2

Attached is a patch against 3.14.1. Directly reverting 24576d2 is impossible as a helper
function was eliminated, a file was renamed, and other patches touched the code, but
more or less my patch is simply a functional revert of yours. At least here it cures the problem ...

cu,
  Knut


> ---
>   drivers/gpu/drm/i915/i915_drv.c |   14 +++++++++++---
>   drivers/gpu/drm/i915/intel_fb.c |    3 +++
>   2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6c4b13c..bf57e1c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -458,6 +458,7 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>   static int i915_drm_freeze(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
>   
>   	/* ignore lid events during suspend */
>   	mutex_lock(&dev_priv->modeset_restore_lock);
> @@ -481,10 +482,14 @@ 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);
>   		dev_priv->enable_hotplug_processing = false;
> +		/*
> +		 * Disable CRTCs directly since we want to preserve sw state
> +		 * for _thaw.
> +		 */
> +		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> +			dev_priv->display.crtc_disable(crtc);
>   	}
>   
>   	i915_save_state(dev);
> @@ -562,7 +567,10 @@ static int __i915_drm_thaw(struct drm_device *dev)
>   		drm_irq_install(dev);
>   
>   		intel_modeset_init_hw(dev);
> -		intel_modeset_setup_hw_state(dev, false);
> +
> +		drm_modeset_lock_all(dev);
> +		intel_modeset_setup_hw_state(dev, true);
> +		drm_modeset_unlock_all(dev);
>   
>   		/*
>   		 * ... but also need to make sure that hotplug processing
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index f203418..8d81c929 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -150,6 +150,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
>   	}
>   	info->screen_size = size;
>   
> +	/* This driver doesn't need a VT switch to restore the mode on resume */
> +	info->skip_vt_switch = true;
> +
>   	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
>   	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
>   


[-- Attachment #2: 0001-i915-Fix-S3-resume-regression-introduced-in-24576d2.patch --]
[-- Type: text/x-patch, Size: 3541 bytes --]

>From fc936457f7f7f178184b5decf538ec3280adb7df Mon Sep 17 00:00:00 2001
From: Knut Petersen <Knut_Petersen@t-online.de>
Date: Mon, 21 Apr 2014 17:37:10 +0200
Subject: [PATCH] [i915] Fix S3 resume regression introduced in 24576d2

Signed-off-by: Knut Petersen <Knut_Petersen@t-online.de>
---
 drivers/gpu/drm/i915/i915_drv.c      | 19 ++++++-------------
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_fbdev.c   |  3 ---
 4 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ec7bb0f..fb1354a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -527,16 +527,13 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
 
+		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+			if (crtc->enabled)
+				intel_crtc_disable(crtc);
+		}
+
 		drm_irq_uninstall(dev);
 		dev_priv->enable_hotplug_processing = false;
-		/*
-		 * Disable CRTCs directly since we want to preserve sw state
-		 * for _thaw.
-		 */
-		mutex_lock(&dev->mode_config.mutex);
-		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-			dev_priv->display.crtc_disable(crtc);
-		mutex_unlock(&dev->mode_config.mutex);
 
 		intel_modeset_suspend_hw(dev);
 	}
@@ -648,11 +645,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		drm_irq_install(dev);
 
 		intel_modeset_init_hw(dev);
-
-		drm_modeset_lock_all(dev);
-		drm_mode_config_reset(dev);
-		intel_modeset_setup_hw_state(dev, true);
-		drm_modeset_unlock_all(dev);
+		intel_modeset_setup_hw_state(dev, false);
 
 		/*
 		 * ... but also need to make sure that hotplug processing
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9b8a7c7..825e888 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4346,7 +4346,7 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
 	intel_crtc_update_sarea(crtc, enable);
 }
 
-static void intel_crtc_disable(struct drm_crtc *crtc)
+void intel_crtc_disable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_connector *connector;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fbfaaba..1cc01fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -634,6 +634,7 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 void intel_mark_idle(struct drm_device *dev);
 void intel_crtc_restore_mode(struct drm_crtc *crtc);
 void intel_crtc_update_dpms(struct drm_crtc *crtc);
+void intel_crtc_disable(struct drm_crtc *crtc);
 void intel_encoder_destroy(struct drm_encoder *encoder);
 void intel_connector_dpms(struct drm_connector *, int mode);
 bool intel_connector_get_hw_state(struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 39eac99..10d70b8 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -184,9 +184,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	}
 	info->screen_size = size;
 
-	/* This driver doesn't need a VT switch to restore the mode on resume */
-	info->skip_vt_switch = true;
-
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
 	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
 
-- 
1.8.4.5


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

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

  parent reply	other threads:[~2014-04-21 16:37 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-26 16:25 [PATCH 1/4] drm/i915: add sprite restore function v3 Jesse Barnes
2013-03-26 16:25 ` [PATCH 2/4] drm/i915: restore cursor and sprite state when forcing a config restore Jesse Barnes
2013-03-26 16:40   ` Rodrigo Vivi
2013-03-26 16:52   ` Daniel Vetter
2013-03-26 17:07   ` [PATCH 1/2] drm/i915: enable VT switchless resume v3 Jesse Barnes
2013-03-26 17:07     ` [PATCH 2/2] drm/i915: emit a hotplug event on resume Jesse Barnes
2013-03-26 20:25   ` drm/i915: restore cursor and sprite state when forcing a config restore v2 Jesse Barnes
2013-03-26 20:46     ` Daniel Vetter
2013-03-26 20:57       ` Jesse Barnes
2013-03-26 16:25 ` [PATCH 3/4] drm/i915: enable VT switchless resume v3 Jesse Barnes
2013-03-26 16:42   ` Rodrigo Vivi
2013-04-03  9:15   ` Daniel Vetter
2013-04-03 10:54     ` Chris Wilson
2013-04-03 15:13       ` Jesse Barnes
2014-04-21 16:37   ` Knut Petersen [this message]
2014-05-16 22:20     ` Jesse Barnes
2014-05-16 22:28       ` Daniel Vetter
2014-05-16 22:38       ` Chris Wilson
2014-05-16 22:42         ` Chris Wilson
2014-05-20 19:53           ` Jesse Barnes
2014-05-20 19:58             ` Jesse Barnes
2014-05-20 20:15               ` Daniel Vetter
2014-05-20 21:10                 ` Jesse Barnes
2014-05-20 21:18                   ` Jesse Barnes
2014-05-20 21:27                     ` Jesse Barnes
2013-03-26 16:25 ` [PATCH 4/4] drm/i915: emit a hotplug event on resume Jesse Barnes
2013-03-26 16:43   ` Rodrigo Vivi
2013-03-26 16:38 ` [PATCH 1/4] drm/i915: add sprite restore function v3 Rodrigo Vivi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5355494B.5040503@t-online.de \
    --to=knut_petersen@t-online.de \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jbarnes@virtuousgeek.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox