On Mon, 09 Sep 2013, "Rafael J. Wysocki" <rjw@xxxxxxx> wrote: > On Monday, September 09, 2013 05:21:18 PM Daniel Vetter wrote: >> On Mon, Sep 09, 2013 at 02:16:12PM +0200, Rafael J. Wysocki wrote: >> > Hi, >> > >> > On Monday, September 09, 2013 11:32:10 AM Daniel Vetter wrote: >> > > Hi Aaaron, >> > > >> > > Have we grown any clue meanwhile about which Intel boxes need this and for >> > > which we still need to keep the acpi backlight around? >> > >> > First of all, there is a bunch of boxes where ACPI backlight works incorrectly >> > because of the Win8 compatibility issue. [In short, if we say we are compatible >> > with Win8, the backlight AML goes into a special code path that is broken on >> > those machines. Presumably Win8 uses native backlight control on them and that >> > AML code path is never executed there.] The collection of machines with this >> > problem appears to be kind of random (various models from various vendors), but >> > I *think* they are machines that originally shipped with Win7 with a Win8 >> > "upgrade" option (which in practice requires the BIOS to be updated anyway). >> > >> > Because of that, last time we tried to switch all of the systems using i915 >> > and telling the BIOS that they are compatible with Win8 over to native backlight >> > control, but that didn't work for two reasons. The first reason is that some >> > user space doesn't know how to use intel_backlight and needs to be taught about >> > that (so some systems are just not ready for that switch). The second and more >> > fundamental reason is that the native backlight control simply doesn't work on >> > some machines and we don't seem to have any idea why and how to debug this >> > particular problem (mostly because we don't have enough information and we >> > don't know what to ask for). >> > >> > > I've grown _very_ reluctant to just adding tons of quirks to our driver for >> > > the backlight. >> > > >> > > Almost all the quirks we have added recently (or that have been proposed >> > > to be added) are for the backlight. Imo that indicates we get something >> > > fundamentally wrong ... >> > >> > This thing isn't really a quirk. It rather is an option for the users of >> > the systems where ACPI backlight doesn't work to switch over to the native >> > backlight control using a command line switch. This way they can at least >> > *see* if the native backlight control works for them and (hopefully) report >> > problems if that's not the case. This gives us a chance to get more >> > information about what the problem is and possibly to make some progress >> > without making changes for everyone, reverting those changes when they don't >> > work etc. >> > >> > An alternative for them is to pass acpi_osi="!Windows 2012" which will probably >> > make the ACPI backlight work for them again, but this rather is a step back, >> > because we can't possibly learn anything new from that. >> >> If Win8 is as broken as we are I'm ok with the module option. It just >> sounded to me like right now we don't know of a way to make all machines >> somewhat happy, combined with the other pile of random backlight issues >> the assumption that we do something (maybe something a bit racy) that >> windows doesn't do isn't too far-fetched. So I'm not wary of the machines >> where the aml is busted for acpi_os=win8, but for the others where this >> broke stuff. >> >> Or do I miss something here? > > The ACPI video driver doesn't do anything new now. It only does things that > did work before we started to tell BIOSes we're compatible with Win8. The > reason why they don't work on some machines now is not related to whether or > not Win8 is broken, but to what is there in the ACPI tables on those machines. > That *is* pure garbage, but Win8 users don't see that (presumably, because > Win8 does't execute the AML in question). We don't see that either with > acpi_osi="!Windows 2012" (because then we don't execute that AML either). > > Whether or not Win8 is broken doesn't matter here. What matters is that we > have to deal with broken AML somehow. > > One way may be to tell everyone affected by this to pass > acpi_osi="!Windows 2012" in the kernel command line or possibly create a > blacklist of those machines in the kernel (which Felipe has been pushing for > recently) and wash our hands clean of this, but that leaves some exceptionally > bad taste in my mouth. > > The alternative is to try to use native backlight in i915, which we did, but > that didn't work on some machines. I don't think we will know why it didn't > work there before we collect some more information and that's not possible > without user testing. So, the idea is to make that testing possible without > hacking the kernel and that's why we're introducing the new command line > switch. And we're going to ask users to try it and report back. The thing that slightly bugs me with the proposed patches is that they're adding a module parameter to i915 to tell ACPI video driver whether to quirk the backlight or not. Before you know, we *will* have requests to add quirks to i915 to tell ACPI video driver this. I think the parameter "Does the ACPI backlight interface work or not" belongs to the ACPI video driver. Feel free to file this in your bikeshedding folder, but I think i915 should only tell ACPI "I have a native backlight interface". What ACPI video driver does with that information is its business. The desired thing to do here would be to check if there's a module parameter or a quirk to disable the ACPI backlight interface. acpi_backlight=DTRT? ;) BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel