coreboot-gerrit May 2022

coreboot-gerrit@coreboot.org

1 participants
6166 discussions

Change in coreboot[master]: soc/intel/alderlake: provide a list of D-states to enter LPM
by Tarun Tuli (Code Review) 02 May '22

02 May '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Paul Menzel. Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63969 ) Change subject: soc/intel/alderlake: provide a list of D-states to enter LPM ...................................................................... Patch Set 13: (9 comments) File src/soc/intel/alderlake/acpi.c: https://review.coreboot.org/c/coreboot/+/63969/comment/d9afeb7e_d5accc16 PS12, Line 179: min_pci_d_states > To me, I think it makes more sense to store a map of PCI device/path to its required D-state, e.g. […] This is actually what I started with. I went with the index hash approach as it ends up in an overall simpler implementation and faster lookup (as we probably can't guarantee any order in this approach). The cost is something that ends up being less readable. It seems the trade-off may be worth it so let me change it back over. https://review.coreboot.org/c/coreboot/+/63969/comment/5e5a8c2e_08f01295 PS12, Line 470: > soc_lpi_include_device() already includes a check for that. Yup precisely! File src/soc/intel/alderlake/acpi.c: https://review.coreboot.org/c/coreboot/+/63969/comment/0a8268ff_7d3ab9f2 PS13, Line 23: #define CPU_PATH_PREFIX "\\_SB.CP" > See `CONFIG_ACPI_CPU_STRING` Ack. Thanks for the hint. https://review.coreboot.org/c/coreboot/+/63969/comment/c326577a_da63cf16 PS13, Line 116: typedef enum { : D0, /* 0 */ : D1, /* 1 */ : D2, /* 2 */ : D3, /* 3 */ : UNDEF : } D_STATES; > In coreboot, we don't usually use a lot of typedefs, and not all capitalized names, either so […] Ack And yes it could apply if the next suggestion is done. (no need for undef) https://review.coreboot.org/c/coreboot/+/63969/comment/0ccb302a_8e00a6cf PS13, Line 440: if (dev && dev->enabled) { : switch (dev->path.type) { : case DEVICE_PATH_PCI: : return (min_pci_d_states[dev->path.pci.devfn] != UNDEF); : : case DEVICE_PATH_APIC: : return true; : : default: : return false; : } : } : return false; > if you take my suggestion from above, this simplifies to: […] I believe we would still want to add a check in here that the entry we're interested in actually exists (as we don't even want to include that device in the count we emit/emit it out if it doesn't). https://review.coreboot.org/c/coreboot/+/63969/comment/2df39651_1173ac1e PS13, Line 455: extern int cpu_get_apic_id(int logical_cpu); > `#include <cpu/cpu. […] Ack https://review.coreboot.org/c/coreboot/+/63969/comment/d18cb982_0dce2345 PS13, Line 499: default: : /* Unhandled */ : acpigen_emit_namestring(NULL); : break; : } > Probably print an error instead? soc_lpi_include_device() should take care of this case already. Yes. Correct, in theory we should actually never get this, but to not overall ruin the output structure in case we ever do (since we need a default case anyways). Added a error as well though for good measure. https://review.coreboot.org/c/coreboot/+/63969/comment/30498432_e1096bec PS13, Line 505: 1 > symbolic constants (e.g. […] Ack https://review.coreboot.org/c/coreboot/+/63969/comment/c330744e_402b3cf2 PS13, Line 521: D0; > nit: […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/63969 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe46a0583c522a8adf0a015cd3a698f694482437 Gerrit-Change-Number: 63969 Gerrit-PatchSet: 13 Gerrit-Owner: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 02 May 2022 23:34:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tarun Tuli <taruntuli(a)google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/mtl: Do initial Meteor Lake SoC commit till bootblock
by Subrata Banik (Code Review) 02 May '22

02 May '22

