[Intel-gfx] [PATCH v3] drm/i915: Fix VGA handling using stop_machine() or mmio

ville.syrjala at linux.intel.com ville.syrjala at linux.intel.com
Wed Oct 2 15:42:55 CEST 2013


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

We have several problems with out VGA handling:
- We try to use the GMCH control VGA disable bit even though it may
  be locked
- If we manage to disable VGA throuh GMCH control, we're no longer
  able to correctly disable the VGA plane
- Taking part in the VGA arbitration is too expensive for X [1]

So let's treat the GMCH control VGA disable bit as read-only and leave
it for the BIOS to set, as it was intended. To disable VGA we will use
the VGA misc register, and to disable VGA IO we will disable IO space
completely via the PCI command register.

But we still need VGA register access during resume (and possibly during
lid event on insane BIOSen) to disable the VGA plane. Also we need to
re-disable VGA memory decode via the VGA misc register on resume.

Luckily up to gen4, VGA registers can be accessed through MMIO.
Unfortunately from gen5 onwards only the legacy VGA IO port range
works. So on gen5+ we still need IO space to be enabled during those
few special moments when we need to access VGA registers.

We still want to opt out of VGA arbitration on gen5+, so we have keep
IO space disabled most of the time. And when we do need to poke at VGA
registers, we enable IO space briefly while no one is looking. To
guarantee that no one is looking we will use stop_machine().

[1] http://lists.x.org/archives/xorg-devel/2013-September/037763.html

v2: Use SNB_GMCH_TRL on SNB+
    Use port IO instead of MMIO on CTG/ELK
    Add WaEnableVGAAccessThroughIOPort comment
    Fix the max number of devices on the bus limit
v3: Allocate the temp space dynamically
    Print some errors if we fail to execute the vga "op" due to alloc failure

Cc: Alex Williamson <alex.williamson at redhat.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Dave Airlie <airlied at redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |  39 +----
 drivers/gpu/drm/i915/i915_suspend.c  |   4 +-
 drivers/gpu/drm/i915/intel_display.c | 318 ++++++++++++++++++++++++++++++-----
 3 files changed, 287 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d35de1b..59a2048 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1225,19 +1225,6 @@ intel_teardown_mchbar(struct drm_device *dev)
 		release_resource(&dev_priv->mch_res);
 }
 
-/* true = enable decode, false = disable decoder */
-static unsigned int i915_vga_set_decode(void *cookie, bool state)
-{
-	struct drm_device *dev = cookie;
-
-	intel_modeset_vga_set_state(dev, state);
-	if (state)
-		return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM |
-		       VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
-	else
-		return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
-}
-
 static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
@@ -1283,25 +1270,11 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		DRM_INFO("failed to find VBIOS tables\n");
 
-	/* If we have > 1 VGA cards, then we need to arbitrate access
-	 * to the common VGA resources.
-	 *
-	 * If we are a secondary display controller (!PCI_DISPLAY_CLASS_VGA),
-	 * then we do not take part in VGA arbitration and the
-	 * vga_client_register() fails with -ENODEV.
-	 */
-	if (!HAS_PCH_SPLIT(dev)) {
-		ret = vga_client_register(dev->pdev, dev, NULL,
-					  i915_vga_set_decode);
-		if (ret && ret != -ENODEV)
-			goto out;
-	}
-
 	intel_register_dsm_handler();
 
 	ret = vga_switcheroo_register_client(dev->pdev, &i915_switcheroo_ops, false);
 	if (ret)
-		goto cleanup_vga_client;
+		goto out;
 
 	/* Initialise stolen first so that we may reserve preallocated
 	 * objects for the BIOS to KMS transition.
@@ -1316,7 +1289,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	intel_init_power_well(dev);
 
-	/* Keep VGA alive until i915_disable_vga_mem() */
+	/* Keep VGA alive until intel_modeset_vga_set_state() */
 	intel_display_power_get(dev, POWER_DOMAIN_VGA);
 
 	/* Important: The output setup functions called by modeset_init need
@@ -1359,10 +1332,13 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	intel_fbdev_initial_config(dev);
 
 	/*
+	 * Disable VGA IO and memory, and
+	 * tell the arbiter to ignore us.
+	 *
 	 * Must do this after fbcon init so that
 	 * vgacon_save_screen() works during the handover.
 	 */
-	i915_disable_vga_mem(dev);
+	intel_modeset_vga_set_state(dev, false);
 	intel_display_power_put(dev, POWER_DOMAIN_VGA);
 
 	/* Only enable hotplug handling once the fbdev is fully set up. */
