[PATCH] ACPI/Intel: Rework Opregion support

[PATCH] ACPI/Intel: Rework Opregion support

[Chris Wilson]

From: Matthew Garrett <mjg@redhat.com>

The Integrated Graphics Device opregion specification defines a mechanism
for the OS and system firmware to collaborate on various graphics-related
functionality. This is currently implemented in the i915 driver but isn't
strictly limited to these devices. Move it to a more generic location and
remove the i915 dependency, while simultaneously reworking i915 to make
use of the new driver.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Acked-by: Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Keith Packard <keithp@keithp.com>
Cc: Len Brown <lenb@kernel.org>
[ickle: rebase onto drm-core-next]
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---

Note: neither the opregion_dev interface or the alse_set_* properly report
failures. As such we have a slight change in behaviour on Ironlake+
platforms and an uncorrected bug for earlier chipsets.
-Chris

---
 drivers/acpi/Kconfig                  |    8 +
 drivers/acpi/Makefile                 |    1 +
 drivers/acpi/acpi_igd_opregion.c      |  411 ++++++++++++++++++++++++++
 drivers/gpu/drm/Kconfig               |    1 +
 drivers/gpu/drm/i915/Makefile         |    1 -
 drivers/gpu/drm/i915/i915_debugfs.c   |    2 +-
 drivers/gpu/drm/i915/i915_dma.c       |   13 +-
 drivers/gpu/drm/i915/i915_drv.c       |    6 +-
 drivers/gpu/drm/i915/i915_drv.h       |   34 +--
 drivers/gpu/drm/i915/i915_irq.c       |    7 +-
 drivers/gpu/drm/i915/intel_bios.c     |    8 +-
 drivers/gpu/drm/i915/intel_lvds.c     |    2 +-
 drivers/gpu/drm/i915/intel_opregion.c |  516 ---------------------------------
 drivers/gpu/drm/i915/intel_panel.c    |    4 +-
 include/acpi/acpi_igd_opregion.h      |  105 +++++++
 15 files changed, 552 insertions(+), 567 deletions(-)
 create mode 100644 drivers/acpi/acpi_igd_opregion.c
 delete mode 100644 drivers/gpu/drm/i915/intel_opregion.c
 create mode 100644 include/acpi/acpi_igd_opregion.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 2aa042a..7ace174 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -383,4 +383,12 @@ config ACPI_HED
 
 source "drivers/acpi/apei/Kconfig"
 
+config ACPI_IGD_OPREGION
+       tristate "ACPI Integrated Graphics Device OpRegion support"
+       help
+         This driver adds support for the Intel ACPI Integrated Graphics
+	 Device OpRegion specification, allowing communication between
+	 the firmware and graphics driver on mobile systems with Intel
+	 graphics
+
 endif	# ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index d113fa5..dcade4b 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_ACPI_SBS)		+= sbs.o
 obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
 obj-$(CONFIG_ACPI_HED)		+= hed.o
 obj-$(CONFIG_ACPI_EC_DEBUGFS)	+= ec_sys.o
+obj-$(CONFIG_ACPI_IGD_OPREGION)	+= acpi_igd_opregion.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o processor_throttling.o
diff --git a/drivers/acpi/acpi_igd_opregion.c b/drivers/acpi/acpi_igd_opregion.c
new file mode 100644
index 0000000..0015ca8
--- /dev/null
+++ b/drivers/acpi/acpi_igd_opregion.c
@@ -0,0 +1,411 @@
+/*
+ * Copyright 2008 Intel Corporation <hong.liu@intel.com>
+ * Copyright 2008 Red Hat <mjg@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/acpi_io.h>
+#include <acpi/video.h>
+#include <acpi/acpi_igd_opregion.h>
+#include <drm/drmP.h>
+
+#define PCI_ASLE 0xe4
+#define PCI_ASLS 0xfc
+
+#define OPREGION_HEADER_OFFSET 0
+#define OPREGION_ACPI_OFFSET   0x100
+#define   ACPI_CLID 0x01ac /* current lid state indicator */
+#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
+#define OPREGION_SWSCI_OFFSET  0x200
+#define OPREGION_ASLE_OFFSET   0x300
+#define OPREGION_VBT_OFFSET    0x400
+
+#define OPREGION_SIGNATURE "IntelGraphicsMem"
+#define MBOX_ACPI      (1<<0)
+#define MBOX_SWSCI     (1<<1)
+#define MBOX_ASLE      (1<<2)
+
+/* ASLE irq request bits */
+#define ASLE_SET_ALS_ILLUM     (1 << 0)
+#define ASLE_SET_BACKLIGHT     (1 << 1)
+#define ASLE_SET_PFIT          (1 << 2)
+#define ASLE_SET_PWM_FREQ      (1 << 3)
+#define ASLE_REQ_MSK           0xf
+
+/* response bits of ASLE irq request */
+#define ASLE_ALS_ILLUM_FAILED	(1<<10)
+#define ASLE_BACKLIGHT_FAILED	(1<<12)
+#define ASLE_PFIT_FAILED	(1<<14)
+#define ASLE_PWM_FREQ_FAILED	(1<<16)
+
+/* ASLE backlight brightness to set */
+#define ASLE_BCLP_VALID                (1<<31)
+#define ASLE_BCLP_MSK          (~(1<<31))
+
+/* ASLE panel fitting request */
+#define ASLE_PFIT_VALID         (1<<31)
+#define ASLE_PFIT_CENTER (1<<0)
+#define ASLE_PFIT_STRETCH_TEXT (1<<1)
+#define ASLE_PFIT_STRETCH_GFX (1<<2)
+
+/* PWM frequency and minimum brightness */
+#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
+#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
+#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
+#define ASLE_PFMB_PWM_VALID (1<<31)
+
+#define ASLE_CBLV_VALID         (1<<31)
+
+#define ACPI_OTHER_OUTPUT (0<<8)
+#define ACPI_VGA_OUTPUT (1<<8)
+#define ACPI_TV_OUTPUT (2<<8)
+#define ACPI_DIGITAL_OUTPUT (3<<8)
+#define ACPI_LVDS_OUTPUT (4<<8)
+
+#ifdef CONFIG_ACPI
+static u32 asle_set_backlight(struct opregion_dev *dev, u32 bclp)
+{
+	struct opregion_asle *asle = dev->opregion.asle;
+	u32 max = dev->max_backlight;
+
+	if (!(bclp & ASLE_BCLP_VALID))
+		return ASLE_BACKLIGHT_FAILED;
+
+	bclp &= ASLE_BCLP_MSK;
+	if (bclp > 255)
+		return ASLE_BACKLIGHT_FAILED;
+
+	if (dev->set_backlight)
+		dev->set_backlight(dev->drm_dev, bclp * max / 255);
+
+	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
+
+	return 0;
+}
+
+static u32 asle_set_als_illum(struct opregion_dev *dev, u32 alsi)
+{
+	if (dev->set_als_illum)
+		dev->set_als_illum(dev->drm_dev, alsi);
+	return 0;
+}
+
+static u32 asle_set_pwm_freq(struct opregion_dev *dev, u32 pfmb)
+{
+	if (pfmb & ASLE_PFMB_PWM_VALID) {
+		if (dev->set_pwm_freq)
+			dev->set_pwm_freq(dev->drm_dev, pfmb);
+	}
+	return 0;
+}
+
+static u32 asle_set_pfit(struct opregion_dev *dev, u32 pfit)
+{
+	if (!(pfit & ASLE_PFIT_VALID))
+		return ASLE_PFIT_FAILED;
+
+	if (dev->set_pfit)
+		dev->set_pfit(dev->drm_dev, pfit);
+
+	return 0;
+}
+
+void igd_opregion_intr(struct opregion_dev *dev)
+{
+	struct opregion_asle *asle = dev->opregion.asle;
+	u32 asle_stat = 0;
+	u32 asle_req;
+
+	if (!asle)
+		return;
+
+	asle_req = asle->aslc & ASLE_REQ_MSK;
+
+	if (!asle_req) {
+		DRM_DEBUG_DRIVER("non asle set request??\n");
+		return;
+	}
+
+	if (asle_req & ASLE_SET_ALS_ILLUM)
+		asle_stat |= asle_set_als_illum(dev, asle->alsi);
+
+	if (asle_req & ASLE_SET_BACKLIGHT)
+		asle_stat |= asle_set_backlight(dev, asle->bclp);
+
+	if (asle_req & ASLE_SET_PFIT)
+		asle_stat |= asle_set_pfit(dev, asle->pfit);
+
+	if (asle_req & ASLE_SET_PWM_FREQ)
+		asle_stat |= asle_set_pwm_freq(dev, asle->pfmb);
+
+	asle->aslc = asle_stat;
+}
+EXPORT_SYMBOL(igd_opregion_intr);
+
+#define ASLE_ALS_EN    (1<<0)
+#define ASLE_BLC_EN    (1<<1)
+#define ASLE_PFIT_EN   (1<<2)
+#define ASLE_PFMB_EN   (1<<3)
+
+void igd_opregion_enable_asle(struct opregion_dev *dev)
+{
+	struct opregion_asle *asle = dev->opregion.asle;
+
+	if (asle && dev->enable_asle) {
+		dev->enable_asle(dev->drm_dev);
+
+		asle->tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
+			ASLE_PFMB_EN;
+		asle->ardy = 1;
+	}
+}
+EXPORT_SYMBOL(igd_opregion_enable_asle);
+
+#define ACPI_EV_DISPLAY_SWITCH (1<<0)
+#define ACPI_EV_LID            (1<<1)
+#define ACPI_EV_DOCK           (1<<2)
+
+static struct igd_opregion *system_opregion;
+
+static int igd_opregion_video_event(struct notifier_block *nb,
+				      unsigned long val, void *data)
+{
+	/* The only video events relevant to opregion are 0x80. These indicate
+	   either a docking event, lid switch or display switch request. In
+	   Linux, these are handled by the dock, button and video drivers.
+	   We might want to fix the video driver to be opregion-aware in
+	   future, but right now we just indicate to the firmware that the
+	   request has been handled */
+
+	struct opregion_acpi *acpi;
+
+	if (!system_opregion)
+		return NOTIFY_DONE;
+
+	acpi = system_opregion->acpi;
+	acpi->csts = 0;
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block igd_opregion_notifier = {
+	.notifier_call = igd_opregion_video_event,
+};
+
+/*
+ * Initialise the DIDL field in opregion. This passes a list of devices to
+ * the firmware. Values are defined by section B.4.2 of the ACPI specification
+ * (version 3)
+ */
+
+static void igd_didl_outputs(struct opregion_dev *dev)
+{
+	struct igd_opregion *opregion = &dev->opregion;
+	struct drm_connector *connector;
+	acpi_handle handle;
+	struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL;
+	unsigned long long device_id;
+	acpi_status status;
+	int i = 0;
+
+	handle = DEVICE_ACPI_HANDLE(&dev->drm_dev->pdev->dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev)))
+		return;
+
+	if (acpi_is_video_device(acpi_dev))
+		acpi_video_bus = acpi_dev;
+	else {
+		list_for_each_entry(acpi_cdev, &acpi_dev->children, node) {
+			if (acpi_is_video_device(acpi_cdev)) {
+				acpi_video_bus = acpi_cdev;
+				break;
+			}
+		}
+	}
+
+	if (!acpi_video_bus) {
+		printk(KERN_WARNING "No ACPI video bus found\n");
+		return;
+	}
+
+	list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
+		if (i >= 8) {
+			dev_printk(KERN_ERR, &dev->drm_dev->pdev->dev,
+				   "More than 8 outputs detected\n");
+			return;
+		}
+		status =
+			acpi_evaluate_integer(acpi_cdev->handle, "_ADR",
+						NULL, &device_id);
+		if (ACPI_SUCCESS(status)) {
+			if (!device_id)
+				goto blind_set;
+			opregion->acpi->didl[i] = (u32)(device_id & 0x0f0f);
+			i++;
+		}
+	}
+
+end:
+	/* If fewer than 8 outputs, the list must be null terminated */
+	if (i < 8)
+		opregion->acpi->didl[i] = 0;
+	return;
+
+blind_set:
+	i = 0;
+	list_for_each_entry(connector, &dev->drm_dev->mode_config.connector_list, head) {
+		int output_type = ACPI_OTHER_OUTPUT;
+		if (i >= 8) {
+			dev_printk(KERN_ERR, &dev->drm_dev->pdev->dev,
+				   "More than 8 outputs detected\n");
+			return;
+		}
+		switch (connector->connector_type) {
+		case DRM_MODE_CONNECTOR_VGA:
+		case DRM_MODE_CONNECTOR_DVIA:
+			output_type = ACPI_VGA_OUTPUT;
+			break;
+		case DRM_MODE_CONNECTOR_Composite:
+		case DRM_MODE_CONNECTOR_SVIDEO:
+		case DRM_MODE_CONNECTOR_Component:
+		case DRM_MODE_CONNECTOR_9PinDIN:
+			output_type = ACPI_TV_OUTPUT;
+			break;
+		case DRM_MODE_CONNECTOR_DVII:
+		case DRM_MODE_CONNECTOR_DVID:
+		case DRM_MODE_CONNECTOR_DisplayPort:
+		case DRM_MODE_CONNECTOR_HDMIA:
+		case DRM_MODE_CONNECTOR_HDMIB:
+			output_type = ACPI_DIGITAL_OUTPUT;
+			break;
+		case DRM_MODE_CONNECTOR_LVDS:
+			output_type = ACPI_LVDS_OUTPUT;
+			break;
+		}
+		opregion->acpi->didl[i] |= (1<<31) | output_type | i;
+		i++;
+	}
+	goto end;
+}
+
+void igd_opregion_init(struct opregion_dev *dev)
+{
+	struct igd_opregion *opregion = &dev->opregion;
+
+	if (!opregion->header)
+		return;
+
+	if (opregion->acpi) {
+		if (drm_core_check_feature(dev->drm_dev, DRIVER_MODESET))
+			igd_didl_outputs(dev);
+
+		/* Notify BIOS we are ready to handle ACPI video ext notifs.
+		 * Right now, all the events are handled by the ACPI video module.
+		 * We don't actually need to do anything with them. */
+		opregion->acpi->csts = 0;
+		opregion->acpi->drdy = 1;
+
+		system_opregion = opregion;
+		register_acpi_notifier(&igd_opregion_notifier);
+	}
+
+	if (opregion->asle)
+		dev->enable_asle(dev->drm_dev);
+}
+EXPORT_SYMBOL(igd_opregion_init);
+
+void igd_opregion_fini(struct opregion_dev *dev)
+{
+	struct igd_opregion *opregion = &dev->opregion;
+
+	if (!opregion->header)
+		return;
+
+	if (opregion->acpi) {
+		opregion->acpi->drdy = 0;
+
+		system_opregion = NULL;
+		unregister_acpi_notifier(&igd_opregion_notifier);
+	}
+
+	/* just clear all opregion memory pointers now */
+	iounmap(opregion->header);
+	opregion->header = NULL;
+	opregion->acpi = NULL;
+	opregion->swsci = NULL;
+	opregion->asle = NULL;
+	opregion->vbt = NULL;
+}
+EXPORT_SYMBOL(igd_opregion_fini);
+#endif
+
+int igd_opregion_setup(struct opregion_dev *dev)
+{
+	struct igd_opregion *opregion = &dev->opregion;
+	void *base;
+	u32 asls, mboxes;
+	int err = 0;
+
+	pci_read_config_dword(dev->drm_dev->pdev, PCI_ASLS, &asls);
+	DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
+	if (asls == 0) {
+		DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
+		return -ENOTSUPP;
+	}
+
+	base = acpi_os_ioremap(asls, OPREGION_SIZE);
+	if (!base)
+		return -ENOMEM;
+
+	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
+		DRM_DEBUG_DRIVER("opregion signature mismatch\n");
+		err = -EINVAL;
+		goto err_out;
+	}
+	opregion->header = base;
+	opregion->vbt = base + OPREGION_VBT_OFFSET;
+
+	mboxes = opregion->header->mboxes;
+	if (mboxes & MBOX_ACPI) {
+		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
+		opregion->acpi = base + OPREGION_ACPI_OFFSET;
+	}
+
+	if (mboxes & MBOX_SWSCI) {
+		DRM_DEBUG_DRIVER("SWSCI supported\n");
+		opregion->swsci = base + OPREGION_SWSCI_OFFSET;
+	}
+	if (mboxes & MBOX_ASLE) {
+		DRM_DEBUG_DRIVER("ASLE supported\n");
+		opregion->asle = base + OPREGION_ASLE_OFFSET;
+	}
+
+	return 0;
+
+err_out:
+	iounmap(base);
+	return err;
+}
+EXPORT_SYMBOL(igd_opregion_setup);
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index a6feb78c..501f4dc 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -99,6 +99,7 @@ config DRM_I915
 	select INPUT if ACPI
 	select ACPI_VIDEO if ACPI
 	select ACPI_BUTTON if ACPI
