Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751757Ab3IJNvo (ORCPT ); Tue, 10 Sep 2013 09:51:44 -0400 Received: from mga11.intel.com ([192.55.52.93]:61857 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752585Ab3IJNvk (ORCPT ); Tue, 10 Sep 2013 09:51:40 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.90,878,1371106800"; d="scan'208";a="393596860" From: Jani Nikula To: "Rafael J. Wysocki" , Daniel Vetter Cc: Aaron Lu , ACPI Devel Mailing List , Matthew Garrett , Daniel Vetter , "intel-gfx\@lists.freedesktop.org" , Yves-Alexis Perez , "linux-kernel\@vger.kernel.org" , "dri-devel\@lists.freedesktop.org" , Seth Forshee , "Lee\, Chun-Yi" , Igor Gnatenko , Henrique de Moraes Holschuh , Lee Chun-Yi , Len Brown Subject: Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8 In-Reply-To: <5155010.Y1gov7SKhP@vostro.rjw.lan> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <522D88C3.7000808@intel.com> <1793135.3qk7NfcINN@vostro.rjw.lan> <20130909152118.GG27291@phenom.ffwll.local> <5155010.Y1gov7SKhP@vostro.rjw.lan> User-Agent: Notmuch/0.16+63~g12e17e8 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) Date: Tue, 10 Sep 2013 16:53:40 +0300 Message-ID: <87eh8wail7.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5928 Lines: 109 On Mon, 09 Sep 2013, "Rafael J. Wysocki" 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/