On Tue, 2008-03-18 at 17:01 -0400, Len Brown wrote: > On Tuesday 18 March 2008, Thomas Renninger wrote: > > On Tue, 2008-03-18 at 02:02 -0400, Len Brown wrote: > > > From: Len Brown <len.brown@xxxxxxxxx> > > > > > > This reverts commit 3fa2cdcc45a0176de15cac9dbf4ed2834ebf8932. > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=9995 > > > > This is so wrong... > > You register a driver on a device which does not exist. > > You do this because on T61 ThinkPads, the external graphics device > > method (soft-int 10) for brightness switching also works for the > > internal Intel graphics card by pure luck... > > > > You introduce this obviously wrong approach and break other machines > > intentionally (this is not a regression, because brightness switching > > was done through thinkpad_acpi before). > > acpi video brightness worked on the T61 before this patch, eg. in 2.6.24. > acpi video brightness stopped working when this patch was applied. > acpi video brightness works again when this patch (and its clone) were reverted. It worked in a very broken way, by using the Nvidia ACPI graphics device, even you have an Intel graphics card plugged in. > > > This is known for months and reverting this in last minute is the wrong > > action. > > This should be: > > - fixed up in BIOS through a OSI("IGD") string > > IGD isn't part of this discussion, since that support didn't ship > in 2.6.24, and isn't shipping in 2.6.25. IGD is the reason why this does not work as expected. > Enabling IGD, or any other feature, via OSI on a platform that has > already shipped is effectively impossible. It would require every > BIOS and every OS to be updated. The only correct fix (as long as Linux does not support IGD) is a BIOS fix like this: Nvidia device _BCM implementation: use_int10 Intel device if (OSI="IGD") _BCM implementation: use_IGD else _BCM implementation: use_int10 and everything could work fine with the video.c implementation. I just realize that this does not work because we return true for OSI=Windows and these do not know about OSI="IGD". This must be changed in future (I am still waiting for examples where it really makes sense and does some good to return OSI=Windows). We now are lucky because on ThinkPads the thinkpad_acpi driver can ugly, but easily work around this. There may pop up other things (e.g. IGD on other machines and whatever other stuff) which will end in a Linux support impasse then. Now this could get inverted and it would work (it is a bit embarrassing to suggest this to a vendor: Can you please invert the logic because we return true for Windows also)...: Nvidia device _BCM implementation: use_int10 Intel device if (OSI="NOIGD") _BCM implementation: use_int10 else _BCM implementation: use_IGD That vendors are publishing OSI="Linux feature support" in their BIOS is a bit optimistic and we might end up trying to dig in the dark fiddling together things in vendor_acpi drivers as we always did. But currently there is a move towards Linux also on laptops/workstations and those vendors who care to publish a BIOS which fully works on Linux and always should need the ability to do this. > > - get workarounded (as it always has been done) in thinkpad-acpi > > meanwhile. > > > > Please try to not load the ACPI video driver. > > Instead load the thinkpad_acpi driver with: > > brightness_enable=1 > > parameter. Does this work? > > If that works, this is the right workaround for T61 with > > internal/on-board > > Intel graphics card. > > Yes, the thinkpad_acpi driver with brightness_enable=1 works. > > The thinkpad_acpi driver is really useful. > It provides ways to get to thinkpad features for which > no standard interface in Linux currently exists. > > However, the thinkpad_acpi driver should not be used > to provide features for which a generic method exists. > > I understand that a distro may latch onto thinkpad_acpi > to get features in the hands of customers as soon as possible. > I understand that the transition off of custom solutions > is never easy. brightness_enable=1 was added to ease that > transition. If you have additional suggestions, please speak up. > > > You can get a dmi blacklisting so that video driver is not loaded on the > > ThinkPads or this can be done in user-space, but pls do not revert this > > obvious fix. > > This patch was far from obvious, including to Matthew, who created it. PCI devices do not have a _STA function. Therefore one has to look up PCI slot:dev.func and check whether the device is present and really plugged into the board. Thomas -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html