+	select ACPI_IGD_OPREGION if ACPI
 	help
 	  Choose this option if you have a system that has "Intel Graphics
 	  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0ae6a7c..776c362 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -28,7 +28,6 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
 	  intel_dvo.o \
 	  intel_ringbuffer.o \
 	  intel_overlay.o \
-	  intel_opregion.o \
 	  dvo_ch7xxx.o \
 	  dvo_ch7017.o \
 	  dvo_ivch.o \
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 09e0327..e67ff81 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1129,7 +1129,7 @@ static int i915_opregion(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct intel_opregion *opregion = &dev_priv->opregion;
+	struct igd_opregion *opregion = &dev_priv->opregion_dev.opregion;
 	int ret;
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 7273037..ff057a8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1990,7 +1990,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev);
 	intel_setup_gmbus(dev);
-	intel_opregion_setup(dev);
+
+	if (IS_MOBILE(dev)) {
+		dev_priv->opregion_dev.max_backlight = intel_panel_get_max_backlight(dev);
+		dev_priv->opregion_dev.set_backlight = intel_panel_set_backlight;
+	}
+	dev_priv->opregion_dev.enable_asle = intel_enable_asle;
+	dev_priv->opregion_dev.drm_dev = dev;
+	igd_opregion_setup(&dev_priv->opregion_dev);
 
 	/* Make sure the bios did its job and set up vital registers */
 	intel_setup_bios(dev);
@@ -2049,7 +2056,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	}
 
 	/* Must be done after probing outputs */
-	intel_opregion_init(dev);
+	igd_opregion_init(&dev_priv->opregion_dev);
 	acpi_video_register();
 
 	setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
@@ -2138,7 +2145,7 @@ int i915_driver_unload(struct drm_device *dev)
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
 
-	intel_opregion_fini(dev);
+	igd_opregion_fini(&dev_priv->opregion_dev);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		/* Flush any outstanding unpin_work. */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c34a8dd..cf66ca8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -316,7 +316,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 	i915_save_state(dev);
 
-	intel_opregion_fini(dev);
+	igd_opregion_fini(&dev_priv->opregion_dev);
 
 	/* Modeset on resume, not lid events */
 	dev_priv->modeset_on_lid = 0;
@@ -366,7 +366,7 @@ static int i915_drm_thaw(struct drm_device *dev)
 	}
 
 	i915_restore_state(dev);
-	intel_opregion_setup(dev);
+	igd_opregion_setup(&dev_priv->opregion_dev);
 
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
@@ -386,7 +386,7 @@ static int i915_drm_thaw(struct drm_device *dev)
 			ironlake_enable_rc6(dev);
 	}
 
-	intel_opregion_init(dev);
+	igd_opregion_init(&dev_priv->opregion_dev);
 
 	dev_priv->modeset_on_lid = 0;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4496505..4d2475b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -36,6 +36,7 @@
 #include <linux/io-mapping.h>
 #include <linux/i2c.h>
 #include <drm/intel-gtt.h>
+#include <acpi/acpi_igd_opregion.h>
 
 /* General customization:
  */
@@ -102,21 +103,6 @@ struct mem_block {
 	struct drm_file *file_priv; /* NULL: free, -1: heap, other: real files */
 };
 
-struct opregion_header;
-struct opregion_acpi;
-struct opregion_swsci;
-struct opregion_asle;
-
-struct intel_opregion {
-	struct opregion_header *header;
-	struct opregion_acpi *acpi;
-	struct opregion_swsci *swsci;
-	struct opregion_asle *asle;
-	void *vbt;
-	u32 __iomem *lid_state;
-};
-#define OPREGION_SIZE            (8*1024)
-
 struct intel_overlay;
 struct intel_overlay_error_state;
 
@@ -322,7 +308,7 @@ typedef struct drm_i915_private {
 	int cfb_plane;
 	int cfb_y;
 
-	struct intel_opregion opregion;
+	struct opregion_dev opregion_dev;
 
 	/* overlay */
 	struct intel_overlay *overlay;
@@ -1237,22 +1223,6 @@ extern inline bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter)
 }
 extern void intel_i2c_reset(struct drm_device *dev);
 
-/* intel_opregion.c */
-extern int intel_opregion_setup(struct drm_device *dev);
-#ifdef CONFIG_ACPI
-extern void intel_opregion_init(struct drm_device *dev);
-extern void intel_opregion_fini(struct drm_device *dev);
-extern void intel_opregion_asle_intr(struct drm_device *dev);
-extern void intel_opregion_gse_intr(struct drm_device *dev);
-extern void intel_opregion_enable_asle(struct drm_device *dev);
-#else
-static inline void intel_opregion_init(struct drm_device *dev) { return; }
-static inline void intel_opregion_fini(struct drm_device *dev) { return; }
-static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
-static inline void intel_opregion_gse_intr(struct drm_device *dev) { return; }
-static inline void intel_opregion_enable_asle(struct drm_device *dev) { return; }
-#endif
-
 /* intel_acpi.c */
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 188b497..8d9f406 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -496,7 +496,7 @@ static irqreturn_t ironlake_irq_handler(struct drm_device *dev)
 		notify_ring(dev, &dev_priv->ring[BCS]);
 
 	if (de_iir & DE_GSE)
-		intel_opregion_gse_intr(dev);
+		igd_opregion_intr(&dev_priv->opregion_dev);
 
 	if (de_iir & DE_PLANEA_FLIP_DONE) {
 		intel_prepare_page_flip(dev, 0);
@@ -1203,9 +1203,8 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
 				blc_event = true;
 		}
 
-
 		if (blc_event || (iir & I915_ASLE_INTERRUPT))
