coreboot-gerrit May 2022

coreboot-gerrit@coreboot.org

1 participants
6166 discussions

Change in coreboot[master]: soc/intel/elkhartlake/systemagent: Disable RAPL based on Kconfig
by Uwe Poeche (Code Review) 02 May '22

02 May '22

Attention is currently required from: Uwe Poeche. Hello build bot (Jenkins), Werner Zeh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63547 to look at the new patch set (#3). Change subject: soc/intel/elkhartlake/systemagent: Disable RAPL based on Kconfig ...................................................................... soc/intel/elkhartlake/systemagent: Disable RAPL based on Kconfig This patch provides the possibility for EHL based boards to disable RAPL settings via SOC_INTEL_DISABLE_POWER_LIMITS config switch. Other than on previouse SOCs this needs to be done in an MCHBAR mapped register rather than via MSR on Elkhart Lake. Test: Check MCHBAR mapped registers (MCH_PKG_POWER_LIMIT) on mc_ehl1. Change-Id: I5be6632b15ab8e14a21b5cd35152f82fec919d9f Signed-off-by: Uwe Poeche <uwe.poeche(a)siemens.com> --- M src/soc/intel/elkhartlake/include/soc/systemagent.h M src/soc/intel/elkhartlake/systemagent.c 2 files changed, 15 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/63547/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5be6632b15ab8e14a21b5cd35152f82fec919d9f Gerrit-Change-Number: 63547 Gerrit-PatchSet: 3 Gerrit-Owner: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: intel/common/block: Provide RAPL and min clock ratio switches in common
by Uwe Poeche (Code Review) 02 May '22

02 May '22

Attention is currently required from: Uwe Poeche, Andrey Petrov. Hello build bot (Jenkins), Werner Zeh, Andrey Petrov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63546 to look at the new patch set (#3). Change subject: intel/common/block: Provide RAPL and min clock ratio switches in common ...................................................................... intel/common/block: Provide RAPL and min clock ratio switches in common There are two APL specific config switches for RAPL and min. cpu clock (APL_SKIP_SET_POWER_LIMITS, APL_SET_MIN_CLOCK_RATIO). This switches could be used in future in other CPU platforms. Move them to common code instead of having them just for one SOC. Test: Make sure that the clock ratio (MSR 0x198) and the RAPL settings (MSR0x610) do not change with this patch applied on mc_apl{1,4,5} mainboard. Change-Id: I3d63d1b9b6c96586a3c20bf8c1d8001b1d7c4bed Signed-off-by: Uwe Poeche <uwe.poeche(a)siemens.com> --- M src/mainboard/siemens/mc_apl1/variants/mc_apl1/Kconfig M src/mainboard/siemens/mc_apl1/variants/mc_apl3/Kconfig M src/mainboard/siemens/mc_apl1/variants/mc_apl4/Kconfig M src/mainboard/siemens/mc_apl1/variants/mc_apl5/Kconfig M src/mainboard/siemens/mc_apl1/variants/mc_apl6/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/common/block/cpu/Kconfig 9 files changed, 25 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/63546/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3d63d1b9b6c96586a3c20bf8c1d8001b1d7c4bed Gerrit-Change-Number: 63546 Gerrit-PatchSet: 3 Gerrit-Owner: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

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

02 May '22

Attention is currently required from: Felix Singer, Jamie Ryu, Wonkyu Kim, Subrata Banik, Ethan Tsao, Paul Menzel, Angel Pons, Raj Astekar, Ravishankar Sarawadi. Ravishankar Sarawadi 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 8: (13 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/62772/comment/0b906848_e0cd807b PS4, Line 16: > Please mention that in the commit message. Done Commit Message: https://review.coreboot.org/c/coreboot/+/62772/comment/361b15c0_be12571f PS5, Line 17: Signed-off-by: Ravi Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> > Add a TEST= line? If you already have messages, please paste some of them, or at least mention the b […] Done File src/soc/intel/meteorlake/Kconfig: https://review.coreboot.org/c/coreboot/+/62772/comment/8038a889_e0f210da PS7, Line 56: ifd2 > Why not "mtl" instead? ifdtool seems to know about it. Done File src/soc/intel/meteorlake/bootblock/pch.c: https://review.coreboot.org/c/coreboot/+/62772/comment/4bb5c273_68ba6208 PS5, Line 14: include <intelblocks/ioc.h> > https://review.coreboot. […] Done File src/soc/intel/meteorlake/bootblock/pch.c: https://review.coreboot.org/c/coreboot/+/62772/comment/2e4ad270_bc562e11 PS7, Line 85: */ > nit: add space before comment end Done https://review.coreboot.org/c/coreboot/+/62772/comment/7f02c942_ca4c9d47 PS7, Line 85: */ > nit: add space before comment end Done File src/soc/intel/meteorlake/bootblock/report_platform.c: https://review.coreboot.org/c/coreboot/+/62772/comment/d434c4de_abef871b PS5, Line 35: pch_table[] = { : { PCI_DID_INTEL_MTL_ESPI_0, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_1, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_2, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_3, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_4, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_5, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_6, "Meteorlake SOC" }, : { PCI_DID_INTEL_MTL_ESPI_7, "Meteorlake SOC" }, > This will have different SKU info string. […] Done File src/soc/intel/meteorlake/bootblock/report_platform.c: https://review.coreboot.org/c/coreboot/+/62772/comment/2dce80a5_0fb46a0c PS7, Line 27: MeteorLake > Hmmm, the other entries spell "Meteorlake" with the 'l' in lowercase. […] "MeteorLake" File src/soc/intel/meteorlake/include/soc/espi.h: https://review.coreboot.org/c/coreboot/+/62772/comment/abb80475_5b4f45e8 PS7, Line 17: */ > nit: Add space before comment end Done https://review.coreboot.org/c/coreboot/+/62772/comment/e33434f1_b52ff014 PS7, Line 18: */ > nit: Add space before comment end Done File src/soc/intel/meteorlake/include/soc/iomap.h: https://review.coreboot.org/c/coreboot/+/62772/comment/c90c4e9e_92903176 PS7, Line 73: /* : #define PID_IOM 0xAA : #define IOM_BASE_ADDR (IOE_PCR_ABOVE_4G_BASE_ADDR + (PID_IOM << 16)) : */ > Why is this commented out? Done File src/soc/intel/meteorlake/include/soc/pci_devs.h: https://review.coreboot.org/c/coreboot/+/62772/comment/c6111fd5_c17af656 PS7, Line 11: pcidev_path_on_root_debug > Could we please use `pcidev_path_on_root` instead? Done https://review.coreboot.org/c/coreboot/+/62772/comment/8f7d1639_c3d07e3e PS7, Line 59: #define PCI_DEV_SLOT_GNA 0x08 : #define PCI_DEVFN_GNA _PCI_DEVFN(GNA, 0) : #define PCI_DEV_GNA _PCI_DEV(GNA, 0) > nit: Move above `PCI_DEV_SLOT_TCSS` to keep ordering? Done -- 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: 8 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: 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: 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-CC: Subrata Banik <subratabanik(a)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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(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 06:17:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com> Comment-In-Reply-To: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-MessageType: comment

1 0

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

02 May '22

Attention is currently required from: Felix Singer, Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Raj Astekar. Hello build bot (Jenkins), Jamie Ryu, Wonkyu Kim, Ethan Tsao, Raj Astekar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/62772 to look at the new patch set (#9). Change subject: soc/intel/mtl: Do initial Meteor Lake SoC commit till bootblock ...................................................................... soc/intel/mtl: Do initial Meteor Lake SoC commit till bootblock Base code is based of Intel Alder Lake SOC code. List of changes: 1. Add required Meteor Lake SoC programming till bootblock 2. Include only required headers into include/soc 3. Include MTL-P related DID, BDF 4. Ref: Processor EDS documents vol1 #621483, vol2 #640858 TEST= Build 'util/abuild/abuild -p none -t google/rex -a -c max'. Signed-off-by: Ravi Sarawadi <ravishankar.sarawadi(a)intel.com> Change-Id: I26479fcc3a3f9c6f8ebf5f198ab0809f0b4a2cc4 --- A src/soc/intel/meteorlake/Kconfig A src/soc/intel/meteorlake/Makefile.inc A src/soc/intel/meteorlake/bootblock/bootblock.c A src/soc/intel/meteorlake/bootblock/pch.c A src/soc/intel/meteorlake/bootblock/report_platform.c A src/soc/intel/meteorlake/include/soc/bootblock.h A src/soc/intel/meteorlake/include/soc/espi.h A src/soc/intel/meteorlake/include/soc/iomap.h A src/soc/intel/meteorlake/include/soc/p2sb.h A src/soc/intel/meteorlake/include/soc/pci_devs.h A src/soc/intel/meteorlake/include/soc/pcr_ids.h A src/soc/intel/meteorlake/include/soc/pm.h A src/soc/intel/meteorlake/include/soc/smbus.h 13 files changed, 1,024 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/62772/9 -- 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: 9 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: 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: 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-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.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: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree
by Werner Zeh (Code Review) 02 May '22

02 May '22

Attention is currently required from: Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63862 ) Change subject: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree ...................................................................... Patch Set 2: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63862/comment/903d05c4_5558b9d9 PS2, Line 9: Controller controllers https://review.coreboot.org/c/coreboot/+/63862/comment/1d8a54b7_033a41df PS2, Line 10: including Do you want to mention that they are controlled by the PSE? Or what is this 'including' referring to? -- To view, visit https://review.coreboot.org/c/coreboot/+/63862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I74e660548b2c44d5dbdb6023d5a36cfdd7e96f43 Gerrit-Change-Number: 63862 Gerrit-PatchSet: 2 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Mon, 02 May 2022 05:49:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Implement TSN GbE driver
by Werner Zeh (Code Review) 02 May '22

02 May '22

Attention is currently required from: Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63861 ) Change subject: soc/intel/elkhartlake: Implement TSN GbE driver ...................................................................... Patch Set 2: (2 comments) File src/soc/intel/elkhartlake/Kconfig: https://review.coreboot.org/c/coreboot/+/63861/comment/ef73d16e_75853e0d PS2, Line 240: TSN_GBE_DRIVER Since this would be an elkhart lake spcific driver should we name it EHL_TSN_DRIVER? File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63861/comment/c5cbe5f0_e8e15f28 PS2, Line 15: if (!io_mem_base) { : printk(BIOS_ERR, "TSN GbE: Error can't find I/O MEM resource\n"); : return; : } This check is not needed here as it is already done in find_resource() for you. and even more, find_resource() will die() if the resource is missing. So you will never reach this code here and therefore can remove it completely. -- To view, visit https://review.coreboot.org/c/coreboot/+/63861 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7522914c56b74486bb088280d2686acf7027d1d3 Gerrit-Change-Number: 63861 Gerrit-PatchSet: 2 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Mon, 02 May 2022 05:45:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses
by Werner Zeh (Code Review) 02 May '22

02 May '22

Attention is currently required from: Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63863 ) Change subject: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses ...................................................................... Patch Set 2: Code-Review+1 (3 comments) File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63863/comment/fcd14049_452b683e PS2, Line 11: adr_to_set Maybe just call it 'mac' here to shorten down the name a bit? https://review.coreboot.org/c/coreboot/+/63863/comment/ebd33131_e7c0760d PS2, Line 16: printk(BIOS_ERR, "TSN GbE: No valid MAC address found\n"); I wouldn't print that with ERROR level as there are valid cases where the mainboard code does not provide an address. I guess INFO would be better here, or? https://review.coreboot.org/c/coreboot/+/63863/comment/e3e14826_2117a78b PS2, Line 25: *mac_p = (*mac_p & 0xFFFF0000) | (adr_to_set[5] << 8) | adr_to_set[4]; How about uing clrsetbits32() from device/mmio.h here? Could look like: clrsetbits32((void *)(io_mem_base + TSN_MAC_ADDRESS0_HIGH_OFFSET), 0xffff, ((adr_to_set[5] << 8) | adr_to_set[4])); And if you shorten down the register and bar pointer name a bit and rename 'adr_to_set' to 'mac' this could fit on one line. -- To view, visit https://review.coreboot.org/c/coreboot/+/63863 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2303a64cfd09fa02734ca9452d26591af2a76221 Gerrit-Change-Number: 63863 Gerrit-PatchSet: 2 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Mon, 02 May 2022 05:41:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/siemens/mc_ehl/Kconfig: disable RAPL
by Werner Zeh (Code Review) 02 May '22

02 May '22

Attention is currently required from: Uwe Poeche. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63548 ) Change subject: mb/siemens/mc_ehl/Kconfig: disable RAPL ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63548/comment/d751a582_283332af PS2, Line 7: mb/siemens/mc_ehl Just 'mb/siemens/mc_ehl' should be enough here. https://review.coreboot.org/c/coreboot/+/63548/comment/efde0815_743cf764 PS2, Line 7: disable Disable https://review.coreboot.org/c/coreboot/+/63548/comment/d5b605e3_4115a78f PS2, Line 9: With that patch RAPL for the mentioned mainboards is deactivated. Maybe better: 'Disable RAPL for all mainboards based on mc_ehl.' https://review.coreboot.org/c/coreboot/+/63548/comment/418bd70b_03fb2ddd PS2, Line 11: mc_ehl1 with and without this patch and check the mchbar : register. Boot mc_ehl1 with this patch and ensure the bits in the MCBAR register are cleared. -- To view, visit https://review.coreboot.org/c/coreboot/+/63548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie58a4b6444d5be088ac2b25ff0a2f5cd33120ace Gerrit-Change-Number: 63548 Gerrit-PatchSet: 2 Gerrit-Owner: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Comment-Date: Mon, 02 May 2022 05:13:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake/systemagent: possibility of deactivate RAPL
by Werner Zeh (Code Review) 02 May '22

02 May '22

Attention is currently required from: Uwe Poeche. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63547 ) Change subject: soc/intel/elkhartlake/systemagent: possibility of deactivate RAPL ...................................................................... Patch Set 2: (7 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63547/comment/bee6183d_2320e40f PS2, Line 7: soc/intel/elkhartlake/systemagent: possibility of deactivate RAPL soc/intel/elkhartlake: Disable RAPL based on Kconfig https://review.coreboot.org/c/coreboot/+/63547/comment/6404fa63_e8bc4cb4 PS2, Line 9: deactivate disable https://review.coreboot.org/c/coreboot/+/63547/comment/f9338ba6_e417c565 PS2, Line 10: as in APL based boards SOC_INTEL_DISABLE_POWER_LIMITS https://review.coreboot.org/c/coreboot/+/63547/comment/65e87c74_680f5cb9 PS2, Line 10: The only : difference in EHL is the necessary usage of an MCHBAR register instead : the relevant MSR (Intel changes EDS at the moment). Other than on previouse SOCs this needs to be done in an MCHBAR mapped register rather than via MSR on Elkhart Lake. https://review.coreboot.org/c/coreboot/+/63547/comment/14f2a443_d5721fb5 PS2, Line 15: On siemens/mc_ehl1 checking the MCHBAR register with and without the : relevant config switch. Check MCHBAR mapped registers (MCH_PKG_POWER_LIMIT) on mc_ehl1. File src/soc/intel/elkhartlake/systemagent.c: https://review.coreboot.org/c/coreboot/+/63547/comment/08c84c31_e6cbc9c9 PS2, Line 51: u32 elkhartlake code usually uses uint{8,16,32}_t types. I would do it here, too to stay consistent. https://review.coreboot.org/c/coreboot/+/63547/comment/940d4ea9_71b5b34f PS2, Line 61: Skip setting RAPL per configuration\n Here you actually disable RAPL directly, so maybe "Disable RAPL" would be more clear here? -- To view, visit https://review.coreboot.org/c/coreboot/+/63547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5be6632b15ab8e14a21b5cd35152f82fec919d9f Gerrit-Change-Number: 63547 Gerrit-PatchSet: 2 Gerrit-Owner: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Comment-Date: Mon, 02 May 2022 05:10:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: intel/common/block: provide config switches from APL specific
by Werner Zeh (Code Review) 02 May '22

02 May '22

Attention is currently required from: Uwe Poeche, Andrey Petrov. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63546 ) Change subject: intel/common/block: provide config switches from APL specific ...................................................................... Patch Set 2: Code-Review+1 (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63546/comment/5085a91e_99f6ea1c PS2, Line 7: provide config switches from APL specific This could be formulated more clear, e.g.: 'intel/common/block: Provide RAPL and min clock ratio switches in common' or the like https://review.coreboot.org/c/coreboot/+/63546/comment/aa5b8a38_3eab939d PS2, Line 10: APL_SKIP_SET_POWER_LIMITS APL_SET_MIN_CLOCK_RATIO https://review.coreboot.org/c/coreboot/+/63546/comment/d2e48349_6e04a537 PS2, Line 12: Therefore they moved here with other name to common/block. Maybe 'Move them to common code instead of having them just for one SOC' https://review.coreboot.org/c/coreboot/+/63546/comment/7251d0dd_2871f1fa PS2, Line 14: mainboards mc_apl1/4/5: compare cpu clock via MSR 0x198 and RAPL : settings via MSR 0x610 before and after the change. Maybe: Make sure that the clock ratio (MSR 0x198) and the RAPL settings (MSR0x610) do not change with this patch applied on mc_apl{1,4,5} mainboard. File src/soc/intel/common/block/cpu/Kconfig: https://review.coreboot.org/c/coreboot/+/63546/comment/d585db15_c9275c2f PS2, Line 144: Some Apollo Lake mainboards do not need the Running Average Power : Limits (RAPL) algorithm for a constant power management. : Set this config option to skip the RAPL configuration. I would rephrase this text as it is not APL specific anymore -- To view, visit https://review.coreboot.org/c/coreboot/+/63546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3d63d1b9b6c96586a3c20bf8c1d8001b1d7c4bed Gerrit-Change-Number: 63546 Gerrit-PatchSet: 2 Gerrit-Owner: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Mon, 02 May 2022 04:45:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2022