search

andreiw / RaspberryPiPkg

DEPRECATED - DO NOT USE | Go here instead ->
https://github.com/tianocore/edk2-platforms/tree/master/Platform/RaspberryPi/RPi3
746 stars 143 forks source link

Replace encoded MsftFunctionConfigs with their standard equivalent, PinFunction #132

Closed Googulator closed 4 years ago

Googulator commented 4 years ago

The MsftFunctionConfig macro got standardized in ACPI 6.2 as PinFunction, and although this isn't documented anywhere, recent builds of Windows 10 support the standard version (at least 18362 does - not sure when it got implemented). Switching to PinFunction would be helpful for letting Linux boot via ACPI.

driver1998 commented 4 years ago

If iasl supports it (it should but I haven't checked), that also means we can get rid of VendorLong and have a more readable code.

Googulator commented 4 years ago

Yes, it's supported in recent versions of iasl.

andreiw commented 4 years ago

(tested) pull requests welcome.

On Mon, Jul 1, 2019 at 8:57 AM Googulator notifications@github.com wrote:

Yes, it's supported in recent versions of iasl.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/andreiw/RaspberryPiPkg/issues/132?email_source=notifications&email_token=AAFS223GZW6VIINJ6JQ3JADP5H5MBA5CNFSM4H3SQ6IKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY6BFLI#issuecomment-507253421, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFS2233WU3QOJIQ7VBEW2TP5H5MBANCNFSM4H3SQ6IA .

-- A

driver1998 commented 4 years ago

Translated all the MsftFunctionConfigs to PinFunction (they are basically the same), and Windows 10 b18362 seems to be happy with that: TIM图片20190702041021

I don't have any SPI device to test so far, but I think it will work fine as well.

pbatard commented 4 years ago

See my comment on PR #134.

driver1998 commented 4 years ago

So PinFunction is not supported in v1809 (17763), with the same behavior like https://github.com/andreiw/RaspberryPiPkg/issues/96.

pbatard commented 4 years ago

My understanding is that this has nothing to do with the Windows version being used, but everything to do with whether the iasl compiler, which is executed to parse the .asl when compiling the UEFI firmware, and which could be executed from any platform (but would mostly be from Linux) potentially being "too old" to understand newer constructs like PinFunction().

In other words, if PinFunction() was introduced to the ACPI specs after 2016, then any ACPI pre-2106 compiler that attempts to compile a file that invokes it will produce an error, which is what I am seeing when trying to compile DSDT.asl on Debian 9.9 (latest), because the native iasl it provides, which comes from the acpica-tools package, dates from 2016...

driver1998 commented 4 years ago

@pbatard This is a recent addition to ACPI spec (in ACPI 6.2), so it is not surprised to see it not supported in old Windows versions.

It is not even documented in MS docs on rhproxy (which is the consumer of this config) yet: https://github.com/MicrosoftDocs/windows-uwp/issues/1751. I use the same DSDT compiled in iasl 20180105, and test it in both 17763 and 18362.

pbatard commented 4 years ago

so it is not surprised to see it not supported in old Windows versions.

Again, this is not a matter of Windows not supporting it. It's a matter of the Linux ACPI compiler not supporting it.

AFAIK, all Windows sees are the compiled binary blobs, and it doesn't care whether they were produced by an ACPI compiler that used PinFunction() or MsftFunctionConfig() or VendorLong() since the end result should be exactly the same.

The only issue here is whether the compiler understands any of the above functions to create the binary blob, and the current default Linux ACPI compiler on Debian only knows about VendorLong() because it was created in 2016, before PinFunction() was introduced.

driver1998 commented 4 years ago

I am using iasl 20180105, which has no problems with PinFunction, it compiles the file just fine.

The output is recognized in 18362, but NOT 17763.

Maybe compare the two compiled blob and see if there is any difference. Especially when PinFunction has one more additional field than MsftFunctionConfig (although it can be left blank).

pbatard commented 4 years ago

I am using iasl 20180105

And as I pointed out, the default iasl from Debian 9.9, which is the most up to date Debian, dates from 2016, and doesn't have PinFunction.

So, once again, I'll leave it to Andrei to decide whether he wants to require people to have to manually upgrade their iasl, since it is likely that the default version provided by their platform will be too old to understand PinFunction, or wait until platforms like Debian have upgraded their default iasl package to include a version that supports PinFunction (such as 20180105).

Please bear in mind that not everybody wants to have to manually update toolchain components on their system, because it can create maintenance headaches. My guess is that a lot of people expect that compilation should work with the default system toolchain (especially if their system is up to date) and that, because there is always a lot of latency between what a toolchain provides and latest compilers and tools, we therefore may have to wait for a while before we see a native iasl on Debian that supports PinFunction...

driver1998 commented 4 years ago

Yes but that is a completely different issue... BTW, the next major release of Debian, Buster, comes with iasl 20181213, and is planned to release by July 6th. Although it is also annoying to require everybody to have nothing but the very latest release...

andreiw commented 4 years ago

Considering there is a difference in output AML, and subsequent difference in supported builds, I’d hide this under a macro (and preprocess the ASL prior to running it through iasl), with the default implementation being the old (and compatible with old iasl)

A

2 июля 2019 г., в 7:55, driver1998 notifications@github.com написал(а):

Yes but that is a completely different issue... BTW, the next major release of Debian, Buster, comes with iasl 20181213, and is planned to release by July 6th. Although it is also annoying to require everybody to have nothing but the very latest release...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

driver1998 commented 4 years ago

As I mentioned in https://github.com/andreiw/RaspberryPiPkg/pull/134#issuecomment-507813600:

Now you can specify -DACPI_PINFUNCTION=1 in the build commandline to use PinFunction, by default it still uses MsftFunctionConfig.

andreiw commented 4 years ago

Thanks for your contribution - it’s merged.

A

4 июля 2019 г., в 16:58, driver1998 notifications@github.com написал(а):

As I mentioned in #134 (comment):

Now you can specify -DACPI_PINFUNCTION=1 in the build commandline to use PinFunction, by default it still uses MsftFunctionConfig.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.