-			intel_opregion_asle_intr(dev);
+			igd_opregion_intr(&dev_priv->opregion_dev);
 
 		/* With MSI, interrupts are only generated when iir
 		 * transitions from zero to nonzero.  If another bit got
@@ -1762,7 +1761,7 @@ int i915_driver_irq_postinstall(struct drm_device *dev)
 		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
 	}
 
-	intel_opregion_enable_asle(dev);
+	igd_opregion_enable_asle(&dev_priv->opregion_dev);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index fb5b4d4..5123cf9 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -611,15 +611,15 @@ intel_parse_bios(struct drm_device *dev)
 
 	init_vbt_defaults(dev_priv);
 
-	/* XXX Should this validation be moved to intel_opregion.c? */
-	if (dev_priv->opregion.vbt) {
-		struct vbt_header *vbt = dev_priv->opregion.vbt;
+	/* XXX Should this validation be moved to acpi_igd_opregion.c? */
+	if (dev_priv->opregion_dev.opregion.vbt) {
+		struct vbt_header *vbt = dev_priv->opregion_dev.opregion.vbt;
 		if (memcmp(vbt->signature, "$VBT", 4) == 0) {
 			DRM_DEBUG_DRIVER("Using VBT from OpRegion: %20s\n",
 					 vbt->signature);
 			bdb = (struct bdb_header *)((char *)vbt + vbt->bdb_offset);
 		} else
-			dev_priv->opregion.vbt = NULL;
+			dev_priv->opregion_dev.opregion.vbt = NULL;
 	}
 
 	if (bdb == NULL) {
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 1a311ad..a499e867e 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -828,7 +828,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
 		 * additional data.  Trust that if the VBT was written into
 		 * the OpRegion then they have validated the LVDS's existence.
 		 */
-		if (dev_priv->opregion.vbt)
+		if (dev_priv->opregion_dev.opregion.vbt)
 			return true;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
deleted file mode 100644
index d2c7104..0000000
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ /dev/null
@@ -1,516 +0,0 @@
-/*
- * Copyright 2008 Intel Corporation <hong.liu@intel.com>
- * Copyright 2008 Red Hat <mjg@redhat.com>
- *
- * Permission is hereby granted, free of charge, to any person obtaining
- * a copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial
- * portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
- * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- *
- */
-
-#include <linux/acpi.h>
-#include <linux/acpi_io.h>
-#include <acpi/video.h>
-
-#include "drmP.h"
-#include "i915_drm.h"
-#include "i915_drv.h"
-#include "intel_drv.h"
-
-#define PCI_ASLE 0xe4
-#define PCI_ASLS 0xfc
-
-#define OPREGION_HEADER_OFFSET 0
-#define OPREGION_ACPI_OFFSET   0x100
-#define   ACPI_CLID 0x01ac /* current lid state indicator */
-#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
-#define OPREGION_SWSCI_OFFSET  0x200
-#define OPREGION_ASLE_OFFSET   0x300
-#define OPREGION_VBT_OFFSET    0x400
-
-#define OPREGION_SIGNATURE "IntelGraphicsMem"
-#define MBOX_ACPI      (1<<0)
-#define MBOX_SWSCI     (1<<1)
-#define MBOX_ASLE      (1<<2)
-
-struct opregion_header {
-       u8 signature[16];
-       u32 size;
-       u32 opregion_ver;
-       u8 bios_ver[32];
-       u8 vbios_ver[16];
-       u8 driver_ver[16];
-       u32 mboxes;
-       u8 reserved[164];
-} __attribute__((packed));
-
-/* OpRegion mailbox #1: public ACPI methods */
-struct opregion_acpi {
-       u32 drdy;       /* driver readiness */
-       u32 csts;       /* notification status */
-       u32 cevt;       /* current event */
-       u8 rsvd1[20];
-       u32 didl[8];    /* supported display devices ID list */
-       u32 cpdl[8];    /* currently presented display list */
-       u32 cadl[8];    /* currently active display list */
-       u32 nadl[8];    /* next active devices list */
-       u32 aslp;       /* ASL sleep time-out */
-       u32 tidx;       /* toggle table index */
-       u32 chpd;       /* current hotplug enable indicator */
-       u32 clid;       /* current lid state*/
-       u32 cdck;       /* current docking state */
-       u32 sxsw;       /* Sx state resume */
-       u32 evts;       /* ASL supported events */
-       u32 cnot;       /* current OS notification */
-       u32 nrdy;       /* driver status */
-       u8 rsvd2[60];
-} __attribute__((packed));
-
-/* OpRegion mailbox #2: SWSCI */
-struct opregion_swsci {
-       u32 scic;       /* SWSCI command|status|data */
-       u32 parm;       /* command parameters */
-       u32 dslp;       /* driver sleep time-out */
-       u8 rsvd[244];
-} __attribute__((packed));
-
-/* OpRegion mailbox #3: ASLE */
-struct opregion_asle {
-       u32 ardy;       /* driver readiness */
-       u32 aslc;       /* ASLE interrupt command */
-       u32 tche;       /* technology enabled indicator */
-       u32 alsi;       /* current ALS illuminance reading */
-       u32 bclp;       /* backlight brightness to set */
-       u32 pfit;       /* panel fitting state */
-       u32 cblv;       /* current brightness level */
-       u16 bclm[20];   /* backlight level duty cycle mapping table */
-       u32 cpfm;       /* current panel fitting mode */
-       u32 epfm;       /* enabled panel fitting modes */
-       u8 plut[74];    /* panel LUT and identifier */
-       u32 pfmb;       /* PWM freq and min brightness */
-       u8 rsvd[102];
-} __attribute__((packed));
-
-/* ASLE irq request bits */
-#define ASLE_SET_ALS_ILLUM     (1 << 0)
-#define ASLE_SET_BACKLIGHT     (1 << 1)
-#define ASLE_SET_PFIT          (1 << 2)
-#define ASLE_SET_PWM_FREQ      (1 << 3)
-#define ASLE_REQ_MSK           0xf
-
-/* response bits of ASLE irq request */
-#define ASLE_ALS_ILLUM_FAILED	(1<<10)
-#define ASLE_BACKLIGHT_FAILED	(1<<12)
-#define ASLE_PFIT_FAILED	(1<<14)
-#define ASLE_PWM_FREQ_FAILED	(1<<16)
-
-/* ASLE backlight brightness to set */
-#define ASLE_BCLP_VALID                (1<<31)
-#define ASLE_BCLP_MSK          (~(1<<31))
-
-/* ASLE panel fitting request */
-#define ASLE_PFIT_VALID         (1<<31)
-#define ASLE_PFIT_CENTER (1<<0)
-#define ASLE_PFIT_STRETCH_TEXT (1<<1)
-#define ASLE_PFIT_STRETCH_GFX (1<<2)
-
-/* PWM frequency and minimum brightness */
-#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
-#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
-#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
-#define ASLE_PFMB_PWM_VALID (1<<31)
-
-#define ASLE_CBLV_VALID         (1<<31)
-
-#define ACPI_OTHER_OUTPUT (0<<8)
-#define ACPI_VGA_OUTPUT (1<<8)
-#define ACPI_TV_OUTPUT (2<<8)
-#define ACPI_DIGITAL_OUTPUT (3<<8)
-#define ACPI_LVDS_OUTPUT (4<<8)
-
-#ifdef CONFIG_ACPI
-static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
-	u32 max;
-
-	if (!(bclp & ASLE_BCLP_VALID))
-		return ASLE_BACKLIGHT_FAILED;
-
-	bclp &= ASLE_BCLP_MSK;
-	if (bclp > 255)
-		return ASLE_BACKLIGHT_FAILED;
-
-	max = intel_panel_get_max_backlight(dev);
-	intel_panel_set_backlight(dev, bclp * max / 255);
-	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
-
-	return 0;
-}
-
-static u32 asle_set_als_illum(struct drm_device *dev, u32 alsi)
-{
-	/* alsi is the current ALS reading in lux. 0 indicates below sensor
-	   range, 0xffff indicates above sensor range. 1-0xfffe are valid */
-	return 0;
-}
-
-static u32 asle_set_pwm_freq(struct drm_device *dev, u32 pfmb)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	if (pfmb & ASLE_PFMB_PWM_VALID) {
-		u32 blc_pwm_ctl = I915_READ(BLC_PWM_CTL);
-		u32 pwm = pfmb & ASLE_PFMB_PWM_MASK;
-		blc_pwm_ctl &= BACKLIGHT_DUTY_CYCLE_MASK;
-		pwm = pwm >> 9;
-		/* FIXME - what do we do with the PWM? */
-	}
-	return 0;
-}
-
-static u32 asle_set_pfit(struct drm_device *dev, u32 pfit)
-{
-	/* Panel fitting is currently controlled by the X code, so this is a
-	   noop until modesetting support works fully */
-	if (!(pfit & ASLE_PFIT_VALID))
-		return ASLE_PFIT_FAILED;
-	return 0;
-}
-
-void intel_opregion_asle_intr(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
-	u32 asle_stat = 0;
-	u32 asle_req;
-
-	if (!asle)
-		return;
-
-	asle_req = asle->aslc & ASLE_REQ_MSK;
-
-	if (!asle_req) {
-		DRM_DEBUG_DRIVER("non asle set request??\n");
-		return;
-	}
-
-	if (asle_req & ASLE_SET_ALS_ILLUM)
-		asle_stat |= asle_set_als_illum(dev, asle->alsi);
-
-	if (asle_req & ASLE_SET_BACKLIGHT)
-		asle_stat |= asle_set_backlight(dev, asle->bclp);
-
-	if (asle_req & ASLE_SET_PFIT)
-		asle_stat |= asle_set_pfit(dev, asle->pfit);
-
-	if (asle_req & ASLE_SET_PWM_FREQ)
-		asle_stat |= asle_set_pwm_freq(dev, asle->pfmb);
-
-	asle->aslc = asle_stat;
-}
-
-/* Only present on Ironlake+ */
-void intel_opregion_gse_intr(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
-	u32 asle_stat = 0;
-	u32 asle_req;
-
-	if (!asle)
-		return;
-
-	asle_req = asle->aslc & ASLE_REQ_MSK;
-
-	if (!asle_req) {
-		DRM_DEBUG_DRIVER("non asle set request??\n");
-		return;
-	}
-
-	if (asle_req & ASLE_SET_ALS_ILLUM) {
-		DRM_DEBUG_DRIVER("Illum is not supported\n");
-		asle_stat |= ASLE_ALS_ILLUM_FAILED;
-	}
-
-	if (asle_req & ASLE_SET_BACKLIGHT)
-		asle_stat |= asle_set_backlight(dev, asle->bclp);
-
-	if (asle_req & ASLE_SET_PFIT) {
-		DRM_DEBUG_DRIVER("Pfit is not supported\n");
-		asle_stat |= ASLE_PFIT_FAILED;
-	}
-
-	if (asle_req & ASLE_SET_PWM_FREQ) {
-		DRM_DEBUG_DRIVER("PWM freq is not supported\n");
-		asle_stat |= ASLE_PWM_FREQ_FAILED;
-	}
-
-	asle->aslc = asle_stat;
-}
-#define ASLE_ALS_EN    (1<<0)
-#define ASLE_BLC_EN    (1<<1)
-#define ASLE_PFIT_EN   (1<<2)
-#define ASLE_PFMB_EN   (1<<3)
-
-void intel_opregion_enable_asle(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
-
-	if (asle) {
-		if (IS_MOBILE(dev))
-			intel_enable_asle(dev);
-
-		asle->tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
-			ASLE_PFMB_EN;
-		asle->ardy = 1;
-	}
-}
-
-#define ACPI_EV_DISPLAY_SWITCH (1<<0)
-#define ACPI_EV_LID            (1<<1)
-#define ACPI_EV_DOCK           (1<<2)
-
-static struct intel_opregion *system_opregion;
-
-static int intel_opregion_video_event(struct notifier_block *nb,
-				      unsigned long val, void *data)
-{
-	/* The only video events relevant to opregion are 0x80. These indicate
-	   either a docking event, lid switch or display switch request. In
-	   Linux, these are handled by the dock, button and video drivers.
-	   We might want to fix the video driver to be opregion-aware in
-	   future, but right now we just indicate to the firmware that the
-	   request has been handled */
-
-	struct opregion_acpi *acpi;
-
-	if (!system_opregion)
-		return NOTIFY_DONE;
-
-	acpi = system_opregion->acpi;
-	acpi->csts = 0;
-
-	return NOTIFY_OK;
-}
-
-static struct notifier_block intel_opregion_notifier = {
-	.notifier_call = intel_opregion_video_event,
-};
-
-/*
- * Initialise the DIDL field in opregion. This passes a list of devices to
- * the firmware. Values are defined by section B.4.2 of the ACPI specification
- * (version 3)
- */
-
-static void intel_didl_outputs(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_opregion *opregion = &dev_priv->opregion;
-	struct drm_connector *connector;
-	acpi_handle handle;
-	struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL;
-	unsigned long long device_id;
-	acpi_status status;
-	int i = 0;
-
-	handle = DEVICE_ACPI_HANDLE(&dev->pdev->dev);
-	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev)))
-		return;
-
-	if (acpi_is_video_device(acpi_dev))
-		acpi_video_bus = acpi_dev;
-	else {
-		list_for_each_entry(acpi_cdev, &acpi_dev->children, node) {
-			if (acpi_is_video_device(acpi_cdev)) {
-				acpi_video_bus = acpi_cdev;
-				break;
-			}
-		}
-	}
-
-	if (!acpi_video_bus) {
-		printk(KERN_WARNING "No ACPI video bus found\n");
-		return;
-	}
-
-	list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
-		if (i >= 8) {
-			dev_printk (KERN_ERR, &dev->pdev->dev,
-				    "More than 8 outputs detected\n");
-			return;
-		}
-		status =
-			acpi_evaluate_integer(acpi_cdev->handle, "_ADR",
-						NULL, &device_id);
-		if (ACPI_SUCCESS(status)) {
-			if (!device_id)
-				goto blind_set;
-			opregion->acpi->didl[i] = (u32)(device_id & 0x0f0f);
-			i++;
-		}
-	}
-
-end:
-	/* If fewer than 8 outputs, the list must be null terminated */
-	if (i < 8)
-		opregion->acpi->didl[i] = 0;
-	return;
-
-blind_set:
-	i = 0;
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		int output_type = ACPI_OTHER_OUTPUT;
-		if (i >= 8) {
-			dev_printk (KERN_ERR, &dev->pdev->dev,
-				    "More than 8 outputs detected\n");
-			return;
-		}
-		switch (connector->connector_type) {
-		case DRM_MODE_CONNECTOR_VGA:
-		case DRM_MODE_CONNECTOR_DVIA:
-			output_type = ACPI_VGA_OUTPUT;
-			break;
-		case DRM_MODE_CONNECTOR_Composite:
-		case DRM_MODE_CONNECTOR_SVIDEO:
-		case DRM_MODE_CONNECTOR_Component:
-		case DRM_MODE_CONNECTOR_9PinDIN:
-			output_type = ACPI_TV_OUTPUT;
-			break;
-		case DRM_MODE_CONNECTOR_DVII:
-		case DRM_MODE_CONNECTOR_DVID:
-		case DRM_MODE_CONNECTOR_DisplayPort:
-		case DRM_MODE_CONNECTOR_HDMIA:
-		case DRM_MODE_CONNECTOR_HDMIB:
-			output_type = ACPI_DIGITAL_OUTPUT;
-			break;
-		case DRM_MODE_CONNECTOR_LVDS:
-			output_type = ACPI_LVDS_OUTPUT;
-			break;
-		}
-		opregion->acpi->didl[i] |= (1<<31) | output_type | i;
-		i++;
-	}
-	goto end;
-}
-
-void intel_opregion_init(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_opregion *opregion = &dev_priv->opregion;
-
-	if (!opregion->header)
-		return;
-
-	if (opregion->acpi) {
-		if (drm_core_check_feature(dev, DRIVER_MODESET))
-			intel_didl_outputs(dev);
-
-		/* Notify BIOS we are ready to handle ACPI video ext notifs.
-		 * Right now, all the events are handled by the ACPI video module.
-		 * We don't actually need to do anything with them. */
-		opregion->acpi->csts = 0;
-		opregion->acpi->drdy = 1;
-
-		system_opregion = opregion;
-		register_acpi_notifier(&intel_opregion_notifier);
-	}
-
-	if (opregion->asle)
-		intel_opregion_enable_asle(dev);
-}
-
-void intel_opregion_fini(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_opregion *opregion = &dev_priv->opregion;
-
-	if (!opregion->header)
-		return;
-
-	if (opregion->acpi) {
-		opregion->acpi->drdy = 0;
-
-		system_opregion = NULL;
-		unregister_acpi_notifier(&intel_opregion_notifier);
-	}
-
-	/* just clear all opregion memory pointers now */
-	iounmap(opregion->header);
-	opregion->header = NULL;
-	opregion->acpi = NULL;
-	opregion->swsci = NULL;
-	opregion->asle = NULL;
-	opregion->vbt = NULL;
-}
-#endif
-
-int intel_opregion_setup(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_opregion *opregion = &dev_priv->opregion;
-	void *base;
-	u32 asls, mboxes;
-	int err = 0;
-
-	pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
-	DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
-	if (asls == 0) {
-		DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
-		return -ENOTSUPP;
-	}
-
-	base = acpi_os_ioremap(asls, OPREGION_SIZE);
-	if (!base)
-		return -ENOMEM;
-
-	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
-		DRM_DEBUG_DRIVER("opregion signature mismatch\n");
-		err = -EINVAL;
-		goto err_out;
-	}
-	opregion->header = base;
-	opregion->vbt = base + OPREGION_VBT_OFFSET;
-
-	opregion->lid_state = base + ACPI_CLID;
-
-	mboxes = opregion->header->mboxes;
-	if (mboxes & MBOX_ACPI) {
-		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
-		opregion->acpi = base + OPREGION_ACPI_OFFSET;
-	}
-
-	if (mboxes & MBOX_SWSCI) {
-		DRM_DEBUG_DRIVER("SWSCI supported\n");
-		opregion->swsci = base + OPREGION_SWSCI_OFFSET;
-	}
-	if (mboxes & MBOX_ASLE) {
-		DRM_DEBUG_DRIVER("ASLE supported\n");
-		opregion->asle = base + OPREGION_ASLE_OFFSET;
-	}
-
-	return 0;
-
-err_out:
-	iounmap(base);
-	return err;
-}
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 4e5ff59..51565bb 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -261,8 +261,8 @@ intel_panel_detect(struct drm_device *dev)
 	 * appears to be either the BIOS or Linux ACPI fault */
 #if 0
 	/* Assume that the BIOS does not lie through the OpRegion... */
-	if (dev_priv->opregion.lid_state)
-		return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
+	if (dev_priv->opregion_dev.opregion.acpi)
+		return ioread32(&dev_priv->opregion_dev.opregion.acpi->clid) & 0x1 ?
 			connector_status_connected :
 			connector_status_disconnected;
 #endif
diff --git a/include/acpi/acpi_igd_opregion.h b/include/acpi/acpi_igd_opregion.h
new file mode 100644
index 0000000..36fefd2
--- /dev/null
+++ b/include/acpi/acpi_igd_opregion.h
@@ -0,0 +1,105 @@
+#ifndef __ACPI_IGD_OPREGION_H__
+#define __ACPI_IGD_OPREGION_H__
+
+#include <drm/drmP.h>
+
+#define OPREGION_SIZE            (8*1024)
+
+struct opregion_header {
+	u8 signature[16];
+	u32 size;
+	u32 opregion_ver;
+	u8 bios_ver[32];
+	u8 vbios_ver[16];
+	u8 driver_ver[16];
+	u32 mboxes;
+	u8 reserved[164];
+} __packed;
+
+/* OpRegion mailbox #1: public ACPI methods */
+struct opregion_acpi {
+	u32 drdy;       /* driver readiness */
+	u32 csts;       /* notification status */
+	u32 cevt;       /* current event */
+	u8 rsvd1[20];
+	u32 didl[8];    /* supported display devices ID list */
+	u32 cpdl[8];    /* currently presented display list */
+	u32 cadl[8];    /* currently active display list */
+	u32 nadl[8];    /* next active devices list */
+	u32 aslp;       /* ASL sleep time-out */
+	u32 tidx;       /* toggle table index */
+	u32 chpd;       /* current hotplug enable indicator */
+	u32 clid;       /* current lid state*/
+	u32 cdck;       /* current docking state */
+	u32 sxsw;       /* Sx state resume */
+	u32 evts;       /* ASL supported events */
+	u32 cnot;       /* current OS notification */
+	u32 nrdy;       /* driver status */
+	u8 rsvd2[60];
+} __packed;
+
+/* OpRegion mailbox #2: SWSCI */
+struct opregion_swsci {
+	u32 scic;       /* SWSCI command|status|data */
+	u32 parm;       /* command parameters */
+	u32 dslp;       /* driver sleep time-out */
+	u8 rsvd[244];
+} __packed;
+
+/* OpRegion mailbox #3: ASLE */
+struct opregion_asle {
+	u32 ardy;       /* driver readiness */
+	u32 aslc;       /* ASLE interrupt command */
+	u32 tche;       /* technology enabled indicator */
+	u32 alsi;       /* current ALS illuminance reading */
+	u32 bclp;       /* backlight brightness to set */
+	u32 pfit;       /* panel fitting state */
+	u32 cblv;       /* current brightness level */
+	u16 bclm[20];   /* backlight level duty cycle mapping table */
+	u32 cpfm;       /* current panel fitting mode */
+	u32 epfm;       /* enabled panel fitting modes */
+	u8 plut[74];    /* panel LUT and identifier */
+	u32 pfmb;       /* PWM freq and min brightness */
+	u8 rsvd[102];
+} __packed;
+
+struct igd_opregion {
+	struct opregion_header *header;
+	struct opregion_acpi *acpi;
+	struct opregion_swsci *swsci;
+	struct opregion_asle *asle;
+	void *vbt;
+};
+
+struct opregion_dev {
+	struct drm_device *drm_dev;
+	u32 max_backlight;
+	void (*set_backlight)(struct drm_device *dev, u32 value);
+	void (*set_als_illum)(struct drm_device *dev, u32 alsi);
+	void (*set_pwm_freq)(struct drm_device *dev, u32 pfmb);
+	void (*set_pfit)(struct drm_device *dev, u32 pfit);
+	void (*enable_asle)(struct drm_device *drm_dev);
+	struct igd_opregion opregion;
+};
+
+#if (defined CONFIG_ACPI_IGD_OPREGION || defined CONFIG_ACPI_IGD_OPREGION_MODULE)
+
+int igd_opregion_setup(struct opregion_dev *dev);
+void igd_opregion_init(struct opregion_dev *dev);
+void igd_opregion_fini(struct opregion_dev *dev);
+void igd_opregion_intr(struct opregion_dev *dev);
+void igd_opregion_enable_asle(struct opregion_dev *dev);
+
+#else
+
+static inline int igd_opregion_setup(struct opregion_dev *dev)
+{
+	return 0;
+}
+
+static inline void igd_opregion_init(struct opregion_dev *dev) {};
+static inline void igd_opregion_fini(struct opregion_dev *dev) {};
+static inline void igd_opregion_intr(struct opregion_dev *dev) {};
+static inline void igd_opregion_enable_asle(struct opregion_dev *dev) {};
+#endif
+#endif
-- 
1.7.4.1

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Indan Zupancic]

