Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Tarun, sridhar siricilla.
Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Tarun, build bot (Jenkins), sridhar siricilla,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80406?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by sridhar siricilla, Verified+1 by build bot (Jenkins)
Change subject: soc/intel/mtl: Increase the IgdDvmt50PreAlloc UPD size to 128MB
......................................................................
soc/intel/mtl: Increase the IgdDvmt50PreAlloc UPD size to 128MB
This patch increases the IgdDvmt50PreAlloc value as per Intel
recommendation starting with GFX PEIM 103x.
TEST=Able to build and boot google/rex.
Change-Id: I236b38a1ac5efbfcd23e373c09204d8a07b97618
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/meteorlake/romstage/fsp_params.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/80406/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80406?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I236b38a1ac5efbfcd23e373c09204d8a07b97618
Gerrit-Change-Number: 80406
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Tarun, sridhar siricilla.
Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Tarun, build bot (Jenkins), sridhar siricilla,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80361?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by sridhar siricilla, Verified+1 by build bot (Jenkins)
Change subject: soc/intel/cmn/sa: Refactor SA common code
......................................................................
soc/intel/cmn/sa: Refactor SA common code
Leverages common SA header definitions for Host Bridge registers.
Renames DSM_BASE_ADDR_REG to BDSM and DPR_REG to DPR for brevity.
TEST= Build and boot successful on google/screebo.
Change-Id: I476f213d75a0978336b3749a5ba1499107eb2238
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/include/soc/systemagent.h
M src/soc/intel/alderlake/systemagent.c
M src/soc/intel/common/block/include/intelblocks/systemagent.h
M src/soc/intel/common/block/systemagent/systemagent_def.h
M src/soc/intel/meteorlake/include/soc/systemagent.h
M src/soc/intel/meteorlake/systemagent.c
6 files changed, 23 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/80361/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80361?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I476f213d75a0978336b3749a5ba1499107eb2238
Gerrit-Change-Number: 80361
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Tarun, sridhar siricilla.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80361?usp=email )
Change subject: soc/intel/cmn/sa: Refactor SA common code
......................................................................
Patch Set 2:
(2 comments)
File src/soc/intel/alderlake/include/soc/systemagent.h:
https://review.coreboot.org/c/coreboot/+/80361/comment/5409bd8e_53f05b79 :
PS2, Line 69: #define MASK_DSM_LENGTH 0xFF00 // [15:8]
: #define MASK_DSM_LENGTH_LSB 8 // used to shift right
: #define MASK_GSM_LENGTH 0xC0 // [7:6]
: #define MASK_GSM_LENGTH_LSB 6 // used to shift right
: #define MASK_DPR_LENGTH 0xFF0 // [11:4]
: #define MASK_DPR_LENGTH_LSB 4 // used to shift right
> Hmm, did you overlook alignment (lines#69, #71, #73)?
Acknowledged
File src/soc/intel/meteorlake/include/soc/systemagent.h:
https://review.coreboot.org/c/coreboot/+/80361/comment/60f185d9_7553728c :
PS2, Line 55: #define MASK_DSM_LENGTH 0xFF00 // [15:8]
: #define MASK_DSM_LENGTH_LSB 8 // used to shift right
: #define MASK_GSM_LENGTH 0xC0 // [7:6]
: #define MASK_GSM_LENGTH_LSB 6 // used to shift right
: #define MASK_DPR_LENGTH 0xFF0 // [11:4]
: #define MASK_DPR_LENGTH_LSB 4 // used to shift right
> nit: here too,, alignment issue(lines#55, #57, #59)?
thanks for the review. I thought i have added the correct alignment. may be lost while rebasing.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80361?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I476f213d75a0978336b3749a5ba1499107eb2238
Gerrit-Change-Number: 80361
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Thu, 08 Feb 2024 10:13:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas, Nicholas Sudsgaard, Paul Menzel.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80332?usp=email )
Change subject: device/azalia: Separate codec checking and initialization
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80332?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I92b6d184abccdbe0e1bfce98a2c959a97a618a29
Gerrit-Change-Number: 80332
Gerrit-PatchSet: 9
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 08 Feb 2024 10:00:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Christian Walter, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Held, Jeff Daly, Johnny Lin, Julius Werner, Kapil Porwal, Martin L Roth, Subrata Banik, Tarun, Tim Chu, Vanessa Eusebio.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80193?usp=email )
Change subject: soc/intel: Use write{64,32,16,8}p and read{64,32,16,8}p
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Are you arguing that we can't do byte-wise arithmetic with `void *`,
No. I only mentioned it because if we'd create a new type, it should support that.
> or more generally that `void *` shouldn't point to something that's not allocated by the C runtime because of language minutiae in the standard?
That's what I had in mind when I wrote "technically ... 100% wrong". But
it's not related to my proposal. I'm not aware of any minutiae that could
bite us here.
I only proposed `mmio_addr` because people asked for type safety.
> (I think the main concern about lack of type safety is that people confuse the order of the address and value arguments, especially since we used to have `writel(val, addr)` functions in the past that had it exactly the other way around, and some other firmware projects still do. Using `void *` at least solves that problem. I think accidentally using some other random pointer is not a common mistake.)
I haven't seen any order mistake for a long time. And writel() etc. are gone
for a while. Do you think it's still a problem?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80193?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4022e3bbfacf35d0529e46dc82cf303dae9438e4
Gerrit-Change-Number: 80193
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 08 Feb 2024 09:38:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Nick Vaccaro, SH Kim, Subrata Banik.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80326?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/google/brya/var/xol: Update memory configuration
......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80326/comment/52b7272f_aa4a1850 :
PS5, Line 12: - K3KL6L60GM-MGCT
: - K3KL8L80CM-MGCT
Please add the vendor name. Maybe also do this in a separate commit, if possible.
File src/mainboard/google/brya/variants/xol/memory.c:
https://review.coreboot.org/c/coreboot/+/80326/comment/f2d3253b_11c81880 :
PS5, Line 68: .UserBd
Unrelated, but from the name, I am unable to deduce the function of the variable.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80326?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I59aabe0870317092f59701bdf88b53bf9731377a
Gerrit-Change-Number: 80326
Gerrit-PatchSet: 5
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
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: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Thu, 08 Feb 2024 09:14:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Paz Zcharya.
Hello Kapil Porwal, Nick Vaccaro, Paz Zcharya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80405?usp=email
to look at the new patch set (#3).
Change subject: soc/intel/cmn/graphics: Use DSM size for calculating GTT offset
......................................................................
soc/intel/cmn/graphics: Use DSM size for calculating GTT offset
This patch overrides the assumption being made by the GFX PEIM blob
between version 1029 and 103x to fix some issue where GTT offset
has calculated based on the eDP panel resoultion rather relying on
the pre-allocated memory limit for IGD.
Now this is causing major blocker for CrOS as GFX PEIM latest version
is no more working with existing OS (causing reboot or blank display).
To fix this issue, coreboot tries to perform an override to restore
PANEL_SURF register to the original value which has calculated based
on pre-allocated memory limit (B0/D0/F0:R 0x50 aka GGC) size.
BUG=b:314887266
TEST=Able to boot to OS where display is functional with latest FSP
(w/ GFX PEIM 1035).
Change-Id: Ied5af0faad73d0c88cab70dee6fe731f1a14653b
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/drivers/intel/gma/i915.h
M src/soc/intel/common/block/graphics/graphics.c
2 files changed, 42 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/80405/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80405?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ied5af0faad73d0c88cab70dee6fe731f1a14653b
Gerrit-Change-Number: 80405
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paz Zcharya <pazz(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: newpatchset