All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH] drm/i915: disable VGA mem when reenabling the power well
Date: Fri, 29 Nov 2013 11:36:25 -0200	[thread overview]
Message-ID: <1385732185-1697-1-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <20131001181829.GP9395@intel.com>

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

Fixes regression introduced by:
    commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
    Author: Paulo Zanoni <paulo.r.zanoni at intel.com>
    Date:   Wed Jul 3 17:12:13 2013 -0300
        drm/i915: switch disable_power_well default value to 1

The bug I'm seeing can be reproduced with:
- Boot your Haswell machine
- Make sure the power well gets disabled, then enabled. You can check
  this by seeing the messages print by hsw_set_power_well
- Stop your display manager
- echo 0 > /sys/class/vtconsole/vtcon1/bind

I can easily reproduce this by blacklising snd_hda_intel and booting
with eDP+HDMI.

If you do this and then look at dmesg, you'll see we're printing
infinite "Unclaimed register" messages. This is happening because
we're stuck on an infinite loop inside console_unlock(), which is
calling many functions from vgacon.c. And the code that's triggering
the error messages is from vgacon_set_cursor_size().

For some reason, what fixes the problem is the code that clears
VGA_MSR_MEM_EN from VGA_MSR_WRITE. If you add some debug messages,
you'll see that we're reading a value of 0 and writing 0 back to it. I
really don't know why this fixes the problem, but I am sure that if
you remove the code line that writes 0 back the bug won't stop. I
suspect this is probably some problem with how the hardware gets
reinitialized when we reenable the power well.

Also notice that function i915_disable_vga_mem already existed and was
removed on commit 6e1b4fdad5157bb9e88777d525704aba24389bee, I just
brought it back. Notice that the intel_drv.h declaration was still
there too.

I know some people will probably request to deeply investigate why
exactly vgacon is doing the wrong thing, and ask me to replace the
"for (;;)" that exists inside console_unlock() with something else,
but my goal here is to just fix the regression. I don't think it's
worth spending that much time on vgacon code. And this is our
regression anyway.

Notice that this is just a partial fix to fd.o #67813. This fixes the
case where the power well is already enabled when we unbind, not when
it's disabled when we unbind.

V2: - Rebase (first version was sent in September).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_pm.c      |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45a87d1..bf22bea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11064,6 +11064,18 @@ void i915_redisable_vga(struct drm_device *dev)
 	}
 }
 
+void i915_disable_vga_mem(struct drm_device *dev)
+{
+	if (HAS_PCH_SPLIT(dev)) {
+		vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
+		outb(inb(VGA_MSR_READ) & ~VGA_MSR_MEM_EN, VGA_MSR_WRITE);
+		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
+						   VGA_RSRC_NORMAL_IO |
+						   VGA_RSRC_NORMAL_MEM);
+		vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
+	}
+}
+
 static void intel_modeset_readout_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e6d98fe..7ebe1bd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5703,6 +5703,8 @@ static void hsw_set_power_well(struct drm_device *dev,
 			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
 				      HSW_PWR_WELL_STATE_ENABLED), 20))
 				DRM_ERROR("Timeout enabling power well\n");
+
+			i915_disable_vga_mem(dev);
 		}
 
 		if (IS_BROADWELL(dev)) {
-- 
1.8.3.1

  reply	other threads:[~2013-11-29 13:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-26 23:05 [PATCH 0/5] module_reload fixes Paulo Zanoni
2013-09-26 23:05 ` [PATCH 1/5] drm/i915: redisable VGA when we disable the power well Paulo Zanoni
2013-10-01 13:47   ` Ville Syrjälä
2013-10-01 18:18     ` Ville Syrjälä
2013-11-29 13:36       ` Paulo Zanoni [this message]
2013-12-11 17:44         ` [PATCH] drm/i915: disable VGA mem when reenabling " Paulo Zanoni
2013-12-11 18:39           ` Daniel Vetter
2013-12-11 18:50             ` Paulo Zanoni
2013-12-11 19:02               ` Daniel Vetter
2013-09-26 23:05 ` [PATCH 2/5] drm/i915: destroy connector sysfs files earlier Paulo Zanoni
2013-09-30 21:10   ` Daniel Vetter
2013-09-26 23:06 ` [PATCH 3/5] tty/vt: add con_bind and con_unbind functions Paulo Zanoni
2013-10-01 13:50   ` Ville Syrjälä
2013-10-08 21:12     ` Paulo Zanoni
2013-10-09  8:10       ` Ville Syrjälä
2013-09-26 23:06 ` [PATCH 4/5] console/fbcon: implement con_bind and con_unbind Paulo Zanoni
2013-09-26 23:06 ` [PATCH 5/5] drm/i915: put/get the power well at the FB bind/unbind functions Paulo Zanoni

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=1385732185-1697-1-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.