Hello,

Some nitpicks below.

On Mon, March 14, 2011 18:59, Chris Wilson wrote:
> From: Matthew Garrett <mjg@redhat.com>
>
> The Integrated Graphics Device opregion specification defines a mechanism
> for the OS and system firmware to collaborate on various graphics-related
> functionality. This is currently implemented in the i915 driver but isn't
> strictly limited to these devices. Move it to a more generic location and
> remove the i915 dependency, while simultaneously reworking i915 to make
> use of the new driver.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Acked-by: Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Alan Cox <alan@linux.intel.com>
> Cc: Keith Packard <keithp@keithp.com>
> Cc: Len Brown <lenb@kernel.org>
> [ickle: rebase onto drm-core-next]
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>
> Note: neither the opregion_dev interface or the alse_set_* properly report
> failures. As such we have a slight change in behaviour on Ironlake+
> platforms and an uncorrected bug for earlier chipsets.
> -Chris

What uncorrected bug?

And are there earlier chipsets with ASLE support at all, besides gen 4?
If there are no gen 2 or gen 3 chipsets with ASLE then the backlight
code can be simplified further.

>
> ---
>  drivers/acpi/Kconfig                  |    8 +
>  drivers/acpi/Makefile                 |    1 +
>  drivers/acpi/acpi_igd_opregion.c      |  411 ++++++++++++++++++++++++++
>  drivers/gpu/drm/Kconfig               |    1 +
>  drivers/gpu/drm/i915/Makefile         |    1 -
>  drivers/gpu/drm/i915/i915_debugfs.c   |    2 +-
>  drivers/gpu/drm/i915/i915_dma.c       |   13 +-
>  drivers/gpu/drm/i915/i915_drv.c       |    6 +-
>  drivers/gpu/drm/i915/i915_drv.h       |   34 +--
>  drivers/gpu/drm/i915/i915_irq.c       |    7 +-
>  drivers/gpu/drm/i915/intel_bios.c     |    8 +-
>  drivers/gpu/drm/i915/intel_lvds.c     |    2 +-
>  drivers/gpu/drm/i915/intel_opregion.c |  516 ---------------------------------
>  drivers/gpu/drm/i915/intel_panel.c    |    4 +-
>  include/acpi/acpi_igd_opregion.h      |  105 +++++++
>  15 files changed, 552 insertions(+), 567 deletions(-)
>  create mode 100644 drivers/acpi/acpi_igd_opregion.c
>  delete mode 100644 drivers/gpu/drm/i915/intel_opregion.c
>  create mode 100644 include/acpi/acpi_igd_opregion.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 2aa042a..7ace174 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -383,4 +383,12 @@ config ACPI_HED
>
>  source "drivers/acpi/apei/Kconfig"
>
> +config ACPI_IGD_OPREGION
> +       tristate "ACPI Integrated Graphics Device OpRegion support"
> +       help
> +         This driver adds support for the Intel ACPI Integrated Graphics
> +	 Device OpRegion specification, allowing communication between
> +	 the firmware and graphics driver on mobile systems with Intel
> +	 graphics
> +

Isn't this option always selected automatically when needed?
That is, it shouldn't show up as an option at all, so people
can't accidentally select it when they don't need it? Though
that doesn't quite work with modules I suppose, never mind.

