Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, Sep 11, 2013 at 11:45:19AM +0300, Jani Nikula wrote: > On Wed, 11 Sep 2013, Aaron Lu wrote: > > It is possible the i915 driver decides not to register a backlight > > interface for the graphics card for some reason(memory allocation failed > > or it knows the native control does not work on this card or whatever), > > so I would prefer let i915 tell ACPI video that it has registered a > > native backlight control interface as Jani has said. > > > > Then together with the video.use_native_backlight, we can register or > > not register ACPI video backlight interface accordingly. Or rather, we > > can simply not register ACPI video backlight interface for Win8 systems > > as long as i915 indicates that it has native backlight control(if the > > native control is broken, i915 should fix it or blacklist it so that > > i915 will not indicate it has native backlight control and ACPI video > > will continue to register its own). > > > > How does this sound? > > Sounds good to me. > > Before plunging forward, have you observed any difference between the > boot modes? We have reports [1] that the backlight behaviour is Not yet from ACPI's point of view. > different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the > acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole > story. This check in patch 2/2 is a policy: for Win8 system, we think the native backlight control has a better chance of working than the ACPI video's, so I think the check is enough in ACPI video. > > Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code > paths, what guarantees do we have of UEFI+CSM or legacy boots working? I suppose the 'tested BIOS code paths' means the pure UEFI boot mode? I don't know what guatantees do we have since I don't know what happened underneath after the backlight register is set in i915 driver, you or other i915 driver people should know more than I do :-) BTW, after the backlight register is set in i915, is it that some find of firmware code will run in response to the setting of the register(e.g. the BLC_PWM_CTL/BLC_PWM_CPU_CTL/PCI_LBPC reg)? Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-09-11 at 13:29 +0300, Jani Nikula wrote:
> On Wed, 11 Sep 2013, Matthew Garrett wrote:
> > On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote:
> >
> >> Before plunging forward, have you observed any difference between the
> >> boot modes? We have reports [1] that the backlight behaviour is
> >> different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the
> >> acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole
> >> story.
> >>
> >> Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code
> >> paths, what guarantees do we have of UEFI+CSM or legacy boots working?
> >
> > We have no evidence of Windows behaving differently based on the exposed
> > firmware type.
>
> By "behaving differently", do you mean internally adapting to the boot
> mode, or exhibiting different behaviour to the user?
As far as backlight control goes, both.
> We have evidence of the firmware behaving differently (VBT, backlight)
> based on the boot mode, all else being equal. We don't adapt to that,
> and we fail. I don't know if we should adapt, or do things differently
> altogether. I don't even know if Windows 8 works on all boot modes on
> the machines in question.
Sure, but Windows knows nothing about VBT or opregion-backed backlight
control. That's up to the Intel driver.
--
Matthew Garrett
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 11 Sep 2013, Matthew Garrett wrote: > On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: > >> Before plunging forward, have you observed any difference between the >> boot modes? We have reports [1] that the backlight behaviour is >> different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the >> acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole >> story. >> >> Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code >> paths, what guarantees do we have of UEFI+CSM or legacy boots working? > > We have no evidence of Windows behaving differently based on the exposed > firmware type. By "behaving differently", do you mean internally adapting to the boot mode, or exhibiting different behaviour to the user? We have evidence of the firmware behaving differently (VBT, backlight) based on the boot mode, all else being equal. We don't adapt to that, and we fail. I don't know if we should adapt, or do things differently altogether. I don't even know if Windows 8 works on all boot modes on the machines in question. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On mer., 2013-09-11 at 08:45 +, Matthew Garrett wrote: > On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: > > > Before plunging forward, have you observed any difference between the > > boot modes? We have reports [1] that the backlight behaviour is > > different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the > > acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole > > story. > > > > Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code > > paths, what guarantees do we have of UEFI+CSM or legacy boots working? > > We have no evidence of Windows behaving differently based on the exposed > firmware type. > I do have an X230 which boots fine using pure UEFI or UEFI+CSM (and I guess I can try booting it with a grml for pure legacy), so I can do tests if needed. Regards, -- Yves-Alexis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: > Before plunging forward, have you observed any difference between the > boot modes? We have reports [1] that the backlight behaviour is > different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the > acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole > story. > > Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code > paths, what guarantees do we have of UEFI+CSM or legacy boots working? We have no evidence of Windows behaving differently based on the exposed firmware type. -- Matthew Garrett
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 11 Sep 2013, Aaron Lu wrote: > It is possible the i915 driver decides not to register a backlight > interface for the graphics card for some reason(memory allocation failed > or it knows the native control does not work on this card or whatever), > so I would prefer let i915 tell ACPI video that it has registered a > native backlight control interface as Jani has said. > > Then together with the video.use_native_backlight, we can register or > not register ACPI video backlight interface accordingly. Or rather, we > can simply not register ACPI video backlight interface for Win8 systems > as long as i915 indicates that it has native backlight control(if the > native control is broken, i915 should fix it or blacklist it so that > i915 will not indicate it has native backlight control and ACPI video > will continue to register its own). > > How does this sound? Sounds good to me. Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data >= ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? BR, Jani. [1] https://bugzilla.kernel.org/show_bug.cgi?id=47941#c96 -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 11 Sep 2013, Aaron Lu aaron...@intel.com wrote: It is possible the i915 driver decides not to register a backlight interface for the graphics card for some reason(memory allocation failed or it knows the native control does not work on this card or whatever), so I would prefer let i915 tell ACPI video that it has registered a native backlight control interface as Jani has said. Then together with the video.use_native_backlight, we can register or not register ACPI video backlight interface accordingly. Or rather, we can simply not register ACPI video backlight interface for Win8 systems as long as i915 indicates that it has native backlight control(if the native control is broken, i915 should fix it or blacklist it so that i915 will not indicate it has native backlight control and ACPI video will continue to register its own). How does this sound? Sounds good to me. Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? BR, Jani. [1] https://bugzilla.kernel.org/show_bug.cgi?id=47941#c96 -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? We have no evidence of Windows behaving differently based on the exposed firmware type. -- Matthew Garrett matthew.garr...@nebula.com
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On mer., 2013-09-11 at 08:45 +, Matthew Garrett wrote: On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? We have no evidence of Windows behaving differently based on the exposed firmware type. I do have an X230 which boots fine using pure UEFI or UEFI+CSM (and I guess I can try booting it with a grml for pure legacy), so I can do tests if needed. Regards, -- Yves-Alexis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 11 Sep 2013, Matthew Garrett matthew.garr...@nebula.com wrote: On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? We have no evidence of Windows behaving differently based on the exposed firmware type. By behaving differently, do you mean internally adapting to the boot mode, or exhibiting different behaviour to the user? We have evidence of the firmware behaving differently (VBT, backlight) based on the boot mode, all else being equal. We don't adapt to that, and we fail. I don't know if we should adapt, or do things differently altogether. I don't even know if Windows 8 works on all boot modes on the machines in question. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-09-11 at 13:29 +0300, Jani Nikula wrote:
On Wed, 11 Sep 2013, Matthew Garrett matthew.garr...@nebula.com wrote:
On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote:
Before plunging forward, have you observed any difference between the
boot modes? We have reports [1] that the backlight behaviour is
different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the
acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole
story.
Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code
paths, what guarantees do we have of UEFI+CSM or legacy boots working?
We have no evidence of Windows behaving differently based on the exposed
firmware type.
By behaving differently, do you mean internally adapting to the boot
mode, or exhibiting different behaviour to the user?
As far as backlight control goes, both.
We have evidence of the firmware behaving differently (VBT, backlight)
based on the boot mode, all else being equal. We don't adapt to that,
and we fail. I don't know if we should adapt, or do things differently
altogether. I don't even know if Windows 8 works on all boot modes on
the machines in question.
Sure, but Windows knows nothing about VBT or opregion-backed backlight
control. That's up to the Intel driver.
--
Matthew Garrett matthew.garr...@nebula.com
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, Sep 11, 2013 at 11:45:19AM +0300, Jani Nikula wrote: On Wed, 11 Sep 2013, Aaron Lu aaron...@intel.com wrote: It is possible the i915 driver decides not to register a backlight interface for the graphics card for some reason(memory allocation failed or it knows the native control does not work on this card or whatever), so I would prefer let i915 tell ACPI video that it has registered a native backlight control interface as Jani has said. Then together with the video.use_native_backlight, we can register or not register ACPI video backlight interface accordingly. Or rather, we can simply not register ACPI video backlight interface for Win8 systems as long as i915 indicates that it has native backlight control(if the native control is broken, i915 should fix it or blacklist it so that i915 will not indicate it has native backlight control and ACPI video will continue to register its own). How does this sound? Sounds good to me. Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is Not yet from ACPI's point of view. different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. This check in patch 2/2 is a policy: for Win8 system, we think the native backlight control has a better chance of working than the ACPI video's, so I think the check is enough in ACPI video. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? I suppose the 'tested BIOS code paths' means the pure UEFI boot mode? I don't know what guatantees do we have since I don't know what happened underneath after the backlight register is set in i915 driver, you or other i915 driver people should know more than I do :-) BTW, after the backlight register is set in i915, is it that some find of firmware code will run in response to the setting of the register(e.g. the BLC_PWM_CTL/BLC_PWM_CPU_CTL/PCI_LBPC reg)? Thanks, Aaron -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, Sep 10, 2013 at 09:23:04PM +0200, Rafael J. Wysocki wrote: > On Tuesday, September 10, 2013 04:53:40 PM Jani Nikula wrote: > > 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
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tuesday, September 10, 2013 04:53:40 PM Jani Nikula wrote: > 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
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 17:21 +0300, Jani Nikula wrote: > On Tue, 10 Sep 2013, Matthew Garrett wrote: > > On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: > > > >> I think the parameter "Does the ACPI backlight interface work or not" > >> belongs to the ACPI video driver. > > > > That depends on how Windows 8 works. If Windows 8 policy is handled by > > the GPU drivers then it belongs in i915. If it's handled by the ACPI > > code then it belongs in the ACPI code. > > I fail to see the logic. Windows 8 policy dictates whether we can use > the AML code or not. IMHO, ACPI code is in the best position to figure > this out and quirk as necessary. It's the part that knows about Windows > 8, not i915. So if nvidia hardware uses the ACPI interface and Intel doesn't, we should still quirk it in the ACPI driver? -- Matthew Garrett
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 10 Sep 2013, Matthew Garrett wrote: > On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: > >> I think the parameter "Does the ACPI backlight interface work or not" >> belongs to the ACPI video driver. > > That depends on how Windows 8 works. If Windows 8 policy is handled by > the GPU drivers then it belongs in i915. If it's handled by the ACPI > code then it belongs in the ACPI code. I fail to see the logic. Windows 8 policy dictates whether we can use the AML code or not. IMHO, ACPI code is in the best position to figure this out and quirk as necessary. It's the part that knows about Windows 8, not i915. > But I have no way of determining that, whereas you work for a company > that produces a Windows 8 video driver… Here I do see the logic. However see signature; I'm not in the best of positions to figure out things about Windows 8 video drivers. ;) But I'll see what I can do. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: > I think the parameter "Does the ACPI backlight interface work or not" > belongs to the ACPI video driver. That depends on how Windows 8 works. If Windows 8 policy is handled by the GPU drivers then it belongs in i915. If it's handled by the ACPI code then it belongs in the ACPI code. But I have no way of determining that, whereas you work for a company that produces a Windows 8 video driver… -- Matthew Garrett
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, Sep 09, 2013 at 11:32:10AM +0200, 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? I've grown _very_
Sorry, no general rule has been found. As Rafael has said, it appears
to be random... And in addition to the "shipped with Win7 with a Win8
upgrade option" case, I also find a Sony laptop that has Win8
pre-installed and a broken ACPI video backlight interface. Its ACPI
video backlight control method is simply a stub, so even the
acpi_osi="!Windows 2012" will not provide a working backlight for this
system.
-Aaron
> 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 ...
>
> Cheers, Daniel
>
> On Mon, Sep 09, 2013 at 04:42:20PM +0800, Aaron Lu wrote:
> > According to Matthew Garrett, "Windows 8 leaves backlight control up
> > to individual graphics drivers rather than making ACPI calls itself.
> > There's plenty of evidence to suggest that the Intel driver for
> > Windows [8] doesn't use the ACPI interface, including the fact that
> > it's broken on a bunch of machines when the OS claims to support
> > Windows 8. The simplest thing to do appears to be to disable the
> > ACPI backlight interface on these systems".
> >
> > There's a problem with that approach, however, because simply
> > avoiding to register the ACPI backlight interface if the firmware
> > calls _OSI for Windows 8 may not work in the following situations:
> > (1) The ACPI backlight interface actually works on the given system
> > and the i915 driver is not loaded (e.g. another graphics driver
> > is used).
> > (2) The ACPI backlight interface doesn't work on the given system,
> > but there is a vendor platform driver that will register its
> > own, equally broken, backlight interface if not prevented from
> > doing so by the ACPI subsystem.
> > Therefore we need to allow the ACPI backlight interface to be
> > registered until the i915 driver is loaded which then will unregister
> > it if the firmware has called _OSI for Windows 8 (or will register
> > the ACPI video driver without backlight support if not already
> > present).
> >
> > For this reason, introduce an alternative function for registering
> > ACPI video, __acpi_video_register(bool), that if ture is passed,
> > will check whether or not the ACPI video driver has already been
> > registered and whether or not the backlight Windows 8 quirk has to
> > be applied. If the quirk has to be applied, it will block the ACPI
> > backlight support and either unregister the backlight interface if
> > the ACPI video driver has already been registered, or register the
> > ACPI video driver without the backlight interface otherwise. Make
> > the i915 driver use __acpi_video_register() instead of
> > acpi_video_register() in i915_driver_load(), and the param passed
> > there is controlled by the i915 module level parameter
> > i915_take_over_backlight, which is set to false by default.
> >
> > This change is evolved from earlier patches of Matthew Garrett,
> > Chun-Yi Lee and Seth Forshee and is heavily based on two patches
> > from Rafael:
> > https://lkml.org/lkml/2013/7/17/720
> > https://lkml.org/lkml/2013/7/24/806
> >
> > Signed-off-by: Aaron Lu
> > ---
> > drivers/acpi/internal.h | 2 ++
> > drivers/acpi/video.c| 24
> > drivers/acpi/video_detect.c | 15 ++-
> > drivers/gpu/drm/i915/i915_dma.c | 2 +-
> > drivers/gpu/drm/i915/i915_drv.c | 5 +
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > include/acpi/video.h| 9 +++--
> > include/linux/acpi.h| 1 +
> > 8 files changed, 47 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> > index 20f4233..a53832e 100644
> > --- a/drivers/acpi/internal.h
> > +++ b/drivers/acpi/internal.h
> > @@ -170,8 +170,10 @@ int acpi_create_platform_device(struct acpi_device
> > *adev,
> >
> > --
> > */
> > #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
> > bool acpi_video_backlight_quirks(void);
> > +bool acpi_video_verify_backlight_support(void);
> > #else
> > static inline bool acpi_video_backlight_quirks(void) { return false; }
> > +static inline bool acpi_video_verify_backlight_support(void) { return
> > false; }
> > #endif
> >
> > #endif /* _ACPI_INTERNAL_H_ */
> > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> > index 5ad5a71..cc709a7 100644
> > --- a/drivers/acpi/video.c
> > +++ b/drivers/acpi/video.c
> > @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
> > *device, int event)
> > unsigned long long
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, Sep 09, 2013 at 11:32:10AM +0200, 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? I've grown _very_
Sorry, no general rule has been found. As Rafael has said, it appears
to be random... And in addition to the shipped with Win7 with a Win8
upgrade option case, I also find a Sony laptop that has Win8
pre-installed and a broken ACPI video backlight interface. Its ACPI
video backlight control method is simply a stub, so even the
acpi_osi=!Windows 2012 will not provide a working backlight for this
system.
-Aaron
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 ...
Cheers, Daniel
On Mon, Sep 09, 2013 at 04:42:20PM +0800, Aaron Lu wrote:
According to Matthew Garrett, Windows 8 leaves backlight control up
to individual graphics drivers rather than making ACPI calls itself.
There's plenty of evidence to suggest that the Intel driver for
Windows [8] doesn't use the ACPI interface, including the fact that
it's broken on a bunch of machines when the OS claims to support
Windows 8. The simplest thing to do appears to be to disable the
ACPI backlight interface on these systems.
There's a problem with that approach, however, because simply
avoiding to register the ACPI backlight interface if the firmware
calls _OSI for Windows 8 may not work in the following situations:
(1) The ACPI backlight interface actually works on the given system
and the i915 driver is not loaded (e.g. another graphics driver
is used).
(2) The ACPI backlight interface doesn't work on the given system,
but there is a vendor platform driver that will register its
own, equally broken, backlight interface if not prevented from
doing so by the ACPI subsystem.
Therefore we need to allow the ACPI backlight interface to be
registered until the i915 driver is loaded which then will unregister
it if the firmware has called _OSI for Windows 8 (or will register
the ACPI video driver without backlight support if not already
present).
For this reason, introduce an alternative function for registering
ACPI video, __acpi_video_register(bool), that if ture is passed,
will check whether or not the ACPI video driver has already been
registered and whether or not the backlight Windows 8 quirk has to
be applied. If the quirk has to be applied, it will block the ACPI
backlight support and either unregister the backlight interface if
the ACPI video driver has already been registered, or register the
ACPI video driver without the backlight interface otherwise. Make
the i915 driver use __acpi_video_register() instead of
acpi_video_register() in i915_driver_load(), and the param passed
there is controlled by the i915 module level parameter
i915_take_over_backlight, which is set to false by default.
This change is evolved from earlier patches of Matthew Garrett,
Chun-Yi Lee and Seth Forshee and is heavily based on two patches
from Rafael:
https://lkml.org/lkml/2013/7/17/720
https://lkml.org/lkml/2013/7/24/806
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/acpi/internal.h | 2 ++
drivers/acpi/video.c| 24
drivers/acpi/video_detect.c | 15 ++-
drivers/gpu/drm/i915/i915_dma.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 5 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
include/acpi/video.h| 9 +++--
include/linux/acpi.h| 1 +
8 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..a53832e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -170,8 +170,10 @@ int acpi_create_platform_device(struct acpi_device
*adev,
--
*/
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
bool acpi_video_backlight_quirks(void);
+bool acpi_video_verify_backlight_support(void);
#else
static inline bool acpi_video_backlight_quirks(void) { return false; }
+static inline bool acpi_video_verify_backlight_support(void) { return
false; }
#endif
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5ad5a71..cc709a7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 09 Sep 2013, Rafael J. Wysocki r...@sisk.pl 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
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: I think the parameter Does the ACPI backlight interface work or not belongs to the ACPI video driver. That depends on how Windows 8 works. If Windows 8 policy is handled by the GPU drivers then it belongs in i915. If it's handled by the ACPI code then it belongs in the ACPI code. But I have no way of determining that, whereas you work for a company that produces a Windows 8 video driver… -- Matthew Garrett matthew.garr...@nebula.com
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 10 Sep 2013, Matthew Garrett matthew.garr...@nebula.com wrote: On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: I think the parameter Does the ACPI backlight interface work or not belongs to the ACPI video driver. That depends on how Windows 8 works. If Windows 8 policy is handled by the GPU drivers then it belongs in i915. If it's handled by the ACPI code then it belongs in the ACPI code. I fail to see the logic. Windows 8 policy dictates whether we can use the AML code or not. IMHO, ACPI code is in the best position to figure this out and quirk as necessary. It's the part that knows about Windows 8, not i915. But I have no way of determining that, whereas you work for a company that produces a Windows 8 video driver… Here I do see the logic. However see signature; I'm not in the best of positions to figure out things about Windows 8 video drivers. ;) But I'll see what I can do. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 17:21 +0300, Jani Nikula wrote: On Tue, 10 Sep 2013, Matthew Garrett matthew.garr...@nebula.com wrote: On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: I think the parameter Does the ACPI backlight interface work or not belongs to the ACPI video driver. That depends on how Windows 8 works. If Windows 8 policy is handled by the GPU drivers then it belongs in i915. If it's handled by the ACPI code then it belongs in the ACPI code. I fail to see the logic. Windows 8 policy dictates whether we can use the AML code or not. IMHO, ACPI code is in the best position to figure this out and quirk as necessary. It's the part that knows about Windows 8, not i915. So if nvidia hardware uses the ACPI interface and Intel doesn't, we should still quirk it in the ACPI driver? -- Matthew Garrett matthew.garr...@nebula.com
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tuesday, September 10, 2013 04:53:40 PM Jani Nikula wrote: On Mon, 09 Sep 2013, Rafael J. Wysocki r...@sisk.pl 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
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, Sep 10, 2013 at 09:23:04PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 10, 2013 04:53:40 PM Jani Nikula wrote: On Mon, 09 Sep 2013, Rafael J. Wysocki r...@sisk.pl 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
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/10/2013 01:22 PM, Igor Gnatenko wrote:
> On Tue, 2013-09-10 at 13:16 +0800, Aaron Lu wrote:
>> On 09/10/2013 01:13 PM, Igor Gnatenko wrote:
>>> On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
> On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c
>> b/drivers/gpu/drm/i915/i915_dma.c
>> index f466980..75fba17 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
>> unsigned long flags)
>> if (INTEL_INFO(dev)->num_pipes) {
>> /* Must be done after probing outputs */
>> intel_opregion_init(dev);
>> -acpi_video_register();
>> +__acpi_video_register(i915_take_over_backlight);
>> }
>>
>> if (IS_GEN5(dev))
>
> I can't compile:
>
>
> DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
> DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
> declaration of function
> '__acpi_video_register' [-Werror=implicit-function-declaration]
> DEBUG:__acpi_video_register(i915_take_over_backlight);
> DEBUG:^
> DEBUG: cc1: some warnings being treated as errors
> DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
> DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
> DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
> DEBUG: make[1]: *** [drivers/gpu] Error 2
> DEBUG: make: *** [drivers] Error 2
>
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
>>>
>>> Thanks. this patch fixes my problems w/ compilation. I've tested this
>>> two patches and after apply I have:
>>> $ tree /sys/class/backlight/
>>> /sys/class/backlight/
>>> |-- acpi_video0
>>> -> ../../devices/pci:00/:00:02.0/backlight/acpi_video0
>>> `-- intel_backlight
>>> ->
>>> ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
>>>
>>> 2 directories, 0 files
>>>
>>> I think it's didn't unregistered.. I may forget. I need to apply one of
>>> patch from Matthew ?
>>
>> You need to specify i915.take_over_backlight=1 in kernel cmdline, that
>> module option is set to false by default for now.
>>
>> Thanks for the test.
>>
>> -Aaron
>>
>>>
>>> Some strings from logs:
>>> DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
>>> thinkpad_acpi: Standard ACPI backlight interface available, not loading
>>> native one
>>>
>>
>
> Thanks for quick answer. Yes. This option do unregister. Thanks. but for
> this patch-set I also need "[PATCH 2/3] ACPI / video: Always call
> acpi_video_init_brightness() on init" from Matthew (for notifications in
> DE).
That patch is reverted as it cause problem for other system:
https://bugs.freedesktop.org/show_bug.cgi?id=68355
OTOH, the thinkpad-acpi module already has a call to _BCL except that
the tpacpi_acpi_handle_locate failed to locate video controller's handle:
https://bugzilla.kernel.org/show_bug.cgi?id=51231#c121
I'll see if I can figure out why.
Thanks,
Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote: > According to Matthew Garrett, "Windows 8 leaves backlight control up > to individual graphics drivers rather than making ACPI calls itself. > There's plenty of evidence to suggest that the Intel driver for > Windows [8] doesn't use the ACPI interface, including the fact that > it's broken on a bunch of machines when the OS claims to support > Windows 8. The simplest thing to do appears to be to disable the > ACPI backlight interface on these systems". > > There's a problem with that approach, however, because simply > avoiding to register the ACPI backlight interface if the firmware > calls _OSI for Windows 8 may not work in the following situations: > (1) The ACPI backlight interface actually works on the given system > and the i915 driver is not loaded (e.g. another graphics driver > is used). > (2) The ACPI backlight interface doesn't work on the given system, > but there is a vendor platform driver that will register its > own, equally broken, backlight interface if not prevented from > doing so by the ACPI subsystem. > Therefore we need to allow the ACPI backlight interface to be > registered until the i915 driver is loaded which then will unregister > it if the firmware has called _OSI for Windows 8 (or will register > the ACPI video driver without backlight support if not already > present). > > For this reason, introduce an alternative function for registering > ACPI video, __acpi_video_register(bool), that if ture is passed, > will check whether or not the ACPI video driver has already been > registered and whether or not the backlight Windows 8 quirk has to > be applied. If the quirk has to be applied, it will block the ACPI > backlight support and either unregister the backlight interface if > the ACPI video driver has already been registered, or register the > ACPI video driver without the backlight interface otherwise. Make > the i915 driver use __acpi_video_register() instead of > acpi_video_register() in i915_driver_load(), and the param passed > there is controlled by the i915 module level parameter > i915_take_over_backlight, which is set to false by default. > > This change is evolved from earlier patches of Matthew Garrett, > Chun-Yi Lee and Seth Forshee and is heavily based on two patches > from Rafael: > https://lkml.org/lkml/2013/7/17/720 > https://lkml.org/lkml/2013/7/24/806 > > Signed-off-by: Aaron Lu Tested-by: Igor Gnatenko > --- > drivers/acpi/internal.h | 2 ++ > drivers/acpi/video.c| 24 > drivers/acpi/video_detect.c | 15 ++- > drivers/gpu/drm/i915/i915_dma.c | 2 +- > drivers/gpu/drm/i915/i915_drv.c | 5 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > include/acpi/video.h| 9 +++-- > include/linux/acpi.h| 1 + > 8 files changed, 47 insertions(+), 12 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-3.fc20.x86_64 -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-1.fc20.x86_64 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 13:16 +0800, Aaron Lu wrote:
> On 09/10/2013 01:13 PM, Igor Gnatenko wrote:
> > On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
> >> On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
> >>> On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index f466980..75fba17 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
> unsigned long flags)
> if (INTEL_INFO(dev)->num_pipes) {
> /* Must be done after probing outputs */
> intel_opregion_init(dev);
> -acpi_video_register();
> +__acpi_video_register(i915_take_over_backlight);
> }
>
> if (IS_GEN5(dev))
> >>>
> >>> I can't compile:
> >>>
> >>>
> >>> DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
> >>> DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
> >>> declaration of function
> >>> '__acpi_video_register' [-Werror=implicit-function-declaration]
> >>> DEBUG:__acpi_video_register(i915_take_over_backlight);
> >>> DEBUG:^
> >>> DEBUG: cc1: some warnings being treated as errors
> >>> DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
> >>> DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
> >>> DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
> >>> DEBUG: make[1]: *** [drivers/gpu] Error 2
> >>> DEBUG: make: *** [drivers] Error 2
> >>>
> >>
> >> The two patches are based on top of Rafael's linux-next tree. I just
> >> tried it again, no compile problem for me. I also tried on today Linus'
> >> master tree, as there are some updates from i915, two conflicts exist.
> >> I've just resolved them and will update it in next revision.
> >> If you want to try it now, please use:
> >> https://github.com/aaronlu/linux acpi_video_rework
> >>
> >> Thanks,
> >> Aaron
> >
> > Thanks. this patch fixes my problems w/ compilation. I've tested this
> > two patches and after apply I have:
> > $ tree /sys/class/backlight/
> > /sys/class/backlight/
> > |-- acpi_video0
> > -> ../../devices/pci:00/:00:02.0/backlight/acpi_video0
> > `-- intel_backlight
> > ->
> > ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
> >
> > 2 directories, 0 files
> >
> > I think it's didn't unregistered.. I may forget. I need to apply one of
> > patch from Matthew ?
>
> You need to specify i915.take_over_backlight=1 in kernel cmdline, that
> module option is set to false by default for now.
>
> Thanks for the test.
>
> -Aaron
>
> >
> > Some strings from logs:
> > DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
> > thinkpad_acpi: Standard ACPI backlight interface available, not loading
> > native one
> >
>
Thanks for quick answer. Yes. This option do unregister. Thanks. but for
this patch-set I also need "[PATCH 2/3] ACPI / video: Always call
acpi_video_init_brightness() on init" from Matthew (for notifications in
DE).
--
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.0-1.fc20.x86_64
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/10/2013 01:13 PM, Igor Gnatenko wrote:
> On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
>> On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
>>> On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (INTEL_INFO(dev)->num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
- acpi_video_register();
+ __acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
>>>
>>> I can't compile:
>>>
>>>
>>> DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
>>> DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
>>> declaration of function
>>> '__acpi_video_register' [-Werror=implicit-function-declaration]
>>> DEBUG:__acpi_video_register(i915_take_over_backlight);
>>> DEBUG:^
>>> DEBUG: cc1: some warnings being treated as errors
>>> DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
>>> DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
>>> DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
>>> DEBUG: make[1]: *** [drivers/gpu] Error 2
>>> DEBUG: make: *** [drivers] Error 2
>>>
>>
>> The two patches are based on top of Rafael's linux-next tree. I just
>> tried it again, no compile problem for me. I also tried on today Linus'
>> master tree, as there are some updates from i915, two conflicts exist.
>> I've just resolved them and will update it in next revision.
>> If you want to try it now, please use:
>> https://github.com/aaronlu/linux acpi_video_rework
>>
>> Thanks,
>> Aaron
>
> Thanks. this patch fixes my problems w/ compilation. I've tested this
> two patches and after apply I have:
> $ tree /sys/class/backlight/
> /sys/class/backlight/
> |-- acpi_video0
> -> ../../devices/pci:00/:00:02.0/backlight/acpi_video0
> `-- intel_backlight
> ->
> ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
>
> 2 directories, 0 files
>
> I think it's didn't unregistered.. I may forget. I need to apply one of
> patch from Matthew ?
You need to specify i915.take_over_backlight=1 in kernel cmdline, that
module option is set to false by default for now.
Thanks for the test.
-Aaron
>
> Some strings from logs:
> DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
> thinkpad_acpi: Standard ACPI backlight interface available, not loading
> native one
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
> On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
> > On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> >> b/drivers/gpu/drm/i915/i915_dma.c
> >> index f466980..75fba17 100644
> >> --- a/drivers/gpu/drm/i915/i915_dma.c
> >> +++ b/drivers/gpu/drm/i915/i915_dma.c
> >> @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
> >> unsigned long flags)
> >>if (INTEL_INFO(dev)->num_pipes) {
> >>/* Must be done after probing outputs */
> >>intel_opregion_init(dev);
> >> - acpi_video_register();
> >> + __acpi_video_register(i915_take_over_backlight);
> >>}
> >>
> >>if (IS_GEN5(dev))
> >
> > I can't compile:
> >
> >
> > DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
> > DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
> > declaration of function
> > '__acpi_video_register' [-Werror=implicit-function-declaration]
> > DEBUG:__acpi_video_register(i915_take_over_backlight);
> > DEBUG:^
> > DEBUG: cc1: some warnings being treated as errors
> > DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
> > DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
> > DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
> > DEBUG: make[1]: *** [drivers/gpu] Error 2
> > DEBUG: make: *** [drivers] Error 2
> >
>
> The two patches are based on top of Rafael's linux-next tree. I just
> tried it again, no compile problem for me. I also tried on today Linus'
> master tree, as there are some updates from i915, two conflicts exist.
> I've just resolved them and will update it in next revision.
> If you want to try it now, please use:
> https://github.com/aaronlu/linux acpi_video_rework
>
> Thanks,
> Aaron
Thanks. this patch fixes my problems w/ compilation. I've tested this
two patches and after apply I have:
$ tree /sys/class/backlight/
/sys/class/backlight/
|-- acpi_video0
-> ../../devices/pci:00/:00:02.0/backlight/acpi_video0
`-- intel_backlight
-> ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
2 directories, 0 files
I think it's didn't unregistered.. I may forget. I need to apply one of
patch from Matthew ?
Some strings from logs:
DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
thinkpad_acpi: Standard ACPI backlight interface available, not loading
native one
--
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.0-1.fc20.x86_64
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
> On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c
>> b/drivers/gpu/drm/i915/i915_dma.c
>> index f466980..75fba17 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
>> long flags)
>> if (INTEL_INFO(dev)->num_pipes) {
>> /* Must be done after probing outputs */
>> intel_opregion_init(dev);
>> -acpi_video_register();
>> +__acpi_video_register(i915_take_over_backlight);
>> }
>>
>> if (IS_GEN5(dev))
>
> I can't compile:
>
>
> DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
> DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
> declaration of function
> '__acpi_video_register' [-Werror=implicit-function-declaration]
> DEBUG:__acpi_video_register(i915_take_over_backlight);
> DEBUG:^
> DEBUG: cc1: some warnings being treated as errors
> DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
> DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
> DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
> DEBUG: make[1]: *** [drivers/gpu] Error 2
> DEBUG: make: *** [drivers] Error 2
>
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 17:21 +0200, Daniel Vetter wrote: > 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. Windows 8 isn't broken because (as far as we can tell) the Intel drivers under Windows 8 never use the ACPI backlight set function. Of course, it would be nice to have that confirmed by Intel. -- Matthew Garrett
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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. Kind regards, Rafael > On Mon, Sep 09, 2013 at 04:42:20PM +0800, Aaron Lu wrote: > > According to Matthew Garrett, "Windows 8 leaves backlight control up > > to individual graphics drivers rather than making ACPI calls itself. > > There's plenty of evidence to suggest that the Intel driver for > > Windows [8] doesn't use the ACPI interface, including the fact that > > it's broken on a bunch of machines when the OS claims to support > > Windows 8. The simplest thing to do appears to be to disable the > > ACPI backlight interface on these systems". > > > > There's a problem with that approach, however, because simply > > avoiding to register the ACPI backlight interface if the firmware > > calls _OSI for Windows 8 may not work in the following situations: > > (1) The ACPI backlight interface actually works on the given system > > and the i915 driver is not loaded (e.g. another graphics driver > > is used). > > (2) The ACPI backlight interface doesn't work on the given system, > > but there is a vendor platform driver that will register its > > own, equally broken, backlight interface if not prevented from > > doing so by the ACPI subsystem. > > Therefore we need to allow the ACPI backlight interface to be > > registered until the i915 driver is loaded which then will unregister > > it if the firmware has called _OSI for Windows 8 (or will register > > the ACPI video driver without backlight support if not already > > present). > > > > For this reason, introduce an alternative function for registering > > ACPI video, __acpi_video_register(bool), that if ture is passed, > > will check whether or not the ACPI video driver has already been > > registered and whether or not the backlight Windows 8 quirk has to > > be applied. If the quirk has to be applied, it will block the ACPI > > backlight support and either unregister the backlight interface if > > the ACPI video driver has already been registered, or register the > > ACPI video driver without the backlight interface otherwise. Make > > the i915 driver use __acpi_video_register() instead of > > acpi_video_register() in i915_driver_load(), and the param passed > > there is controlled by the i915 module level parameter > > i915_take_over_backlight, which is set to false by default. > > > > This change is evolved from earlier patches of Matthew Garrett, > > Chun-Yi Lee and Seth Forshee and is heavily based on two patches > > from Rafael: > >
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f466980..75fba17 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
> if (INTEL_INFO(dev)->num_pipes) {
> /* Must be done after probing outputs */
> intel_opregion_init(dev);
> - acpi_video_register();
> + __acpi_video_register(i915_take_over_backlight);
> }
>
> if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
--
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.0-3.fc20.x86_64
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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? 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 ...
Cheers, Daniel
On Mon, Sep 09, 2013 at 04:42:20PM +0800, Aaron Lu wrote:
> According to Matthew Garrett, "Windows 8 leaves backlight control up
> to individual graphics drivers rather than making ACPI calls itself.
> There's plenty of evidence to suggest that the Intel driver for
> Windows [8] doesn't use the ACPI interface, including the fact that
> it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the
> ACPI backlight interface on these systems".
>
> There's a problem with that approach, however, because simply
> avoiding to register the ACPI backlight interface if the firmware
> calls _OSI for Windows 8 may not work in the following situations:
> (1) The ACPI backlight interface actually works on the given system
> and the i915 driver is not loaded (e.g. another graphics driver
> is used).
> (2) The ACPI backlight interface doesn't work on the given system,
> but there is a vendor platform driver that will register its
> own, equally broken, backlight interface if not prevented from
> doing so by the ACPI subsystem.
> Therefore we need to allow the ACPI backlight interface to be
> registered until the i915 driver is loaded which then will unregister
> it if the firmware has called _OSI for Windows 8 (or will register
> the ACPI video driver without backlight support if not already
> present).
>
> For this reason, introduce an alternative function for registering
> ACPI video, __acpi_video_register(bool), that if ture is passed,
> will check whether or not the ACPI video driver has already been
> registered and whether or not the backlight Windows 8 quirk has to
> be applied. If the quirk has to be applied, it will block the ACPI
> backlight support and either unregister the backlight interface if
> the ACPI video driver has already been registered, or register the
> ACPI video driver without the backlight interface otherwise. Make
> the i915 driver use __acpi_video_register() instead of
> acpi_video_register() in i915_driver_load(), and the param passed
> there is controlled by the i915 module level parameter
> i915_take_over_backlight, which is set to false by default.
>
> This change is evolved from earlier patches of Matthew Garrett,
> Chun-Yi Lee and Seth Forshee and is heavily based on two patches
> from Rafael:
> https://lkml.org/lkml/2013/7/17/720
> https://lkml.org/lkml/2013/7/24/806
>
> Signed-off-by: Aaron Lu
> ---
> drivers/acpi/internal.h | 2 ++
> drivers/acpi/video.c| 24
> drivers/acpi/video_detect.c | 15 ++-
> drivers/gpu/drm/i915/i915_dma.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 5 +
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> include/acpi/video.h| 9 +++--
> include/linux/acpi.h| 1 +
> 8 files changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 20f4233..a53832e 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -170,8 +170,10 @@ int acpi_create_platform_device(struct acpi_device *adev,
>--
> */
> #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
> bool acpi_video_backlight_quirks(void);
> +bool acpi_video_verify_backlight_support(void);
> #else
> static inline bool acpi_video_backlight_quirks(void) { return false; }
> +static inline bool acpi_video_verify_backlight_support(void) { return false;
> }
> #endif
>
> #endif /* _ACPI_INTERNAL_H_ */
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 5ad5a71..cc709a7 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
> *device, int event)
> unsigned long long level_current, level_next;
> int result = -EINVAL;
>
> - /* no warning message if acpi_backlight=vendor is used */
> - if (!acpi_video_backlight_support())
> + /* no warning message if acpi_backlight=vendor or a quirk is used */
> + if (!acpi_video_verify_backlight_support())
> return 0;
>
> if (!device->brightness)
> @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level,
> void *context,
>
> static void acpi_video_dev_register_backlight(struct acpi_video_device
> *device)
> {
> - if (acpi_video_backlight_support()) {
> + if (acpi_video_verify_backlight_support()) {
> struct
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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? 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 ...
Cheers, Daniel
On Mon, Sep 09, 2013 at 04:42:20PM +0800, Aaron Lu wrote:
According to Matthew Garrett, Windows 8 leaves backlight control up
to individual graphics drivers rather than making ACPI calls itself.
There's plenty of evidence to suggest that the Intel driver for
Windows [8] doesn't use the ACPI interface, including the fact that
it's broken on a bunch of machines when the OS claims to support
Windows 8. The simplest thing to do appears to be to disable the
ACPI backlight interface on these systems.
There's a problem with that approach, however, because simply
avoiding to register the ACPI backlight interface if the firmware
calls _OSI for Windows 8 may not work in the following situations:
(1) The ACPI backlight interface actually works on the given system
and the i915 driver is not loaded (e.g. another graphics driver
is used).
(2) The ACPI backlight interface doesn't work on the given system,
but there is a vendor platform driver that will register its
own, equally broken, backlight interface if not prevented from
doing so by the ACPI subsystem.
Therefore we need to allow the ACPI backlight interface to be
registered until the i915 driver is loaded which then will unregister
it if the firmware has called _OSI for Windows 8 (or will register
the ACPI video driver without backlight support if not already
present).
For this reason, introduce an alternative function for registering
ACPI video, __acpi_video_register(bool), that if ture is passed,
will check whether or not the ACPI video driver has already been
registered and whether or not the backlight Windows 8 quirk has to
be applied. If the quirk has to be applied, it will block the ACPI
backlight support and either unregister the backlight interface if
the ACPI video driver has already been registered, or register the
ACPI video driver without the backlight interface otherwise. Make
the i915 driver use __acpi_video_register() instead of
acpi_video_register() in i915_driver_load(), and the param passed
there is controlled by the i915 module level parameter
i915_take_over_backlight, which is set to false by default.
This change is evolved from earlier patches of Matthew Garrett,
Chun-Yi Lee and Seth Forshee and is heavily based on two patches
from Rafael:
https://lkml.org/lkml/2013/7/17/720
https://lkml.org/lkml/2013/7/24/806
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/acpi/internal.h | 2 ++
drivers/acpi/video.c| 24
drivers/acpi/video_detect.c | 15 ++-
drivers/gpu/drm/i915/i915_dma.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 5 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
include/acpi/video.h| 9 +++--
include/linux/acpi.h| 1 +
8 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 20f4233..a53832e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -170,8 +170,10 @@ int acpi_create_platform_device(struct acpi_device *adev,
--
*/
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
bool acpi_video_backlight_quirks(void);
+bool acpi_video_verify_backlight_support(void);
#else
static inline bool acpi_video_backlight_quirks(void) { return false; }
+static inline bool acpi_video_verify_backlight_support(void) { return false;
}
#endif
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5ad5a71..cc709a7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
unsigned long long level_current, level_next;
int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ /* no warning message if acpi_backlight=vendor or a quirk is used */
+ if (!acpi_video_verify_backlight_support())
return 0;
if (!device-brightness)
@@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level,
void *context,
static void acpi_video_dev_register_backlight(struct acpi_video_device
*device)
{
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
if (INTEL_INFO(dev)-num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
- acpi_video_register();
+ __acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
--
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.0-3.fc20.x86_64
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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. Kind regards, Rafael On Mon, Sep 09, 2013 at 04:42:20PM +0800, Aaron Lu wrote: According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. There's a problem with that approach, however, because simply avoiding to register the ACPI backlight interface if the firmware calls _OSI for Windows 8 may not work in the following situations: (1) The ACPI backlight interface actually works on the given system and the i915 driver is not loaded (e.g. another graphics driver is used). (2) The ACPI backlight interface doesn't work on the given system, but there is a vendor platform driver that will register its own, equally broken, backlight interface if not prevented from doing so by the ACPI subsystem. Therefore we need to allow the ACPI backlight interface to be registered until the i915 driver is loaded which then will unregister it if the firmware has called _OSI for Windows 8 (or will register the ACPI video driver without backlight support if not already present). For this reason, introduce an alternative function for registering ACPI video, __acpi_video_register(bool), that if ture is passed, will check whether or not the ACPI video driver has already been registered and whether or not the backlight Windows 8 quirk has to be applied. If the quirk has to be applied, it will block the ACPI backlight support and either unregister the backlight interface if the ACPI video driver has already been registered, or register the ACPI video driver without the backlight interface otherwise. Make the i915 driver use __acpi_video_register() instead of acpi_video_register() in i915_driver_load(), and the param passed there is controlled by the i915 module level parameter i915_take_over_backlight, which is set to false by default. This change is evolved from earlier patches of Matthew Garrett, Chun-Yi Lee and Seth Forshee and is heavily based on two patches from Rafael: https://lkml.org/lkml/2013/7/17/720 https://lkml.org/lkml/2013/7/24/806 Signed-off-by: Aaron Lu
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 17:21 +0200, Daniel Vetter wrote: 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. Windows 8 isn't broken because (as far as we can tell) the Intel drivers under Windows 8 never use the ACPI backlight set function. Of course, it would be nice to have that confirmed by Intel. -- Matthew Garrett matthew.garr...@nebula.com
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
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. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
if (INTEL_INFO(dev)-num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
-acpi_video_register();
+__acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (INTEL_INFO(dev)-num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
- acpi_video_register();
+ __acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
Thanks. this patch fixes my problems w/ compilation. I've tested this
two patches and after apply I have:
$ tree /sys/class/backlight/
/sys/class/backlight/
|-- acpi_video0
- ../../devices/pci:00/:00:02.0/backlight/acpi_video0
`-- intel_backlight
- ../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
2 directories, 0 files
I think it's didn't unregistered.. I may forget. I need to apply one of
patch from Matthew ?
Some strings from logs:
DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
thinkpad_acpi: Standard ACPI backlight interface available, not loading
native one
--
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.0-1.fc20.x86_64
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/10/2013 01:13 PM, Igor Gnatenko wrote:
On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (INTEL_INFO(dev)-num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
- acpi_video_register();
+ __acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
Thanks. this patch fixes my problems w/ compilation. I've tested this
two patches and after apply I have:
$ tree /sys/class/backlight/
/sys/class/backlight/
|-- acpi_video0
- ../../devices/pci:00/:00:02.0/backlight/acpi_video0
`-- intel_backlight
-
../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
2 directories, 0 files
I think it's didn't unregistered.. I may forget. I need to apply one of
patch from Matthew ?
You need to specify i915.take_over_backlight=1 in kernel cmdline, that
module option is set to false by default for now.
Thanks for the test.
-Aaron
Some strings from logs:
DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
thinkpad_acpi: Standard ACPI backlight interface available, not loading
native one
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 13:16 +0800, Aaron Lu wrote:
On 09/10/2013 01:13 PM, Igor Gnatenko wrote:
On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (INTEL_INFO(dev)-num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
-acpi_video_register();
+__acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
Thanks. this patch fixes my problems w/ compilation. I've tested this
two patches and after apply I have:
$ tree /sys/class/backlight/
/sys/class/backlight/
|-- acpi_video0
- ../../devices/pci:00/:00:02.0/backlight/acpi_video0
`-- intel_backlight
-
../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
2 directories, 0 files
I think it's didn't unregistered.. I may forget. I need to apply one of
patch from Matthew ?
You need to specify i915.take_over_backlight=1 in kernel cmdline, that
module option is set to false by default for now.
Thanks for the test.
-Aaron
Some strings from logs:
DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
thinkpad_acpi: Standard ACPI backlight interface available, not loading
native one
Thanks for quick answer. Yes. This option do unregister. Thanks. but for
this patch-set I also need [PATCH 2/3] ACPI / video: Always call
acpi_video_init_brightness() on init from Matthew (for notifications in
DE).
--
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.0-1.fc20.x86_64
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote: According to Matthew Garrett, Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. There's a problem with that approach, however, because simply avoiding to register the ACPI backlight interface if the firmware calls _OSI for Windows 8 may not work in the following situations: (1) The ACPI backlight interface actually works on the given system and the i915 driver is not loaded (e.g. another graphics driver is used). (2) The ACPI backlight interface doesn't work on the given system, but there is a vendor platform driver that will register its own, equally broken, backlight interface if not prevented from doing so by the ACPI subsystem. Therefore we need to allow the ACPI backlight interface to be registered until the i915 driver is loaded which then will unregister it if the firmware has called _OSI for Windows 8 (or will register the ACPI video driver without backlight support if not already present). For this reason, introduce an alternative function for registering ACPI video, __acpi_video_register(bool), that if ture is passed, will check whether or not the ACPI video driver has already been registered and whether or not the backlight Windows 8 quirk has to be applied. If the quirk has to be applied, it will block the ACPI backlight support and either unregister the backlight interface if the ACPI video driver has already been registered, or register the ACPI video driver without the backlight interface otherwise. Make the i915 driver use __acpi_video_register() instead of acpi_video_register() in i915_driver_load(), and the param passed there is controlled by the i915 module level parameter i915_take_over_backlight, which is set to false by default. This change is evolved from earlier patches of Matthew Garrett, Chun-Yi Lee and Seth Forshee and is heavily based on two patches from Rafael: https://lkml.org/lkml/2013/7/17/720 https://lkml.org/lkml/2013/7/24/806 Signed-off-by: Aaron Lu aaron...@intel.com Tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com --- drivers/acpi/internal.h | 2 ++ drivers/acpi/video.c| 24 drivers/acpi/video_detect.c | 15 ++- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 5 + drivers/gpu/drm/i915/i915_drv.h | 1 + include/acpi/video.h| 9 +++-- include/linux/acpi.h| 1 + 8 files changed, 47 insertions(+), 12 deletions(-) -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-3.fc20.x86_64 -- Igor Gnatenko Fedora release 20 (Heisenbug) Linux 3.11.0-1.fc20.x86_64 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 09/10/2013 01:22 PM, Igor Gnatenko wrote:
On Tue, 2013-09-10 at 13:16 +0800, Aaron Lu wrote:
On 09/10/2013 01:13 PM, Igor Gnatenko wrote:
On Tue, 2013-09-10 at 11:27 +0800, Aaron Lu wrote:
On 09/09/2013 07:44 PM, Igor Gnatenko wrote:
On Mon, 2013-09-09 at 16:42 +0800, Aaron Lu wrote:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index f466980..75fba17 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1650,7 +1650,7 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (INTEL_INFO(dev)-num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
-acpi_video_register();
+__acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
I can't compile:
DEBUG: drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load':
DEBUG: drivers/gpu/drm/i915/i915_dma.c:1661:3: error: implicit
declaration of function
'__acpi_video_register' [-Werror=implicit-function-declaration]
DEBUG:__acpi_video_register(i915_take_over_backlight);
DEBUG:^
DEBUG: cc1: some warnings being treated as errors
DEBUG: make[4]: *** [drivers/gpu/drm/i915/i915_dma.o] Error 1
DEBUG: make[3]: *** [drivers/gpu/drm/i915] Error 2
DEBUG: make[2]: *** [drivers/gpu/drm] Error 2
DEBUG: make[1]: *** [drivers/gpu] Error 2
DEBUG: make: *** [drivers] Error 2
The two patches are based on top of Rafael's linux-next tree. I just
tried it again, no compile problem for me. I also tried on today Linus'
master tree, as there are some updates from i915, two conflicts exist.
I've just resolved them and will update it in next revision.
If you want to try it now, please use:
https://github.com/aaronlu/linux acpi_video_rework
Thanks,
Aaron
Thanks. this patch fixes my problems w/ compilation. I've tested this
two patches and after apply I have:
$ tree /sys/class/backlight/
/sys/class/backlight/
|-- acpi_video0
- ../../devices/pci:00/:00:02.0/backlight/acpi_video0
`-- intel_backlight
-
../../devices/pci:00/:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
2 directories, 0 files
I think it's didn't unregistered.. I may forget. I need to apply one of
patch from Matthew ?
You need to specify i915.take_over_backlight=1 in kernel cmdline, that
module option is set to false by default for now.
Thanks for the test.
-Aaron
Some strings from logs:
DMI: LENOVO 23205NG/23205NG, BIOS G2ET92WW (2.52 ) 02/22/2013
thinkpad_acpi: Standard ACPI backlight interface available, not loading
native one
Thanks for quick answer. Yes. This option do unregister. Thanks. but for
this patch-set I also need [PATCH 2/3] ACPI / video: Always call
acpi_video_init_brightness() on init from Matthew (for notifications in
DE).
That patch is reverted as it cause problem for other system:
https://bugs.freedesktop.org/show_bug.cgi?id=68355
OTOH, the thinkpad-acpi module already has a call to _BCL except that
the tpacpi_acpi_handle_locate failed to locate video controller's handle:
https://bugzilla.kernel.org/show_bug.cgi?id=51231#c121
I'll see if I can figure out why.
Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
