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.
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
drivers/acpi/video.c | 26 +++++++++++++++++++++++++-
include/linux/pci_ids.h | 10 ++++++++++
2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 67dec0c..de16769 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -88,6 +88,20 @@ static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device, int type);
static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
+static const struct pci_device_id intel_drm_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) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_0) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_1) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_2) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_3) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_4) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_5) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_6) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_7) },
+ { } /* Terminating entry */
+};
+
static const struct acpi_device_id video_device_ids[] = {
{ACPI_VIDEO_HID, 0},
{"", 0},
@@ -2558,8 +2572,11 @@ 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 i;
+ bool in_blacklist;
for_each_pci_dev(dev) {
+ in_blacklist = 0;
if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
continue;
if (dev->vendor != PCI_VENDOR_ID_INTEL)
@@ -2567,7 +2584,14 @@ static int __init intel_opregion_present(void)
pci_read_config_dword(dev, 0xfc, &address);
if (!address)
continue;
- return 1;
+ for (i = 0; intel_drm_blacklist[i].device != 0; i++) {
+ if (dev->device == intel_drm_blacklist[i].device) {
+ in_blacklist = 1;
+ break;
+ }
+ }
+ if (!in_blacklist)
+ return 1;
}
#endif
return 0;
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index f6a3b2d..d7866d0 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
@@ -2685,6 +2687,14 @@
#define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500
#define PCI_DEVICE_ID_INTEL_IXP2800 0x9004
#define PCI_DEVICE_ID_INTEL_S21152BB 0xb152
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_0 0x4100
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_1 0x4101
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_2 0x4102
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_3 0x4103
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_4 0x4104
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_5 0x4105
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_6 0x4106
+#define PCI_DEVICE_ID_INTEL_MRST_VGA_7 0x4107
#define PCI_VENDOR_ID_SCALEMP 0x8686
#define PCI_DEVICE_ID_SCALEMP_VSMP_CTL 0x1010
--
1.6.0.2
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?
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_0) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_1) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_2) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_3) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_4) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_5) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_6) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_7) },
Moorestown doesn't have ACPI, so I don't think there's any need to
include these.
--
Matthew Garrett | [email protected]
Hi Matthew,
於 一，2010-08-23 於 12:49 +0100，Matthew Garrett 提到：
> 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
I read the bug:
https://bugzilla.kernel.org/show_bug.cgi?id=11259#c28
And your patch comment:
https://patchwork.kernel.org/patch/13147/
I think there have some machines implement _BCL by using opregion, so
causes the issues. But, there sill have many machines implement _BCL by
using EC to control backlight.
Unfortunately, there have no way can detect the difference.
>
> 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?
>
OK, I put a module option also a way to fix the issue for those machines
that were implemented acpi method by call EC. I will put a module option
in acpi video driver then send to you after testing finished.
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_0) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_1) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_2) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_3) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_4) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_5) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_6) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRST_VGA_7) },
>
> Moorestown doesn't have ACPI, so I don't think there's any need to
> include these.
>
OH! OK, thank's for your review!
Joey Lee
On Monday 23 August 2010 13:40:48 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.
>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
...
> @@ -2567,7 +2584,14 @@ static int __init intel_opregion_present(void)
> pci_read_config_dword(dev, 0xfc, &address);
> if (!address)
> continue;
> - return 1;
> + for (i = 0; intel_drm_blacklist[i].device != 0; i++) {
> + if (dev->device == intel_drm_blacklist[i].device) {
> + in_blacklist = 1;
> + break;
You can just return 0 here...
> + }
> + }
> + if (!in_blacklist)
> + return 1;
and get rid of in_blacklist variable.
Makes the code a bit easier to read.
Thomas
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 <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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
On Mon, Aug 23, 2010 at 07:53:21PM +0200, Thomas Renninger wrote:
> 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.
How about we do this instead: add a psb stub driver that does nothing
other than call acpi_backlight_register()?
--
Matthew Garrett | [email protected]
Hi Thomas,
於 一，2010-08-23 於 19:53 +0200，Thomas Renninger 提到：
> >
> > 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?
I respined your patch and build it on 2.6.34 kernel success, but it
causes depmod detected a loop warning then ignore video.ko and i915.ko
linux-4byd:/usr/src/linux-2.6.34-12/drivers/acpi # depmod
WARNING: Loop
detected: /lib/modules/2.6.34-12-desktop/kernel/drivers/acpi/video.ko
needs i915.ko which needs video.ko again!
WARNING:
Module /lib/modules/2.6.34-12-desktop/kernel/drivers/acpi/video.ko
ignored, due to loop
WARNING:
Module /lib/modules/2.6.34-12-desktop/kernel/drivers/gpu/drm/i915/i915.ko ignored, due to loop
WARNING:
Module /lib/modules/2.6.34-12-desktop/kernel/drivers/platform/x86/msi-laptop.ko ignored, due to loop
I think because i915.ko already dependency to video.h when we add
opregion support.
So, now, video.c could not be includ i915_drm.h
Thank's
Joey Lee
>
> -----------------
> 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 <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
>
> ---
> 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
Hi Matthew,
於 一，2010-08-23 於 18:51 +0100，Matthew Garrett 提到：
> On Mon, Aug 23, 2010 at 07:53:21PM +0200, Thomas Renninger wrote:
>
> > 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.
>
> How about we do this instead: add a psb stub driver that does nothing
> other than call acpi_backlight_register()?
>
The attached file is a draft stub driver for poulsbo. Like you said, it
will only call acpi_video_register and acpi_video_unregister when
initial and exit.
And I added the Poulsbo pci id in the module alias.
It's works to me, where can we put the driver? in driver/staging ? or
put in driver/gpu/stub ?
Thank's a lot!
Joey Lee
Hi Matthew, Thomas,
於 二，2010-08-24 於 01:03 -0600，Joey Lee 提到：
> Hi Matthew,
>
> 於 一，2010-08-23 於 18:51 +0100，Matthew Garrett 提到：
> > On Mon, Aug 23, 2010 at 07:53:21PM +0200, Thomas Renninger wrote:
> >
> > > 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.
> >
> > How about we do this instead: add a psb stub driver that does nothing
> > other than call acpi_backlight_register()?
> >
>
> The attached file is a draft stub driver for poulsbo. Like you said, it
> will only call acpi_video_register and acpi_video_unregister when
> initial and exit.
> And I added the Poulsbo pci id in the module alias.
>
> It's works to me, where can we put the driver? in driver/staging ? or
> put in driver/gpu/stub ?
>
The attached file is the patch to add a poulsbo stub module in
drivers/gpu/stub.
Please kindly review it and give me any suggestions.
Thank's a lot!
Joey Lee
On Tue, Aug 24, 2010 at 04:08:59AM -0600, Joey Lee wrote:
> drivers/gpu/Makefile | 2 +-
> drivers/gpu/stub/Kconfig | 13 +++++++++++++
> drivers/gpu/stub/Makefile | 1 +
> drivers/gpu/stub/poulsbo.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/video/Kconfig | 2 ++
> 5 files changed, 61 insertions(+), 1 deletions(-)
> create mode 100644 drivers/gpu/stub/Kconfig
> create mode 100644 drivers/gpu/stub/Makefile
> create mode 100644 drivers/gpu/stub/poulsbo.c
>
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 30879df..cc92778 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -1 +1 @@
> -obj-y += drm/ vga/
> +obj-y += drm/ vga/ stub/
> diff --git a/drivers/gpu/stub/Kconfig b/drivers/gpu/stub/Kconfig
> new file mode 100644
> index 0000000..88fd919
> --- /dev/null
> +++ b/drivers/gpu/stub/Kconfig
> @@ -0,0 +1,13 @@
> +config STUB_POULSBO
> + tristate "Intel Poulsbo Stub Driver"
> + default m
> + # Poulsbo stub depends on ACPI_VIDEO when ACPI is enabled
> + # but for select to work, need to select ACPI_VIDEO's dependencies, ick
> + select ACPI_VIDEO if ACPI
> + help
> + Choose this option if you have a system that has Intel Poulsbo
> + integrated graphics. If M is selected, the module will be called
> + Poulsbo. This driver is a stub driver for Poulsbo that will call
Really? I suspect it'll be called poulsbo.ko :)
> +static int __init poulsbo_init(void)
> +{
> + printk(KERN_INFO "poulsbo: stub driver load.\n");
Don't really need to printk here
> + return acpi_video_register();
> +}
> +
> +static void __exit poulsbo_cleanup(void)
> +{
> + acpi_video_unregister();
> + printk(KERN_INFO "poulsbo: stub driver unloaded.\n");
Or here - we'll get the ACPI video output. Other than that, looks good.
--
Matthew Garrett | [email protected]
Erm. Sorry about that - I just realised that there's actually a fairly
significant problem with it :) Right now you'll perform the register
call unconditionally on module load. You need to make sure that there's
actually Poulsbo hardware in the system, either by registering as a
proper PCI driver and calling acpi_video_register in your probe function
(and unregister in your remove function) or walking the PCI device list
looking for a psb device.
--
Matthew Garrett | [email protected]
Hi Matthew,
於 二，2010-08-24 於 15:22 +0100，Matthew Garrett 提到：
> Erm. Sorry about that - I just realised that there's actually a fairly
> significant problem with it :) Right now you'll perform the register
> call unconditionally on module load. You need to make sure that there's
> actually Poulsbo hardware in the system, either by registering as a
> proper PCI driver and calling acpi_video_register in your probe function
> (and unregister in your remove function) or walking the PCI device list
> looking for a psb device.
>
Sorry! You are right! That's my fail!
I will put the PCI device check in probe function then send patch again.
Appreciate for your review.
Thank's a lot!
Joey Lee
On Tue, Aug 24, 2010 at 03:22:19PM +0100, Matthew Garrett wrote:
> Erm. Sorry about that - I just realised that there's actually a fairly
> significant problem with it :) Right now you'll perform the register
> call unconditionally on module load. You need to make sure that there's
> actually Poulsbo hardware in the system, either by registering as a
> proper PCI driver and calling acpi_video_register in your probe function
> (and unregister in your remove function) or walking the PCI device list
> looking for a psb device.
Ick, don't walk the pci device list, just register a "real" pci device
and do it that way, it's much simpler and plays nicer with the driver
model and the rest of the kernel.
thanks,
greg k-h
On Tue, Aug 24, 2010 at 07:51:30AM -0700, Greg KH wrote:
> On Tue, Aug 24, 2010 at 03:22:19PM +0100, Matthew Garrett wrote:
> > Erm. Sorry about that - I just realised that there's actually a fairly
> > significant problem with it :) Right now you'll perform the register
> > call unconditionally on module load. You need to make sure that there's
> > actually Poulsbo hardware in the system, either by registering as a
> > proper PCI driver and calling acpi_video_register in your probe function
> > (and unregister in your remove function) or walking the PCI device list
> > looking for a psb device.
>
> Ick, don't walk the pci device list, just register a "real" pci device
> and do it that way, it's much simpler and plays nicer with the driver
> model and the rest of the kernel.
My only concern with that (and it's not a strong one) is that it'll
block the psb driver from binding. Of course, the psb driver should be
doing the acpi handling itself anyway, so...
--
Matthew Garrett | [email protected]
On Tue, Aug 24, 2010 at 03:58:03PM +0100, Matthew Garrett wrote:
> On Tue, Aug 24, 2010 at 07:51:30AM -0700, Greg KH wrote:
> > On Tue, Aug 24, 2010 at 03:22:19PM +0100, Matthew Garrett wrote:
> > > Erm. Sorry about that - I just realised that there's actually a fairly
> > > significant problem with it :) Right now you'll perform the register
> > > call unconditionally on module load. You need to make sure that there's
> > > actually Poulsbo hardware in the system, either by registering as a
> > > proper PCI driver and calling acpi_video_register in your probe function
> > > (and unregister in your remove function) or walking the PCI device list
> > > looking for a psb device.
> >
> > Ick, don't walk the pci device list, just register a "real" pci device
> > and do it that way, it's much simpler and plays nicer with the driver
> > model and the rest of the kernel.
>
> My only concern with that (and it's not a strong one) is that it'll
> block the psb driver from binding. Of course, the psb driver should be
> doing the acpi handling itself anyway, so...
If a system really does have this type of driver on it, it can unbind
this driver and then bind itself, from userspace, so I don't see a
problem with this.
thanks,
greg k-h
Hi Matthew, Greg,
The attached file is new patch to add Intel Poulsbo Stub driver.
Follow your suggestion, I changed:
- Add pci id list and direct register it as a pci driver.
- Removed printk in init/exit functions.
- Change the wording in Kconfig.
- Modified the Copyright. Removed "or any later version" and 2
redundant paragraphs.
Please kindly help to review the patch again. Appreciate for any
suggestions.
Thank's a lot!
Joey Lee