coreboot-gerrit September 2022

coreboot-gerrit@coreboot.org

1 participants
4678 discussions

[S] Change in coreboot[master]: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling
by Angel Pons (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Jamie Ryu, Subrata Banik, Tim Wawrzynczak, Kapil Porwal, Ivy Jian, Arthur Heymans, Lean Sheng Tan, Eric Lai, Raj Astekar. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67273 ) Change subject: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > This seems like a bug in depthcharge then, as it seems to be making an assumption about the state of the HW (i.e. BM bit set) before it uses it? If depthcharge is using DMA then IMHO it is responsible for configuring it correctly, including setting BM. > > Isn't that the case with depthcharge always? I remember one issue in CML days with SATA when FSP missed to configure the BM and we were seeing booting issue. Finally, we are seeing the similar issue in MTL again with IGD. Its better we configure required setting in coreboot being independent of FSP is my intention here. We should consider the option of "why not just fix depthcharge" as well. That being said, if depthcharge doesn't "know" that this BM bit is not set (e.g. depthcharge tries to use the framebuffer through the info in coreboot tables and doesn't touch the PCI device), then I'd say coreboot should enable BM. However, in the case of SATA controllers, payloads often scan PCI devices to find SATA controllers, so setting the BM bit on these SATA controllers is the payload's responsibility. -- To view, visit https://review.coreboot.org/c/coreboot/+/67273 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ad9eee8379b7ea1e50224e3fabb347e5f14c25b Gerrit-Change-Number: 67273 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Comment-Date: Thu, 01 Sep 2022 16:17:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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

[S] Change in coreboot[master]: mb/google/skyrim/var/winterhold: Add gpio override settings
by Raul Rangel (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Jason Nien, EricKY Cheng, Martin Roth, Karthikeyan Ramasubramanian. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67209 ) Change subject: mb/google/skyrim/var/winterhold: Add gpio override settings ...................................................................... Patch Set 4: (3 comments) File src/mainboard/google/skyrim/variants/winterhold/gpio.c: https://review.coreboot.org/c/coreboot/+/67209/comment/d6259396_c14c6932 PS4, Line 10: // NC Remove these extra comments. The PAD_NC already makes it clear https://review.coreboot.org/c/coreboot/+/67209/comment/81818e82_6d167530 PS4, Line 22: /* CLK_REQ1_L / EMMC */ How about this? https://review.coreboot.org/c/coreboot/+/67209/comment/b5681c41_8949e17e PS4, Line 25: __weak You don't want to make this weak -- To view, visit https://review.coreboot.org/c/coreboot/+/67209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2086c326cbf46ba6378d18d37dcbbe9fafa6b2bc Gerrit-Change-Number: 67209 Gerrit-PatchSet: 4 Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Comment-Date: Thu, 01 Sep 2022 16:03:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/guybrush: enable display backlight in ramstage
by Raul Rangel (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Jason Nien, Matt DeVillier, Rob Barnes, Karthik Ramasubramanian. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67246 ) Change subject: mb/google/guybrush: enable display backlight in ramstage ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/guybrush/variants/baseboard/gpio.c: https://review.coreboot.org/c/coreboot/+/67246/comment/04b357c5_50ebc524 PS1, Line 141: PAD_GPO > Martin's recollection was that the ACPI code is required in order to have the backlight level restor […] The pin seems to be in the S5 domain, so it should retain its value -- To view, visit https://review.coreboot.org/c/coreboot/+/67246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2519d779954ed89486045aa7de0b18f1c31a4374 Gerrit-Change-Number: 67246 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 01 Sep 2022 16:00:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: src/device: Clear lane error status
by Jonathan Zhang (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Wilson Chou, Marc Jones, Nico Huber, Ryback Hung, Johnny Lin, Tim Wawrzynczak, Shuming Chu (Shuming). Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67264 ) Change subject: src/device: Clear lane error status ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Based on where Wilson has put the call to clear_lane_error_status(), I am guessing that it is the li […] We need to clear lane error status after (last) link training triggered by coreboot. Otherwise lspci would show (erroneous) lane error when booted from coreboot. This issue was surfaced on xeon-sp server. We do not know whether it applies to other mainboard, hence added a Kconfig. -- To view, visit https://review.coreboot.org/c/coreboot/+/67264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6344223636409d8fc25e365a6375fc81e69f41a5 Gerrit-Change-Number: 67264 Gerrit-PatchSet: 2 Gerrit-Owner: Wilson Chou <wilson.chou%quantatw.com(a)gtempaccount.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Ryback Hung <ryback.hung(a)quantatw.com> Gerrit-Reviewer: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Wilson Chou <wilson.chou%quantatw.com(a)gtempaccount.com> Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Ryback Hung <ryback.hung(a)quantatw.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Comment-Date: Thu, 01 Sep 2022 15:51:51 +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

[S] Change in coreboot[master]: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling
by Subrata Banik (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Jamie Ryu, Tim Wawrzynczak, Kapil Porwal, Ivy Jian, Arthur Heymans, Lean Sheng Tan, Eric Lai, Raj Astekar. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67273 ) Change subject: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > This seems like a bug in depthcharge then, as it seems to be making an assumption about the state of the HW (i.e. BM bit set) before it uses it? If depthcharge is using DMA then IMHO it is responsible for configuring it correctly, including setting BM. Isn't that the case with depthcharge always? I remember one issue in CML days with SATA when FSP missed to configure the BM and we were seeing booting issue. Finally, we are seeing the similar issue in MTL again with IGD. Its better we configure required setting in coreboot being independent of FSP is my intention here. -- To view, visit https://review.coreboot.org/c/coreboot/+/67273 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ad9eee8379b7ea1e50224e3fabb347e5f14c25b Gerrit-Change-Number: 67273 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Comment-Date: Thu, 01 Sep 2022 15:31:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: util/docker/coreboot-jenkins-node: Install cmocka
by Nico Huber (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Felix Singer, Martin L Roth. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67272 ) Change subject: util/docker/coreboot-jenkins-node: Install cmocka ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > The coreboot-sdk coreboot build will continue to use 3rdparty/cmocka, I guess. […] It seems a little odd, all the other flashrom dependencies are also in coreboot-sdk. But tests are optional and coreboot builds anyway, so this is not a blocker. -- To view, visit https://review.coreboot.org/c/coreboot/+/67272 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c168e480d6f4cbfbbd175ecb035c88bfcbac00b Gerrit-Change-Number: 67272 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Thu, 01 Sep 2022 15:28:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: util/spd_tools: Rebuild utils when source changes
by Robert Zieba (Code Review) 01 Sep '22

01 Sep '22

Attention is currently required from: Caveh Jalali, Reka Norman. Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67266 ) Change subject: util/spd_tools: Rebuild utils when source changes ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/67266 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4775fe0e00e0f5d4f8b4b47331d836aba53c0e69 Gerrit-Change-Number: 67266 Gerrit-PatchSet: 1 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Comment-Date: Thu, 01 Sep 2022 15:03:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[refs/meta/config]: Replace -is:draft
by Patrick Georgi (Code Review) 01 Sep '22

01 Sep '22

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67281 ) Change subject: Replace -is:draft ...................................................................... Replace -is:draft That flag has been retired by Gerrit a long time ago and was replaced with -is:private and -is:wip for different aspects of what a draft change has been previously. Change-Id: Id4c569af1ad559fc87db55e76a8ca36d893e163d Reviewed-on: https://review.coreboot.org/c/coreboot/+/67281 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin L Roth <gaumless(a)gmail.com> --- M project.config 1 file changed, 17 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Martin L Roth: Looks good to me, approved diff --git a/project.config b/project.config index 19161bb..128a402 100644 --- a/project.config +++ b/project.config @@ -38,4 +38,4 @@ [notify "coreboot-gerrit"] header = to email = coreboot-gerrit(a)coreboot.org - filter = -is:draft + filter = -is:private -is:wip -- To view, visit https://review.coreboot.org/c/coreboot/+/67281 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: refs/meta/config Gerrit-Change-Id: Id4c569af1ad559fc87db55e76a8ca36d893e163d Gerrit-Change-Number: 67281 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2022