coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6696 discussions

Change in coreboot[master]: soc/amd/*/northbridge,root_complex: add comment about PCI BARs
by Felix Held (Code Review) 03 Mar '22

03 Mar '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62547 ) Change subject: soc/amd/*/northbridge,root_complex: add comment about PCI BARs ...................................................................... soc/amd/*/northbridge,root_complex: add comment about PCI BARs Add a comment to point out that the read_resources functions aren't missing a pci_dev_read_resources call that would add the resources for the BARs of the PC device. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ie480832e0d7954135d2171dda986e477ef7b6c09 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62547 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/root_complex.c M src/soc/amd/picasso/root_complex.c M src/soc/amd/sabrina/root_complex.c M src/soc/amd/stoneyridge/northbridge.c 4 files changed, 12 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/root_complex.c b/src/soc/amd/cezanne/root_complex.c index dabfa78..9cb668e 100644 --- a/src/soc/amd/cezanne/root_complex.c +++ b/src/soc/amd/cezanne/root_complex.c @@ -114,6 +114,9 @@ early_reserved_dram_start = e->base; early_reserved_dram_end = e->base + e->size; + /* The root complex has no PCI BARs implemented, so there's no need to call + pci_dev_read_resources for it */ + /* 0x0 - 0x9ffff */ ram_resource(dev, idx++, 0, 0xa0000 / KiB); diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index a3a3d4b..d5fd10d 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -114,6 +114,9 @@ early_reserved_dram_start = e->base; early_reserved_dram_end = e->base + e->size; + /* The root complex has no PCI BARs implemented, so there's no need to call + pci_dev_read_resources for it */ + /* 0x0 - 0x9ffff */ ram_resource(dev, idx++, 0, 0xa0000 / KiB); diff --git a/src/soc/amd/sabrina/root_complex.c b/src/soc/amd/sabrina/root_complex.c index 2a3ed6f..ae1b3b9 100644 --- a/src/soc/amd/sabrina/root_complex.c +++ b/src/soc/amd/sabrina/root_complex.c @@ -116,6 +116,9 @@ early_reserved_dram_start = e->base; early_reserved_dram_end = e->base + e->size; + /* The root complex has no PCI BARs implemented, so there's no need to call + pci_dev_read_resources for it */ + /* 0x0 - 0x9ffff */ ram_resource(dev, idx++, 0, 0xa0000 / KiB); diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 0384e00..85ef19e 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -60,6 +60,9 @@ unsigned int idx = 0; struct resource *res; + /* The northbridge has no PCI BARs implemented, so there's no need to call + pci_dev_read_resources for it */ + /* * This MMCONF resource must be reserved in the PCI domain. * It is not honored by the coreboot resource allocator if it is in -- To view, visit https://review.coreboot.org/c/coreboot/+/62547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie480832e0d7954135d2171dda986e477ef7b6c09 Gerrit-Change-Number: 62547 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/*/northbridge,root_complex: simplify GNB IOAPIC resource index
by Felix Held (Code Review) 03 Mar '22

03 Mar '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62546 ) Change subject: soc/amd/*/northbridge,root_complex: simplify GNB IOAPIC resource index ...................................................................... soc/amd/*/northbridge,root_complex: simplify GNB IOAPIC resource index In the northbridge's and root complex' read_resources function, the GNB IOAPIC resource used MMIO base address of the GNB IOAPIC as index which might be misleading. Instead use idx++ as a unique index for this resource. TEST=Resource allocator doesn't complain and no related warnings or errors in dmesg. The update_constraints console output changes like expected: Before: PCI: 00:00.0 fec01000 base fec01000 limit fec01fff mem (fixed) After: PCI: 00:00.0 0d base fec01000 limit fec01fff mem (fixed) Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I8061364879d772469882fc060f92676de6f600a9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62546 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/root_complex.c M src/soc/amd/picasso/root_complex.c M src/soc/amd/sabrina/root_complex.c M src/soc/amd/stoneyridge/northbridge.c 4 files changed, 4 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/root_complex.c b/src/soc/amd/cezanne/root_complex.c index e29b4e0..dabfa78 100644 --- a/src/soc/amd/cezanne/root_complex.c +++ b/src/soc/amd/cezanne/root_complex.c @@ -168,7 +168,7 @@ } /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, GNB_IO_APIC_ADDR); + gnb_apic = new_resource(dev, idx++); gnb_apic->base = GNB_IO_APIC_ADDR; gnb_apic->size = 0x00001000; gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index 076588e..a3a3d4b 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -166,7 +166,7 @@ } /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, GNB_IO_APIC_ADDR); + gnb_apic = new_resource(dev, idx++); gnb_apic->base = GNB_IO_APIC_ADDR; gnb_apic->size = 0x00001000; gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; diff --git a/src/soc/amd/sabrina/root_complex.c b/src/soc/amd/sabrina/root_complex.c index fc49f4f..2a3ed6f 100644 --- a/src/soc/amd/sabrina/root_complex.c +++ b/src/soc/amd/sabrina/root_complex.c @@ -170,7 +170,7 @@ } /* GNB IOAPIC resource */ - gnb_apic = new_resource(dev, GNB_IO_APIC_ADDR); + gnb_apic = new_resource(dev, idx++); gnb_apic->base = GNB_IO_APIC_ADDR; gnb_apic->size = 0x00001000; gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 0a80648..0384e00 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -68,7 +68,7 @@ mmconf_resource(dev, idx++); /* NB IOAPIC2 resource */ - res = new_resource(dev, IO_APIC2_ADDR); /* IOAPIC2 */ + res = new_resource(dev, idx++); /* IOAPIC2 */ res->base = IO_APIC2_ADDR; res->size = 0x00001000; res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; -- To view, visit https://review.coreboot.org/c/coreboot/+/62546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8061364879d772469882fc060f92676de6f600a9 Gerrit-Change-Number: 62546 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/*/northbridge,root_complex: simplify mmconf_resource index
by Felix Held (Code Review) 03 Mar '22