>  endif	# ACPI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index d113fa5..dcade4b 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_ACPI_SBS)		+= sbs.o
>  obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
>  obj-$(CONFIG_ACPI_HED)		+= hed.o
>  obj-$(CONFIG_ACPI_EC_DEBUGFS)	+= ec_sys.o
> +obj-$(CONFIG_ACPI_IGD_OPREGION)	+= acpi_igd_opregion.o
>
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o processor_throttling.o
> diff --git a/drivers/acpi/acpi_igd_opregion.c b/drivers/acpi/acpi_igd_opregion.c
> new file mode 100644
> index 0000000..0015ca8
> --- /dev/null
> +++ b/drivers/acpi/acpi_igd_opregion.c
> @@ -0,0 +1,411 @@
> +/*
> + * Copyright 2008 Intel Corporation <hong.liu@intel.com>
> + * Copyright 2008 Red Hat <mjg@redhat.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
> + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/acpi_io.h>
> +#include <acpi/video.h>
> +#include <acpi/acpi_igd_opregion.h>
> +#include <drm/drmP.h>
> +
> +#define PCI_ASLE 0xe4
> +#define PCI_ASLS 0xfc
> +
> +#define OPREGION_HEADER_OFFSET 0
> +#define OPREGION_ACPI_OFFSET   0x100
> +#define   ACPI_CLID 0x01ac /* current lid state indicator */
> +#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
> +#define OPREGION_SWSCI_OFFSET  0x200
> +#define OPREGION_ASLE_OFFSET   0x300
> +#define OPREGION_VBT_OFFSET    0x400
> +
> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
> +#define MBOX_ACPI      (1<<0)
> +#define MBOX_SWSCI     (1<<1)
> +#define MBOX_ASLE      (1<<2)
> +
> +/* ASLE irq request bits */
> +#define ASLE_SET_ALS_ILLUM     (1 << 0)
> +#define ASLE_SET_BACKLIGHT     (1 << 1)
> +#define ASLE_SET_PFIT          (1 << 2)
> +#define ASLE_SET_PWM_FREQ      (1 << 3)
> +#define ASLE_REQ_MSK           0xf
> +
> +/* response bits of ASLE irq request */
> +#define ASLE_ALS_ILLUM_FAILED	(1<<10)
> +#define ASLE_BACKLIGHT_FAILED	(1<<12)
> +#define ASLE_PFIT_FAILED	(1<<14)
> +#define ASLE_PWM_FREQ_FAILED	(1<<16)
> +
> +/* ASLE backlight brightness to set */
> +#define ASLE_BCLP_VALID                (1<<31)
> +#define ASLE_BCLP_MSK          (~(1<<31))
> +
> +/* ASLE panel fitting request */
> +#define ASLE_PFIT_VALID         (1<<31)
> +#define ASLE_PFIT_CENTER (1<<0)
> +#define ASLE_PFIT_STRETCH_TEXT (1<<1)
> +#define ASLE_PFIT_STRETCH_GFX (1<<2)
> +
> +/* PWM frequency and minimum brightness */
> +#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
> +#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
> +#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
> +#define ASLE_PFMB_PWM_VALID (1<<31)
> +
> +#define ASLE_CBLV_VALID         (1<<31)
> +
> +#define ACPI_OTHER_OUTPUT (0<<8)
> +#define ACPI_VGA_OUTPUT (1<<8)
> +#define ACPI_TV_OUTPUT (2<<8)
> +#define ACPI_DIGITAL_OUTPUT (3<<8)
> +#define ACPI_LVDS_OUTPUT (4<<8)
> +
> +#ifdef CONFIG_ACPI
> +static u32 asle_set_backlight(struct opregion_dev *dev, u32 bclp)
> +{
> +	struct opregion_asle *asle = dev->opregion.asle;
> +	u32 max = dev->max_backlight;
> +
> +	if (!(bclp & ASLE_BCLP_VALID))
> +		return ASLE_BACKLIGHT_FAILED;
> +
> +	bclp &= ASLE_BCLP_MSK;
> +	if (bclp > 255)
> +		return ASLE_BACKLIGHT_FAILED;
> +
> +	if (dev->set_backlight)
> +		dev->set_backlight(dev->drm_dev, bclp * max / 255);

I would hide the max backlight from the opregion code and move this
calculation into set_brightness. Then change the interface to one
based on a scale from 0-255 or 0-100, which fits better with what
opregion actually does. On the other hand, it's the same code in
a different place, so it doesn't make much difference.

> +
> +	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
> +
> +	return 0;
> +}
> +
> +static u32 asle_set_als_illum(struct opregion_dev *dev, u32 alsi)
> +{
> +	if (dev->set_als_illum)
> +		dev->set_als_illum(dev->drm_dev, alsi);
> +	return 0;
> +}
> +
> +static u32 asle_set_pwm_freq(struct opregion_dev *dev, u32 pfmb)
> +{
> +	if (pfmb & ASLE_PFMB_PWM_VALID) {
> +		if (dev->set_pwm_freq)
> +			dev->set_pwm_freq(dev->drm_dev, pfmb);
> +	}
> +	return 0;
> +}
> +
> +static u32 asle_set_pfit(struct opregion_dev *dev, u32 pfit)
> +{
> +	if (!(pfit & ASLE_PFIT_VALID))
> +		return ASLE_PFIT_FAILED;
> +
> +	if (dev->set_pfit)
> +		dev->set_pfit(dev->drm_dev, pfit);
> +
> +	return 0;
> +}
> +
> +void igd_opregion_intr(struct opregion_dev *dev)
> +{
> +	struct opregion_asle *asle = dev->opregion.asle;
> +	u32 asle_stat = 0;
> +	u32 asle_req;
> +
> +	if (!asle)
> +		return;
> +
> +	asle_req = asle->aslc & ASLE_REQ_MSK;
> +
> +	if (!asle_req) {
> +		DRM_DEBUG_DRIVER("non asle set request??\n");
> +		return;
> +	}
> +
> +	if (asle_req & ASLE_SET_ALS_ILLUM)
> +		asle_stat |= asle_set_als_illum(dev, asle->alsi);
> +
> +	if (asle_req & ASLE_SET_BACKLIGHT)
> +		asle_stat |= asle_set_backlight(dev, asle->bclp);
> +
> +	if (asle_req & ASLE_SET_PFIT)
> +		asle_stat |= asle_set_pfit(dev, asle->pfit);
> +
> +	if (asle_req & ASLE_SET_PWM_FREQ)
> +		asle_stat |= asle_set_pwm_freq(dev, asle->pfmb);
> +
> +	asle->aslc = asle_stat;
> +}
> +EXPORT_SYMBOL(igd_opregion_intr);
> +
> +#define ASLE_ALS_EN    (1<<0)
> +#define ASLE_BLC_EN    (1<<1)
> +#define ASLE_PFIT_EN   (1<<2)
> +#define ASLE_PFMB_EN   (1<<3)
> +
> +void igd_opregion_enable_asle(struct opregion_dev *dev)
> +{
> +	struct opregion_asle *asle = dev->opregion.asle;
> +
> +	if (asle && dev->enable_asle) {
> +		dev->enable_asle(dev->drm_dev);
> +
> +		asle->tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
> +			ASLE_PFMB_EN;

Shouldn't which flags are set depend on which callback functions are set?
Then you can remove all the function != NULL tests too. e.g:

	if (dev->set_als_illum)
		asle->tche |= ASLE_ALS_EN;

And at the end check if any flags were set:

	if (asle->tche)
		asle_>ardy = 1;

> +		asle->ardy = 1;
> +	}
> +}
> +EXPORT_SYMBOL(igd_opregion_enable_asle);
> +
> +#define ACPI_EV_DISPLAY_SWITCH (1<<0)
> +#define ACPI_EV_LID            (1<<1)
> +#define ACPI_EV_DOCK           (1<<2)
> +
> +static struct igd_opregion *system_opregion;
> +
> +static int igd_opregion_video_event(struct notifier_block *nb,
> +				      unsigned long val, void *data)
> +{
> +	/* The only video events relevant to opregion are 0x80. These indicate
> +	   either a docking event, lid switch or display switch request. In
> +	   Linux, these are handled by the dock, button and video drivers.
> +	   We might want to fix the video driver to be opregion-aware in
> +	   future, but right now we just indicate to the firmware that the
> +	   request has been handled */
> +
> +	struct opregion_acpi *acpi;
> +
> +	if (!system_opregion)
> +		return NOTIFY_DONE;
> +
> +	acpi = system_opregion->acpi;
> +	acpi->csts = 0;
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block igd_opregion_notifier = {
> +	.notifier_call = igd_opregion_video_event,
> +};
> +
> +/*
> + * Initialise the DIDL field in opregion. This passes a list of devices to
> + * the firmware. Values are defined by section B.4.2 of the ACPI specification
> + * (version 3)
> + */
> +
> +static void igd_didl_outputs(struct opregion_dev *dev)
> +{
> +	struct igd_opregion *opregion = &dev->opregion;
> +	struct drm_connector *connector;
> +	acpi_handle handle;
> +	struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL;
> +	unsigned long long device_id;
> +	acpi_status status;
> +	int i = 0;
> +
> +	handle = DEVICE_ACPI_HANDLE(&dev->drm_dev->pdev->dev);
> +	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev)))
> +		return;
> +
> +	if (acpi_is_video_device(acpi_dev))
> +		acpi_video_bus = acpi_dev;
> +	else {
> +		list_for_each_entry(acpi_cdev, &acpi_dev->children, node) {
> +			if (acpi_is_video_device(acpi_cdev)) {
> +				acpi_video_bus = acpi_cdev;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (!acpi_video_bus) {
> +		printk(KERN_WARNING "No ACPI video bus found\n");
> +		return;
> +	}
> +
> +	list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
> +		if (i >= 8) {
> +			dev_printk(KERN_ERR, &dev->drm_dev->pdev->dev,
> +				   "More than 8 outputs detected\n");
> +			return;
> +		}
> +		status =
> +			acpi_evaluate_integer(acpi_cdev->handle, "_ADR",
> +						NULL, &device_id);
> +		if (ACPI_SUCCESS(status)) {
> +			if (!device_id)
> +				goto blind_set;
> +			opregion->acpi->didl[i] = (u32)(device_id & 0x0f0f);
> +			i++;
> +		}
> +	}
> +
> +end:
> +	/* If fewer than 8 outputs, the list must be null terminated */
> +	if (i < 8)
> +		opregion->acpi->didl[i] = 0;
> +	return;
> +
> +blind_set:
> +	i = 0;
> +	list_for_each_entry(connector, &dev->drm_dev->mode_config.connector_list, head) {
> +		int output_type = ACPI_OTHER_OUTPUT;
> +		if (i >= 8) {
> +			dev_printk(KERN_ERR, &dev->drm_dev->pdev->dev,
> +				   "More than 8 outputs detected\n");
> +			return;
> +		}
> +		switch (connector->connector_type) {
> +		case DRM_MODE_CONNECTOR_VGA:
> +		case DRM_MODE_CONNECTOR_DVIA:
> +			output_type = ACPI_VGA_OUTPUT;
> +			break;
> +		case DRM_MODE_CONNECTOR_Composite:
> +		case DRM_MODE_CONNECTOR_SVIDEO:
> +		case DRM_MODE_CONNECTOR_Component:
> +		case DRM_MODE_CONNECTOR_9PinDIN:
> +			output_type = ACPI_TV_OUTPUT;
> +			break;
> +		case DRM_MODE_CONNECTOR_DVII:
> +		case DRM_MODE_CONNECTOR_DVID:
> +		case DRM_MODE_CONNECTOR_DisplayPort:
> +		case DRM_MODE_CONNECTOR_HDMIA:
> +		case DRM_MODE_CONNECTOR_HDMIB:
> +			output_type = ACPI_DIGITAL_OUTPUT;
> +			break;
> +		case DRM_MODE_CONNECTOR_LVDS:
> +			output_type = ACPI_LVDS_OUTPUT;
> +			break;
> +		}
> +		opregion->acpi->didl[i] |= (1<<31) | output_type | i;
> +		i++;
> +	}
> +	goto end;
> +}
> +
> +void igd_opregion_init(struct opregion_dev *dev)
> +{
> +	struct igd_opregion *opregion = &dev->opregion;
> +
> +	if (!opregion->header)
> +		return;
> +
> +	if (opregion->acpi) {
> +		if (drm_core_check_feature(dev->drm_dev, DRIVER_MODESET))
> +			igd_didl_outputs(dev);
> +
> +		/* Notify BIOS we are ready to handle ACPI video ext notifs.
> +		 * Right now, all the events are handled by the ACPI video module.
> +		 * We don't actually need to do anything with them. */
> +		opregion->acpi->csts = 0;
> +		opregion->acpi->drdy = 1;
> +
> +		system_opregion = opregion;
> +		register_acpi_notifier(&igd_opregion_notifier);
> +	}
> +
> +	if (opregion->asle)
> +		dev->enable_asle(dev->drm_dev);
> +}
> +EXPORT_SYMBOL(igd_opregion_init);
> +
> +void igd_opregion_fini(struct opregion_dev *dev)
> +{
> +	struct igd_opregion *opregion = &dev->opregion;
> +
> +	if (!opregion->header)
> +		return;
> +
> +	if (opregion->acpi) {
> +		opregion->acpi->drdy = 0;
> +
> +		system_opregion = NULL;
> +		unregister_acpi_notifier(&igd_opregion_notifier);
> +	}
> +
> +	/* just clear all opregion memory pointers now */
> +	iounmap(opregion->header);
> +	opregion->header = NULL;
> +	opregion->acpi = NULL;
> +	opregion->swsci = NULL;
> +	opregion->asle = NULL;
> +	opregion->vbt = NULL;
> +}
> +EXPORT_SYMBOL(igd_opregion_fini);
> +#endif
> +
> +int igd_opregion_setup(struct opregion_dev *dev)
> +{
> +	struct igd_opregion *opregion = &dev->opregion;
> +	void *base;
> +	u32 asls, mboxes;
> +	int err = 0;
> +
> +	pci_read_config_dword(dev->drm_dev->pdev, PCI_ASLS, &asls);
> +	DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
> +	if (asls == 0) {
> +		DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
> +		return -ENOTSUPP;
> +	}
> +
> +	base = acpi_os_ioremap(asls, OPREGION_SIZE);
> +	if (!base)
> +		return -ENOMEM;
> +
> +	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
> +		DRM_DEBUG_DRIVER("opregion signature mismatch\n");
> +		err = -EINVAL;
> +		goto err_out;
> +	}
> +	opregion->header = base;
> +	opregion->vbt = base + OPREGION_VBT_OFFSET;
> +
> +	mboxes = opregion->header->mboxes;
> +	if (mboxes & MBOX_ACPI) {
> +		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
> +		opregion->acpi = base + OPREGION_ACPI_OFFSET;
> +	}
> +
> +	if (mboxes & MBOX_SWSCI) {
> +		DRM_DEBUG_DRIVER("SWSCI supported\n");
> +		opregion->swsci = base + OPREGION_SWSCI_OFFSET;
> +	}
> +	if (mboxes & MBOX_ASLE) {
> +		DRM_DEBUG_DRIVER("ASLE supported\n");
> +		opregion->asle = base + OPREGION_ASLE_OFFSET;
> +	}
> +
> +	return 0;
> +
> +err_out:
> +	iounmap(base);
> +	return err;
> +}
> +EXPORT_SYMBOL(igd_opregion_setup);
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index a6feb78c..501f4dc 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -99,6 +99,7 @@ config DRM_I915
>  	select INPUT if ACPI
>  	select ACPI_VIDEO if ACPI
>  	select ACPI_BUTTON if ACPI
> +	select ACPI_IGD_OPREGION if ACPI
>  	help
>  	  Choose this option if you have a system that has "Intel Graphics
>  	  Media Accelerator" or "HD Graphics" integrated graphics,
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0ae6a7c..776c362 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -28,7 +28,6 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
>  	  intel_dvo.o \
>  	  intel_ringbuffer.o \
>  	  intel_overlay.o \
> -	  intel_opregion.o \
>  	  dvo_ch7xxx.o \
>  	  dvo_ch7017.o \
>  	  dvo_ivch.o \
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 09e0327..e67ff81 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1129,7 +1129,7 @@ static int i915_opregion(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	struct intel_opregion *opregion = &dev_priv->opregion;
> +	struct igd_opregion *opregion = &dev_priv->opregion_dev.opregion;
>  	int ret;
>
>  	ret = mutex_lock_interruptible(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 7273037..ff057a8 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1990,7 +1990,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	/* Try to make sure MCHBAR is enabled before poking at it */
>  	intel_setup_mchbar(dev);
>  	intel_setup_gmbus(dev);
> -	intel_opregion_setup(dev);
> +
> +	if (IS_MOBILE(dev)) {
> +		dev_priv->opregion_dev.max_backlight = intel_panel_get_max_backlight(dev);
> +		dev_priv->opregion_dev.set_backlight = intel_panel_set_backlight;
> +	}
> +	dev_priv->opregion_dev.enable_asle = intel_enable_asle;
> +	dev_priv->opregion_dev.drm_dev = dev;

So we have a drm_device->drm_i915_private->opregion_dev.drm_device loop,
which is a bit messy, but consistent with other existing code.


> +	igd_opregion_setup(&dev_priv->opregion_dev);
>
>  	/* Make sure the bios did its job and set up vital registers */
>  	intel_setup_bios(dev);
> @@ -2049,7 +2056,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	}
>
>  	/* Must be done after probing outputs */
> -	intel_opregion_init(dev);
> +	igd_opregion_init(&dev_priv->opregion_dev);
>  	acpi_video_register();
>
>  	setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
> @@ -2138,7 +2145,7 @@ int i915_driver_unload(struct drm_device *dev)
>  	if (dev->pdev->msi_enabled)
>  		pci_disable_msi(dev->pdev);
>
> -	intel_opregion_fini(dev);
> +	igd_opregion_fini(&dev_priv->opregion_dev);
>
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>  		/* Flush any outstanding unpin_work. */
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c34a8dd..cf66ca8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -316,7 +316,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>
>  	i915_save_state(dev);
>
> -	intel_opregion_fini(dev);
> +	igd_opregion_fini(&dev_priv->opregion_dev);
>
>  	/* Modeset on resume, not lid events */
>  	dev_priv->modeset_on_lid = 0;
> @@ -366,7 +366,7 @@ static int i915_drm_thaw(struct drm_device *dev)
>  	}
>
>  	i915_restore_state(dev);
> -	intel_opregion_setup(dev);
> +	igd_opregion_setup(&dev_priv->opregion_dev);
>
>  	/* KMS EnterVT equivalent */
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> @@ -386,7 +386,7 @@ static int i915_drm_thaw(struct drm_device *dev)
>  			ironlake_enable_rc6(dev);
>  	}
>
> -	intel_opregion_init(dev);
> +	igd_opregion_init(&dev_priv->opregion_dev);
>
>  	dev_priv->modeset_on_lid = 0;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4496505..4d2475b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -36,6 +36,7 @@
>  #include <linux/io-mapping.h>
>  #include <linux/i2c.h>
>  #include <drm/intel-gtt.h>
> +#include <acpi/acpi_igd_opregion.h>
>
>  /* General customization:
>   */
> @@ -102,21 +103,6 @@ struct mem_block {
>  	struct drm_file *file_priv; /* NULL: free, -1: heap, other: real files */
>  };
>
> -struct opregion_header;
> -struct opregion_acpi;
> -struct opregion_swsci;
> -struct opregion_asle;
> -
> -struct intel_opregion {
> -	struct opregion_header *header;
> -	struct opregion_acpi *acpi;
> -	struct opregion_swsci *swsci;
> -	struct opregion_asle *asle;
> -	void *vbt;
> -	u32 __iomem *lid_state;
> -};
> -#define OPREGION_SIZE            (8*1024)
> -
>  struct intel_overlay;
>  struct intel_overlay_error_state;
>
> @@ -322,7 +308,7 @@ typedef struct drm_i915_private {
>  	int cfb_plane;
>  	int cfb_y;
>
> -	struct intel_opregion opregion;
> +	struct opregion_dev opregion_dev;
>
>  	/* overlay */
>  	struct intel_overlay *overlay;
> @@ -1237,22 +1223,6 @@ extern inline bool intel_gmbus_is_forced_bit(struct i2c_adapter
> *adapter)
>  }
>  extern void intel_i2c_reset(struct drm_device *dev);
>
> -/* intel_opregion.c */
> -extern int intel_opregion_setup(struct drm_device *dev);
> -#ifdef CONFIG_ACPI
> -extern void intel_opregion_init(struct drm_device *dev);
> -extern void intel_opregion_fini(struct drm_device *dev);
> -extern void intel_opregion_asle_intr(struct drm_device *dev);
> -extern void intel_opregion_gse_intr(struct drm_device *dev);
> -extern void intel_opregion_enable_asle(struct drm_device *dev);
> -#else
> -static inline void intel_opregion_init(struct drm_device *dev) { return; }
> -static inline void intel_opregion_fini(struct drm_device *dev) { return; }
> -static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
> -static inline void intel_opregion_gse_intr(struct drm_device *dev) { return; }
> -static inline void intel_opregion_enable_asle(struct drm_device *dev) { return; }
> -#endif
> -
>  /* intel_acpi.c */
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 188b497..8d9f406 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -496,7 +496,7 @@ static irqreturn_t ironlake_irq_handler(struct drm_device *dev)
>  		notify_ring(dev, &dev_priv->ring[BCS]);
>
>  	if (de_iir & DE_GSE)
> -		intel_opregion_gse_intr(dev);
> +		igd_opregion_intr(&dev_priv->opregion_dev);
>
>  	if (de_iir & DE_PLANEA_FLIP_DONE) {
>  		intel_prepare_page_flip(dev, 0);
> @@ -1203,9 +1203,8 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
>  				blc_event = true;
>  		}
>
> -
>  		if (blc_event || (iir & I915_ASLE_INTERRUPT))
> -			intel_opregion_asle_intr(dev);
> +			igd_opregion_intr(&dev_priv->opregion_dev);
>
>  		/* With MSI, interrupts are only generated when iir
>  		 * transitions from zero to nonzero.  If another bit got
> @@ -1762,7 +1761,7 @@ int i915_driver_irq_postinstall(struct drm_device *dev)
>  		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
>  	}
>
> -	intel_opregion_enable_asle(dev);
> +	igd_opregion_enable_asle(&dev_priv->opregion_dev);
>
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index fb5b4d4..5123cf9 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -611,15 +611,15 @@ intel_parse_bios(struct drm_device *dev)
>
>  	init_vbt_defaults(dev_priv);
>
> -	/* XXX Should this validation be moved to intel_opregion.c? */
> -	if (dev_priv->opregion.vbt) {
> -		struct vbt_header *vbt = dev_priv->opregion.vbt;
> +	/* XXX Should this validation be moved to acpi_igd_opregion.c? */

Should it? Seems like a good moment to do so.


> +	if (dev_priv->opregion_dev.opregion.vbt) {
> +		struct vbt_header *vbt = dev_priv->opregion_dev.opregion.vbt;
>  		if (memcmp(vbt->signature, "$VBT", 4) == 0) {
>  			DRM_DEBUG_DRIVER("Using VBT from OpRegion: %20s\n",
>  					 vbt->signature);
>  			bdb = (struct bdb_header *)((char *)vbt + vbt->bdb_offset);
>  		} else
> -			dev_priv->opregion.vbt = NULL;
> +			dev_priv->opregion_dev.opregion.vbt = NULL;
>  	}
>
>  	if (bdb == NULL) {
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 1a311ad..a499e867e 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -828,7 +828,7 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
>  		 * additional data.  Trust that if the VBT was written into
>  		 * the OpRegion then they have validated the LVDS's existence.
>  		 */
> -		if (dev_priv->opregion.vbt)
> +		if (dev_priv->opregion_dev.opregion.vbt)
>  			return true;
>  	}
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> deleted file mode 100644
> index d2c7104..0000000
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ /dev/null
> @@ -1,516 +0,0 @@
> -/*
> - * Copyright 2008 Intel Corporation <hong.liu@intel.com>
> - * Copyright 2008 Red Hat <mjg@redhat.com>
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining
> - * a copy of this software and associated documentation files (the
> - * "Software"), to deal in the Software without restriction, including
> - * without limitation the rights to use, copy, modify, merge, publish,
> - * distribute, sub license, and/or sell copies of the Software, and to
> - * permit persons to whom the Software is furnished to do so, subject to
> - * the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the
> - * next paragraph) shall be included in all copies or substantial
> - * portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
> - * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> - * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> - * SOFTWARE.
> - *
> - */
> -
> -#include <linux/acpi.h>
> -#include <linux/acpi_io.h>
> -#include <acpi/video.h>
> -
> -#include "drmP.h"
> -#include "i915_drm.h"
> -#include "i915_drv.h"
> -#include "intel_drv.h"
> -
> -#define PCI_ASLE 0xe4
> -#define PCI_ASLS 0xfc
> -
> -#define OPREGION_HEADER_OFFSET 0
> -#define OPREGION_ACPI_OFFSET   0x100
> -#define   ACPI_CLID 0x01ac /* current lid state indicator */
> -#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
> -#define OPREGION_SWSCI_OFFSET  0x200
> -#define OPREGION_ASLE_OFFSET   0x300
> -#define OPREGION_VBT_OFFSET    0x400
> -
> -#define OPREGION_SIGNATURE "IntelGraphicsMem"
> -#define MBOX_ACPI      (1<<0)
> -#define MBOX_SWSCI     (1<<1)
> -#define MBOX_ASLE      (1<<2)
> -
> -struct opregion_header {
> -       u8 signature[16];
> -       u32 size;
> -       u32 opregion_ver;
> -       u8 bios_ver[32];
> -       u8 vbios_ver[16];
> -       u8 driver_ver[16];
> -       u32 mboxes;
> -       u8 reserved[164];
> -} __attribute__((packed));
> -
> -/* OpRegion mailbox #1: public ACPI methods */
> -struct opregion_acpi {
> -       u32 drdy;       /* driver readiness */
> -       u32 csts;       /* notification status */
> -       u32 cevt;       /* current event */
> -       u8 rsvd1[20];
> -       u32 didl[8];    /* supported display devices ID list */
> -       u32 cpdl[8];    /* currently presented display list */
> -       u32 cadl[8];    /* currently active display list */
> -       u32 nadl[8];    /* next active devices list */
> -       u32 aslp;       /* ASL sleep time-out */
> -       u32 tidx;       /* toggle table index */
> -       u32 chpd;       /* current hotplug enable indicator */
> -       u32 clid;       /* current lid state*/
> -       u32 cdck;       /* current docking state */
> -       u32 sxsw;       /* Sx state resume */
> -       u32 evts;       /* ASL supported events */
> -       u32 cnot;       /* current OS notification */
> -       u32 nrdy;       /* driver status */
> -       u8 rsvd2[60];
> -} __attribute__((packed));
> -
> -/* OpRegion mailbox #2: SWSCI */
> -struct opregion_swsci {
> -       u32 scic;       /* SWSCI command|status|data */
> -       u32 parm;       /* command parameters */
> -       u32 dslp;       /* driver sleep time-out */
> -       u8 rsvd[244];
> -} __attribute__((packed));
> -
> -/* OpRegion mailbox #3: ASLE */
> -struct opregion_asle {
> -       u32 ardy;       /* driver readiness */
> -       u32 aslc;       /* ASLE interrupt command */
> -       u32 tche;       /* technology enabled indicator */
> -       u32 alsi;       /* current ALS illuminance reading */
> -       u32 bclp;       /* backlight brightness to set */
> -       u32 pfit;       /* panel fitting state */
> -       u32 cblv;       /* current brightness level */
> -       u16 bclm[20];   /* backlight level duty cycle mapping table */
> -       u32 cpfm;       /* current panel fitting mode */
> -       u32 epfm;       /* enabled panel fitting modes */
> -       u8 plut[74];    /* panel LUT and identifier */
> -       u32 pfmb;       /* PWM freq and min brightness */
> -       u8 rsvd[102];
> -} __attribute__((packed));
> -
> -/* ASLE irq request bits */
> -#define ASLE_SET_ALS_ILLUM     (1 << 0)
> -#define ASLE_SET_BACKLIGHT     (1 << 1)
> -#define ASLE_SET_PFIT          (1 << 2)
> -#define ASLE_SET_PWM_FREQ      (1 << 3)
> -#define ASLE_REQ_MSK           0xf
> -
> -/* response bits of ASLE irq request */
> -#define ASLE_ALS_ILLUM_FAILED	(1<<10)
> -#define ASLE_BACKLIGHT_FAILED	(1<<12)
> -#define ASLE_PFIT_FAILED	(1<<14)
> -#define ASLE_PWM_FREQ_FAILED	(1<<16)
> -
> -/* ASLE backlight brightness to set */
> -#define ASLE_BCLP_VALID                (1<<31)
> -#define ASLE_BCLP_MSK          (~(1<<31))
> -
> -/* ASLE panel fitting request */
> -#define ASLE_PFIT_VALID         (1<<31)
> -#define ASLE_PFIT_CENTER (1<<0)
> -#define ASLE_PFIT_STRETCH_TEXT (1<<1)
> -#define ASLE_PFIT_STRETCH_GFX (1<<2)
> -
> -/* PWM frequency and minimum brightness */
> -#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
> -#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
> -#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
> -#define ASLE_PFMB_PWM_VALID (1<<31)
> -
> -#define ASLE_CBLV_VALID         (1<<31)
> -
> -#define ACPI_OTHER_OUTPUT (0<<8)
> -#define ACPI_VGA_OUTPUT (1<<8)
> -#define ACPI_TV_OUTPUT (2<<8)
> -#define ACPI_DIGITAL_OUTPUT (3<<8)
> -#define ACPI_LVDS_OUTPUT (4<<8)
> -
> -#ifdef CONFIG_ACPI
> -static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct opregion_asle *asle = dev_priv->opregion.asle;
> -	u32 max;
> -
> -	if (!(bclp & ASLE_BCLP_VALID))
> -		return ASLE_BACKLIGHT_FAILED;
> -
> -	bclp &= ASLE_BCLP_MSK;
> -	if (bclp > 255)
> -		return ASLE_BACKLIGHT_FAILED;
> -
> -	max = intel_panel_get_max_backlight(dev);
> -	intel_panel_set_backlight(dev, bclp * max / 255);
> -	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
> -
> -	return 0;
> -}
> -
> -static u32 asle_set_als_illum(struct drm_device *dev, u32 alsi)
> -{
> -	/* alsi is the current ALS reading in lux. 0 indicates below sensor
> -	   range, 0xffff indicates above sensor range. 1-0xfffe are valid */
> -	return 0;
> -}
> -
> -static u32 asle_set_pwm_freq(struct drm_device *dev, u32 pfmb)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	if (pfmb & ASLE_PFMB_PWM_VALID) {
> -		u32 blc_pwm_ctl = I915_READ(BLC_PWM_CTL);
> -		u32 pwm = pfmb & ASLE_PFMB_PWM_MASK;
> -		blc_pwm_ctl &= BACKLIGHT_DUTY_CYCLE_MASK;
> -		pwm = pwm >> 9;
> -		/* FIXME - what do we do with the PWM? */
> -	}
> -	return 0;
> -}
> -
> -static u32 asle_set_pfit(struct drm_device *dev, u32 pfit)
> -{
> -	/* Panel fitting is currently controlled by the X code, so this is a
> -	   noop until modesetting support works fully */
> -	if (!(pfit & ASLE_PFIT_VALID))
> -		return ASLE_PFIT_FAILED;
> -	return 0;
> -}
> -
> -void intel_opregion_asle_intr(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct opregion_asle *asle = dev_priv->opregion.asle;
> -	u32 asle_stat = 0;
> -	u32 asle_req;
> -
> -	if (!asle)
> -		return;
> -
> -	asle_req = asle->aslc & ASLE_REQ_MSK;
> -
> -	if (!asle_req) {
> -		DRM_DEBUG_DRIVER("non asle set request??\n");
> -		return;
> -	}
> -
> -	if (asle_req & ASLE_SET_ALS_ILLUM)
> -		asle_stat |= asle_set_als_illum(dev, asle->alsi);
> -
> -	if (asle_req & ASLE_SET_BACKLIGHT)
> -		asle_stat |= asle_set_backlight(dev, asle->bclp);
> -
> -	if (asle_req & ASLE_SET_PFIT)
> -		asle_stat |= asle_set_pfit(dev, asle->pfit);
> -
> -	if (asle_req & ASLE_SET_PWM_FREQ)
> -		asle_stat |= asle_set_pwm_freq(dev, asle->pfmb);
> -
> -	asle->aslc = asle_stat;
> -}
> -
> -/* Only present on Ironlake+ */
> -void intel_opregion_gse_intr(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct opregion_asle *asle = dev_priv->opregion.asle;
> -	u32 asle_stat = 0;
> -	u32 asle_req;
> -
> -	if (!asle)
> -		return;
> -
> -	asle_req = asle->aslc & ASLE_REQ_MSK;
> -
> -	if (!asle_req) {
> -		DRM_DEBUG_DRIVER("non asle set request??\n");
> -		return;
> -	}
> -
> -	if (asle_req & ASLE_SET_ALS_ILLUM) {
> -		DRM_DEBUG_DRIVER("Illum is not supported\n");
> -		asle_stat |= ASLE_ALS_ILLUM_FAILED;
> -	}
> -
> -	if (asle_req & ASLE_SET_BACKLIGHT)
> -		asle_stat |= asle_set_backlight(dev, asle->bclp);
> -
> -	if (asle_req & ASLE_SET_PFIT) {
> -		DRM_DEBUG_DRIVER("Pfit is not supported\n");
> -		asle_stat |= ASLE_PFIT_FAILED;
> -	}
> -
> -	if (asle_req & ASLE_SET_PWM_FREQ) {
> -		DRM_DEBUG_DRIVER("PWM freq is not supported\n");
> -		asle_stat |= ASLE_PWM_FREQ_FAILED;
> -	}
> -
> -	asle->aslc = asle_stat;
> -}
> -#define ASLE_ALS_EN    (1<<0)
> -#define ASLE_BLC_EN    (1<<1)
> -#define ASLE_PFIT_EN   (1<<2)
> -#define ASLE_PFMB_EN   (1<<3)
> -
> -void intel_opregion_enable_asle(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct opregion_asle *asle = dev_priv->opregion.asle;
> -
> -	if (asle) {
> -		if (IS_MOBILE(dev))
> -			intel_enable_asle(dev);
> -
> -		asle->tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
> -			ASLE_PFMB_EN;
> -		asle->ardy = 1;
> -	}
> -}
> -
> -#define ACPI_EV_DISPLAY_SWITCH (1<<0)
> -#define ACPI_EV_LID            (1<<1)
> -#define ACPI_EV_DOCK           (1<<2)
> -
> -static struct intel_opregion *system_opregion;
> -
> -static int intel_opregion_video_event(struct notifier_block *nb,
> -				      unsigned long val, void *data)
> -{
> -	/* The only video events relevant to opregion are 0x80. These indicate
> -	   either a docking event, lid switch or display switch request. In
> -	   Linux, these are handled by the dock, button and video drivers.
> -	   We might want to fix the video driver to be opregion-aware in
> -	   future, but right now we just indicate to the firmware that the
> -	   request has been handled */
> -
> -	struct opregion_acpi *acpi;
> -
> -	if (!system_opregion)
> -		return NOTIFY_DONE;
> -
> -	acpi = system_opregion->acpi;
> -	acpi->csts = 0;
> -
> -	return NOTIFY_OK;
> -}
> -
> -static struct notifier_block intel_opregion_notifier = {
> -	.notifier_call = intel_opregion_video_event,
> -};
> -
> -/*
> - * Initialise the DIDL field in opregion. This passes a list of devices to
> - * the firmware. Values are defined by section B.4.2 of the ACPI specification
> - * (version 3)
> - */
> -
> -static void intel_didl_outputs(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_opregion *opregion = &dev_priv->opregion;
> -	struct drm_connector *connector;
> -	acpi_handle handle;
> -	struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL;
> -	unsigned long long device_id;
> -	acpi_status status;
> -	int i = 0;
> -
> -	handle = DEVICE_ACPI_HANDLE(&dev->pdev->dev);
> -	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev)))
> -		return;
> -
> -	if (acpi_is_video_device(acpi_dev))
> -		acpi_video_bus = acpi_dev;
> -	else {
> -		list_for_each_entry(acpi_cdev, &acpi_dev->children, node) {
> -			if (acpi_is_video_device(acpi_cdev)) {
> -				acpi_video_bus = acpi_cdev;
> -				break;
> -			}
> -		}
> -	}
> -
> -	if (!acpi_video_bus) {
> -		printk(KERN_WARNING "No ACPI video bus found\n");
> -		return;
> -	}
> -
> -	list_for_each_entry(acpi_cdev, &acpi_video_bus->children, node) {
> -		if (i >= 8) {
> -			dev_printk (KERN_ERR, &dev->pdev->dev,
> -				    "More than 8 outputs detected\n");
> -			return;
> -		}
> -		status =
> -			acpi_evaluate_integer(acpi_cdev->handle, "_ADR",
> -						NULL, &device_id);
> -		if (ACPI_SUCCESS(status)) {
> -			if (!device_id)
> -				goto blind_set;
> -			opregion->acpi->didl[i] = (u32)(device_id & 0x0f0f);
> -			i++;
> -		}
> -	}
> -
> -end:
> -	/* If fewer than 8 outputs, the list must be null terminated */
> -	if (i < 8)
> -		opregion->acpi->didl[i] = 0;
> -	return;
> -
> -blind_set:
> -	i = 0;
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> -		int output_type = ACPI_OTHER_OUTPUT;
> -		if (i >= 8) {
> -			dev_printk (KERN_ERR, &dev->pdev->dev,
> -				    "More than 8 outputs detected\n");
> -			return;
> -		}
> -		switch (connector->connector_type) {
> -		case DRM_MODE_CONNECTOR_VGA:
> -		case DRM_MODE_CONNECTOR_DVIA:
> -			output_type = ACPI_VGA_OUTPUT;
> -			break;
> -		case DRM_MODE_CONNECTOR_Composite:
> -		case DRM_MODE_CONNECTOR_SVIDEO:
> -		case DRM_MODE_CONNECTOR_Component:
> -		case DRM_MODE_CONNECTOR_9PinDIN:
> -			output_type = ACPI_TV_OUTPUT;
> -			break;
> -		case DRM_MODE_CONNECTOR_DVII:
> -		case DRM_MODE_CONNECTOR_DVID:
> -		case DRM_MODE_CONNECTOR_DisplayPort:
> -		case DRM_MODE_CONNECTOR_HDMIA:
> -		case DRM_MODE_CONNECTOR_HDMIB:
> -			output_type = ACPI_DIGITAL_OUTPUT;
> -			break;
> -		case DRM_MODE_CONNECTOR_LVDS:
> -			output_type = ACPI_LVDS_OUTPUT;
> -			break;
> -		}
> -		opregion->acpi->didl[i] |= (1<<31) | output_type | i;
> -		i++;
> -	}
> -	goto end;
> -}
> -
> -void intel_opregion_init(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_opregion *opregion = &dev_priv->opregion;
> -
> -	if (!opregion->header)
> -		return;
> -
> -	if (opregion->acpi) {
> -		if (drm_core_check_feature(dev, DRIVER_MODESET))
> -			intel_didl_outputs(dev);
> -
> -		/* Notify BIOS we are ready to handle ACPI video ext notifs.
> -		 * Right now, all the events are handled by the ACPI video module.
> -		 * We don't actually need to do anything with them. */
> -		opregion->acpi->csts = 0;
> -		opregion->acpi->drdy = 1;
> -
> -		system_opregion = opregion;
> -		register_acpi_notifier(&intel_opregion_notifier);
> -	}
> -
> -	if (opregion->asle)
> -		intel_opregion_enable_asle(dev);
> -}
> -
> -void intel_opregion_fini(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_opregion *opregion = &dev_priv->opregion;
> -
> -	if (!opregion->header)
> -		return;
> -
> -	if (opregion->acpi) {
> -		opregion->acpi->drdy = 0;
> -
> -		system_opregion = NULL;
> -		unregister_acpi_notifier(&intel_opregion_notifier);
> -	}
> -
> -	/* just clear all opregion memory pointers now */
> -	iounmap(opregion->header);
> -	opregion->header = NULL;
> -	opregion->acpi = NULL;
> -	opregion->swsci = NULL;
> -	opregion->asle = NULL;
> -	opregion->vbt = NULL;
> -}
> -#endif
> -
> -int intel_opregion_setup(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_opregion *opregion = &dev_priv->opregion;
> -	void *base;
> -	u32 asls, mboxes;
> -	int err = 0;
> -
> -	pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
> -	DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
> -	if (asls == 0) {
> -		DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
> -		return -ENOTSUPP;
> -	}
> -
> -	base = acpi_os_ioremap(asls, OPREGION_SIZE);
> -	if (!base)
> -		return -ENOMEM;
> -
> -	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
> -		DRM_DEBUG_DRIVER("opregion signature mismatch\n");
> -		err = -EINVAL;
> -		goto err_out;
> -	}
> -	opregion->header = base;
> -	opregion->vbt = base + OPREGION_VBT_OFFSET;
> -
> -	opregion->lid_state = base + ACPI_CLID;
> -
> -	mboxes = opregion->header->mboxes;
> -	if (mboxes & MBOX_ACPI) {
> -		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
> -		opregion->acpi = base + OPREGION_ACPI_OFFSET;
> -	}
> -
> -	if (mboxes & MBOX_SWSCI) {
> -		DRM_DEBUG_DRIVER("SWSCI supported\n");
> -		opregion->swsci = base + OPREGION_SWSCI_OFFSET;
> -	}
> -	if (mboxes & MBOX_ASLE) {
> -		DRM_DEBUG_DRIVER("ASLE supported\n");
> -		opregion->asle = base + OPREGION_ASLE_OFFSET;
> -	}
> -
> -	return 0;
> -
> -err_out:
> -	iounmap(base);
> -	return err;
> -}
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 4e5ff59..51565bb 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -261,8 +261,8 @@ intel_panel_detect(struct drm_device *dev)
>  	 * appears to be either the BIOS or Linux ACPI fault */
>  #if 0
>  	/* Assume that the BIOS does not lie through the OpRegion... */
> -	if (dev_priv->opregion.lid_state)
> -		return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
> +	if (dev_priv->opregion_dev.opregion.acpi)
> +		return ioread32(&dev_priv->opregion_dev.opregion.acpi->clid) & 0x1 ?