@@ -1386,8 +1362,6 @@ cleanup_gem_stolen:
 	i915_gem_cleanup_stolen(dev);
 cleanup_vga_switcheroo:
 	vga_switcheroo_unregister_client(dev->pdev);
-cleanup_vga_client:
-	vga_client_register(dev->pdev, NULL, NULL, NULL);
 out:
 	return ret;
 }
@@ -1758,7 +1732,6 @@ int i915_driver_unload(struct drm_device *dev)
 		}
 
 		vga_switcheroo_unregister_client(dev->pdev);
-		vga_client_register(dev->pdev, NULL, NULL, NULL);
 	}
 
 	/* Free error state after interrupts are fully disabled. */
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 3538370..cc0d459 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -331,8 +331,10 @@ static void i915_restore_display(struct drm_device *dev)
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_restore_vga(dev);
-	else
+	else {
 		i915_redisable_vga(dev);
+		intel_modeset_vga_set_state(dev, false);
+	}
 }
 
 int i915_save_state(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 76870f0..84e5a5a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -40,6 +40,8 @@
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <linux/dma_remapping.h>
+#include <linux/stop_machine.h>
+#include "../../../pci/pci.h"
 
 static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
@@ -10232,51 +10234,253 @@ static void intel_init_quirks(struct drm_device *dev)
 	}
 }
 
-/* Disable the VGA plane that we never use */
-static void i915_disable_vga(struct drm_device *dev)
+enum vga_op {
+	VGA_OP_MEM_DISABLE,
+	VGA_OP_MEM_ENABLE,
+	VGA_OP_SCREEN_OFF,
+};
+
+struct i915_vga_op {
+	enum vga_op op;
+	struct drm_device *dev;
+	u16 *cmd, *ctl;
+	int cmd_num, ctl_num;
+};
+
+static int i915_vga_temp_alloc(struct i915_vga_op *op)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u8 sr1;
-	u32 vga_reg = i915_vgacntrl_reg(dev);
+	struct drm_device *dev = op->dev;
+	struct pci_dev *pdev, *self = dev->pdev;
+	struct pci_bus *bus;
 
-	vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
-	outb(SR01, VGA_SR_INDEX);
-	sr1 = inb(VGA_SR_DATA);
-	outb(sr1 | 1<<5, VGA_SR_DATA);
-	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
-	udelay(300);
+	list_for_each_entry(bus, &self->bus->children, node)
+		op->ctl_num++;
 
-	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
-	POSTING_READ(vga_reg);
+	list_for_each_entry(pdev, &self->bus->devices, bus_list)
+		if (pdev != self)
+			op->cmd_num++;
+
+	op->cmd = kmalloc_array(op->cmd_num, sizeof(op->cmd[0]), GFP_KERNEL);
+	if (!op->cmd)
+		return -ENOMEM;
+
+	op->ctl = kmalloc_array(op->ctl_num, sizeof(op->ctl[0]), GFP_KERNEL);
+	if (!op->ctl) {
+		kfree(op->cmd);
+		return -ENOMEM;
+	}
+
+	return 0;
 }
 
-static void i915_enable_vga_mem(struct drm_device *dev)
+static void i915_vga_temp_free(struct i915_vga_op *op)
 {
-	/* Enable VGA memory on Intel HD */
-	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_LEGACY_MEM |
-						   VGA_RSRC_NORMAL_IO |
-						   VGA_RSRC_NORMAL_MEM);
-		vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
+	kfree(op->cmd);
+	kfree(op->ctl);
+}
+
+/*
+ * Disable IO decode and VGA bridge forwarding
+ * for everyone else on the same bus.
+ */
+static void i915_vga_bus_disable(struct i915_vga_op *op)
+{
+	struct drm_device *dev = op->dev;
+	struct pci_dev *pdev, *self = dev->pdev;
+	struct pci_bus *bus;
+	int i;
+
+	i = 0;
+	list_for_each_entry(bus, &self->bus->children, node) {
+		if (WARN_ON(i >= op->ctl_num))
+			break;
+
+		pci_read_config_word(bus->self, PCI_BRIDGE_CONTROL,
+				     &op->ctl[i]);
+		pci_write_config_word(bus->self, PCI_BRIDGE_CONTROL,
+				      op->ctl[i] & ~PCI_BRIDGE_CTL_VGA);
+		i++;
+	}
+
+	i = 0;
+	list_for_each_entry(pdev, &self->bus->devices, bus_list) {
+		if (pdev == self)
+			continue;
+
+		if (WARN_ON(i >= op->cmd_num))
+			break;
+
+		pci_read_config_word(pdev, PCI_COMMAND,
+				     &op->cmd[i]);
+		pci_write_config_word(pdev, PCI_COMMAND,
+				      op->cmd[i] & ~PCI_COMMAND_IO);
+		i++;
 	}
 }
 
