| Message ID | 1426150247-18309-1-git-send-email-matthew.garrett@nebula.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
On 03/12/2015 02:50 AM, Matthew Garrett wrote: > The ACPI spec describes _REV as: > > "This predefined object evaluates to the revision of the ACPI Specification > that the specified \_OS implements" > > We've been assuming that this should increment as ACPICA gains support for > new versions of the spec. Unfortunately, Windows always reports "2" for this > value and vendors are now using this as a means to tell whether a system is > running Windows or Linux. From an HP Envy 15: > > If (LOr (LEqual (_REV, 0x03), LEqual (_REV, 0x05))) > > From a Dell XPS 13: > > If ((_REV == 0x05)) > > In both cases, the systems then alter hardware initialisation paths and > device capability reporting. As written, this makes no sense - ACPI > maintains backwards compatibility, so the appropriate test would be >= > rather than ==. On closer examination, the HP code uses the same > initialisation paths as would be used if the system responded to > _OSI("Linux"), and as such the evidence is overwhelmingly that vendors are > using this to alter system behaviour when Linux is detected. > > Since we aim to be compatible with Windows, this tends to break things. The > ideal solution would be to wait for an _OSI() query matching Windows and > then change behaviour, but Windows responds to _REV with 2 even before that > and so vendors might simply change the order of queries in order to continue > IDing Linux. The easiest thing for us to do is just to change the value on > X86 and leave a comment describing why everything is so awful. > > Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org> > --- > include/acpi/acconfig.h | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h > index 03aacfb..cebb8a7 100644 > --- a/include/acpi/acconfig.h > +++ b/include/acpi/acconfig.h > @@ -112,9 +112,19 @@ > * > *****************************************************************************/ > > -/* Version of ACPI supported */ > - > +/* > + * Version of ACPI supported. This is a sad story. Windows reports a _REV of > + * 2 regardless of the spec version implemented. Some vendors are using _REV > + * as a way to distinguish between Windows and Linux, and are breaking systems > + * in the process. We can't guarantee that they'll call _OSI before checking > + * _REV, so hardcode this to 2 on x86 systems right now and leave it at the > + * appropriate spec value for everybody else. > + */ > +#ifdef CONFIG_X86 > +#define ACPI_CA_SUPPORT_LEVEL 2 > +#else > #define ACPI_CA_SUPPORT_LEVEL 5 > +#endif > > /* Maximum count for a semaphore object */ > > More a philosophical question -- the patch seems fine to me, personally, and for arm64, we have to have >= 5 anyway -- but would it make sense to just not acknowledge _REV and deprecate it from the kernel and the spec? I'm already trying to get rid of _OSI because of such silliness and force requests to _OSC where they should be (granted, it will take some time...). It just seems to be that there should be some consequences to the vendors when they do things like this, and while I'm not that keen to break existing things, maybe that's what needs to happen to make the point.
On Mon, Mar 16, 2015 at 02:34:24PM -0600, Al Stone wrote: > More a philosophical question -- the patch seems fine to me, personally, and > for arm64, we have to have >= 5 anyway -- but would it make sense to just not > acknowledge _REV and deprecate it from the kernel and the spec? I'm already > trying to get rid of _OSI because of such silliness and force requests to _OSC > where they should be (granted, it will take some time...). A bunch of systems verify that _REV returns >= 2 and change EC behaviour based on that, so killing it in the near term is unfortunately probably not an option.
On Mon, 16 Mar, at 04:21:51PM, Jason Ekstrand wrote: > > A quick update on the Dell XPS 13 for those of you who are following > this discussion but aren't aware of the XPS 13-specific discussions. > The "problem" triggered by _REV=5 is not a *real* problem. The reason > they special-cased it for the XPS 13 is that the sound card is > dual-mode and can run over either HDA and I2S. Since the I2S support > on Linux isn't great at the moment, they special-cased linux to run it > in HDA mode which has good support. The problem is that, in the A01 > bios update where they changed this, they made a mistake that left the > sound card in an invalid state. A one-line change to the DSDT table > in the bios puts it into HDA mode properly and fixes both the audio > and suspend/resume issues. They should be coming out with a bios > update shortly to fix this. > > I'm not knowledgeable enough to weigh in on the philosophical issues > here, but I thought it was worth explaining the reason for the linux > special-casing. In the case of the new XPS 13, Dell was doing > something useful with their special-casing, they just made a mistake. > If we did start advertising _REV=2 this would cause the laptop (with > the fixed bios) to load the sound card in I2S mode and it would be > less reliable. Sadly no, Dell are not doing something useful. Their use of _REV is entirely misguided for the same reasons using _OSI(Linux) is discouraged in drivers/acpi/osl.c; namely that working around kernel bugs in the BIOS is a terrible solution. Non-Windows BIOS code paths are not validated to the same degree as those traversed by running Windows, which is exactly why we try so hard to emulate Windows whenever we interact with the BIOS. The real way to fix this is to add the necessary support and bug fixes to the kernel, exactly as Bard (Cc'd) has been doing. P.S If Dell XPS13 owners try this patch and audio isn't magically detected, make sure you perform *two* cold boots. There appears to be some level of caching going where the last read _REV value is used.
On 03/23/2015 07:04 AM, Matt Fleming wrote: > On Mon, 16 Mar, at 04:21:51PM, Jason Ekstrand wrote: > Sadly no, Dell are not doing something useful. Their use of _REV is > entirely misguided for the same reasons using _OSI(Linux) is discouraged > in drivers/acpi/osl.c; namely that working around kernel bugs in the > BIOS is a terrible solution. Aside from the mistake that was made in A01 ( which has been corrected for the recently released A02 ), the goal of this workaround is to be able to provide a more functional audio solution across a wider user-base. Supporting HDA audio for Linux means that users from older kernels will still have a mostly working solution and not need to wait for the entire set of I2S kernel and userspace patches to land in their distro of choice. We can't expect everyone to run the latest kernel just to have working audio. The entire I2S experience is maturing (thanks Bard), but even in 4.0-rc5 there are still gaps. > > Non-Windows BIOS code paths are not validated to the same degree as > those traversed by running Windows, which is exactly why we try so hard > to emulate Windows whenever we interact with the BIOS. To be clear - this codepath is activating the Windows 7 audio experience even when Windows 8.1 is detected (Windows 2013 _OSI responds True). It's still a Windows BIOS codepath and it's still heavily validated. There are 3 values you'll find in a decompiled DSDT to achieve this in the _REV test block. 2 of them are used to provide inputs into the EC to toggle HDA or I2S mode. The other modifies what ACPI audio device is presented to the system. There isn't a special OS type to represent Linux here. The selected OSTP corresponds to the Windows 7 OS type. To my knowledge this is the only platform that we have so far introduced this HDA/I2S design. It's also the only platform we have used _REV to change something for Linux specifically. It was a calculated change. This, among other things that have been found will be considered for upcoming designs. > The real way to fix this is to add the necessary support and bug fixes > to the kernel, exactly as Bard (Cc'd) has been doing. I believe a majority of the kernel work is complete, but from some off kernel mailing list discussions I understand that pulseaudio doesn't understand the control interface that gets used for I2S for jack detection. UCM can't be used for it. This leads to a really confusing mixer design that needs a variety of toggles manually changed when headphones or a headset get plugged in. There have also been some stability problem with audio reported after a few days usage. > > P.S If Dell XPS13 owners try this patch and audio isn't magically > detected, make sure you perform *two* cold boots. There appears to be some > level of caching going where the last read _REV value is used. > In any case, if the _REV patch does land in the kernel, I think (we) Dell will still be mostly happy with the results. Anything older than 4.x won't contain the the _REV patch and will run in HDA mode. If someone runs a kernel with the _REV patch included, it will likely have most of the more important I2S patches landed at the same time it runs in I2S mode. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-03-24 at 00:50 -0500, Mario Limonciello wrote: > I believe a majority of the kernel work is complete, but from some off > kernel mailing list discussions I understand that pulseaudio doesn't > understand the control interface that gets used for I2S for jack > detection. There is some work in progress here to standardise the jack kcontrols between HDA, ASoC and other ALSA drivers. I would expect this to be upstream in the next week or two. > UCM can't be used for it. UCM configs and jack support for this DSP and codec combination have now been upstreamed :) > This leads to a really confusing mixer design that needs a variety of > toggles manually changed when headphones or a headset get plugged in. Generic patches to support UCM jack switching are now upstream in pulseaudio too. > There have also been some stability problem with audio reported > after a few days usage. Can you send these to me privately. Thanks Liam -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/24/2015 04:17 AM, Liam Girdwood wrote: > On Tue, 2015-03-24 at 00:50 -0500, Mario Limonciello wrote: > > There is some work in progress here to standardise the jack kcontrols > between HDA, ASoC and other ALSA drivers. I would expect this to be > upstream in the next week or two. > > UCM configs and jack support for this DSP and codec combination have now > been upstreamed :) > > Generic patches to support UCM jack switching are now upstream in > pulseaudio too. > > Can you send these to me privately. > > Thanks > > Liam > > Liam, This is all great news to hear, thank you. I'll send you a message privately about the stability issue. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 24 Mar, at 12:50:47AM, Mario Limonciello wrote: > > Aside from the mistake that was made in A01 ( which has been corrected > for the recently released A02 ), the goal of this workaround is to be > able to provide a more functional audio solution across a wider > user-base. Supporting HDA audio for Linux means that users from older > kernels will still have a mostly working solution and not need to wait > for the entire set of I2S kernel and userspace patches to land in > their distro of choice. We can't expect everyone to run the latest > kernel just to have working audio. Running a recent kernel is the tradeoff to be paid for using brand new hardware. I certainly don't expect to be able to install a 4-year old version of Fedora on a laptop released this year and have it work flawlessly. Besides, distributions aggressively backport patches for new hardware support anyway and you should work with the distribution developers to ensure that happens for your hardware. Preferably before it gets released. Because, fundamentally, when you make these decisions about Linux in the BIOS you're pitting the BIOS and kernel development models against each other. What is going to happen when i2s/i2c finally works correctly in the kernel and distros? It would seem unlikely that a BIOS update would be available to switch everyone to that mode. > The entire I2S experience is maturing (thanks Bard), but even in > 4.0-rc5 there are still gaps. I'm sure the audio folks would love to hear about them. > >Non-Windows BIOS code paths are not validated to the same degree as > >those traversed by running Windows, which is exactly why we try so hard > >to emulate Windows whenever we interact with the BIOS. > > To be clear - this codepath is activating the Windows 7 audio > experience even when Windows 8.1 is detected (Windows 2013 _OSI > responds True). It's still a Windows BIOS codepath and it's still > heavily validated. There are 3 values you'll find in a decompiled > DSDT to achieve this in the _REV test block. 2 of them are used to > provide inputs into the EC to toggle HDA or I2S mode. The other > modifies what ACPI audio device is presented to the system. There > isn't a special OS type to represent Linux here. The selected OSTP > corresponds to the Windows 7 OS type. Thanks, that is clearer and it does address my concerns about Linux-only code paths in the BIOS. But the use of _REV is still only exists to detect Linux (or more accurately an ACPICA OS), and that's a big issue. > In any case, if the _REV patch does land in the kernel, I think (we) > Dell will still be mostly happy with the results. Anything older than > 4.x won't contain the the _REV patch and will run in HDA mode. Unless the patch gets backported to stable kernel versions older than 4.x. Which is likely.
On 03/24/2015 10:24 AM, Matt Fleming wrote: > On Tue, 24 Mar, at 12:50:47AM, Mario Limonciello wrote: > Running a recent kernel is the tradeoff to be paid for using brand new > hardware. I certainly don't expect to be able to install a 4-year old > version of Fedora on a laptop released this year and have it work > flawlessly. Yes, of course older kernels won't include everything, but there are plenty of people out there that don't use (or know how to use) a newer kernel in their distro. There's enough customers that our support staff will deal with mandate particular (older) kernel versions or particular distros. Some stuff can certainly be backported into stable and come in the form of updates to those older kernels, but it is indeed a tradeoff. > Besides, distributions aggressively backport patches for new hardware > support anyway and you should work with the distribution developers to > ensure that happens for your hardware. Preferably before it gets > released. With the XPS 13 (2015) in particular, we've been working with Canonical for a very long time in planning and development. Long enough that when the plans w/ our IHV's for the audio to be HDA in Linux were made before this commit even landed: https://github.com/torvalds/linux/commit/faae404ebdc6bba744919d82e64c16448eb24a36#diff-aa93b5317c200560767b97a9d9301bd8 At that time rt286 I2S wasn't even in the kernel. Bard didn't start to land it until later that year (https://github.com/torvalds/linux/commit/07cf7cbadb4d97a78be61119a406de8fe446467e). Was it really that crazy to plan Linux to take HDA mode? > Because, fundamentally, when you make these decisions about Linux in the > BIOS you're pitting the BIOS and kernel development models against each > other. What is going to happen when i2s/i2c finally works correctly in > the kernel and distros? It would seem unlikely that a BIOS update would > be available to switch everyone to that mode. > Once all this I2S development spun up specific to the XPS 13, that's exactly the plan that we had discussed. Try to get the major distros on board with all the patches that were needed and issue a BIOS update to adjust the behavior. > I'm sure the audio folks would love to hear about them. Liam is aware of the details here. Other than one issue, it optimistically sounds like a majority of the issues will be sorted in the next few weeks on both the kernel and user-space side. > > Unless the patch gets backported to stable kernel versions older than > 4.x. Which is likely. > I would like to respectfully ask that this patch not be added to older stable kernel versions. It will knowingly cause a regression with hardware in the field. If this isn't an appropriate criteria for avoiding to backport a patch to stable, what is? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 24, 2015 at 12:22:18PM -0500, Mario Limonciello wrote: > At that time rt286 I2S wasn't even in the kernel. Bard didn't start > to land it until later that year > (https://github.com/torvalds/linux/commit/07cf7cbadb4d97a78be61119a406de8fe446467e). > Was it really that crazy to plan Linux to take HDA mode? Since we don't expose any way for the platform to detect that it's running Linux, yes. > I would like to respectfully ask that this patch not be added to older > stable kernel versions. It will knowingly cause a regression with > hardware in the field. If this isn't an appropriate criteria for > avoiding to backport a patch to stable, what is? Will it? You haven't shipped the firmware that changes this behaviour yet.
On 03/24/2015 01:01 PM, Matthew Garrett wrote: > On Tue, Mar 24, 2015 at 12:22:18PM -0500, Mario Limonciello wrote: > > Since we don't expose any way for the platform to detect that it's > running Linux, yes. > > Will it? You haven't shipped the firmware that changes this behaviour > yet. > This was posted a few days ago. BIOS A02, it resolves the broken behavior introduced in A01 and Linux. It can be flashed in the BIOS F12 boot menu, no Windows or DOS necessary. http://www.dell.com/support/home/us/en/04/Drivers/DriversDetails?driverId=F2PRR&fileId=3442501672&osCode=WB64A&productCode=xps-13-9343-laptop&languageCode=EN&categoryId=BI Anything bought from now forward will have that BIOS version (or something newer). -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 24, 2015 at 02:53:24PM -0500, Mario Limonciello wrote: > On 03/24/2015 01:01 PM, Matthew Garrett wrote: > >Will it? You haven't shipped the firmware that changes this behaviour > >yet. > > > This was posted a few days ago. BIOS A02, it resolves the broken behavior introduced in A01 and Linux. > > It can be flashed in the BIOS F12 boot menu, no Windows or DOS necessary. > > http://www.dell.com/support/home/us/en/04/Drivers/DriversDetails?driverId=F2PRR&fileId=3442501672&osCode=WB64A&productCode=xps-13-9343-laptop&languageCode=EN&categoryId=BI > > Anything bought from now forward will have that BIOS version (or something newer). Sigh. Then yeah, backporting it to stable is probably out - which means you've made it impossible to fix any other systems that have broken _REV behaviour and which would work fine with stable kernels otherwise.
On Tuesday, March 24, 2015 03:24:12 PM Matt Fleming wrote: > On Tue, 24 Mar, at 12:50:47AM, Mario Limonciello wrote: > > > > Aside from the mistake that was made in A01 ( which has been corrected > > for the recently released A02 ), the goal of this workaround is to be > > able to provide a more functional audio solution across a wider > > user-base. Supporting HDA audio for Linux means that users from older > > kernels will still have a mostly working solution and not need to wait > > for the entire set of I2S kernel and userspace patches to land in > > their distro of choice. We can't expect everyone to run the latest > > kernel just to have working audio. > > Running a recent kernel is the tradeoff to be paid for using brand new > hardware. I certainly don't expect to be able to install a 4-year old > version of Fedora on a laptop released this year and have it work > flawlessly. > > Besides, distributions aggressively backport patches for new hardware > support anyway and you should work with the distribution developers to > ensure that happens for your hardware. Preferably before it gets > released. > > Because, fundamentally, when you make these decisions about Linux in the > BIOS you're pitting the BIOS and kernel development models against each > other. What is going to happen when i2s/i2c finally works correctly in > the kernel and distros? It would seem unlikely that a BIOS update would > be available to switch everyone to that mode. > > > The entire I2S experience is maturing (thanks Bard), but even in > > 4.0-rc5 there are still gaps. > > I'm sure the audio folks would love to hear about them. > > > >Non-Windows BIOS code paths are not validated to the same degree as > > >those traversed by running Windows, which is exactly why we try so hard > > >to emulate Windows whenever we interact with the BIOS. > > > > To be clear - this codepath is activating the Windows 7 audio > > experience even when Windows 8.1 is detected (Windows 2013 _OSI > > responds True). It's still a Windows BIOS codepath and it's still > > heavily validated. There are 3 values you'll find in a decompiled > > DSDT to achieve this in the _REV test block. 2 of them are used to > > provide inputs into the EC to toggle HDA or I2S mode. The other > > modifies what ACPI audio device is presented to the system. There > > isn't a special OS type to represent Linux here. The selected OSTP > > corresponds to the Windows 7 OS type. > > Thanks, that is clearer and it does address my concerns about Linux-only > code paths in the BIOS. But the use of _REV is still only exists to > detect Linux (or more accurately an ACPICA OS), and that's a big issue. > > > In any case, if the _REV patch does land in the kernel, I think (we) > > Dell will still be mostly happy with the results. Anything older than > > 4.x won't contain the the _REV patch and will run in HDA mode. > > Unless the patch gets backported to stable kernel versions older than > 4.x. Which is likely. While I agree in general, one comment. We haven't decided about the patch yet. We may decide to bump up the _REV to 6 when ACPI 6 is out instead. That said the whole using of _REV to special case Linux is broken by design and should be stopped immediately. Especially when it is done by comparing the return value of _REV to a specific number (like 5 or 3). Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/24/2015 03:02 PM, Rafael J. Wysocki wrote: > On Tuesday, March 24, 2015 03:24:12 PM Matt Fleming wrote: > While I agree in general, one comment. > > We haven't decided about the patch yet. We may decide to bump up the _REV > to 6 when ACPI 6 is out instead. I'd be happy with this too. > That said the whole using of _REV to special case Linux is broken by design > and should be stopped immediately. Especially when it is done by comparing > the return value of _REV to a specific number (like 5 or 3). > > Rafael > Yes, it's been made clear to me that this shouldn't be used in the future. I've shared that feedback to my BIOS architecture team. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Matt Fleming <matt <at> codeblueprint.co.uk> writes: > P.S If Dell XPS13 owners try this patch and audio isn't magically > detected, make sure you perform *two* cold boots. There appears to be some > level of caching going where the last read _REV value is used. I suggested in the comments on mjg59's blog that a BIOS option set by the user be used for things like this. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h index 03aacfb..cebb8a7 100644 --- a/include/acpi/acconfig.h +++ b/include/acpi/acconfig.h @@ -112,9 +112,19 @@ * *****************************************************************************/ -/* Version of ACPI supported */ - +/* + * Version of ACPI supported. This is a sad story. Windows reports a _REV of + * 2 regardless of the spec version implemented. Some vendors are using _REV + * as a way to distinguish between Windows and Linux, and are breaking systems + * in the process. We can't guarantee that they'll call _OSI before checking + * _REV, so hardcode this to 2 on x86 systems right now and leave it at the + * appropriate spec value for everybody else. + */ +#ifdef CONFIG_X86 +#define ACPI_CA_SUPPORT_LEVEL 2 +#else #define ACPI_CA_SUPPORT_LEVEL 5 +#endif /* Maximum count for a semaphore object */
The ACPI spec describes _REV as: "This predefined object evaluates to the revision of the ACPI Specification that the specified \_OS implements" We've been assuming that this should increment as ACPICA gains support for new versions of the spec. Unfortunately, Windows always reports "2" for this value and vendors are now using this as a means to tell whether a system is running Windows or Linux. From an HP Envy 15: If (LOr (LEqual (_REV, 0x03), LEqual (_REV, 0x05))) From a Dell XPS 13: If ((_REV == 0x05)) In both cases, the systems then alter hardware initialisation paths and device capability reporting. As written, this makes no sense - ACPI maintains backwards compatibility, so the appropriate test would be >= rather than ==. On closer examination, the HP code uses the same initialisation paths as would be used if the system responded to _OSI("Linux"), and as such the evidence is overwhelmingly that vendors are using this to alter system behaviour when Linux is detected. Since we aim to be compatible with Windows, this tends to break things. The ideal solution would be to wait for an _OSI() query matching Windows and then change behaviour, but Windows responds to _REV with 2 even before that and so vendors might simply change the order of queries in order to continue IDing Linux. The easiest thing for us to do is just to change the value on X86 and leave a comment describing why everything is so awful. Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org> --- include/acpi/acconfig.h | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)