What guarantees that opregion.acpi != NULL here?

Or perhaps just remove that #if 0 code chunk altogether?

>  			connector_status_connected :
>  			connector_status_disconnected;
>  #endif
> diff --git a/include/acpi/acpi_igd_opregion.h b/include/acpi/acpi_igd_opregion.h
> new file mode 100644
> index 0000000..36fefd2
> --- /dev/null
> +++ b/include/acpi/acpi_igd_opregion.h
> @@ -0,0 +1,105 @@
> +#ifndef __ACPI_IGD_OPREGION_H__
> +#define __ACPI_IGD_OPREGION_H__
> +
> +#include <drm/drmP.h>
> +
> +#define OPREGION_SIZE            (8*1024)
> +
> +struct opregion_header {
> +	u8 signature[16];
> +	u32 size;
> +	u32 opregion_ver;
> +	u8 bios_ver[32];
> +	u8 vbios_ver[16];
> +	u8 driver_ver[16];
> +	u32 mboxes;
> +	u8 reserved[164];
> +} __packed;
> +
> +/* OpRegion mailbox #1: public ACPI methods */
> +struct opregion_acpi {
> +	u32 drdy;       /* driver readiness */
> +	u32 csts;       /* notification status */
> +	u32 cevt;       /* current event */
> +	u8 rsvd1[20];
> +	u32 didl[8];    /* supported display devices ID list */
> +	u32 cpdl[8];    /* currently presented display list */
> +	u32 cadl[8];    /* currently active display list */
> +	u32 nadl[8];    /* next active devices list */
> +	u32 aslp;       /* ASL sleep time-out */
> +	u32 tidx;       /* toggle table index */
> +	u32 chpd;       /* current hotplug enable indicator */
> +	u32 clid;       /* current lid state*/
> +	u32 cdck;       /* current docking state */
> +	u32 sxsw;       /* Sx state resume */
> +	u32 evts;       /* ASL supported events */
> +	u32 cnot;       /* current OS notification */
> +	u32 nrdy;       /* driver status */
> +	u8 rsvd2[60];
> +} __packed;
> +
> +/* OpRegion mailbox #2: SWSCI */
> +struct opregion_swsci {
> +	u32 scic;       /* SWSCI command|status|data */
> +	u32 parm;       /* command parameters */
> +	u32 dslp;       /* driver sleep time-out */
> +	u8 rsvd[244];
> +} __packed;
> +
> +/* OpRegion mailbox #3: ASLE */
> +struct opregion_asle {
> +	u32 ardy;       /* driver readiness */
> +	u32 aslc;       /* ASLE interrupt command */
> +	u32 tche;       /* technology enabled indicator */
> +	u32 alsi;       /* current ALS illuminance reading */
> +	u32 bclp;       /* backlight brightness to set */
> +	u32 pfit;       /* panel fitting state */
> +	u32 cblv;       /* current brightness level */
> +	u16 bclm[20];   /* backlight level duty cycle mapping table */
> +	u32 cpfm;       /* current panel fitting mode */
> +	u32 epfm;       /* enabled panel fitting modes */
> +	u8 plut[74];    /* panel LUT and identifier */
> +	u32 pfmb;       /* PWM freq and min brightness */
> +	u8 rsvd[102];
> +} __packed;
> +
> +struct igd_opregion {
> +	struct opregion_header *header;
> +	struct opregion_acpi *acpi;
> +	struct opregion_swsci *swsci;
> +	struct opregion_asle *asle;
> +	void *vbt;
> +};
> +
> +struct opregion_dev {
> +	struct drm_device *drm_dev;
> +	u32 max_backlight;
> +	void (*set_backlight)(struct drm_device *dev, u32 value);
> +	void (*set_als_illum)(struct drm_device *dev, u32 alsi);
> +	void (*set_pwm_freq)(struct drm_device *dev, u32 pfmb);

This gives the impression that systems using ASLE always run in combination
mode, with LBPC used for backlight setting, and ASLE_SET_PWM_FREQ used to
let the driver change BLC_PWM_CTL.

Current driver code doesn't quite follow the Intel documentation, e.g. it
never sets aslp and doesn't use bclm for backlight setting (but it's very
awkward to use).