03 Mar '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62545 ) Change subject: soc/amd/*/northbridge,root_complex: simplify mmconf_resource index ...................................................................... soc/amd/*/northbridge,root_complex: simplify mmconf_resource index In the northbridge's and root complex' read_resources function, the mmconf resource used the number of the MMIO_CONF_BASE MSR as index which might be misleading. Instead use idx++ as a unique index for this resource. TEST=Resource allocator doesn't complain and no related warnings or errors in dmesg. The update_constraints console output changes like expected: Before: PCI: 00:00.0 c0010058 base f8000000 limit fbffffff mem (fixed) After: PCI: 00:00.0 06 base f8000000 limit fbffffff mem (fixed) Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Id66c6153fad86bed36db7bd2455075f4a0850750 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62545 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/root_complex.c M src/soc/amd/picasso/root_complex.c M src/soc/amd/sabrina/root_complex.c M src/soc/amd/stoneyridge/northbridge.c 4 files changed, 5 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/root_complex.c b/src/soc/amd/cezanne/root_complex.c index dde59d4..e29b4e0 100644 --- a/src/soc/amd/cezanne/root_complex.c +++ b/src/soc/amd/cezanne/root_complex.c @@ -138,7 +138,7 @@ ram_resource(dev, idx++, early_reserved_dram_end / KiB, (mem_usable - early_reserved_dram_end) / KiB); - mmconf_resource(dev, MMIO_CONF_BASE); + mmconf_resource(dev, idx++); if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index 4ea8bc2..076588e 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -136,7 +136,7 @@ ram_resource(dev, idx++, early_reserved_dram_end / KiB, (mem_usable - early_reserved_dram_end) / KiB); - mmconf_resource(dev, MMIO_CONF_BASE); + mmconf_resource(dev, idx++); if (!hob) { printk(BIOS_ERR, "%s incomplete because no HOB list was found\n", diff --git a/src/soc/amd/sabrina/root_complex.c b/src/soc/amd/sabrina/root_complex.c index e73c30e..fc49f4f 100644 --- a/src/soc/amd/sabrina/root_complex.c +++ b/src/soc/amd/sabrina/root_complex.c @@ -140,7 +140,7 @@ ram_resource(dev, idx++, early_reserved_dram_end / KiB, (mem_usable - early_reserved_dram_end) / KiB); - mmconf_resource(dev, MMIO_CONF_BASE); + mmconf_resource(dev, idx++); if (!hob) { printk(BIOS_ERR, "Error: %s incomplete because no HOB list was found\n", diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 04472dc..0a80648 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -57,6 +57,7 @@ static void read_resources(struct device *dev) { + unsigned int idx = 0; struct resource *res; /* @@ -64,7 +65,7 @@ * It is not honored by the coreboot resource allocator if it is in * the CPU_CLUSTER. */ - mmconf_resource(dev, MMIO_CONF_BASE); + mmconf_resource(dev, idx++); /* NB IOAPIC2 resource */ res = new_resource(dev, IO_APIC2_ADDR); /* IOAPIC2 */ -- To view, visit https://review.coreboot.org/c/coreboot/+/62545 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id66c6153fad86bed36db7bd2455075f4a0850750 Gerrit-Change-Number: 62545 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: coreboot_tables.c: Expose the ACPI RSDP
by Nico Huber (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62573 ) Change subject: coreboot_tables.c: Expose the ACPI RSDP ...................................................................... Patch Set 2: (1 comment) File src/commonlib/include/commonlib/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/62573/comment/8a326101_5d6f4b4f PS1, Line 586: uint64_t > > > (in C pointers are aligned, right?) […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/62573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I591312a2c48f0cbbb03b2787e4b365e9c932afff Gerrit-Change-Number: 62573 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:43:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables.c: Expose the ACPI RSDP
by Nico Huber (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62573 ) Change subject: coreboot_tables.c: Expose the ACPI RSDP ...................................................................... Patch Set 2: (1 comment) File src/commonlib/include/commonlib/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/62573/comment/5fe7c811_6d8c10e9 PS1, Line 586: uint64_t > > (in C pointers are aligned, right?) > > Says who? x86 supports misaligned everything by design! The compilers just keep trying to shave off every possible bit everywhere making every assumption they possibly can.... that is so frustrating... The spec says so, I think, at least indirectly, and the warnings that we disable ^^ ``` Among the invalid values for dereferencing a pointer by the unary * operator are a null pointer, an address inappropriately aligned for the type of object pointed to, and the address of an object after the end of its lifetime. ``` So strictly speaking pointers don't have to be aligned but the objects. IMO, beside the GNU extensions, it's sound and save. If this is really becoming an issue it's our fault. -- To view, visit https://review.coreboot.org/c/coreboot/+/62573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I591312a2c48f0cbbb03b2787e4b365e9c932afff Gerrit-Change-Number: 62573 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:42:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot
by Julius Werner (Code Review) 03 Mar '22

03 Mar '22

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62516 ) Change subject: libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot ...................................................................... libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot TPM1_MODE and TPM2_MODE defines have to be added to vboot and payload cflags to make them build correctly without requiring payloads to provide defines. Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com> Change-Id: I567a9f04d7089699840dc7e0a063cf3030fb934b Reviewed-on: https://review.coreboot.org/c/coreboot/+/62516 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Julius Werner <jwerner(a)chromium.org> --- M payloads/libpayload/bin/lpgcc 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved diff --git a/payloads/libpayload/bin/lpgcc b/payloads/libpayload/bin/lpgcc index 8bc46a6..77ceda3 100755 --- a/payloads/libpayload/bin/lpgcc +++ b/payloads/libpayload/bin/lpgcc @@ -182,6 +182,14 @@ _CFLAGS="$_CFLAGS -include $BASE/../include/kconfig.h -include $BASE/../include/compiler.h" _CFLAGS="$_CFLAGS -I`$DEFAULT_CC $_ARCHEXTRA -print-search-dirs | head -n 1 | cut -d' ' -f2`include" +if [ "$CONFIG_LP_VBOOT_LIB" = y ]; then + if [ "$CONFIG_LP_VBOOT_TPM2_MODE" = y ]; then + _CFLAGS="$_CFLAGS -DTPM2_MODE" + else + _CFLAGS="$_CFLAGS -DTPM1_MODE" + fi +fi + _LDFLAGS="-L$_LIBDIR $_LDSCRIPT -static -Wl,--gc-sections" if [ $DOLINK -eq 0 ]; then -- To view, visit https://review.coreboot.org/c/coreboot/+/62516 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I567a9f04d7089699840dc7e0a063cf3030fb934b Gerrit-Change-Number: 62516 Gerrit-PatchSet: 4 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Joey Madafferi <joeymadafferi(a)gmail.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot
by Julius Werner (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Jakub Czapiga, Yu-Ping Wu, Joey Madafferi. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62516 ) Change subject: libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/62516 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I567a9f04d7089699840dc7e0a063cf3030fb934b Gerrit-Change-Number: 62516 Gerrit-PatchSet: 3 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Joey Madafferi <joeymadafferi(a)gmail.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Joey Madafferi <joeymadafferi(a)gmail.com> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:38:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/system76/dgpu: Add driver for System76 switchable graphics
by Jeremy Soller (Code Review) 03 Mar '22

03 Mar '22

Jeremy Soller has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43615 ) Change subject: drivers/system76/dgpu: Add driver for System76 switchable graphics ...................................................................... drivers/system76/dgpu: Add driver for System76 switchable graphics This adds a driver for System76 systems with switchable NVIDIA graphics. The driver provides ACPI support for dynamically powering on and off the GPU, a function for enabling the GPU power in the bootblock, and a hook in ramstage for moving GPU prefetch resources above 4 GiB. Tested on system76/addw2, system76/gaze15, and system76/oryp6, which will be submitted in future patches. Signed-off-by: Jeremy Soller <jeremy(a)system76.com> Change-Id: I05833e88457bfc3612cc9a8cbf1eec68dcfc5566 --- A src/drivers/system76/dgpu/Kconfig A src/drivers/system76/dgpu/Makefile.inc A src/drivers/system76/dgpu/acpi/dgpu.asl A src/drivers/system76/dgpu/bootblock.c A src/drivers/system76/dgpu/ramstage.c 5 files changed, 313 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/43615/1 diff --git a/src/drivers/system76/dgpu/Kconfig b/src/drivers/system76/dgpu/Kconfig new file mode 100644 index 0000000..5096671 --- /dev/null +++ b/src/drivers/system76/dgpu/Kconfig @@ -0,0 +1,5 @@ +config DRIVERS_SYSTEM76_DGPU + bool + default n + help + System76 switchable graphics support diff --git a/src/drivers/system76/dgpu/Makefile.inc b/src/drivers/system76/dgpu/Makefile.inc new file mode 100644 index 0000000..a230f97 --- /dev/null +++ b/src/drivers/system76/dgpu/Makefile.inc @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only + +ramstage-$(CONFIG_DRIVERS_SYSTEM76_DGPU) += ramstage.c diff --git a/src/drivers/system76/dgpu/acpi/dgpu.asl b/src/drivers/system76/dgpu/acpi/dgpu.asl new file mode 100644 index 0000000..a721934 --- /dev/null +++ b/src/drivers/system76/dgpu/acpi/dgpu.asl @@ -0,0 +1,201 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (\_SB.PCI0.PEGP) { + Name (_ADR, 0x00010000) + + PowerResource (PWRR, 0, 0) { + Name (_STA, 1) + + Method (_ON) { + Debug = "PEGP.PWRR._ON" + If (_STA != 1) { + \_SB.PCI0.PEGP.DEV0._ON () + _STA = 1 + } + } + + Method (_OFF) { + Debug = "PEGP.PWRR._OFF" + If (_STA != 0) { + \_SB.PCI0.PEGP.DEV0._OFF () + _STA = 0 + } + } + } + + Name (_PR0, Package () { \_SB.PCI0.PEGP.PWRR }) + Name (_PR2, Package () { \_SB.PCI0.PEGP.PWRR }) + Name (_PR3, Package () { \_SB.PCI0.PEGP.PWRR }) +} + +Device (\_SB.PCI0.PEGP.DEV0) { + Name(_ADR, 0x00000000) + Name (_STA, 0xF) + Name (LTRE, 0) + + // Memory mapped PCI express registers + // Not sure what this stuff is, but it is used to get into GC6 + OperationRegion (RPCX, SystemMemory, 0xE0008000, 0x1000) + Field (RPCX, ByteAcc, NoLock, Preserve) { + PVID, 16, + PDID, 16, + CMDR, 8, + Offset (0x19), + PRBN, 8, + Offset (0x84), + D0ST, 2, + Offset (0xAA), + CEDR, 1, + Offset (0xAC), + , 4, + CMLW, 6, + Offset (0xB0), + ASPM, 2, + , 2, + P0LD, 1, + RTLK, 1, + Offset (0xC9), + , 2, + LREN, 1, + Offset (0x11A), + , 1, + VCNP, 1, + Offset (0x214), + Offset (0x216), + P0LS, 4, + Offset (0x248), + , 7, + Q0L2, 1, + Q0L0, 1, + Offset (0x504), + Offset (0x506), + PCFG, 2, + Offset (0x508), + TREN, 1, + Offset (0xC20), + , 4, + P0AP, 2, + Offset (0xC38), + , 3, + P0RM, 1, + Offset (0xC74), + P0LT, 4, + Offset (0xD0C), + , 20, + LREV, 1 + } + + Method (_ON) { + Debug = "PEGP.DEV0._ON" + + If (_STA != 0xF) { + Debug = " If DGPU_PWR_EN low" + If (! GTXS (DGPU_PWR_EN)) { + Debug = " DGPU_PWR_EN high" + STXS (DGPU_PWR_EN) + + Debug = " Sleep 16" + Sleep (16) + } + + Debug = " DGPU_RST_N high" + STXS(DGPU_RST_N) + + Debug = " Sleep 10" + Sleep (10) + + Debug = " Q0L0 = 1" + Q0L0 = 1 + + Debug = " Sleep 16" + Sleep (16) + + Debug = " While Q0L0" + Local0 = 0 + While (Q0L0) { + If ((Local0 > 4)) { + Debug = " While Q0L0 timeout" + Break + } + + Sleep (16) + Local0++ + } + + Debug = " P0RM = 0" + P0RM = 0 + + Debug = " P0AP = 0" + P0AP = 0 + + Debug = Concatenate(" LREN = ", ToHexString(LTRE)) + LREN = LTRE + + Debug = " CEDR = 1" + CEDR = 1 + + Debug = " CMDR |= 7" + CMDR |= 7 + + Debug = " _STA = 0xF" + _STA = 0xF + } + } + + Method (_OFF) { + Debug = "PEGP.DEV0._OFF" + + If (_STA != 0x5) { + Debug = Concatenate(" LTRE = ", ToHexString(LREN)) + LTRE = LREN + + Debug = " Q0L2 = 1" + Q0L2 = 1 + + Debug = " Sleep 16" + Sleep (16) + + Debug = " While Q0L2" + Local0 = Zero + While (Q0L2) { + If ((Local0 > 4)) { + Debug = " While Q0L2 timeout" + Break + } + + Sleep (16) + Local0++ + } + + Debug = " P0RM = 1" + P0RM = 1 + + Debug = " P0AP = 3" + P0AP = 3 + + Debug = " Sleep 10" + Sleep (10) + + Debug = " DGPU_RST_N low" + CTXS(DGPU_RST_N) + + Debug = " While DGPU_GC6 low" + Local0 = Zero + While (! GRXS(DGPU_GC6)) { + If ((Local0 > 4)) { + Debug = " While DGPU_GC6 low timeout" + + Debug = " DGPU_PWR_EN low" + CTXS (DGPU_PWR_EN) + Break + } + + Sleep (16) + Local0++ + } + + Debug = " _STA = 0x5" + _STA = 0x5 + } + } +} diff --git a/src/drivers/system76/dgpu/bootblock.c b/src/drivers/system76/dgpu/bootblock.c new file mode 100644 index 0000000..61f7fbf --- /dev/null +++ b/src/drivers/system76/dgpu/bootblock.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +//TODO: do not require this to be included in mainboard bootblock.c + +#include <console/console.h> +#include <delay.h> +#include <gpio.h> + +static void dgpu_power_enable(int onoff) { + printk(BIOS_DEBUG, "system76: DGPU power %d\n", onoff); + if (onoff) { + gpio_set(DGPU_RST_N, 0); + mdelay(4); + gpio_set(DGPU_PWR_EN, 1); + mdelay(4); + gpio_set(DGPU_RST_N, 1); + } else { + gpio_set(DGPU_RST_N, 0); + mdelay(4); + gpio_set(DGPU_PWR_EN, 0); + } + mdelay(50); +} diff --git a/src/drivers/system76/dgpu/ramstage.c b/src/drivers/system76/dgpu/ramstage.c new file mode 100644 index 0000000..eeb1607 --- /dev/null +++ b/src/drivers/system76/dgpu/ramstage.c @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootstate.h> +#include <console/console.h> +#include <device/pci.h> + +static void dgpu_read_resources(struct device *dev) { + printk(BIOS_INFO, "system76: dgpu_read_resources %s\n", dev_path(dev)); + + pci_dev_read_resources(dev); + + int bar; + // Find all BARs on DGPU, mark them above 4g if prefetchable + for (bar = PCI_BASE_ADDRESS_0; bar <= PCI_BASE_ADDRESS_5; bar += 4) { + printk(BIOS_INFO, " BAR at 0x%02x\n", bar); + + struct resource *res; + res = probe_resource(dev, bar); + if (res) { + if (res->flags & IORESOURCE_PREFETCH) { + printk(BIOS_INFO, " marked above 4g\n"); + res->flags |= IORESOURCE_ABOVE_4G; + } else { + printk(BIOS_INFO, " not prefetch\n"); + } + } else { + printk(BIOS_INFO, " not found\n"); + } + } +} + +static void dgpu_enable_resources(struct device *dev) { + printk(BIOS_INFO, "system76: dgpu_enable_resources %s\n", dev_path(dev)); + + dev->subsystem_vendor = CONFIG_SUBSYSTEM_VENDOR_ID; + dev->subsystem_device = CONFIG_SUBSYSTEM_DEVICE_ID; + printk(BIOS_INFO, " subsystem <- %04x/%04x\n", dev->subsystem_vendor, dev->subsystem_device); + pci_write_config32(dev, 0x40, ((dev->subsystem_device & 0xffff) << 16) | (dev->subsystem_vendor & 0xffff)); + + pci_dev_enable_resources(dev); +} + +static struct device_operations dgpu_pci_ops_dev = { + .read_resources = dgpu_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = dgpu_enable_resources, +#if CONFIG(HAVE_ACPI_TABLES) + .write_acpi_tables = pci_rom_write_acpi_tables, + .acpi_fill_ssdt = pci_rom_ssdt, +#endif + .init = pci_dev_init, + .ops_pci = &pci_dev_ops_pci, +}; + +static void dgpu_above_4g(void *unused) { + struct device *pdev; + + // Find PEG0 + pdev = pcidev_on_root(1, 0); + if (!pdev) { + printk(BIOS_ERR, "system76: failed to find PEG0\n"); + return; + } + printk(BIOS_INFO, "system76: PEG0 at %p, %04x:%04x\n", pdev, pdev->vendor, pdev->device); + + int fn; + for (fn = 0; fn < 8; fn++) { + struct device *dev; + + // Find DGPU functions + dev = pcidev_path_behind(pdev->link_list, PCI_DEVFN(0, fn)); + if (dev) { + printk(BIOS_INFO, "system76: DGPU fn %d at %p, %04x:%04x\n", fn, dev, dev->vendor, dev->device); + dev->ops = &dgpu_pci_ops_dev; + } else { + printk(BIOS_ERR, "system76: failed to find DGPU fn %d\n", fn); + } + } +} + +BOOT_STATE_INIT_ENTRY(BS_DEV_RESOURCES, BS_ON_ENTRY, dgpu_above_4g, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/43615 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I05833e88457bfc3612cc9a8cbf1eec68dcfc5566 Gerrit-Change-Number: 43615 Gerrit-PatchSet: 1 Gerrit-Owner: Jeremy Soller <jeremy(a)system76.com> Gerrit-MessageType: newchange

6 19

Change in coreboot[master]: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and ...
by Cliff Huang (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Bora Guvendik, Selma Bensaid, Paul Menzel. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61887 ) Change subject: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and Redrix ...................................................................... Patch Set 13: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/61887/comment/c28051b1_36013942 PS12, Line 24: MPST > `MPTS` Done -- To view, visit https://review.coreboot.org/c/coreboot/+/61887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f0b7638e90a7862173fca99305398bb250373e9 Gerrit-Change-Number: 61887 Gerrit-PatchSet: 13 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.corp-partner.google.com> Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:34:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and ...
by Cliff Huang (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Bora Guvendik, Cliff Huang, Selma Bensaid, Paul Menzel. Hello Bora Guvendik, build bot (Jenkins), Selma Bensaid, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/61887 to look at the new patch set (#13). Change subject: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and Redrix ...................................................................... mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and Redrix This change is to move MPTS (Mainboard Prepare To Sleep) method from wwan_power.asl to SSDT. MPTS is mainboard-specific method, while wwan_power.asl is meant for WWAN from its name. Having fixed MPTS method (i.e. DSDT) can not cover the case where device only presents and certain CBI bit(s) is(are) set. In Redrix and Brya, there are SKUs with or without 5G, 4G device. For those with 4G, MPTS method should be different. For those with no WWAN device, no MPTS is needed. Having MPTS generating in SSDT also eliminates the need for introducing Kconfig flags to support different devices in the future. MPTS method is created inside mainboard_fill_ssdt function in which the corresponding variant function is called. This will generate the following for the mainboard: Scope (\_SB) { Method (MPTS, 1, Serialized) { Local0 = \_SB.PCI0.RP01.RTD3._STA () If ((Local0 == One)) { \_SB.PCI0.RP01.PXSX.DPTS (Arg0) } } } Test: Check the SSDT for MPTS method under \_SB after boot to OS Use shutdown command and check the GPIO pins from logical analyzer Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I0f0b7638e90a7862173fca99305398bb250373e9 --- M src/mainboard/google/brya/Kconfig.name M src/mainboard/google/brya/mainboard.c M src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/brya/variants/brya0/overridetree.cb M src/mainboard/google/brya/variants/brya4es/overridetree.cb M src/mainboard/google/brya/variants/redrix/overridetree.cb M src/mainboard/google/brya/variants/redrix4es/overridetree.cb 7 files changed, 66 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/61887/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/61887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f0b7638e90a7862173fca99305398bb250373e9 Gerrit-Change-Number: 61887 Gerrit-PatchSet: 13 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.corp-partner.google.com> Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022