Attention is currently required from: Felix Singer, Jamie Ryu, Wonkyu Kim, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Angel Pons, Raj Astekar. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62772 ) Change subject: soc/intel/mtl: Do initial Meteor Lake SoC commit till bootblock ...................................................................... Patch Set 10: Code-Review+1 (1 comment) File src/soc/intel/meteorlake/bootblock/bootblock.c: https://review.coreboot.org/c/coreboot/+/62772/comment/a30f2e46_0c8ae284 PS10, Line 20: bootblock_pch_early_init(); : ioe_p2sb_enable_bar(); : fast_spi_cache_bios_region(); : pch_early_iorange_init(); can you please try to logically separate these functions as in 1. bootblock_pch_early_init -> bootblock_soc_early_init (as MTL doesn't have PCH die anymore) 2. ioe_p2sb_enable_bar -> bootblock_ioe_early_init 3. move ioe_p2sb_enable_bar into bootblock_ioe_early_init 4. move fast_spi_cache_bios_region into bootblock_soc_early_init 5. move pch_early_iorange_init into bootblock_soc_early_init -- To view, visit https://review.coreboot.org/c/coreboot/+/62772 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I26479fcc3a3f9c6f8ebf5f198ab0809f0b4a2cc4 Gerrit-Change-Number: 62772 Gerrit-PatchSet: 10 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Ethan Tsao <ethan.tsao(a)intel.corp-partner.google.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-Comment-Date: Mon, 02 May 2022 23:28:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/amd/agesa/family14: Avoid caching fx devices
by Angel Pons (Code Review) 02 May '22