> +	void (*set_pfit)(struct drm_device *dev, u32 pfit);
> +	void (*enable_asle)(struct drm_device *drm_dev);
> +	struct igd_opregion opregion;
> +};
> +
> +#if (defined CONFIG_ACPI_IGD_OPREGION || defined CONFIG_ACPI_IGD_OPREGION_MODULE)
> +
> +int igd_opregion_setup(struct opregion_dev *dev);
> +void igd_opregion_init(struct opregion_dev *dev);
> +void igd_opregion_fini(struct opregion_dev *dev);
> +void igd_opregion_intr(struct opregion_dev *dev);
> +void igd_opregion_enable_asle(struct opregion_dev *dev);
> +
> +#else
> +
> +static inline int igd_opregion_setup(struct opregion_dev *dev)
> +{
> +	return 0;
> +}
> +
> +static inline void igd_opregion_init(struct opregion_dev *dev) {};
> +static inline void igd_opregion_fini(struct opregion_dev *dev) {};
> +static inline void igd_opregion_intr(struct opregion_dev *dev) {};
> +static inline void igd_opregion_enable_asle(struct opregion_dev *dev) {};
> +#endif
> +#endif
> --
> 1.7.4.1
>

About the naming, as this is Intel-only intel_opregion seems clearer than
igd-opregion, which sounds like it could be anything.

Greetings,

Indan

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Matthew Garrett]

On Tue, Mar 15, 2011 at 02:18:02AM +0100, Indan Zupancic wrote:
> > +
> > +	if (dev->set_backlight)
> > +		dev->set_backlight(dev->drm_dev, bclp * max / 255);
> 
> I would hide the max backlight from the opregion code and move this
> calculation into set_brightness. Then change the interface to one
> based on a scale from 0-255 or 0-100, which fits better with what
> opregion actually does. On the other hand, it's the same code in
> a different place, so it doesn't make much difference.

That would require an extra layer of indirection in every driver, so 
keeping it here seems reasonable.

> > +void igd_opregion_enable_asle(struct opregion_dev *dev)
> > +{
> > +	struct opregion_asle *asle = dev->opregion.asle;
> > +
> > +	if (asle && dev->enable_asle) {
> > +		dev->enable_asle(dev->drm_dev);
> > +
> > +		asle->tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
> > +			ASLE_PFMB_EN;
> 
> Shouldn't which flags are set depend on which callback functions are set?
> Then you can remove all the function != NULL tests too. e.g:

Our experience is that we should give BIOSes the full set of support 
flags even if we don't actually do anything with them. That way we avoid 
cases where BIOS authors refuse to do anything with partial 
implementations (even if we support everything they use) and we get 
feedback when we actually find cases of advanced functionality in the 
real world.

> > +	if (IS_MOBILE(dev)) {
> > +		dev_priv->opregion_dev.max_backlight = intel_panel_get_max_backlight(dev);
> > +		dev_priv->opregion_dev.set_backlight = intel_panel_set_backlight;
> > +	}
> > +	dev_priv->opregion_dev.enable_asle = intel_enable_asle;
> > +	dev_priv->opregion_dev.drm_dev = dev;
> 
> So we have a drm_device->drm_i915_private->opregion_dev.drm_device loop,
> which is a bit messy, but consistent with other existing code.

Anyone who wants to clean that up is welcome to do so, but really I 
optimised for ease of transition here.

> >
> > -	/* XXX Should this validation be moved to intel_opregion.c? */
> > -	if (dev_priv->opregion.vbt) {
> > -		struct vbt_header *vbt = dev_priv->opregion.vbt;
> > +	/* XXX Should this validation be moved to acpi_igd_opregion.c? */
> 
> Should it? Seems like a good moment to do so.

Now that we've got multiple consumers it's probably not helpful to move 
the (potentially chip-specific) VBT handling to general code. We've got 
zero documentation on how GMA500 handles VBT, and not a great deal more 
for i915.

> About the naming, as this is Intel-only intel_opregion seems clearer than
> igd-opregion, which sounds like it could be anything.

It's Intel-only in implementation, not in specification. As much as I 
dislike vendor-neutral specs that have been pushed and implemented by a 
single vendor, I'd rather not make the naming Intel-specific if there's 
a chance someone else can end up depending on it.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [Intel-gfx] [PATCH] ACPI/Intel: Rework Opregion support

[Olivier Galibert]

On Tue, Mar 15, 2011 at 01:52:26AM +0000, Matthew Garrett wrote:
> Now that we've got multiple consumers it's probably not helpful to move 
> the (potentially chip-specific) VBT handling to general code. We've got 
> zero documentation on how GMA500 handles VBT, and not a great deal more 
> for i915.

I'm not sure what you mean by "handles".  If you mean find it, the
gma500 driver code reads a 32-bit pointer to the opregion at offset
0xfc in the pci configuration space.  If you mean parse it, I haven't
seen anything that looked different to what the current kernel code
parses.

  OG.

Re: [Intel-gfx] [PATCH] ACPI/Intel: Rework Opregion support

[Matthew Garrett]

On Tue, Mar 15, 2011 at 02:30:55PM +0100, Olivier Galibert wrote:
> On Tue, Mar 15, 2011 at 01:52:26AM +0000, Matthew Garrett wrote:
> > Now that we've got multiple consumers it's probably not helpful to move 
> > the (potentially chip-specific) VBT handling to general code. We've got 
> > zero documentation on how GMA500 handles VBT, and not a great deal more 
> > for i915.
> 
> I'm not sure what you mean by "handles".  If you mean find it, the
> gma500 driver code reads a 32-bit pointer to the opregion at offset
> 0xfc in the pci configuration space.  If you mean parse it, I haven't
> seen anything that looked different to what the current kernel code
> parses.

Opregion is one mechanism to provide VBT - it doesn't define it.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [Intel-gfx] [PATCH] ACPI/Intel: Rework Opregion support

[Olivier Galibert]

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

On Tue, Mar 15, 2011 at 01:32:40PM +0000, Matthew Garrett wrote:
> Opregion is one mechanism to provide VBT - it doesn't define it.

Then let me repeat that I haven't seen anything in the VBT tables of
the gma500-using netbook I have that didn't seem to be parsed
correctly by the current gpu/drm/i915/intel_bios.c code and its
friends.  Have a look at it if you want.

  OG.


[-- Attachment #2: asl.bin --]
[-- Type: application/octet-stream, Size: 8192 bytes --]

Re: [Intel-gfx] [PATCH] ACPI/Intel: Rework Opregion support

[Matthew Garrett]

On Tue, Mar 15, 2011 at 02:40:59PM +0100, Olivier Galibert wrote:
> On Tue, Mar 15, 2011 at 01:32:40PM +0000, Matthew Garrett wrote:
> > Opregion is one mechanism to provide VBT - it doesn't define it.
> 
> Then let me repeat that I haven't seen anything in the VBT tables of
> the gma500-using netbook I have that didn't seem to be parsed
> correctly by the current gpu/drm/i915/intel_bios.c code and its
> friends.  Have a look at it if you want.

gma600 *certainly* has a bunch of additional functionality. It may be 
that this is code that can be split out in a reasonable manner, but 
there's been sufficient fragility here that I think doing it at this 
stage for the benefit of a driver in staging doesn't buy us a great 
deal.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Chris Wilson]

On Tue, 15 Mar 2011 02:18:02 +0100 (CET), "Indan Zupancic" <indan@nul.nu> wrote:
> Hello,
> 
> Some nitpicks below.
> 
> On Mon, March 14, 2011 18:59, Chris Wilson wrote:
> > Note: neither the opregion_dev interface or the alse_set_* properly report
> > failures. As such we have a slight change in behaviour on Ironlake+
> > platforms and an uncorrected bug for earlier chipsets.
> > -Chris
> 
> What uncorrected bug?

For later chipsets we currently report the failure to respond to the ALSE
requests, for earlier we do not. The patch harmonises the two code paths
reusing the earlier code for later chipsets, hence the change in behaviour
and potential regression. Alternatively, actually reporting the failure
for earlier chipsets may also break existing setups.

> And are there earlier chipsets with ASLE support at all, besides gen 4?
> If there are no gen 2 or gen 3 chipsets with ASLE then the backlight
> code can be simplified further.

The OpRegion interface was devised midway through gen3 (afaik), and you
find it on some i915-class hw and not others. In theory, there is nothing
to prevent a BIOS/ACPI from being rewritten for it to be of use in gen2,
and who knows one such beast may already exist (considering much to our
horror you can still buy gen2 chipsets).

> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 4e5ff59..51565bb 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -261,8 +261,8 @@ intel_panel_detect(struct drm_device *dev)
> >  	 * appears to be either the BIOS or Linux ACPI fault */
> >  #if 0
> >  	/* Assume that the BIOS does not lie through the OpRegion... */
> > -	if (dev_priv->opregion.lid_state)
> > -		return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
> > +	if (dev_priv->opregion_dev.opregion.acpi)
> > +		return ioread32(&dev_priv->opregion_dev.opregion.acpi->clid) & 0x1 ?
> 
> What guarantees that opregion.acpi != NULL here?

You mean other than the explicit test for opregion.acpi != NULL?

> Or perhaps just remove that #if 0 code chunk altogether?

Read the changelog and thread on the patch that disabled this logic, the
failure (or at least inconsistent behaviour with the expectations of the
HP BIOS authors) appears to be in how we initialise ACPI on the HP
machines that causes the initial value of lid state to be incorrect. Since
one of the laptops that Dave tests drm-next on is a HP, he was bitten by
the bug and temporarily (we hope) disabled the logic. Or else once again,
we will continue to light up the panel on a closed laptop.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Indan Zupancic]

On Tue, March 15, 2011 09:37, Chris Wilson wrote:
> On Tue, 15 Mar 2011 02:18:02 +0100 (CET), "Indan Zupancic" <indan@nul.nu> wrote:
>> Hello,
>>
>> Some nitpicks below.
>>
>> On Mon, March 14, 2011 18:59, Chris Wilson wrote:
>> > Note: neither the opregion_dev interface or the alse_set_* properly report
>> > failures. As such we have a slight change in behaviour on Ironlake+
>> > platforms and an uncorrected bug for earlier chipsets.
>> > -Chris
>>
>> What uncorrected bug?
>
> For later chipsets we currently report the failure to respond to the ALSE
> requests, for earlier we do not. The patch harmonises the two code paths
> reusing the earlier code for later chipsets, hence the change in behaviour
> and potential regression. Alternatively, actually reporting the failure
> for earlier chipsets may also break existing setups.

Ah, okay, for the ASLE_SET_ALS_ILLUM/ASLE_SET_PFIT/ASLE_SET_PWM_FREQ cases.
Hopefully this change doesn't cause regressions, that would be sad.

>> And are there earlier chipsets with ASLE support at all, besides gen 4?
>> If there are no gen 2 or gen 3 chipsets with ASLE then the backlight
>> code can be simplified further.
>
> The OpRegion interface was devised midway through gen3 (afaik), and you
> find it on some i915-class hw and not others. In theory, there is nothing
> to prevent a BIOS/ACPI from being rewritten for it to be of use in gen2,
> and who knows one such beast may already exist (considering much to our
> horror you can still buy gen2 chipsets).

Pity, if they're still sold any bets are off.

>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> > index 4e5ff59..51565bb 100644
>> > --- a/drivers/gpu/drm/i915/intel_panel.c
>> > +++ b/drivers/gpu/drm/i915/intel_panel.c
>> > @@ -261,8 +261,8 @@ intel_panel_detect(struct drm_device *dev)
>> >  	 * appears to be either the BIOS or Linux ACPI fault */
>> >  #if 0
>> >  	/* Assume that the BIOS does not lie through the OpRegion... */
>> > -	if (dev_priv->opregion.lid_state)
>> > -		return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
>> > +	if (dev_priv->opregion_dev.opregion.acpi)
>> > +		return ioread32(&dev_priv->opregion_dev.opregion.acpi->clid) & 0x1 ?
>>
>> What guarantees that opregion.acpi != NULL here?
>
> You mean other than the explicit test for opregion.acpi != NULL?

I'm blind. I checked all the rest of the code, but not the line just above it.
Gah!

>> Or perhaps just remove that #if 0 code chunk altogether?
>
> Read the changelog and thread on the patch that disabled this logic, the

I just subscribed to intel-gfx, seemed like a good idea after the reject.

> failure (or at least inconsistent behaviour with the expectations of the
> HP BIOS authors) appears to be in how we initialise ACPI on the HP
> machines that causes the initial value of lid state to be incorrect. Since
> one of the laptops that Dave tests drm-next on is a HP, he was bitten by
> the bug and temporarily (we hope) disabled the logic. Or else once again,
> we will continue to light up the panel on a closed laptop.

Hopefully it's fixed by the next BIOS upgrade by HP...

Everything would be a lot simpler if the BIOSes were open source.

It's shocking with what you guys have to deal with.

Good luck and thanks for all the hard work!

Indan

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Peter Stuge]

Indan Zupancic wrote:
> Everything would be a lot simpler if the BIOSes were open source.

coreboot has existed for about eleven years and some 250 mainboards of
varying shapes and sizes (from laptop to server) are supported, but it's
only just recently that things are really taking off, with the code
release from AMD to initialize their most recent Fusion platform.

http://www.coreboot.org/Welcome_to_coreboot
http://blogs.amd.com/work/2011/02/28/amd-coreboot/
http://news.slashdot.org/story/11/03/04/1736241/AMD-Provides-Fusion-Support-For-Coreboot


//Peter

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Indan Zupancic]

