On Fri, 2007-12-07 at 22:50 +0000, Matthew Garrett wrote: > On Fri, Dec 07, 2007 at 01:55:13PM +0100, Thomas Renninger wrote: > > Hi Matthew, > > > > I posted this one already and you had some concerns about it: > > http://www.mail-archive.com/linux-acpi@xxxxxxxxxxxxxxx/msg09510.html > > > > IMO, this one should still be added to -mm, because: > > - The patch itself is correct, it is the function that is called > > that must make sure to return the corresponding pci device > > No, I don't think that follows. The semantics of > acpi_get_physical_device are currently to return a physical device only > if the node directly corresponds to one. What's the correct physical > device for the video extension? It may be a PCI device, but it's just as > easy to argue that it corresponds to some piece of platform-specific > hardware. What do you mean with "platform-specific" hardware? A graphics card on an ACPI capable system should always be PCI, PCIe or AGP driven and a call to acpi_get_physical_device should return its struct pci structure. AFAIK it may return the bridge device atm, this could be fixed up later, in this case it does not matter as no values of the PCI device are needed, it's only needed to check whether it exists at all and if the bridge exists it's enough. > We could change the semantics and ensure that all leaf nodes of an ACPI > node representing a PCI device share the same physical device. That > would avoid the bug your patch currently introduces of ignoring > functional devices, but would immediately cause the second video device > to reappear on Thinkpads. I thought it is done that way? You mean e.g. going up to the PCI bus object and get the bus value, then get func and slot from the child on which branch our device sits: PCI0 (bus0) / \ XY (slot 2, func1) / | \ A B C You mean here device A, B and C should all return the pci device at 0:2.1, this should be the case? It at least was with the "find pci device" func I came up with my very first patch, before you pointed me to: acpi_get_physical_device This would also work for ThinkPads: PCI0 (bus0) / \ VID AGP (slot2, func0) (slot1, func0) | VID (ADR 0x00) Slot, func no.s are made up. 0:1.0 would point to a PCI-PCI bridge, which only exists on external/ discrete graphics card (with own mem etc.) acpi_get_physical_device could get extended to point to the real graphics pci device behind the bridge 0:2.0 would point to the internal graphics card sitting on a PCIe bus > Incidentally, have you tried this on a Thinkpad with a discrete graphics > controller? I /suspect/ (but can't verify) that you'll end up discarding > the working video extension. No, it should work, see above. Brightness switching may not work because the current ACPI implementation requires a new graphics card driver feature which should be supported by the latest Intel driver (or at least soon). Anyway it is the specified mechanism how things should work. > In summary, I don't think this approach can be made to work. You cannot check in kernel whether an ACPI graphics device exists, but you can in userspace? I doubt that. > You're > throwing out legitimate and working devices. Please prove. I tested this on three totally different machines: - Lenovo - Toshiba - HP Only recent Vista capable systems should need this, that means those tests cover a reasonable amount of all video extension supporting machines. > Instead, we should export > information about the addresses of the video extensions and let > whoever's handling the graphics (which is userspace right now) handle > it. No, this is wrong. Please explain how this should work more detailed, a solution is needed now. Currently the video driver is severely broken for a lot machines (at least Lenovo and Toshiba). Severely means that HW registers are accessed for which no device exist. E.g. if you hit the brightness up/down button on a Lenovo ThinkPad, it may happen that the wrong device brightness functions are served (this is why you may see brightness switching working with an internal Intel graphics card, because the Nvidia/ATI external device's brightness functions are processed which also works with the other device by luck). On the Toshiba, brightness switching does not work (jumps irregularly) with both devices registered and it's again luck that the machine is simply not freezing at all. 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
