Re: [PATCH] Add intel drm blacklist to intel_opregion_present detect
From: Thomas Renninger
Date: Mon Aug 23 2010 - 13:48:08 EST
On Monday 23 August 2010 13:49:38 Matthew Garrett wrote:
> On Mon, Aug 23, 2010 at 07:40:48PM +0800, Lee, Chun-Yi wrote:
> > There have some machines not support by i915 drm driver, e.g. MSI U110/U150,
> > there are use poulsbo chip and drm driver not support it because legal issue.
> > Those machines's acpi backlight control actually work fine and don't need apply
> > the intel opregion support.
> > So, add intel drm blacklist to intel_opregion_present, it can enable the acpi
> > brightness interface on Poulsbo/Morrestown.
>
> I'm still kind of reluctant about this - doing the blacklisting here
> means that there's no way for a native driver to inhibit registration
> from occuring until after opregion setup has taken place, and we found
> that that was necessary on some 915 so I suspect it is on gma500 as
> well. Perhaps it should just be done as a module option, and then
> distributions who want to deal with this case could set it by default?
Hm, needing a module option to get a system running is not what the user expects.
By default, the system should run fine and a module option should be added
as a workaround or for debugging only.
What do you think about below patch which introduces:
- the blacklist based on the previous patch and comments
- the module param -> as workaround and for trying out
- let video.ko and i915_opregion share the same func to check
for opregion support
Should I add a drm/i915 list for this to get another review?
Thanks,
Thomas
-----------------
drm i915: Better communicate opregion support with video.ko
and add a i915 module parameter to enable/disable opregion code
and add a blacklist where we know opregion currently does not work.
This code is compile tested only on latest 2.6 Linus git tree.
Thorough review or some testing is still needed.
Signed-off-by: Thomas Renninger <trenn@xxxxxxx>
CC: joeyli.kernel@xxxxxxxxx
CC: gregkh@xxxxxxx
CC: Dennis.Jansen@xxxxxx
CC: linux-acpi@xxxxxxxxxxxxxxx
CC: mjg@xxxxxxxxxx
---
drivers/acpi/video.c | 16 +++++--
drivers/gpu/drm/i915/i915_opregion.c | 77 +++++++++++++++++++++++++++++++--
include/drm/i915_drm.h | 1 +
include/linux/pci_ids.h | 2 +
4 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 67dec0c..b91b51e 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -46,6 +46,9 @@
#include <acpi/acpi_drivers.h>
#include <linux/suspend.h>
#include <acpi/video.h>
+#if defined(CONFIG_DRM_I915) || defined(CONFIG_DRM_I915_MODULE)
+#include <drm/i915_drm.h>
+#endif
#define PREFIX "ACPI: "
@@ -2557,17 +2560,20 @@ static int __init intel_opregion_present(void)
{
#if defined(CONFIG_DRM_I915) || defined(CONFIG_DRM_I915_MODULE)
struct pci_dev *dev = NULL;
- u32 address;
+ int err;
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;
+ err = intel_opregion_device_support(dev);
+ if (err == 0)
+ return 1;
+ else
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No opregion support"
+ " on Intel graphics card: %d\n",
+ err));
}
#endif
return 0;
diff --git a/drivers/gpu/drm/i915/i915_opregion.c b/drivers/gpu/drm/i915/i915_opregion.c
index ea5d3fe..eb49896 100644
--- a/drivers/gpu/drm/i915/i915_opregion.c
+++ b/drivers/gpu/drm/i915/i915_opregion.c
@@ -26,6 +26,8 @@
*/
#include <linux/acpi.h>
+#include <linux/pci_ids.h>
+#include <linux/pci.h>
#include <acpi/video.h>
#include "drmP.h"
@@ -464,15 +466,43 @@ blind_set:
goto end;
}
-int intel_opregion_init(struct drm_device *dev, int resume)
+static int i915_opregion = 1;
+module_param_named(opregion, i915_opregion, int, 0400);
+MODULE_PARM_DESC(opregion, "opregion support (backlight,display output,..) "
+ "(1=on [default], 0=off, 2=force)");
+
+static const struct pci_device_id intel_opregion_blacklist[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SCH_VGA_0) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SCH_VGA_1) },
+ { }
+};
+
+/* Returns zero if opregion device is supported or an error code if not */
+int intel_opregion_device_support(struct pci_dev *pdev)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_opregion *opregion = &dev_priv->opregion;
+ struct intel_opregion opregion;
void *base;
u32 asls, mboxes;
- int err = 0;
+ int i, err = 0;
+
+ if (i915_opregion == 0)
+ return -ENODEV;
+
+ for (i = 0; intel_opregion_blacklist[i].device != 0; i++) {
+ if (pdev->device == intel_opregion_blacklist[i].device) {
+ if (i915_opregion != 2) {
+ DRM_INFO("opregion support blacklisted for"
+ " device %s, let video.ko handle it\n",
+ pci_name(pdev));
+ return -ENOTSUPP;
+ } else {
+ DRM_INFO("Force opregion on blacklisted"
+ " device %s\n", pci_name(pdev));
+ }
+ }
+ }
- pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
+ pci_read_config_dword(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");
@@ -483,6 +513,43 @@ int intel_opregion_init(struct drm_device *dev, int resume)
if (!base)
return -ENOMEM;
+ opregion.header = base;
+ if (memcmp(opregion.header->signature, OPREGION_SIGNATURE, 16)) {
+ DRM_DEBUG_DRIVER("opregion signature mismatch\n");
+ err = -EINVAL;
+ goto out;
+ }
+
+ mboxes = opregion.header->mboxes;
+ if (!(mboxes & MBOX_ACPI)) {
+ DRM_DEBUG_DRIVER("Public ACPI methods not supported\n");
+ err = -ENOTSUPP;
+ goto out;
+ }
+
+out:
+ iounmap(opregion.header);
+ return err;
+}
+EXPORT_SYMBOL_GPL(intel_opregion_device_support);
+
+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;
+ void *base;
+ u32 asls, mboxes;
+ int err = 0;
+
+ err = intel_opregion_device_support(dev->pdev);
+ if (err)
+ return err;
+
+ pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
+ base = ioremap(asls, OPREGION_SZ);
+ if (!base)
+ return -ENOMEM;
+
opregion->header = base;
if (memcmp(opregion->header->signature, OPREGION_SIGNATURE, 16)) {
DRM_DEBUG_DRIVER("opregion signature mismatch\n");
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 8f8b072..3fa5f20 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -40,6 +40,7 @@ extern bool i915_gpu_raise(void);
extern bool i915_gpu_lower(void);
extern bool i915_gpu_busy(void);
extern bool i915_gpu_turbo_disable(void);
+extern int intel_opregion_device_support(struct pci_dev *pdev);
#endif
/* Each region is a minimum of 16k, and there are at most 255 of them.
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index f6a3b2d..2861dd8 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2675,6 +2675,8 @@
#define PCI_DEVICE_ID_INTEL_82443GX_0 0x71a0
#define PCI_DEVICE_ID_INTEL_82443GX_2 0x71a2
#define PCI_DEVICE_ID_INTEL_82372FB_1 0x7601
+#define PCI_DEVICE_ID_INTEL_SCH_VGA_0 0x8108
+#define PCI_DEVICE_ID_INTEL_SCH_VGA_1 0x8109
#define PCI_DEVICE_ID_INTEL_SCH_LPC 0x8119
#define PCI_DEVICE_ID_INTEL_SCH_IDE 0x811a
#define PCI_DEVICE_ID_INTEL_82454GX 0x84c4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/