On Tue, March 15, 2011 12:27, Peter Stuge wrote:
> coreboot has existed for about eleven years and some 250 mainboards of
> varying shapes and sizes (from laptop to server) are supported, but it's

I've been wanting to get rid of BIOSes and use Coreboot for ages,
but the amount of hassle needed to get it working for my hardware
and the lack of features (suspend), as well the chance of bricking
the system always put me off.

> only just recently that things are really taking off, with the code
> release from AMD to initialize their most recent Fusion platform.

They don't give their Linux devs any Fusion hardware, nor do they
open the UVD spec, but at least they release info like this.

Greetings,

Indan

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Alex Deucher]

On Tue, Mar 15, 2011 at 7:46 AM, Indan Zupancic <indan@nul.nu> wrote:
> On Tue, March 15, 2011 12:27, Peter Stuge wrote:
>> coreboot has existed for about eleven years and some 250 mainboards of
>> varying shapes and sizes (from laptop to server) are supported, but it's
>
> I've been wanting to get rid of BIOSes and use Coreboot for ages,
> but the amount of hassle needed to get it working for my hardware
> and the lack of features (suspend), as well the chance of bricking
> the system always put me off.
>
>> only just recently that things are really taking off, with the code
>> release from AMD to initialize their most recent Fusion platform.
>
> They don't give their Linux devs any Fusion hardware, nor do they
> open the UVD spec, but at least they release info like this.
>

They do give us fusion hw; before launch even.  That's why we had
Linux support before hw was available publicly.  My board just
happened to get bricked recently during a failed bios upgrade.  A new
one is on the way.  Also we are looking into a potential release of
UVD, but unfortunately, our decode hw is intimately tied in with our
drm implementation and if someone managed to use the released
information to compromise the drm in our windows driver it would very
negatively impact our ability to sell into the windows market and
would probably get the open source graphics initiative shut down.

Alex

> Greetings,
>
> Indan
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Indan Zupancic]

On Tue, March 15, 2011 17:06, Alex Deucher wrote:
> On Tue, Mar 15, 2011 at 7:46 AM, Indan Zupancic <indan@nul.nu> wrote:
>> They don't give their Linux devs any Fusion hardware, nor do they
>> open the UVD spec, but at least they release info like this.
>
> They do give us fusion hw; before launch even.  That's why we had
> Linux support before hw was available publicly.  My board just
> happened to get bricked recently during a failed bios upgrade.
> A new one is on the way.

Okay, that's better than I thought. I remember a dev saying that
no one had Fusion hardware, that's where I got this notion from.

> Also we are looking into a potential release of
> UVD, but unfortunately, our decode hw is intimately tied in with
> our drm implementation and if someone managed to use the released
> information to compromise the drm in our windows driver it would very
> negatively impact our ability to sell into the windows market and
> would probably get the open source graphics initiative shut down.

Are you talking about HDCP or something else? Because the HDCP master
key already leaked, so the whole security aspect of it is already a
joke, open source UVD won't make any difference.

Basically you're telling me that I or someone else should reverse
engineer de Catalyst driver and break all drm before you consider
opening up UVD? I'd argue that opening up UVD now is more secure
because it takes away the only morivation to break UVD's drm.

Alternatively, can't you open up UVD spec except the drm bits,
so people can at least write their own UVD driver to watch
unencrypted data?

It would be nice to have an open source Fusion based media player/
IPTV decoder, but I guess that's hoping for too much.

I understand if AMD/ATI wants to keep its 3D driver secret, but
hardware video decoding?! If you have to keep it secret it means
shortcuts were taken and it's all insecure anyway. That if it gets
broken the Open source driver gets blamed is ridiculous and more
politics than anything else.

This is getting more and more off-topic though, sorry for the noise.

Greetings,

Indan

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Alex Deucher]

On Tue, Mar 15, 2011 at 8:02 PM, Indan Zupancic <indan@nul.nu> wrote:
> On Tue, March 15, 2011 17:06, Alex Deucher wrote:
>> On Tue, Mar 15, 2011 at 7:46 AM, Indan Zupancic <indan@nul.nu> wrote:
>>> They don't give their Linux devs any Fusion hardware, nor do they
>>> open the UVD spec, but at least they release info like this.
>>
>> They do give us fusion hw; before launch even.  That's why we had
>> Linux support before hw was available publicly.  My board just
>> happened to get bricked recently during a failed bios upgrade.
>> A new one is on the way.
>
> Okay, that's better than I thought. I remember a dev saying that
> no one had Fusion hardware, that's where I got this notion from.
>
>> Also we are looking into a potential release of
>> UVD, but unfortunately, our decode hw is intimately tied in with
>> our drm implementation and if someone managed to use the released
>> information to compromise the drm in our windows driver it would very
>> negatively impact our ability to sell into the windows market and
>> would probably get the open source graphics initiative shut down.
>
> Are you talking about HDCP or something else? Because the HDCP master
> key already leaked, so the whole security aspect of it is already a
> joke, open source UVD won't make any difference.
>

It's not HDCP, encrypted bluray is the main issue.  And while there
are hacks for bluray around already, contractual obligations don't
care whether existing hacks are available or not.

> Basically you're telling me that I or someone else should reverse
> engineer de Catalyst driver and break all drm before you consider
> opening up UVD? I'd argue that opening up UVD now is more secure
> because it takes away the only morivation to break UVD's drm.
>
> Alternatively, can't you open up UVD spec except the drm bits,
> so people can at least write their own UVD driver to watch
> unencrypted data?

Without going into detail, they are very intertwined.  The hw was
designed long before we planned to support open source as much as we
are now.  Fortunately, we have been working with our hw teams to make
future UVD implementations take the open source driver into account.

>
> It would be nice to have an open source Fusion based media player/
> IPTV decoder, but I guess that's hoping for too much.
>
> I understand if AMD/ATI wants to keep its 3D driver secret, but
> hardware video decoding?! If you have to keep it secret it means
> shortcuts were taken and it's all insecure anyway. That if it gets
> broken the Open source driver gets blamed is ridiculous and more
> politics than anything else.

We don't keep the 3D engine secret.  Most of the code we've written
and specs we've released are 3D engine stuff.  Fortunately 3D is less
tied up in drm compared to video.  It's all about mitigating risk.
Imagine you run a company and someone asks you this:  "Can we release
programming info for a part of our chip to support 2-3% of our market
that may put at risk 90% of our market?"  What would you do?  The
reason the review is taking so long is that we want to be real sure
that the risk is safe.

>
> This is getting more and more off-topic though, sorry for the noise.
>

Agreed.

Alex

> Greetings,
>
> Indan
>
>
>

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Indan Zupancic]

On Wed, March 16, 2011 03:17, Alex Deucher wrote:
> It's not HDCP, encrypted bluray is the main issue.  And while
> there are hacks for bluray around already, contractual obligations
> don't care whether existing hacks are available or not.

So the contract says to keep it secret, not to make it secure.
Wonderful.

> Without going into detail, they are very intertwined.  The hw was
> designed long before we planned to support open source as much as we
> are now.  Fortunately, we have been working with our hw teams to make
> future UVD implementations take the open source driver into account.

It has nothing to do with open source, if you need to trust the
driver you're already screwed from a security point of view.

>>
>> It would be nice to have an open source Fusion based media player/
>> IPTV decoder, but I guess that's hoping for too much.
>>
>> I understand if AMD/ATI wants to keep its 3D driver secret, but
>> hardware video decoding?! If you have to keep it secret it means
>> shortcuts were taken and it's all insecure anyway. That if it gets
>> broken the Open source driver gets blamed is ridiculous and more
>> politics than anything else.
>
> We don't keep the 3D engine secret.  Most of the code we've written
> and specs we've released are 3D engine stuff.  Fortunately 3D is less
> tied up in drm compared to video.

That's not what I said, I was talking about the driver. There are always
details not documented, and plenty of optimisation tricks that make the
hardware go fast. Just compare the performance of the Windows driver
with the open source Linux driver. It doesn't give the impression that
AMD is putting much effort in the open source drivers or that it takes
it seriously.

> It's all about mitigating risk.
> Imagine you run a company and someone asks you this:  "Can we release
> programming info for a part of our chip to support 2-3% of our market
> that may put at risk 90% of our market?"  What would you do?  The
> reason the review is taking so long is that we want to be real sure
> that the risk is safe.

That's what AMD doesn't get, it only has 2-3% because its drivers
aren't great. If AMD had good performing open source drivers they
would not only get a bigger market share, their hardware would also
be used more for embedded kind of things like decoders, media boxes
and who knows what.

The risk you're talking about is how to best hide that the hardware
is totally insecure as far as the drm goes. Well no, you wouldn't
want to make that public indeed. It's probably the crazy drm's that
more or less forced AMD to implement it this way, but still.

>>
>> This is getting more and more off-topic though, sorry for the noise.
>
> Agreed.

Last post from me about this...

Greetings,

Indan

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Jerome Glisse]

On Wed, Mar 16, 2011 at 2:26 AM, Indan Zupancic <indan@nul.nu> wrote:
> On Wed, March 16, 2011 03:17, Alex Deucher wrote:
>> It's not HDCP, encrypted bluray is the main issue.  And while
>> there are hacks for bluray around already, contractual obligations
>> don't care whether existing hacks are available or not.
>
> So the contract says to keep it secret, not to make it secure.
> Wonderful.
>
>> Without going into detail, they are very intertwined.  The hw was
>> designed long before we planned to support open source as much as we
>> are now.  Fortunately, we have been working with our hw teams to make
>> future UVD implementations take the open source driver into account.
>
> It has nothing to do with open source, if you need to trust the
> driver you're already screwed from a security point of view.
>
>>>
>>> It would be nice to have an open source Fusion based media player/
>>> IPTV decoder, but I guess that's hoping for too much.
>>>
>>> I understand if AMD/ATI wants to keep its 3D driver secret, but
>>> hardware video decoding?! If you have to keep it secret it means
>>> shortcuts were taken and it's all insecure anyway. That if it gets
>>> broken the Open source driver gets blamed is ridiculous and more
>>> politics than anything else.
>>
>> We don't keep the 3D engine secret.  Most of the code we've written
>> and specs we've released are 3D engine stuff.  Fortunately 3D is less
>> tied up in drm compared to video.
>
> That's not what I said, I was talking about the driver. There are always
> details not documented, and plenty of optimisation tricks that make the
> hardware go fast. Just compare the performance of the Windows driver
> with the open source Linux driver. It doesn't give the impression that
> AMD is putting much effort in the open source drivers or that it takes
> it seriously.

People are over thinking and believe secret recipes is what actually
make driver fast. From my experience i would say that it's just the
open source driver stack that is limiting performances, mostly because
it require a huge amount of work. I believe there is several hundred
of engineers working on the closed driver just to optimize it and
improve it while the open source stacks have couple dozen and not all
working on same hw but working on AMD, Intel, NVidia.

You can check that with some of the test that were in r600 demo, iirc
correctly with direct hw access perf were close to theoretical hw
performance.

Cheers,
Jerome

Re: [PATCH] ACPI/Intel: Rework Opregion support

[Dave Airlie]

>
> Read the changelog and thread on the patch that disabled this logic, the
> failure (or at least inconsistent behaviour with the expectations of the
> HP BIOS authors) appears to be in how we initialise ACPI on the HP
> machines that causes the initial value of lid state to be incorrect. Since
> one of the laptops that Dave tests drm-next on is a HP, he was bitten by
> the bug and temporarily (we hope) disabled the logic. Or else once again,
> we will continue to light up the panel on a closed laptop.

Yeah I've no idea if the ACPI implementation is wrong or not :-(

I suspects its the BIOS actually, the BIOS wants _INI called before _REG.
The ACPI video device isn't in the EC address space, so it ends up in the
default region address space. So we'd really need to know how Windows
sets up the _REG calls for the non-EC spaces and where its calls the _INI
methods wrt to that.

The thing is I think the actual ACPI lid device is fine, its just opregion isn't
updated until we get an event later.

Dave.