Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36356 )
Change subject: soc/intel/cannonlake: set LT_LOCK_MEMORY at end of POST
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36356/2/src/soc/intel/cannonlake/f…
File src/soc/intel/cannonlake/finalize.c:
https://review.coreboot.org/c/coreboot/+/36356/2/src/soc/intel/cannonlake/f…
PS2, Line 116: mp_run_on_all_cpus
given the scope of LT_LOCK_MEMORY (package), this seems unnecessary
--
To view, visit https://review.coreboot.org/c/coreboot/+/36356
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaadd4996653c4f27d268b1c4773c1e2e86114912
Gerrit-Change-Number: 36356
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Reviewer: Varun Joshi <varun.joshi(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 03 Aug 2020 22:14:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36357 )
Change subject: soc/intel/icelake: set LT_LOCK_MEMORY at end of POST
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36357/2/src/soc/intel/icelake/fina…
File src/soc/intel/icelake/finalize.c:
https://review.coreboot.org/c/coreboot/+/36357/2/src/soc/intel/icelake/fina…
PS2, Line 115: mp_run_on_all_cpus
given the scope of LT_LOCK_MEMORY (package), this seems unnecessary
--
To view, visit https://review.coreboot.org/c/coreboot/+/36357
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib728e61bc874acf505348e72c00a99e0a6efd2cb
Gerrit-Change-Number: 36357
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 03 Aug 2020 22:14:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36357 )
Change subject: soc/intel/icelake: set LT_LOCK_MEMORY at end of POST
......................................................................
Patch Set 2:
> Patch Set 2: -Code-Review
>
> Thinking about this, MP init works different with FSP2.1, so it
> should be tested, IMHO. Might even be a no-op.
SkipMpInit was moved to FSP-M; Analysis of the most recent icl Fsp showed that the same applies to icl as does for skl or cnl in regard to LT_MEMORY_LOCK -> the patch is required, when SkipMpInitPreMem=1.
Could anyone test this please?
Without this patch and native mp init, msr 0x2e7 should be 0x0.
With this patch applied 0x2e7 should be 0x1.
--
To view, visit https://review.coreboot.org/c/coreboot/+/36357
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib728e61bc874acf505348e72c00a99e0a6efd2cb
Gerrit-Change-Number: 36357
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
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 21:30:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43494 )
Change subject: soc/intel/common/cpu: Update COS mask calculation for nem enhanced mode
......................................................................
Patch Set 3:
(4 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/43494/3//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/43494/3//COMMIT_MSG@23
PS3, Line 23: mas
maps
https://review.coreboot.org/c/coreboot/+/43494/3/src/soc/intel/common/block…
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
PS3:
Why bother to set the other masks (MASK_0 & MASK_1) if they're unused (PQR_ASSOC set to pick mask #2)
https://review.coreboot.org/c/coreboot/+/43494/3/src/soc/intel/common/block…
PS3, Line 425: 0x02
0x200000000
https://review.coreboot.org/c/coreboot/+/43494/3/src/soc/intel/common/block…
PS3, Line 445: 0x01
0x100000000
--
To view, visit https://review.coreboot.org/c/coreboot/+/43494
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I54e047161853bfc70516c1d607aa479e68836d04
Gerrit-Change-Number: 43494
Gerrit-PatchSet: 3
Gerrit-Owner: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Usha P <usha.p(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 03 Aug 2020 21:00:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28950 )
Change subject: lenovo/x230: introduce FHD variant
......................................................................
Patch Set 12:
> Patch Set 12:
>
> Sorry about omitting OS info -- tested on Linux 5.7.11 (Arch, same behavior on KDE Plasma, Gnome, and LXDE)
>
> > Patch Set 12:
> >
> > > Patch Set 12:
> > >
> > > I'm seeing an issue where zero-brightness (Backlight off) is being treated as full brightness. So instead of turning off the backlight when the lid is closed, it's set to max.
> > >
> > > Tested with patchset 12 on master @ e2f5fb25
> >
> > Which OS did you test this on?
Intel backlight handling on Linux is rather messed up. There's multiple ways to do it, and there's quirk handling (which is itself a quirk).
--
To view, visit https://review.coreboot.org/c/coreboot/+/28950
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0355d39a61956792e69bccd5274cfc2749d72bf0
Gerrit-Change-Number: 28950
Gerrit-PatchSet: 12
Gerrit-Owner: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kilian Neuner <cb(a)9-r.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Peter Lemenkov <lemenkov(a)gmail.com>
Gerrit-Reviewer: Richard Slindee
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Aleksei Kharlamov <derlafff(a)ya.ru>
Gerrit-CC: Christian Herzog
Gerrit-CC: Holger Levsen <holger(a)layer-acht.org>
Gerrit-CC: Matthias Wiedhalm
Gerrit-CC: Pavel Kovalenko <su(a)nitrocaster.me>
Gerrit-CC: Sergey Alirzaev <zl29ah(a)gmail.com>
Gerrit-CC: Tom Hiller <thrilleratplay(a)gmail.com>
Gerrit-CC: Tomasz Jan Góralczyk
Gerrit-CC: clayton craft
Gerrit-CC: slact
Gerrit-Comment-Date: Mon, 03 Aug 2020 21:00:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
slact has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28950 )
Change subject: lenovo/x230: introduce FHD variant
......................................................................
Patch Set 12:
Sorry about omitting OS info -- tested on Linux 5.7.11 (Arch, same behavior on KDE Plasma, Gnome, and LXDE)
> Patch Set 12:
>
> > Patch Set 12:
> >
> > I'm seeing an issue where zero-brightness (Backlight off) is being treated as full brightness. So instead of turning off the backlight when the lid is closed, it's set to max.
> >
> > Tested with patchset 12 on master @ e2f5fb25
>
> Which OS did you test this on?
--
To view, visit https://review.coreboot.org/c/coreboot/+/28950
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0355d39a61956792e69bccd5274cfc2749d72bf0
Gerrit-Change-Number: 28950
Gerrit-PatchSet: 12
Gerrit-Owner: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kilian Neuner <cb(a)9-r.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Peter Lemenkov <lemenkov(a)gmail.com>
Gerrit-Reviewer: Richard Slindee
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Aleksei Kharlamov <derlafff(a)ya.ru>
Gerrit-CC: Christian Herzog
Gerrit-CC: Holger Levsen <holger(a)layer-acht.org>
Gerrit-CC: Matthias Wiedhalm
Gerrit-CC: Pavel Kovalenko <su(a)nitrocaster.me>
Gerrit-CC: Sergey Alirzaev <zl29ah(a)gmail.com>
Gerrit-CC: Tom Hiller <thrilleratplay(a)gmail.com>
Gerrit-CC: Tomasz Jan Góralczyk
Gerrit-CC: clayton craft
Gerrit-CC: slact
Gerrit-Comment-Date: Mon, 03 Aug 2020 20:56:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41758 )
Change subject: mb/intel/jasperlake_rvp: Replace static camera ACPI by driver
......................................................................
Patch Set 25: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/41758/25//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/41758/25//COMMIT_MSG@11
PS25, Line 11:
I believe this also adds support for CAM0, which was not the previous ASL.
--
To view, visit https://review.coreboot.org/c/coreboot/+/41758
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib439572bc1d15ef02c86c7bfa88af6b16eb06f97
Gerrit-Change-Number: 41758
Gerrit-PatchSet: 25
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Daniel Kang <daniel.h.kang(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kiran2 Kumar <kiran2.kumar(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Delco <delco(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 03 Aug 2020 20:41:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44121 )
Change subject: nb/intel/sandybridge: Refactor `get_pcie_bar`
......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44121/4/src/northbridge/intel/sand…
File src/northbridge/intel/sandybridge/northbridge.c:
https://review.coreboot.org/c/coreboot/+/44121/4/src/northbridge/intel/sand…
PS4, Line 50: return 0;
> It might be personal preference but I think return 1 on error is more common.
The other instances of `decode_pcie_bar` return 0 on failure. I'd rather be consistent with the rest of the northbridges. Did you notice a certain repeating pattern across all of them? 😄
https://review.coreboot.org/c/coreboot/+/44121/4/src/northbridge/intel/sand…
PS4, Line 143: decode_pcie_bar
> if this return an 'error' it should probably die given the pci mmconfig accesses done pretty much ev […]
I agree. It shouldn't error out because we're programming MMCONF in the bootblock. I'd rather clean this up later, though.
--
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: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.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 20:13:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment