diff mbox

drm/i915: Sanitize the output registers after resume

Message ID 1303929788-24078-2-git-send-email-seth.forshee@canonical.com
State New
Headers show

Commit Message

Seth Forshee April 27, 2011, 6:43 p.m. UTC
From: Chris Wilson <chris@chris-wilson.co.uk>

BugLink: http://bugs.launchpad.net/bugs/745304

Similar to booting, we need to inspect the state left by the BIOS and
remove any conflicting bits before we take over. The example reported by
Seth Forshee is very similar to the bug we encountered with the state left
by grub2, that the crtc pipe<->planning mapping was reversed from our
expectations and so we failed to turn off the outputs when booting or,
in this case, resuming. This may be in fact the same bug, but triggered
at resume time.

This patch rearranges the code we already have to clear up the
conflicting state upon init and calls it from reset (which is called
after we have lost control of the hardware, i.e. along both the boot and
resume paths) instead.

Reported-and-tested-by: Seth Forshee <seth.forshee@canonical.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=35796
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
(cherry picked from commit f6e5b1603b8bb7131b6778d0d4e2e5dda120a379)

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/intel_display.c |   68 ++++++++++++++++++----------------
 1 files changed, 36 insertions(+), 32 deletions(-)

Comments

Brad Figg April 27, 2011, 7:10 p.m. UTC | #1
On 04/27/2011 11:43 AM, Seth Forshee wrote:
> From: Chris Wilson<chris@chris-wilson.co.uk>
>
> BugLink: http://bugs.launchpad.net/bugs/745304
>
> Similar to booting, we need to inspect the state left by the BIOS and
> remove any conflicting bits before we take over. The example reported by
> Seth Forshee is very similar to the bug we encountered with the state left
> by grub2, that the crtc pipe<->planning mapping was reversed from our
> expectations and so we failed to turn off the outputs when booting or,
> in this case, resuming. This may be in fact the same bug, but triggered
> at resume time.
>
> This patch rearranges the code we already have to clear up the
> conflicting state upon init and calls it from reset (which is called
> after we have lost control of the hardware, i.e. along both the boot and
> resume paths) instead.
>
> Reported-and-tested-by: Seth Forshee<seth.forshee@canonical.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=35796
> Signed-off-by: Chris Wilson<chris@chris-wilson.co.uk>
> Cc: stable@kernel.org
> Reviewed-by: Keith Packard<keithp@keithp.com>
> Signed-off-by: Keith Packard<keithp@keithp.com>
> (cherry picked from commit f6e5b1603b8bb7131b6778d0d4e2e5dda120a379)
>
> Signed-off-by: Seth Forshee<seth.forshee@canonical.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c |   68 ++++++++++++++++++----------------
>   1 files changed, 36 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 49fb54f..ecf8f94 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5630,36 +5630,6 @@ cleanup_work:
>   	return ret;
>   }
>
> -static void intel_crtc_reset(struct drm_crtc *crtc)
> -{
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> -	/* Reset flags back to the 'unknown' status so that they
> -	 * will be correctly set on the initial modeset.
> -	 */
> -	intel_crtc->dpms_mode = -1;
> -}
> -
> -static struct drm_crtc_helper_funcs intel_helper_funcs = {
> -	.dpms = intel_crtc_dpms,
> -	.mode_fixup = intel_crtc_mode_fixup,
> -	.mode_set = intel_crtc_mode_set,
> -	.mode_set_base = intel_pipe_set_base,
> -	.mode_set_base_atomic = intel_pipe_set_base_atomic,
> -	.load_lut = intel_crtc_load_lut,
> -	.disable = intel_crtc_disable,
> -};
> -
> -static const struct drm_crtc_funcs intel_crtc_funcs = {
> -	.reset = intel_crtc_reset,
> -	.cursor_set = intel_crtc_cursor_set,
> -	.cursor_move = intel_crtc_cursor_move,
> -	.gamma_set = intel_crtc_gamma_set,
> -	.set_config = drm_crtc_helper_set_config,
> -	.destroy = intel_crtc_destroy,
> -	.page_flip = intel_crtc_page_flip,
> -};
> -
>   static void intel_sanitize_modesetting(struct drm_device *dev,
>   				       int pipe, int plane)
>   {
> @@ -5710,6 +5680,42 @@ static void intel_sanitize_modesetting(struct drm_device *dev,
>   	}
>   }
>
> +static void intel_crtc_reset(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	/* Reset flags back to the 'unknown' status so that they
> +	 * will be correctly set on the initial modeset.
> +	 */
> +	intel_crtc->dpms_mode = -1;
> +
> +	/* We need to fix up any BIOS configuration that conflicts with
> +	 * our expectations.
> +	 */
> +	intel_sanitize_modesetting(dev, intel_crtc->pipe, intel_crtc->plane);
> +}
> +
> +static struct drm_crtc_helper_funcs intel_helper_funcs = {
> +	.dpms = intel_crtc_dpms,
> +	.mode_fixup = intel_crtc_mode_fixup,
> +	.mode_set = intel_crtc_mode_set,
> +	.mode_set_base = intel_pipe_set_base,
> +	.mode_set_base_atomic = intel_pipe_set_base_atomic,
> +	.load_lut = intel_crtc_load_lut,
> +	.disable = intel_crtc_disable,
> +};
> +
> +static const struct drm_crtc_funcs intel_crtc_funcs = {
> +	.reset = intel_crtc_reset,
> +	.cursor_set = intel_crtc_cursor_set,
> +	.cursor_move = intel_crtc_cursor_move,
> +	.gamma_set = intel_crtc_gamma_set,
> +	.set_config = drm_crtc_helper_set_config,
> +	.destroy = intel_crtc_destroy,
> +	.page_flip = intel_crtc_page_flip,
> +};
> +
>   static void intel_crtc_init(struct drm_device *dev, int pipe)
>   {
>   	drm_i915_private_t *dev_priv = dev->dev_private;
> @@ -5759,8 +5765,6 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>
>   	setup_timer(&intel_crtc->idle_timer, intel_crtc_idle_timer,
>   		    (unsigned long)intel_crtc);
> -
> -	intel_sanitize_modesetting(dev, intel_crtc->pipe, intel_crtc->plane);
>   }
>
>   int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,

Looks good. Seth mentioned that this also fixes a regression from Maverick.

Acked-by: Brad Figg <brad.figg@canonical.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 49fb54f..ecf8f94 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5630,36 +5630,6 @@  cleanup_work:
 	return ret;
 }
 
-static void intel_crtc_reset(struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	/* Reset flags back to the 'unknown' status so that they
-	 * will be correctly set on the initial modeset.
-	 */
-	intel_crtc->dpms_mode = -1;
-}
-
-static struct drm_crtc_helper_funcs intel_helper_funcs = {
-	.dpms = intel_crtc_dpms,
-	.mode_fixup = intel_crtc_mode_fixup,
-	.mode_set = intel_crtc_mode_set,
-	.mode_set_base = intel_pipe_set_base,
-	.mode_set_base_atomic = intel_pipe_set_base_atomic,
-	.load_lut = intel_crtc_load_lut,
-	.disable = intel_crtc_disable,
-};
-
-static const struct drm_crtc_funcs intel_crtc_funcs = {
-	.reset = intel_crtc_reset,
-	.cursor_set = intel_crtc_cursor_set,
-	.cursor_move = intel_crtc_cursor_move,
-	.gamma_set = intel_crtc_gamma_set,
-	.set_config = drm_crtc_helper_set_config,
-	.destroy = intel_crtc_destroy,
-	.page_flip = intel_crtc_page_flip,
-};
-
 static void intel_sanitize_modesetting(struct drm_device *dev,
 				       int pipe, int plane)
 {
@@ -5710,6 +5680,42 @@  static void intel_sanitize_modesetting(struct drm_device *dev,
 	}
 }
 
+static void intel_crtc_reset(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	/* Reset flags back to the 'unknown' status so that they
+	 * will be correctly set on the initial modeset.
+	 */
+	intel_crtc->dpms_mode = -1;
+
+	/* We need to fix up any BIOS configuration that conflicts with
+	 * our expectations.
+	 */
+	intel_sanitize_modesetting(dev, intel_crtc->pipe, intel_crtc->plane);
+}
+
+static struct drm_crtc_helper_funcs intel_helper_funcs = {
+	.dpms = intel_crtc_dpms,
+	.mode_fixup = intel_crtc_mode_fixup,
+	.mode_set = intel_crtc_mode_set,
+	.mode_set_base = intel_pipe_set_base,
+	.mode_set_base_atomic = intel_pipe_set_base_atomic,
+	.load_lut = intel_crtc_load_lut,
+	.disable = intel_crtc_disable,
+};
+
+static const struct drm_crtc_funcs intel_crtc_funcs = {
+	.reset = intel_crtc_reset,
+	.cursor_set = intel_crtc_cursor_set,
+	.cursor_move = intel_crtc_cursor_move,
+	.gamma_set = intel_crtc_gamma_set,
+	.set_config = drm_crtc_helper_set_config,
+	.destroy = intel_crtc_destroy,
+	.page_flip = intel_crtc_page_flip,
+};
+
 static void intel_crtc_init(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -5759,8 +5765,6 @@  static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	setup_timer(&intel_crtc->idle_timer, intel_crtc_idle_timer,
 		    (unsigned long)intel_crtc);
-
-	intel_sanitize_modesetting(dev, intel_crtc->pipe, intel_crtc->plane);
 }
 
 int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,