On Thu, 2009-03-19 at 21:35 +0000, Matthew Garrett wrote:
> [ACPI] Populate DIDL before registering ACPI video device on Intel
>
> Intel graphics hardware that implements the ACPI IGD OpRegion spec
> requires that the list of display devices be populated before any ACPI
> video methods are called. Detect when this is the case and defer
> registration until the opregion code calls it. Fixes crashes on HP
> laptops as seen in kernel bugzilla #11259.
>
> Signed-off-by: Matthew Garrett <mjg@xxxxxxxxxx>
As far as DRM changes,
Acked-by: Eric Anholt <eric@xxxxxxxxxx>
(I'm assuming this'll go through the ACPI tree)
> ---
>
> This version fixes an attempted re-registration of the video device on
> resume.
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index bb5ed05..64e987c 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -37,6 +37,8 @@
> #include <linux/thermal.h>
> #include <linux/video_output.h>
> #include <linux/sort.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> #include <asm/uaccess.h>
>
> #include <acpi/acpi_bus.h>
> @@ -2124,7 +2126,27 @@ static int acpi_video_bus_remove(struct acpi_device *device, int type)
> return 0;
> }
>
> -static int __init acpi_video_init(void)
> +static int __init intel_opregion_present(void)
> +{
> +#if defined(CONFIG_DRM_I915) || defined(CONFIG_DRM_I915_MODULE)
> + struct pci_dev *dev = NULL;
> + u32 address;
> +
> + for_each_pci_dev(dev) {
> + if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> + continue;
> + if (dev->vendor != PCI_VENDOR_ID_INTEL)
> + continue;
> + pci_read_config_dword(dev, 0xfc, &address);
> + if (!address)
> + continue;
> + return 1;
> + }
> +#endif
> + return 0;
> +}
> +
> +int acpi_video_register(void)
> {
> int result = 0;
>
> @@ -2141,6 +2163,22 @@ static int __init acpi_video_init(void)
>
> return 0;
> }
> +EXPORT_SYMBOL(acpi_video_register);
> +
> +/*
> + * This is kind of nasty. Hardware using Intel chipsets may require
> + * the video opregion code to be run first in order to initialise
> + * state before any ACPI video calls are made. To handle this we defer
> + * registration of the video class until the opregion code has run.
> + */
> +
> +static int __init acpi_video_init(void)
> +{
> + if (intel_opregion_present())
> + return 0;
> +
> + return acpi_video_register();
> +}
>
> static void __exit acpi_video_exit(void)
> {
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 6dab63b..5881b6a 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1144,8 +1144,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> if (!IS_I945G(dev) && !IS_I945GM(dev))
> pci_enable_msi(dev->pdev);
>
> - intel_opregion_init(dev);
> -
> spin_lock_init(&dev_priv->user_irq_lock);
> dev_priv->user_irq_refcount = 0;
>
> @@ -1164,6 +1162,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> }
> }
>
> + /* Must be done after probing outputs */
> + intel_opregion_init(dev, 0);
> +
> return 0;
>
> out_iomapfree:
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b293ef0..209592f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -99,7 +99,7 @@ static int i915_resume(struct drm_device *dev)
>
> i915_restore_state(dev);
>
> - intel_opregion_init(dev);
> + intel_opregion_init(dev, 1);
>
> /* KMS EnterVT equivalent */
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 17fa408..aee6f9e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -654,7 +654,7 @@ extern int i915_restore_state(struct drm_device *dev);
>
> #ifdef CONFIG_ACPI
> /* i915_opregion.c */
> -extern int intel_opregion_init(struct drm_device *dev);
> +extern int intel_opregion_init(struct drm_device *dev, int resume);
> extern void intel_opregion_free(struct drm_device *dev);
> extern void opregion_asle_intr(struct drm_device *dev);
> extern void opregion_enable_asle(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_opregion.c b/drivers/gpu/drm/i915/i915_opregion.c
> index ff01283..6942772 100644
> --- a/drivers/gpu/drm/i915/i915_opregion.c
> +++ b/drivers/gpu/drm/i915/i915_opregion.c
> @@ -26,6 +26,7 @@
> */
>
> #include <linux/acpi.h>
> +#include <acpi/video.h>
>
> #include "drmP.h"
> #include "i915_drm.h"
> @@ -136,6 +137,12 @@ struct opregion_asle {
>
> #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)
> +
> static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -282,7 +289,58 @@ static struct notifier_block intel_opregion_notifier = {
> .notifier_call = intel_opregion_video_event,
> };
>
> -int intel_opregion_init(struct drm_device *dev)
> +/*
> + * 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;
> + int 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++;
> + }
> +
> + /* If fewer than 8 outputs, the list must be null terminated */
> + if (i < 8)
> + opregion->acpi->didl[i] = 0;
> +}
> +
> +int intel_opregion_init(struct drm_device *dev, int resume)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_opregion *opregion = &dev_priv->opregion;
> @@ -312,6 +370,11 @@ int intel_opregion_init(struct drm_device *dev)
> if (mboxes & MBOX_ACPI) {
> DRM_DEBUG("Public ACPI methods supported\n");
> opregion->acpi = base + OPREGION_ACPI_OFFSET;
> + if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> + intel_didl_outputs(dev);
> + if (!resume)
> + acpi_video_register();
> + }
> } else {
> DRM_DEBUG("Public ACPI methods not supported\n");
> err = -ENOTSUPP;
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> new file mode 100644
> index 0000000..f0275bb
> --- /dev/null
> +++ b/include/acpi/video.h
> @@ -0,0 +1,11 @@
> +#ifndef __ACPI_VIDEO_H
> +#define __ACPI_VIDEO_H
> +
> +#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
> +extern int acpi_video_register(void);
> +#else
> +static inline int acpi_video_register(void) { return 0; }
> +#endif
> +
> +#endif
> +
>
--
Eric Anholt
eric@xxxxxxxxxx eric.anholt@xxxxxxxxx
Attachment:
signature.asc
Description: This is a digitally signed message part