02 May '22

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/64010 ) Change subject: nb/amd/agesa/family14: Avoid caching fx devices ...................................................................... nb/amd/agesa/family14: Avoid caching fx devices Avoid caching fx devices to simplify the code. Ideally, these fx devices would have their own PCI driver to avoid having to get them from another PCI device's own driver. Change-Id: Ifc6510c00e2b1e46b35dea85199c7c73d75226f7 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/amd/agesa/family14/northbridge.c 1 file changed, 31 insertions(+), 71 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/64010/1 diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c index 4737b19..0fb5b4f 100644 --- a/src/northbridge/amd/agesa/family14/northbridge.c +++ b/src/northbridge/amd/agesa/family14/northbridge.c @@ -18,9 +18,15 @@ #include <northbridge/amd/agesa/agesa_helper.h> #include <sb_cimx.h> -static struct device *__f0_dev; -static struct device *__f1_dev; -static unsigned int fx_devs = 0; +static struct device *get_fx_dev(u8 func) +{ + struct device *dev = pcidev_on_root(DEV_CDB, func); + + if (!dev) + die("Cannot find 0:0x%x.%u\n", DEV_CDB, func); + + return dev; +} static u32 get_io_addr_index(u32 nodeid, u32 linkn) { @@ -35,74 +41,37 @@ static void set_io_addr_reg(struct device *dev, u32 nodeid, u32 linkn, u32 reg, u32 io_min, u32 io_max) { + struct device *f1_dev = get_fx_dev(1); u32 tempreg; /* io range allocation */ tempreg = (nodeid & 0xf) | ((nodeid & 0x30) << (8 - 4)) | (linkn << 4) | ((io_max & 0xf0) << (12 - 4)); //limit - pci_write_config32(__f1_dev, reg+4, tempreg); + pci_write_config32(f1_dev, reg+4, tempreg); tempreg = 3 | ((io_min & 0xf0) << (12 - 4)); //base :ISA and VGA ? - pci_write_config32(__f1_dev, reg, tempreg); + pci_write_config32(f1_dev, reg, tempreg); } static void set_mmio_addr_reg(u32 nodeid, u32 linkn, u32 reg, u32 index, u32 mmio_min, u32 mmio_max, u32 nodes) { + struct device *f1_dev = get_fx_dev(1); u32 tempreg; /* io range allocation */ tempreg = (nodeid & 0xf) | (linkn << 4) | (mmio_max & 0xffffff00); - pci_write_config32(__f1_dev, reg + 4, tempreg); + pci_write_config32(f1_dev, reg + 4, tempreg); tempreg = 3 | (nodeid & 0x30) | (mmio_min & 0xffffff00); - pci_write_config32(__f1_dev, reg, tempreg); -} - -static void get_fx_devs(void) -{ - if (fx_devs) - return; - - __f0_dev = pcidev_on_root(DEV_CDB, 0); - __f1_dev = pcidev_on_root(DEV_CDB, 1); - - if (__f0_dev == NULL || __f1_dev == NULL) { - die("Cannot find 0:0x18.[0|1]\n"); - } - - fx_devs = 1; -} - -static u32 f1_read_config32(unsigned int reg) -{ - if (fx_devs == 0) - get_fx_devs(); - return pci_read_config32(__f1_dev, reg); -} - -static void f1_write_config32(unsigned int reg, u32 value) -{ - int i; - if (fx_devs == 0) - get_fx_devs(); - for (i = 0; i < fx_devs; i++) { - struct device *dev; - dev = __f1_dev; - if (dev && dev->enabled) { - pci_write_config32(dev, reg, value); - } - } + pci_write_config32(f1_dev, reg, tempreg); } static int get_dram_base_limit(u32 nodeid, resource_t *basek, resource_t *limitk) { + struct device *f1_dev = get_fx_dev(1); u32 temp; - if (fx_devs == 0) - get_fx_devs(); - - - temp = pci_read_config32(__f1_dev, 0x40 + (nodeid << 3)); //[39:24] at [31:16] + temp = pci_read_config32(f1_dev, 0x40 + (nodeid << 3)); //[39:24] at [31:16] if (!(temp & 1)) return 0; // this memory range is not enabled /* @@ -114,7 +83,7 @@ * BKDG address[35:0] <= {DramLimit[35:24], FF_FFFFh} converted as above but * ORed with 0xffff to get real limit before shifting. */ - temp = pci_read_config32(__f1_dev, 0x44 + (nodeid << 3)); //[39:24] at [31:16] + temp = pci_read_config32(f1_dev, 0x44 + (nodeid << 3)); //[39:24] at [31:16] *limitk = ((temp & 0x0fff0000) | 0xffff) >> (10 - 8); *limitk += 1; // round up last byte @@ -138,7 +107,7 @@ val = 1 | (nodeid << 4) | (linkn << 12); /* it will routing (1)mmio 0xa0000:0xbffff (2) io 0x3b0:0x3bb, 0x3c0:0x3df */ - f1_write_config32(0xf4, val); + pci_write_config32(get_fx_dev(1), 0xf4, val); } @@ -149,11 +118,8 @@ unsigned int nodeid, link = 0; int result; res = 0; - for (nodeid = 0; !res && (nodeid < fx_devs); nodeid++) { - struct device *dev; - dev = __f0_dev; - if (!dev) - continue; + for (nodeid = 0; !res && (nodeid < 1); nodeid++) { + struct device *dev = get_fx_dev(0); for (link = 0; !res && (link < 8); link++) { res = probe_resource(dev, IOINDEX(0x1000 + reg, link)); } @@ -303,7 +269,7 @@ u32 hole; if (get_dram_base_limit(0, &basek, &limitk)) { - hole = pci_read_config32(__f1_dev, 0xf0); + hole = pci_read_config32(get_fx_dev(1), 0xf0); if (hole & 1) { // we find the hole mem_hole.hole_startk = (hole & (0xff << 24)) >> 10; mem_hole.node_id = 0; // record the node No with hole @@ -454,27 +420,21 @@ printk(BIOS_DEBUG, "\nFam14h - %s\n", __func__); /* Find the already assigned resource pairs */ - get_fx_devs(); + struct device *f1_dev = get_fx_dev(1); for (reg = 0x80; reg <= 0xc0; reg += 0x08) { u32 base, limit; - base = f1_read_config32(reg); - limit = f1_read_config32(reg + 0x04); + base = pci_read_config32(f1_dev, reg); + limit = pci_read_config32(f1_dev, reg + 0x04); /* Is this register allocated? */ if ((base & 3) != 0) { unsigned int reg_link; - struct device *reg_dev; + struct device *reg_dev = get_fx_dev(0); reg_link = (limit >> 4) & 7; - reg_dev = __f0_dev; - if (reg_dev) { - /* Reserve the resource */ - struct resource *res; - res = - new_resource(reg_dev, - IOINDEX(0x1000 + reg, - reg_link)); - if (res) { - res->flags = 1; - } + /* Reserve the resource */ + struct resource *res; + res = new_resource(reg_dev, IOINDEX(0x1000 + reg, reg_link)); + if (res) { + res->flags = 1; } } } -- To view, visit https://review.coreboot.org/c/coreboot/+/64010 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifc6510c00e2b1e46b35dea85199c7c73d75226f7 Gerrit-Change-Number: 64010 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: lib/spd_bin: Update names and definitions for LPDDR5
by Cliff Huang (Code Review) 02 May '22

02 May '22

Attention is currently required from: Bora Guvendik, Anil Kumar K, Subrata Banik, Jeremy Compostella, Jérémy Compostella, Selma Bensaid, Paul Menzel, Angel Pons, Bernardo Perez Priego. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58101 ) Change subject: lib/spd_bin: Update names and definitions for LPDDR5 ...................................................................... Patch Set 13: (1 comment) File src/lib/spd_bin.c: https://review.coreboot.org/c/coreboot/+/58101/comment/d1b903cc_d961c003 PS13, Line 27: use_ddr4_ddr5_params > > I wouldn't worry too much about the names. […] okay. then, I think I can abandon this review. -- To view, visit https://review.coreboot.org/c/coreboot/+/58101 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I52ecf24a313d4cbdd0859c623533630c6a6c3713 Gerrit-Change-Number: 58101 Gerrit-PatchSet: 13 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.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: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)intel.corp-partner.google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Comment-Date: Mon, 02 May 2022 23:18:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/amd/agesa/family14: Clean up fx_devs stuff
by Angel Pons (Code Review) 02 May '22

02 May '22

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/64009 ) Change subject: nb/amd/agesa/family14: Clean up fx_devs stuff ...................................................................... nb/amd/agesa/family14: Clean up fx_devs stuff This platform does not support multiple nodes, so get rid of unnecessary complexity. Also remove variables for unused devices. Change-Id: Ic8475d42627c48336c98afdfe659f3bbfb173c3c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/amd/agesa/family14/northbridge.c 1 file changed, 23 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/64009/1 diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c index cf67d93..4737b19 100644 --- a/src/northbridge/amd/agesa/family14/northbridge.c +++ b/src/northbridge/amd/agesa/family14/northbridge.c @@ -18,12 +18,8 @@ #include <northbridge/amd/agesa/agesa_helper.h> #include <sb_cimx.h> -#define FX_DEVS 1 - -static struct device *__f0_dev[FX_DEVS]; -static struct device *__f1_dev[FX_DEVS]; -static struct device *__f2_dev[FX_DEVS]; -static struct device *__f4_dev[FX_DEVS]; +static struct device *__f0_dev; +static struct device *__f1_dev; static unsigned int fx_devs = 0; static u32 get_io_addr_index(u32 nodeid, u32 linkn) @@ -44,10 +40,10 @@ /* io range allocation */ tempreg = (nodeid & 0xf) | ((nodeid & 0x30) << (8 - 4)) | (linkn << 4) | ((io_max & 0xf0) << (12 - 4)); //limit - pci_write_config32(__f1_dev[0], reg+4, tempreg); + pci_write_config32(__f1_dev, reg+4, tempreg); tempreg = 3 | ((io_min & 0xf0) << (12 - 4)); //base :ISA and VGA ? - pci_write_config32(__f1_dev[0], reg, tempreg); + pci_write_config32(__f1_dev, reg, tempreg); } static void set_mmio_addr_reg(u32 nodeid, u32 linkn, u32 reg, u32 index, @@ -57,37 +53,31 @@ u32 tempreg; /* io range allocation */ tempreg = (nodeid & 0xf) | (linkn << 4) | (mmio_max & 0xffffff00); - pci_write_config32(__f1_dev[0], reg + 4, tempreg); + pci_write_config32(__f1_dev, reg + 4, tempreg); tempreg = 3 | (nodeid & 0x30) | (mmio_min & 0xffffff00); - pci_write_config32(__f1_dev[0], reg, tempreg); -} - -static struct device *get_node_pci(u32 nodeid, u32 fn) -{ - return pcidev_on_root(DEV_CDB + nodeid, fn); + pci_write_config32(__f1_dev, reg, tempreg); } static void get_fx_devs(void) { - int i; - for (i = 0; i < FX_DEVS; i++) { - __f0_dev[i] = get_node_pci(i, 0); - __f1_dev[i] = get_node_pci(i, 1); - __f2_dev[i] = get_node_pci(i, 2); - __f4_dev[i] = get_node_pci(i, 4); - if (__f0_dev[i] != NULL && __f1_dev[i] != NULL) - fx_devs = i + 1; - } - if (__f1_dev[0] == NULL || __f0_dev[0] == NULL || fx_devs == 0) { + if (fx_devs) + return; + + __f0_dev = pcidev_on_root(DEV_CDB, 0); + __f1_dev = pcidev_on_root(DEV_CDB, 1); + + if (__f0_dev == NULL || __f1_dev == NULL) { die("Cannot find 0:0x18.[0|1]\n"); } + + fx_devs = 1; } static u32 f1_read_config32(unsigned int reg) { if (fx_devs == 0) get_fx_devs(); - return pci_read_config32(__f1_dev[0], reg); + return pci_read_config32(__f1_dev, reg); } static void f1_write_config32(unsigned int reg, u32 value) @@ -97,7 +87,7 @@ get_fx_devs(); for (i = 0; i < fx_devs; i++) { struct device *dev; - dev = __f1_dev[i]; + dev = __f1_dev; if (dev && dev->enabled) { pci_write_config32(dev, reg, value); } @@ -112,7 +102,7 @@ get_fx_devs(); - temp = pci_read_config32(__f1_dev[nodeid], 0x40 + (nodeid << 3)); //[39:24] at [31:16] + temp = pci_read_config32(__f1_dev, 0x40 + (nodeid << 3)); //[39:24] at [31:16] if (!(temp & 1)) return 0; // this memory range is not enabled /* @@ -124,7 +114,7 @@ * BKDG address[35:0] <= {DramLimit[35:24], FF_FFFFh} converted as above but * ORed with 0xffff to get real limit before shifting. */ - temp = pci_read_config32(__f1_dev[nodeid], 0x44 + (nodeid << 3)); //[39:24] at [31:16] + temp = pci_read_config32(__f1_dev, 0x44 + (nodeid << 3)); //[39:24] at [31:16] *limitk = ((temp & 0x0fff0000) | 0xffff) >> (10 - 8); *limitk += 1; // round up last byte @@ -161,7 +151,7 @@ res = 0; for (nodeid = 0; !res && (nodeid < fx_devs); nodeid++) { struct device *dev; - dev = __f0_dev[nodeid]; + dev = __f0_dev; if (!dev) continue; for (link = 0; !res && (link < 8); link++) { @@ -313,7 +303,7 @@ u32 hole; if (get_dram_base_limit(0, &basek, &limitk)) { - hole = pci_read_config32(__f1_dev[0], 0xf0); + hole = pci_read_config32(__f1_dev, 0xf0); if (hole & 1) { // we find the hole mem_hole.hole_startk = (hole & (0xff << 24)) >> 10; mem_hole.node_id = 0; // record the node No with hole @@ -471,15 +461,10 @@ limit = f1_read_config32(reg + 0x04); /* Is this register allocated? */ if ((base & 3) != 0) { - unsigned int nodeid, reg_link; + unsigned int reg_link; struct device *reg_dev; - if (reg < 0xc0) { // mmio - nodeid = (limit & 0xf) + (base & 0x30); - } else { // io - nodeid = (limit & 0xf) + ((base >> 4) & 0x30); - } reg_link = (limit >> 4) & 7; - reg_dev = __f0_dev[nodeid]; + reg_dev = __f0_dev; if (reg_dev) { /* Reserve the resource */ struct resource *res; -- To view, visit https://review.coreboot.org/c/coreboot/+/64009 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic8475d42627c48336c98afdfe659f3bbfb173c3c Gerrit-Change-Number: 64009 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/usb: Add chip driver for VL822 USB hub
by Karthik Ramasubramanian (Code Review) 02 May '22

02 May '22

Attention is currently required from: Raul Rangel, Tim Wawrzynczak, Jon Murphy. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63968 ) Change subject: drivers/usb: Add chip driver for VL822 USB hub ...................................................................... Patch Set 2: (2 comments) File src/drivers/usb/vl/acpi_vl822.c: https://review.coreboot.org/c/coreboot/+/63968/comment/3da5d507_cbc394d0 PS2, Line 91: ViaLabs VL822 > Nothing about this file looks like it's VL822 specific. […] I agree. There is nothing specific about this driver for VL822. I can rename it to a generic USB hub File src/drivers/usb/vl/chip.h: https://review.coreboot.org/c/coreboot/+/63968/comment/48d109c7_17ca5fb1 PS2, Line 9: port_count > Or maybe we add a port_count parameter to the existing `drivers/usb/acpi` and change this condition? […] I thought about this approach such that USB hub driver is integrated with USB ACPI driver. But the acpi_name operation for the USB leaf node (i.e. downstream ports) has to be NULL such that the hub(parent) can define the ACPI name for them. Once it is defined and returns NULL as ACPI name for leaf nodes, then it breaks the ACPI path for leaf nodes. https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thi… -- To view, visit https://review.coreboot.org/c/coreboot/+/63968 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I11d7ccc42d3dce8e136eb771f120825980e5c027 Gerrit-Change-Number: 63968 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Comment-Date: Mon, 02 May 2022 23:02:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/agah: Add GPU power sequencing
by Tim Wawrzynczak (Code Review) 02 May '22

02 May '22

Attention is currently required from: Ivy Jian, Angel Pons, Nick Vaccaro, Eric Lai. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62384 ) Change subject: mb/google/brya/var/agah: Add GPU power sequencing ...................................................................... Patch Set 6: (2 comments) Patchset: PS3: > Thanks to you! Let me know if this is ready to go and I'll give a +2 Ok, I think this newest rev should be good for the now 😊 File src/mainboard/google/brya/variants/agah/variant.c: https://review.coreboot.org/c/coreboot/+/62384/comment/2fabc974_c50579bf PS3, Line 164: 0x100 > Ack Oops yes it's PCI config space size, updated the code with a #define. -- To view, visit https://review.coreboot.org/c/coreboot/+/62384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1072be12ef58af5859e2a2d19c4a9c1adc0b0f88 Gerrit-Change-Number: 62384 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Mon, 02 May 2022 23:02:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/acpi: Add support for NBCI _DSM subfunction
by Tim Wawrzynczak (Code Review) 02 May '22

02 May '22

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/64008 ) Change subject: mb/google/brya/acpi: Add support for NBCI _DSM subfunction ...................................................................... mb/google/brya/acpi: Add support for NBCI _DSM subfunction The Nvidia GPU supports another function named NBCI (NoteBook Common Interface), which has some subfunctions which are required for the Nvidia kernel driver to consume. BUG=b:214581763 TEST=build Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I19eb9417923d297a084d6f5329682e91cd506a9e --- M src/mainboard/google/brya/acpi/gpu_top.asl A src/mainboard/google/brya/acpi/nbci.asl 2 files changed, 82 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/64008/1 diff --git a/src/mainboard/google/brya/acpi/gpu_top.asl b/src/mainboard/google/brya/acpi/gpu_top.asl index bfc5aea..0fb2867 100644 --- a/src/mainboard/google/brya/acpi/gpu_top.asl +++ b/src/mainboard/google/brya/acpi/gpu_top.asl @@ -22,6 +22,7 @@ #include "power.asl" #include "nvop.asl" #include "nvjt.asl" + #include "nbci.asl" Method (_DSM, 4, Serialized) { @@ -39,6 +40,13 @@ Return (NVJT (Arg2, Arg3)) } } + ElseIf (Arg0 == ToUUID (UUID_NBCI)) + { + If (ToInteger (Arg1) >= REVISION_MIN_NBCI) + { + Return (NBCI (Arg2, Arg3)) + } + } Return (NV_ERROR_UNSUPPORTED) } diff --git a/src/mainboard/google/brya/acpi/nbci.asl b/src/mainboard/google/brya/acpi/nbci.asl new file mode 100644 index 0000000..0564f7a --- /dev/null +++ b/src/mainboard/google/brya/acpi/nbci.asl @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#define NBCI_FUNC_SUPPORT 0 +#define NBCI_FUNC_PLATCAPS 1 +#define NBCI_FUNC_GETOBJBYTYPE 16 +#define NBCI_FUNC_GETCALLBACKS 19 + +/* 'DR' in ASCII, for DRiver Object */ +#define NBCI_OBJTYPE_DR 0x4452 + +Method (NBCI, 2, Serialized) +{ + Switch (ToInteger (Arg0)) + { + Case (NBCI_FUNC_SUPPORT) + { + Return (ITOB( + (1 << NBCI_FUNC_SUPPORT) | + (1 << NBCI_FUNC_PLATCAPS) | + (1 << NBCI_FUNC_GETOBJBYTYPE))) + } + Case (NBCI_FUNC_PLATCAPS) + { + Return (ITOB( + (0 << 10) | /* No 3D Hotkeys */ + (0 << 9) | /* Do not enumerate a dock */ + (0 << 7) | /* Does not have DISPLAYSTATUS */ + (0 << 5) | /* No LID support */ + (0 << 4))) /* No Aux power state request */ + } + Case (NBCI_FUNC_GETCALLBACKS) + { + Return (0) + } + Case (NBCI_FUNC_GETOBJBYTYPE) + { + CreateWordField (Arg1, 2, BFF0) + If (BFF0 == NBCI_OBJTYPE_DR) + { + Return (Buffer(0xa1) + { + /* DR ("Driver Object") is a data object which + * might vary depending on the eDP panel used. */ + 0x57, 0x74, 0xdc, 0x86, 0x75, 0x84, 0xec, 0xe7, + 0x52, 0x44, 0xa1, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, 0xde, 0x10, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x34, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x47, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x45, 0x00, 0x00, 0x00, 0x03, 0x00, + 0x51, 0x00, 0x00, 0x00, 0x04, 0x00, 0x4f, 0x00, + 0x00, 0x00, 0x05, 0x00, 0x4d, 0x00, 0x00, 0x00, + 0x06, 0x00, 0x4b, 0x00, 0x00, 0x00, 0x07, 0x00, + 0x49, 0x00, 0x00, 0x00, 0x08, 0x00, 0x47, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xd9, 0x1c, + 0x04, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x41, 0x5d, 0xc9, 0x00, 0x01, 0x24, 0x2e, 0x00, + 0x02, 0x00, 0xff, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0xd9, 0x1c, 0x04, 0x00, 0x00, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x60, 0x68, 0x9e, + 0x35, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00 + }) + } Else { + Return (NV_ERROR_UNSPECIFIED) + } + + } + } + + Return (NV_ERROR_UNSUPPORTED) +} -- To view, visit https://review.coreboot.org/c/coreboot/+/64008 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19eb9417923d297a084d6f5329682e91cd506a9e Gerrit-Change-Number: 64008 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/brya/var/taeko{4es}: Remove extraneous __weak attributes
by Tim Wawrzynczak (Code Review) 02 May '22

02 May '22

Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63179 to look at the new patch set (#2). Change subject: mb/google/brya/var/taeko{4es}: Remove extraneous __weak attributes ...................................................................... mb/google/brya/var/taeko{4es}: Remove extraneous __weak attributes Functions that are intended to override weak ones defined in the baseboard should not also be declared weak, otherwise how would the linker know which copy to keep. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Ia2ceee77d00a5baa915fd1f306d76e79aa609e65 --- M src/mainboard/google/brya/variants/taeko/memory.c M src/mainboard/google/brya/variants/taeko4es/memory.c 2 files changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/63179/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63179 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia2ceee77d00a5baa915fd1f306d76e79aa609e65 Gerrit-Change-Number: 63179 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya/var/agah: Select INCLUDE_NVIDIA_GPU_ASL
by Tim Wawrzynczak (Code Review) 02 May '22

02 May '22

Attention is currently required from: Subrata Banik. Hello build bot (Jenkins), Subrata Banik, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/62932 to look at the new patch set (#3). Change subject: mb/google/brya/var/agah: Select INCLUDE_NVIDIA_GPU_ASL ...................................................................... mb/google/brya/var/agah: Select INCLUDE_NVIDIA_GPU_ASL The agah variant will include an Nvidia GN20 series GPU, therefore select the INCLUDE_NVIDIA_GPU_ASL Kconfig to include the respective ASL code into the DSDT. BUG=b:214581763 TEST=build patch train Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Icc718d01506ccb4dd42841239e96926f4ddaa9c9 --- M src/mainboard/google/brya/Kconfig.name 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/62932/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/62932 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icc718d01506ccb4dd42841239e96926f4ddaa9c9 Gerrit-Change-Number: 62932 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2022