Attention is currently required from: Arthur Heymans, Christian Walter, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Held, Jeff Daly, Johnny Lin, Kapil Porwal, Martin L Roth, Nico Huber, Subrata Banik, Tarun, Tim Chu, Vanessa Eusebio.
Julius Werner 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:
> the x86 socs mainly use integers for mmio addresses while the arm socs mainly use pointers for mmio […]
Acknowledged
--
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: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Sat, 24 Feb 2024 01:54:16 +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: Arthur Heymans, Michael Niewöhner, Paul Menzel.
Nicholas Sudsgaard has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80727?usp=email )
Change subject: include/device/azalia_device.h: Correct location2 shift to 28 bits
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80727/comment/69891b19_171c0f56 :
PS2, Line 7: include/device/azalia_device.h: Fix incorrect bit shift
> please add Intel HDA Specification as reference (Rev. 1. […]
Done
https://review.coreboot.org/c/coreboot/+/80727/comment/07e7c8e5_7f1c04a7 :
PS2, Line 7: Fix incorrect bit shift
> Maybe more specific: […]
Done
https://review.coreboot.org/c/coreboot/+/80727/comment/5bfa90ea_e9355cf1 :
PS2, Line 8:
> Please mention the source, that this is the correct value.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80727?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: Ia5a3431b70783cb88e866d0fd8ea5530100f3d52
Gerrit-Change-Number: 80727
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Fri, 23 Feb 2024 23:50:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Nicholas Sudsgaard, Paul Menzel.
Hello Arthur Heymans, Michael Niewöhner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80727?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by Paul Menzel, Code-Review+2 by Arthur Heymans, Verified+1 by build bot (Jenkins)
Change subject: include/device/azalia_device.h: Correct location2 shift to 28 bits
......................................................................
include/device/azalia_device.h: Correct location2 shift to 28 bits
The location is specified to be in range of 29:24, which is further
divided into upper bits (location2) [5:4] and lower bits (location1)
[3:0].
Intel High Definition Audio Specification, rev. 1.0a, page 178,
Figure 74. Configuration Data Structure.
TEST=Timeless build using AZALIA_PIN_DESC() and without now produce the
same binary.
Change-Id: Ia5a3431b70783cb88e866d0fd8ea5530100f3d52
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
M src/include/device/azalia_device.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/80727/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80727?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: Ia5a3431b70783cb88e866d0fd8ea5530100f3d52
Gerrit-Change-Number: 80727
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Nicholas Sudsgaard.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80727?usp=email )
Change subject: include/device/azalia_device.h: Fix incorrect bit shift
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80727/comment/f8764a8f_30ba6f09 :
PS2, Line 9: TEST=Timeless build using AZALIA_PIN_DESC() and without now produce the
: same binary.
> When porting that board I decoded the values with that wrong 27 bit shift. […]
btw. this now also explains some weird values from the vendor... it was just wrong decoding 😄
--
To view, visit https://review.coreboot.org/c/coreboot/+/80727?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: Ia5a3431b70783cb88e866d0fd8ea5530100f3d52
Gerrit-Change-Number: 80727
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 22:51:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Nicholas Sudsgaard.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80727?usp=email )
Change subject: include/device/azalia_device.h: Fix incorrect bit shift
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80727/comment/8ca37c53_691be374 :
PS2, Line 7: include/device/azalia_device.h: Fix incorrect bit shift
please add Intel HDA Specification as reference (Rev. 1.0a, Figure 74)
https://review.coreboot.org/c/coreboot/+/80727/comment/49d69d1d_9e4d63bb :
PS2, Line 9: TEST=Timeless build using AZALIA_PIN_DESC() and without now produce the
: same binary.
> > The connected pins have these values: […]
When porting that board I decoded the values with that wrong 27 bit shift. That is why everything is wrong now ofc. I will push a patch to correct the board tomorrow.
Not sure about the order, though. Your patch first or mine? binaries will change either way
--
To view, visit https://review.coreboot.org/c/coreboot/+/80727?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: Ia5a3431b70783cb88e866d0fd8ea5530100f3d52
Gerrit-Change-Number: 80727
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 22:49:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier.
Forest Mittelberg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80712?usp=email )
Change subject: ec/chromeec: Enable auto fan control on startup
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80712?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: I08a8562531f8af0c71230477d0221d536443f096
Gerrit-Change-Number: 80712
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 22:28:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Leah Rowe.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80717?usp=email )
Change subject: nb/haswell: Disable iGPU when dGPU is used
......................................................................
Patch Set 2:
(1 comment)
File src/northbridge/intel/haswell/gma.c:
https://review.coreboot.org/c/coreboot/+/80717/comment/4df423cd_5e57ffd8 :
PS2, Line 469: dev->enabled = 0;
> > It looks odd indeed. I guess for the original i945 it made no difference, […]
There's still the chance that there is a GFX card in another slot. Which is,
IIRC, what the original code was written for. But, yeah, i945 is a little different.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80717?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: I1df0a3aa42f8475b7741007bf3e28c2e089d916b
Gerrit-Change-Number: 80717
Gerrit-PatchSet: 2
Gerrit-Owner: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 22:27:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Leah Rowe, Nico Huber.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80717?usp=email )
Change subject: nb/haswell: Disable iGPU when dGPU is used
......................................................................
Patch Set 2:
(1 comment)
File src/northbridge/intel/haswell/gma.c:
https://review.coreboot.org/c/coreboot/+/80717/comment/7a999253_55af9a1b :
PS2, Line 469: dev->enabled = 0;
> It looks odd indeed. I guess for the original i945 it made no difference,
> but here (and in snb) we have things like power management to configure
> (and resource allocation could be nice too).
>
> It seems worth to test again with `dev->enabled` left untouched.
On i945 it's indeed meaningless, romstage messes with IGD if the PEG slot is filled. It's not possible to use PEG and the IGD at the same time last time I tried on i945. On later gens that worked just fine, regardless of what device is decoding VGA.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80717?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: I1df0a3aa42f8475b7741007bf3e28c2e089d916b
Gerrit-Change-Number: 80717
Gerrit-PatchSet: 2
Gerrit-Owner: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Leah Rowe <leahleahrowerowe(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 22:20:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80715?usp=email )
Change subject: amdfwtool: Use Makefile.mk for Makefile settings
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
File util/amdfwtool/Makefile:
https://review.coreboot.org/c/coreboot/+/80715/comment/484dbeab_1db27942 :
PS2, Line 23: HEADER
Should this be `$(amdfwheader)`?
File util/amdfwtool/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/80715/comment/8db7c86d_3a863ffb :
PS2, Line 26: $(top)/util/amdfwtool
I think you could try to use `$(dir)` here instead of the full `$(top)/util/amdfwtool`.
(`dir` is the loop variable in the top-level Makefile when the Makefile.mk gets included)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80715?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: Id5b869f49b34b22e6a02fc086e7b42975141a87e
Gerrit-Change-Number: 80715
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 22:05:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment