coreboot-gerrit August 2020

coreboot-gerrit@coreboot.org

1 participants
2457 discussions

Change in coreboot[master]: soc/intel/skylake: acpi: drop HWP's dependency on EIST
by Matt Delco (Code Review) 03 Aug '20

03 Aug '20

Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44137 ) Change subject: soc/intel/skylake: acpi: drop HWP's dependency on EIST ...................................................................... Patch Set 3: (1 comment) The basis of the change (i.e., that the chip features are independent) is valid, though I'm not clear on the benefit/need unless there's some Intel chips that now support HWP but not EIST. Most of the Intel-based systems that don't set "eist_enable" in their devicetree are probably doing it by accidental oversight, e.g.: https://review.coreboot.org/c/coreboot/+/29664 particularly since coreboot is most often used by Linux (where the pstate driver typical forces on EIST or HWP) vs Windows (which expects the features to be advertised/enable by the firmware before the OS attempts to use them). In the past there's been some informal chats before about whether eist_enable should be enabled by default or disappear entirely. The current code does force an unnecessary relationship between HWP and EIST though it does help to force people to remember to enable EIST. What I describe here is mostly trying to provide some background/content on the situation rather than be an argument against the change. IMO, the the current way these features are structured in coreboot aren't ideal but addressing them would be a much bigger change that's probably outside the scope of what anyone wants to do right now. https://review.coreboot.org/c/coreboot/+/44137/3/src/soc/intel/skylake/acpi… File src/soc/intel/skylake/acpi.c: https://review.coreboot.org/c/coreboot/+/44137/3/src/soc/intel/skylake/acpi… PS3, Line 402: { I'm not knowledgable about the coreboot coding standard, though in general I think it tries to avoid braces on single-line statements. -- To view, visit https://review.coreboot.org/c/coreboot/+/44137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93d888ddce7b54e91b54e5b4fdd4d9cf16630eda Gerrit-Change-Number: 44137 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 03 Aug 2020 17:04:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/sandybridge: Refactor `get_pcie_bar`
by Angel Pons (Code Review) 03 Aug '20

03 Aug '20

Hello Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44121 to look at the new patch set (#4). Change subject: nb/intel/sandybridge: Refactor `get_pcie_bar` ...................................................................... nb/intel/sandybridge: Refactor `get_pcie_bar` Turn it into `decode_pcie_bar`, taken from gm45. Change-Id: Id1c2cfbcac1a798d046beced790930511dc97972 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/northbridge.c 1 file changed, 17 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/44121/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/44121 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id1c2cfbcac1a798d046beced790930511dc97972 Gerrit-Change-Number: 44121 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/zork: Pass oscout system clk to rt5682
by Akshu Agrawal (Code Review) 03 Aug '20

03 Aug '20

Hello Akshu Agrawal, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/44010 to review the following change. Change subject: mb/google/zork: Pass oscout system clk to rt5682 ...................................................................... mb/google/zork: Pass oscout system clk to rt5682 mclk-name is set to 48Mhz oscclk1, oscout system clock is the linked to rt5682 mclk. Signed-off-by: Akshu Agrawal <akshu.agrawal(a)amd.com> BUG=b:158906189 TEST=rt5682 driver get the correct clk and tested audio playback Change-Id: Ic565e8e0573e085e5759b2d3688cc0a4533b67fe --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/44010/1 diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb index e2d9183..d155a6b 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb @@ -301,10 +301,13 @@ register "uid" = "1" register "desc" = ""Realtek RT5682"" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(62)" - register "property_count" = "1" + register "property_count" = "2" register "property_list[0].type" = "ACPI_DP_TYPE_INTEGER" register "property_list[0].name" = ""realtek,jd-src"" register "property_list[0].integer" = "1" + register "property_list[1].type" = "ACPI_DP_TYPE_STRING" + register "property_list[1].name" = ""realtek,mclk-name"" + register "property_list[1].string" = ""oscout1"" device i2c 1a on end end end diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb index bbef702..0831b7e 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb @@ -330,10 +330,13 @@ register "uid" = "1" register "desc" = ""Realtek RT5682"" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(62)" - register "property_count" = "1" + register "property_count" = "2" register "property_list[0].type" = "ACPI_DP_TYPE_INTEGER" register "property_list[0].name" = ""realtek,jd-src"" register "property_list[0].integer" = "1" + register "property_list[1].type" = "ACPI_DP_TYPE_STRING" + register "property_list[1].name" = ""realtek,mclk-name"" + register "property_list[1].string" = ""oscout1"" device i2c 1a on end end end -- To view, visit https://review.coreboot.org/c/coreboot/+/44010 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic565e8e0573e085e5759b2d3688cc0a4533b67fe Gerrit-Change-Number: 44010 Gerrit-PatchSet: 1 Gerrit-Owner: Akshu Agrawal <akshu.agrawal(a)amd.corp-partner.google.com> Gerrit-Reviewer: Akshu Agrawal <akshu.agrawal(a)amd.com> Gerrit-MessageType: newchange

2 6

Change in coreboot[master]: mb/google/zork: set is_rv to 1 for RV family
by Akshu Agrawal (Code Review) 03 Aug '20

03 Aug '20

Hello Akshu Agrawal, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/44009 to review the following change. Change subject: mb/google/zork: set is_rv to 1 for RV family ...................................................................... mb/google/zork: set is_rv to 1 for RV family RV has difference in clk framework. To differentiate set the fmw property to 1 for boards using RV family of SoC. Signed-off-by: Akshu Agrawal <akshu.agrawal(a)amd.com> BUG=b:158906189 TEST=rt5682 driver get the correct clk and tested audio playback Change-Id: I685ded1607c2c7edc5e48f0bada258ebde192bb8 --- M src/soc/amd/picasso/acpi/sb_fch.asl 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/44009/1 diff --git a/src/soc/amd/picasso/acpi/sb_fch.asl b/src/soc/amd/picasso/acpi/sb_fch.asl index 7623d9b..4eca52c 100644 --- a/src/soc/amd/picasso/acpi/sb_fch.asl +++ b/src/soc/amd/picasso/acpi/sb_fch.asl @@ -352,6 +352,14 @@ Name (_CRS, ResourceTemplate() { Memory32Fixed (ReadWrite, ACPIMMIO_MISC_BASE, 0x100) }) + Name (_DSD, Package () + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package () { "is-rv", 1 }, + }, + }) Method (_STA, 0x0, NotSerialized) { Return (0x0F) -- To view, visit https://review.coreboot.org/c/coreboot/+/44009 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I685ded1607c2c7edc5e48f0bada258ebde192bb8 Gerrit-Change-Number: 44009 Gerrit-PatchSet: 1 Gerrit-Owner: Akshu Agrawal <akshu.agrawal(a)amd.corp-partner.google.com> Gerrit-Reviewer: Akshu Agrawal <akshu.agrawal(a)amd.com> Gerrit-MessageType: newchange

2 7

Change in coreboot[master]: soc/intel/skylake: acpi: drop HWP's dependency on EIST
by Michael Niewöhner (Code Review) 03 Aug '20

03 Aug '20

Hello Felix Singer, build bot (Jenkins), Matt Delco, Nico Huber, Paul Menzel, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44137 to look at the new patch set (#3). Change subject: soc/intel/skylake: acpi: drop HWP's dependency on EIST ...................................................................... soc/intel/skylake: acpi: drop HWP's dependency on EIST Enhanced Intel SpeedStep Technology (EIST) and Intel Speed Shift Technology (ISST) - also know as HWP - are two independent mechanisms for controlling voltage and frequency based on performance hints. When HWP is enabled, it overrides the software-based EIST. It does not depend on EIST, though, but can be enabled on it's own. Break up that currently existing dependency in ACPI generation code. It was tested that HWP can be enabled and gets used by the Linux pstate cpufreq driver. With HWP disabled, the frequency does not decrease, even not in powersave mode. After enabling HWP the frequency changed in relation to the current workload. Change-Id: I93d888ddce7b54e91b54e5b4fdd4d9cf16630eda Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/soc/intel/skylake/acpi.c 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/44137/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/44137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93d888ddce7b54e91b54e5b4fdd4d9cf16630eda Gerrit-Change-Number: 44137 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/skylake: acpi: drop HWP's dependency on EIST
by Michael Niewöhner (Code Review) 03 Aug '20

03 Aug '20

Hello Felix Singer, Matt Delco, Nico Huber, Paul Menzel, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44137 to look at the new patch set (#2). Change subject: soc/intel/skylake: acpi: drop HWP's dependency on EIST ...................................................................... soc/intel/skylake: acpi: drop HWP's dependency on EIST Enhanced Intel SpeedStep Technology (EIST) and Intel Speed Shift Technology (ISST) - also know as HWP - are two independent mechanisms for controlling voltage and frequency based on performance hints. When HWP is enabled, it overrides the software-based EIST. It does not depend on EIST, though, but can be enabled on it's own. Break up that currently exisiting dependency in ACPI generation code. It was tested that HWP can be enabled and gets used by the Linux pstate cpufreq driver. With HWP disabled, the frequency does not decrease, even not in powersave mode. After enabling HWP the frequency changed in relation to the current workload. Change-Id: I93d888ddce7b54e91b54e5b4fdd4d9cf16630eda Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/soc/intel/skylake/acpi.c 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/44137/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/44137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93d888ddce7b54e91b54e5b4fdd4d9cf16630eda Gerrit-Change-Number: 44137 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/skylake: acpi: drop HWP's dependency on EIST
by Michael Niewöhner (Code Review) 03 Aug '20

03 Aug '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44137 ) Change subject: soc/intel/skylake: acpi: drop HWP's dependency on EIST ...................................................................... Patch Set 1: CPPC code should be moved to common acpi code later (after migration of skl) -- To view, visit https://review.coreboot.org/c/coreboot/+/44137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93d888ddce7b54e91b54e5b4fdd4d9cf16630eda Gerrit-Change-Number: 44137 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 03 Aug 2020 14:59:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/**/{devicetree,overridetree}.cb: Indent with tabs
by Paul Menzel (Code Review) 03 Aug '20

03 Aug '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44113 ) Change subject: mb/**/{devicetree,overridetree}.cb: Indent with tabs ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/44113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic3d61f5210d21d9613fc50b47b90af71f544169a Gerrit-Change-Number: 44113 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 03 Aug 2020 14:35:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/volteer: pull-up GPP_D16 instead of driving it
by Paul Menzel (Code Review) 03 Aug '20

03 Aug '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44116 ) Change subject: mb/google/volteer: pull-up GPP_D16 instead of driving it ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/44116/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/44116/1//COMMIT_MSG@7 PS1, Line 7: pull-up Pull up -- To view, visit https://review.coreboot.org/c/coreboot/+/44116 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I096d76ec12b7c3afaf02e621fd301b6704913d5d Gerrit-Change-Number: 44116 Gerrit-PatchSet: 1 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(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)users.sourceforge.net> Gerrit-Comment-Date: Mon, 03 Aug 2020 14:32:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/volteer/*/gpio.c: add GPP_D16 to early_gpio_table
by Caveh Jalali (Code Review) 03 Aug '20

03 Aug '20

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44117 ) Change subject: mb/google/volteer/*/gpio.c: add GPP_D16 to early_gpio_table ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/44117 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icf5e770f540e5d1e27b40f270bb004f4196bc7be Gerrit-Change-Number: 44117 Gerrit-PatchSet: 1 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Mon, 03 Aug 2020 11:31:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2020