-void i915_disable_vga_mem(struct drm_device *dev)
+/*
+ * Restore IO decode and VGA bridge forwarding
+ * for everyone else on the same bus.
+ */
+static void i915_vga_bus_restore(struct i915_vga_op *op)
 {
-	/* Disable VGA memory on Intel HD */
-	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);
+	struct drm_device *dev = op->dev;
+	struct pci_dev *pdev, *self = dev->pdev;
+	struct pci_bus *bus;
+	int i;
+
+	i = 0;
+	list_for_each_entry(pdev, &self->bus->devices, bus_list) {
+		if (pdev == self)
+			continue;
+
+		if (i >= op->cmd_num)
+			break;
+
+		pci_write_config_word(pdev, PCI_COMMAND,
+				      op->cmd[i]);
+		i++;
+	}
+
+	i = 0;
+	list_for_each_entry(bus, &self->bus->children, node) {
+		if (i >= op->ctl_num)
+			break;
+
+		pci_write_config_word(bus->self, PCI_BRIDGE_CONTROL,
+				      op->ctl[i]);
+		i++;
 	}
 }
 
+/*
+ * Hide our VGA IO access from the rest of the system
+ * using stop_machine().
+ *
+ * Note that we assume that the IGD is on the root bus.
+ */
+static int i915_vga_stop_machine_cb(void *data)
+{
+	struct i915_vga_op *op = data;
+	struct drm_device *dev = op->dev;
+	u16 cmd;
+	u8 tmp;
+
+	i915_vga_bus_disable(op);
+
+	pci_read_config_word(dev->pdev, PCI_COMMAND, &cmd);
+	pci_write_config_word(dev->pdev, PCI_COMMAND, cmd | PCI_COMMAND_IO);
+
+	switch (op->op) {
+	case VGA_OP_MEM_DISABLE:
+		tmp = inb(VGA_MSR_READ);
+		tmp &= ~VGA_MSR_MEM_EN;
+		outb(tmp, VGA_MSR_WRITE);
+		break;
+	case VGA_OP_MEM_ENABLE:
+		tmp = inb(VGA_MSR_READ);
+		tmp |= VGA_MSR_MEM_EN;
+		outb(tmp, VGA_MSR_WRITE);
+		break;
+	case VGA_OP_SCREEN_OFF:
+		outb(SR01, VGA_SR_INDEX);
+		tmp = inb(VGA_SR_DATA);
+		tmp |= 1 << 5;
+		outb(tmp, VGA_SR_DATA);
+		break;
+	}
+
+	pci_write_config_word(dev->pdev, PCI_COMMAND, cmd);
+
+	i915_vga_bus_restore(op);
+
+	return 0;
+}
+
+/* MMIO based VGA register access, pre-Gen5 only */
+static void i915_vga_execute_mmio(struct drm_device *dev, enum vga_op op)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u8 tmp;
+
+	switch (op) {
+	case VGA_OP_MEM_DISABLE:
+		tmp = I915_READ8(VGA_MSR_READ);
+		tmp &= ~VGA_MSR_MEM_EN;
+		I915_WRITE8(VGA_MSR_WRITE, tmp);
+		break;
+	case VGA_OP_MEM_ENABLE:
+		tmp = I915_READ8(VGA_MSR_READ);
+		tmp |= VGA_MSR_MEM_EN;
+		I915_WRITE8(VGA_MSR_WRITE, tmp);
+		break;
+	case VGA_OP_SCREEN_OFF:
+		I915_WRITE8(VGA_SR_INDEX, SR01);
+		tmp = I915_READ8(VGA_SR_DATA);
+		tmp |= 1 << 5;
+		I915_WRITE8(VGA_SR_DATA, tmp);
+		break;
+	}
+}
+
+static int i915_vga_execute(struct drm_device *dev, enum vga_op op)
+{
+	int ret = 0;
+
+	/* WaEnableVGAAccessThroughIOPort:ctg,elk,ilk,snb,ivb,vlv,hsw */
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+		struct i915_vga_op vga_op = {
+			.op = op,
+			.dev = dev,
+		};
+
+		/*
+		 * Protect against hot(un)plug during/after
+		 * counting how much temp space we need.
+		 */
+		down_read(&pci_bus_sem);
+
+		ret = i915_vga_temp_alloc(&vga_op);
+
+		if (ret == 0) {
+			stop_machine(i915_vga_stop_machine_cb, &vga_op, NULL);
+
+			i915_vga_temp_free(&vga_op);
+		}
+
+		up_read(&pci_bus_sem);
+	} else {
+		i915_vga_execute_mmio(dev, op);
+	}
+
+	return ret;
+}
+
+/* Disable the VGA plane that we never use */
+static void i915_disable_vga(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 vga_reg = i915_vgacntrl_reg(dev);
+	u16 gmch_ctrl;
+
+	/* already disabled? */
+	if (I915_READ(vga_reg) & VGA_DISP_DISABLE)
+		return;
+
+	pci_read_config_word(dev_priv->bridge_dev, INTEL_INFO(dev)->gen >= 6 ?
+			     SNB_GMCH_CTRL : INTEL_GMCH_CTRL, &gmch_ctrl);
+	if (gmch_ctrl & INTEL_GMCH_VGA_DISABLE)
+		DRM_ERROR("VGA plane enabled while VGA disabled via GMCH control\n");
+
+	if (i915_vga_execute(dev, VGA_OP_SCREEN_OFF))
+		DRM_ERROR("Failed to disable VGA plane correctly\n");
+
+	udelay(300);
+
+	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
+	POSTING_READ(vga_reg);
+}
+
 void intel_modeset_init_hw(struct drm_device *dev)
 {
 	intel_prepare_ddi(dev);
@@ -10553,7 +10757,6 @@ void i915_redisable_vga(struct drm_device *dev)
 	if (I915_READ(vga_reg) != VGA_DISP_DISABLE) {
 		DRM_DEBUG_KMS("Something enabled VGA plane, disabling it\n");
 		i915_disable_vga(dev);
-		i915_disable_vga_mem(dev);
 	}
 }
 
@@ -10755,7 +10958,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_disable_fbc(dev);
 
-	i915_enable_vga_mem(dev);
+	intel_modeset_vga_set_state(dev, true);
 
 	intel_disable_gt_powersave(dev);
 
@@ -10798,12 +11001,47 @@ int intel_modeset_vga_set_state(struct drm_device *dev, bool state)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u16 gmch_ctrl;
 
-	pci_read_config_word(dev_priv->bridge_dev, INTEL_GMCH_CTRL, &gmch_ctrl);
-	if (state)
-		gmch_ctrl &= ~INTEL_GMCH_VGA_DISABLE;
-	else
-		gmch_ctrl |= INTEL_GMCH_VGA_DISABLE;
-	pci_write_config_word(dev_priv->bridge_dev, INTEL_GMCH_CTRL, gmch_ctrl);
+	/* Is VGA totally disabled for the IGD? */
+	pci_read_config_word(dev_priv->bridge_dev, INTEL_INFO(dev)->gen >= 6 ?
+			     SNB_GMCH_CTRL : INTEL_GMCH_CTRL, &gmch_ctrl);
+	if (gmch_ctrl & INTEL_GMCH_VGA_DISABLE)
+		return 0;
+
+	if (state) {
+		int ret;
+
+		ret = i915_vga_execute(dev, VGA_OP_MEM_ENABLE);
+		if (ret) {
+			DRM_ERROR("Failed to enable VGA memory decode\n");
+			return ret;
+		}
+
+		/*
+		 * Leave PCI_COMMAND_IO alone for now. vgaarb
+		 * should re-enable it if and when needed.
+		 */
+
+		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
+						   VGA_RSRC_LEGACY_MEM |
+						   VGA_RSRC_NORMAL_IO |
+						   VGA_RSRC_NORMAL_MEM);
+	} else {
+		int ret;
+		u16 cmd;
+
+		ret = i915_vga_execute(dev, VGA_OP_MEM_DISABLE);
+		if (ret) {
+			DRM_ERROR("Failed to disable VGA memory decode\n");
+			return ret;
+		}
+
+		pci_read_config_word(dev->pdev, PCI_COMMAND, &cmd);
+		cmd &= ~PCI_COMMAND_IO;
+		pci_write_config_word(dev->pdev, PCI_COMMAND, cmd);
+
+		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_NORMAL_MEM);
+	}
+
 	return 0;
 }
 
-- 
1.8.1.5




More information about the Intel-gfx mailing list