-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hello,
Following patch converts the run-time SSDT patching via update_ssdt funtion to new AML code generator. Compile-tested on all changed targets. I think it should work because it works for Asus M2V-MX SE.
Signed-off-by: Rudolf Marek r.marek@assembler.cz
Perhaps also some private SSDTs of each MB could be also somehow converted. Its quite complicated and I dont own any board to test, so lets just convert the generic stuff.
Rudolf
Rudolf Marek wrote:
+++ coreboot-v2/src/mainboard/agami/aruma/acpi_tables.c 2009-02-01 12:04:04.738807653 +0100 @@ -15,6 +15,7 @@ #include <device/pci_ids.h> #include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> +#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
Other than that it would be nice to have at least one of these boards tested before the commit.
//Peter
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Peter Stuge wrote:
Rudolf Marek wrote:
+++ coreboot-v2/src/mainboard/agami/aruma/acpi_tables.c 2009-02-01 12:04:04.738807653 +0100 @@ -15,6 +15,7 @@ #include <device/pci_ids.h> #include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> +#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
This works for me. Also I used it for includes for K8T890 chipset. I dont remember why I used this kind of construction. Maybe also because I needed some header which resides in other then include dir?
Other than that it would be nice to have at least one of these boards tested before the commit.
Yep. Lets wait.
R.
//Peter
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
On Sun, Feb 01, 2009 at 06:31:50PM +0100, Rudolf Marek wrote:
Peter Stuge wrote:
Rudolf Marek wrote:
+++ coreboot-v2/src/mainboard/agami/aruma/acpi_tables.c 2009-02-01 12:04:04.738807653 +0100 @@ -15,6 +15,7 @@ #include <device/pci_ids.h> #include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> +#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
This works for me. Also I used it for includes for K8T890 chipset. I dont remember why I used this kind of construction. Maybe also because I needed some header which resides in other then include dir?
Other than that it would be nice to have at least one of these boards tested before the commit.
Yep. Lets wait.
How hard would it be to add this for the m57sli? I'd be more than happy to test on that board.
Thanks, Ward.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
How hard would it be to add this for the m57sli? I'd be more than happy to test on that board.
Huh there is no ACPI support at all for this board correct? I was looking into src/mainboard/gigabyte/m57sli right?
Maybe you can start adding the ACPI support for this board. The only problem would be the PMIO register layout, but this can be checked from original BIOS.
Rudolf
Peter Stuge wrote:
Rudolf Marek wrote:
+++ coreboot-v2/src/mainboard/agami/aruma/acpi_tables.c 2009-02-01 12:04:04.738807653 +0100 @@ -15,6 +15,7 @@ #include <device/pci_ids.h> #include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> +#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
I don't think I can agree... northbridge specific code should really really live under northbridge.
Whether we want to add some syntactic sugar to make it look nicer than the above is another question.
Stefan
On Sun, Feb 1, 2009 at 10:02 AM, Stefan Reinauer stepan@coresystems.de wrote:
Peter Stuge wrote:
Rudolf Marek wrote:
+++ coreboot-v2/src/mainboard/agami/aruma/acpi_tables.c 2009-02-01 12:04:04.738807653 +0100 @@ -15,6 +15,7 @@ #include <device/pci_ids.h> #include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> +#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
I don't think I can agree... northbridge specific code should really really live under northbridge.
Whether we want to add some syntactic sugar to make it look nicer than the above is another question.
If it's a huge concern add a -I for that directory in the CFLAGS.
ron
Stefan Reinauer wrote:
+#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
I don't think I can agree... northbridge specific code should really really live under northbridge.
Code no question - but headers that are for use elsewhere?
I realize this is v2, but maybe want to improve that anyway?
//Peter
Peter Stuge wrote:
Stefan Reinauer wrote:
+#include <../../../northbridge/amd/amdk8/amdk8_acpi.h>
../../../ that can't be right. Maybe the .h should go into include/ somewhere instead?
I don't think I can agree... northbridge specific code should really really live under northbridge.
Code no question - but headers that are for use elsewhere?
Absolutely. If a component uses headers from another component, it should get them from there. A clean design demands we do not mix generic code and component code.
I realize this is v2, but maybe want to improve that anyway?
Which is why I wrote the suggestion to begin with. Breaking the component design is very far from what the more modular v3 design ever tried to achieve. If we broke v3 in this matter, we must fix it too. As Ron said. It can be done with a simple -I flag. No component code/headers under /include, it just doesn't make sense from a design perspective, and breaking the design for syntactic sugar is not a good approach.
Stefan
On 01.02.2009 17:15, Rudolf Marek wrote:
Hello,
Following patch converts the run-time SSDT patching via update_ssdt funtion to new AML code generator. Compile-tested on all changed targets. I think it should work because it works for Asus M2V-MX SE.
Signed-off-by: Rudolf Marek r.marek@assembler.cz
Perhaps also some private SSDTs of each MB could be also somehow converted. Its quite complicated and I dont own any board to test, so lets just convert the generic stuff.
Some comments on the patch: It does not apply cleanly against the latest tree, but that's probably just minor. ./mainboard/amd/dbm690t/acpi_tables.c still has AmlCode_ssdt somewhere in there. iwill/dk8_htx and agami/aruma still reference the old ssdt code in their Config.lb.
Having the same code for Fam10h would be great and probably shrink the differences between K8 and Fam10 even further. We need to merge that code anyway.
Overall, I like the patch. I'll boot the old and the new code and dump my ACPI tables so you can take a look.
Regards, Carl-Daniel
On 03.02.2009 01:30, Carl-Daniel Hailfinger wrote:
On 01.02.2009 17:15, Rudolf Marek wrote:
Hello,
Following patch converts the run-time SSDT patching via update_ssdt funtion to new AML code generator. Compile-tested on all changed targets. I think it should work because it works for Asus M2V-MX SE.
Signed-off-by: Rudolf Marek r.marek@assembler.cz
Perhaps also some private SSDTs of each MB could be also somehow converted. Its quite complicated and I dont own any board to test, so lets just convert the generic stuff.
Some comments on the patch: It does not apply cleanly against the latest tree, but that's probably just minor. mainboard/amd/dbm690t/acpi_tables.c still has AmlCode_ssdt somewhere in there. iwill/dk8_htx and agami/aruma still reference the old ssdt code in their Config.lb.
Having the same code for Fam10h would be great and probably shrink the differences between K8 and Fam10 even further. We need to merge that code anyway.
Overall, I like the patch. I'll boot the old and the new code and dump my ACPI tables so you can take a look.
Old and new ACPI tables are attached. Please ignore the TOM and TOM2 values in the DSDT. They are wrong (mainboard code bug). Unless I'm missing something, your code produces a SSDT which is identical to the previous one on my Asus M2A-VM (690G/SB600) board.
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Regards,
Carl-Daniel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi all,
Its in. Committed revision 3929.
I wont have time for this until next Tuesday. I have in plan to tune up the first patch as I discussed this with Carl-Daniel. If someone has time please do that so.
Thanks,
Rudolf
-
Carl-Daniel Hailfinger -
Peter Stuge -
ron minnich -
Rudolf Marek -
Stefan Reinauer -
Ward Vandewege